alibaba / fastjson2

🚄 FASTJSON2 is a Java JSON library with excellent performance.
Apache License 2.0
3.66k stars 475 forks source link

[BUG] Deserialization from a json file and then Serialization back to json file produces incorrect result #1685

Open hshar89 opened 1 year ago

hshar89 commented 1 year ago

Code sample -

  public static void main(String[] args) throws FileNotFoundException {
        RunTimeConfig runTimeConfig =
                JSON.parseObject(new FileReader("src/main/resources/fastjsontesting/RuntimeConfigVeryShort.json"),
                        RunTimeConfig.class);
        JSON.writeTo(new FileOutputStream("src/main/resources/dummy.json"), runTimeConfig);
    }
////////////////////////////////////////    
package daytodaycode.model;

 @Data
@SuperBuilder
@EqualsAndHashCode(callSuper = true)
@NoArgsConstructor
@AllArgsConstructor
public class RunTimeConfig extends Metadata implements Serializable {
  private static final long serialVersionUID = 1L;
  private String name;

  private List<TransformConfig> transformConfigs;

  @Override
  public void compile() {}
}

////////////////////////////////////////
package daytodaycode.model;
@SuperBuilder
@Data
@NoArgsConstructor
@AllArgsConstructor
@ToString
public abstract class Metadata implements Serializable {
  private static final long serialVersionUID = 1L;

  @EqualsAndHashCode.Exclude private OffsetDateTime lastMetadataRefreshTime;

  /** Compile the metadata * */
  public abstract void compile();
}

////////////////////////////////////////
package daytodaycode.model;

public interface TransformConfig extends DataFrameFunctionConfig {
     String getInputDataFrameName();
}

////////////////////////////////////////
package daytodaycode.model;

import lombok.AllArgsConstructor;
import lombok.Builder;
import lombok.EqualsAndHashCode;
import lombok.Getter;
import lombok.ToString;

@AllArgsConstructor
@Builder
@Getter
@ToString
@EqualsAndHashCode
public class ChangeEventTransformConfig implements TransformConfig{

    private static final long serialVersionUID = 1L;
    private final String inputDataFrameName;
    private final String outputDataFrameName;
    private final String tableName;

}
////////////////////////////////////////
package daytodaycode.model;

import com.fasterxml.jackson.annotation.JsonTypeInfo;
import com.fasterxml.jackson.annotation.JsonTypeInfo.Id;
@JsonTypeInfo(
        use = Id.CLASS,
        property = "type"
)
public interface DataFrameFunctionConfig {
    String getOutputDataFrameName();
}

RuntimeConfigVeryShort.json file

{
  "name": "test",
  "transformConfigs": [
    {
      "type": "daytodaycode.model.ChangeEventTransformConfig",
      "inputDataFrameName": "change_df",
      "outputDataFrameName": "change_df_transformed",
      "tableName": "change_table"
    }
  ]
}

Expected Output -

 {
  "name": "test",
  "transformConfigs": [
    {
      "type": "daytodaycode.model.ChangeEventTransformConfig",
      "inputDataFrameName": "change_df",
      "outputDataFrameName": "change_df_transformed",
      "tableName": "change_table"
    }
  ]
}

Actual Output -

{
  "name": "test",
  "transformConfigs": [
    {
      "inputDataFrameName": "change_df",
      "outputDataFrameName": "change_df_transformed"
    }
  ]
}

FASTJson2 version --> 2.0.37 I went through the code and found these problematic pieces, In ObjectReaderCreator class, under createObjectReader method, the following piece of code creates an instance for ObjectReaderInterface.

if (objectClass.isInterface()) {
            return new ObjectReaderInterface(
                    objectClass,
                    null,
                    null,
                    0L,
                    null,
                    null,
                    fieldReaderArray
            );
        }

There is no beanInfo.typeKey reference in the parameters. So the type information is lost. The below change adds the information from beanInfo class to the parameters.

if (objectClass.isInterface()) {
            return new ObjectReaderInterface(
                    objectClass,
                    beanInfo.typeKey,
                    null,
                    beanInfo.readerFeatures,
                    null,
                    null,
                    fieldReaderArray
            );
        }

The above is required for this condition in ObjectReaderInterface class

JSONReader.Context context = jsonReader.getContext();
            long features3, hash = jsonReader.readFieldNameHashCode();
            JSONReader.AutoTypeBeforeHandler autoTypeFilter = context.getContextAutoTypeBeforeHandler();
 if (i == 0
                    && hash == getTypeKeyHash()
                    && ((((features3 = (features | getFeatures() | context.getFeatures())) & JSONReader.Feature.SupportAutoType.mask) != 0) || autoTypeFilter != null)
            )

In ObjectReaderBaseModule.ReaderAnnotationProcessor class, there are two changes required. The first is handling interfaces and the other is for superClass in the getBeanInfo method. Below are the rough changes I have made. The superClass code piece is required to handle any annotation on the super class. Right now if I take an abstract class and add an annotation to that, it is not recognized (for example create an abstract class called AbstractDataFrameFunctionConfig and add the @JsonTypeInfo annotation to that and then try to deserialize it. Keep the hierarchy the same, just change it from interface to abstract)

 Class superClass = objectClass.getSuperclass();
            if (superClass != null && superClass != Object.class && superClass != Enum.class) {
                getBeanInfo(beanInfo, superClass);
            }
            Class<?>[] interfaces = objectClass.getInterfaces();
            if (interfaces.length > 0) {
                for (Class<?> superInterface : interfaces) {
                    getBeanInfo(beanInfo, superInterface);
                }
            }

In ObjectWriterBaseModule.WriterAnnotationProcessor getBeanInfo method added the following change

    if (objectClass != null) {
                Class superclass = objectClass.getSuperclass();
                if (superclass != Object.class && superclass != null && superclass != Enum.class) {
                    getBeanInfo(beanInfo, superclass);

                    if (beanInfo.seeAlso != null && beanInfo.seeAlsoNames != null) {
                        for (int i = 0; i < beanInfo.seeAlso.length; i++) {
                            Class seeAlso = beanInfo.seeAlso[i];
                            if (seeAlso == objectClass && i < beanInfo.seeAlsoNames.length) {
                                String seeAlsoName = beanInfo.seeAlsoNames[i];
                                if (seeAlsoName != null && seeAlsoName.length() != 0) {
                                    beanInfo.typeName = seeAlsoName;
                                    break;
                                }
                            }
                        }
                    }
                }
                Class<?>[] interfaces = objectClass.getInterfaces();
                if (interfaces.length > 0) {
                    for (Class<?> superInterface : interfaces) {
                        getBeanInfo(beanInfo, superInterface);

                        if (beanInfo.seeAlso != null && beanInfo.seeAlsoNames != null) {
                            for (int i = 0; i < beanInfo.seeAlso.length; i++) {
                                Class seeAlso = beanInfo.seeAlso[i];
                                if (seeAlso == objectClass && i < beanInfo.seeAlsoNames.length) {
                                    String seeAlsoName = beanInfo.seeAlsoNames[i];
                                    if (seeAlsoName != null && seeAlsoName.length() != 0) {
                                        beanInfo.typeName = seeAlsoName;
                                        break;
                                    }
                                }
                            }
                        }
                    }
                }
            }

With the above changes, I am able to get the expected output.

hshar89 commented 1 year ago

@wenshao Did you get a chance to take a look at this?