Typed ACS and transaction streams for Java codegen #15159

chunlokling-da · 2022-10-04T10:21:55Z

chunlokling-da · 2022-10-04T10:22:52Z

...ort/java/bindings/src/main/java/com/daml/ledger/javaapi/data/codegen/InterfaceCompanion.java

+
+  @Override
+  public TransactionFilter transactionFilter(Set<String> parties) {
+    Filter filter = new InclusiveFilter(Collections.emptySet(), Map.of(TEMPLATE_ID, Filter.Interface.INCLUDE_VIEW));


Does it makes sense to always include view or maybe we should pass in a method argument here and let the caller decide

Filter.Interface.INCLUDE_VIEW

If you don't include the view, the decoding part of the utility is guaranteed to fail to find the view. This utility should work in harmony with the other utilities that contribute to the goal of typed ACS/transaction streams.

S11001001 · 2022-10-05T16:01:14Z

...a/bindings/src/main/java/com/daml/ledger/javaapi/data/codegen/ContractWithInterfaceView.java

+import java.util.Optional;
+import java.util.Set;
+
+public final class ContractWithInterfaceView<Id, View> extends Contract<Id, View> {


An example of why I am reluctant to add extends clauses to class or interface type parameters that aren't strictly necessary.

Sure, when defining Contract, we could have made its second type parameter extends Template. That would have compiled at the time. However, it has no functional benefits for the API, and then you would be stuck here.

While adding constraints to class tparams can be useful, it is more often no better than DatatypeContexts aka "stupid theta" in Haskell, and just as worth avoiding.

...a/bindings/src/main/java/com/daml/ledger/javaapi/data/codegen/ContractWithInterfaceView.java

…ransaction-streams # Conflicts: # language-support/java/bindings/src/main/java/com/daml/ledger/javaapi/data/codegen/InterfaceCompanion.java

CHANGELOG_BEGIN CHANGELOG_END

ray-roestenburg-da

LGTM

language-support/java/bindings-rxjava/src/main/java/com/daml/ledger/rxjava/ContractUtil.java

...java/bindings-rxjava/src/main/java/com/daml/ledger/rxjava/grpc/ActiveContractClientImpl.java

...rt/java/bindings-rxjava/src/main/java/com/daml/ledger/rxjava/grpc/TransactionClientImpl.java

language-support/java/bindings/src/main/java/com/daml/ledger/javaapi/data/ActiveContracts.java

language-support/java/bindings/src/main/java/com/daml/ledger/javaapi/data/codegen/Contract.java

...port/java/bindings/src/main/java/com/daml/ledger/javaapi/data/codegen/ContractCompanion.java

...ort/java/bindings/src/main/java/com/daml/ledger/javaapi/data/codegen/InterfaceCompanion.java

S11001001 · 2022-10-07T15:13:13Z

@chunlokling-da Additionally, I created #15191 to follow-up on this PR; if you want to think about what this PR would mean for that issue, I won't stop you 🙂

…ransaction-streams # Conflicts: # language-support/java/bindings/src/main/java/com/daml/ledger/javaapi/data/codegen/ContractCompanion.java # language-support/java/bindings/src/main/java/com/daml/ledger/javaapi/data/codegen/ContractTypeCompanion.java # language-support/java/bindings/src/main/java/com/daml/ledger/javaapi/data/codegen/InterfaceCompanion.java # language-support/java/codegen/src/main/scala/com/digitalasset/daml/lf/codegen/backend/java/inner/InterfaceClass.scala

.../java/bindings/src/main/java/com/daml/ledger/javaapi/data/codegen/ContractTypeCompanion.java

...egen/src/main/scala/com/digitalasset/daml/lf/codegen/backend/java/inner/InterfaceClass.scala

S11001001 · 2022-10-11T18:28:51Z

language-support/java/bindings-rxjava/src/main/java/com/daml/ledger/rxjava/ContractUtil.java

+  static <Ct> ContractUtil<Ct> of(ContractCompanion<Ct, ?, ?> companion) {
+    Filter filter = new InclusiveFilter(Set.of(companion.TEMPLATE_ID), Collections.emptyMap());
+    return new ContractUtil<>(companion::fromCreatedEvent, filter);
+  }
+
+  static <Cid, View> ContractUtil<Contract<Cid, View>> of(


I suggest doing at least the ACS part of #15191 in this PR, if leaving aside the transaction stream part, as a sanity check for the API.

Sure, it is now using the new ACS method in IouMain

language-support/java/bindings-rxjava/src/main/java/com/daml/ledger/rxjava/ContractUtil.java

… the new getActiveContracts. to address Stephen's comments

S11001001 · 2022-10-12T13:35:55Z

Per discussion out-of-band,

please revert the Java 8 changes

@chunlokling-da I believe there are symbol accessibility problems in this PR, but if you want to pick up #15191 right away to change the source/target in quickstart-java and update the example, then I think we can merge this one.

S11001001 · 2022-10-12T13:42:02Z

@chunlokling-da The other bearing that #15191 has on this PR is: as we have discussed, the idea of the rxjava additions is a bit of a crucible to ensure the bindings library and codegen provide enough support to make implementing this kind of stream relatively easy to do.

Likewise, the change to the quickstart-java is to ensure that the feature is powerful enough to support our tutorial and yields tutorial code that looks good; we can make further changes to this feature to ensure that both of those things are true in the PR for that issue.

S11001001 · 2022-10-12T16:31:04Z

language-support/java/bindings/src/main/java/com/daml/ledger/javaapi/data/ActiveContracts.java

+  @NonNull
+  public Optional<String> getOffset() {
+    // Empty string indicates that the field is not present in the protobuf.
+    return Optional.of(offset).filter(off -> !offset.equals(""));
+  }
+
+  @NonNull
+  public List<@NonNull Ct> getContracts() {
+    return activeContracts;
+  }
+
+  @NonNull
+  public String getWorkflowId() {
+    return workflowId;
+  }


In general, we should define public final properties rather than private properties with getters.

You can consider this for a follow-up PR though, e.g. the quickstart-java one.

For the record, we do not care that Intellij thinks you shouldn't have Optional fields.

Sure will do it in the quickstart-java issue: #15191

...egen/src/main/scala/com/digitalasset/daml/lf/codegen/backend/java/inner/InterfaceClass.scala

...degen/src/main/scala/com/digitalasset/daml/lf/codegen/backend/java/inner/TemplateClass.scala

added transactionFilter

43c9f36

chunlokling-da linked an issue Oct 4, 2022 that may be closed by this pull request

typed ACS and transaction streams for Java codegen #14969

Closed

chunlokling-da commented Oct 4, 2022

View reviewed changes

chunlokling-da self-assigned this Oct 4, 2022

chunlokling-da added 3 commits October 4, 2022 13:50

move transactionFilter to TransactionFilter

08d0254

fromCreatedEvent

7178636

format

ca68c0c

S11001001 reviewed Oct 5, 2022

View reviewed changes

...a/bindings/src/main/java/com/daml/ledger/javaapi/data/codegen/ContractWithInterfaceView.java Outdated Show resolved Hide resolved

chunlokling-da added 9 commits October 6, 2022 11:12

added getTransaction with contract type companion

b76ff64

ACS using contract type companion

7269048

Merge remote-tracking branch 'origin/main' into 14969-typed-acs-and-t…

ff2cde0

…ransaction-streams # Conflicts: # language-support/java/bindings/src/main/java/com/daml/ledger/javaapi/data/codegen/InterfaceCompanion.java

position

14af3cc

add change log

1057819

CHANGELOG_BEGIN CHANGELOG_END

Ct

ac6fca6

Ct

783383c

format

6532d5d

better exception

85d8973

chunlokling-da marked this pull request as ready for review October 6, 2022 11:22

chunlokling-da requested a review from ray-roestenburg-da as a code owner October 6, 2022 11:22

fixed test cases

99868fb

chunlokling-da changed the title ~~added transactionFilter~~ Typed ACS and transaction streams for Java codegen Oct 6, 2022

chunlokling-da requested a review from a team October 6, 2022 14:13

chunlokling-da added component/java-ecosystem Java development experience team/ledger-clients Related to the Ledger Clients team's components. ledger clients facade enriching the ledger API by bindings/codegen, JSON API, custom view labels Oct 6, 2022

ray-roestenburg-da approved these changes Oct 6, 2022

View reviewed changes

S11001001 reviewed Oct 6, 2022

View reviewed changes

S11001001 mentioned this pull request Oct 7, 2022

use typed ACS/transaction streams in quickstart-java examples #15191

Closed

7 tasks

chunlokling-da added 8 commits October 10, 2022 19:26

added java doc and rename method to GetContracts

bc65de3

address Stephen's comments

ecb1ea8

address Stephen's comments

ec9fc8f

address Stephen's comments

f9c0fa4

remove unused import

7b45bd3

format

f8f58ba

address Stephen's comments

2e00060

S11001001 reviewed Oct 11, 2022

View reviewed changes

chunlokling-da added 2 commits October 11, 2022 19:57

address Stephen's comments

73ee886

Make some codegen code to be java 8 compatible; Modify IouMain to use…

5e70489

… the new getActiveContracts. to address Stephen's comments

chunlokling-da requested review from stefanobaghino-da, a team and hrischuk-da as code owners October 12, 2022 10:32

chunlokling-da added 2 commits October 12, 2022 17:17

use 1.11, no need to be java 8 compatible

c8b2200

revert

922d1fd

S11001001 approved these changes Oct 12, 2022

View reviewed changes

11 not 1.11

24bbec2

chunlokling-da merged commit 74dd242 into main Oct 12, 2022

chunlokling-da deleted the 14969-typed-acs-and-transaction-streams branch October 12, 2022 20:01

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Typed ACS and transaction streams for Java codegen #15159

Typed ACS and transaction streams for Java codegen #15159

chunlokling-da commented Oct 4, 2022

chunlokling-da Oct 4, 2022 •

edited

Loading

S11001001 Oct 5, 2022

S11001001 Oct 5, 2022

ray-roestenburg-da left a comment

S11001001 commented Oct 7, 2022

S11001001 Oct 11, 2022

chunlokling-da Oct 12, 2022

S11001001 commented Oct 12, 2022 •

edited by chunlokling-da

Loading

S11001001 commented Oct 12, 2022

S11001001 Oct 12, 2022

S11001001 Oct 12, 2022

S11001001 Oct 12, 2022

chunlokling-da Oct 12, 2022

Typed ACS and transaction streams for Java codegen #15159

Typed ACS and transaction streams for Java codegen #15159

Conversation

chunlokling-da commented Oct 4, 2022

chunlokling-da Oct 4, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ray-roestenburg-da left a comment

Choose a reason for hiding this comment

S11001001 commented Oct 7, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

S11001001 commented Oct 12, 2022 • edited by chunlokling-da Loading

S11001001 commented Oct 12, 2022

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

chunlokling-da Oct 4, 2022 •

edited

Loading

S11001001 commented Oct 12, 2022 •

edited by chunlokling-da

Loading