From 13dd4f22526a57f27c1b4bbb89619b11af31815a Mon Sep 17 00:00:00 2001 From: Patrick Linskey Date: Wed, 23 Jan 2008 18:15:09 +0000 Subject: [PATCH] Improve error message when checking metadata access types; correct recursive method to always include superclass information. git-svn-id: https://svn.apache.org/repos/asf/openjpa/trunk@614606 13f79535-47bb-0310-9956-ffa450edef68 --- .../meta/AbstractMetaDataDefaults.java | 43 +++++++++++++-- .../apache/openjpa/meta/localizer.properties | 4 ++ .../jdbc/annotations/TestMixedAccess.java | 52 +++++++++++++++++++ .../UnenhancedInappropriateTransient.java | 34 ++++++++++++ .../annotations/UnenhancedMixedAccess.java | 37 +++++++++++++ .../PersistenceMetaDataDefaults.java | 34 ++++++++---- 6 files changed, 192 insertions(+), 12 deletions(-) create mode 100644 openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/jdbc/annotations/TestMixedAccess.java create mode 100644 openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/jdbc/annotations/UnenhancedInappropriateTransient.java create mode 100644 openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/jdbc/annotations/UnenhancedMixedAccess.java diff --git a/openjpa-kernel/src/main/java/org/apache/openjpa/meta/AbstractMetaDataDefaults.java b/openjpa-kernel/src/main/java/org/apache/openjpa/meta/AbstractMetaDataDefaults.java index 762eee919..5a7a5eba0 100644 --- a/openjpa-kernel/src/main/java/org/apache/openjpa/meta/AbstractMetaDataDefaults.java +++ b/openjpa-kernel/src/main/java/org/apache/openjpa/meta/AbstractMetaDataDefaults.java @@ -24,6 +24,7 @@ import java.lang.reflect.Method; import java.lang.reflect.Modifier; import java.security.AccessController; import java.security.PrivilegedActionException; +import java.util.List; import org.apache.openjpa.enhance.PCRegistry; import org.apache.openjpa.enhance.Reflection; @@ -153,9 +154,17 @@ public abstract class AbstractMetaDataDefaults // the same time access = getAccessType(meta); if ((access & ClassMetaData.ACCESS_FIELD) != 0 - && (access & ClassMetaData.ACCESS_PROPERTY) != 0) - throw new UserException(_loc.get("access-field-and-prop", - meta.getDescribedType().getName())); + && (access & ClassMetaData.ACCESS_PROPERTY) != 0) { + List fields = getFieldAccessNames(meta); + List props = getPropertyAccessNames(meta); + if (fields != null || props != null) + throw new UserException(_loc.get( + "access-field-and-prop-hints", + meta.getDescribedType().getName(), fields, props)); + else + throw new UserException(_loc.get("access-field-and-prop", + meta.getDescribedType().getName())); + } } meta.setAccessType(access); @@ -263,6 +272,34 @@ public abstract class AbstractMetaDataDefaults return ClassMetaData.ACCESS_FIELD; } + /** + * Return the list of fields in meta that use field access, + * or null if a list of fields is unobtainable. An empty list + * should be returned if the list of fields is obtainable, but there + * happens to be no field access in meta. + * + * This is used for error reporting purposes only, so need not be efficient. + * + * This implementation returns null. + */ + protected List getFieldAccessNames(ClassMetaData meta) { + return null; + } + + /** + * Return the list of methods in meta that use property access, + * or null if a list of methods is unobtainable. An empty list + * should be returned if the list of methods is obtainable, but there + * happens to be no property access in meta. + * + * This is used for error reporting purposes only, so need not be efficient. + * + * This implementation returns null. + */ + protected List getPropertyAccessNames(ClassMetaData meta) { + return null; + } + /** * Return the field name for the given member. This will only be invoked * on members of the right type (field vs. method). Return null if the diff --git a/openjpa-kernel/src/main/resources/org/apache/openjpa/meta/localizer.properties b/openjpa-kernel/src/main/resources/org/apache/openjpa/meta/localizer.properties index 5777c013d..d5100131a 100644 --- a/openjpa-kernel/src/main/resources/org/apache/openjpa/meta/localizer.properties +++ b/openjpa-kernel/src/main/resources/org/apache/openjpa/meta/localizer.properties @@ -267,6 +267,10 @@ lifecycle-resolved: Could add the following callback adapters to "{0}", as \ the lifecycle metadata is already resolved: {1} access-field-and-prop: Type "{0}" attempts to use both field and property \ access. Only one access method is permitted. +access-field-and-prop-hints: Type "{0}" attempts to use both field and \ + property access. Only one access method is permitted. Field access is used \ + on the following fields: {1}. Property access is used on the following \ + methods: {2}. unsupported-id-type: Type "{0}" declares field "{1}" as a primary key, but \ keys of type "{2}" are not supported. empty-fg-name: Attempt to add an unnamed fetch group to "{0}". diff --git a/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/jdbc/annotations/TestMixedAccess.java b/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/jdbc/annotations/TestMixedAccess.java new file mode 100644 index 000000000..af8cf486d --- /dev/null +++ b/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/jdbc/annotations/TestMixedAccess.java @@ -0,0 +1,52 @@ +/* + * 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.jdbc.annotations; + +import javax.persistence.PersistenceException; +import javax.persistence.EntityManagerFactory; + +import org.apache.openjpa.persistence.test.PersistenceTestCase; + +public class TestMixedAccess extends PersistenceTestCase { + + public void testMixedAccessEntityError() { + try { + EntityManagerFactory emf = createEMF(UnenhancedMixedAccess.class); + emf.createEntityManager().close(); + } catch (RuntimeException e) { + String msg = e.getMessage(); + if (!(msg.contains("UnenhancedMixedAccess.id") && + msg.contains("UnenhancedMixedAccess.getStringField"))) + throw e; + } + } + + public void testInappropriateTransientError() { + try { + EntityManagerFactory emf = createEMF( + UnenhancedInappropriateTransient.class); + emf.createEntityManager().close(); + } catch (RuntimeException e) { + String msg = e.getMessage(); + if (!(msg.contains("UnenhancedInappropriateTransient.id") && + msg.contains("UnenhancedInappropriateTransient.prePersist"))) + throw e; + } + } +} diff --git a/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/jdbc/annotations/UnenhancedInappropriateTransient.java b/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/jdbc/annotations/UnenhancedInappropriateTransient.java new file mode 100644 index 000000000..14c34ef89 --- /dev/null +++ b/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/jdbc/annotations/UnenhancedInappropriateTransient.java @@ -0,0 +1,34 @@ +/* + * 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.jdbc.annotations; + +import javax.persistence.Entity; +import javax.persistence.Id; +import javax.persistence.Transient; +import javax.persistence.PrePersist; + +@Entity +public class UnenhancedInappropriateTransient { + + @Id private int id; + + @Transient @PrePersist public void prePersist() { + throw new UnsupportedOperationException(); + } +} \ No newline at end of file diff --git a/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/jdbc/annotations/UnenhancedMixedAccess.java b/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/jdbc/annotations/UnenhancedMixedAccess.java new file mode 100644 index 000000000..b1a9752ab --- /dev/null +++ b/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/jdbc/annotations/UnenhancedMixedAccess.java @@ -0,0 +1,37 @@ +/* + * 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.jdbc.annotations; + +import javax.persistence.Entity; +import javax.persistence.Id; +import javax.persistence.Transient; + +@Entity +public class UnenhancedMixedAccess { + + @Id private int id; + + @Transient public String getStringField() { + throw new UnsupportedOperationException(); + } + + public void setStringField(String str) { + throw new UnsupportedOperationException(); + } +} diff --git a/openjpa-persistence/src/main/java/org/apache/openjpa/persistence/PersistenceMetaDataDefaults.java b/openjpa-persistence/src/main/java/org/apache/openjpa/persistence/PersistenceMetaDataDefaults.java index 124055b96..dbbc4bb7f 100644 --- a/openjpa-persistence/src/main/java/org/apache/openjpa/persistence/PersistenceMetaDataDefaults.java +++ b/openjpa-persistence/src/main/java/org/apache/openjpa/persistence/PersistenceMetaDataDefaults.java @@ -30,6 +30,8 @@ import java.util.HashMap; import java.util.HashSet; import java.util.Map; import java.util.Set; +import java.util.List; +import java.util.ArrayList; import javax.persistence.Basic; import javax.persistence.Embeddable; import javax.persistence.Embedded; @@ -257,22 +259,36 @@ public class PersistenceMetaDataDefaults return ClassMetaData.ACCESS_UNKNOWN; int access = 0; - if (usesAccess((Field[]) AccessController.doPrivileged( - J2DoPriv5Helper.getDeclaredFieldsAction(cls)))) + if (annotated((Field[]) AccessController.doPrivileged( + J2DoPriv5Helper.getDeclaredFieldsAction(cls))).size() > 0) access |= ClassMetaData.ACCESS_FIELD; - if (usesAccess((Method[]) AccessController.doPrivileged( - J2DoPriv5Helper.getDeclaredMethodsAction(cls))) + if (annotated((Method[]) AccessController.doPrivileged( + J2DoPriv5Helper.getDeclaredMethodsAction(cls))).size() > 0 || cls.isInterface()) // OpenJPA managed ifaces must use prop access access |= ClassMetaData.ACCESS_PROPERTY; - return (access == 0) ? getAccessType(cls.getSuperclass()) : access; + return getAccessType(cls.getSuperclass()) | access; + } + + @Override + protected List getFieldAccessNames(ClassMetaData meta) { + return annotated((Field[]) AccessController.doPrivileged( + J2DoPriv5Helper.getDeclaredFieldsAction(meta.getDescribedType()))); + } + + @Override + protected List getPropertyAccessNames(ClassMetaData meta) { + return annotated((Method[]) AccessController.doPrivileged( + J2DoPriv5Helper.getDeclaredMethodsAction(meta.getDescribedType()))); } /** - * Return whether the given members have persistence annotations. + * Return the members of members that have persistence + * annotations. */ - private static boolean usesAccess(AnnotatedElement[] members) { + private static List annotated(AnnotatedElement[] members) { Annotation[] annos; String name; + List annotated = new ArrayList(members.length); for (int i = 0; i < members.length; i++) { annos = (Annotation[]) AccessController.doPrivileged(J2DoPriv5Helper .getAnnotationsAction(members[i])); @@ -281,10 +297,10 @@ public class PersistenceMetaDataDefaults if ((name.startsWith("javax.persistence.") || name.startsWith("org.apache.openjpa.persistence.")) && !_ignoredAnnos.contains(name)) - return true; + annotated.add(members[i]); } } - return false; + return annotated; } protected boolean isDefaultPersistent(ClassMetaData meta, Member member,