overloaded choices in json-api #14410

S11001001 · 2022-07-13T17:17:19Z

CHANGELOG_BEGIN
- [JSON API] For exercise endpoints, when multiple choices with the same
  name are defined, the one directly defined on the ``templateId``,
  which may be a template or interface ID, will be selected; if a
  template ID is specified that only inherits the choices with the same
  name, the new ``choiceInterfaceId`` option must be specified to select
  the source of the choice.
CHANGELOG_END

3 overload integration tests
add changelog

…erloaded-choices-json-api

…lution

…erloaded-choices-json-api

remyhaemmerle-da

This looks reasonable.

Just few naive questions.

remyhaemmerle-da · 2022-07-25T18:12:42Z

ledger-service/http-json/src/it/scala/http/HttpServiceIntegrationTest.scala

      locator: domain.ContractLocator[v.Value],
-      to: domain.Party,
+      choice: TExercise[Inj],
+      choiceInterfaceId: Option[domain.ContractTypeId.Interface.OptionalPkg] = None,


Why choiceInterfaceId is not a field of TExercise ?

No particular reason, take a look at 9757fb0 and let me know what you think.

ledger-service/http-json/src/itlib/scala/http/AbstractHttpServiceIntegrationTest.scala

ledger-service/http-json/src/main/scala/com/digitalasset/http/PackageService.scala

remyhaemmerle-da · 2022-07-25T18:19:59Z

ledger-service/http-json/src/main/scala/com/digitalasset/http/domain.scala

            f: PackageService.ResolveTemplateRecordType,
            g: PackageService.ResolveChoiceArgType,
            h: PackageService.ResolveKeyType,
-        ): Error \/ LfType =
+        ) =


Why do we drop the type signature ?

Specifying the type signature of an override is usually pointless noise, and this change makes this one pointless and wrong noise. The only signal to be had is in the definition of type TypeFromCtId immediately above.

Specifying the type signature of an override is usually pointless noise, and this change makes this one pointless and wrong noise. The only signal to be had is in the definition of type TypeFromCtId immediately above.

I don't think it is. Having the signature fully available makes it easy to understand what the function does at a glance without having to go through the inheritance hiearachy.

It's a typeclass and its instances. We don't restate the types of typeclass methods in every instance for the same reason.

ledger-service/http-json/src/main/scala/com/digitalasset/http/json/JsonProtocol.scala

stefanobaghino-da

Mostly a documentation review.

My eye slipped on a couple of implementation details and I added a comment there as well.

libs-scala/nonempty/src/main/scala/nonempty/Patterns.scala

stefanobaghino-da · 2022-07-26T07:25:30Z

ledger-service/http-json/src/main/scala/com/digitalasset/http/domain.scala

            f: PackageService.ResolveTemplateRecordType,
            g: PackageService.ResolveChoiceArgType,
            h: PackageService.ResolveKeyType,
-        ): Error \/ LfType =
+        ) =


Specifying the type signature of an override is usually pointless noise, and this change makes this one pointless and wrong noise. The only signal to be had is in the definition of type TypeFromCtId immediately above.

I don't think it is. Having the signature fully available makes it easy to understand what the function does at a glance without having to go through the inheritance hiearachy.

docs/source/json-api/index.rst

…erloaded-choices-json-api

@remyhaemmerle-da

- suggested by @remyhaemmerle-da; thanks

@remyhaemmerle-da

- suggested by @remyhaemmerle-da; thanks

@remyhaemmerle-da

- suggested by @remyhaemmerle-da; thanks

@stefanobaghino-da

- suggested by @stefanobaghino-da; thanks

…erloaded-choices-json-api

Documentation improvements addressed in 0eeb531

stefanobaghino-da · 2022-07-27T05:46:41Z

I removed my blocker since the documentation has been improved as required by 0eeb531. Thank you, @S11001001.

…erloaded-choices-json-api

introduce doc-only template ID aliases for interface IDs

4f03777

S11001001 added component/json-api HTTP JSON API team/ledger-clients Related to the Ledger Clients team's components. labels Jul 13, 2022

S11001001 self-assigned this Jul 13, 2022

S11001001 added 26 commits July 13, 2022 14:25

Merge commit 'f090d99c1dedeaefb89cad63a1da4c3a23f83745' into 13923-ov…

0130bb4

…erloaded-choices-json-api

minimize some imports

3235cc2

choiceInterfaceId arg for exercise, stub out some resolution

96b758d

Merge commit 'bf64e705e9014e0796241f70acea8034af6facfe' into 13923-ov…

443cccb

…erloaded-choices-json-api

alternate design based on a single contract type ID

22b33e6

account for choiceInterfaceId in tests

2dfe548

missing implicit resolution

17cb1fd

add manual indirect resolution to resolveChoiceArgType

