feat: Support more services on Java #504

robin-aws · 2024-08-06T22:56:39Z

Description of changes:
Tweaking several aspects of Java codegen to generalize more effectively beyond DDB and KMS:

Implement a general pattern for determining the base exception and client typesRE for a service based on the sdkId, rather than hardcoding only DDB or KMS.
Removed the shapeRequiresTypeConversionFromStringToStructure special casing, as it didn't work correctly for other services, and it actually turns out that the KMS project in the MPL was already patching it back to the correct thing anyway :)
Handling more name transformations generally:
- Using Utils.unCapitalize from the awssdk:codegen package instead of our own copy, to match names better.
- Adding Request/Response to the name of the operation, instead of always replacing Input/Output suffixes of any structure.
Switched the lakeformation test model to omit the GetWorkUnitResults operation, since it has a streaming result. This is specifically a blocker on Java because the result type becomes a ResponseInputStream<GetWorkUnitResultsResponse> instead of a GetWorkUnitResultsResponse, so the shim code doesn't compile otherwise.

I verified that regenerating the DDB and KMS java code in MPL does not change on this version, aside from making a patching unnecessary as above.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

(should be looking at operation name instead)

…obin-aws/support-more-services-on-java

texastony · 2024-10-03T13:55:45Z

...-codegen/src/main/java/software/amazon/polymorph/smithyjava/nameresolver/AwsSdkNativeV2.java

@@ -162,7 +152,7 @@ private TypeName typeForShapeNoEnum(ShapeId shapeId) {
    final Shape shape = model.expectShape(shapeId);

    if (shape.hasTrait(EnumTrait.class)) {
-      if (shapeRequiresTypeConversionFromStringToStructure(shapeId)) {
+      if (true || shapeRequiresTypeConversionFromStringToStructure(shapeId)) {


An or with a constant true is illogical.

Is there a TODO here? Or should we just remove the if?

Implicit TODO, I was experimenting with whether we needed this test anymore. Will resolve before marking the PR actually ready :)

Yeah... there was an edge case with Enums and the Java SDK
that I used this to work around.

I swear I described it somewhere in Java codegen...

texastony · 2024-10-04T16:41:37Z

SmithyDafnyMakefile.mk

@@ -525,6 +525,7 @@ transpile_test_java: _transpile_test_all _mv_test_java
 # To avoid `java/implementation-java` the code is generated and then moved.
 _mv_implementation_java:
 	rm -rf runtimes/java/src/main/dafny-generated
+	mkdir -p runtimes/java/src/main


Is this right?
If you run mkdir against an existing directory, does it succeed?
Or does it fail?

It succeeds - -p makes it recursively create all missing directories, so it's perfectly happy if all already exist

Notice the same thing is already on the runtimes/java/src/test directory. I actually don't know how we got away without this fix for so long. :)

Because you did not use to generate the template files,
and we had to manually stub the __default.java files in the Java extern source path.

Thus, we always had a directory.

alex-chew · 2024-10-08T19:44:04Z

...hy-dafny-codegen/src/main/java/software/amazon/polymorph/smithyjava/nameresolver/Native.java

@@ -378,4 +372,8 @@ public ClassName baseErrorForService() {
      "Use `ClassName.get(RuntimeException.class)` instead."
    );
  }
+
+  protected ClassName classNameForAwsSdkShape(final Shape shape) {
+    throw new UnsupportedOperationException();


Can we add a message here to explain why it's unsupported?

Did you one better and made Native abstract so this could be abstract too. :)

Scratch that, the Native class is used directly for non SDK-style projects as well. I reverted and added an exception message instead. The better change would be to have an intermediate base class in between Native and AwsSdkNativeV1/2, but that looks like a substantial refactoring.

alex-chew · 2024-10-08T19:44:55Z

...-codegen/src/main/java/software/amazon/polymorph/smithyjava/nameresolver/AwsSdkNativeV2.java

-      );
-    }
-  }
+  private static void checkForAwsServiceConstants(String namespace) {}


Is there a reason to keep this?

I had initially thought so, since although I've removed all the checks necessary there could be some in the future. But on second thought, I think those kinds of checks should be model validations instead anyway, so I've removed this.

These should be validations instead if we add some

This reverts commit f87f309. # Conflicts: # codegen/smithy-dafny-codegen/src/main/java/software/amazon/polymorph/smithyjava/generator/library/JavaLibrary.java

…smithy-lang/smithy-dafny into robin-aws/support-more-services-on-java

