Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Changes to support better separation between applying global and type serialization #7

Merged
merged 3 commits into from
Oct 6, 2016
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions subzero-all-it-hz38/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -37,4 +37,17 @@
</dependency>
</dependencies>

<repositories>
<repository>
<id>sonatype-snapshots</id>
<name>Sonatype Snapshot Repository</name>
<url>https://oss.sonatype.org/content/repositories/snapshots</url>
<releases>
<enabled>false</enabled>
</releases>
<snapshots>
<enabled>true</enabled>
</snapshots>
</repository>
</repositories>
</project>
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
package info.jerrinot.subzero;

import info.jerrinot.subzero.internal.strategy.GlobalKryoStrategy;

public abstract class AbstractGlobalUserSerializer<T> extends AbstractSerializer<T> {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

JavaDoc is missing. I think users will need to extend this classes if and only if

  1. they need to register custom Kryo serializers
  2. the out-of-the-box way to register custom serializers (=properties for now) is not sufficient.

Is this correct? Do you see any other user-case?


public AbstractGlobalUserSerializer(UserSerializer userSerializer) {
super(new GlobalKryoStrategy(userSerializer));
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
package info.jerrinot.subzero;

import com.hazelcast.core.HazelcastInstance;
import com.hazelcast.core.HazelcastInstanceAware;
import com.hazelcast.nio.ObjectDataInput;
import com.hazelcast.nio.ObjectDataOutput;
import com.hazelcast.nio.serialization.StreamSerializer;
import info.jerrinot.subzero.internal.strategy.KryoStrategy;

import java.io.IOException;
import java.io.InputStream;
import java.io.OutputStream;

public abstract class AbstractSerializer<T> implements StreamSerializer<T>, HazelcastInstanceAware {
Copy link
Owner

@jerrinot jerrinot Oct 5, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this probably should be package private, not public. We do not want users to create own subclasses of this class.

private int autoGeneratedTypeId;
private HazelcastInstance hazelcastInstance;
private KryoStrategy<T> strategy;

AbstractSerializer(KryoStrategy<T> strategy) {
this.strategy = strategy;
}

@Override
public final void write(ObjectDataOutput out, T object) throws IOException {
strategy.write((OutputStream) out, object);
}

@Override
public final T read(ObjectDataInput in) throws IOException {
return strategy.read((InputStream) in);
}

/**
* Override this method to returns your own type ID.
* <p>
* Default implementation relies on serializers registration order - all
* your cluster members have to register SubZero in the same order otherwise
* things will get out-of-sync you you will get tons of serializations errors.
* <p>
* All serializers registered in Hazelcast have to return a unique type ID.
*
* @return serializer type ID.
*/
@Override
public int getTypeId() {
return autoGeneratedTypeId;
}

@Override
public final void destroy() {
strategy.destroy(hazelcastInstance);
}

@Override
public final void setHazelcastInstance(HazelcastInstance hazelcastInstance) {
strategy.setHazelcastInstance(hazelcastInstance);
this.hazelcastInstance = hazelcastInstance;
this.autoGeneratedTypeId = strategy.newId();
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
package info.jerrinot.subzero;

import info.jerrinot.subzero.internal.PropertyUserSerializer;
import info.jerrinot.subzero.internal.strategy.TypedKryoStrategy;

public abstract class AbstractTypeSpecificUserSerializer<T> extends AbstractSerializer<T> {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

JavaDoc is needed here. with a description when and how-to extend this class


public AbstractTypeSpecificUserSerializer(Class<T> clazz, com.esotericsoftware.kryo.Serializer serializer) {
super(new TypedKryoStrategy<T>(clazz, UserSerializerConfig.register(clazz, serializer)));
}

/**
* @param clazz class this serializer uses.
*
*/
public AbstractTypeSpecificUserSerializer(Class<T> clazz) {
super(new TypedKryoStrategy<T>(clazz, new PropertyUserSerializer()));
}
}
60 changes: 3 additions & 57 deletions subzero-core/src/main/java/info/jerrinot/subzero/Serializer.java
Original file line number Diff line number Diff line change
@@ -1,75 +1,21 @@
package info.jerrinot.subzero;

import com.hazelcast.core.HazelcastInstance;
import com.hazelcast.core.HazelcastInstanceAware;
import com.hazelcast.nio.ObjectDataInput;
import com.hazelcast.nio.ObjectDataOutput;
import com.hazelcast.nio.serialization.StreamSerializer;
import info.jerrinot.subzero.internal.PropertyUserSerializer;
import info.jerrinot.subzero.internal.strategy.GlobalKryoStrategy;
import info.jerrinot.subzero.internal.strategy.KryoStrategy;
import info.jerrinot.subzero.internal.strategy.TypedKryoStrategy;

import java.io.IOException;
import java.io.InputStream;
import java.io.OutputStream;

public class Serializer<T> implements StreamSerializer<T>, HazelcastInstanceAware {
private int autoGeneratedTypeId;
private HazelcastInstance hazelcastInstance;
private KryoStrategy<T> strategy;
public final class Serializer<T> extends AbstractSerializer<T> {

Serializer() {
this.strategy = new GlobalKryoStrategy<T>(userSerializers());
super(new GlobalKryoStrategy<T>(new PropertyUserSerializer()));
}

/**
* @param clazz class this serializer uses.
*
*/
public Serializer(Class<T> clazz) {
this.strategy = new TypedKryoStrategy<T>(clazz, userSerializers());
}

@Override
public final void write(ObjectDataOutput out, T object) throws IOException {
strategy.write((OutputStream) out, object);
}

@Override
public final T read(ObjectDataInput in) throws IOException {
return strategy.read((InputStream) in);
super(new TypedKryoStrategy<T>(clazz, new PropertyUserSerializer()));
}

public UserSerializer userSerializers() {
return new PropertyUserSerializer();
}

/**
* Override this method to returns your own type ID.
*
* Default implementation relies on serializers registration order - all
* your cluster members have to register SubZero in the same order otherwise
* things will get out-of-sync you you will get tons of serializations errors.
*
* All serializers registered in Hazelcast have to return a unique type ID.
*
* @return serializer type ID.
*/
@Override
public int getTypeId() {
return autoGeneratedTypeId;
}

@Override
public final void destroy() {
strategy.destroy(hazelcastInstance);
}

@Override
public final void setHazelcastInstance(HazelcastInstance hazelcastInstance) {
strategy.setHazelcastInstance(hazelcastInstance);
this.hazelcastInstance = hazelcastInstance;
this.autoGeneratedTypeId = strategy.newId();
}
}
49 changes: 43 additions & 6 deletions subzero-core/src/main/java/info/jerrinot/subzero/SubZero.java
Original file line number Diff line number Diff line change
Expand Up @@ -23,22 +23,38 @@ private SubZero() {
* Use SubZero as a global serializer.
*
* This method configures Hazelcast to delegate a class serialization to SubZero when the class
* has no explicit strategy configured.
* has no explicit strategy configured. Uses {@link Serializer} serializer implementation
* internally.
*
* @param config Hazelcast configuration to inject SubZero into
* @return Hazelcast configuration.
*/
public static Config useAsGlobalSerializer(Config config) {
return useAsGlobalSerializerInternal(config, Serializer.class);
}

/**
* Use SubZero as a global serializer.
*
* This method configures Hazelcast to delegate a class serialization to SubZero when the class
* has no explicit strategy configured.
*
* @param config Hazelcast configuration to inject SubZero into
* @param serializerClazz Class of global serializer implementation to use
* @return Hazelcast configuration.
*/
public static <T> Config useAsGlobalSerializer(Config config, Class<? extends AbstractGlobalUserSerializer> serializerClazz) {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would not add this method. Methods in the SubZero class are for user-convenience. For a quick-start if the default SubZero is good enough for you. If you are implementing own serializer then you do not need the convenience.

I expect majority of users won't/shouldn't need to create custom subclasses. The extra methods here could be confusing for new users.

Copy link
Author

@wneild wneild Oct 5, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The way I see it, SubZero is all about providing convenience, so I'm not convinced about not having some convenience method to add custom serialization configuration. The alternative is forcing the user to write out what's already available internally to SubZero class when registering the custom global serializer to hazelcast, which seems a waste.

I couldn't comment on how widely used custom serialization is (probably about 10% of Kryo users going by github stars https://github.com/magro/kryo-serializers) but I will remove the methods if you're not convinced.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, makes sense. let's keep it in.

SerializationConfig serializationConfig = config.getSerializationConfig();
injectSubZero(serializationConfig);
injectSubZero(serializationConfig, serializerClazz);
return config;
}

/**
* Use SubZero as a global serializer.
*
* This method configures Hazelcast to delegate a class serialization to SubZero when the class
* has no explicit strategy configured.
* has no explicit strategy configured. Uses {@link Serializer} serializer implementation
* internally.
*
* This method it intended to be used to configure {@link ClientConfig} instances, but
* I do not want to create a hard-dependency on Hazelcast Client module.
Expand All @@ -47,6 +63,27 @@ public static Config useAsGlobalSerializer(Config config) {
* @return Hazelcast configuration.
*/
public static <T> T useAsGlobalSerializer(T config) {
return useAsGlobalSerializerInternal(config, Serializer.class);
}

/**
* Use SubZero as a global serializer.
*
* This method configures Hazelcast to delegate a class serialization to SubZero when the class
* has no explicit strategy configured.
*
* This method it intended to be used to configure {@link ClientConfig} instances, but
* I do not want to create a hard-dependency on Hazelcast Client module.
*
* @param config Hazelcast configuration to inject SubZero into
* @param serializerClazz Class of global serializer implementation to use
* @return Hazelcast configuration.
*/
public static <T> T useAsGlobalSerializer(T config, Class<? extends AbstractGlobalUserSerializer> serializerClazz) {
return useAsGlobalSerializer(config, serializerClazz);
}

private static <T> T useAsGlobalSerializerInternal(T config, Class<? extends AbstractSerializer> serializerClazz) {
String className = config.getClass().getName();
SerializationConfig serializationConfig;
if (className.equals("com.hazelcast.client.config.ClientConfig")) {
Expand All @@ -58,17 +95,17 @@ public static <T> T useAsGlobalSerializer(T config) {
} else {
throw new IllegalArgumentException("Unknown configuration object " + config);
}
injectSubZero(serializationConfig);
injectSubZero(serializationConfig, serializerClazz);
return config;
}

private static void injectSubZero(SerializationConfig serializationConfig) {
private static void injectSubZero(SerializationConfig serializationConfig, Class<? extends AbstractSerializer> serializerClazz) {
GlobalSerializerConfig globalSerializerConfig = serializationConfig.getGlobalSerializerConfig();
if (globalSerializerConfig == null) {
globalSerializerConfig = new GlobalSerializerConfig();
serializationConfig.setGlobalSerializerConfig(globalSerializerConfig);
}
globalSerializerConfig.setClassName(Serializer.class.getName()).setOverrideJavaSerialization(true);
globalSerializerConfig.setClassName(serializerClazz.getName()).setOverrideJavaSerialization(true);
}

/**
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
package info.jerrinot.subzero.example;

import info.jerrinot.subzero.Serializer;
import info.jerrinot.subzero.AbstractTypeSpecificUserSerializer;

import java.util.HashMap;

Expand All @@ -19,7 +19,7 @@
* }
* </pre>
*/
public class HashMapSerializerExample extends Serializer<HashMap> {
public class HashMapSerializerExample extends AbstractTypeSpecificUserSerializer<HashMap> {

public HashMapSerializerExample() {
super(HashMap.class);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ public class SerializerTest {

@Test
public void givenSingleSerializerExist_whenHazelcastInstanceIsInjected_thenTypeIdIsGreaterThenZero() {
Serializer serializer = new Serializer();
AbstractSerializer serializer = new Serializer();
serializer.setHazelcastInstance(newMockHazelcastInstance());

HazelcastInstance hz = newMockHazelcastInstance();
Expand All @@ -27,9 +27,9 @@ public void givenSingleSerializerExist_whenHazelcastInstanceIsInjected_thenTypeI

@Test
public void givenTwoSerializerExist_whenTheSameHazelcastInstanceIsInjected_thenTheyHaveDifferentTypeId() {
Serializer serializer1 = new Serializer();
AbstractSerializer serializer1 = new Serializer();
serializer1.setHazelcastInstance(newMockHazelcastInstance());
Serializer serializer2 = new Serializer();
AbstractSerializer serializer2 = new Serializer();
serializer2.setHazelcastInstance(newMockHazelcastInstance());

HazelcastInstance hz = newMockHazelcastInstance();
Expand All @@ -43,9 +43,9 @@ public void givenTwoSerializerExist_whenTheSameHazelcastInstanceIsInjected_thenT

@Test
public void givenTwoSerializerExist_whenDifferentHazelcastInstanceAreInjected_thenTheyHaveTheSameTypeId() {
Serializer serializer1 = new Serializer();
AbstractSerializer serializer1 = new Serializer();
serializer1.setHazelcastInstance(newMockHazelcastInstance());
Serializer serializer2 = new Serializer();
AbstractSerializer serializer2 = new Serializer();
serializer2.setHazelcastInstance(newMockHazelcastInstance());

HazelcastInstance hz1 = newMockHazelcastInstance();
Expand All @@ -58,6 +58,17 @@ public void givenTwoSerializerExist_whenDifferentHazelcastInstanceAreInjected_th
assertEquals(typeId1, typeId2);
}

@Test
public void givenSpecificNeutralSerializerExist_whenObjectIsSerializedAndDeserialized_thenItHasTheSameInternalState() throws IOException {
String input = "foo";
AbstractSerializer<String> serializer = new Serializer();
serializer.setHazelcastInstance(newMockHazelcastInstance());

String output = TestUtils.serializeAndDeserializeObject(serializer, input);

assertEquals(input, output);
}

@Test(expected = AssertionError.class)
public void givenTwoTypeSpecificSerializersWithTheSameTypeExists_whenTheSameHazelcastInstanceIsInjected_thenTheyThrowError() {
Serializer serializer1 = new Serializer(String.class);
Expand Down Expand Up @@ -89,17 +100,6 @@ public void givenTwoTypeSpecificSerializersWithTheSameTypeExists_whenDifferentHa
assertEquals(typeId1, typeId2);
}

@Test
public void givenSpecificNeutralSerializerExist_whenObjectIsSerializedAndDeserialized_thenItHasTheSameInternalState() throws IOException {
String input = "foo";
Serializer<String> serializer = new Serializer();
serializer.setHazelcastInstance(newMockHazelcastInstance());

String output = TestUtils.serializeAndDeserializeObject(serializer, input);

assertEquals(input, output);
}

@Test
public void givenTypeSpecificSerializerExist_whenObjectIsSerializedAndDeserialized_thenItHasTheSameInternalState() throws IOException {
String input = "foo";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,7 @@
import java.util.Collection;

import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.tuple;
import static org.junit.Assert.assertEquals;
import static org.mockito.Matchers.anyObject;

public class SubZeroTest {

Expand Down
Loading