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

overrideStandardValueWriter only applied to first java.nio.file.Path valued field of bean #112

Closed
jhonnen opened this issue Feb 14, 2024 · 4 comments
Labels

Comments

@jhonnen
Copy link

jhonnen commented Feb 14, 2024

The overrideStandardValueWriter mechanism seems to work only for the first field of the given type for a Bean. Here's a failing test case that demonstrates the issue (can be added to ValueWriterModifierTest):

public void testMultipleFields() throws Exception {

    TestBean input = new TestBean();
    input.p1 = Paths.get("some/path");
    input.p2 = Paths.get("some/other/path");

    String json = JSON.builder()
            .register(new JacksonJrExtension() {
                @Override
                protected void register(ExtensionContext ctxt) {
                    ctxt.insertModifier(new ReaderWriterModifier() {
                        @Override
                        public ValueWriter overrideStandardValueWriter(JSONWriter writeContext, Class<?> type, int stdTypeId) {
                            if (type == Path.class) {
                                return new PathWriter();
                            }
                            return super.overrideStandardValueWriter(writeContext, type, stdTypeId);
                        }

                    });

                    ctxt.insertProvider(new ReaderWriterProvider() {
                        @Override
                        public ValueWriter findValueWriter(JSONWriter writeContext, Class<?> type) {
                            if (Path.class.isAssignableFrom(type)) {
                                return new PathWriter();
                            }
                            return super.findValueWriter(writeContext, type);
                        }
                    });
                }
            }).build().asString(input);
    assertEquals(aposToQuotes("{'p1':'some/path','p2':'some/other/path'}"), json);
}

private static class TestBean {
    public Path p1;
    public Path p2;
}

private static class PathWriter implements ValueWriter {
    @Override
    public void writeValue(JSONWriter context, JsonGenerator g, Object value) throws IOException {
        g.writeString(((Path) value).toString().replace(File.separatorChar, '/'));
    }

    @Override
    public Class<?> valueType() {
        return Path.class;
    }
}

The test fails with

junit.framework.ComparisonFailure: 
Expected :{"p1":"some/path","p2":"some/other/path"}
Actual   :{"p1":"some/path","p2":["some","other","path"]}

--> on the second Path field, the Iterable serialization wrongly has precedence over my configured writer.

@cowtowncoder
Copy link
Member

@jhonnen That does sound like an interesting bug. Thank you for reporting!

cowtowncoder added a commit that referenced this issue Feb 16, 2024
@cowtowncoder
Copy link
Member

I wonder if this has something to do with the fact Path is Iterable. Should not matter of course, just noting.

@cowtowncoder cowtowncoder changed the title overrideStandardValueWriter only applied to first field of bean overrideStandardValueWriter only applied to first java.nio.file.Path valued field of bean Feb 17, 2024
@cowtowncoder
Copy link
Member

Yes, somehow detection as SER_ITERABLE caused the problem. I worked around it by exclusion so registration should work, but I think as a follow-up I will:

  1. Try to see if custom serializer overrides for "simple" (i.e. recognized) types have this problem more generally (and if so, how to fix)
  2. Add explicit read/write support for java.nio.file.Path, out of the box

cowtowncoder added a commit that referenced this issue Feb 17, 2024
…alizer

Fix #112: make custom serialization work for `java.nio.file.Path`
@cowtowncoder
Copy link
Member

Ok, looks like there is a problem that is generalizable for "simple" (recognized and supported JDK types) types like String, too. So need to figure this out.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants