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

Invalid date should fail to parse #50

Closed
jamesagnew opened this Issue Nov 11, 2014 · 1 comment

Comments

Projects
None yet
1 participant
@jamesagnew
Owner

jamesagnew commented Nov 11, 2014

Reported on the mailing list here by Joseph Althman: https://groups.google.com/forum/#!topic/hapi-fhir/MT6eDnuzItk

The following date field parses successfully even though it's invalid:
"birthDate":"2000-15-31"

This should probably be an exception flat out.

@jamesagnew

This comment has been minimized.

Owner

jamesagnew commented Nov 12, 2014

From Joe:
Do you think there would be some way of holding on to the original pre type-coercion string content and then use that when transforming back to XML? Like Rails has this concept of pre type casting values that might be applicable here. I'm not sure how difficult this would be to do. Otherwise the parsing exception would probably work fine as well.

From Me:
It would actually be quite easy to do it that way- the primitive datatypes all have two pairs of methods:
get/setValue([type])
get/setValueAsString(String)

..where the first takes a type which is appropriate to the primitive in question (java.util.Date for DateTimeDt, java.math.BigDecimal for DecimalDt, etc.) and the second just takes a String.

The behaviour in all of these types right now is that the parser/encoders use the get/setValueAsString methods, but these methods actually all convert to the appropriate type right away. If we deferred that conversion until the first time getValue() was called, we would get exactly the behaviour you're describing (the weird date would be encoded back in it's weird form to XML, and the validator could catch it)

I see one pro and one con to this approach:
Pro- It provides the kind of validation you're describing. Actually as a second pro, I assume we'd see a small performance boost in applications that don't need the parsed Date value and only want the string, since with the current approach we parse and then re-encode if you only care about the string.
Con- If an invalid date string gets passed to the DateTimeDt (say someone passes the string "hello" instead of a date because of a bug in their code), you currently get an exception immediately which can be helpful in figuring out where bugs lie. The proposed fix defers that exception and makes bugs harder to track.
Pro- Actually, one more pro would be that it makes the parser a bit more resilient to bad data. Technically if you are using HAPI as a client and a server returned "hello" in a date string we'd flat out reject the response. With this we could still accept the response but give an exception when someone tried to access that value..

I think over the course of writing this I went from not liking this idea to liking it. :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment