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

Methods in Json re-create JsonProvider on each execution, Json should cache the result of JsonProvider.provider() #346

Open
rbygrave opened this issue Dec 14, 2021 · 2 comments

Comments

@rbygrave
Copy link

rbygrave commented Dec 14, 2021

The methods in Json use JsonProvider.provider() and this re-creates the JsonProvider on each execution.

The javadoc for JsonProvider.provider() suggests that the caller should cache the resulting JsonProvider and jakarta.json.Json does not do this, so I think this is a bug.

For example, in Json we see:

    /**
     * ...
     */
    public static JsonParser createParser(Reader reader) {
        return JsonProvider.provider().createParser(reader);
    }

    /**
     * ...
     */
    public static JsonParser createParser(InputStream in) {
        return JsonProvider.provider().createParser(in);
    }

In the javadoc for JsonProvider.provider() is says:

"Creates a JSON provider object. ... Users are recommended to cache the result of this method."

I believe that means that Json should be changed to indeed cache the result of JsonProvider.provider().

@lukasj
Copy link
Contributor

lukasj commented Dec 14, 2021

see #26 and #154

@rbygrave
Copy link
Author

rbygrave commented Dec 15, 2021

the reason here was problematic caching in server environment. EE compliant server must provide some implementation which is typically loaded by bootstrap/server root classloader but also allow usage of different implementation which may be part of some web application (thus loaded by webapp classloader).

So is this saying that jakarta.json.Json is loaded by the bootstrap/server root classloader, and hence on per webapp basis each webapp can't then use it's own potentially different provider. The net effect is that jakarta.json.Json can't cache the provider as it is per EE container.

Ok, I think I see that. My initial thought is that it seems that could be fixable by using a well known system property that EE servers would set (or anyone that wants this existing behavior). That is, EE compliant server sets a well known system property that effectively keep this existing behavior and the absence of that system property for non-EE, microservices style, everyone else etc JsonProvider.provider() only initialises the provider once.

Something like:

  • Not this PR, leave jakarta.json.Json as it is calling JsonProvider.provider()
  • Modify JsonProvider.provider() to do existing behavior (in the presence of a well known system property that EE servers set) and otherwise only initialise a JsonProvider once and cache it in JsonProvider.

Did I miss the issue / problem?
Does the approach of using a well known system property sound ok?

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