Support generic types as contract keys in codegen (digital-asset#1743)

Fixes digital-asset#1728
FlashSheridan · Jun 18, 2019 · 02e9503 · 02e9503
1 parent a15d9e2
commit 02e9503
Show file tree

Hide file tree

Showing 10 changed files with 183 additions and 24 deletions.
diff --git a/docs/source/support/release-notes.rst b/docs/source/support/release-notes.rst
@@ -9,6 +9,12 @@ This page contains release notes for the SDK.
 HEAD — ongoing
 --------------
 
+Java Codegen
+~~~~~~~~~~~~
+
+- Support generic types (including tuples) as contract keys in codegen.
+  See `#1728 <https://github.com/digital-asset/daml/issues/1728>`__.
+
 .. _release-0-13-3:
 
 0.13.3 - 2019-06-18

diff --git a/language-support/java/codegen/src/it/daml/Lib.daml b/language-support/java/codegen/src/it/daml/Lib.daml
@@ -13,4 +13,5 @@ import Tests.VariantTest
 import Tests.Import
 import Tests.Escape
 import Tests.MapTest
-import Tests.Serializable
+import Tests.Serializable
+import Tests.ContractKeys
diff --git a/language-support/java/codegen/src/it/daml/Tests/ContractKeys.daml b/language-support/java/codegen/src/it/daml/Tests/ContractKeys.daml
@@ -0,0 +1,81 @@
+-- Copyright (c) 2019 Digital Asset (Switzerland) GmbH and/or its affiliates. All rights reserved.
+-- SPDX-License-Identifier: Apache-2.0
+
+daml 1.2
+module Tests.ContractKeys where
+
+data PartyAndInt =
+  PartyAndInt
+    with
+      party: Party
+      int: Int
+
+template NoKey
+  with
+    owner : Party
+  where
+    signatory owner
+
+    controller owner can
+      NoKey_Choice : ()
+        do return ()
+
+template PartyKey
+  with
+    owner : Party
+  where
+    signatory owner
+
+    key owner: Party
+    maintainer key
+
+    controller owner can
+      PartyKey_Choice : ()
+        do return ()
+
+template RecordKey
+  with
+    owner : Party
+    number : Int
+  where
+    signatory owner
+
+    key PartyAndInt owner number : PartyAndInt
+    maintainer key.party
+
+    controller owner can
+      RecordKey_Choice : ()
+        do return ()
+
+template TupleKey
+  with
+    owner : Party
+    number : Int
+  where
+    signatory owner
+
+    key (owner, number): (Party, Int)
+    maintainer key._1
+
+    controller owner can
+      TupleKey_Choice : ()
+        do return ()
+
+template NestedTupleKey
+  with
+    t1_1 : Party
+    t1_2 : Int
+    t1_3 : Text
+    t2_1 : Int
+    t2_2 : Bool
+    t2_3 : Text
+    t2_4 : Int
+  where
+    signatory t1_1
+
+    key ((t1_1, t1_2, t1_3), (t2_1, t2_2, t2_3, t2_4)): ((Party, Int, Text), (Int, Bool, Text, Int))
+    maintainer key._1._1
+
+    controller t1_1 can
+      NestedTupleKey_Choice : ()
+        do return ()
diff --git a/language-support/java/codegen/src/it/java-latest/com/digitalasset/lf_latest/AllTests.java b/language-support/java/codegen/src/it/java-latest/com/digitalasset/lf_latest/AllTests.java
@@ -10,6 +10,7 @@
 @RunWith(Suite.class)
 @Suite.SuiteClasses({
         MapTest.class,
+        ContractKeysTest.class,
         AllGenericTests.class
 })
 public class AllTests {

diff --git a/...-support/java/codegen/src/it/java-latest/com/digitalasset/lf_latest/ContractKeysTest.java b/...-support/java/codegen/src/it/java-latest/com/digitalasset/lf_latest/ContractKeysTest.java
@@ -0,0 +1,71 @@
+// Copyright (c) 2019 Digital Asset (Switzerland) GmbH and/or its affiliates. All rights reserved.
+// SPDX-License-Identifier: Apache-2.0
+
+package com.digitalasset.lf_latest;
+
+import da.types.Tuple2;
+import da.types.Tuple3;
+import da.types.Tuple4;
+import org.junit.jupiter.api.Test;
+import org.junit.platform.commons.util.ReflectionUtils;
+import org.junit.platform.runner.JUnitPlatform;
+import org.junit.runner.RunWith;
+import tests.contractkeys.*;
+
+import java.util.Optional;
+
+import static org.junit.jupiter.api.Assertions.*;
+
+@RunWith(JUnitPlatform.class)
+public class ContractKeysTest {
+
+    // NOTE: the tests are mostly here to make sure the code compiles
+
+    NoKey.Contract noKey = new NoKey.Contract(new NoKey.ContractId("no-key"), new NoKey("Alice"), Optional.empty());
+    PartyKey.Contract partyKey = new PartyKey.Contract(new PartyKey.ContractId("party-key"), new PartyKey("Alice"), Optional.empty(), Optional.of("Alice"));
+    RecordKey.Contract recordKey = new RecordKey.Contract(new RecordKey.ContractId("record-key"), new RecordKey("Alice", 42L), Optional.empty(), Optional.of(new PartyAndInt("Alice", 42L)));
+    TupleKey.Contract tupleKey = new TupleKey.Contract(new TupleKey.ContractId("tuple-key"), new TupleKey("Alice", 42L), Optional.empty(), Optional.of(new Tuple2<>("Alice", 42L)));
+    NestedTupleKey.Contract nestedTupleKey = new NestedTupleKey.Contract(new NestedTupleKey.ContractId("nested-tuple-key"), new NestedTupleKey("Alice", 42L, "blah", 47L, true, "foobar", 0L), Optional.empty(), Optional.of(new Tuple2<>(new Tuple3<>("Alice", 42L, "blah"), new Tuple4<>(47L, true, "foobar", 0L))));
+
+    @Test
+    void noKeyHasNoKey() {
+        assertFalse(ReflectionUtils.readFieldValue(NoKey.Contract.class, "key", noKey).isPresent());
+    }
+
+    @Test
+    void allOthersHaveKeys() {
+        assertTrue(partyKey.key.isPresent());
+        assertTrue(recordKey.key.isPresent());
+        assertTrue(tupleKey.key.isPresent());
+        assertTrue(nestedTupleKey.key.isPresent());
+    }
+
+    @Test
+    void partyKeyIsString() {
+        assertEquals(partyKey.key.get(), "Alice");
+    }
+
+    @Test
+    void recordKeyIsPartyAndInt() {
+        assertEquals(recordKey.key.get().party, "Alice");
+        assertEquals(recordKey.key.get().int$.longValue(), 42L);
+    }
+
+    @Test
+    void tupleKeyIsPartyAndInt() {
+        assertEquals(tupleKey.key.get()._1, "Alice");
+        assertEquals(tupleKey.key.get()._2.longValue(), 42L);
+    }
+
+    @Test
+    void nestedTupleKeyIsPartyAndInt() {
+        assertEquals(nestedTupleKey.key.get()._1._1, "Alice");
+        assertEquals(nestedTupleKey.key.get()._1._2.longValue(), 42L);
+        assertEquals(nestedTupleKey.key.get()._1._3, "blah");
+        assertEquals(nestedTupleKey.key.get()._2._1.longValue(), 47L);
+        assertEquals(nestedTupleKey.key.get()._2._2.booleanValue(), true);
+        assertEquals(nestedTupleKey.key.get()._2._3, "foobar");
+        assertEquals(nestedTupleKey.key.get()._2._4.longValue(), 0L);
+    }
+
+}
diff --git a/language-support/java/codegen/src/it/java/com/digitalasset/AllGenericTests.java b/language-support/java/codegen/src/it/java/com/digitalasset/AllGenericTests.java
@@ -15,7 +15,9 @@
         DecoderTest.class,
         RecordTest.class,
         ListTest.class,
-        VariantTest.class
+        VariantTest.class,
+        SerializableTest.class,
+        TemplateMethodTest.class
 })
 public class AllGenericTests {
 }
