[openjpa] branch master updated: OPENJPA-2743 skip AttributeConverter when loading the Metamodel and preparing the broker to avoid to fail if it was registered in persistent classes (allowed in JPA to bypass scanning but enable converters)

classic Classic list List threaded Threaded
1 message Options
Reply | Threaded
Open this post in threaded view
|

[openjpa] branch master updated: OPENJPA-2743 skip AttributeConverter when loading the Metamodel and preparing the broker to avoid to fail if it was registered in persistent classes (allowed in JPA to bypass scanning but enable converters)

rmannibucau
This is an automated email from the ASF dual-hosted git repository.

rmannibucau pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/openjpa.git


The following commit(s) were added to refs/heads/master by this push:
     new ef98e18  OPENJPA-2743 skip AttributeConverter when loading the Metamodel and preparing the broker to avoid to fail if it was registered in persistent classes (allowed in JPA to bypass scanning but enable converters)
ef98e18 is described below

commit ef98e1848096448ba7e60f65839d69451dfcc06d
Author: Romain Manni-Bucau <[hidden email]>
AuthorDate: Mon Jul 29 17:40:09 2019 +0200

    OPENJPA-2743 skip AttributeConverter when loading the Metamodel and preparing the broker to avoid to fail if it was registered in persistent classes (allowed in JPA to bypass scanning but enable converters)
---
 .../org/apache/openjpa/jdbc/meta/MappingTool.java  |  2 +-
 .../apache/openjpa/conf/OpenJPAConfiguration.java  |  4 ++
 .../openjpa/conf/OpenJPAConfigurationImpl.java     | 16 +++++
 .../org/apache/openjpa/enhance/PCEnhancer.java     |  4 ++
 .../apache/openjpa/meta/MetaDataRepository.java    | 22 +++++-
 .../apache/openjpa/lib/conf/ClassListValue.java    | 80 +++++++++++++++++++++
 openjpa-persistence-jdbc/src/main/ant/enhancer.xml |  1 +
 .../jdbc/JDBCPersistenceProductDerivation.java     |  6 ++
 .../persistence/meta/TestMetamodelWithEnum.java    | 83 ++++++++++++++++++++++
 .../openjpa/persistence/meta/MetamodelImpl.java    |  4 ++
 10 files changed, 220 insertions(+), 2 deletions(-)

diff --git a/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/meta/MappingTool.java b/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/meta/MappingTool.java
index 511eabb..1e368de 100644
--- a/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/meta/MappingTool.java
+++ b/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/meta/MappingTool.java
@@ -748,7 +748,7 @@ public class MappingTool
         ClassMapping mapping = repos.getMapping(cls, null, false);
         if (mapping != null)
             return mapping;
-        if (!validate || cls.isInterface()
+        if (!validate || cls.isInterface() || repos.skipMetadata(cls)
             || repos.getPersistenceAware(cls) != null)
             return null;
         throw new MetaDataException(_loc.get("no-meta", cls, cls.getClassLoader()));
diff --git a/openjpa-kernel/src/main/java/org/apache/openjpa/conf/OpenJPAConfiguration.java b/openjpa-kernel/src/main/java/org/apache/openjpa/conf/OpenJPAConfiguration.java
index bd844f6..703b487 100644
--- a/openjpa-kernel/src/main/java/org/apache/openjpa/conf/OpenJPAConfiguration.java
+++ b/openjpa-kernel/src/main/java/org/apache/openjpa/conf/OpenJPAConfiguration.java
@@ -2009,5 +2009,9 @@ public interface OpenJPAConfiguration
     String getDropScriptTarget();
 
     String getLoadScriptSource();
+
+    Collection<Class<?>> getTypesWithoutEnhancement();
+
+    void setTypesWithoutEnhancement(Collection<Class<?>> value);
 }
 
diff --git a/openjpa-kernel/src/main/java/org/apache/openjpa/conf/OpenJPAConfigurationImpl.java b/openjpa-kernel/src/main/java/org/apache/openjpa/conf/OpenJPAConfigurationImpl.java
index 17d9b84..6727e14 100644
--- a/openjpa-kernel/src/main/java/org/apache/openjpa/conf/OpenJPAConfigurationImpl.java
+++ b/openjpa-kernel/src/main/java/org/apache/openjpa/conf/OpenJPAConfigurationImpl.java
@@ -18,6 +18,8 @@
  */
 package org.apache.openjpa.conf;
 
