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

Enhancements for Json support. #70

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all 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
15 changes: 15 additions & 0 deletions src/one/nio/serial/Before.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
package one.nio.serial;

import java.lang.annotation.ElementType;
import java.lang.annotation.Retention;
import java.lang.annotation.RetentionPolicy;
import java.lang.annotation.Target;

/**
* Constructor calls are skipped on JSON deserialization unless annotated with this annotation.
*/
@Retention(RetentionPolicy.RUNTIME)
@Target({ElementType.CONSTRUCTOR})
public @interface Before {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is it not enough @default annotation on fields?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hm, I'll get back to you on this one.


}
21 changes: 20 additions & 1 deletion src/one/nio/serial/DateSerializer.java
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@

package one.nio.serial;

import one.nio.util.DateParser;

import java.io.IOException;
import java.util.Date;

Expand Down Expand Up @@ -54,7 +56,24 @@ public void toJson(Date obj, StringBuilder builder) {

@Override
public Date fromJson(JsonReader in) throws IOException {
return new Date(in.readLong());
String numberOrParsableText = in.readString();
numberOrParsableText = numberOrParsableText.trim();
if (numberOrParsableText.length() == 0) {
return null;
}
char ch0 = numberOrParsableText.charAt(0);
boolean isNumber = ch0 == '-' || (ch0 >= '0' && ch0 <= '9');
for (int i = 1; i < numberOrParsableText.length() && isNumber; i++) {
char ch1 = numberOrParsableText.charAt(i);
if (ch1 < '0' || ch1 > '9') {
isNumber = false;
}
}
if (isNumber) {
return new Date(Long.parseLong(numberOrParsableText));
} else {
return new Date(DateParser.parse(numberOrParsableText));
}
}

@Override
Expand Down
7 changes: 5 additions & 2 deletions src/one/nio/serial/GeneratedSerializer.java
Original file line number Diff line number Diff line change
Expand Up @@ -39,12 +39,15 @@ public class GeneratedSerializer extends Serializer {
static final AtomicInteger renamedFields = new AtomicInteger();
static final AtomicInteger unsupportedFields = new AtomicInteger();

private final boolean jsonOnlySerialization;
Copy link
Collaborator

Choose a reason for hiding this comment

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

What do you think about creating separate JsonSerializer with own configuration ?
It seems that json serialization can be extended in the fututre and it whould be to heavy for java serializer

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Great idea. I'll see what I can come up with.


private FieldDescriptor[] fds;
private FieldDescriptor[] defaultFields;
private Delegate delegate;

GeneratedSerializer(Class cls) {
GeneratedSerializer(Class cls, boolean jsonOnlySerialization) {
super(cls);
this.jsonOnlySerialization = jsonOnlySerialization;

Field[] ownFields = getSerializableFields();
this.fds = new FieldDescriptor[ownFields.length / 2];
Expand Down Expand Up @@ -108,7 +111,7 @@ public void skipExternal(ObjectInput in) throws IOException, ClassNotFoundExcept

@Override
public byte[] code() {
return DelegateGenerator.generate(cls, fds, defaultFields);
return DelegateGenerator.generate(cls, fds, defaultFields, jsonOnlySerialization);
}

@Override
Expand Down
17 changes: 17 additions & 0 deletions src/one/nio/serial/Json.java
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,11 @@

public class Json {


// https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Number/MIN_SAFE_INTEGER
public static final long JS_MIN_SAFE_INTEGER = -9007199254740991L;
public static final long JS_MAX_SAFE_INTEGER = 9007199254740991L;

public static void appendChar(StringBuilder builder, char c) {
builder.append('"');
if (c == '"' || c == '\\') {
Expand Down Expand Up @@ -70,6 +75,18 @@ public static void appendObject(StringBuilder builder, Object obj) throws IOExce
}
}

public static void appendLong(StringBuilder builder, long value) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I believe writing long as string should be configurable (as in JACKSON/GSON)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fine with me.

if (isJsSafeInteger(value)) {
builder.append(value);
} else {
builder.append('"').append(value).append('"');
}
}

public static boolean isJsSafeInteger(long value) {
return JS_MIN_SAFE_INTEGER <= value && value <= JS_MAX_SAFE_INTEGER;
}

@SuppressWarnings("unchecked")
public static String toJson(Object obj) throws IOException {
if (obj == null) {
Expand Down
80 changes: 62 additions & 18 deletions src/one/nio/serial/JsonReader.java
Original file line number Diff line number Diff line change
Expand Up @@ -74,10 +74,22 @@ public final void expect(int b, String message) throws IOException {
}

public final boolean readBoolean() throws IOException {
boolean readBooleanAsString = false;
if (next == '\"')
{
readBooleanAsString = true;
read();
}
int b = read();
if (b == 't' && read() == 'r' && read() == 'u' && read() == 'e') {
if (readBooleanAsString) {
expect('\"', "Unexpected end of string");
}
return true;
} else if (b == 'f' && read() == 'a' && read() == 'l' && read() == 's' && read() == 'e') {
if (readBooleanAsString) {
expect('\"', "Unexpected end of string");
}
return false;
}
throw exception("Expected boolean");
Expand Down Expand Up @@ -114,6 +126,13 @@ public final double readDouble() throws IOException {
public final String readNumber() throws IOException {
StringBuilder sb = new StringBuilder();

boolean readNumberAsString = false;
if (next == '"')
{
readNumberAsString = true;
read();
}

// Sign
if (next == '-') {
sb.append((char) read());
Expand Down Expand Up @@ -150,6 +169,10 @@ public final String readNumber() throws IOException {
} while (next >= '0' && next <= '9');
}

if (readNumberAsString)
{
expect('\"', "Unexpected end of string");
}
return sb.toString();
}

Expand All @@ -158,9 +181,9 @@ public final int readHexChar() throws IOException {
if (b >= '0' && b <= '9') {
return b - '0';
} else if (b >= 'A' && b <= 'F') {
return b - 'A';
return b - 'A' + 10;
} else if (b >= 'a' && b <= 'f') {
return b - 'a';
return b - 'a' + 10;
}
throw exception("Invalid escape character");
}
Expand Down Expand Up @@ -193,23 +216,44 @@ public Object readNull() throws IOException {
}

public String readString() throws IOException {
StringBuilder sb = new StringBuilder();
expect('\"', "Expected string");
while (next >= 0 && next != '\"') {
int b = read();
if ((b & 0x80) == 0) {
sb.append(b == '\\' ? readEscapeChar() : (char) b);
} else if ((b & 0xe0) == 0xc0) {
sb.append((char) ((b & 0x1f) << 6 | (read() & 0x3f)));
} else if ((b & 0xf0) == 0xe0) {
sb.append((char) ((b & 0x0f) << 12 | (read() & 0x3f) << 6 | (read() & 0x3f)));
} else {
int v = (b & 0x07) << 18 | (read() & 0x3f) << 12 | (read() & 0x3f) << 6 | (read() & 0x3f);
sb.append((char) (0xd800 | (v - 0x10000) >>> 10)).append((char) (0xdc00 | (v & 0x3ff)));
}
switch (next) {
case '\"':
read();
StringBuilder sb = new StringBuilder();
while (next >= 0 && next != '\"') {
int b = read();
if ((b & 0x80) == 0) {
sb.append(b == '\\' ? readEscapeChar() : (char) b);
} else if ((b & 0xe0) == 0xc0) {
sb.append((char) ((b & 0x1f) << 6 | (read() & 0x3f)));
} else if ((b & 0xf0) == 0xe0) {
sb.append((char) ((b & 0x0f) << 12 | (read() & 0x3f) << 6 | (read() & 0x3f)));
} else {
int v = (b & 0x07) << 18 | (read() & 0x3f) << 12 | (read() & 0x3f) << 6 | (read() & 0x3f);
sb.append((char) (0xd800 | (v - 0x10000) >>> 10)).append((char) (0xdc00 | (v & 0x3ff)));
}
}
expect('\"', "Unexpected end of string");
return sb.toString();
case 'f':
incubos marked this conversation as resolved.
Show resolved Hide resolved
case 't':
return Boolean.toString(readBoolean());
case '0':
case '1':
case '2':
case '3':
case '4':
case '5':
case '6':
case '7':
case '8':
case '9':
case '-':
case '.':
return readNumber();
incubos marked this conversation as resolved.
Show resolved Hide resolved
default:
throw exception("Unexpected start of string");
}
expect('\"', "Unexpected end of string");
return sb.toString();
}

public byte[] readBinary() throws IOException {
Expand Down
2 changes: 1 addition & 1 deletion src/one/nio/serial/LongSerializer.java
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ public void skip(DataStream in) throws IOException {

@Override
public void toJson(Long obj, StringBuilder builder) {
builder.append(obj.longValue());
Json.appendLong(builder, obj);
}

@Override
Expand Down
9 changes: 4 additions & 5 deletions src/one/nio/serial/Repository.java
Original file line number Diff line number Diff line change
Expand Up @@ -372,14 +372,13 @@ private static Serializer generateFor(Class<?> cls) {
serializer = new CollectionSerializer(cls);
} else if (Map.class.isAssignableFrom(cls) && !hasOptions(cls, FIELD_SERIALIZATION)) {
serializer = new MapSerializer(cls);
} else if (Serializable.class.isAssignableFrom(cls)) {
if (cls.getName().startsWith("java.time.") && JavaInternals.findMethod(cls, "writeReplace") != null) {
} else {
boolean extendsSerializable = Serializable.class.isAssignableFrom(cls);
if (extendsSerializable && cls.getName().startsWith("java.time.") && JavaInternals.findMethod(cls, "writeReplace") != null) {
serializer = new JavaTimeSerializer(cls);
} else {
serializer = new GeneratedSerializer(cls);
serializer = new GeneratedSerializer(cls, !extendsSerializable);
}
} else {
serializer = new InvalidSerializer(cls);
Copy link
Member

Choose a reason for hiding this comment

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

Replacement of InvalidSerializer by GeneratedSerializer (JSON-only) looks like a serious behavior change, because GeneratedSerializer implements Serializer methods, but InvalidSerializer throws exceptions. So a user of the upstream version would get an exception if she forgot to implement Serializable, but in this version she would not.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I think as suggested by @atimofeyev maybe it makes sense to make a separete JsonSerializer with a bit different behavior. Such as not needing to implement Serializable.

}
serializer.generateUid();
} catch (Throwable e) {
Expand Down
64 changes: 58 additions & 6 deletions src/one/nio/serial/gen/DelegateGenerator.java
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
package one.nio.serial.gen;

import one.nio.gen.BytecodeGenerator;
import one.nio.serial.Before;
import one.nio.serial.Default;
import one.nio.serial.FieldDescriptor;
import one.nio.serial.JsonName;
Expand All @@ -38,6 +39,7 @@
import java.io.ObjectOutputStream;
import java.lang.invoke.MethodHandleInfo;
import java.lang.invoke.MethodType;
import java.lang.reflect.Constructor;
import java.lang.reflect.Field;
import java.lang.reflect.Method;
import java.lang.reflect.Modifier;
Expand Down Expand Up @@ -113,21 +115,30 @@ public static Delegate instantiate(Class cls, FieldDescriptor[] fds, byte[] code
}

public static Delegate instantiate(Class cls, FieldDescriptor[] fds, FieldDescriptor[] defaultFields) {
return instantiate(cls, fds, generate(cls, fds, defaultFields));
return instantiate(cls, fds, generate(cls, fds, defaultFields, false));
}

public static byte[] generate(Class cls, FieldDescriptor[] fds, FieldDescriptor[] defaultFields) {
public static byte[] generate(Class cls, FieldDescriptor[] fds, FieldDescriptor[] defaultFields, boolean jsonOnly) {
String className = "sun/reflect/Delegate" + index.getAndIncrement() + '_' + cls.getSimpleName();

ClassWriter cv = new ClassWriter(ClassWriter.COMPUTE_MAXS | ClassWriter.COMPUTE_FRAMES);
cv.visit(V1_6, ACC_PUBLIC | ACC_FINAL, className, null, MAGIC_CLASS,
new String[]{"one/nio/serial/gen/Delegate"});

generateConstructor(cv, className);
generateCalcSize(cv, cls, fds);
generateWrite(cv, cls, fds);
generateRead(cv, cls, fds, defaultFields, className);
generateSkip(cv, fds);

if (!jsonOnly) {
Copy link
Member

Choose a reason for hiding this comment

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

if with a negated condition and both branches present doesn't look idiomatic, don't you think? Can we switch the branches and eliminate negation.

generateCalcSize(cv, cls, fds);
generateWrite(cv, cls, fds);
generateRead(cv, cls, fds, defaultFields, className);
generateSkip(cv, fds);
} else {
generateThrowCalcSize(cv, cls);
generateThrowWrite(cv, cls);
generateThrowRead(cv, cls);
generateThrowSkip(cv, cls);
}

generateToJson(cv, cls, fds);
generateFromJson(cv, cls, fds, defaultFields, className);

Expand Down Expand Up @@ -360,6 +371,30 @@ private static void generateSkip(ClassVisitor cv, FieldDescriptor[] fds) {
mv.visitEnd();
}

private static void generateThrowNonSerializableMethod(ClassWriter cv, String methodName, String descriptor, Class cls) {
MethodVisitor mv = cv.visitMethod(ACC_PUBLIC | ACC_FINAL, methodName, descriptor,
null, new String[]{"java/io/NotSerializableException"});
mv.visitCode();
emitThrow(mv, "java/io/NotSerializableException", cls.getName());
mv.visitEnd();
}

private static void generateThrowCalcSize(ClassWriter cv, Class cls) {
generateThrowNonSerializableMethod(cv, "calcSize", "(Ljava/lang/Object;Lone/nio/serial/CalcSizeStream;)V", cls);
}

private static void generateThrowWrite(ClassWriter cv, Class cls) {
generateThrowNonSerializableMethod(cv, "write", "(Ljava/lang/Object;Lone/nio/serial/DataStream;)V", cls);
}

private static void generateThrowRead(ClassWriter cv, Class cls) {
generateThrowNonSerializableMethod(cv, "read", "(Lone/nio/serial/DataStream;)Ljava/lang/Object;", cls);
}

private static void generateThrowSkip(ClassWriter cv, Class cls) {
generateThrowNonSerializableMethod(cv, "skip", "(Lone/nio/serial/DataStream;)V", cls);
}

private static void generateToJson(ClassVisitor cv, Class cls, FieldDescriptor[] fds) {
MethodVisitor mv = cv.visitMethod(ACC_PUBLIC | ACC_FINAL, "toJson", "(Ljava/lang/Object;Ljava/lang/StringBuilder;)V",
null, new String[]{"java/io/IOException"});
Expand Down Expand Up @@ -401,6 +436,10 @@ private static void generateToJson(ClassVisitor cv, Class cls, FieldDescriptor[]
mv.visitMethodInsn(INVOKESTATIC, "one/nio/serial/Json", "appendChar", "(Ljava/lang/StringBuilder;C)V", false);
mv.visitVarInsn(ALOAD, 2);
break;
case Long:
mv.visitMethodInsn(INVOKESTATIC, "one/nio/serial/Json", "appendLong", "(Ljava/lang/StringBuilder;J)V", false);
mv.visitVarInsn(ALOAD, 2);
break;
default:
mv.visitMethodInsn(INVOKEVIRTUAL, "java/lang/StringBuilder", "append", srcType.appendSignature(), false);
}
Expand Down Expand Up @@ -433,6 +472,19 @@ private static void generateFromJson(ClassVisitor cv, Class cls, FieldDescriptor
// Create instance
mv.visitTypeInsn(NEW, Type.getInternalName(cls));

// support for calling constructor annotated with @Before
for (Class searchClass = cls; searchClass != null; searchClass = searchClass.getSuperclass()) {
Copy link
Member

Choose a reason for hiding this comment

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

The cycle might look tricky to a seasoned reader (I've spent a minute or two to parse the meaning), but I have no suggestions how to simplify that, unfortunately.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I am not a fan of constructor call either. Will go trough a few use cases and see if @Default would work.

Constructor constructor = JavaInternals.findConstructor(searchClass);
if (constructor != null && constructor.getAnnotation(Before.class) != null) {
if (searchClass != cls) {
throw new IllegalArgumentException("To avoid unexpected behavior it is required to add @Before to no-arg constructor in " + cls + " because parent class does the same.");
}
mv.visitInsn(DUP);
mv.visitMethodInsn(INVOKESPECIAL, Type.getInternalName(cls), "<init>", "()V", false);
break;
}
}

// Prepare a multimap (fieldHash -> fds) for lookupswitch
TreeMap<Integer, FieldDescriptor> fieldHashes = new TreeMap<>();
boolean isRecord = JavaFeatures.isRecord(cls);
Expand Down
Loading