Skip to content

Commit

Permalink
[generics] Fix broken signatures in proxy classes
Browse files Browse the repository at this point in the history 
Some generic mappings were creating partially valid signatures which terminated early, losing generics information for additional contracts. These tests and fix ensure that the signature is generated correctly

Signed-off-by: Tim Ward <[email protected]>
  • Loading branch information
@timothyjward
timothyjward committed Sep 23, 2024
1 parent 3a5d80e commit aa9d8e8
Show file tree
Hide file tree
Showing 2 changed files with 232 additions and 6 deletions.
80 changes: 74 additions & 6 deletions ...se.osgitech.rest/src/main/java/org/eclipse/osgitech/rest/proxy/ExtensionProxyFactory.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,11 @@ public class ExtensionProxyFactory {
private static final String OBJECT_INTERNAL_NAME = Type.getInternalName(Object.class);

/**
* @param simpleName
* Generate a proxy class which copies the signature of the delegate
*
* @param className - the name to use for the new class
* @param delegate - the object to proxy
* @param contracts - the extension contracts to honour
*/
public static byte[] generateClass(String className, Object delegate, List<Class<?>> contracts) {
Class<? extends Object> delegateClazz = delegate.getClass();
Expand Down Expand Up @@ -141,6 +145,15 @@ public static byte[] generateClass(String className, Object delegate, List<Class
}
}

/**
* Gather information about the type variables and generic superclasses for
* the supplied class
*
* @param toCheck - the class to check
* @param typeInfo - the known type name to type information mapping (modified by this method)
* @param contextMapping - A mapping of type names to the class which defines them (modified by this method)
* @param remainingContracts - the extension contracts left to be checked
*/
private static void populateInterfacesAndGenericSuperclasses(Class<? extends Object> toCheck,
Map<String, ParameterizedType> typeInfo, Map<String, String> contextMapping, Set<Class<?>> remainingContracts) {
if(toCheck == Object.class) {
Expand Down Expand Up @@ -169,6 +182,16 @@ private static void populateInterfacesAndGenericSuperclasses(Class<? extends Obj
return;
}

/**
* Determine whether the given Type Variable is used (possibly indirectly) in
* one or more contract interfaces
*
* @param tv - the type variable to check
* @param typeInfo - the known type name to type information mapping
* @param context - A mapping of type names to the class which defines them
* @param contracts - the interfaces that we're proxying
* @return true if <code>tv</code> is used in the contracts
*/
private static <T> boolean isUsedInContracts(TypeVariable<Class<T>> tv, Map<String, ParameterizedType> typeInfo, Map<String, String> context, List<Class<?>> contracts) {
BiPredicate<String, String> contractUses = (varName, contextClass) -> contracts.stream()
.filter(c -> contextClass.equals(context.get(c.getName())))
Expand Down Expand Up @@ -203,8 +226,17 @@ private static <T> boolean isUsedInContracts(TypeVariable<Class<T>> tv, Map<Stri
}

/**
* @param typeInfo
* @return
* Generate the Generic class signature for this proxy class.
* <p>
* The syntax is available at <a
* href="https://docs.oracle.com/javase/specs/jvms/se11/html/jvms-4.html#jvms-4.7.9.1"/>
* but we use ASM's generator to help.
*
* @param delegateClazz - the class that we're creating a proxy for
* @param typeInfo - the known type name to type information mapping
* @param context - A mapping of type names to the class which defines them
* @param contracts - the interfaces that we're proxying
* @return the class signature
*/
private static String generateGenericClassSignature(Class<?> delegateClazz, Map<String, ParameterizedType> typeInfo, Map<String, String> context, List<Class<?>> contracts) {
if(typeInfo.isEmpty()) {
Expand Down Expand Up @@ -250,6 +282,13 @@ private static String generateGenericClassSignature(Class<?> delegateClazz, Map<
return writer.toString();
}

/**
* Find the reified type information, if any, for the supplied type variable
*
* @param t - the type to reify
* @param typeInfo - the known type name to type information mapping
* @param context - A mapping of type names to the class which defines them
*/
private static java.lang.reflect.Type getPossibleReifiedTypeFor(TypeVariable<?> tv, Map<String, ParameterizedType> typeInfo, Map<String, String> context) {
Class<?> declaringType = (Class<?>) tv.getGenericDeclaration();

Expand All @@ -271,6 +310,11 @@ private static java.lang.reflect.Type getPossibleReifiedTypeFor(TypeVariable<?>
return tv;
}

/**
* Maps a type variable to a stream of nested variables
* @param t the variable to map
* @return
*/
private static Stream<TypeVariable<?>> toTypeVariables(java.lang.reflect.Type t) {
if(t instanceof Class<?>) {
return Stream.empty();
Expand All @@ -284,6 +328,17 @@ private static Stream<TypeVariable<?>> toTypeVariables(java.lang.reflect.Type t)
}
}

/**
* Fill in the signature for the supplied type variable
* <p>
* Note that we don't visit the end of any type parameter that we create. This is
* expected to be handled by the caller closing their SignatureVisitor
*
* @param t - the type to visit
* @param sv - the visitor to update with type information
* @param typeInfo - the known type name to type information mapping
* @param context - A mapping of type names to the class which defines them
*/
private static void visitTypeParameter(java.lang.reflect.Type t, SignatureVisitor sv, Map<String, ParameterizedType> typeInfo, Map<String, String> context) {
if(t instanceof Class<?>) {
Class<?> clazz = (Class<?>) t;
Expand All @@ -292,7 +347,7 @@ private static void visitTypeParameter(java.lang.reflect.Type t, SignatureVisito
} else if (clazz.isArray()) {
SignatureVisitor av = sv.visitArrayType();
visitTypeParameter(clazz.getComponentType(), av, typeInfo, context);
av.visitEnd();
// Do not visit the end
} else {
sv.visitClassType(Type.getInternalName(clazz));
}
Expand All @@ -302,6 +357,7 @@ private static void visitTypeParameter(java.lang.reflect.Type t, SignatureVisito
Arrays.stream(pt.getActualTypeArguments()).forEach(ta -> {
SignatureVisitor tav = sv.visitTypeArgument(SignatureVisitor.INSTANCEOF);
visitTypeParameter(ta, tav, typeInfo, context);
// Here we must visit the end as we created a new class type context
tav.visitEnd();
});
} else if (t instanceof TypeVariable<?>) {
Expand All @@ -312,7 +368,7 @@ private static void visitTypeParameter(java.lang.reflect.Type t, SignatureVisito
} else {
SignatureVisitor tav = sv.visitTypeArgument(SignatureVisitor.INSTANCEOF);
visitTypeParameter(t, tav, typeInfo, context);
tav.visitEnd();
// Do not visit the end
}
} else if (t instanceof WildcardType) {
WildcardType wt = (WildcardType) t;
Expand All @@ -326,12 +382,22 @@ private static void visitTypeParameter(java.lang.reflect.Type t, SignatureVisito
types = wt.getUpperBounds();
}
Arrays.stream(types).forEach(ty -> visitTypeParameter(ty, tav, typeInfo, context));
tav.visitEnd();
// Do not visit the end
} else {
throw new IllegalArgumentException("Unhandled generic type " + t.getClass() + " " + t.toString());
}
}

/**
* Visit the member information of a defined annotation and write it into the
* class file.
*
* @param a - the annotation to copy from
* @param av - the visitor to copy into
* @throws IllegalAccessException
* @throws IllegalArgumentException
* @throws InvocationTargetException
*/
private static void visitAnnotationMembers(Annotation a, AnnotationVisitor av) throws IllegalAccessException, IllegalArgumentException, InvocationTargetException {
for (Method method : a.annotationType().getDeclaredMethods()) {
Class<?> returnType = method.getReturnType();
Expand Down Expand Up @@ -367,6 +433,8 @@ private static void visitAnnotationMembers(Annotation a, AnnotationVisitor av) t
}

/**
* Get the simple name for this generated class
*
* @return
*/
public static String getSimpleName(Integer rank, Long id) {
Expand Down
158 changes: 158 additions & 0 deletions ...lipse.osgitech.rest/src/test/java/org/eclipse/osgitech/rest/proxy/ExtensionProxyTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -550,6 +550,100 @@ public void testMultiInterfaceMapper() throws Exception {

}

@SuppressWarnings({ "unchecked", "rawtypes" })
@Test
public void testMultiInterfaceGenericComplex() throws Exception {

TestMultiInterfaceGenericComplex<Object, Object> mi = new TestMultiInterfaceGenericComplex<>();

// Deliberately re-order the interfaces relative to the implements clause
Class<?> proxyClazz = pcl.define("test.MultiInterfaceGenericComplex", mi,
Arrays.asList(MessageBodyWriter.class, MessageBodyReader.class));

assertTrue(MessageBodyReader.class.isAssignableFrom(proxyClazz));
assertTrue(MessageBodyWriter.class.isAssignableFrom(proxyClazz));
Type[] genericInterfaces = proxyClazz.getGenericInterfaces();

assertEquals(2, genericInterfaces.length);

assertTrue(genericInterfaces[0] instanceof ParameterizedType);
ParameterizedType pt = (ParameterizedType) genericInterfaces[0];
assertEquals(MessageBodyWriter.class, pt.getRawType());
TypeVariable<?> tv = (TypeVariable) pt.getActualTypeArguments()[0];
assertEquals("W", tv.getName());
assertArrayEquals(new Type[] {Object.class}, tv.getBounds());

assertTrue(genericInterfaces[1] instanceof ParameterizedType);
pt = (ParameterizedType) genericInterfaces[1];
assertEquals(MessageBodyReader.class, pt.getRawType());
tv = (TypeVariable) pt.getActualTypeArguments()[0];
assertEquals("R", tv.getName());
assertArrayEquals(new Type[] {Object.class}, tv.getBounds());


Object instance = proxyClazz.getConstructor(Supplier.class)
.newInstance((Supplier<?>) () -> mi);

ByteArrayOutputStream baos = new ByteArrayOutputStream();
((MessageBodyWriter) instance).writeTo("ignore me", Object.class, null, null, null, null, baos);

assertArrayEquals(new byte[]{0x42}, baos.toByteArray());


ByteArrayInputStream bais = new ByteArrayInputStream(baos.toByteArray());


assertEquals("tada", ((MessageBodyReader) instance).readFrom(Object.class, null, null, null, null, bais));

}

@SuppressWarnings({ "unchecked", "rawtypes" })
@Test
public void testMultiInterfaceGenericComplexTwo() throws Exception {

TestMultiInterfaceGenericComplexTwo<Integer, String> mi = new TestMultiInterfaceGenericComplexTwo<>();

// Deliberately re-order the interfaces relative to the implements clause
Class<?> proxyClazz = pcl.define("test.MultiInterfaceGenericComplexTwo", mi,
Arrays.asList(MessageBodyWriter.class, MessageBodyReader.class));

assertTrue(MessageBodyReader.class.isAssignableFrom(proxyClazz));
assertTrue(MessageBodyWriter.class.isAssignableFrom(proxyClazz));
Type[] genericInterfaces = proxyClazz.getGenericInterfaces();

assertEquals(2, genericInterfaces.length);

assertTrue(genericInterfaces[0] instanceof ParameterizedType);
ParameterizedType pt = (ParameterizedType) genericInterfaces[0];
assertEquals(MessageBodyWriter.class, pt.getRawType());
TypeVariable<?> tv = (TypeVariable) pt.getActualTypeArguments()[0];
assertEquals("W", tv.getName());
assertArrayEquals(new Type[] {CharSequence.class}, tv.getBounds());

assertTrue(genericInterfaces[1] instanceof ParameterizedType);
pt = (ParameterizedType) genericInterfaces[1];
assertEquals(MessageBodyReader.class, pt.getRawType());
tv = (TypeVariable) pt.getActualTypeArguments()[0];
assertEquals("R", tv.getName());
assertArrayEquals(new Type[] {Number.class}, tv.getBounds());

Object instance = proxyClazz.getConstructor(Supplier.class)
.newInstance((Supplier<?>) () -> mi);

ByteArrayOutputStream baos = new ByteArrayOutputStream();
((MessageBodyWriter) instance).writeTo("banana", CharSequence.class, null, null, null, null, baos);

// 4 characters, "anan"
assertArrayEquals(new byte[]{0,4,97,110,97,110}, baos.toByteArray());


ByteArrayInputStream bais = new ByteArrayInputStream(baos.toByteArray());


assertEquals(17, ((MessageBodyReader) instance).readFrom(Number.class, null, null, null, null, bais));

}

public static class TestExceptionMapper implements ExceptionMapper<NullPointerException> {

/*
Expand Down Expand Up @@ -739,7 +833,71 @@ public Integer readFrom(Class<Integer> type, Type genericType, Annotation[] anno
}
}
}

public static abstract class BaseTestMultiInterfaceGenericCompelex<R, W>
implements MessageBodyReader<R>, MessageBodyWriter<W> {

}

public static class TestMultiInterfaceGenericComplex<R extends Object, W extends Object>
extends BaseTestMultiInterfaceGenericCompelex<R, W> {

@Override
public boolean isWriteable(Class<?> type, Type genericType, Annotation[] annotations, MediaType mediaType) {
return false;
}

@Override
public void writeTo(W t, Class<?> type, Type genericType, Annotation[] annotations, MediaType mediaType,
MultivaluedMap<String, Object> httpHeaders, OutputStream entityStream)
throws IOException, WebApplicationException {
entityStream.write(0x42);
}

@Override
public boolean isReadable(Class<?> type, Type genericType, Annotation[] annotations, MediaType mediaType) {
return false;
}

@SuppressWarnings("unchecked")
@Override
public R readFrom(Class<R> type, Type genericType, Annotation[] annotations, MediaType mediaType,
MultivaluedMap<String, String> httpHeaders, InputStream entityStream)
throws IOException, WebApplicationException {
return (R) "tada";
}
}

public static class TestMultiInterfaceGenericComplexTwo<R extends Number, W extends CharSequence>
extends BaseTestMultiInterfaceGenericCompelex<R, W> {

@Override
public boolean isWriteable(Class<?> type, Type genericType, Annotation[] annotations, MediaType mediaType) {
return false;
}

@Override
public void writeTo(W t, Class<?> type, Type genericType, Annotation[] annotations, MediaType mediaType,
MultivaluedMap<String, Object> httpHeaders, OutputStream entityStream)
throws IOException, WebApplicationException {
try(DataOutputStream daos = new DataOutputStream(entityStream)) {
daos.writeUTF(t.subSequence(1, t.length() - 1).toString());
}
}

@Override
public boolean isReadable(Class<?> type, Type genericType, Annotation[] annotations, MediaType mediaType) {
return false;
}

@SuppressWarnings("unchecked")
@Override
public R readFrom(Class<R> type, Type genericType, Annotation[] annotations, MediaType mediaType,
MultivaluedMap<String, String> httpHeaders, InputStream entityStream)
throws IOException, WebApplicationException {
return (R) Integer.valueOf(17);
}
}

public static class PublicClassLoader extends ClassLoader {

Expand Down

0 comments on commit aa9d8e8

Please sign in to comment.