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

provide @JsonbTransient on class level #213

Closed
nimo23 opened this issue Dec 14, 2019 · 7 comments
Closed

provide @JsonbTransient on class level #213

nimo23 opened this issue Dec 14, 2019 · 7 comments

Comments

@nimo23
Copy link

nimo23 commented Dec 14, 2019

Please provide @JsonbTransient on class level to list all properties which should be ignored for (de)serializing:

@JsonbTransient(value = {"id", "username", "getType", "setAge"})
public class User {

   private int id;
   private String username;
   private String type;
   private int age;

   public String getType(){
        return type;
    }

   public int setAge(){
        this.age = age;
   }

}

With this, the user can set and see all transient properties at a glance. And it avoids cluttering/polluting the class with a bunch of @JsonbTransient at each property/method.

Another solution could be:

// excludes properties/methods for json processing, others will be included
@JsonbExclude(value = {"id", "setAge"})

// inlcudes properties/methods for json processing, others will be excluded
@JsonbInclude(value = {"username", "getType"})

public class User {
    ..
}
@rmannibucau
Copy link

Hi @nimo23, you can do it with the visibility api. I think it matches more a custom need like that cause this looks less natural and highly error prone compared to putting jsonbtransient per field.

@nimo23
Copy link
Author

nimo23 commented Dec 14, 2019

you can do it with the visibility api.

With visibility api, you cannot include/exclude specific fields/methods.

I think it matches more a custom need like that cause this looks less natural and highly error prone compared to putting jsonbtransient per field.

I dont think so. I described the benefits when putting it on class level. Btw, Jackson already has an annotation like that:


// jackson annotation
@JsonIgnoreProperties({ "id" })
public class User {

}

Highly error prone? Why? Then something like this should also be considered highly error prone:

@JsonbPropertyOrder(value = {
		"idUser",
		"username"})
public class User {
..
}

@rmannibucau
Copy link

Ok, here are the points to take in consideration IMHO:

  1. Does it enable any new use case? No, we already handle it and it does not decouple the code from the config so it is purely syntaxic sugar and duplicates existing api.
  2. Is it typesafe and model safe? No since it is based on strings, we will get the same ambiguity than the order annotation (are string json names or java field names) and the same user issues based on typo (the kind of issues you can spend a long time for nothing). Using an index in jsonbpriority would have been saner but not much inheritance friendly - whereas transient being can be handled more easily with accessors since it owns a single data (its presence) and no attribute.
  3. Can the user handle such api easily? Yes, a custom visibility strategy does the job in a few lines and even enables more advanced conventions and api (I am thinking to this custom annotation but also libraries custom api).
  4. This is likely not the mainstream jackson api so not sure se should promote it as a standard.
  5. Other jakarta specs dont have it - even jaxb - and there is no real issue with it since the years it is used.
  6. It makes it irrelevant on fields until creating another annotation which is not that nice on api side IMHO.

Hope it explains more where I am coming from.

@nimo23
Copy link
Author

nimo23 commented Dec 14, 2019

  1. Does it enable any new use case?

No. But with this, the user can set and see all transient properties at a glance. And it avoids cluttering/polluting the class with a bunch of @JsonbTransient at each property/method.

  1. Is it typesafe and model safe?

Unfortunately, not. But @JsonbPropertyOrder is also not typesafe, the same as adding strings in jpa @TypedQuery - it is also not typesafe. But this does not mean it does not work.

  1. Yes, a custom visibility strategy does the job in a few lines and even enables more advanced conventions and api

Do you mean PropertyVisibilityStrategy where we can only set isVisible for all fields or all methods?

  1. This is likely not the mainstream jackson api

It is a legal jackson annotation with reasonable use cases. If not, it would not exist. I dont know what you mean with mainstream.

  1. Other jakarta specs dont have it

I am only talking about json-b spec. Only because it does not exit in json-b yet, does not mean that users have no need for that.

  1. It makes it irrelevant on fields until creating another annotation which is not that nice on api side IMHO.

I dont know what you mean.

@rmannibucau
Copy link

  1. I guess it is a choice between 2 code style and it had been done in 1.0
  2. I explained why the exception of order api
  3. Yes, default impl being trivial
    4.i meant "used often enough", in dozens of apps using jackson I see at works none use that and with another dozen using jsonb there is no lack of that (ie using transient at field or method level is satisfying and simple with a unique codestyle which is a big plus)
  4. Point is that this need would have popped in other spec if more than a codestyle and fact is it didnt so no reason jsonb becomes an exception IMHO
  5. @JsonbTransient({}) or @JsonbTransient({"foo"}) String bar; would become an error at field level so api is no more natural IMO so we loose more than we gain as overall result if you cumulate all points.

@aguibert
Copy link
Contributor

I agree with @rmannibucau here -- I don't like how the @JsonbPropertyOrder refers to properties as strings because it is very error prone and I don't think we should spread this sort of pattern further in the spec unless it unlocks a compelling use case that was not possible before. We often have users get confused about this, mainly because:

  • If you type the string wrong in the @JsonbPropertyOrder annotation, it is silently ignored. This means as Java fields/methods get renamed or refactored it could change the behavior of @JsonbPropertyOrder because there is no type-safety guarantee like when the annotation is on the java element
  • If you want to apply an ordering to a field/method that is renamed (e.g. @JsonbProperty("foo") public String bar;) it's not obvious without carefully reading the spec what string to put in @JsonbPropertyOrder annotation ("foo" or "bar")

Regarding this point:

And it avoids cluttering/polluting the class with a bunch of @JsonbTransient at each property/method.

We have an issue open for doing all JSON-B customizations programatically without modifying the source class at all here: #88
I think this issue would solve the concern of "not cluttering the class with JSON-B annotations" while also unlocking a new use case of being able to apply JSON-B annotations for 3rd-party classes that the user cannot change.

@nimo23
Copy link
Author

nimo23 commented Dec 14, 2019

@rmannibucau, @aguibert Yes, you are right. Now I understand. Thanks for explanation.

I think this issue would solve the concern of "not cluttering the class with JSON-B annotations"

yes, absolutly. Thanks!

@nimo23 nimo23 closed this as completed Dec 14, 2019
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

3 participants