Skip to content

Commit

Permalink
fix(java): invalid collections returned with non-class elements (#1197)
Browse files Browse the repository at this point in the history
The Java runtime for jsii used to not be able to necessarily hold the
type information necessary to correctly process the contents of
collections, as the generic information is lost.

In order to address this, introduced a new `NativeType<T>` class that is
able to carry:
1. The actual declared type of the value (via generic type parameter)
2. The raw type of the value (reified)
3. The correct JavaType to pass to Jackson to obtain the correct result

It also preserves the specific behavior around de-serialization of
`JsiiObject` sub-classes as well as jsii interfaces, correctly
requesting de-serialization as the `JsiiObject` class, and in a second
phase `transform` this into the correct value type as needed.

Introduced a new test that validates the `List<T>` and `Map<String, T>`
values received from `node` are deserialized to instances with the
appropriate content type.

Fixes #1196
  • Loading branch information
RomainMuller authored and mergify[bot] committed Jan 27, 2020
1 parent 2652043 commit bbc2302
Show file tree
Hide file tree
Showing 79 changed files with 1,338 additions and 269 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -1423,5 +1423,41 @@ protected override string Property
set => _property = $"String<{value}>";
}
}

[Fact(DisplayName = Prefix + nameof(CollectionOfInterfaces_ListOfStructs))]
public void CollectionOfInterfaces_ListOfStructs()
{
foreach (var elt in InterfaceCollections.ListOfStructs())
{
Assert.IsAssignableFrom<IStructA>(elt);
}
}

[Fact(DisplayName = Prefix + nameof(CollectionOfInterfaces_ListOfInterfaces))]
public void CollectionOfInterfaces_ListOfInterfaces()
{
foreach (var elt in InterfaceCollections.ListOfInterfaces())
{
Assert.IsAssignableFrom<IBell>(elt);
}
}

[Fact(DisplayName = Prefix + nameof(CollectionOfInterfaces_MapOfStructs))]
public void CollectionOfInterfaces_MapOfStructs()
{
foreach (var elt in InterfaceCollections.MapOfStructs().Values)
{
Assert.IsAssignableFrom<IStructA>(elt);
}
}

[Fact(DisplayName = Prefix + nameof(CollectionOfInterfaces_MapOfInterfaces))]
public void CollectionOfInterfaces_MapOfInterfaces()
{
foreach (var elt in InterfaceCollections.MapOfInterfaces().Values)
{
Assert.IsAssignableFrom<IBell>(elt);
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -1691,4 +1691,32 @@ protected void setProperty(String value) {

assertEquals("Wrapped<String<Oomf!>>", abstractSuite.workItAll("Oomf!"));
}

@Test
public void collectionOfInterfaces_ListOfStructs() {
for (final Object obj : InterfaceCollections.listOfStructs()) {
assertTrue(obj + " is an instance of " + StructA.class.getCanonicalName(), obj instanceof StructA);
}
}

@Test
public void collectionOfInterfaces_ListOfInterfaces() {
for (final Object obj : InterfaceCollections.listOfInterfaces()) {
assertTrue(obj + " is an instance of " + IBell.class.getCanonicalName(), obj instanceof IBell);
}
}

@Test
public void collectionOfInterfaces_MapOfStructs() {
for (final Object obj : InterfaceCollections.mapOfStructs().values()) {
assertTrue(obj + " is an instance of " + StructA.class.getCanonicalName(), obj instanceof StructA);
}
}

@Test
public void collectionOfInterfaces_MapOfInterfaces() {
for (final Object obj : InterfaceCollections.mapOfInterfaces().values()) {
assertTrue(obj + " is an instance of " + IBell.class.getCanonicalName(), obj instanceof IBell);
}
}
}
4 changes: 2 additions & 2 deletions packages/@jsii/java-runtime/pom.xml.t.js
Original file line number Diff line number Diff line change
Expand Up @@ -113,10 +113,10 @@ process.stdout.write(`<?xml version="1.0" encoding="UTF-8"?>
<scope>test</scope>
</dependency>
<!-- https://mvnrepository.com/artifact/org.mockito/mockito-core -->
<!-- https://mvnrepository.com/artifact/org.mockito/mockito-junit-jupiter -->
<dependency>
<groupId>org.mockito</groupId>
<artifactId>mockito-core</artifactId>
<artifactId>mockito-junit-jupiter</artifactId>
<version>\${mockito.version}</version>
<scope>test</scope>
</dependency>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
import software.amazon.jsii.api.Callback;
import software.amazon.jsii.api.CreateRequest;
import software.amazon.jsii.api.JsiiOverride;
import com.fasterxml.jackson.core.JsonProcessingException;
import com.fasterxml.jackson.databind.JsonNode;
import com.fasterxml.jackson.databind.node.ArrayNode;
import com.fasterxml.jackson.databind.node.JsonNodeFactory;
Expand All @@ -12,7 +11,6 @@
import java.io.IOException;
import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
import java.util.List;

import static software.amazon.jsii.Util.extractResource;
Expand Down Expand Up @@ -226,7 +224,7 @@ public List<Callback> pendingCallbacks() {

List<Callback> result = new ArrayList<>();
callbacksArray.forEach(node -> {
result.add(JsiiObjectMapper.treeToValue(node, Callback.class));
result.add(JsiiObjectMapper.treeToValue(node, NativeType.forClass(Callback.class)));
});

return result;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,7 @@

import java.io.PrintWriter;
import java.io.StringWriter;
import java.lang.reflect.Constructor;
import java.lang.reflect.InvocationTargetException;
import java.lang.reflect.Method;
import java.lang.reflect.Modifier;
import java.lang.reflect.*;
import java.util.*;

import static software.amazon.jsii.Util.isJavaPropertyMethod;
Expand Down Expand Up @@ -345,7 +342,7 @@ private JsonNode invokeCallbackSet(final SetRequest req) {
final String setterMethodName = getterMethodName.replaceFirst("g", "s");
final Method setter = this.findCallbackSetter(obj.getClass(), setterMethodName, getter.getReturnType());

final Object arg = JsiiObjectMapper.treeToValue(req.getValue(), setter.getParameterTypes()[0]);
final Object arg = JsiiObjectMapper.treeToValue(req.getValue(), NativeType.forType(setter.getGenericParameterTypes()[0]));
return JsiiObjectMapper.valueToTree(invokeMethod(obj, setter, arg));
}

Expand All @@ -359,10 +356,10 @@ private JsonNode invokeCallbackMethod(final InvokeRequest req, final String cook
Object obj = this.getObject(req.getObjref());
Method method = this.findCallbackMethod(obj.getClass(), cookie);

final Class<?>[] argTypes = method.getParameterTypes();
final Type[] argTypes = method.getGenericParameterTypes();
final Object[] args = new Object[argTypes.length];
for (int i = 0; i < argTypes.length; i++) {
args[i] = JsiiObjectMapper.treeToValue(req.getArgs().get(i), argTypes[i]);
args[i] = JsiiObjectMapper.treeToValue(req.getArgs().get(i), NativeType.forType(argTypes[i]));
}

return JsiiObjectMapper.valueToTree(invokeMethod(obj, method, args));
Expand Down
Loading

0 comments on commit bbc2302

Please sign in to comment.