From 229f24547add9fdf5c7414cf98895bf85f83e9d6 Mon Sep 17 00:00:00 2001 From: "A. Abram White" Date: Mon, 19 Mar 2007 21:46:45 +0000 Subject: [PATCH] OPENJPA-174 : Remove legacy code throwing an exception when attempting to construct an oid instance for an abstract persistent type. Improve error message in ApplicationIds when attempting to create a new id instance when the id class is abstract. git-svn-id: https://svn.apache.org/repos/asf/incubator/openjpa/trunk@520117 13f79535-47bb-0310-9956-ffa450edef68 --- .../org/apache/openjpa/kernel/BrokerImpl.java | 40 ++++----- .../apache/openjpa/util/ApplicationIds.java | 6 ++ .../openjpa/kernel/localizer.properties | 2 - .../apache/openjpa/util/localizer.properties | 5 +- .../persistence/inheritance/AbstractBase.java | 38 +++++++++ .../inheritance/ConcreteSubclass.java | 34 ++++++++ .../inheritance/TestFindAbstractClass.java | 83 +++++++++++++++++++ 7 files changed, 180 insertions(+), 28 deletions(-) create mode 100644 openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/inheritance/AbstractBase.java create mode 100644 openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/inheritance/ConcreteSubclass.java create mode 100644 openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/inheritance/TestFindAbstractClass.java diff --git a/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/BrokerImpl.java b/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/BrokerImpl.java index f7f1bd7a0..b91b7a3b7 100644 --- a/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/BrokerImpl.java +++ b/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/BrokerImpl.java @@ -1080,37 +1080,27 @@ public class BrokerImpl try { ClassMetaData meta = _conf.getMetaDataRepositoryInstance(). getMetaData(cls, _loader, true); - - // delegate to store manager for datastore ids - if (meta.getIdentityType() == ClassMetaData.ID_DATASTORE) { + switch (meta.getIdentityType()) { + case ClassMetaData.ID_DATASTORE: + // delegate to store manager for datastore ids if (val instanceof String && ((String) val).startsWith(StateManagerId.STRING_PREFIX)) return new StateManagerId((String) val); return _store.newDataStoreId(val, meta); - } else if (meta.getIdentityType() == ClassMetaData.ID_UNKNOWN) + case ClassMetaData.ID_APPLICATION: + if (ImplHelper.isAssignable(meta.getObjectIdType(), + val.getClass())) { + if (!meta.isOpenJPAIdentity() + && meta.isObjectIdTypeShared()) + return new ObjectId(cls, val); + return val; + } + Object[] arr = (val instanceof Object[]) ? (Object[]) val + : new Object[]{ val }; + return ApplicationIds.fromPKValues(arr, meta); + default: throw new UserException(_loc.get("meta-unknownid", cls)); - - if (val instanceof String - && !_conf.getCompatibilityInstance().getStrictIdentityValues()) - { - // bug #958: section 9.6 of the JDO 1.0.1 specification states - // that a fatal internal exception should be thrown when - // invoking this method on an abstract class - if (Modifier.isAbstract(cls.getModifiers())) - throw new InternalException(_loc.get("objectid-abstract", - cls)); - return PCRegistry.newObjectId(cls, (String) val); } - if (ImplHelper.isAssignable(meta.getObjectIdType(), val.getClass())) - { - if (!meta.isOpenJPAIdentity() && meta.isObjectIdTypeShared()) - return new ObjectId(cls, val); - return val; - } - - Object[] arr = (val instanceof Object[]) ? (Object[]) val - : new Object[]{ val }; - return ApplicationIds.fromPKValues(arr, meta); } catch (OpenJPAException ke) { throw ke; } catch (ClassCastException cce) { diff --git a/openjpa-kernel/src/main/java/org/apache/openjpa/util/ApplicationIds.java b/openjpa-kernel/src/main/java/org/apache/openjpa/util/ApplicationIds.java index d2d4db43b..1c6dd431d 100644 --- a/openjpa-kernel/src/main/java/org/apache/openjpa/util/ApplicationIds.java +++ b/openjpa-kernel/src/main/java/org/apache/openjpa/util/ApplicationIds.java @@ -27,6 +27,7 @@ import org.apache.openjpa.enhance.Reflection; import org.apache.openjpa.kernel.ObjectIdStateManager; import org.apache.openjpa.kernel.OpenJPAStateManager; import org.apache.openjpa.kernel.StoreManager; +import org.apache.openjpa.lib.util.Localizer; import org.apache.openjpa.meta.ClassMetaData; import org.apache.openjpa.meta.FieldMetaData; import org.apache.openjpa.meta.JavaTypes; @@ -41,6 +42,9 @@ import serp.util.Numbers; */ public class ApplicationIds { + private static final Localizer _loc = Localizer.forPackage + (ApplicationIds.class); + /** * Return the primary key values for the given object id. The values * will be returned in the same order as the metadata primary key fields. @@ -165,6 +169,8 @@ public class ApplicationIds { // default to reflection Class oidType = meta.getObjectIdType(); + if (Modifier.isAbstract(oidType.getModifiers())) + throw new UserException(_loc.get("objectid-abstract", meta)); Object copy = null; try { copy = oidType.newInstance(); diff --git a/openjpa-kernel/src/main/resources/org/apache/openjpa/kernel/localizer.properties b/openjpa-kernel/src/main/resources/org/apache/openjpa/kernel/localizer.properties index 7063c1cf4..8e9058b64 100644 --- a/openjpa-kernel/src/main/resources/org/apache/openjpa/kernel/localizer.properties +++ b/openjpa-kernel/src/main/resources/org/apache/openjpa/kernel/localizer.properties @@ -180,8 +180,6 @@ dup-load: Cannot load object with id "{0}". Instance "{1}" with the same id \ already exists in the L1 cache. This can occur when you \ assign an existing id to a new instance, and before flushing attempt to \ load the existing instance for that id. -objectid-abstract: Cannot create new application identity instance for \ - abstract class "{0}". bad-id-value: The given value "{0}" cannot be converted into an identity \ for "{2}". The value is the wrong type ({1}). factory-init: Starting OpenJPA {0} diff --git a/openjpa-kernel/src/main/resources/org/apache/openjpa/util/localizer.properties b/openjpa-kernel/src/main/resources/org/apache/openjpa/util/localizer.properties index 84adb8e60..35cd9fdb4 100644 --- a/openjpa-kernel/src/main/resources/org/apache/openjpa/util/localizer.properties +++ b/openjpa-kernel/src/main/resources/org/apache/openjpa/util/localizer.properties @@ -16,7 +16,8 @@ failed: Failed: nested: Nested: null-oid: Null id value encountered while creating datastore identity for "{0}". unknown-oid: While creating datastore identity for "{0}", unknown id value \ - "{1}" of type "{2}" encountered. Cannot convert to org.apache.openjpa.util.Id. + "{1}" of type "{2}" encountered. Cannot convert to \ + org.apache.openjpa.util.Id. bad-ser-oid: Encountered object id "{0}" in serialized data, but the \ corresponding persistent object no longer exists. Substituting null into \ the deserialization stream. @@ -67,3 +68,5 @@ dup-oid: A duplicate object id exception has occurred. Each object you \ unique: A unique constraint violation has occurred. ref-integrity: A referential integrity constraint has occurred. no-store-exts: No store-specific facade found matching "{0}". Using default. +objectid-abstract: Cannot create new application identity instance for \ + abstract class "{0}". diff --git a/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/inheritance/AbstractBase.java b/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/inheritance/AbstractBase.java new file mode 100644 index 000000000..dc51f7ac1 --- /dev/null +++ b/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/inheritance/AbstractBase.java @@ -0,0 +1,38 @@ +/* + * Copyright 2006 The Apache Software Foundation. + * + * Licensed 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.inheritance; + +import javax.persistence.Entity; +import javax.persistence.Id; +import javax.persistence.Inheritance; +import javax.persistence.InheritanceType; + +@Entity +@Inheritance(strategy=InheritanceType.JOINED) +public abstract class AbstractBase { + + @Id + private String id; + + public String getId() { + return id; + } + + public void setId(String id) { + this.id = id; + } +} + diff --git a/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/inheritance/ConcreteSubclass.java b/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/inheritance/ConcreteSubclass.java new file mode 100644 index 000000000..b683bb0bd --- /dev/null +++ b/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/inheritance/ConcreteSubclass.java @@ -0,0 +1,34 @@ +/* + * Copyright 2006 The Apache Software Foundation. + * + * Licensed 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.inheritance; + +import javax.persistence.Entity; + +@Entity +public class ConcreteSubclass + extends AbstractBase { + + private int subclassData; + + public int getSubclassData() { + return subclassData; + } + + public void setSubclassData(int subclassData) { + this.subclassData = subclassData; + } +} + diff --git a/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/inheritance/TestFindAbstractClass.java b/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/inheritance/TestFindAbstractClass.java new file mode 100644 index 000000000..3128ede59 --- /dev/null +++ b/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/inheritance/TestFindAbstractClass.java @@ -0,0 +1,83 @@ +/* + * Copyright 2006 The Apache Software Foundation. + * + * Licensed 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.inheritance; + +import java.util.HashMap; +import java.util.Map; +import javax.persistence.EntityManager; +import javax.persistence.EntityManagerFactory; +import javax.persistence.Persistence; + +import junit.framework.TestCase; +import junit.textui.TestRunner; + +/** + * Test that you can find a concrete subclass record when passing in its + * abstract base class to EntityManager.find(). + * + * @author Abe White + */ +public class TestFindAbstractClass + extends TestCase { + + private EntityManagerFactory emf; + + public void setUp() { + String types = AbstractBase.class.getName() + ";" + + ConcreteSubclass.class.getName(); + Map props = new HashMap(System.getProperties()); + props.put("openjpa.MetaDataFactory", "jpa(Types=" + types + ")"); + emf = Persistence.createEntityManagerFactory("test", props); + + ConcreteSubclass e = new ConcreteSubclass(); + e.setId("id"); + e.setSubclassData(1); + + EntityManager em = emf.createEntityManager(); + em.getTransaction().begin(); + em.persist(e); + em.getTransaction().commit(); + em.close(); + } + + public void tearDown() { + if (emf == null) + return; + try { + EntityManager em = emf.createEntityManager(); + em.getTransaction().begin(); + em.createQuery("delete from ConcreteSubclass").executeUpdate(); + em.getTransaction().commit(); + em.close(); + emf.close(); + } catch (Exception e) { + } + } + + public void testFind() { + EntityManager em = emf.createEntityManager(); + AbstractBase e = em.find(AbstractBase.class, "id"); + assertNotNull(e); + assertTrue(e instanceof ConcreteSubclass); + assertEquals(1, ((ConcreteSubclass) e).getSubclassData()); + em.close(); + } + + public static void main(String[] args) { + TestRunner.run(TestFindAbstractClass.class); + } +} +