+import static java.util.Arrays.asList;
+
 import java.util.Collection;
 import java.util.HashSet;
 import java.util.Map;
@@ -54,6 +56,7 @@ import org.apache.openjpa.kernel.Seq;
 import org.apache.openjpa.kernel.exps.AggregateListener;
 import org.apache.openjpa.kernel.exps.FilterListener;
 import org.apache.openjpa.lib.conf.BooleanValue;
+import org.apache.openjpa.lib.conf.ClassListValue;
 import org.apache.openjpa.lib.conf.ConfigurationImpl;
 import org.apache.openjpa.lib.conf.Configurations;
 import org.apache.openjpa.lib.conf.IntValue;
@@ -178,6 +181,7 @@ public class OpenJPAConfigurationImpl
     public BooleanValue postLoadOnMerge;
     public BooleanValue optimizeIdCopy;
     public BooleanValue useTcclForSelectNew;
+    public ClassListValue typesWithoutEnhancement;
 
     // JPA Properties
     public IntValue databaseAction;
@@ -672,6 +676,8 @@ public class OpenJPAConfigurationImpl
         useTcclForSelectNew.setDefault("false");
         useTcclForSelectNew.set(false);
 
+        typesWithoutEnhancement = new ClassListValue();
+
         // initialize supported options that some runtimes may not support
         supportedOptions.add(OPTION_NONTRANS_READ);
         supportedOptions.add(OPTION_OPTIMISTIC);
@@ -2278,5 +2284,15 @@ public class OpenJPAConfigurationImpl
             setUseTCCLinSelectNew(useTcclForSelectNew.booleanValue());
         }
     }
+
+    @Override
+    public Collection<Class<?>> getTypesWithoutEnhancement() {
+        return asList(typesWithoutEnhancement.get());
+    }
+
+    @Override
+    public void setTypesWithoutEnhancement(Collection<Class<?>> value) {
+        typesWithoutEnhancement.set(value.toArray(new Class[value.size()]));
+    }
 }
 
