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

CBORParser seems to promote float to double #32

Closed
richardstartin opened this Issue Nov 12, 2016 · 5 comments

Comments

Projects
None yet
2 participants
@richardstartin

richardstartin commented Nov 12, 2016

I want to use Jackson CBOR because the CBOR RFC has self describing types and distinguishes between different sizes of floating point and integral numbers. I want to use this feature of the format specification to infer a schema from collections of CBOR messages.

By default, the CBORGenerator will write longs < Integer.MAX_VALUE as ints, but this can be disabled.

CBORGenerator writes floats and doubles properly. CBORParser correctly identifies floats as floats (nextToken case 26) but constructs a double nevertheless. This seems to be because JsonToken in Jackson core does not differentiate between floats and doubles.

Here's a simple test to demonstrate the issue:

  public void testPrimitiveTypeInvariance() throws IOException {
    ObjectMapper mapper = new ObjectMapper(
            new CBORFactory().disable(CBORGenerator.Feature.WRITE_MINIMAL_INTS)
    );
    Map<String, Object> map = ImmutableMap.of("longField", 1L, "intField", 1, "doubleField", 1.0, "floatField", 1.0f);
    byte[] json = mapper.writeValueAsBytes(map);
    Map<String, Object> fromCbor = mapper.readerFor(Map.class).readValue(json);
    test(fromCbor);
  }

  private void test(Map<String, Object> map) {
    Assert.assertTrue("long not preserved: " + className(map.get("longField")), map.get("longField") instanceof Long);
    Assert.assertTrue("int not preserved: " + className(map.get("intField")), map.get("intField") instanceof Integer);
    Assert.assertTrue("double not preserved: " + className(map.get("doubleField")), map.get("doubleField") instanceof Double);
    Assert.assertTrue("float not preserved: " + className(map.get("floatField")), map.get("floatField") instanceof Float);
  }


  private String className(Object o) {
    if(null == o) return "null";
    return o.getClass().getCanonicalName();
  }

The version of jackson-dataformat-cbor is 2.8.3.

@cowtowncoder

This comment has been minimized.

Show comment
Hide comment
@cowtowncoder

cowtowncoder Nov 14, 2016

Member

This sounds familiar. Issue #28 was for Avro backend, and for NumberNode.

Thank you for reporting the problem. It should be solvable since although same token is used for both, underlying physical type can be determined (via getNumberType()).

Member

cowtowncoder commented Nov 14, 2016

This sounds familiar. Issue #28 was for Avro backend, and for NumberNode.

Thank you for reporting the problem. It should be solvable since although same token is used for both, underlying physical type can be determined (via getNumberType()).

@cowtowncoder

This comment has been minimized.

Show comment
Hide comment
@cowtowncoder

cowtowncoder Nov 16, 2016

Member

Looks like this is a problem with jackson-databind; need to create an issue there, to be fixed in 2.8.6 (2.8.5 was just released).

Member

cowtowncoder commented Nov 16, 2016

Looks like this is a problem with jackson-databind; need to create an issue there, to be fixed in 2.8.6 (2.8.5 was just released).

@richardstartin

This comment has been minimized.

Show comment
Hide comment
@richardstartin

richardstartin Nov 16, 2016

Nice work. Would it be helpful if I verify somehow?

richardstartin commented Nov 16, 2016

Nice work. Would it be helpful if I verify somehow?

@cowtowncoder

This comment has been minimized.

Show comment
Hide comment
@cowtowncoder

cowtowncoder Nov 16, 2016

Member

Sure. Fix is in for jackson-databind, branch 2.8 (and master too), so as long as you compile locally (or use Snapshot via sonatype snapshot repo) you should be able to verify.

Member

cowtowncoder commented Nov 16, 2016

Sure. Fix is in for jackson-databind, branch 2.8 (and master too), so as long as you compile locally (or use Snapshot via sonatype snapshot repo) you should be able to verify.

@cowtowncoder cowtowncoder added this to the 2.8.6 milestone Dec 8, 2016

cowtowncoder added a commit that referenced this issue Dec 8, 2016

@cowtowncoder

This comment has been minimized.

Show comment
Hide comment
@cowtowncoder

cowtowncoder Dec 8, 2016

Member

Added a test in master, passes.

Member

cowtowncoder commented Dec 8, 2016

Added a test in master, passes.

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