Skip to content

Commit

Permalink
fix(java): module "<module name>" not found error (#1906)
Browse files Browse the repository at this point in the history
Struct proxies did not trigger loading of their parent module when
passed as a parameter, which then causes the `@jsii/kernel` type checker
to be unable to locate the struct type definition for checking pruposes,
leading to a crash.

This updatates the `JsiiSerializable` serialization process to
proactively load the relevant modules (the call is idempotent and short
circuits if the module is known to already be loaded).

Fixes #1904



---

By submitting this pull request, I confirm that my contribution is made under the terms of the [Apache 2.0 license].

[Apache 2.0 license]: https://www.apache.org/licenses/LICENSE-2.0
  • Loading branch information
RomainMuller authored Aug 19, 2020
1 parent 8c58644 commit d0b9ffd
Show file tree
Hide file tree
Showing 18 changed files with 570 additions and 31 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
using Amazon.JSII.Tests.CalculatorNamespace;
using CompositeOperation = Amazon.JSII.Tests.CalculatorNamespace.Composition.CompositeOperation;
using Amazon.JSII.Tests.CalculatorNamespace.LibNamespace;
using Amazon.JSII.Tests.CalculatorNamespace.BaseOfBaseNamespace;
using Newtonsoft.Json.Linq;
using Xunit;
using Xunit.Abstractions;
Expand Down Expand Up @@ -46,6 +47,14 @@ void IDisposable.Dispose()
_serviceContainerFixture.Dispose();
}

[Fact(DisplayName = Prefix + nameof(UseNestedStruct))]
public void UseNestedStruct()
{
StaticConsumer.Consume(
new Amazon.JSII.Tests.CustomSubmoduleName.NestingClass.NestedStruct { Name = "Bond, James Bond" }
);
}

