Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

(java) anyof union incorrect $jsii.interfaces in JsiiObjectRef #3726

Closed
matusfaro opened this issue Aug 29, 2022 · 5 comments · Fixed by #3730
Closed

(java) anyof union incorrect $jsii.interfaces in JsiiObjectRef #3726

matusfaro opened this issue Aug 29, 2022 · 5 comments · Fixed by #3730
Labels
bug This issue is a bug. needs-triage This issue or PR still needs to be triaged.

Comments

@matusfaro
Copy link

Describe the bug

Initially added to the bug under aws/aws-cdk, but this seems to be the right place.

For anyOf union types like this one:

        "ArtifactManifest": {
            "type": "object",
            "properties": {
                "properties": {
                    "anyOf": [
                        {
                            "$ref": "#/definitions/AwsCloudFormationStackProperties"
                        },
                        {
                            "$ref": "#/definitions/AssetManifestProperties"
                        },
                        {
                            "$ref": "#/definitions/TreeArtifactProperties"
                        },
                        {
                            "$ref": "#/definitions/NestedCloudAssemblyProperties"
                        }
                    ]
                    ...
                },

The Java generated code looks like this:

public interface ArtifactManifest extends software.amazon.jsii.JsiiSerializable {
...
    default @Nullable Object getProperties() {...}

Where Object is a JsiiObject that can only be converted to the first type in the union AwsCloudFormationStackProperties otherwise it throws an exception. To reproduce this issue:

AssemblyManifest assemblyManifest = Manifest.loadAssemblyManifest("manifest.json"));
assemblyManifest.getArtifacts().values().stream()
         // Filter out only asset artifacts
         .filter(artifactManifest -> ArtifactType.ASSET_MANIFEST.equals(artifactManifest.getType()))
         // Expect the properties to be of type AssetManifestProperties
         .map(artifactManifest -> Kernel.get(artifactManifest, "properties", AssetManifestProperties.class))

Which throws:

Caused by: software.amazon.jsii.JsiiException: Type Object or interface(s) @aws-cdk/cloud-assembly-schema.AwsCloudFormationStackProperties, @aws-cdk/cloud-assembly-schema.AwsCloudFormationStackProperties doesn't have a property 'file'
Error: Type Object or interface(s) @aws-cdk/cloud-assembly-schema.AwsCloudFormationStackProperties, @aws-cdk/cloud-assembly-schema.AwsCloudFormationStackProperties doesn't have a property 'file'
    at exports.Kernel._typeInfoForProperty (/private/var/folders/15/6r443qw917d9y61l_sw28wfc0000gn/T/jsii-java-runtime8967244983856758059/lib/program.js:5413:27)
    at exports.Kernel.get (/private/var/folders/15/6r443qw917d9y61l_sw28wfc0000gn/T/jsii-java-runtime8967244983856758059/lib/program.js:5020:96)
    at exports.KernelHost.processRequest (/private/var/folders/15/6r443qw917d9y61l_sw28wfc0000gn/T/jsii-java-runtime8967244983856758059/lib/program.js:6083:36)
    at exports.KernelHost.run (/private/var/folders/15/6r443qw917d9y61l_sw28wfc0000gn/T/jsii-java-runtime8967244983856758059/lib/program.js:6057:48)
    at Immediate._onImmediate (/private/var/folders/15/6r443qw917d9y61l_sw28wfc0000gn/T/jsii-java-runtime8967244983856758059/lib/program.js:6058:46)
    at process.processImmediate (node:internal/timers:471:21)
    at software.amazon.jsii.JsiiRuntime.processErrorResponse (JsiiRuntime.java:124)
    at software.amazon.jsii.JsiiRuntime.requestResponse (JsiiRuntime.java:95)
    at software.amazon.jsii.JsiiClient.getPropertyValue (JsiiClient.java:112)
    at software.amazon.jsii.Kernel.get (Kernel.java:72)
    at software.amazon.awscdk.cloudassembly.schema.AssetManifestProperties$Jsii$Proxy.<init> (AssetManifestProperties.java:144)
    at jdk.internal.reflect.NativeConstructorAccessorImpl.newInstance0 (Native Method)
    at jdk.internal.reflect.NativeConstructorAccessorImpl.newInstance (NativeConstructorAccessorImpl.java:77)
    at jdk.internal.reflect.DelegatingConstructorAccessorImpl.newInstance (DelegatingConstructorAccessorImpl.java:45)
    at java.lang.reflect.Constructor.newInstanceWithCaller (Constructor.java:499)
    at java.lang.reflect.Constructor.newInstance (Constructor.java:480)
    at software.amazon.jsii.JsiiObject.asInterfaceProxy (JsiiObject.java:377)
    at software.amazon.jsii.NativeType$SimpleNativeType.transform (NativeType.java:157)
    at software.amazon.jsii.JsiiObjectMapper.treeToValue (JsiiObjectMapper.java:47)
    at software.amazon.jsii.Kernel.get (Kernel.java:73)
...

When I looked under the hood, the Kernel.get method first takes the JsiiObject and converts it into a JsiiObjectRef which it then tries to expand and throws the exception. The returned JsiiObjectRef is the problem as the token interfaces $jsii.interfaces is pointing to AwsCloudFormationStackProperties instead of the expected AssetManifestProperties.

If anyone is interested in a workaround, I wrote a utillity class residing in the aws package space and simply override the JsiiObjectRef token interface to match the correct class like so:

package software.amazon.jsii
...
public class JsiiUtil {

    @SneakyThrows
    public static <T> T getProperty(Object jsiiObject, String propertyName, Class<T> clazz) {
        // Get property by name similar to Kernel.get()
        final JsiiEngine engine = JsiiEngine.getEngineFor(jsiiObject);
        final JsiiObjectRef objRef = engine.nativeToObjRef(jsiiObject);
        final ObjectNode propertyObjectRefNode = (ObjectNode) engine.getClient().getPropertyValue(objRef, propertyName);

        // Ensure we are requesting the correct fully qualified name of our class
        // This is a workaround for a bug in JSII
        Jsii jsiiAnnotation = clazz.getAnnotation(Jsii.class);
        propertyObjectRefNode.set(JsiiObjectRef.TOKEN_INTERFACES, new ObjectMapper()
                .createArrayNode()
                .add(jsiiAnnotation.fqn()));
        JsiiObjectRef propertyObjectRef = JsiiObjectRef.parse(propertyObjectRefNode);

        // Find out suitable constructor
        Jsii.Proxy proxyAnnotation = clazz.getAnnotation(Jsii.Proxy.class);
        Class<? extends JsiiObject> clazzImpl = proxyAnnotation.value();
        Constructor<? extends JsiiObject> clazzImplConstructor = clazzImpl.getDeclaredConstructor(JsiiObjectRef.class);
        clazzImplConstructor.setAccessible(true);

        // Finally create our instance
        return (T) clazzImplConstructor.newInstance(propertyObjectRef);
    }
}

The source code is here

Expected Behavior

Expected to be able to use anyof union from Java

Current Behavior

Throws exception, can only convert to first type in the union

Reproduction Steps

AssemblyManifest assemblyManifest = Manifest.loadAssemblyManifest("manifest.json"));
assemblyManifest.getArtifacts().values().stream()
         // Filter out only asset artifacts
         .filter(artifactManifest -> ArtifactType.ASSET_MANIFEST.equals(artifactManifest.getType()))
         // Expect the properties to be of type AssetManifestProperties
         .map(artifactManifest -> Kernel.get(artifactManifest, "properties", AssetManifestProperties.class))

manifest.json:

{
  "version": "20.0.0",
  "artifacts": {
    "Tree": {
      "type": "cdk:tree",
      "properties": {
        "file": "tree.json"
      }
    },
    "my-package.assets": {
      "type": "cdk:asset-manifest",
      "properties": {
        "file": "my-package.assets.json",
        "requiresBootstrapStackVersion": 6,
      }
    }
    ...
}

Possible Solution

No response

Additional Information/Context

No response

SDK version used

jsii-runtime 1.65.0 cdk-cloud-assembly 1.167.0

Environment details (OS name and version, etc.)

macos 12.2.1

@matusfaro matusfaro added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Aug 29, 2022
@RomainMuller
Copy link
Contributor

Ah yes... In this case, the value has multiple serialization options (one per struct in the union), and struct serialization happens by-reference, and always succeeds... This is where the interface list assignment is made, and this actually causes a bad assignment to be made here...

RomainMuller added a commit that referenced this issue Aug 29, 2022
Adds an `UnsafeCast.unsafeCast` method to the Jsii runtime for Java that
allows unsafely casting an instance to a managed interface of the user's
choice.

This can be useful when dealing with type unions composed of interfaces
or structs, as there is otherwise no way to convert the instance without
jumping through hoops.

Fixes #3726 (sort of)
@RomainMuller
Copy link
Contributor

If I was to fix the "incorrect fqn assignment", you'd still be stuck. The only "real" solution I can offer at this stage is a utility to user-land cast instances to a different interface type... so I made a PR to this effect.

@RomainMuller
Copy link
Contributor

I'll mention here that we have an open RFC about the type unions problem => aws/aws-cdk-rfcs#193

@matusfaro
Copy link
Author

If I was to fix the "incorrect fqn assignment", you'd still be stuck. The only "real" solution I can offer at this stage is a utility to user-land cast instances to a different interface type... so I made a PR to this effect.

Thanks a lot, I'll use it for now.

@mergify mergify bot closed this as completed in 4a52d4c Aug 30, 2022
@github-actions
Copy link
Contributor

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This issue is a bug. needs-triage This issue or PR still needs to be triaged.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants