From ef98e1848096448ba7e60f65839d69451dfcc06d Mon Sep 17 00:00:00 2001 From: Romain Manni-Bucau Date: Mon, 29 Jul 2019 17:40:09 +0200 Subject: [PATCH] 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) --- .../apache/openjpa/jdbc/meta/MappingTool.java | 2 +- .../openjpa/conf/OpenJPAConfiguration.java | 4 + .../conf/OpenJPAConfigurationImpl.java | 16 ++++ .../apache/openjpa/enhance/PCEnhancer.java | 4 + .../openjpa/meta/MetaDataRepository.java | 22 ++++- .../openjpa/lib/conf/ClassListValue.java | 80 ++++++++++++++++++ .../src/main/ant/enhancer.xml | 1 + .../JDBCPersistenceProductDerivation.java | 6 ++ .../meta/TestMetamodelWithEnum.java | 83 +++++++++++++++++++ .../persistence/meta/MetamodelImpl.java | 4 + 10 files changed, 220 insertions(+), 2 deletions(-) create mode 100644 openjpa-lib/src/main/java/org/apache/openjpa/lib/conf/ClassListValue.java create mode 100644 openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/meta/TestMetamodelWithEnum.java 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 511eabb79..1e368de96 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 bd844f620..703b487fe 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> getTypesWithoutEnhancement(); + + void setTypesWithoutEnhancement(Collection> 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 17d9b84c6..6727e14f2 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> getTypesWithoutEnhancement() { + return asList(typesWithoutEnhancement.get()); + } + + @Override + public void setTypesWithoutEnhancement(Collection> 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 b8e0af738..73672b731 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 633173e16..c1c0ca7b3 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> _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 000000000..76a5b0ba9 --- /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 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 dd41f85ae..922da25c0 100644 --- a/openjpa-persistence-jdbc/src/main/ant/enhancer.xml +++ b/openjpa-persistence-jdbc/src/main/ant/enhancer.xml @@ -74,6 +74,7 @@ + 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 5fa68987f..25704887e 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 000000000..3ccc97b07 --- /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 { + @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 6a1f76e9b..593eb71e3 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> 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) {