OpenHFT / Chronicle-Wire

A Low Garbage Java Serialisation Library that supports multiple formats
http://chronicle.software
Apache License 2.0
513 stars 123 forks source link

Making typed JSON more usable in JavaScript #796

Closed RobAustin closed 9 months ago

RobAustin commented 10 months ago

Taking the way we encode this Java Class as an example :

private static class DtoWithClassReference extends SelfDescribingMarshallable {
        private Class<?> implClass;
        private boolean bool;
}

If we populate this class and output it as YAML, it could look like this:

!net.openhft.chronicle.wire.JSONWireTest$DtoWithClassReference {
  implClass: !type net.openhft.chronicle.wire.JSONWireTest,
  bool: false
}

and our JSON with my recent change is now ,without types:

{
  "implClass": {"@type":"net.openhft.chronicle.wire.JSONWireTest"},
   "bool": false
}

but with types its currently:

{
  "@net.openhft.chronicle.wire.JSONWireTest$DtoWithClassReference": {
    "implClass": { "@type": "net.openhft.chronicle.wire.JSONWireTest"   },
    "bool": false
  }
}

While this is valid JSON, nesting our data within '@net.openhft.chronicle.wire.JSONWireTest$DtoWithClassReference' to provide type information is proving to be unworkable in the client-side JavaScript code, especially with nested objects. A simpler approach, better suited for typed Java, has been proposed as follows:

{
  "@instance": "net.openhft.chronicle.wire.JSONWireTest$DtoWithClassReference",
  "implClass": {"@type":"net.openhft.chronicle.wire.JSONWireTest"},
   "bool": false
}

We are effectively adopting the same format as we would without Java types, but with the addition of an '@instance' field value to each object.

maxim-ponomarev commented 10 months ago

Sounds great, that is exactly what I wanted! Getting rid of type wrappers would make consuming JSON in JavaScript/TypeScript 1000-times easier!

maxim-ponomarev commented 10 months ago

I see this as (maybe) a new flavour of JSONWire (to keep backward compatibility)

maxim-ponomarev commented 10 months ago

@yevgenp, please, contact me for any questions.

yevgenp commented 10 months ago

I don't think it's feasible. I've tried to add @instance field and, though it works fine with a custom class like above, it creates a lot of other issues and potential huge code impact (if even possible). For example when serializing a value class (byte[], LocalDateTime, BigDecimal etc), tested in Issue327Test. It produces {"@instance":"java.time.LocalTime","17:01"} which is an invalid JSON.

I believe we should keep it as it is as at least currently it produces a valid JSON:

{
  "@net.openhft.chronicle.wire.JSONWireTest$DtoWithClassReference": {
    "implClass": { "@type": "net.openhft.chronicle.wire.JSONWireTest"   },
    "bool": false
  }
}

It would be very hard to modify the code as the current stack is limited by the inheritance and expects @type related wrappers to enclose the internal object. Adding an @instance field does not conform to the way we write/read a JSON.

maxim-ponomarev commented 9 months ago

@yevgenp thank you! I will investigate other options.