Skip to content

IonValueMapper.builder() not implemented, does not register modules #509

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

Closed
mr-robert opened this issue Aug 13, 2024 · 4 comments
Closed

Comments

@mr-robert
Copy link

mr-robert commented Aug 13, 2024

(follow-up from #497)

Using Jackson-dataformat-ion 2.17

When constructing a new IonValueMapper using constructor new IonValueMapper(onSystem) modules are registered:

Constructor code:

    public IonValueMapper(IonSystem ionSystem, PropertyNamingStrategy strategy) {
        super(new IonFactory((ObjectCodec)null, ionSystem));
        this.registerModule(new IonValueModule());
        this.registerModule(new EnumAsIonSymbolModule());
        this.setPropertyNamingStrategy(strategy);
    }

However, when constructing using builder:
IonValueMapper.builder(ionSystem).build()

the modules are not registered, ultimately by chain of method calls, the following code is run in IonObjectMapper.class :

    public static Builder builder(IonFactory streamFactory) {
        return new Builder(new IonObjectMapper(streamFactory));
    }

this creates an IonObjectMapper , without registering IonValueModule or EnumAsIonSymbolModule.

I think that the correct/expected behavior should be that using IonValueMapper.builder(...) should ultimately create a mapper which is the same as using constructor with the same parameters. Therefore it seems that there may need to be some changes or additional builder overrides added in IonValueMapper.

Workaround for this is just to not use builder to create IonValueMapper , or manually registering the modules.

@cowtowncoder
Copy link
Member

Ok this is bit tricky, due to static methods etc. But I will try to cobble something together.
For Jackson 3.0 IonValueMapper is deprecated, replaced by proper builder of IonObjectMapper, fwtw.

@cowtowncoder
Copy link
Member

@mr-robert Did pr #510, merged; not confident it fully works but if you have a chance maybe you could have a look (2.18.0-SNAPSHOT should be built by CI, or build locally)?

@rnoack1
Copy link

rnoack1 commented Feb 3, 2025

Hi,

Wondering if you could explain this comment:

For Jackson 3.0 IonValueMapper is deprecated, replaced by proper builder of IonObjectMapper, fwtw.

What is the "proper" builder in IonObjectMapper which corresponds to IonValueMapper, and does this builder exist in 2.18.x or only in upcoming 3.x Jackson?

@cowtowncoder
Copy link
Member

It would be via builder-style construction, which does exist in 2.x branch too:

    IonMapper mapper = IonMapper.builder() // or 'builder(IonFactory f)`
                 // possible configuration and then
                .build();

in 3.0 IonMapper also has relevant readValue(IonValue, targetType) calls.
Actually, looking at 2.19 it too has these methods. But 2.19 has builders for IonValueMapper as well.

But overall, 3.0 will remove need for (and use of) IonValueMapper. I don't remember details of if and why it's still needed for 2.18 and later.

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

No branches or pull requests

3 participants