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

deserialization to case class with required field #203

Open
igreenfield opened this issue May 5, 2015 · 23 comments
Open

deserialization to case class with required field #203

igreenfield opened this issue May 5, 2015 · 23 comments

Comments

@igreenfield
Copy link

I have this case class:

 case class Time(val a: String, val b: String)

and this json:

 {
    "a" : "12345"
 }

When I serialize this json to the Time class I expect to get serialization error.
Something like: "missing required field"

@nbauernfeind
Copy link
Member

This is not a bug... Your json doesn't have a 'b' field.

Try:

@JsonIgnoreProperties(ignoreUnknown = true)
case class Time(val a: String, val b: String)

Then 'b' will be null.

Another alternative:

case class Time(val a: String, val b: Option[String])

Then 'b' should be None.

@igreenfield
Copy link
Author

I know this is not bug. but I need a way to tell jackson to fail the parse hance this json miss one required field...

@nbauernfeind
Copy link
Member

Oh you're not getting an error? I misunderstood; I see the same behavior. This happens even without the scala specifics.

See this example

class T2{
  @JsonProperty val a: String = null
  @JsonProperty val b: String = "missing"
  override def toString = s"$a $b"
}
object T extends App {
  val mapper = new ObjectMapper() with ScalaObjectMapper
  println(mapper.readValue[T2]("{\"a\":\"a\", \"b\":\"b\"}"))
  println(mapper.readValue[T2]("{\"a\":\"a\"}"))
}

Which has output "a b", "a missing". So it's using a [sane] default value (in your example it sets to null).

You used to be able to set @JsonProperty(required = true) but that doesn't seem to throw in the non-scala use case either. Maybe @cowtowncoder or @christophercurrie can chime in on what happened to required properties.

@cowtowncoder
Copy link
Member

Databind has nothing to enforce required properties, information is passed solely as metadata for other processing such as JSON Schema generation.

One way to add checks for missing on plain Java side is to use a creator method, and add check in there. This does not allow distinguishing between explicit nulls and missing values, but if nulls are not legal it can work.

Since case class support is specific to Scala module, however, perhaps it is possible to enforce required-ness for that special case. I am guessing handling is similar to use of creator methods, so information may be available.

In theory Creator method handling could also be improved to catch missing values, more easily than the generic POJO binding. There is also an RFE for general support for enforcing required-ness, but that is unfortunately much bigger technical challenge (has to do with state tracking etc) and unlikely to be implemented on short term.

@cowtowncoder
Copy link
Member

Quick follow-up question: assuming I was able to actually add support for this for Creators (perhaps case class handling just uses creator mechanism), would it:

  1. be ok to require required for all mandatory parameters (more typing, but existing annotations)
  2. be good to add a new property for @JsonCreator, like "requireAll=true", to allow quick override
  3. add a new DeserializationFeature for requiring all creator parameters for everything, by default?

or some combination thereof? I understand that for your use case you just want them all to be mandatory, but since creator functionality is widely used there are other existing preferences.
So I want to try to minimize problems caused by changing behavior, by requiring little bit of explicit definitions.

Option (2) would actually also allow case class handling to use default of "yes, require all", regardless of @JsonCreator annotation, I think. So there's that.

@christophercurrie
Copy link
Member

I don't personally have a strong preference on mechanism; I suspect most users would want a global config, at least, as well as per-creator or per-parameter control, but I have no evidence either way.

@cowtowncoder
Copy link
Member

I am thinking that "all three" makes sense, based on historical track record... I tend to end up having to add global/local-override layers if I do not start with them. And it is not that much more work realistically.

What I can do is file an issue for databind, for adding (1) first. If that is feasible, other options should be easy to add.

@cowtowncoder
Copy link
Member

Added:

FasterXML/jackson-databind#781

@nbauernfeind
Copy link
Member

That sounds like a good plan. I didn't realize that required was only documentation.

@cowtowncoder
Copy link
Member

@nbauernfeind It was... sort of speculative addition. I had the best intention to try to support it, but implementation is unfortunately quite more complicated than it would appear. So at the moment it is just for documentation.

@igreenfield
Copy link
Author

