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

[#719] JSON-B extension. #720

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

bmarwell
Copy link

@bmarwell bmarwell commented Apr 15, 2022

Fixes #719:

This commit will add a JSON-B extension based on JSON-B specification 1.0.2.

Noteworthy

  • Since this specification uses Java 8 features (static methods in interfaces), the property jdk.version was set to 8.
  • Since we now use Java 8, there is little advantage in using custom assertions, this null input is validated by java.util.Objcets.requireNonNull and will throw a NullPointerException (which makes more sense anyway) instead of an IllegalArgumentException.
  • Instead of Strings.UTF_8 the JDK8-included StandardCharsets.UTF_8 was used.
  • I used Johnzon for testing, but only because it is an Apache project. It should work with Eclipse Yasson without any changes. If desirable, I could implement a maven profile for this – or even better, an IT project (e.g. intergation-tests/json/jsonb-yasson and intergation-tests/json/jsonb-sth-else).
  • The (provided) dependency on json-api (aka json-p for JSON-Parser) is needed at least for Johnzon, but probably for some Exceptions as well. Johnzon fails because javax.json.JsonException is missing. But since json-api/json-p is a transitive dependency for json-bind-api (json-b) as well, this should be okay.

Other notes

  • The provided scope is because of JakarteEE Application Server usage. Tested on OpenLiberty. But for JavaSE apps, users of this lib would need to include those by hand, as well as the implementation. This should go into the README.

Tasks

  • Added implementation.
  • README.md updated.
  • Added notice about JavaSE vs JakartaEE environments and their ([not-]provided) dependencies.
  • Added "when-to-use" note.

README.md Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
extensions/pom.xml Outdated Show resolved Hide resolved
@bdemers
Copy link
Member

bdemers commented Apr 18, 2022

Great work!

@bmarwell
Copy link
Author

Oh, shouldn’t the profile name be "nonJDK7"? And did I, by accident, not rebase the PR?

@bdemers
Copy link
Member

bdemers commented Apr 19, 2022

good point, java8plus, nonJDK7 or something like that would work

@bmarwell
Copy link
Author

good point, java8plus, nonJDK7 or something like that would work

Nah you already have "nonJDK7" in your root-pom.xml.
I will rebase this PR later when I have a first version of my modules-PR available. I finished it yesterday evening but didn’t push it yet, just a moment.

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

Successfully merging this pull request may close these issues.

Add JSON-B extension
2 participants