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

Problem with fetching snapshot by commit id (from version 5.6.1) when using MongoDB #958

Closed
patpro1 opened this issue Apr 16, 2020 · 4 comments

Comments

@patpro1
Copy link

patpro1 commented Apr 16, 2020

Update from version 5.6.0 to 5.6.1 contains change in this line in MongoRepository.java (

.map(CommitId::valueAsNumber).collect(Collectors.toSet()));
):

.map(c -> c.valueAsNumber().doubleValue()).collect(Collectors.toSet()));

was changed to:

.map(CommitId::valueAsNumber).collect(Collectors.toSet()));

After that change when snapshot has commit id with minor version different than 0 (e.g. 372136.02) this snapshot is not fetched from repository by commit id - problem is in passing BigDecimal value to query instead of double value (with double value snapshot is returned without problems - checked by debugging on line

if (coreConfiguration.getCommitIdGenerator() == CommitIdGenerator.SYNCHRONIZED_SEQUENCE) {
)

I've added screens from debug between two version to explain the problem:

VERSION 5.6.0

Correct behavior of version 5 6 0

VERSION 5.6.1

Wrong behavior of version 5 6 1

@patpro1 patpro1 changed the title Problem with fetching Problem with fetching snapshot by commit id (from version 5.6.1) when using MongoDB Apr 16, 2020
@bartoszwalacik
Copy link
Member

@patpro1 don't paste your print-screens , provide a runnable test case as stated in Guidelines for Bug Reporting Guidelines for Bug Reporting

https://github.com/javers/javers/blob/master/CONTRIBUTING.md#guidelines-for-bug-reporting

@bartoszwalacik
Copy link
Member

Reopen if you want to contribute a PR or if you come up with a failing test case

@patpro1
Copy link
Author

patpro1 commented Jul 5, 2020

@bartoszwalacik I've pushed failing test to my fork:

patpro1@4ea9eac

I hope it's enough?

@bartoszwalacik
Copy link
Member

fixed in 5.10.4

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

2 participants