*Description of changes:* Tweaking several aspects of Java codegen to generalize more effectively beyond DDB and KMS: * Implement a general pattern for determining the base exception and client typesRE for a service based on the `sdkId`, rather than hardcoding only DDB or KMS. * Removed the `shapeRequiresTypeConversionFromStringToStructure` special casing, as it didn't work correctly for other services, and it actually turns out that the KMS project in the MPL was already patching it back to the correct thing anyway :) * Handling more name transformations generally: * Using `Utils.unCapitalize` from the `awssdk:codegen` package instead of our own copy, to match names better. * Adding `Request`/`Response` to the name of the operation, instead of always replacing `Input`/`Output` suffixes of any structure. * Switched the lakeformation test model to omit the `GetWorkUnitResults` operation, since it has a streaming result. This is specifically a blocker on Java because the result type becomes a `ResponseInputStream<GetWorkUnitResultsResponse>` instead of a `GetWorkUnitResultsResponse`, so the shim code doesn't compile otherwise. I verified that regenerating the DDB and KMS java code in MPL does not change on this version, aside from making a patching unnecessary as above.

robin-aws added 12 commits July 12, 2024 16:43

Quick fixes to name discovery

323b919

Fix a case that needs pascalCase

8018081

Another case

19a7bdc

Avoid typeforListSetOrMapNoEnum for now

7c53e2a

More correct unCapitalize

ea422d1

Disable Input -> Request rules for now

a04a1bd

(should be looking at operation name instead)

Enable glue and lakeformation in CI on Java

d1548c7

operation + Request/Response instead

b997983

Try different solution to list of enums

9a2107f

Fix most unit tests

d9c08d2

Merge branch 'main-1.x' of github.com:smithy-lang/smithy-dafny into r…

222e20a

…obin-aws/support-more-services-on-java

Merge branch 'main-1.x' into robin-aws/support-more-services-on-java

f82dfe8

texastony reviewed Oct 3, 2024

View reviewed changes

robin-aws added 3 commits October 3, 2024 14:55

Makefile tweak

dd9f308

Filter out operation that uses streaming, typo

e557380

Fix dotnet

ec2b23d

texastony reviewed Oct 4, 2024

View reviewed changes

robin-aws added 6 commits October 4, 2024 10:16

Rename Model to model (1/2)

b0ae9f9

Rename Model to model (2/2)

0394f53

Fixes

23089b5

Formatting

d94e8fd

Fix build file

7110728

Remove dead code

496df4c

robin-aws changed the title ~~Robin aws/support more services on java~~ feat: Support more services on Java Oct 4, 2024

Unused imports

3819333

robin-aws marked this pull request as ready for review October 4, 2024 20:37

robin-aws requested a review from a team as a code owner October 4, 2024 20:37

Merge branch 'main-1.x' into robin-aws/support-more-services-on-java

3ae96db

alex-chew reviewed Oct 8, 2024

View reviewed changes

Removing hooks for AWS service limitations

0ff9a8d

These should be validations instead if we add some

robin-aws added 7 commits October 9, 2024 09:02

Make Native abstract

f87f309

Formatting

8a07215

Merge branch 'main-1.x' into robin-aws/support-more-services-on-java

1e3d68f

Revert "Make Native abstract"

69c149a

This reverts commit f87f309. # Conflicts: # codegen/smithy-dafny-codegen/src/main/java/software/amazon/polymorph/smithyjava/generator/library/JavaLibrary.java

Add exception message instead

a8b7347

Merge branch 'robin-aws/support-more-services-on-java' of github.com:…

f9ecbd8

…smithy-lang/smithy-dafny into robin-aws/support-more-services-on-java

Formatting

6edde6d

alex-chew approved these changes Oct 9, 2024

View reviewed changes

robin-aws merged commit 6268c40 into main-1.x Oct 9, 2024
80 checks passed

robin-aws deleted the robin-aws/support-more-services-on-java branch October 9, 2024 17:26

robin-aws mentioned this pull request Oct 10, 2024

fix: Provide default implementation for Native.classNameForAwsSdkShape() #629

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Support more services on Java #504

feat: Support more services on Java #504

robin-aws commented Aug 6, 2024 •

edited

Loading

texastony Oct 3, 2024

robin-aws Oct 3, 2024

texastony Oct 3, 2024

texastony Oct 4, 2024

robin-aws Oct 4, 2024

robin-aws Oct 4, 2024

texastony Oct 7, 2024

alex-chew Oct 8, 2024

robin-aws Oct 9, 2024

robin-aws Oct 9, 2024

alex-chew Oct 8, 2024

robin-aws Oct 9, 2024

feat: Support more services on Java #504

feat: Support more services on Java #504

Conversation

robin-aws commented Aug 6, 2024 • 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

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

robin-aws commented Aug 6, 2024 •

edited

Loading