Learn how easy it is to sync an existing GitHub or Google Code repo to a SourceForge project! See Demo

Close

Custom Transformer sometimes(!) not invoked

Help
2012-03-19
2013-03-19
  • We use wrapper objects for our database identifiers. These wrapper object only contain one single attribute "value" of type String. We use the Transformer described below to serialize the class:

    public class IdentifierTransformer implements Transformer, Inline {
      @Override
      public void transform(Object object) {
        Identifier identifer = (Identifier) object;
        if (identifer != null && isNotBlank(identifer.getValue())) {
          JSONContext.get().writeQuoted(identifer.getValue());
        }
        else {
          JSONContext.get().write(null);
        }
      }
      @Override
      public Boolean isInline() {
        return false;
      }
    }
    

    I observed a very strange behaviour:
    It seems, that the IdentifierTransformer is not always used in our continuous integration runs. In some runs, the identifier attribute of our domain objects is serialized inline. In other runs it is serialized as a complete object, with a class attribute and a value attribute - without any code change. I need some help in debugging this strange behaviour. I want to find out which are the circumctances when the incorrect serialization was applied.

    Joerg

     
  • I'd have to see your unit test code and how the serialization is setup.  Sounds like it could be a multi-threaded issue where the setup of the JSONSerializer isn't finished in all cases before the test starts to run.  Or if you are doing lazy initialization of the JSONSerializer and it could potentially serialize something prior to the JSONSerializer is fully populated.  If you are using other Transformers, includes, etc do those show up?  Are those setup before or after the IdentifierTransformer?  I'd look to answer some of those questions first.

     
  • Sorry for the late reply.
    The problem occurs during our Integrationtests with Webdriver, we were not be able to reproduce them in unittests.
    Here are the main parts of our serializer setup:

    public void serialize(BusinessObject businessObject) {
      new BusinessClassFlexJsonSerializer().serializeToJson(businessObject);
    }
    public class BusinessClassFlexJsonSerializer extends FlexJsonSerializer<BusinessClass> {
      public BusinesObjectFlexJsonSerializer() {
        serializer.transform(someSpecificTransformer, someSpecificClass);
        ...
      }
    }
    public class FlexJsonSerializer<T> {
      final JSONSerializer serializer = new JSONSerializer();
      public FlexJsonSerializer() {
        serializer.prettyPrint(false);
        registerTransformer();
      }
      private void registerTransformer() {
        serializer.transform(someTransformer, someClass);
        ...
        serializer.transform(new IdentifierTransformer(), Identifier.class);
        serializer.transform(otherTransformer, otherClass);
        ...
      }
      public String serializeToJson(T businessObject) {
        return serializer.serialize(businessObject);
      }
    }
    

    I already thought about some multithreading issues. Therefor I create a new instance of our Serializer every time a serialization need to be executed. That is quite expensive, but it did not solve the problem.

    During our integrationtests we use the jetty servlet container that ships with maven-jetty-plugin version 6.1.22.

     
  • my collegue has just managed to reproduce the problem. I'll post the result tomorrow.

     
  • Olli
    Olli
    2012-04-25

    One problem seems to be the concurrent access to the java.util.HashMap in the class TypeTransformerMap. The 'put' operation triggers a resize and redistribution of the internal data structure. I figured out in the debugger that the size() of the HashMap is 20 but the internal data array only has 8 transformers stored.
    I have fixed the problem by inheriting the TypeTransformerMap from ConcurrentHashMap and added a NullTransformer field for null.

    public class TypeTransformerMap extends ConcurrentHashMap<Class, Transformer>
    
      @Override
      public Transformer put(Class key, Transformer value) {
        if (key == null) {
          nullTransformer = value;
          return nullTransformer;
        }
        return super.put(key, value);
      }
      @Override
      public Transformer get(Object key) {
        if (key == null) {
          return nullTransformer;
        }
        return super.get(key);
      }
    [code]
    Olli
    
     
  • Hello,
    I thought the Thread-safe initialization of Serializer (see the code in my last  reply: The businessclassSerializer ist an anonymous variable) would have fixed our issue. But another problem has risen: At some serializations the ObjectTransformer was not found anymore.

    I think, it's a Threading issue, too:
    You can have a look on the TransformerUtil class, line 82:

    Collections.unmodifiableMap(defaultTransformers);
    

    This statement does not change the defaultTransformers object, but returned an unmodifiable map that was not assigned to anything.
    I have patched the TransformerUtil class in order to find out if defaultTransformers were modified:

    package flexjson;
    import flexjson.transformer.ArrayTransformer;
    import flexjson.transformer.BasicDateTransformer;
    import flexjson.transformer.BooleanTransformer;
    import flexjson.transformer.CharacterTransformer;
    import flexjson.transformer.ClassTransformer;
    import flexjson.transformer.EnumTransformer;
    import flexjson.transformer.HibernateTransformer;
    import flexjson.transformer.IterableTransformer;
    import flexjson.transformer.MapTransformer;
    import flexjson.transformer.NullTransformer;
    import flexjson.transformer.NumberTransformer;
    import flexjson.transformer.ObjectTransformer;
    import flexjson.transformer.StringTransformer;
    import flexjson.transformer.Transformer;
    import flexjson.transformer.TransformerWrapper;
    import flexjson.transformer.TypeTransformerMap;
    import java.math.BigDecimal;
    import java.math.BigInteger;
    import java.util.Arrays;
    import java.util.Date;
    import java.util.Map;
    public class TransformerUtil {
      private static boolean initialized = false;
      private static final TypeTransformerMap defaultTransformers = new TypeTransformerMap() {
        @Override
        public Transformer put(Class key, Transformer value) {
          if (initialized) {
            throw new UnsupportedOperationException("read only");
          }
          else {
            return super.put(key, value);
          }
        }
      };
      static {
        // define all standard type transformers
        Transformer transformer = new NullTransformer();
        defaultTransformers.put(null, new TransformerWrapper(transformer));
        transformer = new ObjectTransformer();
        defaultTransformers.put(Object.class, new TransformerWrapper(transformer));
        transformer = new ClassTransformer();
        defaultTransformers.put(Class.class, new TransformerWrapper(transformer));
        transformer = new BooleanTransformer();
        defaultTransformers.put(boolean.class, new TransformerWrapper(transformer));
        defaultTransformers.put(Boolean.class, new TransformerWrapper(transformer));
        transformer = new NumberTransformer();
        defaultTransformers.put(Number.class, new TransformerWrapper(transformer));
        defaultTransformers.put(Integer.class, new TransformerWrapper(transformer));
        defaultTransformers.put(int.class, new TransformerWrapper(transformer));
        defaultTransformers.put(Long.class, new TransformerWrapper(transformer));
        defaultTransformers.put(long.class, new TransformerWrapper(transformer));
        defaultTransformers.put(Double.class, new TransformerWrapper(transformer));
        defaultTransformers.put(double.class, new TransformerWrapper(transformer));
        defaultTransformers.put(Float.class, new TransformerWrapper(transformer));
        defaultTransformers.put(float.class, new TransformerWrapper(transformer));
        defaultTransformers.put(BigDecimal.class, new TransformerWrapper(transformer));
        defaultTransformers.put(BigInteger.class, new TransformerWrapper(transformer));
        transformer = new StringTransformer();
        defaultTransformers.put(String.class, new TransformerWrapper(transformer));
        transformer = new CharacterTransformer();
        defaultTransformers.put(Character.class, new TransformerWrapper(transformer));
        defaultTransformers.put(char.class, new TransformerWrapper(transformer));
        transformer = new BasicDateTransformer();
        defaultTransformers.put(Date.class, new TransformerWrapper(transformer));
        transformer = new EnumTransformer();
        defaultTransformers.put(Enum.class, new TransformerWrapper(transformer));
        transformer = new IterableTransformer();
        defaultTransformers.put(Iterable.class, new TransformerWrapper(transformer));
        transformer = new MapTransformer();
        defaultTransformers.put(Map.class, new TransformerWrapper(transformer));
        transformer = new NullTransformer();
        defaultTransformers.put(void.class, new TransformerWrapper(transformer));
        transformer = new ArrayTransformer();
        defaultTransformers.put(Arrays.class, new TransformerWrapper(transformer));
        try {
          Class hibernateProxy = Class.forName("org.hibernate.proxy.HibernateProxy");
          defaultTransformers.put(hibernateProxy, new TransformerWrapper(new HibernateTransformer()));
        }
        catch (ClassNotFoundException ex) {
          // no hibernate so ignore.
        }
        initialized = true;
      }
      public static TypeTransformerMap getDefaultTypeTransformers() {
        return defaultTransformers;
      }
    }
    

    Then I have run my unittests. Everthing failed. Obviously the serialization tries to modify the defaultTransformers.
    My Quickfix:
    Replace the statement

    throw new UnsupportedOperationException("read only");
    

    with

    return value;
    

    Now my code runs again, but I can't use the caching of Transformers.

    Greetings, Joerg

     
  • sdismo
    sdismo
    2012-06-27

    Hi,

    We have come across the same problem. Sometimes arbitrary custom transformers just disappear. 

    Our analysis:

    The transformer caching mechanism is broken with respect to thread safety. This is what olli-o points out above. New entries are added via TypeTransformerMap.updateTransformers. A new entry is added when you, for instance, encounter an object of a specific class for the first time. If the same instance of JSONSerializer is used by multiple threads you will sooner or later have two threads adding entries at the same time. As TypeTransformerMap extends HashMap there is no protection against corruption from simultaneous update from two threads.

    The most common type of corruption happens when a HashMap needs to be extended and re-balanced. What usually happens is that some key-value pairs are lost. If the lost transformers are part of the default set then they are found there on next invocation and the caching map will “self heal”. A custom transformer could however be lost forever (i.e. until you restart the application).

    The fix from olli-o seems to resolve our problem. As with all thread safety problems we need to exercise our application for quite some time to be completely sure.

    The other fix suggested by joergpfruender is a bit more brutal as it disables the transformer cache. It should work but we have not tested it. This fix points however on another disturbing thought. The default set of transformers is not immutable. Wrapping via Collections does not happen as the author intended. The first version of this fix suggests that the default set is actually manipulated, and could thus be subject to corruption as described above. Such a corruption would be fatal.

    As far as we can see there are three problems here.

    1. The TypeTransformerMap is a MashMap and is thus not thread safe.

    2. The default set of transformers are not immutable as intended.

    3. The caching of transformers appears to leak into the (assumed immutable) default set of transformers. This is clearly not the intention of the design but there is no known problems from this.

    We have no simple unit test style code to verify the above problems. How shall we proceed? Shall we file a bug report?

    Regards
    Simon

     
  • Brandon Goodin
    Brandon Goodin
    2012-06-27

    Greetings All,

    I have taken note of this thread and am looking into proving out the assertions (which I believe are correct) with unit tests.

    Charlie, I apologize for disappearing for some time on this. If you are already at work with a fix on this please let me know. Otherwise, I will continue with proving out the unit tests and subsequently providing a fix.

    Much thanks,
    Brandon

     
  • Has anyone written a unit test that demonstrates the problem?  Can you share that?  I just want to make sure whatever we do fixes the issues people have.

     
  • Here's the patch I have created:

    Index: src/test/java/flexjson/transformer/TypeTransformerMapTest.java
    ===================================================================
    --- src/test/java/flexjson/transformer/TypeTransformerMapTest.java  (revision )
    +++ src/test/java/flexjson/transformer/TypeTransformerMapTest.java  (revision )
    @@ -0,0 +1,27 @@
    +package flexjson.transformer;
    +
    +import flexjson.TransformerUtil;
    +import org.junit.Test;
    +
    +import java.util.Map;
    +
    +public class TypeTransformerMapTest {
    +
    +  @Test
    +  public void testGetTransformer_will_not_modify_the_parentTransformerMap() throws Exception {
    +
    +    Map<Class,Transformer> defaultTypeTransformers = TransformerUtil.getDefaultTypeTransformers();
    +    int sizeBeforeTransformation = defaultTypeTransformers.size();
    +    TypeTransformerMap typeTransformerMap = new TypeTransformerMap(defaultTypeTransformers);
    +
    +    typeTransformerMap.getTransformer(EnumToSerialize.OPTION_1);
    +    typeTransformerMap.getTransformer(new BeanToSerialize());
    +
    +    int sizeAfterTransformation = typeTransformerMap.parentTransformerMap.size();
    +
    +    org.junit.Assert.assertEquals(sizeBeforeTransformation, sizeAfterTransformation);
    +  }
    +
    +
    +
    +}
    Index: src/main/java/flexjson/transformer/TypeTransformerMap.java
    ===================================================================
    --- src/main/java/flexjson/transformer/TypeTransformerMap.java  (revision 118)
    +++ src/main/java/flexjson/transformer/TypeTransformerMap.java  (revision )
    @@ -17,76 +17,84 @@
    
     import java.util.Arrays;
     import java.util.HashMap;
    +import java.util.Map;
    
     /**
    - * This class is used to lookup type transformers from specific to generic implementation.
    - * For example if an ArrayList transformer is provided
    + * This class is used to lookup type transformers from specific to generic implementation. For example if an ArrayList transformer
    + * is provided
      */
     public class TypeTransformerMap extends HashMap<Class, Transformer> {
    
    -    private TypeTransformerMap parentTransformerMap;
    +  TypeTransformerMap parentTransformerMap;
    
    -    public TypeTransformerMap() {
    -    }
    +  public TypeTransformerMap() {
    +  }
    
    -    public TypeTransformerMap(TypeTransformerMap parentTransformerMap) {
    -        this.parentTransformerMap = parentTransformerMap;
    +  public TypeTransformerMap(Map<Class, Transformer> defaultTransformers) {
    +    this.parentTransformerMap = new TypeTransformerMap() {
    +      @Override
    +      protected Transformer updateTransformers(Class key, Transformer transformer) {
    +        return transformer; // parent transformer Map should not be updated
    -    }
    +      }
    +    };
    +    parentTransformerMap.putAll(defaultTransformers);
    +  }
    
    -    @SuppressWarnings("unchecked")
    -    public Transformer getTransformer(Object key) {
    -        Transformer transformer = findTransformer(key == null ? void.class : key.getClass(), key == null ? void.class : key.getClass());
    -        if (transformer == null && parentTransformerMap != null) {
    -            // if no transformers found in child then check parent
    -            transformer = parentTransformerMap.getTransformer(key);
    +  @SuppressWarnings("unchecked")
    +  public Transformer getTransformer(Object key) {
    +    Transformer transformer = findTransformer(key == null ? void.class : key.getClass(), key == null ? void.class : key.getClass());
    +    if (transformer == null && parentTransformerMap != null) {
    +      // if no transformers found in child then check parent
    +      transformer = parentTransformerMap.getTransformer(key);
    -            if(transformer != null) {
    +      if (transformer != null) {
    -                updateTransformers(key == null ? void.class : key.getClass(), transformer);
    -            }
    -        }
    -        return transformer;
    -    }
    +        updateTransformers(key == null ? void.class : key.getClass(), transformer);
    +      }
    +    }
    +    return transformer;
    +  }
    
    -    Transformer findTransformer(Class key, Class originalKey) {
    +  Transformer findTransformer(Class key, Class originalKey) {
    
    -        if(key == null) return null;
    +    if (key == null) { return null; }
    
    -        // if specific type found
    -        if (containsKey(key)) {
    +    // if specific type found
    +    if (containsKey(key)) {
    -            if(key != originalKey) {
    +      if (key != originalKey) {
    -                return updateTransformers(originalKey, get(key));
    +        return updateTransformers(originalKey, get(key));
    -            } else {
    +      }
    +      else {
    -                return get(key);
    -            }
    -        }
    +        return get(key);
    +      }
    +    }
    
    -        // handle arrays specially if no specific array type handler
    -        // Arrays.class is used for this because it would never appear
    -        // in an object that needs to be serialized.
    -        if (key.isArray()) {
    -            return updateTransformers(originalKey, get(Arrays.class));
    -        }
    +    // handle arrays specially if no specific array type handler
    +    // Arrays.class is used for this because it would never appear
    +    // in an object that needs to be serialized.
    +    if (key.isArray()) {
    +      return updateTransformers(originalKey, get(Arrays.class));
    +    }
    
    -        // check for interface transformer
    -        for (Class interfaze : key.getInterfaces()) {
    +    // check for interface transformer
    +    for (Class interfaze : key.getInterfaces()) {
    
    -            if (containsKey(interfaze)) {
    -                return updateTransformers(originalKey, get(interfaze));
    +      if (containsKey(interfaze)) {
    +        return updateTransformers(originalKey, get(interfaze));
    -            } else {
    +      }
    +      else {
    -                Transformer t = findTransformer( interfaze, originalKey );
    +        Transformer t = findTransformer(interfaze, originalKey);
    -                if( t != null ) return t;
    +        if (t != null) { return t; }
    -            }
    -        }
    +      }
    +    }
    
    -        // if no interface transformers then check superclass
    -        return findTransformer(key.getSuperclass(), originalKey);
    +    // if no interface transformers then check superclass
    +    return findTransformer(key.getSuperclass(), originalKey);
    -
    -    }
    +  }
    
    -    private Transformer updateTransformers(Class key, Transformer transformer) {
    +  protected Transformer updateTransformers(Class key, Transformer transformer) {
    -        // only make changes to the child TypeTransformerMap
    -        if (transformer != null) {
    -            put(key, transformer);
    -        }
    -        return transformer;
    -    }
    +    // only make changes to the child TypeTransformerMap
    +    if (transformer != null) {
    +      put(key, transformer);
    +    }
    +    return transformer;
    +  }
     }
    Index: src/main/java/flexjson/TransformerUtil.java
    ===================================================================
    --- src/main/java/flexjson/TransformerUtil.java (revision 148)
    +++ src/main/java/flexjson/TransformerUtil.java (revision )
    @@ -1,89 +1,107 @@
     package flexjson;
    
    -import flexjson.transformer.*;
    +import flexjson.transformer.ArrayTransformer;
    +import flexjson.transformer.BasicDateTransformer;
    +import flexjson.transformer.BooleanTransformer;
    +import flexjson.transformer.CharacterTransformer;
    +import flexjson.transformer.ClassTransformer;
    +import flexjson.transformer.DefaultCalendarTransformer;
    +import flexjson.transformer.EnumTransformer;
    +import flexjson.transformer.HibernateTransformer;
    +import flexjson.transformer.IterableTransformer;
    +import flexjson.transformer.MapTransformer;
    +import flexjson.transformer.NullTransformer;
    +import flexjson.transformer.NumberTransformer;
    +import flexjson.transformer.ObjectTransformer;
    +import flexjson.transformer.StringTransformer;
    +import flexjson.transformer.Transformer;
    +import flexjson.transformer.TransformerWrapper;
    
     import java.math.BigDecimal;
     import java.math.BigInteger;
    -import java.util.*;
    +import java.util.Arrays;
    +import java.util.Calendar;
    +import java.util.Collections;
    +import java.util.Date;
    +import java.util.HashMap;
    +import java.util.Map;
    +import java.util.concurrent.ConcurrentHashMap;
    
     public class TransformerUtil {
    
    -    private static final TypeTransformerMap defaultTransformers = new TypeTransformerMap();
    +  private static final Map<Class, Transformer> defaultTransformers = new HashMap<Class, Transformer>();
    
    -    static {
    -        // define all standard type transformers
    -        Transformer transformer = new NullTransformer();
    -        defaultTransformers.put(null, new TransformerWrapper(transformer));
    +  static {
    +    // define all standard type transformers
    +    Transformer transformer = new NullTransformer();
    +    defaultTransformers.put(null, new TransformerWrapper(transformer));
    
    -        transformer = new ObjectTransformer();
    -        defaultTransformers.put(Object.class, new TransformerWrapper(transformer));
    +    transformer = new ObjectTransformer();
    +    defaultTransformers.put(Object.class, new TransformerWrapper(transformer));
    
    -        transformer = new ClassTransformer();
    -        defaultTransformers.put(Class.class, new TransformerWrapper(transformer));
    +    transformer = new ClassTransformer();
    +    defaultTransformers.put(Class.class, new TransformerWrapper(transformer));
    
    -        transformer = new BooleanTransformer();
    -        defaultTransformers.put(boolean.class, new TransformerWrapper(transformer));
    -        defaultTransformers.put(Boolean.class, new TransformerWrapper(transformer));
    +    transformer = new BooleanTransformer();
    +    defaultTransformers.put(boolean.class, new TransformerWrapper(transformer));
    +    defaultTransformers.put(Boolean.class, new TransformerWrapper(transformer));
    
    -        transformer = new NumberTransformer();
    -        defaultTransformers.put(Number.class, new TransformerWrapper(transformer));
    +    transformer = new NumberTransformer();
    +    defaultTransformers.put(Number.class, new TransformerWrapper(transformer));
    
    -        defaultTransformers.put(Integer.class, new TransformerWrapper(transformer));
    -        defaultTransformers.put(int.class, new TransformerWrapper(transformer));
    +    defaultTransformers.put(Integer.class, new TransformerWrapper(transformer));
    +    defaultTransformers.put(int.class, new TransformerWrapper(transformer));
    
    -        defaultTransformers.put(Long.class, new TransformerWrapper(transformer));
    -        defaultTransformers.put(long.class, new TransformerWrapper(transformer));
    +    defaultTransformers.put(Long.class, new TransformerWrapper(transformer));
    +    defaultTransformers.put(long.class, new TransformerWrapper(transformer));
    
    -        defaultTransformers.put(Double.class, new TransformerWrapper(transformer));
    -        defaultTransformers.put(double.class, new TransformerWrapper(transformer));
    +    defaultTransformers.put(Double.class, new TransformerWrapper(transformer));
    +    defaultTransformers.put(double.class, new TransformerWrapper(transformer));
    
    -        defaultTransformers.put(Float.class, new TransformerWrapper(transformer));
    -        defaultTransformers.put(float.class, new TransformerWrapper(transformer));
    +    defaultTransformers.put(Float.class, new TransformerWrapper(transformer));
    +    defaultTransformers.put(float.class, new TransformerWrapper(transformer));
    
    -        defaultTransformers.put(BigDecimal.class, new TransformerWrapper(transformer));
    -        defaultTransformers.put(BigInteger.class, new TransformerWrapper(transformer));
    +    defaultTransformers.put(BigDecimal.class, new TransformerWrapper(transformer));
    +    defaultTransformers.put(BigInteger.class, new TransformerWrapper(transformer));
    
    -        transformer = new StringTransformer();
    -        defaultTransformers.put(String.class, new TransformerWrapper(transformer));
    +    transformer = new StringTransformer();
    +    defaultTransformers.put(String.class, new TransformerWrapper(transformer));
    
    -        transformer = new CharacterTransformer();
    -        defaultTransformers.put(Character.class, new TransformerWrapper(transformer));
    -        defaultTransformers.put(char.class, new TransformerWrapper(transformer));
    +    transformer = new CharacterTransformer();
    +    defaultTransformers.put(Character.class, new TransformerWrapper(transformer));
    +    defaultTransformers.put(char.class, new TransformerWrapper(transformer));
    
    -        transformer = new BasicDateTransformer();
    -        defaultTransformers.put(Date.class, new TransformerWrapper(transformer));
    +    transformer = new BasicDateTransformer();
    +    defaultTransformers.put(Date.class, new TransformerWrapper(transformer));
    
    -        transformer = new DefaultCalendarTransformer();
    -        defaultTransformers.put(Calendar.class, new TransformerWrapper(transformer));
    +    transformer = new DefaultCalendarTransformer();
    +    defaultTransformers.put(Calendar.class, new TransformerWrapper(transformer));
    
    -        transformer = new EnumTransformer();
    -        defaultTransformers.put(Enum.class, new TransformerWrapper(transformer));
    +    transformer = new EnumTransformer();
    +    defaultTransformers.put(Enum.class, new TransformerWrapper(transformer));
    
    -        transformer = new IterableTransformer();
    -        defaultTransformers.put(Iterable.class, new TransformerWrapper(transformer));
    +    transformer = new IterableTransformer();
    +    defaultTransformers.put(Iterable.class, new TransformerWrapper(transformer));
    
    -        transformer = new MapTransformer();
    -        defaultTransformers.put(Map.class, new TransformerWrapper(transformer));
    +    transformer = new MapTransformer();
    +    defaultTransformers.put(Map.class, new TransformerWrapper(transformer));
    
    -        transformer = new NullTransformer();
    -        defaultTransformers.put(void.class, new TransformerWrapper(transformer));
    +    transformer = new NullTransformer();
    +    defaultTransformers.put(void.class, new TransformerWrapper(transformer));
    
    -        transformer = new ArrayTransformer();
    -        defaultTransformers.put(Arrays.class, new TransformerWrapper(transformer));
    +    transformer = new ArrayTransformer();
    +    defaultTransformers.put(Arrays.class, new TransformerWrapper(transformer));
    
    -        try {
    -            Class hibernateProxy = Class.forName("org.hibernate.proxy.HibernateProxy");
    -            defaultTransformers.put(hibernateProxy, new TransformerWrapper(new HibernateTransformer()));
    +    try {
    +      Class hibernateProxy = Class.forName("org.hibernate.proxy.HibernateProxy");
    +      defaultTransformers.put(hibernateProxy, new TransformerWrapper(new HibernateTransformer()));
    -        } catch (ClassNotFoundException ex) {
    +    }
    +    catch (ClassNotFoundException ex) {
    -            // no hibernate so ignore.
    -        }
    +      // no hibernate so ignore.
    +    }
    -
    -
    -        Collections.unmodifiableMap(defaultTransformers);
    -    }
    +  }
    
    -    public static TypeTransformerMap getDefaultTypeTransformers() {
    -        return defaultTransformers;
    +  public static Map<Class, Transformer> getDefaultTypeTransformers() {
    +    return Collections.unmodifiableMap(defaultTransformers);
    -    }
    +  }
    -
     }
    Index: src/test/java/flexjson/transformer/BeanToSerialize.java
    ===================================================================
    --- src/test/java/flexjson/transformer/BeanToSerialize.java (revision )
    +++ src/test/java/flexjson/transformer/BeanToSerialize.java (revision )
    @@ -0,0 +1,34 @@
    +package flexjson.transformer;
    +
    +import java.io.Serializable;
    +
    +public class BeanToSerialize implements Serializable{
    +
    +  private String string;
    +  private Integer integer;
    +  private EnumToSerialize enumToSerialize;
    +
    +  public String getString() {
    +    return string;
    +  }
    +
    +  public void setString(String string) {
    +    this.string = string;
    +  }
    +
    +  public Integer getInteger() {
    +    return integer;
    +  }
    +
    +  public void setInteger(Integer integer) {
    +    this.integer = integer;
    +  }
    +
    +  public EnumToSerialize getEnumToSerialize() {
    +    return enumToSerialize;
    +  }
    +
    +  public void setEnumToSerialize(EnumToSerialize enumToSerialize) {
    +    this.enumToSerialize = enumToSerialize;
    +  }
    +}
    Index: src/test/java/flexjson/transformer/EnumToSerialize.java
    ===================================================================
    --- src/test/java/flexjson/transformer/EnumToSerialize.java (revision )
    +++ src/test/java/flexjson/transformer/EnumToSerialize.java (revision )
    @@ -0,0 +1,9 @@
    +package flexjson.transformer;
    +
    +import java.io.Serializable;
    +
    +public enum EnumToSerialize implements Serializable {
    +
    +  OPTION_1,
    +  OPTION_2
    +}
    Index: src/test/java/flexjson/TransformerUtilTest.java
    ===================================================================
    --- src/test/java/flexjson/TransformerUtilTest.java (revision )
    +++ src/test/java/flexjson/TransformerUtilTest.java (revision )
    @@ -0,0 +1,24 @@
    +package flexjson;
    +
    +import flexjson.transformer.Transformer;
    +import org.junit.Test;
    +
    +public class TransformerUtilTest {
    +
    +  @Test
    +  public void testGetDefaultTypeTransformers_will_return_an_immutable_map() throws Exception {
    +    Transformer dummyTransformer = new Transformer() {
    +      public void transform(Object object) {
    +
    +      }
    +    };
    +    Class dummyClass = getClass();
    +    try {
    +    TransformerUtil.getDefaultTypeTransformers().put(dummyClass, dummyTransformer);
    +      org.junit.Assert.fail("DefaultTypeTransformers should not be modifiable");
    +    }
    +    catch (Exception expected) {
    +
    +    }
    +  }
    +}
    
     
  • sdismo
    sdismo
    2012-06-29

    Hi,

    Here is a unit test created to catch corruption of the TypeTransformerMap.

    First a simple wrapper class (as a separate class as there seems to be issues with deserializing inner classes).

    package flexjson;
    import java.util.UUID;
    public class Wrapper {
        private UUID uuid = UUID.randomUUID();
        private Object object;
        public Wrapper() {}
        public UUID getUuid() { return uuid; }
        public void setUuid(UUID uuid) { this.uuid = uuid; }
        public Object getObject() { return object; }
        public void setObject(Object object) { this.object = object; }
    }
    

    Then the test class itself. We use JUnit as our test framework.

    package flexjson;
    import flexjson.transformer.AbstractTransformer;
    import junit.framework.TestCase;
    import java.lang.reflect.Type;
    import java.util.ArrayList;
    import java.util.List;
    import java.util.Map;
    import java.util.UUID;
    import java.util.concurrent.Callable;
    import java.util.concurrent.ExecutorService;
    import java.util.concurrent.Executors;
    import java.util.concurrent.Future;
    import java.util.concurrent.atomic.AtomicInteger;
    public class TestTypeTransformerMap extends TestCase {
        private AtomicInteger roundRobinCounter = new AtomicInteger();
        private String[] classes = {
                "java.lang.ArithmeticException",
                "java.lang.ArrayIndexOutOfBoundsException",
                "java.lang.ArrayStoreException",
                "java.lang.ClassCastException",
                "java.lang.ClassNotFoundException",
                "java.lang.CloneNotSupportedException",
                "java.lang.Exception",
                "java.lang.IllegalAccessException",
                "java.lang.IllegalArgumentException",
                "java.lang.IllegalMonitorStateException",
                "java.lang.IllegalStateException",
                "java.lang.IllegalThreadStateException",
                "java.lang.IndexOutOfBoundsException",
                "java.lang.InstantiationException",
                "java.lang.InterruptedException",
                "java.lang.NegativeArraySizeException",
                "java.lang.NoSuchFieldException",
                "java.lang.NoSuchMethodException",
                "java.lang.NullPointerException",
                "java.lang.NumberFormatException",
                "java.lang.RuntimeException",
                "java.lang.SecurityException",
                "java.lang.StringIndexOutOfBoundsException",
                "java.lang.UnsupportedOperationException",
                "java.io.CharConversionException",
                "java.io.EOFException",
                "java.io.FileNotFoundException",
                "java.io.InterruptedIOException",
                "java.io.IOException",
                "java.io.NotActiveException",
                "java.io.NotSerializableException",
                "java.io.StreamCorruptedException",
                "java.io.UnsupportedEncodingException",
                "java.io.UTFDataFormatException",
                "java.net.BindException",
                "java.net.ConnectException",
                "java.net.MalformedURLException",
                "java.net.NoRouteToHostException",
                "java.net.PortUnreachableException",
                "java.net.ProtocolException",
                "java.net.SocketException",
                "java.net.SocketTimeoutException",
                "java.net.UnknownHostException",
                "java.net.UnknownServiceException",
                "java.util.ConcurrentModificationException",
                "java.util.EmptyStackException",
                "java.util.FormatterClosedException",
                "java.util.InputMismatchException",
                "java.util.NoSuchElementException",
                "java.util.TooManyListenersException",
                "java.util.concurrent.BrokenBarrierException",
                "java.util.concurrent.CancellationException",
                "java.util.concurrent.RejectedExecutionException",
                "java.util.concurrent.TimeoutException",
        };
        private class UUIDTransformer extends AbstractTransformer {
            public void transform(Object object) {
                if (object instanceof UUID) {
                    UUID uuid = (UUID)object;
                    getContext().writeOpenObject();
                    getContext().writeName("class");
                    getContext().writeQuoted(object.getClass().getName());
                    getContext().writeComma();
                    getContext().writeName("UUID");
                    getContext().writeQuoted(uuid.toString());
                    getContext().writeCloseObject();
                } else {
                    throw new RuntimeException("Unexpected type of object: " + object.getClass());
                }
            }
        }
        private class UUIDFactory implements ObjectFactory {
            public Object instantiate(ObjectBinder context, Object value, Type targetType, Class targetClass) {
                if (value instanceof Map) {
                    Map map = (Map)value;
                    String uuidString;
                    Object uuid = map.get("UUID");
                    if (uuid != null) {
                        if (uuid instanceof String) {
                            uuidString = (String)uuid;
                        } else {
                            throw new RuntimeException("Unexpected type of value in Map: " + uuid.getClass());
                        }
                        return UUID.fromString(uuidString);
                    } else {
                        throw new RuntimeException("This is how we see the problem: a UUID without UUID field");     // put breakpoint here
                    }
                } else {
                    throw new RuntimeException("Unexpected type of value: " + value.getClass());
                }
            }
        }
        private class StackTraceElementFactory implements ObjectFactory {
            public Object instantiate(ObjectBinder context, Object value, Type targetType, Class targetClass) {
                if (value instanceof Map) {
                    Map map = (Map)value;
                    String className;
                    Object obj = map.get("className");
                    if (obj instanceof String) {
                        className = (String)obj;
                    } else {
                        throw new RuntimeException("Unexpected type of value in Map (className): " + obj.getClass());
                    }
                    String methodName;
                    obj = map.get("methodName");
                    if (obj instanceof String) {
                        methodName = (String)obj;
                    } else {
                        throw new RuntimeException("Unexpected type of value in Map (methodName): " + obj.getClass());
                    }
                    String fileName = null;
                    obj = map.get("fileName");
                    if (obj != null) {
                        if (obj instanceof String) {
                            fileName = (String)obj;
                        } else {
                            throw new RuntimeException("Unexpected type of value in Map (fileName): " + obj.getClass());
                        }
                    }
                    Integer lineNumber;
                    obj = map.get("lineNumber");
                    if (obj instanceof Integer) {
                        lineNumber = (Integer)obj;
                    } else {
                        throw new RuntimeException("Unexpected type of value in Map (lineNumber): " + obj.getClass());
                    }
                    return new StackTraceElement(className, methodName, fileName, lineNumber);
                } else {
                    throw new RuntimeException("Unexpected type of value: " + value.getClass());
                }
            }
        }
        private class Worker implements Callable<Boolean> {
            private JSONSerializer serializer;
            private JSONDeserializer deSerializer;
            private Worker(JSONSerializer serializer, JSONDeserializer deSerializer) {
                this.serializer = serializer;
                this.deSerializer = deSerializer;
            }
            public Boolean call() throws Exception {
                Wrapper wrapper = new Wrapper();
                UUID uuid = wrapper.getUuid();
                wrapper.setObject(getObject());
                String jsonString = serializer.deepSerialize(wrapper);
                try {
                    Object object = deSerializer.deserialize(jsonString);
                    return ((Wrapper)object).getUuid().equals(uuid);
                } catch (RuntimeException ex) {
                    // System.out.println(jsonString);
                    // ex.printStackTrace();
                    return false;
                }
            }
        }
        private Object getObject() throws Exception {
            int selector = Math.abs(roundRobinCounter.getAndIncrement() % classes.length);
            Class aClass = Class.forName(classes[selector]);
            Wrapper wrapper = new Wrapper();
            wrapper.setObject(aClass.newInstance());
            return wrapper;
        }
        public void testForTransformerCacheBug() throws Exception {
            ExecutorService exec = Executors.newFixedThreadPool(16);
            for (int i=0; i<1024; i++) {
                JSONSerializer serializer = new JSONSerializer();
                JSONDeserializer deSerializer = new JSONDeserializer();
                serializer.transform(new UUIDTransformer(), UUID.class);
                deSerializer.use(UUID.class, new UUIDFactory());
                deSerializer.use(StackTraceElement.class, new StackTraceElementFactory());
                List<Worker> workList = new ArrayList<Worker>(classes.length);
                for (String aClass : classes) {
                    workList.add(new Worker(serializer, deSerializer));
                }
                List<Future<Boolean>> futureList = exec.invokeAll(workList);
                for (Future<Boolean> future : futureList) {
                    if (!future.get().equals(Boolean.TRUE)) {
                        fail("UUID failed round trip test (i=" + i + ')');
                    }
                }
            }
            exec.shutdown();
        }
    }
    

    This test case is fairly close to our real case. I use a number of standard java exception classes instead of our own. This test case fails all the time on my dual-core developer machine, usually within the first 100 loops.

    You may want to use a debugger and set a breakpoint on the line marked above. From there you can examine the TypeTransformerMap and see that the transformer for class UUID has been replaced with a standard ObjectTransformer.

    Hope this helps.
    Best Regards
    Simon

     
  • sdismo
    sdismo
    2012-06-29

    Perhaps worth to add that the above test case have not yet failed with the fix from olli-o applied.

     
  • Brandon Goodin
    Brandon Goodin
    2012-08-12

    Hey Everyone,

    Just wanted to drop a note that there is a fix committed to SVN on trunk for this. sdismo, I appreciate the effort to put the unit test in place. But, I was not able to get it to work and was not able to generate a unit test that would reproduce your issue. Would you be willing to check out the trunk code and give it a try to see if it fixes your issue? Before we release this into the wild I want to make sure it is addressing the issue identified.

    Three issues I addressed with this:
    #1 Prevent the default TypeTransformerMap from being modified. I placed the ability to lock the parent TypeTransformerMap.
    #2 Address issues resulting from concurrent access and modification to the underlying map. We now extend the ConcurrentHashMap

     
  • sdismo
    sdismo
    2012-08-13

    Great! I am more than happy to do some testing. It will take a few days before I have any news. Thanks for looking into this issue.

     
  • sdismo
    sdismo
    2012-08-17

    I have taken svn revision 151 of your repository. The unit test above passes (the TC needs a minor fix for new class JsonNumber).

    Next step is to push this change set into the soak test (this is where we originally found the problem). It will stay in the soak during next week.

    So far, so good! 

     
  • sdismo
    sdismo
    2012-08-17

    It seems like there is other bug in svn version 151. This bug prevents us from even passing the gate into the soak.

    Something is wrong with deserialization with respect to properly populating fields in super class. Things look fine on the wire, but none of the super class fields are populated after deserialization.

    I guess that I should consider 151 as a development snapshot.

     
  • Brandon Goodin
    Brandon Goodin
    2012-08-17

    sdismo, thanks for taking the time to test this out. It is much appreciated.

    Yes, anything that is built off of trunk should be considered a development snapshot.

    I'll take a look at the deserialization issue. Charlie is more familiar with deserialization and it is more likely he will be the one that addresses it.

     
  • Sdismo,

    Would you be willing to share a unit test that is failing during deserialization so we can add that to our test to make sure we covered the problem for now and going forward.

    Thanks
    Charlie

     
  • sdismo
    sdismo
    2012-08-17

    Hi Charlie,

    I'll try to extract a simple unit test that illustrates this new problem. The cases I have already requires way too much of our application to be comprehensive. Hope to get that done early next week.

    Best Regards
    Simon

     
  • sdismo
    sdismo
    2012-08-20

    I may have jumped to conclusion in post no. 16 above.

    Something has changed in how deserialization populates fields. This change is not necessarily a bug. It seems like one of our base classes have managed to get away without proper setter methods for all fields. I guess that the current release of FlexJSON manages to sort that out by matching field names, and that the development snapshot does not.

    I consider the missing setter a bug in our code. Falling back to a secondary path at deserialization does also most likely have a (small) performance impact. From our point of view the changed behavior is good as it brings out mistakes in our code. 
        
    I have just pushed a new change set into soak test. I'll keep you posted on the progress.

    Best Regards
    Simon
      

     
  • sdismo
    sdismo
    2012-08-28

    Hi,

    A week of soak testing is concluded with somewhat mixed results.

    1. We have no functional issues with svn branch 151. The problems described earlier in this thread are no longer present.

    2. We have performance issues with svn branch 151. This has most likely nothing to do with the resolution of the problems described in this thread. I have some feedback on what is causing performance degradation for our application. I do not think that this thread is the appropriate place to continue such a discussion. Pls advice if I should open a new thread or what else to to.

    Thanks for your effort to solve this issue.
    Best Regards
    Simon     

     
  • Brandon Goodin
    Brandon Goodin
    2012-08-28

    Hey Simon,

    Please, let's open another thread and discuss the performance issues.

    Thanks,
    Brandon