As for Scala I would prefer that the Option type will mark the not required fields and all other field would be required.

@yairogen
Copy link

yairogen commented May 6, 2015

@cowtowncoder I am quite surprised about your comment regarding the required attribute. Can I please ask that if issue #781 mentioned above is not solved in the next version - at least update documentation to state it is not implemented?

@cowtowncoder
Copy link
Member

@yairogen This is documented:

http://fasterxml.github.io/jackson-annotations/javadoc/2.5/com/fasterxml/jackson/annotation/JsonProperty.html#required%28%29

although we can always modify description.

@igreenfield the problem is not the interpretation of what properties should be required, but rather that there is nothing to enforce it.

@cowtowncoder
Copy link
Member

I implemented #781 for 2.6.0, so that required check is now honored for Creator (constructor) properties. If case class handling uses creator property construction (which I think is the case, but I may be wrong), it should then use similar requirement rules.

I'll work with @christophercurrie to try to make sure this also works with case class handling.

@igreenfield
Copy link
Author

@cowtowncoder I test it with 2.6.0-rc2 version get the same problem.

@sesponda
Copy link

sesponda commented Sep 7, 2015

Thanks for implementing #781. I still have a problem I'm not sure how to solve. If I enable DeserializationFeature.FAIL_ON_MISSING_CREATOR_PROPERTIES with this class:

case class CustomerCreateCmd(name: String, email: Option[String] = None) 

Users are forced to set the email field. If i disable the option, I have another problem: users can skip the name field, which will become a null value in the Case class (I would expect an error).
Ideally, I'd like to make non-Option fields required. I've tried this, with no success:

CustomerCreateCmd(name: String, @JsonProperty(value = "email", required = false) email: Option[String] = None)

Any hints?...
Thank you,

@christophercurrie
Copy link
Member

@sesponda Right now, there's no explicit support for what you're trying to achieve. The only way I can think of to solve the issue is to rewrite your class:

case class CustomerCreateCmd(name: String)
{
  var email: Option[String] = None
}

If you enable FAIL_ON_MISSING_CREATOR_PROPERTIES, then name will be required, but email will not.

At this time, there's no support for default values for constructor parameters, and supporting them will be tricky until 2.12 is released; this would be a prerequisite to handling this case more gracefully.

@christophercurrie
Copy link
Member

@igreenfield I agree with your goals, but FAIL_ON_MISSING_CREATOR_PROPERTIES has specific semantics to core Jackson, so the Scala module will need to implement it's own feature to support this use case. If core Jackson ever implements similar behavior for Java 8's Optional, then there's a chance of re-use there.

@NTCoding
Copy link

I'm trying to discuss with the jackson team how to get this feature implemented or a hook that we can hook into. If anyone would like to help me explain the situation to them please add your comments here: FasterXML/jackson-databind#988

@bosyi
Copy link

bosyi commented Jun 29, 2018

I set mapper.configure(DeserializationFeature.FAIL_ON_MISSING_CREATOR_PROPERTIES, true),
define case class with optional field and if this field is missing in json I still have an exception.
If you program in functional style this scala plugin is useless until this get fixed. Am I not right?

@Vistritium
Copy link

Vistritium commented Nov 3, 2022

I solved this issue with DeserializationFeature.FAIL_ON_NULL_CREATOR_PROPERTIES

@pjfanning pjfanning changed the title serialization to case class with required field deserialization to case class with required field Nov 3, 2022
@pjfanning
Copy link
Member

pjfanning commented Nov 3, 2022

jackson-module-scala supports default values in case class constructors since v2.11.0 (#87). There have been subsequent changes too, so I would encourage anyone who needs this support to upgrade to latest version.

pjfanning added a commit that referenced this issue Nov 3, 2022
@pjfanning
Copy link
Member

pjfanning commented Nov 3, 2022

When using latest jackson-module-scala versions:

I added these 2 tests:
e701b21

I'm not sure it makes sense to change the existing behaviour.

  • this default behaviour is long established and some users may depend on it
  • if you don't like the default behaviour, you have 2 and possibly other solutions you can use

pjfanning added a commit that referenced this issue Nov 3, 2022
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

10 participants