Author: curtisr7
Date: Tue Mar 22 18:23:28 2011
New Revision: 1084280
URL: http://svn.apache.org/viewvc?rev=1084280&view=rev
Log:
OPENJPA-1928: Fix @Factory method resolution when name is ambiguous. Patch
contributed by Edward Sargisson.
Added:
openjpa/trunk/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/meta/TestFactoryMethod.java
(with props)
openjpa/trunk/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/meta/common/apps/MetaTest7.java
(with props)
Modified:
openjpa/trunk/openjpa-kernel/src/main/java/org/apache/openjpa/meta/FieldMetaData.java
Modified:
openjpa/trunk/openjpa-kernel/src/main/java/org/apache/openjpa/meta/FieldMetaData.java
URL:
http://svn.apache.org/viewvc/openjpa/trunk/openjpa-kernel/src/main/java/org/apache/openjpa/meta/FieldMetaData.java?rev=1084280&r1=1084279&r2=1084280&view=diff
==============================================================================
---
openjpa/trunk/openjpa-kernel/src/main/java/org/apache/openjpa/meta/FieldMetaData.java
(original)
+++
openjpa/trunk/openjpa-kernel/src/main/java/org/apache/openjpa/meta/FieldMetaData.java
Tue Mar 22 18:23:28 2011
@@ -1571,7 +1571,7 @@ public class FieldMetaData
_factMethod = getDeclaredType().getConstructor
(new Class[]{ getType() });
else
- _factMethod = findMethod(_factName);
+ _factMethod = findMethodByNameAndType(_factName,
getType());
} catch (OpenJPAException ke) {
throw ke;
} catch (Exception e) {
@@ -1597,6 +1597,22 @@ public class FieldMetaData
* @return the method for invocation
*/
private Method findMethod(String method) {
+ return findMethodByNameAndType(method, null);
+ }
+
+ /**
+ * Find the method for the specified name and type. Possible forms are:
+ * <ul>
+ * <li>toExternalString</li>
+ * <li>MyFactoryClass.toExternalString</li>
+ * <li>com.company.MyFactoryClass.toExternalString</li>
+ * </ul>
+ *
+ * @param method the name of the method to locate
+ * @param type The type of the parameter which will pass the object from
the database.
+ * @return the method for invocation
+ */
+ private Method findMethodByNameAndType(String method, Class<?> type) {
if (StringUtils.isEmpty(method))
return null;
@@ -1629,7 +1645,12 @@ public class FieldMetaData
if (Modifier.isStatic(methods[i].getModifiers())
&& (params.length == 1 || (params.length == 2
&& isStoreContextParameter(params[1]))))
- return methods[i];
+
+ if (type == null) {
+ return methods[i];
+ } else if
(isConvertibleToByMethodInvocationConversion(type, params[0])) {
+ return methods[i];
+ }
if (!Modifier.isStatic(methods[i].getModifiers())
&& (params.length == 0 || (params.length == 1
&& isStoreContextParameter(params[0]))))
@@ -1640,6 +1661,131 @@ public class FieldMetaData
return null;
}
+ /**
+ * Test if the {@code sourceType} is convertible to the {@code
destType}.
+ * Convertible follows the rules in Java Language Specification, 3rd
Ed, s5.3 and means that:
+ * <ul>
+ * <li>{@code sourceType} and {@code destType} are the same type
(identity conversion)</li>
+ * <li>For primitive types: that {@code sourceType} can be widened into
{@code destType}
+ * or that {@code sourceType} can be boxed into a class assignable to
{@code destType}.</li>
+ * <li>For non-primitive types: that the {@code sourceType} can be
unboxed into a primitive
+ * that is the same as, or can be widened into,
+ * {@code destType} or {@code sourceType} can be assigned to {@code
destType}.</li>
+ *
+ * @return True iff the conditions above are true.
+ */
+ private boolean isConvertibleToByMethodInvocationConversion(Class<?>
sourceType, Class<?> destType) {
+ // Note that class.isAssignableFrom is a widening reference
conversion test
+ if (sourceType.isPrimitive()) {
+ return
isConvertibleToByIdentityPrimitiveConversion(sourceType, destType)
+ ||
isConvertibleToByWideningPrimitive(sourceType, destType)
+ || destType.isAssignableFrom(box(sourceType));
+ } else {
+ // Note that unbox will return null if the sourceType
is not a wrapper.
+ // The identity primitive conversion and widening
primitive handle this.
+ return
isConvertibleToByIdentityPrimitiveConversion(unbox(sourceType), destType)
+ ||
isConvertibleToByWideningPrimitive(unbox(sourceType), destType)
+ || destType.isAssignableFrom(sourceType);
+ }
+ }
+
+ /**
+ * @return The results of unboxing {@code sourceType} following Java
Language Specification, 3rd Ed, s5.1.8
+ */
+ private Class<?> unbox(Class<?> sourceType) {
+ if (sourceType == java.lang.Boolean.class) {
+ return java.lang.Boolean.TYPE;
+ } else if (sourceType == java.lang.Byte.class) {
+ return java.lang.Byte.TYPE;
+ } else if (sourceType == java.lang.Short.class) {
+ return java.lang.Short.TYPE;
+ } else if (sourceType == java.lang.Character.class) {
+ return java.lang.Character.TYPE;
+ } else if (sourceType == java.lang.Integer.class) {
+ return java.lang.Integer.TYPE;
+ } else if (sourceType == java.lang.Long.class) {
+ return java.lang.Long.TYPE;
+ } else if (sourceType == java.lang.Float.class) {
+ return java.lang.Float.TYPE;
+ } else if (sourceType == java.lang.Double.class) {
+ return java.lang.Double.TYPE;
+ } else {
+ return null;
+ }
+ }
+
+ /**
+ * @return The results of unboxing {@code sourceType} following Java
Language Specification, 3rd Ed, s5.1.7
+ */
+ private Class<?> box(Class<?> sourceType) {
+ if (sourceType.isPrimitive()) {
+ if (sourceType == java.lang.Boolean.TYPE) {
+ return java.lang.Boolean.class;
+ } else if (sourceType == java.lang.Byte.TYPE) {
+ return java.lang.Byte.class;
+ } else if (sourceType == java.lang.Short.TYPE) {
+ return java.lang.Short.class;
+ } else if (sourceType == java.lang.Character.TYPE) {
+ return java.lang.Character.class;
+ } else if (sourceType == java.lang.Integer.TYPE) {
+ return java.lang.Integer.class;
+ } else if (sourceType == java.lang.Long.TYPE) {
+ return java.lang.Long.class;
+ } else if (sourceType == java.lang.Float.TYPE) {
+ return java.lang.Float.class;
+ } else if (sourceType == java.lang.Double.TYPE) {
+ return java.lang.Double.class;
+ }
+ return null; // Should never be reached because all
primitives are accounted for above.
+ } else {
+ throw new IllegalArgumentException("Cannot box a type
that is not a primitive.");
+ }
+ }
+
+ /**
+ * @return true iff {@sourceType} can be converted by a widening
primitive conversion
+ * following Java Language Specification, 3rd Ed, s5.1.2
+ */
+ private boolean isConvertibleToByWideningPrimitive(Class<?> sourceType,
Class<?> destType) {
+ // Widening conversion following Java Language Specification,
s5.1.2.
+ if (sourceType == java.lang.Byte.TYPE) {
+ return destType == java.lang.Short.TYPE ||
+ destType == java.lang.Integer.TYPE ||
+ destType == java.lang.Long.TYPE ||
+ destType == java.lang.Float.TYPE ||
+ destType == java.lang.Double.TYPE;
+ } else if (sourceType == java.lang.Short.TYPE) {
+ return destType == java.lang.Integer.TYPE ||
+ destType == java.lang.Long.TYPE ||
+ destType == java.lang.Float.TYPE ||
+ destType == java.lang.Double.TYPE;
+ } else if (sourceType == java.lang.Character.TYPE) {
+ return destType == java.lang.Integer.TYPE ||
+ destType == java.lang.Long.TYPE ||
+ destType == java.lang.Float.TYPE ||
+ destType == java.lang.Double.TYPE;
+ } else if (sourceType == java.lang.Integer.TYPE) {
+ return destType == java.lang.Long.TYPE ||
+ destType == java.lang.Float.TYPE ||
+ destType == java.lang.Double.TYPE;
+ } else if (sourceType == java.lang.Long.TYPE) {
+ return destType == java.lang.Float.TYPE ||
+ destType == java.lang.Double.TYPE;
+ } else if (sourceType == java.lang.Float.TYPE) {
+ return destType == java.lang.Double.TYPE;
+ }
+ return false;
+ }
+
+ /**
+ * Returns true iff the sourceType is a primitive that can be converted
to
+ * destType using an identity conversion - i.e. sourceType and destType
are the same type.
+ * following Java Language Specification, 3rd Ed, s5.1.1
+ */
+ private boolean isConvertibleToByIdentityPrimitiveConversion(Class<?>
sourceType, Class<?> destType) {
+ return sourceType != null && sourceType.isPrimitive() &&
sourceType == destType;
+ }
+
/**
* Return true if the given type is a store context type; we can't
* use the standard <code>isAssignableFrom</code> because of classloader
Added:
openjpa/trunk/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/meta/TestFactoryMethod.java
URL:
http://svn.apache.org/viewvc/openjpa/trunk/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/meta/TestFactoryMethod.java?rev=1084280&view=auto
==============================================================================
---
openjpa/trunk/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/meta/TestFactoryMethod.java
(added)
+++
openjpa/trunk/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/meta/TestFactoryMethod.java
Tue Mar 22 18:23:28 2011
@@ -0,0 +1,181 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.openjpa.persistence.meta;
+
+import java.lang.reflect.Member;
+import java.lang.reflect.Method;
+
+import javax.persistence.EntityManager;
+
+import org.apache.openjpa.kernel.Broker;
+import org.apache.openjpa.meta.ClassMetaData;
+import org.apache.openjpa.meta.FieldMetaData;
+import org.apache.openjpa.meta.MetaDataRepository;
+import org.apache.openjpa.persistence.JPAFacadeHelper;
+import org.apache.openjpa.persistence.common.utils.AbstractTestCase;
+import org.apache.openjpa.persistence.meta.common.apps.MetaTest7;
+
+/**
+ * <p>
+ * Tests the {@code getFactoryMethod} method
+ * </p>
+ *
+ */
+public class TestFactoryMethod extends AbstractTestCase {
+
+ private MetaDataRepository _repos = null;
+ private ClassMetaData _metaTest7 = null;
+
+ public TestFactoryMethod(String test) {
+ super(test, "metacactusapp");
+ }
+
+ public void setUp() throws Exception {
+ _repos = getRepository();
+ _metaTest7 = _repos.getMetaData(MetaTest7.class, null, true);
+ }
+
+ protected MetaDataRepository getRepository() throws Exception {
+ // return new
OpenJPAConfigurationImpl().newMetaDataRepositoryInstance();
+ // return getConfiguration().newMetaDataRepositoryInstance();
+ EntityManager em = currentEntityManager();
+ Broker broker = JPAFacadeHelper.toBroker(em);
+ return broker.getConfiguration().newMetaDataRepositoryInstance();
+ }
+
+ /**
+ * Tests that getFactoryMethod() identifies the method using both the name
and the type. This is required when the
+ * factory method is overloaded - i.e. same name but different types.
+ */
+ public void testFactoryMatchesByType() {
+ FieldMetaData fieldMetaData = _metaTest7.getField("status");
+
+ Member factoryMember = fieldMetaData.getFactoryMethod();
+
+ assertEquals("valueOf", factoryMember.getName());
+
+ Method factoryMethod = (Method) factoryMember;
+
+ Class<?>[] parameterTypes = factoryMethod.getParameterTypes();
+ assertEquals("Both valueOf methods take just 1 parameter", 1,
parameterTypes.length);
+ assertEquals("Need to match the type of the underlying field " + "or
the factory method will fail when called",
+ String.class, parameterTypes[0]);
+ }
+
+ /**
+ * Tests that getFactoryMethod() identifies the method using both the name
and the type. This is required when the
+ * factory method is overloaded - i.e. same name but different types. This
tests that an Externalizer returning int
+ * can be matched with a Factory taking long (a widening conversion).
+ */
+ public void testFactoryMatchesByTypeWidening() {
+ FieldMetaData fieldMetaData = _metaTest7.getField("intLongStatus");
+
+ Member factoryMember = fieldMetaData.getFactoryMethod();
+
+ assertEquals("valueOf", factoryMember.getName());
+
+ Method factoryMethod = (Method) factoryMember;
+
+ Class<?>[] parameterTypes = factoryMethod.getParameterTypes();
+ assertEquals("Both valueOf methods take just 1 parameter", 1,
parameterTypes.length);
+ assertEquals("Need to match the type of the underlying field " + "or
the factory method will fail when called",
+ long.class, parameterTypes[0]);
+ }
+
+ /**
+ * Tests that getFactoryMethod() identifies the method using both the name
and the type. This is required when the
+ * factory method is overloaded - i.e. same name but different types. This
tests that an Externalizer returning int
+ * can be matched with a Factory taking Integer (a boxing conversion).
+ */
+ public void testFactoryMatchesByTypeBoxing() {
+ FieldMetaData fieldMetaData = _metaTest7.getField("intIntegerStatus");
+
+ Member factoryMember = fieldMetaData.getFactoryMethod();
+
+ assertEquals("valueOf", factoryMember.getName());
+
+ Method factoryMethod = (Method) factoryMember;
+
+ Class<?>[] parameterTypes = factoryMethod.getParameterTypes();
+ assertEquals("Both valueOf methods take just 1 parameter", 1,
parameterTypes.length);
+ assertEquals("Need to match the type of the underlying field " + "or
the factory method will fail when called",
+ Integer.class, parameterTypes[0]);
+ }
+
+ /**
+ * Tests that getFactoryMethod() identifies the method using both the name
and the type. This is required when the
+ * factory method is overloaded - i.e. same name but different types. This
tests that an Externalizer returning
+ * Integer can be matched with a Factory taking Integer (an identity
conversion).
+ */
+ public void testFactoryMatchesByTypeWrapper() {
+ FieldMetaData fieldMetaData =
_metaTest7.getField("integerIntegerStatus");
+
+ Member factoryMember = fieldMetaData.getFactoryMethod();
+
+ assertEquals("valueOf", factoryMember.getName());
+
+ Method factoryMethod = (Method) factoryMember;
+
+ Class<?>[] parameterTypes = factoryMethod.getParameterTypes();
+ assertEquals("Both valueOf methods take just 1 parameter", 1,
parameterTypes.length);
+ assertEquals("Need to match the type of the underlying field " + "or
the factory method will fail when called",
+ Integer.class, parameterTypes[0]);
+ }
+
+ /**
+ * Tests that getFactoryMethod() identifies the method using both the name
and the type. This is required when the
+ * factory method is overloaded - i.e. same name but different types. This
tests that an Externalizer returning
+ * Integer can be matched with a Factory taking int (an unboxing
conversion).
+ */
+ public void testFactoryMatchesByTypeUnboxing() {
+ FieldMetaData fieldMetaData = _metaTest7.getField("integerIntStatus");
+
+ Member factoryMember = fieldMetaData.getFactoryMethod();
+
+ assertEquals("valueOf", factoryMember.getName());
+
+ Method factoryMethod = (Method) factoryMember;
+
+ Class<?>[] parameterTypes = factoryMethod.getParameterTypes();
+ assertEquals("Both valueOf methods take just 1 parameter", 1,
parameterTypes.length);
+ assertEquals("Need to match the type of the underlying field " + "or
the factory method will fail when called",
+ int.class, parameterTypes[0]);
+ }
+
+ /**
+ * Tests that getFactoryMethod() identifies the method using both the name
and the type. This is required when the
+ * factory method is overloaded - i.e. same name but different types. This
tests that an Externalizer returning
+ * Integer can be matched with a Factory taking long (an unboxing
conversion following by a widening primitive
+ * conversion).
+ */
+ public void testFactoryMatchesByTypeUnboxingWidening() {
+ FieldMetaData fieldMetaData = _metaTest7.getField("integerLongStatus");
+
+ Member factoryMember = fieldMetaData.getFactoryMethod();
+
+ assertEquals("valueOf", factoryMember.getName());
+
+ Method factoryMethod = (Method) factoryMember;
+
+ Class<?>[] parameterTypes = factoryMethod.getParameterTypes();
+ assertEquals("Both valueOf methods take just 1 parameter", 1,
parameterTypes.length);
+ assertEquals("Need to match the type of the underlying field " + "or
the factory method will fail when called",
+ long.class, parameterTypes[0]);
+ }
+}
Propchange:
openjpa/trunk/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/meta/TestFactoryMethod.java
------------------------------------------------------------------------------
svn:eol-style = native
Added:
openjpa/trunk/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/meta/common/apps/MetaTest7.java
URL:
http://svn.apache.org/viewvc/openjpa/trunk/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/meta/common/apps/MetaTest7.java?rev=1084280&view=auto
==============================================================================
---
openjpa/trunk/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/meta/common/apps/MetaTest7.java
(added)
+++
openjpa/trunk/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/meta/common/apps/MetaTest7.java
Tue Mar 22 18:23:28 2011
@@ -0,0 +1,202 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.openjpa.persistence.meta.common.apps;
+
+import javax.persistence.Column;
+import javax.persistence.Entity;
+import javax.persistence.Table;
+
+import org.apache.openjpa.persistence.Externalizer;
+import org.apache.openjpa.persistence.Factory;
+import org.apache.openjpa.persistence.Persistent;
+
+@Entity
+@Table(name = "PER_JDBC_KERN_EMP")
+public class MetaTest7 {
+
+ public long id;
+ private MetaTest7Status status;
+ private MetaTest7IntLongStatus intLongStatus;
+ private MetaTest7IntIntegerStatus intIntegerStatus;
+ private MetaTest7IntegerIntegerStatus integerIntegerStatus;
+ private MetaTest7IntegerIntStatus integerIntStatus;
+ private MetaTest7IntegerLongStatus integerLongStatus;
+
+ @Externalizer("getName")
+ @Persistent(optional = false)
+ @Column(name = "status")
+ @Factory("valueOf")
+ public MetaTest7Status getStatus() {
+ return status;
+ }
+
+ public void setStatus(MetaTest7Status status) {
+ this.status = status;
+ }
+
+ @Externalizer("getName")
+ @Persistent(optional = false)
+ @Column(name = "intLongStatus")
+ @Factory("valueOf")
+ public MetaTest7IntLongStatus getIntLongStatus() {
+ return intLongStatus;
+ }
+
+ public void setIntLongStatus(MetaTest7IntLongStatus status) {
+ this.intLongStatus = status;
+ }
+
+ @Externalizer("getName")
+ @Persistent(optional = false)
+ @Column(name = "intIntegerStatus")
+ @Factory("valueOf")
+ public MetaTest7IntIntegerStatus getIntIntegerStatus() {
+ return intIntegerStatus;
+ }
+
+ public void setIntIntegerStatus(MetaTest7IntIntegerStatus status) {
+ this.intIntegerStatus = status;
+ }
+
+ @Externalizer("getName")
+ @Persistent(optional = false)
+ @Column(name = "integerIntegerStatus")
+ @Factory("valueOf")
+ public MetaTest7IntegerIntegerStatus getIntegerIntegerStatus() {
+ return integerIntegerStatus;
+ }
+
+ public void setIntegerIntegerStatus(MetaTest7IntegerIntegerStatus status) {
+ this.integerIntegerStatus = status;
+ }
+
+ @Externalizer("getName")
+ @Persistent(optional = false)
+ @Column(name = "integerIntStatus")
+ @Factory("valueOf")
+ public MetaTest7IntegerIntStatus getIntegerIntStatus() {
+ return integerIntStatus;
+ }
+
+ public void setIntegerIntStatus(MetaTest7IntegerIntStatus status) {
+ this.integerIntStatus = status;
+ }
+
+ @Externalizer("getName")
+ @Persistent(optional = false)
+ @Column(name = "integerLongStatus")
+ @Factory("valueOf")
+ public MetaTest7IntegerLongStatus getIntegerLongStatus() {
+ return integerLongStatus;
+ }
+
+ public void setIntegerLongStatus(MetaTest7IntegerLongStatus status) {
+ this.integerLongStatus = status;
+ }
+
+ static class MetaTest7IntegerIntegerStatus {
+
+ public Integer getName() {
+ return 0;
+ }
+
+ public static MetaTest7IntegerIntegerStatus valueOf(final Integer
ordinal) {
+ return null;
+ }
+
+ public static MetaTest7IntegerIntegerStatus valueOf(final String name)
{
+ return null;
+ }
+ }
+
+ static class MetaTest7IntegerIntStatus {
+
+ public Integer getName() {
+ return 0;
+ }
+
+ public static MetaTest7IntegerIntStatus valueOf(final int ordinal) {
+ return null;
+ }
+
+ public static MetaTest7IntegerIntStatus valueOf(final String name) {
+ return null;
+ }
+ }
+
+ static class MetaTest7IntegerLongStatus {
+
+ public Integer getName() {
+ return 0;
+ }
+
+ public static MetaTest7IntegerLongStatus valueOf(final long ordinal) {
+ return null;
+ }
+
+ public static MetaTest7IntegerLongStatus valueOf(final String name) {
+ return null;
+ }
+ }
+
+ static class MetaTest7IntIntegerStatus {
+
+ public int getName() {
+ return 0;
+ }
+
+ public static MetaTest7IntIntegerStatus valueOf(final Integer ordinal)
{
+ return null;
+ }
+
+ public static MetaTest7IntIntegerStatus valueOf(final String name) {
+ return null;
+ }
+ }
+
+ static class MetaTest7IntLongStatus {
+
+ public int getName() {
+ return 0;
+ }
+
+ public static MetaTest7IntLongStatus valueOf(final long ordinal) {
+ return null;
+ }
+
+ public static MetaTest7IntLongStatus valueOf(final String name) {
+ return null;
+ }
+ }
+
+ static class MetaTest7Status {
+
+ public String getName() {
+ return null;
+ }
+
+ public static MetaTest7Status valueOf(final int ordinal) {
+ return null;
+ }
+
+ public static MetaTest7Status valueOf(final String name) {
+ return null;
+ }
+ }
+}
Propchange:
openjpa/trunk/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/meta/common/apps/MetaTest7.java
------------------------------------------------------------------------------
svn:eol-style = native