[Fact(DisplayName = Prefix + nameof(PrimitiveTypes))]
public void PrimitiveTypes()
{
Expand Down
8 changes: 7 additions & 1 deletion packages/@jsii/java-runtime-test/pom.xml.t.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ process.stdout.write(`<?xml version="1.0" encoding="UTF-8"?>
<dependency>
<groupId>org.junit.jupiter</groupId>
<artifactId>junit-jupiter-engine</artifactId>
<version>5.6.1</version>
<version>[5.6.2,6)</version>
<scope>test</scope>
</dependency>
Expand Down Expand Up @@ -83,6 +83,12 @@ process.stdout.write(`<?xml version="1.0" encoding="UTF-8"?>
<generateBackupPoms>false</generateBackupPoms>
</configuration>
</plugin>
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-surefire-plugin</artifactId>
<version>3.0.0-M5</version>
</plugin>
</plugins>
</build>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ public void beforeEach(final ExtensionContext extensionContext) throws Exception
final String prefix = type == MessageInspector.MessageType.Request ? "<" : ">";
try {
trace.add(String.format("%s %s%n", prefix, objectMapper.writeValueAsString(message)));
System.err.printf("%s %s%n", prefix, objectMapper.writerWithDefaultPrettyPrinter().writeValueAsString(message));
} catch (final IOException e) {
throw new UncheckedIOException(e);
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
package software.amazon.jsii;

import java.io.File;
import java.lang.management.ManagementFactory;
import java.net.MalformedURLException;
import java.net.URL;
import java.net.URLClassLoader;
import java.security.AccessController;
Expand Down Expand Up @@ -58,7 +61,7 @@ private static String binaryNameOf(final Class<?> clazz) {
private ReloadingClassLoader(final ClassLoader parent, final Class<?> ...toReload) {
super(
Stream.of(toReload)
.flatMap(clazz -> Stream.of(((URLClassLoader) clazz.getClassLoader()).getURLs()))
.flatMap(clazz -> urlsFromClassLoader(clazz.getClassLoader()))
.toArray(URL[]::new),
parent
);
Expand All @@ -78,4 +81,24 @@ protected Class<?> loadClass(String name, boolean resolve) throws ClassNotFoundE
}
return result;
}

private static Stream<URL> urlsFromClassLoader(final ClassLoader classLoader) {
if (classLoader instanceof URLClassLoader) {
return Stream.of(((URLClassLoader)classLoader).getURLs());
}
// In java >= 9, class loaders may not always be URLClassLoaders, so we need this:
return Stream
.of(ManagementFactory.getRuntimeMXBean()
.getClassPath()
.split(File.pathSeparator))
.map(ReloadingClassLoader::toURL);
}

private static URL toURL(final String classPathEntry) {
try {
return new File(classPathEntry).toURI().toURL();
} catch (final MalformedURLException ex) {
throw new IllegalArgumentException("URL could not be obtained from " + classPathEntry, ex);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,9 @@
import software.amazon.jsii.JsiiException;
import software.amazon.jsii.ReloadingClassLoader;
import software.amazon.jsii.tests.calculator.*;
import software.amazon.jsii.tests.calculator.baseofbase.StaticConsumer;
import software.amazon.jsii.tests.calculator.composition.CompositeOperation;
import software.amazon.jsii.tests.calculator.custom_submodule_name.NestingClass.NestedStruct;
import software.amazon.jsii.tests.calculator.lib.EnumFromScopedModule;
import software.amazon.jsii.tests.calculator.lib.IFriendly;
import software.amazon.jsii.tests.calculator.lib.MyFirstStruct;
Expand Down Expand Up @@ -41,6 +43,15 @@
@SuppressWarnings("deprecated")
@ExtendWith(ComplianceSuiteHarness.class)
public class ComplianceTest {
@Test
public void useNestedStruct() {
StaticConsumer.consume(
new NestedStruct.Builder()
.name("Bond, James Bond")
.build()
);
}

/**
* Verify that we can marshal and unmarshal objects without type information.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ public void setUp() {
jsiiRuntime = new JsiiRuntime();
this.client = jsiiRuntime.getClient();

this.client.loadModule(new software.amazon.jsii.tests.calculator.baseofbase.$Module());
this.client.loadModule(new software.amazon.jsii.tests.calculator.base.$Module());
this.client.loadModule(new software.amazon.jsii.tests.calculator.lib.$Module());
this.client.loadModule(new software.amazon.jsii.tests.calculator.$Module());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -201,11 +201,19 @@ public JsonDeserializer<?> modifyMapLikeDeserializer(DeserializationConfig confi
* Serializer for classes that extend JsiiObject and any other class that implements a jsii interface.
* We use the JsiiSerializable interface as a way to identify "anything jsii-able".
*/
private static final class JsiiSerializer extends JsonSerializer<JsiiSerializable> {
private final class JsiiSerializer extends JsonSerializer<JsiiSerializable> {
@Override
public void serialize(final JsiiSerializable o,
final JsonGenerator jsonGenerator,
final SerializerProvider serializerProvider) throws IOException {
// First, ensure the relevant interfaces' modules have been loaded (in case "o" is a struct instance)
for (final Class<?> iface : o.getClass().getInterfaces()) {
final Jsii jsii = JsiiEngine.tryGetJsiiAnnotation(iface, true);
if (jsii != null) {
getEngine().loadModule(jsii.module());
}
}
// Then dump the JSON out
jsonGenerator.writeTree(o.$jsii$toJson());
}
}
Expand Down
5 changes: 5 additions & 0 deletions packages/@jsii/kernel/test/kernel.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2086,6 +2086,11 @@ async function createCalculatorSandbox(name: string) {

sandbox.traceEnabled = `${process.env.JSII_DEBUG}` === '1';

sandbox.load({
tarball: await preparePackage('@scope/jsii-calc-base-of-base'),
name: '@scope/jsii-calc-base-of-base',
version: calcBaseVersion,
});
sandbox.load({
tarball: await preparePackage('@scope/jsii-calc-base'),
name: '@scope/jsii-calc-base',
Expand Down
19 changes: 14 additions & 5 deletions packages/@jsii/runtime/test/__snapshots__/kernel-host.test.js.snap

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions packages/@jsii/runtime/test/kernel-host.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import { KernelHost, InputOutput, Input, Output } from '../lib';

test('can load libraries from within a callback', () => {
const inout = new TestInputOutput([
{ api: 'load', ...loadRequest('@scope/jsii-calc-base-of-base') },
{ api: 'load', ...loadRequest('@scope/jsii-calc-base') },
{ api: 'load', ...loadRequest('@scope/jsii-calc-lib') },
{
Expand Down
8 changes: 8 additions & 0 deletions packages/@scope/jsii-calc-base-of-base/lib/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,3 +11,11 @@ export class Very {
return 42;
}
}

export class StaticConsumer {
private constructor() {}

public static consume(..._args: any[]): void {
return;
}
}
32 changes: 31 additions & 1 deletion packages/@scope/jsii-calc-base-of-base/test/assembly.jsii
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,36 @@
],
"name": "IVeryBaseInterface"
},
"@scope/jsii-calc-base-of-base.StaticConsumer": {
"assembly": "@scope/jsii-calc-base-of-base",
"fqn": "@scope/jsii-calc-base-of-base.StaticConsumer",
"kind": "class",
"locationInModule": {
"filename": "lib/index.ts",
"line": 15
},
"methods": [
{
"locationInModule": {
"filename": "lib/index.ts",
"line": 18
},
"name": "consume",
"parameters": [
{
"name": "_args",
"type": {
"primitive": "any"
},
"variadic": true
}
],
"static": true,
"variadic": true
}
],
"name": "StaticConsumer"
},
"@scope/jsii-calc-base-of-base.Very": {
"assembly": "@scope/jsii-calc-base-of-base",
"fqn": "@scope/jsii-calc-base-of-base.Very",
Expand Down Expand Up @@ -111,5 +141,5 @@
}
},
"version": "0.0.0",
"fingerprint": "v3mwSbKGQ2aa8g0DEKIeaEXh3csX7PX2MJJxHVRufhI="
"fingerprint": "DziQGJ8nlNWkjkg4nENQT5/9Gl+eeFGRp1b+csGazr8="
}
7 changes: 7 additions & 0 deletions packages/@scope/jsii-calc-lib/lib/submodule/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,4 +30,11 @@ export namespace NestingClass {
export class NestedClass {
public readonly property: string = 'property';
}

/**
* This is a struct, nested within a class. Normal.
*/
export interface NestedStruct {
readonly name: string;
}
}
36 changes: 35 additions & 1 deletion packages/@scope/jsii-calc-lib/test/assembly.jsii
Original file line number Diff line number Diff line change
Expand Up @@ -625,6 +625,40 @@
}
]
},
"@scope/jsii-calc-lib.submodule.NestingClass.NestedStruct": {
"assembly": "@scope/jsii-calc-lib",
"datatype": true,
"docs": {
"remarks": "Normal.",
"stability": "deprecated",
"summary": "This is a struct, nested within a class."
},
"fqn": "@scope/jsii-calc-lib.submodule.NestingClass.NestedStruct",
"kind": "interface",
"locationInModule": {
"filename": "lib/submodule/index.ts",
"line": 37
},
"name": "NestedStruct",
"namespace": "submodule.NestingClass",
"properties": [
{
"abstract": true,
"docs": {
"stability": "deprecated"
},
"immutable": true,
"locationInModule": {
"filename": "lib/submodule/index.ts",
"line": 38
},
"name": "name",
"type": {
"primitive": "string"
}
}
]
},
"@scope/jsii-calc-lib.submodule.ReflectableEntry": {
"assembly": "@scope/jsii-calc-lib",
"datatype": true,
Expand Down Expand Up @@ -723,5 +757,5 @@
}
},
"version": "0.0.0",
"fingerprint": "f/4VuNiOkSgTgLR80loQUAzAuzFi+25rmfLcRWKDCrY="
"fingerprint": "mi8mRAruztXaT0gYqEr9O6UHWWWkiz7XpCdNrWQMPuA="
}
Loading

0 comments on commit d0b9ffd

Please sign in to comment.