Ignite-4949 Incorrect behavior CacheJdbcPojoStore if binary marshaller enable

modules/core/src/main/java/org/apache/ignite/cache/store/jdbc/CacheAbstractJdbcStore.java

@@ -48,7 +48,6 @@
 import org.apache.ignite.IgniteCheckedException;
 import org.apache.ignite.IgniteException;
 import org.apache.ignite.IgniteLogger;
-import org.apache.ignite.binary.BinaryObject;
 import org.apache.ignite.cache.store.CacheStore;
 import org.apache.ignite.cache.store.CacheStoreSession;
 import org.apache.ignite.cache.store.jdbc.dialect.BasicJdbcDialect;
@@ -58,6 +57,7 @@
 import org.apache.ignite.cache.store.jdbc.dialect.MySQLDialect;
 import org.apache.ignite.cache.store.jdbc.dialect.OracleDialect;
 import org.apache.ignite.cache.store.jdbc.dialect.SQLServerDialect;
+import org.apache.ignite.internal.binary.BinaryEnumObjectImpl;
 import org.apache.ignite.internal.binary.BinaryMarshaller;
 import org.apache.ignite.internal.util.tostring.GridToStringExclude;
 import org.apache.ignite.internal.util.typedef.C1;
@@ -537,22 +537,35 @@ private void checkTypeConfiguration(@Nullable String cacheName, TypeKind kind, S
 
     /**
      * @param type Type name to check.
+     * @param binarySupported True if binary marshaller enable.
      * @return {@code True} if class not found.
      */
-    protected TypeKind kindForName(String type) {
+    protected TypeKind kindForName(String type, boolean binarySupported) {
         if (BUILT_IN_TYPES.contains(type))
             return TypeKind.BUILT_IN;
 
+        if (binarySupported)
+            return TypeKind.BINARY;
+
         try {
             Class.forName(type);
 
             return TypeKind.POJO;
         }
-        catch(ClassNotFoundException ignored) {
-            return TypeKind.BINARY;
+        catch (ClassNotFoundException e) {
+            throw new CacheException("Can not find class " + type +
+                ", check your classPath or try to use BinaryMarshaller", e);
         }
     }
 
+    /**
+     * @param type Type name to check.
+     * @return {@code True} if class not found.
+     */
+    protected TypeKind kindForName(String type) {
+        return kindForName(type, ignite.configuration().getMarshaller() instanceof BinaryMarshaller);
+    }
+
     /**
      * @param cacheName Cache name to check mappings for.
      * @return Type mappings for specified cache name.
@@ -587,11 +600,7 @@ private Map<Object, EntryMapping> getOrCreateCacheMappings(@Nullable String cach
                     String keyType = type.getKeyType();
                     String valType = type.getValueType();
 
-                    TypeKind keyKind = kindForName(keyType);
-
-                    if (!binarySupported && keyKind == TypeKind.BINARY)
-                        throw new CacheException("Key type has no class [cache=" + U.maskName(cacheName) +
-                            ", type=" + keyType + "]");
+                    TypeKind keyKind = kindForName(keyType, binarySupported);
 
                     checkTypeConfiguration(cacheName, keyKind, keyType, type.getKeyFields());
 
@@ -601,21 +610,11 @@ private Map<Object, EntryMapping> getOrCreateCacheMappings(@Nullable String cach
                         throw new CacheException("Key type must be unique in type metadata [cache=" +
                             U.maskName(cacheName) + ", type=" + keyType + "]");
 
-                    TypeKind valKind = kindForName(valType);
+                    TypeKind valKind = kindForName(valType, binarySupported);
 
                     checkTypeConfiguration(cacheName, valKind, valType, type.getValueFields());
 
                     entryMappings.put(keyTypeId, new EntryMapping(cacheName, dialect, type, keyKind, valKind, sqlEscapeAll));
-
-                    // Add one more binding to binary typeId for POJOs,
-                    // because object could be passed to store in binary format.
-                    if (binarySupported && keyKind == TypeKind.POJO) {
-                        keyTypeId = typeIdForTypeName(TypeKind.BINARY, keyType);
-
-                        valKind = valKind == TypeKind.POJO ? TypeKind.BINARY : valKind;
-
-                        entryMappings.put(keyTypeId, new EntryMapping(cacheName, dialect, type, TypeKind.BINARY, valKind, sqlEscapeAll));
-                    }
                 }
 
                 Map<String, Map<Object, EntryMapping>> mappings = new HashMap<>(cacheMappings);
@@ -1349,10 +1348,17 @@ protected void fillParameter(PreparedStatement stmt, int idx, JdbcTypeField fiel
                             // No-op.
                     }
                 }
-                else if (field.getJavaFieldType().isEnum() && fieldVal instanceof Enum) {
-                    Enum val = (Enum)fieldVal;
+                else if (field.getJavaFieldType().isEnum()) {
+                    if (fieldVal instanceof Enum) {
+                        Enum val = (Enum)fieldVal;
 
-                    fieldVal = NUMERIC_TYPES.contains(field.getDatabaseFieldType()) ? val.ordinal() : val.name();
+                        fieldVal = NUMERIC_TYPES.contains(field.getDatabaseFieldType()) ? val.ordinal() : val.name();
+                    }
+                    else if (fieldVal instanceof BinaryEnumObjectImpl) {
+                        BinaryEnumObjectImpl val = (BinaryEnumObjectImpl)fieldVal;
+
+                        fieldVal = val.enumOrdinal();
+                    }
                 }
 
                 stmt.setObject(idx, fieldVal);
@@ -1404,14 +1410,14 @@ protected int fillKeyParameters(PreparedStatement stmt, EntryMapping m, Object k
      */
     protected int fillValueParameters(PreparedStatement stmt, int idx, EntryMapping em, Object val)
         throws CacheWriterException {
-        TypeKind valKind = em.valueKind();
-
-        // Object could be passed by cache in binary format in case of cache configured with setStoreKeepBinary(true).
-        if (valKind == TypeKind.POJO && val instanceof BinaryObject)
-            valKind = TypeKind.BINARY;
-
         for (JdbcTypeField field : em.uniqValFlds) {
-            Object fieldVal = extractParameter(em.cacheName, em.valueType(), valKind, field.getJavaFieldName(), val);
+            Object fieldVal = extractParameter(
+                em.cacheName,
+                em.valueType(),
+                em.valueKind(),
+                field.getJavaFieldName(),
+                val
+            );
 
             fillParameter(stmt, idx++, field, fieldVal);
         }

modules/core/src/main/java/org/apache/ignite/cache/store/jdbc/CacheJdbcPojoStore.java

@@ -23,7 +23,6 @@
 import java.lang.reflect.Method;
 import java.sql.ResultSet;
 import java.sql.SQLException;
-import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.HashMap;

modules/core/src/main/java/org/apache/ignite/internal/processors/cache/store/CacheOsStoreManager.java

@@ -77,6 +77,9 @@ public CacheOsStoreManager(GridKernalContext ctx, CacheConfiguration cfg) {
 
     /** {@inheritDoc} */
     @Override public boolean convertBinary() {
+        if (alwaysKeepBinary)
+            return false;
+
         return configuredConvertBinary() && !(cfgStore instanceof PlatformCacheStore);
     }
 

modules/core/src/main/java/org/apache/ignite/internal/processors/cache/store/GridCacheStoreManagerAdapter.java

@@ -33,10 +33,11 @@
 import javax.cache.integration.CacheWriterException;
 import org.apache.ignite.IgniteCheckedException;
 import org.apache.ignite.IgniteException;
-import org.apache.ignite.IgniteSystemProperties;
+import org.apache.ignite.cache.store.CacheStore;
 import org.apache.ignite.cache.store.CacheStore;
 import org.apache.ignite.cache.store.CacheStoreSession;
 import org.apache.ignite.cache.store.CacheStoreSessionListener;
+import org.apache.ignite.cache.store.jdbc.CacheJdbcPojoStore;
 import org.apache.ignite.configuration.CacheConfiguration;
 import org.apache.ignite.internal.GridKernalContext;
 import org.apache.ignite.internal.processors.cache.CacheEntryImpl;
@@ -76,8 +77,6 @@
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
 
-import static org.apache.ignite.IgniteSystemProperties.IGNITE_LOCAL_STORE_KEEPS_PRIMARY_ONLY;
-
 /**
  * Store manager.
  */
@@ -113,6 +112,9 @@ public abstract class GridCacheStoreManagerAdapter extends GridCacheManagerAdapt
     /** */
     private boolean globalSesLsnrs;
 
+    /** Always keep binary. */
+    protected boolean alwaysKeepBinary;
+
     /** {@inheritDoc} */
     @SuppressWarnings("unchecked")
     @Override public void initialize(@Nullable CacheStore cfgStore, Map sesHolders) throws IgniteCheckedException {
@@ -148,6 +150,9 @@ public abstract class GridCacheStoreManagerAdapter extends GridCacheManagerAdapt
         sesHolder = sesHolder0;
 
         locStore = U.hasAnnotation(cfgStore, CacheLocalStore.class);
+
+        if (cfgStore instanceof CacheJdbcPojoStore)
+            alwaysKeepBinary = true;
     }
 
     /** {@inheritDoc} */

modules/core/src/test/java/org/apache/ignite/cache/store/jdbc/CacheJdbcPojoStoreAbstractSelfTest.java

@@ -35,6 +35,7 @@
 import org.apache.ignite.configuration.CacheConfiguration;
 import org.apache.ignite.configuration.ConnectorConfiguration;
 import org.apache.ignite.configuration.IgniteConfiguration;
+import org.apache.ignite.internal.binary.BinaryMarshaller;
 import org.apache.ignite.internal.util.typedef.internal.U;
 import org.apache.ignite.marshaller.Marshaller;
 import org.apache.ignite.spi.discovery.tcp.TcpDiscoverySpi;
@@ -220,6 +221,7 @@ protected CacheConfiguration cacheConfiguration() throws Exception {
         cc.setCacheMode(PARTITIONED);
         cc.setAtomicityMode(transactional ? TRANSACTIONAL : ATOMIC);
         cc.setWriteBehindEnabled(false);
+        cc.setStoreKeepBinary(storeKeepBinary());
 
         CacheJdbcPojoStoreFactory<Object, Object> storeFactory = new CacheJdbcPojoStoreFactory<>();
         storeFactory.setDialect(new H2Dialect());
@@ -236,6 +238,13 @@ protected CacheConfiguration cacheConfiguration() throws Exception {
         return cc;
     }
 
+    /**
+     * @return Flag indicate keep value in binary format or not.
+     */
+    protected boolean storeKeepBinary(){
+        return false;
+    }
+
     /**
      * Fill in-memory database with sample data.
      *
@@ -396,6 +405,8 @@ public void testLoadCachePrimitiveKeysTx() throws Exception {
      * @throws Exception If failed.
      */
     private void checkPutRemove() throws Exception {
+        boolean binaryMarshaller = marshaller() instanceof BinaryMarshaller || marshaller() == null;
+
         IgniteCache<Object, Person> c1 = grid().cache(CACHE_NAME);
 
         Connection conn = getConnection();
@@ -428,7 +439,9 @@ private void checkPutRemove() throws Exception {
             assertEquals(-2, rs.getInt(2));
             assertEquals(testDate, rs.getDate(3));
             assertEquals("Person-to-test-put-insert", rs.getString(4));
-            assertEquals(testGender.toString(), rs.getString(5));
+
+            assertEquals(testGender.toString(),
+                binaryMarshaller ? Gender.values()[rs.getInt(5)].toString(): rs.getString(5));
 
             assertFalse("Unexpected more data in result set", rs.next());
 
@@ -447,7 +460,9 @@ private void checkPutRemove() throws Exception {
             assertEquals(-3, rs.getInt(2));
             assertEquals(testDate, rs.getDate(3));
             assertEquals("Person-to-test-put-update", rs.getString(4));
-            assertEquals(testGender.toString(), rs.getString(5));
+
+            assertEquals(testGender.toString(),
+                binaryMarshaller ? Gender.values()[rs.getInt(5)].toString(): rs.getString(5));
 
             assertFalse("Unexpected more data in result set", rs.next());
 

modules/core/src/test/java/org/apache/ignite/cache/store/jdbc/CacheJdbcPojoStoreBinaryMarshallerStoreKeepBinarySelfTest.java

@@ -0,0 +1,28 @@
+/*
+ * 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.ignite.cache.store.jdbc;
+
+/**
+ *
+ */
+public class CacheJdbcPojoStoreBinaryMarshallerStoreKeepBinarySelfTest extends CacheJdbcPojoStoreBinaryMarshallerSelfTest {
+    /** {@inheritDoc} */
+    @Override protected boolean storeKeepBinary() {
+        return true;
+    }
+}

modules/core/src/test/java/org/apache/ignite/cache/store/jdbc/CacheJdbcPojoStoreBinaryMarshallerStoreKeepBinaryWithSqlEscapeSelfTest.java

@@ -0,0 +1,28 @@
+/*
+ * 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.ignite.cache.store.jdbc;
+
+/**
+ *
+ */
+public class CacheJdbcPojoStoreBinaryMarshallerStoreKeepBinaryWithSqlEscapeSelfTest extends CacheJdbcPojoStoreBinaryMarshallerWithSqlEscapeSelfTest {
+    /** {@inheritDoc} */
+    @Override protected boolean storeKeepBinary() {
+        return true;
+    }
+}