diff --git a/language-support/java/codegen/src/it/java/com/digitalasset/SerializableTest.java b/language-support/java/codegen/src/it/java/com/digitalasset/SerializableTest.java
@@ -3,7 +3,6 @@
 
 package com.digitalasset;
 
-import com.daml.ledger.javaapi.TestDecoder;
 import com.daml.ledger.javaapi.data.Int64;
 import com.daml.ledger.javaapi.data.Record;
 import org.junit.jupiter.api.Assertions;
@@ -12,8 +11,6 @@
 import org.junit.runner.RunWith;
 import tests.serializable.serializability.Serializable;
 
-import static org.junit.jupiter.api.Assertions.assertTrue;
-
 @RunWith(JUnitPlatform.class)
 public class SerializableTest {
 

diff --git a/...c/main/scala/com/digitalasset/daml/lf/codegen/backend/java/inner/FromValueGenerator.scala b/...c/main/scala/com/digitalasset/daml/lf/codegen/backend/java/inner/FromValueGenerator.scala
@@ -97,7 +97,7 @@ private[inner] object FromValueGenerator extends StrictLogging {
       "$T $L = $L",
       toJavaTypeName(fieldType, packagePrefixes),
       field,
-      composite(fieldType, field, accessor, Iterator.from(0).map(n => s"v$$$n"), packagePrefixes))
+      extractor(fieldType, field, accessor, newNameGenerator, packagePrefixes))
 
   // Primitive extractors that map a type to the method to retrieve it
   // Missing on purpose: `Record` and `Variant` -- those should be dealt
