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

Remove dependency on Jackson via SmallRye -2.x #1458

Merged
merged 11 commits into from
Mar 9, 2020
Merged

Remove dependency on Jackson via SmallRye -2.x #1458

merged 11 commits into from
Mar 9, 2020

Conversation

tjquinno
Copy link
Member

@tjquinno tjquinno commented Mar 2, 2020

Not ready for approval yet -- need to update the helidon-build-tools version in the top-level Helidon pom once we have a new helidon-build-tools release.

Overview - removing our dependency on Jackson

(fixes #1109)

Our OpenAPI implementation layers on SmallRye’s OpenAPI module, which on its own uses Jackson to parse the OpenAPI JSON or YAML and to parse the JSON in OpenAPI @Extension annotations. The SmallRye API has for some time allowed the calling code to — and Helidon now does — completely bypass the main parsing (doing so itself before invoking SmallRye), and a recent SmallRye enhancement allows the caller to provide its own way to do the @Extension parsing.

Removing the Jackson dependency involves three main areas of change:

  1. creating the helidon-build-tools SnakeYAML plug-in, (merged separately)
  2. changing the Helidon OpenAPI pom to drive that plug-in, and
  3. changing the Helidon OpenAPI code to
    enhance the SnakeYAML-created object model for OpenAPI in important ways,
    to parse YAML and JSON using SnakeYAML (rather than leaving that to SmallRye), and
    driving the SmallRye API slightly differently to accomplish the above.

The plug-in

SnakeYAML does a good job identifying properties from the MicroProfile OpenAPI interfaces, but SnakeYAML cannot (nor can any tool that looks only at compiled byte code) build a complete map between the JSON/YAML serialization and the object model. The SnakeYAML API allows callers to provide this additional information.

The plug-in analyzes the MP OpenAPI interfaces and the SmallRye implementation classes and generates a class that includes this helper logic for SnakeYAML.

The Helidon OpenAPI module component pom.xml

The pom:

  1. enhances the dependency on SmallRye to exclude Jackson,
  2. invokes the dependency plug-in to extract the source code for the MicroProfile OpenAPI interfaces and the SmallRye implementations, and
  3. invokes the new helidon-build-tools SnakeYAML plug-in to generate the helper class.

The Helidon OpenAPI changes

The new plug-in is independent of OpenAPI, yet there are some aspects of the OpenAPI interfaces and the SmallRye implementations that need special attention to correctly map between JSON/YAML and the object model. These adjustments to the model occur in the OpenAPISupport#adjustTypeDescriptions method.

In MP OpenAPI 1.1.2, several interfaces extend Map<X, Y>. This “confuses” SnakeYAML slightly, in that when deserializing a document it instantiates an Object for the children instead of the actual implementation class for the interface Y. MP OpenAPI seems to be abandoning this approach in 2.0, but we still need to deal with it now. That happens in CustomConstructor.

The SnakeYAML API relies heavily on the TypeDescription class for conveying non-default information about mapping between documents and the object model. In OpenAPI several interfaces have the same unusual needs, and those are collected in the Helidon OpenAPI-specific ExpandedTypeDescription class. (The class which the plug-in generates takes a type parameter which extends TypeDescription and a factory function for creating new instances of the desired type.)

For parsing the OpenAPI @Extension annotation (always JSON), SmallRye accepts an AnnotationScannerExtension which, among other things, supports the parseExtension method. The Helidon version of this is based heavily on the source from SmallRye, the key difference being in how the JSON is parsed and handled. There might be ways of doing this with less code but patterning closely after the (working) SmallRye implementation that uses Jackson seemed wise.

Oru custom Serializer class mostly sets SnakeYAML formatting configuration before invoking SnakeYAML’s Yaml#dump method to serialize the object model. We need to tailor some of the processing beyond what the config settings support, hence the CustomRepresenter and TagSuppressingWriter inner classes. Note that one of those customizations is to work around a bug in the MP OpenAPI TCK (see the comments in the code).

@tjquinno tjquinno added open-api 2.x Issues for 2.x version branch labels Mar 2, 2020
@tjquinno tjquinno self-assigned this Mar 2, 2020
@tjquinno tjquinno changed the title WIP: Remove dependency on Jackson via SmallRye -2.x [skip ci] WIP: Remove dependency on Jackson via SmallRye -2.x Mar 2, 2020
Copy link
Member

@ljnelson ljnelson left a comment

Choose a reason for hiding this comment

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

I appreciate the digestible PR size. Only minor changes requested (and not all of my comments are requested changes).

romain-grecourt
romain-grecourt previously approved these changes Mar 4, 2020
@tjquinno tjquinno changed the title [skip ci] WIP: Remove dependency on Jackson via SmallRye -2.x Remove dependency on Jackson via SmallRye -2.x Mar 4, 2020
ljnelson
ljnelson previously approved these changes Mar 4, 2020
@tjquinno tjquinno dismissed stale reviews from ljnelson and romain-grecourt via c93cdee March 4, 2020 21:39
ljnelson
ljnelson previously approved these changes Mar 4, 2020
Comment on lines 39 to +41
<!-- TCK requires Jackson, not JSON-B -->
<groupId>org.glassfish.jersey.media</groupId>
<artifactId>jersey-media-json-binding</artifactId>
<groupId>jakarta.json.bind</groupId>
<artifactId>jakarta.json.bind-api</artifactId>
Copy link
Member

Choose a reason for hiding this comment

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

Just a dumb comment: comment says Jackson, artifact is JSON-B, yes?

@tjquinno tjquinno merged commit 75d5343 into helidon-io:master Mar 9, 2020
@tjquinno tjquinno added this to Closed in Backlog via automation Mar 9, 2020
@tjquinno tjquinno deleted the remove-jackson-2.x branch March 9, 2020 15:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2.x Issues for 2.x version branch open-api
Projects
Backlog
  
Closed
Development

Successfully merging this pull request may close these issues.

Jackson dependency brought in by OpenApi
3 participants