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

Implement serialization #4

Closed
andylowry opened this issue Mar 28, 2017 · 9 comments
Closed

Implement serialization #4

andylowry opened this issue Mar 28, 2017 · 9 comments

Comments

@andylowry
Copy link
Contributor

Should be mostly straightforward, but will need to implement a means of keeping track of ordering of member initialization in ObjectOverlay types. The corresponding JSON structures should appear in the same order by default in serialized output.

@andylowry
Copy link
Contributor Author

See also #42 which I've marked as duplicating this issue.

@slinkydeveloper
Copy link
Contributor

Also IMHO Serialization of models should deref all models: for example. if i want json schema for a request body, the serialization should give me the complete schema, with deferenced $ref

@andylowry
Copy link
Contributor Author

Thanks, there are definitely use-cases for that, but I think it should be an option. It's similar to something we did with the "Normalizer" that we implemented for Swagger 2.0 in our RepreZen API Studio product. See our article (look for "inlining" options) for details.

I'll probably do #4 in a vanilla fashion first, and then think about inlining options.

@slinkydeveloper
Copy link
Contributor

+1 for that as an option

@andylowry
Copy link
Contributor Author

andylowry commented May 24, 2017

@slinkydeveloper Thanks for your offer of help with serialization.

Here are a few thoughts that I would be going in with if I were starting to work on this:

  1. I believe this should all be part of the JsonOverlay framework (i.e. in package com.reprezen.kaizen.oasparser.jsonoverlay rather than in any of the OAS-specific packages.
  2. There's an exsiting method of JsonOverlay called toJson() that's supposed to create a JsonNode representing this object's content. That should probably be the core of the serializer. It would make it possible to serialize any part of an OpenApi3 object.
  3. The original use of toJson was for something I abandoned, but I haven't yet removed all its vestigs (hence issue Clean up JsonOverlay framework #3). My original idea was that I would maintain the underlying JsonNode structure at all times, to reflect changes made via the mutation methods. That synchronization turned out to be very difficult, and I eventually decided that it wasn't really yielding any benefit, so I dropped that goal entirely. It means that serialization needs to be explicitly called for when needed, rather than the serialized form of the object always being immediately available. So I think it makes sense to use toJson for serialization, but you may find some cruft around it relating to this abandoned strategy.
  4. For the most part, toJson should be fairly straightforward - including lists and maps. Objects will be more complicated.
  5. A bit of background on collections:
  • There are four types of "collection" managed by the framework: MapOverlay, ValMapOverlay, ListOverlay and ValListOverlay.
  • The maps manage what in JSON are objects, but without fixed properties defined by some schema (ObjectOverlay handles those). Examples are the OAS Paths Object, Responses Object, etc.
  • The lists are for orderded lists of overlay objects (JSON arrays). Examples are OAS servers and parameters lists.
  • The difference between ValXxx (value) and Xxx is that methods of the value classes reflect value types that are not the same as the underlying overlay type of the values in the collection. E.g. all the vendor-extensions fields in the API are maps of String -> Object but the underlying overlay object type is AnyObjectOverlay. In contrast, paths are managed by a map of String -> Path, and in this case Path is both the API-level class representing a path object, and the overlay class used to represent that value. The former case would use ValMapOverlay<Object, AnyObjectOverlay> while the latter would use MapOverlay<Path>.
  • Even the map objects maintain their values in a LinkedHashMap so that order of definition is retained and will naturally be reflected in the serializer output.n (All collection objects actually use an object of type CollectionStore to keep their data, and it's that class that maintains ordering.)
  1. About "object overlays":
  • These are most of the familiar OAS object types, like Path and Info and OpenApi3 (the root object). They're all extensions of the ObjectOverlay class from the JsonOverlay framework.
  • These objects, unlike the map objects, do not maintain property values using a map. Instead, the superclass declares instance members that hold overlay objects of the correct type. E.g. Info declares a StringOverlay member named title.
  • Motivated initially by the needs of the BigParseTest test, I created a sort of "reflection" capability in ObjectOverlay so that one can discover what properties an object declares and obtain the property values, without having to code to the object's specific interface. See the code starting with the declaration of fieldData in the ObjectOverlay class.
  • This reflection mechanism can probably be used to serialize these objects (find the names of the declared properties and add them and their values to the JSON object under construction). You'll probably want to change the fieldData map from a HashMap to a LinkedHashMap so properties are ordered better than randomly. Ordering still won't be as it should be by default - i.e. order of property creation (either during parse or subsequently via mutation methods). That will require additional bookkeeping. But it's probably better to tackle that separately, and having things ordered according to member declaration in the superclass is much better than random.

As for the overall API, it probably makes sense to do something like this, as methods of JsonOverlay:

  • toJsonNode() yields a Jackson JsonNode value
  • toJson calls toJsonNode and converts the value to JSON
  • toYaml calls toJsonNode and converts the value to YAML

Let me know your thoughts, and any questions you have. I'll try to be responsive, but the same urgent work that's pulled me away from this project for a while will continue to limit my bandwidth, unfortunately.

So fork away and have fun with it!

@andylowry
Copy link
Contributor Author

@slinkydeveloper One other point - if you don't pay any attention to references, you should get the serialization output you prefer, rather than what I'd actually prefer as the default. But you'll fail with stack overflow on recursive structures. Might be OK as a first step, and there are some outstanding issues with references may also cause some problems along the way. Let me know if/when you want to address this aspect.

slinkydeveloper added a commit to slinkydeveloper/KaiZen-OpenApi-Parser that referenced this issue May 25, 2017
For more informations check out GettingStarted.md diff of this commit
@slinkydeveloper
Copy link
Contributor

After a little bit of investigation, I wrote a draft of the mechanism to deference (as you said, i found that actually the method to convert to JsonNode already exists, it only needs to be declared inside interfaces to avoid the cast). Check out my commit: slinkydeveloper@a17144b , this snippet https://github.com/slinkydeveloper/KaiZen-OpenApi-Parser/blob/master/GettingStarted.md#get-jsonnode-of-a-complete-deferenced-jsonnode and run this test: https://github.com/slinkydeveloper/KaiZen-OpenApi-Parser/blob/master/kaizen-openapi-parser/src/test/java/com/reprezen/swaggerparser/test/DerefTest.java
It's a really bad quality code, but i think you can get some snippets from it. The "awesome on/off switch" Reference.derefAllTree = true it's only a convenience way for me to create a working mechanism. This parameter is used inside reference class to force walk inside json tree to find references.
Actually with Reference.derefAllTree = true if you pass a JsonOverlay that is a reference object, getJson() and getDereferencedJsonTree() return the same result. Otherwise, if JsonOverlay is another type of object, getJson() don't dereference, while getDereferencedJsonTree() derefence (check out this slinkydeveloper@a17144b#diff-4d68265eddb12acdb909f802d5a1464cR134 )

@slinkydeveloper
Copy link
Contributor

Refactored all my derefence mechanism, now it's a lot more simpler and doesn't affect the parsing routine. I reverted back changes i've done in Reference class. I removed the magic on/off switch (you can watch the DerefTest) and the deference mechanism doesn't affect parsing routine. Now the function getDereferencedJsonTree() does:

  • Dereference all pointers inside json tree (included pointers to external files)
  • return a JsonNode that can be serialized to string (JsonNode.toString())

Also it seems that if you use set methods of interfaces inside model3 package, it change the internal json value so when you call getDereferencedJsonTree() it returns the user transformed JsonNode. Now it need only to add some convenience methods. This is the pr: #43

andylowry added a commit that referenced this issue Sep 22, 2017
The `toJson()` on `JsonOverlay` now checks whether its initial json is
still current, and if so just uses that. Thus for the vast majority of
applications that are read-only, serialization will be extremely
fast. However, the mutation API methods now all notice when an overlay
object's value has changed, and they mark their JSON value as no longer
current. Then `toJson()` calls `createJson()`, whose job it is to create
a new JSON structure representing the current overlay value. This is
implemented for all but `ObjectOverlay`, for which preserving ordering
of child structures is difficult and needs work.
andylowry added a commit that referenced this issue Sep 25, 2017
These will support creating JSON from an ObjectOverlay, and will also
replace the reflection-based field accessor mechanism.
andylowry added a commit that referenced this issue Sep 25, 2017
This replaces the reflection-based accessors formerly implemented by
`ObjectOverlay`. Path-based value retrieval is now implemented across
all JsonOverlay classes, in the form of the `find(JsonPointer)`
method. This is far more powerful than the prior feature, and without
the cost of reflection. ObjectOverlay extension classes must now, at
construction time, provide information required by the `ObjectOverlay`
implementation of this feature. The `ObjectOverlay` constructors call
out to abstract  method `installPropertyAccessors` to trigger this.
andylowry added a commit that referenced this issue Sep 25, 2017
Required fixing some issues with PropertyAccessors, and chaging
Link.href to Link.operationRef in the test yaml
andylowry added a commit that referenced this issue Sep 25, 2017
Running codegen from scratch yielded a SchemaImpl java source that was
missing a required import of JsonPointer. Now that's fixed.
andylowry added a commit that referenced this issue Sep 25, 2017
The `createJson` method now works for all overlay types and creates
correct JSON for each. However, it blindly follows references and so
does not yield the same result as a `toJson` method when the in the
presence of references, and when the parsed model has not been
modified. Following references should be an option, and without it,
`createJson` and `toJson` should yield equivalent trees.
andylowry added a commit that referenced this issue Sep 25, 2017
Also modified JsonLoader to use the trimmed model text only to guess
whether it's JSON or YAML, but then actually parse the untrimmed
text. Using the trimmed text had unintended potential effect of dropping
a line terminator in a final block-literal string in a YAML file.
andylowry added a commit that referenced this issue Sep 25, 2017
This shouldn't have changed anything, but a reformatted version of
SecurityRequirementImpl had been mistakenly checked in. This is now as
produced by code generator, to avoid future confusion.
andylowry added a commit that referenced this issue Sep 25, 2017
Boolean option enables link following in both toJson and createJson
methods. Locally available JSON object in a JsonOverlay object should be
one that includes references, so toJson only updates locally stored JSON
if followRefs is false.
andylowry added a commit that referenced this issue Sep 25, 2017
Also changed getJson method to protected, as it shouldn't be part of the
public API
@andylowry
Copy link
Contributor Author

Initial implementation in #82

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants