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

Disable the Number to String automatic conversion #796

Closed
fabianpiau opened this issue May 19, 2015 · 23 comments
Closed

Disable the Number to String automatic conversion #796

fabianpiau opened this issue May 19, 2015 · 23 comments

Comments

@fabianpiau
Copy link

By default, Jackson is converting numbers to strings. In most cases, that's fine. But not in my situation unfortunately. Is there a way to prevent that?

For example:

JSON

{
"numberAsString": 123
}

JAVA

private String numberAsString;

It works just fine. But the thing is that I want this automatic boxing not enabled and got an exception instead. So the user is forced to use a String. I.e.

{
"numberAsString": "123"
}

I don't think such a feature exist, I look on http://wiki.fasterxml.com/JacksonFeaturesGenerator and a bit everywhere like http://stackoverflow.com/questions/7806316/jackson-json-converts-integers-into-strings

I tried to create a custom deserializer, but it seems that the value is already converted as a string when I got to the deserializer, and I don't have the possibility to determine if it was a string or an integer originally in the request.

Thanks

Fabian

@cowtowncoder
Copy link
Member

What deserializer sees, literally, for:

{
"numberAsString": 123
}```

is a sequence of events:

START_OBJECT, FIELD_NAME, VALUE_NUMBER_INT, END_OBJECT


or, rather, that is the whole document. At point where deserializer for property `numberAsString` is invoked, it will see `JsonToken.VALUE_NUMBER_INT`. No conversion is performed by `JsonParser` unless explicitly requested.

In your case, you would probably want to directly specify the deserializer for property with `@JsonDeserialize(using=MyNumberDeserializer.class)`, although if you wanted it to be used globally, you could instead register it by creating `SimpleModule`, calling its `addDeserializer()` method, then doing 'mapper.registerModule(myModule);

@fabianpiau
Copy link
Author

Great! It is working. I now have my deserializer like this:

public class ForceStringDeserializer extends JsonDeserializer<String> {

    @Override
    public String deserialize(JsonParser jsonParser, DeserializationContext deserializationContext) throws IOException {
        if (jsonParser.getCurrentToken() == JsonToken.VALUE_NUMBER_INT) {
            throw deserializationContext.wrongTokenException(jsonParser, JsonToken.VALUE_STRING, "Attempted to parse int to string but this is forbidden");
        }
        return jsonParser.getValueAsString();
    }
}

Like you were saying, a module is more convenient to have a global behavior, instead of annotate many fields. So I created a simple module, but it doesn't work in all cases.

{
"numberAsString": 123
}

This is fine but if I have nested objects, it doesn't go in the deserializer provided by the module.

{
 "myObject": {
"numberAsString": 123
}
}

I guess it is because "myObject" is not a String. Is that correct?

@fabianpiau
Copy link
Author

After some experiments, it is definitely because myObject is not a string. I created a module for MyObject type and I went to the associated deserializer.

My second question is:
Is it possible to create a generic simple module to be applied to ALL the properties of any objects that are of a specific type (in my case string)? (can be a nested property or not)

@cowtowncoder
Copy link
Member

No, that should not matter. Registered serializer is bound to type, not location, and should work for all fields declared with type String. On the other hand, if type was something else, like Object, different deserializer is used.

One note on deserializer: perhaps check should rather verify that current token is of type JsonToken.VALUE_STRING. Unless you want to allow true and false to be accepted, or floating point numbers (VALUE_NUMBER_FLOAT), or Objects/Arrays.

@fabianpiau
Copy link
Author

Yes I want to allow false and true, actually I wanted to force only for the numbers. The thing is that when you have a number starting with 0, it is not working because of the leading 0. I wanted to make consistent and fail for every numbers, and force the user to use String instead.

For the module, I got something like that:

module.addDeserializer(String.class, new ForceStringDeserializer())

which is working fine for the first case. But not in the second case with the inner string.

I tried:

module.addDeserializer(String.class, new ForceStringDeserializer())
module.addDeserializer(MyObject.class, new ForceMyObjectDeserializer())

And I was getting in my second deserializer for the second case. But that's not very interesting as I will have to create as many deserializers as I have objects, in that case I prefer to duplicate the annotations...

I am pretty sure, I am missing something, maybe you have an idea.
Thanks for your time anyway.

@cowtowncoder
Copy link
Member

@fabianpiau What I really need at this point are two things:

  1. Class MyObject that contains String value for which deserializer is not used and
  2. Code that invokes ObjectMappers readValue() method

since behavior as you describe does not make sense. Globally registered deserializer should be applied both for "root value" and transitive property values.

@fabianpiau
Copy link
Author

The first one is pretty simple. A POJO MyObject with a String field.
For the second one, sorry I did not give you the whole context. I am using Dropwizard, so the binding is automatic.

@cowtowncoder
Copy link
Member

Unfortunately I can not reproduce the issue. I can run test:

    static class UCStringDeserializer extends StdDeserializer<String> {
        public UCStringDeserializer() { super(String.class); }

        @Override
        public String deserialize(JsonParser p, DeserializationContext ctxt)
                throws IOException {
            return p.getText().toUpperCase();
        }
    }

    protected static class StringWrapper {
        public String str;

        public StringWrapper() { }
        public StringWrapper(String value) {
            str = value;
        }
    }

  public void testCustomStringDeser() throws Exception
  {

        ObjectMapper mapper = new ObjectMapper().registerModule(
                new SimpleModule().addDeserializer(String.class, new UCStringDeserializer())
                );
        assertEquals("FOO", mapper.readValue(quote("foo"), String.class));
        StringWrapper sw = mapper.readValue("{\"str\":\"foo\"}", StringWrapper.class);
        assertNotNull(sw);
        assertEquals("FOO", sw.str);
  }

This is with 2.6.0-rc1, but should work on any 2.x version.

So there is something special about MyObject; either definition itself, or maybe there's a custom deserializer for it? (custom deserializers must properly delegate handling of values of properties and not directly read Strings -- otherwise they would by-pass overrides).

@fabianpiau
Copy link
Author

Ok, I know why it is working in the case there are no inner properties. Actually, you're right, there is no relation with the fact there are inner properties.

I was starting to do some copy/paste then I saw a difference between the working case and the not working one.

public class MyObjectWorking {

    private String numberAsString;

    @JsonCreator
    public MyObjectWorking (String numberAsString) {
        this.numberAsString = numberAsString;
    }

    // Getter and setter
}

The constructor with the JsonCreator annotation is the difference. I removed it, and I was not going in the custom deserializer anymore. In my other object (more complex with a nested object), I don't have a constructor.

Do you know how to make it work without having to create a constructor? I try to add a constructor for my second object, but then I have other issue. ! java.lang.IllegalArgumentException: Argument #0 of constructor.

@cowtowncoder
Copy link
Member

I think at this point full example would make most sense, since I get confused with kinds of cases that work, and with what JSON.

However, I suspect the difference here is between two constructors:

   @JsonCreator // explicitly delegating, calls registered String deserializer, then passes String
   public POJO(String str) { ... }

    // Implicit constructor, String passed as-is directly, no deserializer
   public POJO2(String str) {   }

As to whether second case should go through deserializer or not.... I am not sure.
It is an older feature than @JsonCreator, and only works for int, long, String and boolean.
But given that it has been there for so long (since 1.1 or even 1.0), I am hesitant to change that behavior. Even though in a way you could think of second form as a short-cut for first. Except that it does not work identically.

@fabianpiau
Copy link
Author

I remove the @JsonCreator annotation on the constructor (of the working example). And it still works. I am not sure whether this annotation is really used or not, I prefer to keep it in case of regression.

At the end, I will keep the duplicated annotations on a couple of fields, I don't want to spend too much time on this.

Thanks for your help :)

@cowtowncoder
Copy link
Member

@fabianpiau Makes sense to me.

@sanjay1920
Copy link

@fabianpiau
I think your resolution fits best to my requirement but where to write ForceStringDeserializer class in a huge code base

@martin-mucha
Copy link

thanks for this discussion and provided "workaround". But this should really be DeserializationFeature. Automatic type conversion are evil and as such belongs to javascript. It should have never been on turned on by default, and thus there should be easy way to turn it off at least. It's kinda weird have to write ForceStringDeserializer into every project. Can someone reconsider adding it?

@joca-bt
Copy link

joca-bt commented Jun 13, 2019

I also have this issue. This should be a feature, something like ACCEPT_NUMBER_AS_STRING similarly to the already existing ACCEPT_FLOAT_AS_INT.

I also have other issues, for example, if the field is a boolean, "true" gets converted to true, which I would like to disable. Basically, I need to disable all automatic basic type conversion between fields.

What's the best way of doing this?

EDIT: http://fasterxml.github.io/jackson-databind/javadoc/2.9/com/fasterxml/jackson/databind/MapperFeature.html#ALLOW_COERCION_OF_SCALARS seems to do the trick.

@cowtowncoder
Copy link
Member

@martin-mucha If I want to hear a rant on your personal opinion on things, I'll be sure to ask. Until then, please try to stick to facts, and write blog posts somewhere else. All of coercions have been added by user requests and contributions, so while I definitely agree that they should and need to be configurable (and well documented), there is strong demand for more permissive acceptance of values.

This is especially because your client may well be written in Javascript or Perl or one of many, many languages (and libraries, frameworks), with various limitations on kinds of things they can or will produce.

@cowtowncoder
Copy link
Member

From this, most I am going to say is that I hope to have time to refine this: the problem is that adding tons of (De)SerializationFeatures is not scalable from dev/use perspective. But figuring out good API for this is not trivial: possibly another Feature type (on/off) is warranted: but even that may be simplistic.

@alfonz19
Copy link

@cowtowncoder Please take it easy. There's no point in arguing about one sentence, I wrote it, sorry, next time I will be careful.

I just asked to reconsider to have this possibility readily available for everyone. I have this 'turning off of unwanted features' in all my projects already, this might just be useful for others as well. Java is typically more strict language and users do want to have strong validations. Some environments do not tolerate lenient behavior.

@joca-bt
Copy link

joca-bt commented Jun 17, 2019

I have similar constraints. Something like ALLOW_COERCION_OF_SCALARS but applied to strings would be great!

@Blacklands
Copy link

I would also be interested in this feature.
Making custom (de)serializers for a large number of objects adds up to quite a bit of work, and it's especially annoying when you have to do it only for the purpose of getting more strict typing.

@cowtowncoder
Copy link
Member

#2113 to be released in 2.12.0 should allow strict handling of many of use cases.
It is not possible to have just one setting due to existing behavior that is scattered around functionality, unfortunately. So it is necessary to specify cases where features are needed -- and for that new issues are needed.

@ZengruiWang
Copy link

Hi, I've upgraded to 2.12.1 and I was able to use coercionConfigFor to prevent the integer to boolean conversion. But I'm having a hard time using it to prevent the number to string conversion.

I have a similar setting as below

this.mapper.coercionConfigFor(LogicalType.Textual)
  .setCoercion(CoercionInputShape.Float, CoercionAction.Fail);

The unit test below failed

@Test
void testDeserializeFloatToStringFailure() {
    assertThrows(InvalidFormatException.class, 
    () -> Mapper.JSON.mapper().readValue("{\"test-str\":1.1}", TestEntity.class));
}

Has the case above been supported yet? Thanks!

@cowtowncoder
Copy link
Member

@ZengruiWang there is a new open issue, #3013 that covers this I think. Short answer is that your usage is correct but existing deserializers do not yet check for this particular coercion. They should.

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

8 participants