From 872abe814124315e5dff20477ea3c2638a8b8eaf Mon Sep 17 00:00:00 2001 From: Richard Zowalla Date: Wed, 2 Sep 2026 19:54:38 +0200 Subject: [PATCH] [OPENJPA-2974] Convert String cache mode properties to their enum again convertToKernelValue() returned a String value for cache.retrieveMode and cache.storeMode untouched. Since PR #144 the bean property for those keys resolves to the new JPA 3.2 setters on EntityManagerImpl, which take the jakarta enums, so em.setProperty("jakarta.persistence.cache.retrieveMode", "USE") reached Method.invoke with a String and died with an argument type mismatch. The same applies to the map passed to createEntityManager(). The conversion is now driven by the target type rather than by the key: a value already of the target type passes through, a String is parsed into the target enum, and an enum of a different type is mapped by constant name, which is what lets a jakarta CacheStoreMode reach a kernel DataCacheStoreMode. An unusable String now fails with a message naming the property and its legal values instead of an argument type mismatch. The early return this replaces cannot simply be dropped: it was added with those setters and is what keeps the enum-valued form working. Being type-driven, it also fixes jakarta.persistence.lock.scope="EXTENDED", which failed in StringUtil.parse with "Unsupported type". A null mode now resets the fetch plan to its default rather than meaning BYPASS, so clearing one of these properties does not leave the plan bypassing the cache; both setters document that. --- .../property/TestEMProperties.java | 97 +++++++++++++++ .../persistence/EntityManagerImpl.java | 13 +- .../openjpa/persistence/JPAProperties.java | 52 +++++--- .../openjpa/persistence/localizer.properties | 2 + .../persistence/TestJPAProperties.java | 116 ++++++++++++++++++ 5 files changed, 261 insertions(+), 19 deletions(-) create mode 100644 openjpa-persistence/src/test/java/org/apache/openjpa/persistence/TestJPAProperties.java diff --git a/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/property/TestEMProperties.java b/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/property/TestEMProperties.java index da7f14e7d1..51ff37bcf6 100644 --- a/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/property/TestEMProperties.java +++ b/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/property/TestEMProperties.java @@ -21,8 +21,14 @@ import java.util.HashMap; import java.util.Map; +import jakarta.persistence.CacheRetrieveMode; +import jakarta.persistence.CacheStoreMode; import jakarta.persistence.EntityManager; +import org.apache.openjpa.kernel.DataCacheRetrieveMode; +import org.apache.openjpa.kernel.DataCacheStoreMode; +import org.apache.openjpa.persistence.FetchPlan; +import org.apache.openjpa.persistence.JPAProperties; import org.apache.openjpa.persistence.OpenJPAPersistence; import org.apache.openjpa.persistence.OpenJPAQuery; import org.apache.openjpa.persistence.test.SingleEMFTestCase; @@ -78,4 +84,95 @@ public void testQueryTimeoutPropertySetOnEntityManager() { em.clear(); em.close(); } + + public void testCacheModeStringSetOnEntityManager() { + EntityManager em = emf.createEntityManager(); + + em.setProperty(JPAProperties.CACHE_RETRIEVE_MODE, "BYPASS"); + em.setProperty(JPAProperties.CACHE_STORE_MODE, "REFRESH"); + + assertEquals(CacheRetrieveMode.BYPASS, em.getCacheRetrieveMode()); + assertEquals(CacheStoreMode.REFRESH, em.getCacheStoreMode()); + FetchPlan fetchPlan = OpenJPAPersistence.cast(em).getFetchPlan(); + assertEquals(DataCacheRetrieveMode.BYPASS, fetchPlan.getCacheRetrieveMode()); + assertEquals(DataCacheStoreMode.REFRESH, fetchPlan.getCacheStoreMode()); + + em.close(); + } + + public void testCacheModeStringOnEntityManagerCreation() { + Map properties = new HashMap(); + properties.put(JPAProperties.CACHE_RETRIEVE_MODE, "BYPASS"); + properties.put(JPAProperties.CACHE_STORE_MODE, "REFRESH"); + EntityManager em = emf.createEntityManager(properties); + + assertEquals(CacheRetrieveMode.BYPASS, em.getCacheRetrieveMode()); + assertEquals(CacheStoreMode.REFRESH, em.getCacheStoreMode()); + FetchPlan fetchPlan = OpenJPAPersistence.cast(em).getFetchPlan(); + assertEquals(DataCacheRetrieveMode.BYPASS, fetchPlan.getCacheRetrieveMode()); + assertEquals(DataCacheStoreMode.REFRESH, fetchPlan.getCacheStoreMode()); + + em.close(); + } + + public void testCacheModeStringIsCaseInsensitive() { + EntityManager em = emf.createEntityManager(); + + em.setProperty(JPAProperties.CACHE_RETRIEVE_MODE, "use"); + em.setProperty(JPAProperties.CACHE_STORE_MODE, " refresh "); + + assertEquals(CacheRetrieveMode.USE, em.getCacheRetrieveMode()); + assertEquals(CacheStoreMode.REFRESH, em.getCacheStoreMode()); + + em.close(); + } + + public void testCacheModeEnumStillWorks() { + EntityManager em = emf.createEntityManager(); + + em.setProperty(JPAProperties.CACHE_RETRIEVE_MODE, CacheRetrieveMode.BYPASS); + em.setProperty(JPAProperties.CACHE_STORE_MODE, CacheStoreMode.REFRESH); + + assertEquals(CacheRetrieveMode.BYPASS, em.getCacheRetrieveMode()); + assertEquals(CacheStoreMode.REFRESH, em.getCacheStoreMode()); + em.close(); + + Map properties = new HashMap(); + properties.put(JPAProperties.CACHE_RETRIEVE_MODE, CacheRetrieveMode.BYPASS); + properties.put(JPAProperties.CACHE_STORE_MODE, CacheStoreMode.REFRESH); + em = emf.createEntityManager(properties); + + assertEquals(CacheRetrieveMode.BYPASS, em.getCacheRetrieveMode()); + assertEquals(CacheStoreMode.REFRESH, em.getCacheStoreMode()); + + em.close(); + } + + public void testInvalidCacheModeStringFails() { + EntityManager em = emf.createEntityManager(); + try { + em.setProperty(JPAProperties.CACHE_RETRIEVE_MODE, "NOPE"); + fail("Expected an IllegalArgumentException for an invalid cache retrieve mode"); + } catch (IllegalArgumentException iae) { + String message = iae.getMessage(); + assertTrue(message, message.contains("cache.retrieveMode")); + assertTrue(message, message.contains("USE")); + assertTrue(message, message.contains("BYPASS")); + } finally { + em.close(); + } + } + + public void testNullCacheModeResetsToDefault() { + EntityManager em = emf.createEntityManager(); + + em.setProperty(JPAProperties.CACHE_STORE_MODE, null); + em.setProperty(JPAProperties.CACHE_RETRIEVE_MODE, "null"); + + FetchPlan fetchPlan = OpenJPAPersistence.cast(em).getFetchPlan(); + assertEquals(DataCacheRetrieveMode.USE, fetchPlan.getCacheRetrieveMode()); + assertEquals(DataCacheStoreMode.USE, fetchPlan.getCacheStoreMode()); + + em.close(); + } } diff --git a/openjpa-persistence/src/main/java/org/apache/openjpa/persistence/EntityManagerImpl.java b/openjpa-persistence/src/main/java/org/apache/openjpa/persistence/EntityManagerImpl.java index 6ce4b39210..61d33c5364 100644 --- a/openjpa-persistence/src/main/java/org/apache/openjpa/persistence/EntityManagerImpl.java +++ b/openjpa-persistence/src/main/java/org/apache/openjpa/persistence/EntityManagerImpl.java @@ -2877,15 +2877,22 @@ public CacheRetrieveMode getCacheRetrieveMode() { return getFetchPlan().getCacheRetrieveMode() == DataCacheRetrieveMode.USE ? CacheRetrieveMode.USE : CacheRetrieveMode.BYPASS; } + /** + * Sets the cache retrieve mode. A null mode resets the fetch plan to its + * default, {@link DataCacheRetrieveMode#USE}, so that clearing the + * jakarta.persistence.cache.retrieveMode property does not + * leave the plan bypassing the cache. + */ @Override public void setCacheRetrieveMode(CacheRetrieveMode cacheRetrieveMode) { - getFetchPlan().setCacheRetrieveMode(cacheRetrieveMode == CacheRetrieveMode.USE - ? DataCacheRetrieveMode.USE : DataCacheRetrieveMode.BYPASS); + getFetchPlan().setCacheRetrieveMode(cacheRetrieveMode == CacheRetrieveMode.BYPASS + ? DataCacheRetrieveMode.BYPASS : DataCacheRetrieveMode.USE); } @Override public void setCacheStoreMode(CacheStoreMode cacheStoreMode) { - DataCacheStoreMode storeMode = switch (cacheStoreMode) { + // a null mode resets the fetch plan to its default, as above + DataCacheStoreMode storeMode = cacheStoreMode == null ? DataCacheStoreMode.USE : switch (cacheStoreMode) { case USE: yield DataCacheStoreMode.USE; case REFRESH: yield DataCacheStoreMode.REFRESH; default: yield DataCacheStoreMode.BYPASS; diff --git a/openjpa-persistence/src/main/java/org/apache/openjpa/persistence/JPAProperties.java b/openjpa-persistence/src/main/java/org/apache/openjpa/persistence/JPAProperties.java index fa57f10496..2f6ee85767 100644 --- a/openjpa-persistence/src/main/java/org/apache/openjpa/persistence/JPAProperties.java +++ b/openjpa-persistence/src/main/java/org/apache/openjpa/persistence/JPAProperties.java @@ -18,6 +18,7 @@ */ package org.apache.openjpa.persistence; +import java.util.Arrays; import java.util.HashMap; import java.util.Locale; import java.util.Map; @@ -27,6 +28,7 @@ import org.apache.openjpa.kernel.DataCacheRetrieveMode; import org.apache.openjpa.kernel.DataCacheStoreMode; +import org.apache.openjpa.lib.util.Localizer; import org.apache.openjpa.lib.util.StringUtil; /** @@ -41,6 +43,8 @@ * */ public class JPAProperties { + private static final Localizer _loc = Localizer.forPackage(JPAProperties.class); + private static final String REGEX_DOT = "\\."; public static final String PREFIX = "jakarta.persistence."; @@ -124,30 +128,46 @@ public static String getBeanProperty(String key) { * * @return the same value if the given key is not a valid JPA property key or the value is null. */ + @SuppressWarnings({"unchecked", "rawtypes"}) public static T convertToKernelValue(Class resultType, String key, Object value) { - if (value == null) + if (value == null) { return null; - if (JPAProperties.isValidKey(key)) { - // works because enum values are identical String - if (value instanceof CacheRetrieveMode || (value instanceof String && CACHE_RETRIEVE_MODE.equals(key))) { - return (T) value; - } else if (value instanceof CacheStoreMode || (value instanceof String && CACHE_STORE_MODE.equals(key))) { - return (T) value; + } + if (!isValidKey(key) || resultType == null || resultType.isInstance(value)) { + return (T) value; + } + if (value instanceof String) { + String str = (String) value; + if ("null".equals(str)) { + return null; } - - // If the value doesn't match the result type, attempt to convert - if(resultType != null && !resultType.isAssignableFrom(value.getClass())) { - if (value instanceof String) { - if ("null".equals(value)) { - return null; - } - return StringUtil.parse((String) value, resultType); - } + if (resultType.isEnum()) { + return (T) toEnumValue(resultType, key, str); } + return StringUtil.parse(str, resultType); + } + // e.g. jakarta CacheStoreMode -> kernel DataCacheStoreMode; the constant names are identical + if (resultType.isEnum() && value instanceof Enum) { + return (T) toEnumValue(resultType, key, ((Enum) value).name()); } return (T) value; } + /** + * Converts the given constant name to a constant of the given enumerated type. + * + * @throws IllegalArgumentException if the given name does not denote a constant of the given type. + */ + @SuppressWarnings({"unchecked", "rawtypes"}) + private static Object toEnumValue(Class type, String key, String name) { + try { + return Enum.valueOf((Class) type, name.trim().toUpperCase(Locale.ENGLISH)); + } catch (IllegalArgumentException iae) { + throw new IllegalArgumentException(_loc.get("bad-jpa-prop-value", name, key, + Arrays.toString(type.getEnumConstants())).getMessage(), iae); + } + } + /** * Convert the given kernel value to a value visible to the user. * diff --git a/openjpa-persistence/src/main/resources/org/apache/openjpa/persistence/localizer.properties b/openjpa-persistence/src/main/resources/org/apache/openjpa/persistence/localizer.properties index 72ac6e12ea..354a19c781 100644 --- a/openjpa-persistence/src/main/resources/org/apache/openjpa/persistence/localizer.properties +++ b/openjpa-persistence/src/main/resources/org/apache/openjpa/persistence/localizer.properties @@ -261,3 +261,5 @@ EntityManagers this property should not be used because these EntityManagers may time. shared-cache-mode-take-precedence= The DataCache is set to {0} while the shared-cache-mode Element or \ jakarta.persistence.sharedCache.mode property is set to NONE. The shared-cache-mode takes precedence and caching is disabled. +bad-jpa-prop-value: The value "{0}" is not valid for the property "{1}". \ + Permitted values are {2}. diff --git a/openjpa-persistence/src/test/java/org/apache/openjpa/persistence/TestJPAProperties.java b/openjpa-persistence/src/test/java/org/apache/openjpa/persistence/TestJPAProperties.java new file mode 100644 index 0000000000..f031c8e755 --- /dev/null +++ b/openjpa-persistence/src/test/java/org/apache/openjpa/persistence/TestJPAProperties.java @@ -0,0 +1,116 @@ +/* + * 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; + +import jakarta.persistence.CacheRetrieveMode; +import jakarta.persistence.CacheStoreMode; +import jakarta.persistence.PessimisticLockScope; + +import org.apache.openjpa.kernel.DataCacheRetrieveMode; +import org.apache.openjpa.kernel.DataCacheStoreMode; + +import org.junit.Test; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertSame; +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; + +/** + * Tests the conversion of user supplied property values to values consumable by the kernel. + */ +public class TestJPAProperties { + + @Test + public void testStringConvertedToSpecificationEnum() { + assertEquals(CacheRetrieveMode.USE, JPAProperties.convertToKernelValue(CacheRetrieveMode.class, + JPAProperties.CACHE_RETRIEVE_MODE, "USE")); + assertEquals(CacheStoreMode.BYPASS, JPAProperties.convertToKernelValue(CacheStoreMode.class, + JPAProperties.CACHE_STORE_MODE, "bypass")); + assertEquals(CacheStoreMode.REFRESH, JPAProperties.convertToKernelValue(CacheStoreMode.class, + JPAProperties.CACHE_STORE_MODE, " REFRESH ")); + } + + @Test + public void testStringConvertedToKernelEnum() { + assertEquals(DataCacheStoreMode.REFRESH, JPAProperties.convertToKernelValue(DataCacheStoreMode.class, + JPAProperties.CACHE_STORE_MODE, "REFRESH")); + assertEquals(DataCacheRetrieveMode.BYPASS, JPAProperties.convertToKernelValue(DataCacheRetrieveMode.class, + JPAProperties.CACHE_RETRIEVE_MODE, "BYPASS")); + } + + @Test + public void testEnumConvertedAcrossSpecificationAndKernel() { + assertEquals(DataCacheStoreMode.REFRESH, JPAProperties.convertToKernelValue(DataCacheStoreMode.class, + JPAProperties.CACHE_STORE_MODE, CacheStoreMode.REFRESH)); + assertEquals(CacheStoreMode.REFRESH, JPAProperties.convertToKernelValue(CacheStoreMode.class, + JPAProperties.CACHE_STORE_MODE, DataCacheStoreMode.REFRESH)); + assertEquals(DataCacheRetrieveMode.BYPASS, JPAProperties.convertToKernelValue(DataCacheRetrieveMode.class, + JPAProperties.CACHE_RETRIEVE_MODE, CacheRetrieveMode.BYPASS)); + assertEquals(CacheRetrieveMode.BYPASS, JPAProperties.convertToKernelValue(CacheRetrieveMode.class, + JPAProperties.CACHE_RETRIEVE_MODE, DataCacheRetrieveMode.BYPASS)); + } + + @Test + public void testMatchingEnumIsPassedThrough() { + assertSame(CacheRetrieveMode.BYPASS, JPAProperties.convertToKernelValue(CacheRetrieveMode.class, + JPAProperties.CACHE_RETRIEVE_MODE, CacheRetrieveMode.BYPASS)); + assertSame(DataCacheStoreMode.USE, JPAProperties.convertToKernelValue(DataCacheStoreMode.class, + JPAProperties.CACHE_STORE_MODE, DataCacheStoreMode.USE)); + } + + @Test + public void testInvalidEnumStringFails() { + try { + JPAProperties.convertToKernelValue(CacheRetrieveMode.class, JPAProperties.CACHE_RETRIEVE_MODE, "NOPE"); + fail("Expected an IllegalArgumentException for an invalid cache retrieve mode"); + } catch (IllegalArgumentException iae) { + String message = iae.getMessage(); + assertTrue(message, message.contains(JPAProperties.CACHE_RETRIEVE_MODE)); + assertTrue(message, message.contains("NOPE")); + assertTrue(message, message.contains("USE")); + assertTrue(message, message.contains("BYPASS")); + } + } + + @Test + public void testNullValues() { + assertNull(JPAProperties.convertToKernelValue(CacheStoreMode.class, JPAProperties.CACHE_STORE_MODE, null)); + assertNull(JPAProperties.convertToKernelValue(CacheStoreMode.class, JPAProperties.CACHE_STORE_MODE, "null")); + } + + @Test + public void testNonEnumValuesAreUnchanged() { + assertEquals(Integer.valueOf(12345), JPAProperties.convertToKernelValue(Integer.class, + JPAProperties.QUERY_TIMEOUT, "12345")); + assertEquals(Integer.valueOf(500), JPAProperties.convertToKernelValue(int.class, + JPAProperties.LOCK_TIMEOUT, "500")); + assertEquals(Integer.valueOf(200), JPAProperties.convertToKernelValue(Integer.class, + JPAProperties.QUERY_TIMEOUT, Integer.valueOf(200))); + assertEquals("USE", JPAProperties.convertToKernelValue(CacheStoreMode.class, + "openjpa.some.other.property", "USE")); + } + + @Test + public void testLockScopeString() { + assertEquals(PessimisticLockScope.EXTENDED, JPAProperties.convertToKernelValue(PessimisticLockScope.class, + JPAProperties.LOCK_SCOPE, "EXTENDED")); + } +}