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

add value reader for records #715

Closed

Conversation

okaponta
Copy link

@okaponta okaponta commented May 2, 2023

#546

support Record -> POJO

POJO -> Record is unimplemented, see below for details.
#546 (comment)

@yangyaofei
Copy link

Hi, I found bug here.

class Test{
    UUID userId;
}

record TestDto(UUID userId){}

Test test = modelMapper.map(new TestDto(UUID.randomUUID()), Test.class)

In this code, the test.userId is null.

@WatchdataGitHub
Copy link

WatchdataGitHub commented Jul 11, 2023 via email

@yangyaofei
Copy link

I got solution.

    @Override
    public Member<Record> getMember(Record record, String memberName) {
        Field field = matchField(record, memberName);
        Class<?> type = field != null ? field.getType() : Record.class;
        return new Member<>(type) {
            @Override
            public Object get(Record source, String memberName) {
                return RecordValueReader.this.get(source, memberName);
            }
        };
    }

You use field.getClass() which is wrong, and I don't think it's necessary to return Record.class when null.

@okaponta
Copy link
Author

@yangyaofei
Thanks for reporting and the solution. fixed in commit e7a2538

@okaponta
Copy link
Author

CI failed, but record doesn't exist in java8, so this would not success.
We have to wait until ModelMapper supports only version upper than java14.

@chhsiao90
Copy link
Member

There're still a lot of users using java 8, so upgrade java version is not an option. How about creating separated module?

@okaponta
Copy link
Author

means using modelMapper.registerModule() ?

actually, I found something like that. Maybe you can use this extension...
https://github.com/modelmapper/modelmapper/blob/master/extensions/jooq/src/main/java/org/modelmapper/jooq/RecordValueReader.java

@yangyaofei
Copy link

@okaponta

ModelMapper modelMapper = new ModelMapper();
modelMapper.getConfiguration().addValueReader(new RecordValueReader());

I use your code like this. It's good.

@okaponta
Copy link
Author

@yangyaofei
thanks, I also used it like this.

@Configuration
public class JavaConfig {

    @Bean
    public ModelMapper modelMapper() {
        ModelMapper mapper = new ModelMapper();
        mapper.getConfiguration().addValueReader(new RecordValueReader());
        return mapper;
    }
}

How about creating separated module?

actually, I'm not sure what specific actions to take based on the feedback provided.
I would appreciate it if someone could help me understand what exactly I should do.

@chhsiao90
Copy link
Member

Thanks @okaponta for the contribution. Moved the code into separated repository https://github.com/modelmapper/modelmapper-module-record and I also had a released org.modelmapper:modelmapper-module-record:1.0.0 and it should be available soon.

@chhsiao90 chhsiao90 closed this Jul 14, 2023
@okaponta okaponta deleted the feature/add-record-value-reader branch July 18, 2023 04:04
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

Successfully merging this pull request may close these issues.

None yet

4 participants