You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@ignite.apache.org by al...@apache.org on 2020/02/14 08:03:02 UTC

[ignite] branch master updated: IGNITE-12624 Java thin client: typeId generation for system types fixed - Fixes #7416.

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

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


The following commit(s) were added to refs/heads/master by this push:
     new 59917f0  IGNITE-12624 Java thin client: typeId generation for system types fixed - Fixes #7416.
59917f0 is described below

commit 59917f0731ea79168a3a9f03226348a6a36346b6
Author: Aleksey Plekhanov <pl...@gmail.com>
AuthorDate: Fri Feb 14 10:53:56 2020 +0300

    IGNITE-12624 Java thin client: typeId generation for system types fixed - Fixes #7416.
---
 .../ignite/internal/MarshallerContextImpl.java     |  82 +++---------
 .../internal/client/thin/TcpIgniteClient.java      |  19 ++-
 .../apache/ignite/marshaller/MarshallerUtils.java  |  68 ++++++++++
 .../org/apache/ignite/client/FunctionalTest.java   | 140 +++++++++------------
 4 files changed, 158 insertions(+), 151 deletions(-)

diff --git a/modules/core/src/main/java/org/apache/ignite/internal/MarshallerContextImpl.java b/modules/core/src/main/java/org/apache/ignite/internal/MarshallerContextImpl.java
index 02aefd9..72f19a1 100644
--- a/modules/core/src/main/java/org/apache/ignite/internal/MarshallerContextImpl.java
+++ b/modules/core/src/main/java/org/apache/ignite/internal/MarshallerContextImpl.java
@@ -17,16 +17,12 @@
 
 package org.apache.ignite.internal;
 
-import java.io.BufferedReader;
 import java.io.File;
 import java.io.IOException;
-import java.io.InputStreamReader;
-import java.net.URL;
 import java.util.AbstractMap;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
-import java.util.Enumeration;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.Iterator;
@@ -54,6 +50,7 @@ import org.apache.ignite.internal.util.typedef.internal.CU;
 import org.apache.ignite.internal.util.typedef.internal.U;
 import org.apache.ignite.lang.IgnitePredicate;
 import org.apache.ignite.marshaller.MarshallerContext;
+import org.apache.ignite.marshaller.MarshallerUtils;
 import org.apache.ignite.marshaller.jdk.JdkMarshaller;
 import org.apache.ignite.plugin.PluginProvider;
 import org.jetbrains.annotations.NotNull;
