[OPENJPA-2974] Convert String cache mode properties to their enum again

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<String, Object> properties = new HashMap<String, Object>();
+        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<String, Object> properties = new HashMap<String, Object>();
+        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();
+    }
 }

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
+	 * <code>jakarta.persistence.cache.retrieveMode</code> 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;

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> T  convertToKernelValue(Class<T> 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<Enum>) 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.
      *

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}.

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"));
+    }
+}