ledger-api: Kill legacy identifier (digital-asset#2211)

* ledger-api: drop support for legacy identifier * address scala-codegen: Stephen's comments * language support: fix test
TuranDev · Jul 23, 2019 · 53de5ed · 53de5ed
1 parent d0dd201
commit 53de5ed
Show file tree

Hide file tree

Showing 60 changed files with 215 additions and 820 deletions.
diff --git a/daml-lf/engine/src/main/scala/com/digitalasset/daml/lf/engine/DeprecatedIdentifier.scala b/daml-lf/engine/src/main/scala/com/digitalasset/daml/lf/engine/DeprecatedIdentifier.scala
diff --git a/extractor/src/main/scala/com/digitalasset/extractor/ledger/types/LedgerValue.scala b/extractor/src/main/scala/com/digitalasset/extractor/ledger/types/LedgerValue.scala
@@ -92,7 +92,7 @@ object LedgerValue {
 
   private def convertIdentifier(
       apiIdentifier: api.value.Identifier): String \/ Option[Ref.Identifier] = {
-    val api.value.Identifier(packageId, _, moduleName, entityName) = apiIdentifier
+    val api.value.Identifier(packageId, moduleName, entityName) = apiIdentifier
     some(packageId)
       .filter(_.nonEmpty)
       .traverseU { _ =>

diff --git a/language-support/hs/bindings/src/DA/Ledger/Convert.hs b/language-support/hs/bindings/src/DA/Ledger/Convert.hs
@@ -101,7 +101,6 @@ lowerIdentifier = \case
     Identifier{..} ->
         LL.Identifier {
         identifierPackageId = unPackageId pid,
-        identifierName = "", -- marked as deprecated in .proto
         identifierModuleName = unModuleName mod,
         identifierEntityName = unEntityName ent }
 

diff --git a/language-support/java/bindings/src/main/java/com/daml/ledger/javaapi/data/Identifier.java b/language-support/java/bindings/src/main/java/com/daml/ledger/javaapi/data/Identifier.java
@@ -41,8 +41,6 @@ public Identifier(@NonNull String packageId, @NonNull  String moduleName, @NonNu
     public static Identifier fromProto(ValueOuterClass.Identifier identifier) {
         if (!identifier.getModuleName().isEmpty() && !identifier.getEntityName().isEmpty()) {
             return new Identifier(identifier.getPackageId(), identifier.getModuleName(), identifier.getEntityName());
-        } else if (!identifier.getName().isEmpty()) {
-            return new Identifier(identifier.getPackageId(), identifier.getName());
         } else {
             throw new IllegalArgumentException(String.format("Invalid identifier [%s]: both module_name and entity_name must be set.", identifier));
         }

diff --git a/...kka/src/test/scala/com/digitalasset/ledger/client/binding/DomainTransactionMapperUT.scala b/...kka/src/test/scala/com/digitalasset/ledger/client/binding/DomainTransactionMapperUT.scala
@@ -36,20 +36,23 @@ class DomainTransactionMapperUT extends WordSpec with Matchers with AkkaTest {
         CreatedEvent(
           "createdEventId",
           contractId,
-          Some(Identifier("createdTemplateId")),
+          Some(Identifier("pkgId", "modName", "createdTemplateId")),
           None,
           Some(Record()))))
 
   def archivedEvent(contractId: String) =
     Event(
       Archived(
-        ArchivedEvent("archivedEventId", contractId, Some(Identifier("archivedTemplateId")))))
+        ArchivedEvent(
+          "archivedEventId",
+          contractId,
+          Some(Identifier("pkgId", "modName", "archivedTemplateId")))))
 
   def domainCreatedEvent(contractId: String) =
     DomainCreatedEvent(
       EventId("createdEventId"),
       ContractId(contractId),
-      TemplateId(Identifier("createdTemplateId")),
+      TemplateId(Identifier("pkgId", "modName", "createdTemplateId")),
       List.empty,
       CreateArguments(Record()),
       mockContract
@@ -59,16 +62,17 @@ class DomainTransactionMapperUT extends WordSpec with Matchers with AkkaTest {
     DomainArchivedEvent(
       EventId("archivedEventId"),
       ContractId(contractId),
-      TemplateId(Identifier("archivedTemplateId")),
-      List.empty)
+      TemplateId(Identifier("pkgId", "modName", "archivedTemplateId")),
+      List.empty
+    )
 
   case class MockTemplate() extends Template[MockTemplate] {
     override protected[this] def templateCompanion(
         implicit d: DummyImplicit): TemplateCompanion[MockTemplate] =
       new TemplateCompanion.Empty[MockTemplate] {
         override val onlyInstance = MockTemplate()
         override val id: Primitive.TemplateId[MockTemplate] =
-          ` templateId`("packageId", "moduleId", "templateId")
+          ` templateId`("packageId", "modName", "templateId")
         override val consumingChoices: Set[Choice] = Set.empty
       }
   }

diff --git a/...dings/src/main/scala/com/digitalasset/ledger/client/binding/binding/EventDecoderApi.scala b/...dings/src/main/scala/com/digitalasset/ledger/client/binding/binding/EventDecoderApi.scala
@@ -9,16 +9,7 @@ import scala.collection.immutable.{Map, Seq}
 import scalaz.Id.Id
 import com.digitalasset.ledger.api.v1.{event => rpcevent, value => rpcvalue}
 
-abstract class EventDecoderApi(
-    rawUnsupportedTemplates: Map[String, (String, String)],
-    val templateTypes: Seq[TemplateCompanion[_]]) {
-  val unsupportedTemplates: Map[String, Primitive.TemplateId[_]] = {
-    type K[A] = Map[String, A]
-    Primitive.TemplateId.substEx[K](rawUnsupportedTemplates transform {
-      case (_, (packageId, s)) =>
-        rpcvalue.Identifier(packageId, s)
-    })
-  }
+abstract class EventDecoderApi(val templateTypes: Seq[TemplateCompanion[_]]) {
 
   @SuppressWarnings(Array("org.wartremover.warts.Any"))
   val decoderTable: Map[ApiTypes.TemplateId, rpcevent.CreatedEvent => Option[Template[_]]] =

diff --git a/...la/bindings/src/main/scala/com/digitalasset/ledger/client/binding/binding/Primitive.scala b/...la/bindings/src/main/scala/com/digitalasset/ledger/client/binding/binding/Primitive.scala
@@ -13,7 +13,6 @@ import scala.collection.{immutable => imm}
 import scala.language.higherKinds
 import java.time.{Instant, LocalDate, LocalDateTime}
 import java.util.TimeZone
-import com.github.ghik.silencer.silent
 
 sealed abstract class Primitive {
   type Int64 = Long
@@ -101,34 +100,11 @@ sealed abstract class Primitive {
 
     private[binding] def substEx[F[_]](fa: F[rpcvalue.Identifier]): F[TemplateId[_]]
 
-    /** Package ID, module name, and entity name, unless the underlying
-      * gRPC Identifier was built with the very old and ambiguous two-argument
-      * form.  Use [[LegacyTemplateId]] instead if you want to extract
-      * the two-argument form.
+    /** Package ID, module name, and entity name.
       */
     def unapply[Tpl](t: TemplateId[Tpl]): Option[(String, String, String)]
   }
 
-  private[digitalasset] object LegacyIdentifier {
-    // suppress deprecation warnings because we _need_ to use the deprecated .name here -- the entire
-    // point of this method is to process it.
-    @silent
-    def unapply(t: rpcvalue.Identifier): Some[(String, String)] =
-      // TODO SC DEL-6727 use this instead with daml-lf value interface
-      // rpcvalue.Identifier unapply t.unwrap
-      t match {
-        case rpcvalue.Identifier(packageId, "", moduleName, entityName) =>
-          Some((packageId, s"$moduleName.$entityName"))
-        case rpcvalue.Identifier(packageId, entityName, _, _) =>
-          Some((packageId, entityName))
-      }
-  }
-
-  object LegacyTemplateId {
-    def unapply[Tpl](t: TemplateId[Tpl]): Some[(String, String)] =
-      LegacyIdentifier unapply t.unwrap
-  }
-
   private[binding] def substContractId[F[_], Tpl](tc: F[ApiTypes.ContractId]): F[ContractId[Tpl]]
 
   private[binding] def createFromArgs[Tpl](
@@ -187,26 +163,19 @@ private[client] object OnlyPrimitive extends Primitive {
     override def apply[Tpl <: Template[Tpl]](
         packageId: String,
         moduleName: String,
-        entityName: String): TemplateId[Tpl] =
+        entityName: String
+    ): TemplateId[Tpl] =
       ApiTypes.TemplateId(
-        rpcvalue.Identifier(
-          packageId = packageId,
-          name = s"$moduleName.$entityName",
-          moduleName = moduleName,
-          entityName = entityName))
+        rpcvalue
+          .Identifier(packageId = packageId, moduleName = moduleName, entityName = entityName))
 
     private[binding] override def substEx[F[_]](fa: F[rpcvalue.Identifier]) =
       ApiTypes.TemplateId subst fa
 
-    override def unapply[Tpl](t: TemplateId[Tpl]): Option[(String, String, String)] =
-      // TODO SC DEL-6727 use this instead with daml-lf value interface
-      // rpcvalue.Identifier unapply t.unwrap
-      t.unwrap match {
-        case rpcvalue.Identifier(packageId, _, moduleName, entityName)
-            if moduleName.nonEmpty && entityName.nonEmpty =>
-          Some((packageId, moduleName, entityName))
-        case _ => None
-      }
+    override def unapply[Tpl](t: TemplateId[Tpl]): Some[(String, String, String)] = {
+      val rpcvalue.Identifier(packageId, moduleName, entityName) = t.unwrap
+      Some((packageId, moduleName, entityName))
+    }
   }
 
   object ContractId extends ContractIdApi {

diff --git a/.../scala/bindings/src/main/scala/com/digitalasset/ledger/client/binding/binding/Value.scala b/.../scala/bindings/src/main/scala/com/digitalasset/ledger/client/binding/binding/Value.scala
@@ -58,8 +58,7 @@ object Value {
       baseVariantId: rpcvalue.Identifier,
       caseName: String
   ): rpcvalue.Identifier =
-    baseVariantId copy (name = s"${baseVariantId.name}.$caseName", entityName =
-      s"${baseVariantId.entityName}.$caseName")
+    baseVariantId copy (entityName = s"${baseVariantId.entityName}.$caseName")
 }
 
 object DamlCodecs extends encoding.ValuePrimitiveEncoding[Value] {
@@ -173,11 +172,7 @@ abstract class ValueRefCompanion {
       packageId: String,
       moduleName: String,
       entityName: String): rpcvalue.Identifier =
-    rpcvalue.Identifier(
-      packageId = packageId,
-      name = s"$moduleName.$entityName",
-      moduleName = moduleName,
-      entityName = entityName)
+    rpcvalue.Identifier(packageId = packageId, moduleName = moduleName, entityName = entityName)
 
   protected final def ` record`(elements: (String, rpcvalue.Value)*): VSum.Record =
     VSum.Record(Primitive.arguments(` dataTypeId`, elements))

diff --git a/...gs/src/test/scala/com/digitalasset/ledger/api/refinements/CompositeCommandAdapterUT.scala b/...gs/src/test/scala/com/digitalasset/ledger/api/refinements/CompositeCommandAdapterUT.scala
@@ -20,7 +20,9 @@ class CompositeCommandAdapterUT extends WordSpec with Matchers {
 
     "translate CompositeCommand to SubmitRequest" in {
       val commands =
-        Seq(Command(Create(CreateCommand(Some(Identifier("packageId", "templateId")), None))))
+        Seq(
+          Command(
+            Create(CreateCommand(Some(Identifier("packageId", "moduleName", "templateId")), None))))
 
       val submittedTraceContext = Some(TraceContext(1, 2, 3, Some(4L), true))
       val compositeCommand = CompositeCommand(

diff --git a/...testing/src/main/scala/com/digitalasset/ledger/client/binding/encoding/ShowEncoding.scala b/...testing/src/main/scala/com/digitalasset/ledger/client/binding/encoding/ShowEncoding.scala
@@ -25,24 +25,25 @@ abstract class ShowEncoding extends LfTypeEncoding {
 
   type VariantCases[A] = Show[A]
 
-  override def record[A](recordId: Identifier, fi: RecordFields[A]): Out[A] = {
-    val P.LegacyIdentifier(_, recName) = recordId
+  private def name(id: Identifier) =
+    s"${id.moduleName}.${id.entityName}"
+
+  override def record[A](recordId: Identifier, fi: RecordFields[A]): Out[A] =
     show { a: A =>
-      Cord(recName, "(", fi.show(a), ")")
+      Cord(name(recordId), "(", fi.show(a), ")")
     }
-  }
 
   override def emptyRecord[A](recordId: Identifier, element: () => A): Out[A] = {
-    val P.LegacyIdentifier(_, recName) = recordId
-    val shown = Cord(recName, "()")
+    val shown = Cord(name(recordId), "()")
     show { _: A =>
       shown
     }
   }
 
-  override def field[A](fieldName: String, o: Out[A]): Field[A] = show { a: A =>
-    Cord(fieldName, " = ", o.show(a))
-  }
+  override def field[A](fieldName: String, o: Out[A]): Field[A] =
+    show { a: A =>
+      Cord(fieldName, " = ", o.show(a))
+    }
 
   override def fields[A](fi: Field[A]): RecordFields[A] = fi
 

diff --git a/...codegen-testing/src/test/scala/com/digitalasset/ledger/client/binding/PrimitiveSpec.scala b/...codegen-testing/src/test/scala/com/digitalasset/ledger/client/binding/PrimitiveSpec.scala
@@ -11,7 +11,6 @@ import org.scalatest.prop.GeneratorDrivenPropertyChecks
 import shapeless.test.illTyped
 
 import com.digitalasset.ledger.client.binding.{Primitive => P}
-import com.digitalasset.ledger.api.v1.{value => rpcvalue}
 
 class PrimitiveSpec extends WordSpec with Matchers with GeneratorDrivenPropertyChecks {
   import PrimitiveSpec._
@@ -59,23 +58,6 @@ class PrimitiveSpec extends WordSpec with Matchers with GeneratorDrivenPropertyC
     }
   }
 
-  "LegacyIdentifier" should {
-    "match TemplateIds" in {
-      val t = P.TemplateId("Foo", "Bar", "Baz")
-      t match { // we skip `inside` specifically to verify scalac's selector/pattern type comparison
-        case P.LegacyTemplateId(p, i) =>
-          p should ===("Foo")
-          i should ===("Bar.Baz")
-      }
-    }
-
-    "believe the 'name' field if module/entity missing" in {
-      val P.LegacyIdentifier(p, i) = rpcvalue.Identifier("Foo", "Bar.Baz", "", "")
-      p should ===("Foo")
-      i should ===("Bar.Baz")
-    }
-  }
-
   "Timestamp.discardNanos" should {
     import ValueSpec.timestampArb
 

diff --git a/...g/src/test/scala/com/digitalasset/ledger/client/binding/encoding/LfTypeEncodingSpec.scala b/...g/src/test/scala/com/digitalasset/ledger/client/binding/encoding/LfTypeEncodingSpec.scala
@@ -81,7 +81,7 @@ object LfTypeEncodingSpec {
     final case class TLeft[A, B](body: A) extends TrialVariant[A, B]
     final case class TRight[A, B](one: B, two: B) extends TrialVariant[A, B]
 
-    override protected val ` dataTypeId` = rpcvalue.Identifier("hello", "TrialVariant")
+    override protected val ` dataTypeId` = rpcvalue.Identifier("hello", "Trial", "Variant")
 
     implicit def `TrialVariant arb`[A: Arbitrary, B: Arbitrary]: Arbitrary[TrialVariant[A, B]] =
       Arbitrary(
@@ -160,7 +160,7 @@ object LfTypeEncodingSpec {
   final case class TrialSubRec[A](num: P.Int64, a: A) extends ValueRef
 
   object TrialSubRec extends ValueRefCompanion {
-    override protected val ` dataTypeId` = rpcvalue.Identifier("hello", "TrialSubRec")
+    override protected val ` dataTypeId` = rpcvalue.Identifier("hello", "Trial", "SubRec")
 
     implicit def `TrialSubRec arb`[A: Arbitrary]: Arbitrary[TrialSubRec[A]] =
       Arbitrary(arbitrary[(P.Int64, A)] map (TrialSubRec[A] _).tupled)
@@ -207,7 +207,7 @@ object LfTypeEncodingSpec {
   final case class TrialEmptyRec() extends ValueRef
 
   object TrialEmptyRec extends ValueRefCompanion {
-    override protected val ` dataTypeId` = rpcvalue.Identifier("hello", "TrialSubRec")
+    override protected val ` dataTypeId` = rpcvalue.Identifier("hello", "Trial", "SubRec")
 
     implicit val `TrialEmptyRec arb`: Arbitrary[TrialEmptyRec] =
       Arbitrary(Gen const TrialEmptyRec())

diff --git a/...ing/src/test/scala/com/digitalasset/ledger/client/binding/encoding/ShowEncodingSpec.scala b/...ing/src/test/scala/com/digitalasset/ledger/client/binding/encoding/ShowEncodingSpec.scala
@@ -44,13 +44,13 @@ class ShowEncodingSpec extends WordSpec with Matchers {
     "show t.CallablePayout 1" in {
       val escapedName = "\"sch\\u00F6n\""
       val expected: String =
-        s"""MyMain.CallablePayout(receiver = P@"Alice", subr = TrialSubRec(num = 10, a = 100), lst = [1,2,3], emptyRec = TrialSubRec(), variant = TLeft($escapedName))"""
+        s"""MyMain.CallablePayout(receiver = P@"Alice", subr = Trial.SubRec(num = 10, a = 100), lst = [1,2,3], emptyRec = Trial.SubRec(), variant = TLeft($escapedName))"""
       contract1.show.toString should ===(expected)
     }
 
     "show t.CallablePayout 2" in {
       val expected: String =
-        """MyMain.CallablePayout(receiver = P@"Alice", subr = TrialSubRec(num = 11, a = 111), lst = [10,20,30], emptyRec = TrialSubRec(), variant = TRight(TrialVariant.TRight(one = CID@abc123, two = CID@def456)))"""
+        """MyMain.CallablePayout(receiver = P@"Alice", subr = Trial.SubRec(num = 11, a = 111), lst = [10,20,30], emptyRec = Trial.SubRec(), variant = TRight(Trial.Variant.TRight(one = CID@abc123, two = CID@def456)))"""
       contract2.show.toString should ===(expected)
     }
   }