@@ -114,41 +111,28 @@ public class MarshallerContextImpl implements MarshallerContext {
         try {
             ClassLoader ldr = U.gridClassLoader();
 
-            Enumeration<URL> urls = ldr.getResources(CLS_NAMES_FILE);
-
-            boolean foundClsNames = false;
-
-            while (urls.hasMoreElements()) {
-                processResource(urls.nextElement());
-
-                foundClsNames = true;
-            }
-
-            if (!foundClsNames)
-                throw new IgniteException("Failed to load class names properties file packaged with ignite binaries " +
-                    "[file=" + CLS_NAMES_FILE + ", ldr=" + ldr + ']');
+            MarshallerUtils.processSystemClasses(ldr, plugins, clsName -> {
+                int typeId = clsName.hashCode();
 
-            URL jdkClsNames = ldr.getResource(JDK_CLS_NAMES_FILE);
+                MappedName oldClsName;
 
-            if (jdkClsNames == null)
-                throw new IgniteException("Failed to load class names properties file packaged with ignite binaries " +
-                    "[file=" + JDK_CLS_NAMES_FILE + ", ldr=" + ldr + ']');
+                if ((oldClsName = sysTypesMap.put(typeId, new MappedName(clsName, true))) != null) {
+                    if (!oldClsName.className().equals(clsName))
+                        throw new IgniteException(
+                            "Duplicate type ID [id="
+                                + typeId
+                                + ", oldClsName="
+                                + oldClsName
+                                + ", clsName="
+                                + clsName + ']');
+                }
 
-            processResource(jdkClsNames);
+                sysTypesSet.add(clsName);
+            });
 
             checkHasClassName(GridDhtPartitionFullMap.class.getName(), ldr, CLS_NAMES_FILE);
             checkHasClassName(GridDhtPartitionMap.class.getName(), ldr, CLS_NAMES_FILE);
             checkHasClassName(HashMap.class.getName(), ldr, JDK_CLS_NAMES_FILE);
-
-            if (plugins != null && !plugins.isEmpty()) {
-                for (PluginProvider plugin : plugins) {
-                    Enumeration<URL> pluginUrls = ldr.getResources("META-INF/" + plugin.name().toLowerCase()
-                        + ".classnames.properties");
-
-                    while (pluginUrls.hasMoreElements())
-                        processResource(pluginUrls.nextElement());
-                }
-            }
         }
         catch (IOException e) {
             throw new IllegalStateException("Failed to initialize marshaller context.", e);
@@ -222,40 +206,6 @@ public class MarshallerContextImpl implements MarshallerContext {
                 "[clsName=" + clsName + ", fileName=" + fileName + ", ldr=" + ldr + ']');
     }
 
-    /**
-     * @param url Resource URL.
-     * @throws IOException In case of error.
-     */
-    private void processResource(URL url) throws IOException {
-        try (BufferedReader rdr = new BufferedReader(new InputStreamReader(url.openStream()))) {
-            String line;
-
-            while ((line = rdr.readLine()) != null) {
-                if (line.isEmpty() || line.startsWith("#"))
-                    continue;
-
-                String clsName = line.trim();
-
-                int typeId = clsName.hashCode();
-
-                MappedName oldClsName;
-
-                if ((oldClsName = sysTypesMap.put(typeId, new MappedName(clsName, true))) != null) {
-                    if (!oldClsName.className().equals(clsName))
-                        throw new IgniteException(
-                                "Duplicate type ID [id="
-                                        + typeId
-                                        + ", oldClsName="
-                                        + oldClsName
-                                        + ", clsName="
-                                        + clsName + ']');
-                }
-
-                sysTypesSet.add(clsName);
-            }
-        }
-    }
-
     /** {@inheritDoc} */
     @Override public boolean registerClassName(
         byte platformId,
diff --git a/modules/core/src/main/java/org/apache/ignite/internal/client/thin/TcpIgniteClient.java b/modules/core/src/main/java/org/apache/ignite/internal/client/thin/TcpIgniteClient.java
index b6226a1..57d447f 100644
--- a/modules/core/src/main/java/org/apache/ignite/internal/client/thin/TcpIgniteClient.java
+++ b/modules/core/src/main/java/org/apache/ignite/internal/client/thin/TcpIgniteClient.java
@@ -20,6 +20,7 @@ package org.apache.ignite.internal.client.thin;
 import java.io.IOException;
 import java.util.Arrays;
 import java.util.Collection;
+import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.concurrent.ConcurrentHashMap;
@@ -52,6 +53,7 @@ import org.apache.ignite.internal.binary.streams.BinaryOutputStream;
 import org.apache.ignite.internal.util.typedef.internal.U;
 import org.apache.ignite.lang.IgnitePredicate;
 import org.apache.ignite.marshaller.MarshallerContext;
+import org.apache.ignite.marshaller.MarshallerUtils;
 import org.apache.ignite.marshaller.jdk.JdkMarshaller;
 
 /**
@@ -335,6 +337,21 @@ public class TcpIgniteClient implements IgniteClient {
         /** Type ID -> class name map. */
         private Map<Integer, String> cache = new ConcurrentHashMap<>();
 
+        /** System types. */
+        private final Collection<String> sysTypes = new HashSet<>();
+
+        /**
+         * Default constructor.
+         */
+        public ClientMarshallerContext() {
+            try {
+                MarshallerUtils.processSystemClasses(U.gridClassLoader(), null, sysTypes::add);
+            }
+            catch (IOException e) {
+                throw new IllegalStateException("Failed to initialize marshaller context.", e);
+            }
+        }
+
         /** {@inheritDoc} */
         @Override public boolean registerClassName(
             byte platformId,
@@ -431,7 +448,7 @@ public class TcpIgniteClient implements IgniteClient {
 
         /** {@inheritDoc} */
         @Override public boolean isSystemType(String typeName) {
-            return false;
+            return sysTypes.contains(typeName);
         }
 
         /** {@inheritDoc} */
diff --git a/modules/core/src/main/java/org/apache/ignite/marshaller/MarshallerUtils.java b/modules/core/src/main/java/org/apache/ignite/marshaller/MarshallerUtils.java
index d5cf386..9568b71 100644
--- a/modules/core/src/main/java/org/apache/ignite/marshaller/MarshallerUtils.java
+++ b/modules/core/src/main/java/org/apache/ignite/marshaller/MarshallerUtils.java
@@ -24,12 +24,18 @@ import java.io.FileNotFoundException;
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.InputStreamReader;
+import java.net.URL;
+import java.util.Collection;
+import java.util.Enumeration;
+import java.util.function.Consumer;
 import org.apache.ignite.IgniteCheckedException;
+import org.apache.ignite.IgniteException;
 import org.apache.ignite.IgniteSystemProperties;
 import org.apache.ignite.internal.ClassSet;
 import org.apache.ignite.lang.IgnitePredicate;
 import org.apache.ignite.lang.IgniteProductVersion;
 import org.apache.ignite.marshaller.jdk.JdkMarshaller;
+import org.apache.ignite.plugin.PluginProvider;
 import org.jetbrains.annotations.Nullable;
 
 /**
@@ -220,4 +226,66 @@ public class MarshallerUtils {
                 "[path=" + fileName + ']', e);
         }
     }
+
+    /**
+     * Find all system class names (for JDK or Ignite classes) and process them with a given consumer.
+     *
+     * @param ldr Class loader.
+     * @param plugins Plugins.
+     * @param proc Class processor (class name consumer).
+     */
+    public static void processSystemClasses(ClassLoader ldr, @Nullable Collection<PluginProvider> plugins,
+        Consumer<String> proc) throws IOException {
+        Enumeration<URL> urls = ldr.getResources(CLS_NAMES_FILE);
+
+        boolean foundClsNames = false;
+
+        while (urls.hasMoreElements()) {
+            processResource(urls.nextElement(), proc);
+
+            foundClsNames = true;
+        }
+
+        if (!foundClsNames)
+            throw new IgniteException("Failed to load class names properties file packaged with ignite binaries " +
+                "[file=" + CLS_NAMES_FILE + ", ldr=" + ldr + ']');
+
+        URL jdkClsNames = ldr.getResource(JDK_CLS_NAMES_FILE);
+
+        if (jdkClsNames == null)
+            throw new IgniteException("Failed to load class names properties file packaged with ignite binaries " +
+                "[file=" + JDK_CLS_NAMES_FILE + ", ldr=" + ldr + ']');
+
+        processResource(jdkClsNames, proc);
+
+        if (plugins != null && !plugins.isEmpty()) {
+            for (PluginProvider plugin : plugins) {
+                Enumeration<URL> pluginUrls = ldr.getResources("META-INF/" + plugin.name().toLowerCase()
+                    + ".classnames.properties");
+
+                while (pluginUrls.hasMoreElements())
+                    processResource(pluginUrls.nextElement(), proc);
+            }
+        }
+    }
+
+    /**
+     * Process resource containing class names.
+     *
+     * @param url Resource URL.
+     * @param proc Class processor (class name consumer).
+     * @throws IOException In case of error.
+     */
+    private static void processResource(URL url, Consumer<String> proc) throws IOException {
+        try (BufferedReader rdr = new BufferedReader(new InputStreamReader(url.openStream()))) {
+            String line;
+
+            while ((line = rdr.readLine()) != null) {
+                if (line.isEmpty() || line.startsWith("#"))
+                    continue;
+
+                proc.accept(line.trim());
+            }
+        }
+    }
 }
diff --git a/modules/core/src/test/java/org/apache/ignite/client/FunctionalTest.java b/modules/core/src/test/java/org/apache/ignite/client/FunctionalTest.java
index 020c1a5..c369c27 100644
--- a/modules/core/src/test/java/org/apache/ignite/client/FunctionalTest.java
+++ b/modules/core/src/test/java/org/apache/ignite/client/FunctionalTest.java
@@ -21,7 +21,6 @@ import java.lang.management.ManagementFactory;
 import java.util.AbstractMap.SimpleEntry;
 import java.util.ArrayList;
 import java.util.Arrays;
-import java.util.Collection;
 import java.util.Collections;
 import java.util.Date;
 import java.util.HashMap;
@@ -63,7 +62,6 @@ import org.apache.ignite.internal.client.thin.ClientServerError;
 import org.apache.ignite.internal.processors.odbc.ClientListenerProcessor;
 import org.apache.ignite.internal.processors.platform.cache.expiry.PlatformExpiryPolicy;
 import org.apache.ignite.internal.processors.platform.client.ClientStatus;
-import org.apache.ignite.internal.util.GridLeanMap;
 import org.apache.ignite.internal.util.typedef.F;
 import org.apache.ignite.internal.util.typedef.T2;
 import org.apache.ignite.internal.util.typedef.internal.U;
@@ -292,72 +290,71 @@ public class FunctionalTest {
         try (Ignite ignite = Ignition.start(Config.getServerConfiguration());
              IgniteClient client = Ignition.startClient(getClientConfiguration())
         ) {
-            IgniteCache<Object, Object> thickCache = ignite.getOrCreateCache(Config.DEFAULT_CACHE_NAME);
-            ClientCache<Object, Object> thinCache = client.getOrCreateCache(Config.DEFAULT_CACHE_NAME);
+            ignite.getOrCreateCache(Config.DEFAULT_CACHE_NAME);
 
             Person person = new Person(1, "name");
 
             // Primitive and built-in types.
-            checkDataType(thinCache, thickCache, (byte)1);
-            checkDataType(thinCache, thickCache, (short)1);
-            checkDataType(thinCache, thickCache, 1);
-            checkDataType(thinCache, thickCache, 1L);
-            checkDataType(thinCache, thickCache, 1.0f);
-            checkDataType(thinCache, thickCache, 1.0d);
-            checkDataType(thinCache, thickCache, 'c');
-            checkDataType(thinCache, thickCache, true);
-            checkDataType(thinCache, thickCache, "string");
-            checkDataType(thinCache, thickCache, UUID.randomUUID());
-            checkDataType(thinCache, thickCache, new Date());
+            checkDataType(client, ignite, (byte)1);
+            checkDataType(client, ignite, (short)1);
+            checkDataType(client, ignite, 1);
+            checkDataType(client, ignite, 1L);
+            checkDataType(client, ignite, 1.0f);
+            checkDataType(client, ignite, 1.0d);
+            checkDataType(client, ignite, 'c');
+            checkDataType(client, ignite, true);
+            checkDataType(client, ignite, "string");
+            checkDataType(client, ignite, UUID.randomUUID());
+            checkDataType(client, ignite, new Date());
 
             // Enum.
-            checkDataType(thinCache, thickCache, CacheAtomicityMode.ATOMIC);
+            checkDataType(client, ignite, CacheAtomicityMode.ATOMIC);
 
             // Binary object.
-            checkDataType(thinCache, thickCache, person);
+            checkDataType(client, ignite, person);
 
             // Arrays.
-            checkDataType(thinCache, thickCache, new byte[] {(byte)1});
-            checkDataType(thinCache, thickCache, new short[] {(short)1});
-            checkDataType(thinCache, thickCache, new int[] {1});
-            checkDataType(thinCache, thickCache, new long[] {1L});
-            checkDataType(thinCache, thickCache, new float[] {1.0f});
-            checkDataType(thinCache, thickCache, new double[] {1.0d});
-            checkDataType(thinCache, thickCache, new char[] {'c'});
-            checkDataType(thinCache, thickCache, new boolean[] {true});
-            checkDataType(thinCache, thickCache, new String[] {"string"});
-            checkDataType(thinCache, thickCache, new UUID[] {UUID.randomUUID()});
-            checkDataType(thinCache, thickCache, new Date[] {new Date()});
-            checkDataType(thinCache, thickCache, new int[][] {new int[] {1}});
-
-            checkDataType(thinCache, thickCache, new CacheAtomicityMode[] {CacheAtomicityMode.ATOMIC});
-
-            checkDataType(thinCache, thickCache, new Person[] {person});
-            checkDataType(thinCache, thickCache, new Person[][] {new Person[] {person}});
-            checkDataType(thinCache, thickCache, new Object[] {1, "string", person, new Person[] {person}});
+            checkDataType(client, ignite, new byte[] {(byte)1});
+            checkDataType(client, ignite, new short[] {(short)1});
+            checkDataType(client, ignite, new int[] {1});
+            checkDataType(client, ignite, new long[] {1L});
+            checkDataType(client, ignite, new float[] {1.0f});
+            checkDataType(client, ignite, new double[] {1.0d});
+            checkDataType(client, ignite, new char[] {'c'});
+            checkDataType(client, ignite, new boolean[] {true});
+            checkDataType(client, ignite, new String[] {"string"});
+            checkDataType(client, ignite, new UUID[] {UUID.randomUUID()});
+            checkDataType(client, ignite, new Date[] {new Date()});
+            checkDataType(client, ignite, new int[][] {new int[] {1}});
+
+            checkDataType(client, ignite, new CacheAtomicityMode[] {CacheAtomicityMode.ATOMIC});
+
+            checkDataType(client, ignite, new Person[] {person});
+            checkDataType(client, ignite, new Person[][] {new Person[] {person}});
+            checkDataType(client, ignite, new Object[] {1, "string", person, new Person[] {person}});
 
             // Lists.
-            checkDataType(thinCache, thickCache, Collections.emptyList());
-            checkDataType(thinCache, thickCache, Collections.singletonList(person));
-            checkDataType(thinCache, thickCache, Arrays.asList(person, person));
-            checkDataType(thinCache, thickCache, new ArrayList<>(Arrays.asList(person, person)));
-            checkDataType(thinCache, thickCache, new LinkedList<>(Arrays.asList(person, person)));
-            checkDataType(thinCache, thickCache, Arrays.asList(Arrays.asList(person, person), person));
+            checkDataType(client, ignite, Collections.emptyList());
+            checkDataType(client, ignite, Collections.singletonList(person));
+            checkDataType(client, ignite, Arrays.asList(person, person));
+            checkDataType(client, ignite, new ArrayList<>(Arrays.asList(person, person)));
+            checkDataType(client, ignite, new LinkedList<>(Arrays.asList(person, person)));
+            checkDataType(client, ignite, Arrays.asList(Arrays.asList(person, person), person));
 
             // Sets.
-            checkDataType(thinCache, thickCache, Collections.emptySet());
-            checkDataType(thinCache, thickCache, Collections.singleton(person));
-            checkDataType(thinCache, thickCache, new HashSet<>(Arrays.asList(1, 2)));
-            checkDataType(thinCache, thickCache, new HashSet<>(Arrays.asList(Arrays.asList(person, person), person)));
-            checkDataType(thinCache, thickCache, new HashSet<>(new ArrayList<>(Arrays.asList(Arrays.asList(person,
+            checkDataType(client, ignite, Collections.emptySet());
+            checkDataType(client, ignite, Collections.singleton(person));
+            checkDataType(client, ignite, new HashSet<>(Arrays.asList(1, 2)));
+            checkDataType(client, ignite, new HashSet<>(Arrays.asList(Arrays.asList(person, person), person)));
+            checkDataType(client, ignite, new HashSet<>(new ArrayList<>(Arrays.asList(Arrays.asList(person,
                 person), person))));
 
             // Maps.
-            checkDataType(thinCache, thickCache, Collections.emptyMap());
-            checkDataType(thinCache, thickCache, Collections.singletonMap(1, person));
-            checkDataType(thinCache, thickCache, F.asMap(1, person));
-            checkDataType(thinCache, thickCache, new HashMap<>(F.asMap(1, person)));
-            checkDataType(thinCache, thickCache, new HashMap<>(F.asMap(new HashSet<>(Arrays.asList(1, 2)),
+            checkDataType(client, ignite, Collections.emptyMap());
+            checkDataType(client, ignite, Collections.singletonMap(1, person));
+            checkDataType(client, ignite, F.asMap(1, person));
+            checkDataType(client, ignite, new HashMap<>(F.asMap(1, person)));
+            checkDataType(client, ignite, new HashMap<>(F.asMap(new HashSet<>(Arrays.asList(1, 2)),
                 Arrays.asList(person, person))));
         }
     }
@@ -365,12 +362,14 @@ public class FunctionalTest {
     /**
      * Check that we get the same value from the cache as we put before.
      *
-     * @param thinCache Thin client cache.
-     * @param thickCache Thick client cache.
+     * @param client Thin client.
+     * @param ignite Ignite node.
      * @param obj Value of data type to check.
      */
-    private void checkDataType(ClientCache<Object, Object> thinCache, IgniteCache<Object, Object> thickCache,
-        Object obj) {
+    private void checkDataType(IgniteClient client, Ignite ignite, Object obj) {
+        IgniteCache<Object, Object> thickCache = ignite.cache(Config.DEFAULT_CACHE_NAME);
+        ClientCache<Object, Object> thinCache = client.cache(Config.DEFAULT_CACHE_NAME);
+
         Integer key = 1;
 
         thinCache.put(key, obj);
@@ -381,12 +380,9 @@ public class FunctionalTest {
 
         assertEqualsArraysAware(obj, cachedObj);
 
-        // TODO IGNITE-12624 Put object to thick cache to register binary type (workaround for system types registration)
-        thickCache.put(2, obj);
+        assertEqualsArraysAware(obj, thickCache.get(key));
 
-        // TODO IGNITE-12624 Skip check for system types marshalled by optimized marshaller.
-        if (!hasSystemOptimizedMarshallerType(obj))
-            assertEqualsArraysAware(obj, thickCache.get(key));
+        assertEquals(client.binary().typeId(obj.getClass().getName()), ignite.binary().typeId(obj.getClass().getName()));
 
         if (!obj.getClass().isArray()) { // TODO IGNITE-12578
             // Server-side comparison with the original object.
@@ -398,30 +394,6 @@ public class FunctionalTest {
     }
 
     /**
-     * Check recursively if the object has system types which marshalled by optimized marshaller.
-     *
-     * Note: This is temporary method needed to workaround IGNITE-12624.
-     */
-    private boolean hasSystemOptimizedMarshallerType(Object obj) {
-        if (obj.getClass().getName().startsWith("java.util.Collections") ||
-            obj.getClass().getName().startsWith("java.util.Arrays") ||
-            obj instanceof GridLeanMap)
-            return true;
-        else if (obj instanceof Collection) {
-            for (Object obj0 : (Iterable<?>)obj) {
-                if (hasSystemOptimizedMarshallerType(obj0))
-                    return true;
-            }
-        }
-        else if (obj instanceof Map) {
-            return hasSystemOptimizedMarshallerType(((Map)obj).values()) ||
-                hasSystemOptimizedMarshallerType(((Map)obj).keySet());
-        }
-
-        return false;
-    }
-
-    /**
      * Assert values equals (deep equals for arrays).
      *
      * @param exp Expected value.