nested CD registration fix

hazelcast/serialization/api.py

@@ -887,7 +887,7 @@ def write_short(self, field_name, value):
 
     def write_portable(self, field_name, portable):
         """
-        Writes a Portabl
+        Writes a Portable
 
         :param field_name: (str), name of the field.
         :param portable: (:class:`hazelcast.serialization.api.Portable`, portable to be written.

hazelcast/serialization/portable/classdef.py

@@ -27,26 +27,22 @@
 
 
 class FieldDefinition(object):
-    def __init__(self, index, field_name, field_type, version, factory_id=0, class_id=0):
+    def __init__(self, index, field_name, field_type, class_def=None):
         self.index = index
         self.field_name = field_name
         self.field_type = field_type
-        self.version = version
-        self.factory_id = factory_id
-        self.class_id = class_id
+        self.class_def = class_def
 
     def __eq__(self, other):
-        return isinstance(other, self.__class__) \
-               and (self.index, self.field_name, self.field_type, self.version, self.factory_id, self.class_id) == \
-                   (other.index, other.field_name, other.field_type, other.version, other.factory_id, other.class_id)
+        return isinstance(other, self.__class__) and self.index == other.index and \
+               self.field_name == other.field_name and self.field_type == other.field_type \
+               and self.class_def == self.class_def
 
     def __repr__(self):
-        return "FieldDefinition[ ix:{}, name:{}, type:{}, version:{}, fid:{}, cid:{}]".format(self.index,
-                                                                                              self.field_name,
-                                                                                              self.field_type,
-                                                                                              self.version,
-                                                                                              self.factory_id,
-                                                                                              self.class_id)
+        return "FieldDefinition[ ix:{}, name:{}, type:{}, class_def:{}]".format(self.index,
+                                                                                self.field_name,
+                                                                                self.field_type,
+                                                                                self.class_def)
 
 
 class ClassDefinition(object):
@@ -86,7 +82,9 @@ def get_field_type(self, field_name):
     def get_field_class_id(self, field_name):
         fd = self.get_field(field_name)
         if fd:
-            return fd.class_id
+            if fd.class_def:
+                return fd.class_def.class_id
+            return 0
         raise ValueError("Unknown field: {}".format(field_name))
 
     def get_field_count(self):
@@ -97,8 +95,8 @@ def set_version_if_not_set(self, version):
             self.version = version
 
     def __eq__(self, other):
-        return isinstance(other, self.__class__) and (self.factory_id, self.class_id, self.version, self.field_defs) == \
-                                                     (other.factory_id, other.class_id, other.version, other.field_defs)
+        return isinstance(other, self.__class__) and self.factory_id == other.factory_id and self.class_id == \
+               other.class_id and self.version == other.version and self.field_defs == other.field_defs
 
     def __ne__(self, other):
         return not self.__eq__(other)
@@ -107,7 +105,9 @@ def __repr__(self):
         return "fid:{}, cid:{}, v:{}, fields:{}".format(self.factory_id, self.class_id, self.version, self.field_defs)
 
     def __hash__(self):
-        return id(self)//16
+        result = self.class_id
+        result = 31 * result + self.version
+        return result
 
 
 class ClassDefinitionBuilder(object):
@@ -123,87 +123,85 @@ def __init__(self, factory_id, class_id, version=0):
     def add_portable_field(self, field_name, class_def):
         if class_def.class_id is None or class_def.class_id == 0:
             raise ValueError("Portable class id cannot be zero!")
-        self._add_field_by_type(field_name, FieldType.PORTABLE, class_def.version,
-                                class_def.factory_id, class_def.class_id)
+        self._add_field_by_type(field_name, FieldType.PORTABLE, class_def=class_def)
         return self
 
     def add_byte_field(self, field_name):
-        self._add_field_by_type(field_name, FieldType.BYTE, self.version)
+        self._add_field_by_type(field_name, FieldType.BYTE)
         return self
 
     def add_boolean_field(self, field_name):
-        self._add_field_by_type(field_name, FieldType.BOOLEAN, self.version)
+        self._add_field_by_type(field_name, FieldType.BOOLEAN)
         return self
 
     def add_char_field(self, field_name):
-        self._add_field_by_type(field_name, FieldType.CHAR, self.version)
+        self._add_field_by_type(field_name, FieldType.CHAR)
         return self
 
     def add_short_field(self, field_name):
-        self._add_field_by_type(field_name, FieldType.SHORT, self.version)
+        self._add_field_by_type(field_name, FieldType.SHORT)
         return self
 
     def add_int_field(self, field_name):
-        self._add_field_by_type(field_name, FieldType.INT, self.version)
+        self._add_field_by_type(field_name, FieldType.INT)
         return self
 
     def add_long_field(self, field_name):
-        self._add_field_by_type(field_name, FieldType.LONG, self.version)
+        self._add_field_by_type(field_name, FieldType.LONG)
         return self
 
     def add_float_field(self, field_name):
-        self._add_field_by_type(field_name, FieldType.FLOAT, self.version)
+        self._add_field_by_type(field_name, FieldType.FLOAT)
         return self
 
     def add_double_field(self, field_name):
-        self._add_field_by_type(field_name, FieldType.DOUBLE, self.version)
+        self._add_field_by_type(field_name, FieldType.DOUBLE)
         return self
 
     def add_utf_field(self, field_name):
-        self._add_field_by_type(field_name, FieldType.UTF, self.version)
+        self._add_field_by_type(field_name, FieldType.UTF)
         return self
 
     def add_portable_array_field(self, field_name, class_def):
         if class_def.class_id is None or class_def.class_id == 0:
             raise ValueError("Portable class id cannot be zero!")
-        self._add_field_by_type(field_name, FieldType.PORTABLE_ARRAY, class_def.version,
-                                class_def.factory_id, class_def.class_id)
+        self._add_field_by_type(field_name, FieldType.PORTABLE_ARRAY, class_def=class_def)
         return self
 
     def add_byte_array_field(self, field_name):
-        self._add_field_by_type(field_name, FieldType.BYTE_ARRAY, self.version)
+        self._add_field_by_type(field_name, FieldType.BYTE_ARRAY)
         return self
 
     def add_boolean_array_field(self, field_name):
-        self._add_field_by_type(field_name, FieldType.BOOLEAN_ARRAY, self.version)
+        self._add_field_by_type(field_name, FieldType.BOOLEAN_ARRAY)
         return self
 
     def add_char_array_field(self, field_name):
-        self._add_field_by_type(field_name, FieldType.CHAR_ARRAY, self.version)
+        self._add_field_by_type(field_name, FieldType.CHAR_ARRAY)
         return self
 
     def add_short_array_field(self, field_name):
-        self._add_field_by_type(field_name, FieldType.SHORT_ARRAY, self.version)
+        self._add_field_by_type(field_name, FieldType.SHORT_ARRAY)
         return self
 
     def add_int_array_field(self, field_name):
-        self._add_field_by_type(field_name, FieldType.INT_ARRAY, self.version)
+        self._add_field_by_type(field_name, FieldType.INT_ARRAY)
         return self
 
     def add_long_array_field(self, field_name):
-        self._add_field_by_type(field_name, FieldType.LONG_ARRAY, self.version)
+        self._add_field_by_type(field_name, FieldType.LONG_ARRAY)
         return self
 
     def add_float_array_field(self, field_name):
-        self._add_field_by_type(field_name, FieldType.FLOAT_ARRAY, self.version)
+        self._add_field_by_type(field_name, FieldType.FLOAT_ARRAY)
         return self
 
     def add_double_array_field(self, field_name):
-        self._add_field_by_type(field_name, FieldType.DOUBLE_ARRAY, self.version)
+        self._add_field_by_type(field_name, FieldType.DOUBLE_ARRAY)
         return self
 
     def add_utf_array_field(self, field_name):
-        self._add_field_by_type(field_name, FieldType.UTF_ARRAY, self.version)
+        self._add_field_by_type(field_name, FieldType.UTF_ARRAY)
         return self
 
     def add_field_def(self, field_def):
@@ -221,9 +219,9 @@ def build(self):
             cd.add_field_def(field_def)
         return cd
 
-    def _add_field_by_type(self, field_name, field_type, version, factory_id=0, class_id=0):
+    def _add_field_by_type(self, field_name, field_type, class_def=None):
         self._check()
-        self._field_defs.append(FieldDefinition(self._index, field_name, field_type, version, factory_id, class_id))
+        self._field_defs.append(FieldDefinition(self._index, field_name, field_type, class_def=class_def))
         self._index += 1
 
     def _check(self):

hazelcast/serialization/portable/context.py

@@ -70,8 +70,9 @@ def read_class_definition(self, data_in, factory_id, class_id, version):
                     self.read_class_definition(data_in, field_factory_id, field_class_id, field_version)
                 else:
                     register = False
-            builder.add_field_def(FieldDefinition(i, field_name.decode('ascii'), field_type, field_version,
-                                                  field_factory_id, field_class_id))
+            fd = FieldDefinition(i, field_name.decode('ascii'), field_type,
+                                 class_def=ClassDefinition(field_factory_id, field_class_id, field_version))
+            builder.add_field_def(fd)
         class_def = builder.build()
         if register:
             class_def = self.register_class_definition(class_def)

hazelcast/serialization/portable/reader.py

@@ -4,6 +4,7 @@
 from hazelcast.serialization.portable.classdef import FieldType
 from hazelcast.six.moves import range
 
+
 class DefaultPortableReader(PortableReader):
     def __init__(self, portable_serializer, data_input, class_def):
         self._portable_serializer = portable_serializer
@@ -270,10 +271,11 @@ def _read_position_by_field_def(self, fd):
 
 
 def _check_factory_and_class(field_def, factory_id, class_id):
-    if factory_id != field_def.factory_id:
-        raise ValueError("Invalid factoryId! Expected: {}, Current: {}".format(factory_id, field_def.factory_id))
-    if class_id != field_def.class_id:
-        raise ValueError("Invalid classId! Expected: {}, Current: {}".format(class_id, field_def.class_id))
+    if factory_id != field_def.class_def.factory_id:
+        raise ValueError("Invalid factoryId! Expected: {}, Current: {}"
+                         .format(factory_id, field_def.class_def.factory_id))
+    if class_id != field_def.class_def.class_id:
+        raise ValueError("Invalid classId! Expected: {}, Current: {}".format(class_id, field_def.class_def.class_id))
 
 
 class MorphingPortableReader(DefaultPortableReader):

hazelcast/serialization/portable/writer.py

@@ -102,8 +102,8 @@ def write_portable(self, field_name, portable):
         is_none = portable is None
         self._out.write_boolean(is_none)
 
-        self._out.write_int(fd.factory_id)
-        self._out.write_int(fd.class_id)
+        self._out.write_int(fd.class_def.factory_id)
+        self._out.write_int(fd.class_def.class_id)
 
         if not is_none:
             _check_portable_attributes(fd, portable)
@@ -113,8 +113,8 @@ def write_portable_array(self, field_name, values):
         fd = self._set_position(field_name, FieldType.PORTABLE_ARRAY)
         _len = NULL_ARRAY_LENGTH if values is None else len(values)
         self._out.write_int(_len)
-        self._out.write_int(fd.factory_id)
-        self._out.write_int(fd.class_id)
+        self._out.write_int(fd.class_def.factory_id)
+        self._out.write_int(fd.class_def.class_id)
         if _len > 0:
             _offset = self._out.position()
             self._out.write_zero_bytes(_len * 4)
@@ -170,11 +170,11 @@ def end(self):
 
 
 def _check_portable_attributes(field_def, portable):
-    if field_def.factory_id != portable.get_factory_id():
+    if field_def.class_def.factory_id != portable.get_factory_id():
         raise HazelcastSerializationError("Wrong Portable type! Generic portable types are not supported! "
                                           "Expected factory-id: {}, Actual factory-id: {}"
                                           .format(field_def.factory_id, portable.get_factory_id()))
-    if field_def.class_id != portable.get_class_id():
+    if field_def.class_def.class_id != portable.get_class_id():
         raise HazelcastSerializationError("Wrong Portable type! Generic portable types are not supported! "
                                           "Expected class-id: {}, Actual class-id: {}"
                                           .format(field_def.class_id, portable.get_class_id()))

hazelcast/serialization/service.py

@@ -80,24 +80,37 @@ def _register_constant_serializers(self):
         self._registry.safe_register_serializer(self._registry._python_serializer)
 
     def register_class_definitions(self, class_definitions, check_error):
-        class_defs = dict()
+        factory_map = dict()
         for cd in class_definitions:
-            if cd in class_defs:
-                raise HazelcastSerializationError("Duplicate registration found for class-id:{}".format(cd.class_id))
-            class_defs[cd.class_id] = cd
+            factory_id = cd.factory_id
+            class_def_map = factory_map.get(factory_id)
+            if not class_def_map:
+                class_def_map = {}
+                factory_map[factory_id] = class_def_map
+            class_id = cd.class_id
+            if not class_def_map.get(class_id):
+                class_def_map[class_id] = cd
+
         for cd in class_definitions:
-            self.register_class_definition(cd, class_defs, check_error)
+            self.register_class_definition(cd, factory_map, check_error)
 
-    def register_class_definition(self, cd, class_defs, check_error):
+    def register_class_definition(self, cd, factory_map, check_error):
+        if not factory_map[cd.factory_id].get(cd.class_id):
+            factory_map[cd.factory_id][cd.class_id] = cd
+        if cd != factory_map[cd.factory_id][cd.class_id]:
+            raise HazelcastSerializationError("Duplicate registration found for class-id:{}".format(cd.class_id))
         field_names = cd.get_field_names()
         for field_name in field_names:
             fd = cd.get_field(field_name)
             if fd.field_type == FieldType.PORTABLE or fd.field_type == FieldType.PORTABLE_ARRAY:
-                nested_cd = class_defs.get(fd.class_id, None)
+                fid = fd.class_def.factory_id
+                cid = fd.class_def.class_id
+                class_def_map = factory_map[fid]
+                nested_cd = fd.class_def or class_def_map.get(cid, None)
                 if nested_cd is not None:
-                    self.register_class_definition(nested_cd, class_defs, check_error)
+                    self.register_class_definition(nested_cd, factory_map, check_error)
                     self._portable_context.register_class_definition(nested_cd)
                 elif check_error:
                     raise HazelcastSerializationError(
-                            "Could not find registered ClassDefinition for class-id:{}".format(fd.class_id))
+                            "Could not find registered ClassDefinition for class-id:{}".format(fd.class_def.class_id))
         self._portable_context.register_class_definition(cd)