Skip to content

Commit

Permalink
aaberg#314: Simplified the code to simply skip over bridge methods
Browse files Browse the repository at this point in the history
  • Loading branch information
@mvysny
mvysny committed Jan 28, 2019
1 parent 130cb1f commit 95a4c92
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 61 deletions.
22 changes: 6 additions & 16 deletions core/src/main/java/org/sql2o/reflection/PojoMetadata.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,10 @@ private PropertyAndFieldInfo initializePropertyInfo() {

// prepare methods. Methods will override fields, if both exists.
for (Method m : theClass.getDeclaredMethods()) {
if (m.isBridge()) {
// skip bridge methods: https://github.com/aaberg/sql2o/issues/314
continue;
}

if (m.getName().startsWith("get")) {
String propertyName = m.getName().substring(3);
Expand All @@ -123,14 +127,7 @@ private PropertyAndFieldInfo initializePropertyInfo() {
} else {
propertyName = propertyName.toLowerCase();
}

final Getter existingGetter = propertyGetters.get(propertyName);
if (existingGetter != null && existingGetter.getType() != Object.class && m.getReturnType() == Object.class) {
// don't overwrite existing getter if it has more concrete type. See https://github.com/aaberg/sql2o/issues/314
// for more details.
} else {
propertyGetters.put(propertyName, factoryFacade.newGetter(m));
}
propertyGetters.put(propertyName, factoryFacade.newGetter(m));
}

if (m.getName().startsWith("set") && m.getParameterTypes().length == 1) {
Expand All @@ -143,14 +140,7 @@ private PropertyAndFieldInfo initializePropertyInfo() {
} else {
propertyName = propertyName.toLowerCase();
}

final Setter existingSetter = propertySetters.get(propertyName);
if (existingSetter != null && existingSetter.getType() != Object.class && m.getParameterTypes()[0] == Object.class) {
// don't overwrite existing setter if it has more concrete type. See https://github.com/aaberg/sql2o/issues/314
// for more details.
} else {
propertySetters.put(propertyName, factoryFacade.newSetter(m));
}
propertySetters.put(propertyName, factoryFacade.newSetter(m));
}
}
theClass = theClass.getSuperclass();
Expand Down
58 changes: 13 additions & 45 deletions core/src/test/java/org/sql2o/reflection/PojoMetadataTest.java
View file
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package org.sql2o.reflection;

import javassist.*;
import org.junit.AssumptionViolatedException;
import org.junit.Test;

import java.util.HashMap;
Expand All @@ -20,20 +19,7 @@ public class PojoMetadataTest {
*/
@Test
public void testPrefersGetterWithMoreConcreteType() throws Exception {
Class<?> clazz = createClassWithGetters(false);
if (clazz.getDeclaredMethods()[0].getReturnType() == Object.class) {
// the method order is undefined :( We need to have two methods, first one returning String, second one
// returning Object. The old PojoMetadata would incorrectly use the latter; the fixed version would use the
// function returning more concrete result.
//
// try to create the class with reversed method ordering, maybe it helps?
clazz = createClassWithGetters(true);
if (clazz.getDeclaredMethods()[0].getReturnType() == Object.class) {
// nah, didn't help. bail out.
throw new AssumptionViolatedException("Can't enforce method order");
}
}

Class<?> clazz = createClassWithGetters();
final PojoMetadata metadata = new PojoMetadata(clazz, false, false, new HashMap<String, String>(), true);
assertEquals(String.class, metadata.getPropertyGetter("id").getType());
}
Expand All @@ -44,53 +30,35 @@ public void testPrefersGetterWithMoreConcreteType() throws Exception {
*/
@Test
public void testPrefersSetterWithMoreConcreteType() throws Exception {
Class<?> clazz = createClassWithSetters(false);
if (clazz.getDeclaredMethods()[0].getParameterTypes()[0] == Object.class) {
// the method order is undefined :( We need to have two methods, first one returning String, second one
// returning Object. The old PojoMetadata would incorrectly use the latter; the fixed version would use the
// function returning more concrete result.
//
// try to create the class with reversed method ordering, maybe it helps?
clazz = createClassWithSetters(true);
if (clazz.getDeclaredMethods()[0].getParameterTypes()[0] == Object.class) {
// nah, didn't help. bail out.
throw new AssumptionViolatedException("Can't enforce method order");
}
}

Class<?> clazz = createClassWithSetters();
final PojoMetadata metadata = new PojoMetadata(clazz, false, false, new HashMap<String, String>(), true);
assertEquals(String.class, metadata.getPropertySetter("id").getType());
}

private Class<?> createClassWithGetters(boolean objectThenString) throws CannotCompileException, NotFoundException {
private Class<?> createClassWithGetters() throws CannotCompileException, NotFoundException {
final ClassPool pool = ClassPool.getDefault();
final CtClass ctClass = pool.makeClass("my.test.Klass" + UUID.randomUUID().toString().replace("-", ""));
ctClass.addField(new CtField(pool.get("java.lang.String"), "id", ctClass));
if (objectThenString) {
ctClass.addMethod(CtNewMethod.make("public Object getId() { return id; }", ctClass));
ctClass.addMethod(CtNewMethod.make("public String getId() { return id; }", ctClass));
} else {
ctClass.addMethod(CtNewMethod.make("public String getId() { return id; }", ctClass));
ctClass.addMethod(CtNewMethod.make("public Object getId() { return id; }", ctClass));
}
ctClass.addMethod(withBridge(CtNewMethod.make("public Object getId() { return id; }", ctClass)));
ctClass.addMethod(CtNewMethod.make("public String getId() { return id; }", ctClass));
final Class<?> clazz = ctClass.toClass();
assertEquals(2, clazz.getDeclaredMethods().length);
return clazz;
}

private Class<?> createClassWithSetters(boolean objectThenString) throws CannotCompileException, NotFoundException {
private Class<?> createClassWithSetters() throws CannotCompileException, NotFoundException {
final ClassPool pool = ClassPool.getDefault();
final CtClass ctClass = pool.makeClass("my.test.Klass" + UUID.randomUUID().toString().replace("-", ""));
ctClass.addField(new CtField(pool.get("java.lang.Object"), "id", ctClass));
if (objectThenString) {
ctClass.addMethod(CtNewMethod.make("public void setId(Object id) { this.id = id; }", ctClass));
ctClass.addMethod(CtNewMethod.make("public void setId(String id) { this.id = id; }", ctClass));
} else {
ctClass.addMethod(CtNewMethod.make("public void setId(String id) { this.id = id; }", ctClass));
ctClass.addMethod(CtNewMethod.make("public void setId(Object id) { this.id = id; }", ctClass));
}
ctClass.addMethod(withBridge(CtNewMethod.make("public void setId(Object id) { this.id = id; }", ctClass)));
ctClass.addMethod(CtNewMethod.make("public void setId(String id) { this.id = id; }", ctClass));
final Class<?> clazz = ctClass.toClass();
assertEquals(2, clazz.getDeclaredMethods().length);
return clazz;
}

private static CtMethod withBridge(CtMethod method) {
method.setModifiers(method.getModifiers() | 0x00000040); // Modifier.BRIDGE field not public :(
return method;
}
}

0 comments on commit 95a4c92

Please sign in to comment.