Skip to content

Commit

Permalink
Merge pull request #48 from timothyjward/fix/generics
Browse files Browse the repository at this point in the history 
[generics] Fix broken signatures in proxy classes
  • Loading branch information
@juergen-albert
juergen-albert authored Sep 23, 2024
2 parents 3a5d80e + aa9d8e8 commit b0de24e
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 b0de24e

Please sign in to comment.