Open garretwilson opened 1 year ago
Looks like this is especially important, because the default implementation will find a private constructor accepting a String
using AnnotatedConstructor
from StdValueInstantiator.createFromString(DeserializationContext ctxt, String value)
:
public Object createFromString(DeserializationContext ctxt, String value) throws IOException
{
if (_fromStringCreator != null) {
try {
return _fromStringCreator.call1(value);
} catch (Exception t) {
…
A user may not realize that this internal constructor is being called, because it seemingly "just works". Unfortunately for my particular MyValueType
, there will be tens of thousands of instances of the exact same value being represented. (This is typical of value classes.) So instead of calling new MyValueType("foo")
I need to call MyValueType.from("foo")
.
Because there is no easy way to register value types with ObjectMapper
, it may be that many users are having huge unnecessary memory bloat with their custom value classes, because the code works and they didn't know they needed to register a deserializer for the value type factory method (and wouldn't readily know how anyway, and wouldn't have time to research it over days as I am).
I think such a facility definitely needs to be added and called out in the documentation.
you can mark the factory method as a JsonCreator (with a mixin if necessary)
you can mark the factory method as a JsonCreator (with a mixin if necessary)
Thanks, but the whole point of this exercise was to configure the mapper without modifying the class itself.
then use a mixin?
then use a mixin?
Why should I have to create a completely new class?
Why can't there be an existing class I can simply instantiate, telling it the name of my factory method, or providing it with a method reference?
I know there are workarounds. This ticket was opened so that I wouldn't have to use a workaround.
if you dont want to add a mixin for some reason (e.g. because the name of the method is dynamic), you can instead add a value instantiator.
if you dont want to add a mixin for some reason …
I don't want to add a mixin because I don't want to write an extra class to go along with every value class I use, and I prefer not to litter the source code of the value class with serialization-specific annotations.
I don't know why this request is difficult to understand. I can do this to serialize a class to a string:
…
module.addSerializer(Bar.class, new ToStringSerializer());
…
I want to do the analogous thing to register a deserializer for my value class, indicating the factory method to use.
Do you agree that ToStringSerializer
is useful, or do you think it should be removed and that we should be forced to write a mixin class for every Foo
and Bar
value class we create? If you agree ToStringSerializer
is useful, I don't understand why you don't think the analogous deserializer helper is appropriate.
I'm even offering to write this class. I just want to make sure Tatu or someone will accept it so I don't waste my time doing it as part of the existing Jackson project.
@garretwilson Tatu is on vacation. Can you wait a few weeks?
Can you wait a few weeks?
Oh, sure, no problem at all! Believe me I have a thousand things ahead of this anyway on my list of things to do.
For the moment I have this in my source code:
//TODO create utility class; see https://github.com/FasterXML/jackson-databind/issues/4004
.addDeserializer(Bar.class, new JsonDeserializer<Bar>() {
@Override
public Bar deserialize(JsonParser parser, DeserializationContext context) throws IOException, JacksonException {
return Bar.of(parser.getValueAsString());
}
})
No rush on this—just wanted to file this while I was thinking about it.
But now how about deserializing Bar using a static factory method?
I believe you can declare a static method named either valueOf
or fromString
, and Jackson will automagically use that for deserialization:
public class Bar {
...
public static Bar valueOf(String stringVal) {
...
}
}
// or...
public class Bar {
...
public static Bar fromString(String stringVal) {
...
}
}
I believe you can declare a static method named either
valueOf
orfromString
, and Jackson will automagically use that for deserialization:
Thanks, but the static factory method of my value class has a different name.
And since you brought this up … 😁
I realize that at the time Jackson was written, valueOf()
and fromString()
were the convention for value class factory methods (e.g. Integer.valueOf(int)
). Value classes at the time were relatively rare (remember AWT Insets
😱 and even Date
?) and not well understood in the community as such; see for example the note on Value-based classes in the JDK 8 which reflects a growing awareness.
However nowadays value classes/objects are better understood (see Fowler's ValueObject for an overview) and many newer languages include facilities for value classes built-in. The naming convention is evolving merely to use of(…)
when referring to to components (e.g. Set.of(…)
) or parse(CharSequence)
when converting from canonical lexical form (e.g. Instant.parse(CharSequence)
).
In essence Jackson, in recognizing valueOf()
and fromString()
as special, is stuck on an older convention which was still evolving. (I'm not criticizing Jackson in this regard, unless it would be to suggest revisiting these conventions or making them pluggable, which I guess is what this ticket is to some extent.) I certainly wouldn't want to design my classes around those rules of yesteryear.
P.S. I was just looking over the lesson I wrote on value objects around 10 years ago, and interestingly I find this sentence:
Modern factory methods commonly use the name
of(…)
rather thanvalueOf(…)
.
Sure, everybody has got their own preferences. Just letting you know such facility exists.
Of course I can write one of these myself, but I'm curious whether this exists already. Basically I'd like to do this... I'm even offering to write this class. I just want to make sure Tatu or someone will accept it so I don't waste my time doing it as part of the existing Jackson project.
Now we know that thing you preferred doesn't exist in Jackson out-of-the-box. So whether the Jackson core maintainers want to accept it into the codebase or not, you'd still have to create it anyway to satisfy your personal preference, right? No reason to wait time to get cracking good luck. 👍
Ok I am back and... I think it makes sense to offer easier means for adding basic closure-based deserializer(s), maybe just (or at first) for things that accept JSON Strings, like more advanced FromStringDeserializer
.
It would be given supported type (like Locale.class
) and then value-accessor Function
. Or something along those lines.
And as suggested we could/should add convenience method(s) in SimpleModule
if that's helpful.
... that is, yes, exactly like you said.
Implementation should probably also support CoercionConfig
to allow binding from other JSON scalar types, like numbers -- I think FromStringDeserializer
already does that I guess.
We could perhaps extend FromStringDeserializer
as FunctionalFromStringDeserializer
, implement _deserialize(...)
via Function
evaluation.
Maybe we should make ValueInstantiator easier to implement (with a lambda?) and create a way to add it without a Module similar to (de)serializers.
While ValueInstantiator
is a possibility, addition of methods in Module
is just syntactic sugar and could be left out & just offer convenience deserializer(s).
As I noted on Stack Overflow, I'm using Java 17 and I have a
Bar
type I want serialized to JSON using itstoString()
method. I'm serializing using a JacksonObjectMapper
(created via aJsonMapper.Builder
). I found out from my other Stack Overflow question that I can specify aToStringSerializer
as a serializer in aModule
, as the answer indicated. (I had done this years ago, but forgotten how.)But now how about deserializing
Bar
using a static factory method? Looking at the code fromFromStringDeserializer.Std._deserialize(…)
, I see that known types are simply checked using acase
and then the static factory methods are explicitly invoked, e.g.Charset.forName(value)
orURI.create(value)
. This indicates to me that there may not be an existing deserializer for general serializer types—otherwise we could just usenew StaticFactoryDeserializer("forName")
andnew StaticFactoryDeserializer("create")
, etc. to do the same thing.Of course I can write one of these myself, but I'm curious whether this exists already. Basically I'd like to do this:
StaticFactoryDeserializer
would also allow aFunction
, so I could use it like this:(Probably the deserializer would create its own function instance internally when created with the method name, but that's an implementation detail.)
Does such a deserializer exist, or is one planned? If I wrote one, would it be something you'd be interested in including in Jackson?