jsii icon indicating copy to clipboard operation
jsii copied to clipboard

Typescript union types are not correctly resolved in Java

Open RoKish opened this issue 5 years ago • 7 comments

In Typescript CloudFormationStackArtifact#getAssets returns AssetMetadataEntry[], where AssetMetadataEntry = FileAssetMetadataEntry | ContainerImageAssetMetadataEntry. In Java this method returns a List<Object>, which is, actually, a list of JSII objects. The JSII objects are supposed to reference FileAssetMetadataEntry and ContainerImageAssetMetadataEntry objects from JSII runtime, however the reference type is always FileAssetMetadataEntry:

final Object asset = stackArtifact.getAssets().get(0);
// the reference interface will always be FileAssetMetadataEntry even though it's actually ContainerImageAssetMetadataEntry
JsiiEngine.getInstance().nativeToObjRef(asset).getInterfaces() 

The issue is also actual for software.amazon.awscdk.cloudassembly.schema.MissingContext#getProps. It is supposed to return ContextQueryProperties which is a union type in Typescript, however, the JSII object reference is always of type AmiContextQuery (looks like it always takes the first type in the union type definition as both FileAssetMetadataEntry and AmiContextQuery are defined first.

Reproduction Steps

  1. Define a container image asset in your stack
  2. Synthesize the templates
  3. Try to access repositoryName of the asset:
Object asset = stackArtifact.getAssets().get(0);
JsiiEngine jsiiEngine = JsiiEngine.getInstance();
JsiiObjectRef objectRef = jsiiEngine.nativeToObjRef(asset);
JsiiClient client = jsiiEngine.getClient();
String packaging = client.getPropertyValue(objectRef, "packaging").asText();
assert packaging.equals("container-image");
String repositoryName = client.getPropertyValue(objectRef, "repositoryName").asText(); //causes an exception

Error Log

Error: Type Object or interface(s) @aws-cdk/cloud-assembly-schema.FileAssetMetadataEntry doesn't have a property 'repositoryName'
    at Kernel._typeInfoForProperty (/private/var/folders/g6/wh2_ynfn4nb9wcpv5vjfphww0000gn/T/jsii-java-runtime14150496997154827187/jsii-runtime.js:8205:19)
    at Kernel.get (/private/var/folders/g6/wh2_ynfn4nb9wcpv5vjfphww0000gn/T/jsii-java-runtime14150496997154827187/jsii-runtime.js:7642:25)
    at KernelHost.processRequest (/private/var/folders/g6/wh2_ynfn4nb9wcpv5vjfphww0000gn/T/jsii-java-runtime14150496997154827187/jsii-runtime.js:7388:28)
    at KernelHost.run (/private/var/folders/g6/wh2_ynfn4nb9wcpv5vjfphww0000gn/T/jsii-java-runtime14150496997154827187/jsii-runtime.js:7328:14)
    at Immediate._onImmediate (/private/var/folders/g6/wh2_ynfn4nb9wcpv5vjfphww0000gn/T/jsii-java-runtime14150496997154827187/jsii-runtime.js:7331:37)
    at processImmediate (internal/timers.js:456:21)

Environment

  • CLI Version : 1.41.0
  • Framework Version: 1.41.0
  • OS : macOs Catalina
  • Language : Java

RoKish avatar May 27 '20 07:05 RoKish

Hey!

Thanks for reporting. That looks weird indeed... I'll need to come up with a jsii-level repro to clarify the situation here...

RomainMuller avatar May 28 '20 07:05 RomainMuller

Thank you! Just in case, I've created a repo reproducing the issue: https://github.com/RoKish/cdk-union-type-bug.

RoKish avatar May 28 '20 08:05 RoKish

Thanks for that - this is super helpful.

This issue is actually pretty complex... There is a lack of runtime-available information and the jsii kernel (JS side) is doing its best here... The problem is while I can come up with a solution that possibly fixes this particular instance, a fix that solves the issue generally is a lot more difficult to achieve.


Thinking though some of the options to make this a little nicer (basically, giving you as a user more control over what you can do), it appears the best way would be to surface some Union model out instead of trying to be clever:

  1. An ugly way is to offer a Union class that has a boolean isInstance(Class<?> clazz) as well as an <T> T asType(Class<T> class) throws ClassCastException method.
  2. A less ugly solution would be to code-generate a distinct class for each union we return, with directed methods for each candidate type (boolean isFileAssetMetadataEntry(), FileAssetMetadataEntry asFileAssetMetadataEntry(), ...)
    • The challenge is with finding a way to name those types that does not risk conflicting with another user-defined type. For example in your case here that'd be something like StackArtifact.getAsset$Result
    • We'd also possibly generate tons of duplicated code (which is probably not the biggest problem here)

RomainMuller avatar Jun 09 '20 13:06 RomainMuller

This issue has not received any attention in 1 year. If you want to keep this issue open, please leave a comment below and auto-close will be canceled.

github-actions[bot] avatar Jun 21 '22 12:06 github-actions[bot]

Issue is still present and makes parsing of manifest from Java not possible.

From an ArtifactManifest object, the property properties in java-land is a JsiiObject which after attempting to use as a AssetManifestProperties throws an error since it incorrectly thinks it should be AwsCloudFormationStackProperties.

To reproduce error:

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))

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)
...

matusfaro avatar Aug 28 '22 15:08 matusfaro

I'll mention here that we have an open RFC about the type unions problem => https://github.com/aws/aws-cdk-rfcs/issues/193

RomainMuller avatar Aug 29 '22 13:08 RomainMuller

This is a jsii issue so I'm moving this to that repo.

TheRealAmazonKendra avatar Jan 27 '23 02:01 TheRealAmazonKendra