Skip to content

Commit

Permalink
JSON API Upgrades: ETX-500 require fully qualified template ids (#19149)
Browse files Browse the repository at this point in the history
- update all instances of ContractTypeId.OptionalPkg to ContractTypeId.RequiredPkg
- removed ContractTypeId.OptionalPkg and ContractTypeId.NoPkg type aliases entirely
- update tests to use fully qualified template ids, i.e. including a package reference
- update integration tests which do json-api queries to include a package reference in template ids

We still need to distinguish package name vs package id at the point where we resolve contract type ids, so we keep the logic that we used on missing package id, and do the same thing in the case of a package name. This logic deals with the fact that new packages can get uploaded in the background, and so we need to be able to treat a package name as a dynamically late-bound mapping to specific package.
  • Loading branch information
raphael-speyer-da committed May 10, 2024
1 parent ab9f5c7 commit 581021a
Show file tree
Hide file tree
Showing 26 changed files with 415 additions and 522 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@ withDamlServiceIn path command args act = withDevNull $ \devNull -> do
data DamlStartResource = DamlStartResource
{ projDir :: FilePath
, tmpDir :: FilePath
, packageRef :: String
, alice :: String
, aliceHeaders :: RequestHeaders
, startStdin :: Handle
Expand Down Expand Up @@ -161,10 +162,13 @@ damlStart tmpDir _disableUpgradeValidation = do
(authorizationHeaders "Alice") -- dummy party here, not important
scriptOutput <- readFileUTF8 (projDir </> scriptOutputFile)
let alice = (read scriptOutput :: String)
-- TODO(raphael-speyer-da): Use a package name once supported by canton out of the box.
let packageRef = "6310b9aa3d506211b592dd657afb167d63637453f31eae986fad5aa1b6d61401"
pure $
DamlStartResource
{ projDir = projDir
, tmpDir = tmpDir
, packageRef = packageRef
, sandboxPort = ledger ports
, jsonApiPort = jsonApiPort
, startStdin = startStdin
Expand Down Expand Up @@ -334,7 +338,7 @@ damlStartTests getDamlStart =
let subtest :: forall t. String -> IO t -> IO t
subtest m p = step m >> p
subtest "sandbox and json-api come up" $ do
DamlStartResource {jsonApiPort, alice, aliceHeaders} <- getDamlStart
DamlStartResource {jsonApiPort, alice, aliceHeaders, packageRef} <- getDamlStart
manager <- newManager defaultManagerSettings
initialRequest <-
parseRequest $ "http://localhost:" <> show jsonApiPort <> "/v1/create"
Expand All @@ -346,7 +350,7 @@ damlStartTests getDamlStart =
RequestBodyLBS $
Aeson.encode $
Aeson.object
[ "templateId" Aeson..= Aeson.String "Main:T"
[ "templateId" Aeson..= (packageRef ++ ":Main:T")
, "payload" Aeson..= [alice]
]
}
Expand All @@ -363,15 +367,15 @@ damlStartTests getDamlStart =
callCommandSilentIn projDir $ unwords
["daml", "ledger", "allocate-party", "--port", show sandboxPort, "Bob"]
subtest "Run init-script" $ do
DamlStartResource {jsonApiPort, aliceHeaders} <- getDamlStart
DamlStartResource {jsonApiPort, aliceHeaders, packageRef} <- getDamlStart
initialRequest <- parseRequest $ "http://localhost:" <> show jsonApiPort <> "/v1/query"
let queryRequest = initialRequest
{ method = "POST"
, requestHeaders = aliceHeaders
, requestBody =
RequestBodyLBS $
Aeson.encode $
Aeson.object ["templateIds" Aeson..= [Aeson.String "Main:T"]]
Aeson.object ["templateIds" Aeson..= [packageRef ++ ":Main:T"]]
}
manager <- newManager defaultManagerSettings
queryResponse <- httpLbs queryRequest manager
Expand Down Expand Up @@ -458,7 +462,7 @@ damlStartTestsWithoutValidation :: SdkVersioned => IO DamlStartResource -> TestT
damlStartTestsWithoutValidation getDamlStart =
-- We use testCaseSteps to make sure each of these tests runs in sequence, not in parallel.
testCase "daml start without upgrade validation - hot reload" $ do
DamlStartResource {projDir, jsonApiPort, startStdin, stdoutChan, alice, aliceHeaders} <- getDamlStart
DamlStartResource {projDir, jsonApiPort, startStdin, stdoutChan, alice, aliceHeaders, packageRef} <- getDamlStart
stdoutReadChan <- atomically $ dupTChan stdoutChan
writeFileUTF8 (projDir </> "daml/Main.daml") $
unlines
Expand All @@ -478,6 +482,7 @@ damlStartTestsWithoutValidation getDamlStart =
untilM_ (pure ()) $ do
line <- atomically $ readTChan stdoutReadChan
pure ("Rebuild complete" `isInfixOf` line)
let newPackageRef = "8caa3f3b63b66d0338e0bb8b931c5c57644a4362b3bd82c482bd37d7438abcc8"
initialRequest <-
parseRequest $ "http://localhost:" <> show jsonApiPort <> "/v1/query"
manager <- newManager defaultManagerSettings
Expand All @@ -488,7 +493,7 @@ damlStartTestsWithoutValidation getDamlStart =
, requestBody =
RequestBodyLBS $
Aeson.encode $
Aeson.object ["templateIds" Aeson..= [Aeson.String "Main:T"]]
Aeson.object ["templateIds" Aeson..= [packageRef ++ ":Main:T"]]
}
let queryRequestS =
initialRequest
Expand All @@ -497,7 +502,7 @@ damlStartTestsWithoutValidation getDamlStart =
, requestBody =
RequestBodyLBS $
Aeson.encode $
Aeson.object ["templateIds" Aeson..= [Aeson.String "Main:S"]]
Aeson.object ["templateIds" Aeson..= [newPackageRef ++ ":Main:S"]]
}
queryResponseT <- httpLbs queryRequestT manager
queryResponseS <- httpLbs queryRequestS manager
Expand Down
1 change: 1 addition & 0 deletions sdk/language-support/ts/codegen/tests/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ daml_compile(
name = "hidden",
srcs = ["daml/Hidden.daml"],
data_dependencies = ["//language-support/ts/codegen/tests:build-and-lint.dar"],
project_name = "hidden",
target = "1.dev",
)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -917,7 +917,7 @@ describe("interfaces", () => {
// meaningful we *must not* codegen or load Hidden
type NotVisibleInTs = { owner: Party };
const NotVisibleInTs: Pick<Template<{ owner: Party }>, "templateId"> = {
templateId: "Hidden:NotVisibleInTs",
templateId: "#hidden:Hidden:NotVisibleInTs",
};
const initialPayload: NotVisibleInTs = { owner: ALICE_PARTY };

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,8 @@ sealed abstract class ContractTypeId[+PkgId]
import scala.util.hashing.{MurmurHash3 => H}
H.productHash(this, H.productSeed, ignorePrefix = true)
}

def fqn: String = s"${packageId.toString}:${moduleName}:${entityName}"
}

object ResolvedQuery {
Expand Down Expand Up @@ -240,9 +242,7 @@ object ContractTypeId extends ContractTypeIdLike[ContractTypeId] {

/** A contract type ID companion. */
sealed abstract class ContractTypeIdLike[CtId[T] <: ContractTypeId[T]] {
type OptionalPkg = CtId[Option[String]]
type RequiredPkg = CtId[String]
type NoPkg = CtId[Unit]
type Resolved = ContractTypeId.ResolvedOf[CtId]

// treat the companion like a typeclass instance
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,9 @@ object HttpServiceTestFixture extends LazyLogging with Assertions with Inside {

private val doNotReloadPackages = FiniteDuration(100, DAYS)

// This may need to be updated if the Account.daml is updated.
val staticPkgIdAccount = "b3c9564bb7334bfd0f82099893eba518afc3b68a4d5c66cb2835498252db93e9"

def withHttpService[A](
testName: String,
ledgerPort: Port,
Expand Down Expand Up @@ -416,7 +419,7 @@ object HttpServiceTestFixture extends LazyLogging with Assertions with Inside {
postJsonStringRequest(uri, json.prettyPrint, headers)

def postCreateCommand(
cmd: domain.CreateCommand[v.Record, domain.ContractTypeId.Template.OptionalPkg],
cmd: domain.CreateCommand[v.Record, domain.ContractTypeId.Template.RequiredPkg],
encoder: DomainJsonEncoder,
uri: Uri,
headers: List[HttpHeader],
Expand All @@ -432,7 +435,7 @@ object HttpServiceTestFixture extends LazyLogging with Assertions with Inside {
}

def postArchiveCommand(
templateId: domain.ContractTypeId.OptionalPkg,
templateId: domain.ContractTypeId.RequiredPkg,
contractId: domain.ContractId,
encoder: DomainJsonEncoder,
uri: Uri,
Expand Down Expand Up @@ -484,8 +487,8 @@ object HttpServiceTestFixture extends LazyLogging with Assertions with Inside {
owner: domain.Party,
number: String,
time: v.Value.Sum.Timestamp = TimestampConversion.roundInstantToMicros(Instant.now),
): domain.CreateCommand[v.Record, domain.ContractTypeId.Template.OptionalPkg] = {
val templateId = domain.ContractTypeId.Template(None, "Account", "Account")
): domain.CreateCommand[v.Record, domain.ContractTypeId.Template.RequiredPkg] = {
val templateId = domain.ContractTypeId.Template(staticPkgIdAccount, "Account", "Account")
val timeValue = v.Value(time)
val enabledVariantValue =
v.Value(v.Value.Sum.Variant(v.Variant(None, "Enabled", Some(timeValue))))
Expand All @@ -504,8 +507,8 @@ object HttpServiceTestFixture extends LazyLogging with Assertions with Inside {
owners: Seq[domain.Party],
number: String,
time: v.Value.Sum.Timestamp = TimestampConversion.roundInstantToMicros(Instant.now),
): domain.CreateCommand[v.Record, domain.ContractTypeId.Template.OptionalPkg] = {
val templateId = domain.ContractTypeId.Template(None, "Account", "SharedAccount")
): domain.CreateCommand[v.Record, domain.ContractTypeId.Template.RequiredPkg] = {
val templateId = domain.ContractTypeId.Template(staticPkgIdAccount, "Account", "SharedAccount")
val timeValue = v.Value(time)
val ownersEnc = v.Value(
v.Value.Sum.List(v.List(domain.Party.unsubst(owners).map(o => v.Value(v.Value.Sum.Party(o)))))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -212,7 +212,7 @@ abstract class FailureTests
)
)
_ = status shouldBe StatusCodes.OK
query = jsObject("""{"templateIds": ["Account:Account"]}""")
query = jsObject(s"""{"templateIds": ["$staticPkgIdAccount:Account:Account"]}""")
(status, output) <- headersWithParties(List(p)).flatMap(
postRequest(
uri = uri.withPath(Uri.Path("/v1/query")),
Expand Down Expand Up @@ -276,7 +276,7 @@ abstract class FailureTests
)
)
_ = status shouldBe StatusCodes.OK
query = jsObject("""{"templateIds": ["Account:Account"]}""")
query = jsObject(s"""{"templateIds": ["$staticPkgIdAccount:Account:Account"]}""")
(status, output) <- headersWithParties(List(p)).flatMap(
postRequest(
uri = uri.withPath(Uri.Path("/v1/query")),
Expand Down Expand Up @@ -339,8 +339,8 @@ abstract class FailureTests
"/v1/stream/query can reconnect" taggedAs availabilitySecurity in withHttpService {
(uri, encoder, _, client) =>
val query =
"""[
{"templateIds": ["Account:Account"]}
s"""[
{"templateIds": ["$staticPkgIdAccount:Account:Account"]}
]"""

val offset = Promise[Offset]()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ abstract class HttpServiceIntegrationTest
import util.ErrorOps._
import com.daml.jwt.domain.Jwt

val command0: domain.CreateCommand[v.Record, domain.ContractTypeId.Template.OptionalPkg] =
val command0: domain.CreateCommand[v.Record, domain.ContractTypeId.Template.RequiredPkg] =
iouCreateCommand(domain.Party("Alice"))

type F[A] = EitherT[Future, JsonError, A]
Expand All @@ -139,7 +139,7 @@ abstract class HttpServiceIntegrationTest
): F[JsValue]
command1 <- (EitherT.rightT(jwt(uri)): F[Jwt])
.flatMap(decoder.decodeCreateCommand(jsVal, _, fixture.ledgerId))
} yield command1.bimap(removeRecordId, removePackageId) should ===(command0)
} yield command1.bimap(removeRecordId, identity) should ===(command0)

(x.run: Future[JsonError \/ Assertion]).map(_.fold(e => fail(e.shows), identity))
}
Expand Down Expand Up @@ -178,8 +178,8 @@ abstract class HttpServiceIntegrationTest

def createIouAndExerciseTransfer(
fixture: UriFixture with EncoderFixture,
initialTplId: domain.ContractTypeId.Template.OptionalPkg,
exerciseTid: domain.ContractTypeId.OptionalPkg,
initialTplId: domain.ContractTypeId.Template.RequiredPkg,
exerciseTid: domain.ContractTypeId.RequiredPkg,
choice: TExercise[_] = tExercise(choiceArgType = echoTextVA)(echoTextSample),
) = for {
aliceH <- fixture.getUniquePartyAndAuthHeaders("Alice")
Expand Down Expand Up @@ -210,17 +210,13 @@ abstract class HttpServiceIntegrationTest
inside(exerciseTest) { case domain.OkResponse(er, None, StatusCodes.OK) =>
inside(jdecode[String](er.exerciseResult)) { case \/-(decoded) => decoded }
}
object Transferrable {
val Transferrable: domain.ContractTypeId.Interface.OptionalPkg =
domain.ContractTypeId.Interface(None, "Transferrable", "Transferrable")
}

"templateId = interface ID" in withHttpService { fixture =>
for {
_ <- uploadPackage(fixture)(ciouDar)
result <- createIouAndExerciseTransfer(
fixture,
initialTplId = domain.ContractTypeId.Template(None, "IIou", "TestIIou"),
initialTplId = TpId.IIou.TestIIou,
// whether we can exercise by interface-ID
exerciseTid = TpId.IIou.IIou,
) map exerciseSucceeded
Expand All @@ -234,9 +230,9 @@ abstract class HttpServiceIntegrationTest
_ <- uploadPackage(fixture)(ciouDar)
result <- createIouAndExerciseTransfer(
fixture,
initialTplId = CIou.CIou,
initialTplId = TpId.CIou.CIou,
// whether we can exercise inherited by concrete template ID
exerciseTid = CIou.CIou,
exerciseTid = TpId.CIou.CIou,
) map exerciseSucceeded
} yield result should ===("Bob invoked IIou.Transfer")
}
Expand All @@ -246,8 +242,8 @@ abstract class HttpServiceIntegrationTest
_ <- uploadPackage(fixture)(ciouDar)
result <- createIouAndExerciseTransfer(
fixture,
initialTplId = CIou.CIou,
exerciseTid = CIou.CIou,
initialTplId = TpId.CIou.CIou,
exerciseTid = TpId.CIou.CIou,
choice = tExercise(choiceInterfaceId = Some(TpId.IIou.IIou), choiceArgType = echoTextVA)(
echoTextSample
),
Expand All @@ -261,8 +257,8 @@ abstract class HttpServiceIntegrationTest
_ <- uploadPackage(fixture)(ciouDar)
result <- createIouAndExerciseTransfer(
fixture,
initialTplId = CIou.CIou,
exerciseTid = CIou.CIou,
initialTplId = TpId.CIou.CIou,
exerciseTid = TpId.CIou.CIou,
choice = tExercise(choiceName = "Overridden", choiceArgType = echoTextPairVA)(
ShRecord(echo = ShRecord(_1 = "yes", _2 = "no"))
),
Expand All @@ -276,9 +272,9 @@ abstract class HttpServiceIntegrationTest
_ <- uploadPackage(fixture)(ciouDar)
result <- createIouAndExerciseTransfer(
fixture,
initialTplId = CIou.CIou,
exerciseTid = CIou.CIou,
choice = tExercise(Some(Transferrable.Transferrable), "Overridden", echoTextVA)(
initialTplId = TpId.CIou.CIou,
exerciseTid = TpId.CIou.CIou,
choice = tExercise(Some(TpId.Transferrable.Transferrable), "Overridden", echoTextVA)(
ShRecord(echo = "yesyes")
),
) map exerciseSucceeded
Expand All @@ -290,8 +286,8 @@ abstract class HttpServiceIntegrationTest
_ <- uploadPackage(fixture)(ciouDar)
response <- createIouAndExerciseTransfer(
fixture,
initialTplId = CIou.CIou,
exerciseTid = CIou.CIou,
initialTplId = TpId.CIou.CIou,
exerciseTid = TpId.CIou.CIou,
choice = tExercise(choiceName = "Ambiguous", choiceArgType = echoTextVA)(
ShRecord(echo = "ambiguous-test")
),
Expand All @@ -308,8 +304,8 @@ abstract class HttpServiceIntegrationTest
_ <- uploadPackage(fixture)(ciouDar)
result <- createIouAndExerciseTransfer(
fixture,
initialTplId = CIou.CIou,
exerciseTid = CIou.CIou,
initialTplId = TpId.CIou.CIou,
exerciseTid = TpId.CIou.CIou,
choice = tExercise(choiceName = "TransferPlease", choiceArgType = echoTextVA)(
echoTextSample
),
Expand All @@ -326,7 +322,7 @@ abstract class HttpServiceIntegrationTest
aliceH <- fixture.getUniquePartyAndAuthHeaders("Alice")
(alice, aliceHeaders) = aliceH
createTest <- postCreateCommand(
iouCommand(alice, CIou.CIou),
iouCommand(alice, TpId.CIou.CIou),
fixture,
aliceHeaders,
)
Expand All @@ -337,7 +333,7 @@ abstract class HttpServiceIntegrationTest
encodeExercise(encoder)(
iouTransfer(
domain.EnrichedContractKey(
TpId.unsafeCoerce[domain.ContractTypeId.Template, Option[String]](TpId.IIou.IIou),
TpId.unsafeCoerce[domain.ContractTypeId.Template, String](TpId.IIou.IIou),
v.Value(v.Value.Sum.Party(domain.Party unwrap alice)),
),
tExercise()(ShRecord(echo = "bob")),
Expand All @@ -348,7 +344,7 @@ abstract class HttpServiceIntegrationTest
.parseResponse[JsValue]
} yield inside(exerciseTest) {
case domain.ErrorResponse(Seq(lookup), None, StatusCodes.BadRequest, _) =>
lookup should include regex raw"Cannot resolve template ID, given: TemplateId\(None,IIou,IIou\)"
lookup should include regex raw"Cannot resolve template ID, given: TemplateId\([a-z0-9]+,IIou,IIou\)"
}
}

Expand Down Expand Up @@ -383,7 +379,7 @@ object HttpServiceIntegrationTest {
private val echoTextSample: echoTextVA.Inj = ShRecord(echo = "Bob")

private def tExercise(
choiceInterfaceId: Option[domain.ContractTypeId.Interface.OptionalPkg] = None,
choiceInterfaceId: Option[domain.ContractTypeId.Interface.RequiredPkg] = None,
choiceName: String = "Transfer",
choiceArgType: VA = echoTextVA,
)(
Expand All @@ -392,7 +388,7 @@ object HttpServiceIntegrationTest {
TExercise(choiceInterfaceId, choiceName, choiceArgType, choiceArg)

private final case class TExercise[Inj](
choiceInterfaceId: Option[domain.ContractTypeId.Interface.OptionalPkg],
choiceInterfaceId: Option[domain.ContractTypeId.Interface.RequiredPkg],
choiceName: String,
choiceArgType: VA.Aux[Inj],
choiceArg: Inj,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,9 @@ abstract class HttpServiceWithPostgresIntTest
val searchDataSet = genSearchDataSet(party)
searchExpectOk(
searchDataSet,
jsObject("""{"templateIds": ["Iou:Iou"], "query": {"currency": "EUR"}}"""),
jsObject(
s"""{"templateIds": ["${TpId.Iou.Iou.fqn}"], "query": {"currency": "EUR"}}"""
),
fixture,
headers,
).flatMap { searchResult =>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ class WebSocketCloseTest
.subprotocols(subprotocols.head, subprotocols.tail: _*)
.buildAsync(wsUri, listener)
.asScala
_ <- ws.sendText("""{"templateIds": ["Iou:Iou"]}""", true).asScala
_ <- ws.sendText(s"""{"templateIds": ["${TpId.Iou.Iou.fqn}"]}""", true).asScala
_ <- ws.sendClose(WebSocket.NORMAL_CLOSURE, "ok").asScala
_ <- serverCloseReceived.future
} yield {
Expand Down

0 comments on commit 581021a

Please sign in to comment.