diff --git a/openjpa-kernel/src/main/java/org/apache/openjpa/enhance/PCEnhancer.java b/openjpa-kernel/src/main/java/org/apache/openjpa/enhance/PCEnhancer.java
index b8e0af7..73672b7 100644
--- a/openjpa-kernel/src/main/java/org/apache/openjpa/enhance/PCEnhancer.java
+++ b/openjpa-kernel/src/main/java/org/apache/openjpa/enhance/PCEnhancer.java
@@ -530,6 +530,10 @@ public class PCEnhancer {
     public int run() {
         Class<?> type = _managedType.getType();
         try {
+            // if enum, skip, no need of any meta
+            if (_pc.isEnum())
+                return ENHANCE_NONE;
+
             // if managed interface, skip
             if (_pc.isInterface())
                 return ENHANCE_INTERFACE;
diff --git a/openjpa-kernel/src/main/java/org/apache/openjpa/meta/MetaDataRepository.java b/openjpa-kernel/src/main/java/org/apache/openjpa/meta/MetaDataRepository.java
index 633173e..c1c0ca7 100644
--- a/openjpa-kernel/src/main/java/org/apache/openjpa/meta/MetaDataRepository.java
+++ b/openjpa-kernel/src/main/java/org/apache/openjpa/meta/MetaDataRepository.java
@@ -167,6 +167,9 @@ public class MetaDataRepository implements PCRegistry.RegisterClassListener, Con
     // A boolean used to decide whether to filter Class<?> objects submitted by the PCRegistry listener system
     private boolean _filterRegisteredClasses = false;
 
+    // we should skip these types for the enhancement
+    private Collection<Class<?>> _typesWithoutEnhancement;
+
     /**
      * Default constructor. Configure via {@link Configurable}.
      */
@@ -992,7 +995,7 @@ public class MetaDataRepository implements PCRegistry.RegisterClassListener, Con
 
     /**
      * Create an {@link Order} for the given field and declaration. This method delegates to
-     * {@link #newRelatedFieldOrder} and {@link #newValueFieldOrder} by default.
+     * {@link #newRelatedFieldOrder} and {@link #newValueOrder(FieldMetaData, boolean)} by default.
      */
     protected Order newOrder(FieldMetaData owner, String name, boolean asc) {
         // paths can start with (or equal) '#element'
@@ -1298,6 +1301,16 @@ public class MetaDataRepository implements PCRegistry.RegisterClassListener, Con
         return _pawares.get(cls);
     }
 
+    public boolean skipMetadata(final Class<?> cls) {
+        if (cls == null || cls.isEnum()) {
+            return true;
+        }
+        if (_typesWithoutEnhancement == null) {
+            return false;
+        }
+        return _typesWithoutEnhancement.stream().anyMatch(it -> it.isAssignableFrom(cls));
+    }
+
     /**
      * Gets all the metadatas for persistence-aware classes
      *
@@ -1539,6 +1552,9 @@ public class MetaDataRepository implements PCRegistry.RegisterClassListener, Con
                     setMetaModel(cls);
                     continue;
                 }
+                if (skipMetadata(cls)) {
+                    continue;
+                }
                 if (cls != null) {
                     classes.add(cls);
 
@@ -1922,6 +1938,10 @@ public class MetaDataRepository implements PCRegistry.RegisterClassListener, Con
         _conf = (OpenJPAConfiguration) conf;
         _log = _conf.getLog(OpenJPAConfiguration.LOG_METADATA);
         _filterRegisteredClasses = _conf.getCompatibilityInstance().getFilterPCRegistryClasses();
+        _typesWithoutEnhancement = _conf.getTypesWithoutEnhancement();
+        if (_typesWithoutEnhancement == null || _typesWithoutEnhancement.isEmpty()) {
+            _typesWithoutEnhancement = null;
+        }
     }
 
     @Override
diff --git a/openjpa-lib/src/main/java/org/apache/openjpa/lib/conf/ClassListValue.java b/openjpa-lib/src/main/java/org/apache/openjpa/lib/conf/ClassListValue.java
new file mode 100644
index 0000000..76a5b0b
--- /dev/null
+++ b/openjpa-lib/src/main/java/org/apache/openjpa/lib/conf/ClassListValue.java
@@ -0,0 +1,80 @@
+/*
+ * 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.lib.conf;
+
+import static java.util.Optional.ofNullable;
+
+import java.security.AccessController;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
+
+import org.apache.openjpa.lib.util.J2DoPrivHelper;
+import org.apache.openjpa.lib.util.StringUtil;
+
+public class ClassListValue extends Value {
+    private Class<?>[] _values = new Class<?>[0];
+
+    public void set(final Class<?>[] values) {
+        assertChangeable();
+        if (values != null) {
+            _values = values;
+        }
+        valueChanged();
+    }
+
+    @Override
+    public Class<?>[] get() {
+        return _values;
+    }
+
+    @Override
+    public Class<Class[]> getValueType() {
+        return Class[].class;
+    }
+
+    @Override
+    protected String getInternalString() {
+        return Stream.of(_values).map(Class::getName).collect(Collectors.joining(","));
+    }
+
+    @Override
+    protected void setInternalString(String val) {
+        String[] vals = StringUtil.split(val, ",", 0);
+        if (vals != null) {
+            for (int i = 0; i < vals.length; i++)
+                vals[i] = vals[i].trim();
+        }
+
+        final ClassLoader loader = AccessController.doPrivileged(J2DoPrivHelper.getContextClassLoaderAction());
+        set(ofNullable(StringUtil.split(val, ",", 0))
+                .map(it -> Stream.of(it).map(v -> {
+                    try {
+                        return loader.loadClass(v.trim());
+                    } catch (final ClassNotFoundException e) {
+                        throw new IllegalStateException(e);
+                    }
+                }).toArray(Class<?>[]::new))
+                .orElse(null));
+    }
+
+    @Override
+    protected void setInternalObject(Object obj) {
+        set((Class<?>[]) obj);
+    }
+}
diff --git a/openjpa-persistence-jdbc/src/main/ant/enhancer.xml b/openjpa-persistence-jdbc/src/main/ant/enhancer.xml
index dd41f85..922da25 100644
--- a/openjpa-persistence-jdbc/src/main/ant/enhancer.xml
+++ b/openjpa-persistence-jdbc/src/main/ant/enhancer.xml
@@ -74,6 +74,7 @@
         <fileset id="enhance.path.ref"
                  dir="${project.build.testOutputDirectory}">
             <include name="**/*.class" />
+            <include name="org/apache/openjpa/persistence/meta/TestMetamodelWithEnum$Main.class" />
             <exclude name="**/inheritance/serializable/*.class" />
             <exclude name="**/detach/serializable/*.class" />
             <exclude name="**/proxy/entities/*.class" />
diff --git a/openjpa-persistence-jdbc/src/main/java/org/apache/openjpa/persistence/jdbc/JDBCPersistenceProductDerivation.java b/openjpa-persistence-jdbc/src/main/java/org/apache/openjpa/persistence/jdbc/JDBCPersistenceProductDerivation.java
index 5fa6898..2570488 100644
--- a/openjpa-persistence-jdbc/src/main/java/org/apache/openjpa/persistence/jdbc/JDBCPersistenceProductDerivation.java
+++ b/openjpa-persistence-jdbc/src/main/java/org/apache/openjpa/persistence/jdbc/JDBCPersistenceProductDerivation.java
@@ -18,12 +18,16 @@
  */
 package org.apache.openjpa.persistence.jdbc;
 
+import static java.util.Collections.singleton;
+
 import java.security.AccessController;
 import java.util.Collections;
 import java.util.HashSet;
 import java.util.Map;
 import java.util.Set;
 
+import javax.persistence.AttributeConverter;
+
 import org.apache.openjpa.conf.OpenJPAConfiguration;
 import org.apache.openjpa.conf.OpenJPAProductDerivation;
 import org.apache.openjpa.conf.Specification;
@@ -96,6 +100,8 @@ public class JDBCPersistenceProductDerivation
 
         conf.lockManagerPlugin.setAlias("mixed", "org.apache.openjpa.jdbc.kernel.MixedLockManager");
 
+        conf.typesWithoutEnhancement.set(new Class<?>[]{ AttributeConverter.class });
+
         return true;
     }
 
diff --git a/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/meta/TestMetamodelWithEnum.java b/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/meta/TestMetamodelWithEnum.java
new file mode 100644
index 0000000..3ccc97b
--- /dev/null
+++ b/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/meta/TestMetamodelWithEnum.java
@@ -0,0 +1,83 @@
+/*
+ * 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 javax.persistence.AttributeConverter;
+import javax.persistence.Converter;
+import javax.persistence.Entity;
+import javax.persistence.Id;
+
+import org.apache.openjpa.persistence.test.SingleEMFTestCase;
+
+public class TestMetamodelWithEnum extends SingleEMFTestCase {
+
+    @Override
+    public void setUp() {
+        super.setUp(
+                "openjpa.RuntimeUnenhancedClasses", "unsupported",
+                "openjpa.DynamicEnhancementAgent", "false",
+                Main.class, AttributeConverterImpl.class);
+    }
+
+    public void testEnsureEnumDontFail() { // OPENJPA-2743
+        assertNotNull(emf.getMetamodel());
+    }
+
+    @Converter
+    public static class AttributeConverterImpl implements AttributeConverter<MyEnum, String> {
+        @Override
+        public String convertToDatabaseColumn(final MyEnum myEnum) {
+            return myEnum.name();
+        }
+
+        @Override
+        public MyEnum convertToEntityAttribute(final String s) {
+            return MyEnum.valueOf(s);
+        }
+    }
+
+    @Entity
+    public static class Main {
+        @Id
+        private String id;
+
+        private MyEnum enumColumn;
+
+        public String getId() {
+            return id;
+        }
+
+        public void setId(final String id) {
+            this.id = id;
+        }
+
+        public MyEnum getEnumColumn() {
+            return enumColumn;
+        }
+
+        public void setEnumColumn(final MyEnum enumColumn) {
+            this.enumColumn = enumColumn;
+        }
+    }
+
+    public enum MyEnum {
+        A
+    }
+}
diff --git a/openjpa-persistence/src/main/java/org/apache/openjpa/persistence/meta/MetamodelImpl.java b/openjpa-persistence/src/main/java/org/apache/openjpa/persistence/meta/MetamodelImpl.java
index 6a1f76e..593eb71 100644
--- a/openjpa-persistence/src/main/java/org/apache/openjpa/persistence/meta/MetamodelImpl.java
+++ b/openjpa-persistence/src/main/java/org/apache/openjpa/persistence/meta/MetamodelImpl.java
@@ -83,6 +83,10 @@ public class MetamodelImpl implements Metamodel, Resolver {
         this.repos = repos;
         Collection<Class<?>> classes = repos.loadPersistentTypes(true, null);
         for (Class<?> cls : classes) {
+            if (repos.skipMetadata(cls)) { // AttributeConverters, enums etc....
+                continue;
+            }
+
          ClassMetaData meta = repos.getMetaData(cls, null, true);
             PersistenceType type = getPersistenceType(meta);
             switch (type) {