@@ -140,14 +140,13 @@ private[inner] object FromValueGenerator extends StrictLogging {
 
   /**
     * Generates extractor for types that are not immediately covered by primitive extractors
-    * Relies on the underlying [[nested]] method to generate extractor recursively
-    * @param typeName The type of the field being accessed
+    * @param damlType The type of the field being accessed
     * @param field The name of the field being accessed
     * @param accessor The [[CodeBlock]] that defines how to access the item in the first place
-    * @param args An iterator providing diverse argument names to be used in [[nested]]
+    * @param args An iterator providing argument names for nested calls without shadowing
     * @return A [[CodeBlock]] that defines the extractor for the whole composite type
     */
-  private def composite(
+  private[inner] def extractor(
       damlType: Type,
       field: String,
       accessor: CodeBlock,
@@ -176,7 +175,7 @@ private[inner] object FromValueGenerator extends StrictLogging {
             .of("$L.getValues().stream().map($L -> ", optMapArg, listMapArg))
           .add(CodeBlock.of(
             "$L",
-            composite(param, listMapArg, CodeBlock.of("$L", listMapArg), args, packagePrefixes)))
+            extractor(param, listMapArg, CodeBlock.of("$L", listMapArg), args, packagePrefixes)))
           .add(
             CodeBlock
               .of(
@@ -197,7 +196,7 @@ private[inner] object FromValueGenerator extends StrictLogging {
             outerOptArg,
             outerOptArg,
             innerOptArg,
-            composite(param, innerOptArg, CodeBlock.of("$L", innerOptArg), args, packagePrefixes)
+            extractor(param, innerOptArg, CodeBlock.of("$L", innerOptArg), args, packagePrefixes)
           ))
           .add(orElseThrow(apiType, field))
           .build()
@@ -225,7 +224,7 @@ private[inner] object FromValueGenerator extends StrictLogging {
           .add(
             CodeBlock.of(
               "$L",
-              composite(
+              extractor(
                 param,
                 entryArg,
                 CodeBlock.of("$L.getValue()", entryArg),
@@ -248,7 +247,7 @@ private[inner] object FromValueGenerator extends StrictLogging {
           toJavaTypeName(targ, packagePrefixes) -> CodeBlock.of(
             "$L -> $L",
             innerArg,
-            composite(targ, field, CodeBlock.of("$L", innerArg), args, packagePrefixes))
+            extractor(targ, field, CodeBlock.of("$L", innerArg), args, packagePrefixes))
         }.unzip
 
         val targsCode = CodeBlock.join(targs.map(CodeBlock.of("$L", _)).asJava, ", ")

diff --git a/...en/src/main/scala/com/digitalasset/daml/lf/codegen/backend/java/inner/TemplateClass.scala b/...en/src/main/scala/com/digitalasset/daml/lf/codegen/backend/java/inner/TemplateClass.scala
@@ -126,7 +126,9 @@ private[inner] object TemplateClass extends StrictLogging {
           contractClassName,
           templateClassName,
           contractIdClassName,
-          contractKeyClassName))
+          key,
+          packagePrefixes
+        ))
       .addMethods(ObjectMethods(contractClassName, fields, templateClassName).asJava)
       .build()
   }
@@ -200,14 +202,17 @@ private[inner] object TemplateClass extends StrictLogging {
   private val getContractId = CodeBlock.of("event.getContractId()")
   private val getArguments = CodeBlock.of("event.getArguments()")
   private val getAgreementText = CodeBlock.of("event.getAgreementText()")
-  private def getContractKey(t: TypeName) =
-    CodeBlock.of("event.getContractKey().map($T::fromValue)", t)
+  private def getContractKey(t: Type, packagePrefixes: Map[PackageId, String]) =
+    CodeBlock.of(
+      "event.getContractKey().map(e -> $L)",
+      FromValueGenerator.extractor(t, "e", CodeBlock.of("e"), newNameGenerator, packagePrefixes))
 
   private[inner] def generateFromCreatedEvent(
       className: ClassName,
       templateClassName: ClassName,
       idClassName: ClassName,
-      maybeContractKeyClassName: Option[TypeName]) = {
+      maybeContractKeyType: Option[Type],
+      packagePrefixes: Map[PackageId, String]) = {
 
     val spec =
       MethodSpec
@@ -216,8 +221,8 @@ private[inner] object TemplateClass extends StrictLogging {
         .returns(className)
         .addParameter(classOf[CreatedEvent], "event")
 
-    val params = Vector(getContractId, getArguments, getAgreementText) ++ maybeContractKeyClassName
-      .map(getContractKey)
+    val params = Vector(getContractId, getArguments, getAgreementText) ++ maybeContractKeyType
+      .map(getContractKey(_, packagePrefixes))
       .toList
 
     spec.addStatement("return fromIdAndRecord($L)", CodeBlock.join(params.asJava, ", ")).build()

diff --git a/...src/main/scala/com/digitalasset/daml/lf/codegen/backend/java/inner/ToValueGenerator.scala b/...src/main/scala/com/digitalasset/daml/lf/codegen/backend/java/inner/ToValueGenerator.scala
@@ -10,9 +10,7 @@ import com.digitalasset.daml.lf.data.ImmArray.ImmArraySeq
 import com.digitalasset.daml.lf.data.Ref.PackageId
 import com.digitalasset.daml.lf.iface._
 import com.squareup.javapoet._
-import com.typesafe.scalalogging.Logger
 import javax.lang.model.element.Modifier
-import org.slf4j.LoggerFactory
 
 import scala.collection.JavaConverters._
 
@@ -32,8 +30,6 @@ import scala.collection.JavaConverters._
 @SuppressWarnings(Array("org.wartremover.warts.Option2Iterable"))
 object ToValueGenerator {
 
-  private val logger: Logger = Logger(LoggerFactory.getLogger(getClass.getName))
-
   import Types._
 
   def generateToValueForRecordLike(