b90c896

Merge commit 'e023fcfc8cc9616e175b5fa85116326f38d8b62f' into 13923-ov…

9cfc8be

…erloaded-choices-json-api

new Singleton pattern for non-empty maps/sets

a5296b5

don't redo the work of the resolver; reuse it in ChoiceTypeMap

f79e8d6

account for choiceInterfaceId in exercise command JSON

bd0b1ac

getting the interface ID from the package service to the JSON decoder

10e09ec

let everything but ExerciseCommand follow the old path

f55bacd

define functor for TemplateId

5de4671

deal with the extra choice interface ID inference in ExerciseCommand

5f16eb3

remove unused function

f3ff260

pass inferred interface ID back from package service

2fc0ab9

pass interface ID to command service

5027afc

reenable exercise-by-interface test

e49f3e2

add choiceInterfaceId to CreateAndExerciseCommand

98faeb8

factor the template ID resolutions for create-and-exercise

6afc3ab

use the choiceInterfaceId for regular exercise command lookups

3d7f184

add test to pass a choiceInterfaceId

3151b66

fix case where uninferred choice interface ID suppresses its own reso…

b1507f9

…lution

document choiceInterfaceId

35598a9

S11001001 added 8 commits July 21, 2022 14:05

unsubst is subst

56e2583

test the exercise results

6d36492

test that template-defined choice is preferred

c192cc9

test that interface-defined choice may be selected

c7a0e6e

refactor success out of createIouAndExerciseTransfer

1e73cac

ambiguity failure test

7d23368

remove bad duplicated comments

05c594f

Merge commit '67b47c6dff4b064129ee445d63cfa0cf5a1ffc68' into 13923-ov…

25fcb28

…erloaded-choices-json-api

S11001001 marked this pull request as ready for review July 21, 2022 22:23

S11001001 requested review from stefanobaghino-da and ray-roestenburg-da as code owners July 21, 2022 22:23

S11001001 requested a review from a team July 21, 2022 22:23

remyhaemmerle-da approved these changes Jul 25, 2022

View reviewed changes

stefanobaghino-da previously requested changes Jul 26, 2022

View reviewed changes

stefanobaghino-da reviewed Jul 26, 2022

View reviewed changes

docs/source/json-api/index.rst Show resolved Hide resolved

S11001001 added 6 commits July 26, 2022 11:47

Merge commit '17fba251c927820ef86e27355d6d07d0cd36074e' into 13923-ov…

faf5d7b

…erloaded-choices-json-api

include choiceInterfaceId in TExercise

9757fb0

- suggested by @remyhaemmerle-da; thanks

use named arguments when rebuilding package State

b129b0e

- suggested by @remyhaemmerle-da; thanks

use Iterable for extending a map

f3781ed

- suggested by @remyhaemmerle-da; thanks

document the semantics of choiceInterfaceId and templateId

0eeb531

- suggested by @stefanobaghino-da; thanks

Merge commit '8309cfef1ef6b4eba402121b7e634d325f404f58' into 13923-ov…

378070f

…erloaded-choices-json-api

S11001001 requested a review from stefanobaghino-da July 26, 2022 16:48

Merge commit 'fdcbb6903996f0a4bb0e9aeb90d39c5e894a285b' into 13923-ov…

990a145

…erloaded-choices-json-api

S11001001 mentioned this pull request Jul 27, 2022

pass in interface ID to create-and-exercise, exercise-by-key commands #14549

Open

point to #14549 where appropriate, remove TODO we'll skip

34be558

S11001001 enabled auto-merge (squash) July 27, 2022 13:48

S11001001 merged commit 1a9dc02 into main Jul 27, 2022

S11001001 deleted the 13923-overloaded-choices-json-api branch July 27, 2022 15:40

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

overloaded choices in json-api #14410

overloaded choices in json-api #14410

S11001001 commented Jul 13, 2022 •

edited

Loading

remyhaemmerle-da left a comment •

edited

Loading

remyhaemmerle-da Jul 25, 2022

S11001001 Jul 26, 2022

remyhaemmerle-da Jul 25, 2022

S11001001 Jul 25, 2022

remyhaemmerle-da Jul 25, 2022

stefanobaghino-da Jul 26, 2022

S11001001 Jul 26, 2022

stefanobaghino-da left a comment

stefanobaghino-da Jul 26, 2022

stefanobaghino-da commented Jul 27, 2022

overloaded choices in json-api #14410

overloaded choices in json-api #14410

Conversation

S11001001 commented Jul 13, 2022 • edited Loading

remyhaemmerle-da left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

stefanobaghino-da left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

stefanobaghino-da commented Jul 27, 2022

S11001001 commented Jul 13, 2022 •

edited

Loading

remyhaemmerle-da left a comment •

edited

Loading