-
Notifications
You must be signed in to change notification settings - Fork 66
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
lib/json: unify the writing API using serialization only #2312
Conversation
+1 |
54b5aa0
to
1e54045
Compare
+1 |
1e54045
to
99d329d
Compare
Signed-off-by: Alexis Laferrière <[email protected]>
Signed-off-by: Alexis Laferrière <[email protected]>
45d8a8e
to
d02cb01
Compare
cdcfa6d
to
5a35318
Compare
The remaining tests fail because of #2296. I may have to rename the |
Signed-off-by: Alexis Laferrière <[email protected]>
Signed-off-by: Alexis Laferrière <[email protected]>
Signed-off-by: Alexis Laferrière <[email protected]>
…json Signed-off-by: Alexis Laferrière <[email protected]>
… Map Signed-off-by: Alexis Laferrière <[email protected]>
Signed-off-by: Alexis Laferrière <[email protected]>
Signed-off-by: Alexis Laferrière <[email protected]>
2be86ff
to
690e9c3
Compare
Rerolled to fix all clients by making the generated JSON closer to what generated
Because of the refactor, I'll have to do a manual merge with #2311. |
@@ -120,7 +120,7 @@ class Achievement | |||
json["reward"].as(Int)) | |||
end | |||
|
|||
redef fun to_json do | |||
redef fun to_json_object do do |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do do?
@@ -104,7 +104,7 @@ class GameEvent | |||
time = new ISODate.from_string(json["time"].as(String)) | |||
end | |||
|
|||
redef fun to_json do | |||
redef fun to_json_object do do |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do do!
Signed-off-by: Alexis Laferrière <[email protected]>
Signed-off-by: Alexis Laferrière <[email protected]>
Signed-off-by: Alexis Laferrière <[email protected]>
Signed-off-by: Alexis Laferrière <[email protected]>
Signed-off-by: Alexis Laferrière <[email protected]>
Signed-off-by: Alexis Laferrière <[email protected]>
Signed-off-by: Alexis Laferrière <[email protected]>
Signed-off-by: Alexis Laferrière <[email protected]>
690e9c3
to
d00ed38
Compare
Removed do dos. |
There are a total of 3 API to read JSON: | ||
* `JsonDeserializer` reads JSON to recreate complex Nit objects (discussed here), | ||
* the module `json::dynamic` provides an easy API to explore JSON objects, | ||
* the module `json::static` provides the low-level parse JSON with creates basic Nit objects. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure what you mean by with creates basic Nit objects
, can you clarify this sentence?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed it, that sentence was scrambled...
d00ed38
to
a0c39f7
Compare
…& update doc Signed-off-by: Alexis Laferrière <[email protected]>
Signed-off-by: Alexis Laferrière <[email protected]>
a0c39f7
to
88b5b3f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With the modified sentence in the README, +1
Some context first, before this PR, there were 2 API to write JSON: * The module `static` provided `to_json` and `to_pretty_json` which supported writing only basic Nit types associated with JSON types and objects with a manually implemented `to_json`. * The module `serialization` provides `serialize_to_json` which supports all `Serializable` types, a superset of the JSON types, and where standard Nit objects are serialized to JSON objects by generated code. The advantages of the `serialization` API are: * Objects written to JSON with some metadata (mainly the name of the Nit type) can be deserialized back to Nit objects. * The boilerplate code to write the attributes is generated, usually a single `is serialize` after the module declarations does the trick. (`core_serialize_to`, the equivalent of `to_json`, doesn't have to be implemented in each classes, the generated version is usually enough.) * Implementing `core_serialize_to` (or leaving it to the generated code) instead of `to_json` makes the object compatible with both the JSON and binary serialization services. * In general, the `serialization` API allows to easily write Nit objects to JSON and build JSON objects using `Map` instances: ~~~ import json::serialization class Person serialize var name: String var year_of_birth: Int end var bob = new Person("Bob", 1986) assert bob.serialize_to_json(pretty=true, plain=true) == """ { "name": "Bob", "year_of_birth": 1986 }""" var charlie = new Map[String, nullable Serializable] charlie["name"] = "Charlie" charlie["year_of_birth"] = 1968 assert charlie.serialize_to_json(pretty=true, plain=true) == """ { "name": "Charlie", "year_of_birth": 1968 }""" ~~~ So this PR drops the `static` writing API, and replaces its services (`to_json` and `to_pretty_json`) by aliases in `serialization`. Some subclasses have been updated to implement the recursive `serialize_to` instead of the `to_json`, the changes are minimal to avoid breaking any software, but they are not optimal. The result is a single JSON writing API! The generated JSON should not have changed much (maybe a bit less or more white spaces), except for some error messages which were translated to invalid JSON and are now serialized to a valid JSON object. I expect some tests to fail as this is a big change, and clients of the `json::static` writing services (@Morriar) should probably double check my changes to their code. In the future, I'm thinking of dropping the name `serialize_to_json` and `to_pretty_json` and move the optionnal parameters to `to_json`. The JSON module also really needs refactoring at this point. Pull-Request: #2312 Reviewed-by: Alexandre Terrasa <[email protected]> Reviewed-by: Romain Chanoir <[email protected]> Reviewed-by: Jean Privat <[email protected]> Reviewed-by: Lucas Bajolet <[email protected]>
Was broken since #2312 * Migrate `model_json` to new serialization API * Add tests to `model_json` to ensure this doesn't happen anymore * Migrate `web` package and `nitweb` to new API @xymus a lot of serialization for you. Note: for each entity, I needed two deserialization version `full` and no full. Wasn't sure on how to handle this concern properly with your API Pull-Request: #2327 Reviewed-by: Alexis Laferrière <[email protected]>
Was broken since #2312 * Migrate `model_json` to new serialization API * Add tests to `model_json` to ensure this doesn't happen anymore * Migrate `web` package and `nitweb` to new API @xymus a lot of serialization for you. Note: for each entity, I needed two deserialization version `full` and no full. Wasn't sure on how to handle this concern properly with your API Pull-Request: #2327 Reviewed-by: Alexis Laferrière <[email protected]>
Was broken since #2312 * Migrate `model_json` to new serialization API * Add tests to `model_json` to ensure this doesn't happen anymore * Migrate `web` package and `nitweb` to new API @xymus a lot of serialization for you. Note: for each entity, I needed two deserialization version `full` and no full. Wasn't sure on how to handle this concern properly with your API Pull-Request: #2327 Reviewed-by: Alexis Laferrière <[email protected]>
Some context first, before this PR, there were 2 API to write JSON:
static
providedto_json
andto_pretty_json
which supported writing only basic Nit types associated with JSON types and objects with a manually implementedto_json
.serialization
providesserialize_to_json
which supports allSerializable
types, a superset of the JSON types, and where standard Nit objects are serialized to JSON objects by generated code.The advantages of the
serialization
API are:is serialize
after the module declarations does the trick. (core_serialize_to
, the equivalent ofto_json
, doesn't have to be implemented in each classes, the generated version is usually enough.)core_serialize_to
(or leaving it to the generated code) instead ofto_json
makes the object compatible with both the JSON and binary serialization services.serialization
API allows to easily write Nit objects to JSON and build JSON objects usingMap
instances:So this PR drops the
static
writing API, and replaces its services (to_json
andto_pretty_json
) by aliases inserialization
. Some subclasses have been updated to implement the recursiveserialize_to
instead of theto_json
, the changes are minimal to avoid breaking any software, but they are not optimal. The result is a single JSON writing API!The generated JSON should not have changed much (maybe a bit less or more white spaces), except for some error messages which were translated to invalid JSON and are now serialized to a valid JSON object.
I expect some tests to fail as this is a big change, and clients of the
json::static
writing services (@Morriar) should probably double check my changes to their code.In the future, I'm thinking of dropping the name
serialize_to_json
andto_pretty_json
and move the optionnal parameters toto_json
. The JSON module also really needs refactoring at this point.