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

transient-keyword should be ignored by jsonb #269

Closed
nimo23 opened this issue Apr 13, 2021 · 7 comments
Closed

transient-keyword should be ignored by jsonb #269

nimo23 opened this issue Apr 13, 2021 · 7 comments
Labels
wontfix This will not be worked on

Comments

@nimo23
Copy link

nimo23 commented Apr 13, 2021

Describe the bug
Properties with transient-keyword should not be a trigger for jsonb for not (de)serializing.

To Reproduce
I have this:

// should (de)serialize it, but jsonb ignores @JsonbProperty and give "transient" a higher priority, 
// thus jsonb, unfortunatley, does not (de)serialize user.
@JsonbProperty
private transient User user;

With the above code, jsonb (with its implementation yasson) does not generate json for property user. Only when removing transient-keyword, then yasson will generate the json.

Expected behavior
Jsonb should ignore the java keyword transient for its (de)serializing stuff. Only @JsonbTransient should be a trigger for omitting properties. And if this is not possible, then marking a property with @JsonbProperty should have priority over transient- keyword.

@nimo23 nimo23 changed the title transient should be ignored by jsonb transient-keyword should be ignored by jsonb Apr 13, 2021
@rmannibucau
Copy link

Hi @nimo23, think it is already covered https://github.com/eclipse-ee4j/jsonb-api/blob/cc5273d2b0ff06f778747428fb672830844337be/spec/src/main/asciidoc/jsonb.adoc:

JSON Binding implementations MUST NOT deserialize into transient, final or static fields and MUST ignore name/value pairs corresponding to such fields.

and

Implementations MUST support serialization of final fields. Transient and static fields MUST be ignored during serialization operation.

and AFAIK it is expected because these fields are generally not serializable (generally speaking).

@nimo23
Copy link
Author

nimo23 commented Apr 13, 2021

@rmannibucau I think, you did not understand #269 (comment):

Transient and static fields MUST be ignored during serialization operation.

This is exactly what I don't like because transient-keyword can be used for other use cases than jsonb. If I use "transient" for the other use case, it clashes with jsonb spec..

Jsonb should behave like this:

// Jsonb MUST (de)serialize user (even with keyword "transient"), if the user marks this property with @JsonbProperty. This is currently NOT the case.
// The "transient" keyword is used for another use case than jsonb, so I cannot omit it.
// Unfortunatley, if I do not omit "transient", then jsonb does not (de)serialize this property.
@JsonbProperty 
private transient User user;

Btw, I have explicitly marked a property with jsonb annotations (@JsonbProperty), and, unfortunately, the spec currently says: "Hey, I ignore my own annotation because of the transient keyword).

The spec should behave like this:

// (de)serialize because of explicitly marked with @JsonbProperty (actually, not covered by spec)
@JsonbProperty 
private transient User user;
// no (de)serialize because of explicitly marked with transient (already covered by spec)
private transient User user;
// no (de)serialize because of explicitly marked with @JsonbTransient (already covered by spec)
@JsonbProperty
@JsonbTransient
private User user;

@rmannibucau
Copy link

@nimo23 well I see two points:

  1. we must not break users so I fear it is too late to change until we add a @JsonbIncludeTransient on the class (which I think is overkill since you can drop transient)
  2. this was done for the mentionned reason: there is the case you mention where this is done for java serialization and would work for jsonb but you also have all the cases it is done because it will never work for any serialization (ClassLoader for ex) and the case you reuse models in JSONB (implicitly or explicitly like to store in a DB) and you must respect transient

So overall I don't see the default changing since it prove to be the best but we can get a toggle to change it...and luckiyl it already exists with property visibility so I guess we must just ensure it works already and clarify this API more than changing anything else.

@nimo23
Copy link
Author

nimo23 commented Apr 13, 2021

and you must respect transient

To sum up:
Currently, I cannot solve the conflict when using transient keyword for other use cases than json-b. There is no chance to tell json-b: "Hey, do the (de)serializing on that field even with a transient keyword, because I added explicitly @JsonbProperty."

So overall I don't see the default changing since it prove to be the best but we can get a toggle to change it...and luckiyl it already exists with property visibility so I guess we must just ensure it works already and clarify this API more than changing anything else.

Yes, I could add PropertyVisibility with field and method visibility set to true. However, then I change it globally only because json-b ignores its own @JsonbProperty in favour of "transient"..

@rmannibucau
Copy link

@nimo23 you mean @JsonbProperty public transient String foo;? Have to admit from a code design perspective it looks like a workaround and not a solution to me. JsonbProperty is designed to rename a property, not to include/exclude properties. Concretely public class Foo { @JsonbProperty private String foo; } will serialize any Foo instance as {}, same happens to properties filtering so I wouldn't encourage this misusage and prefer to promote the SPI usage instead.

@m0mus
Copy link
Contributor

m0mus commented Apr 13, 2021

We believe that Java modifiers like transient and visibility options like private and public have the highest priority and it's functionality must be consistent over all upper stack frameworks. It means that if a field is transient it must behave as transient in JSONB. If you have a use case where you need to serialise or desirealize a transient property, your architecture is bad and you have to change it.

I am closing this issue as won't fix.

@m0mus m0mus closed this as completed Apr 13, 2021
@m0mus m0mus added the wontfix This will not be worked on label Apr 13, 2021
@nimo23
Copy link
Author

nimo23 commented Apr 13, 2021

We believe that Java modifiers like transient and visibility options like private and public have the highest priority and it's functionality must be consistent over all upper stack frameworks.

Yes, this is reasonable. After thinking about, I come to the same conclusion.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
wontfix This will not be worked on
Projects
None yet
Development

No branches or pull requests

3 participants