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

Skip persisting empty commits (with 0 snapshots) in SQL repository #505

Closed
mdii opened this Issue Mar 1, 2017 · 11 comments

Comments

Projects
None yet
2 participants
@mdii
Contributor

mdii commented Mar 1, 2017

Hi,

I am using JaVers 3.0.0 with Spring Boot. I noticed that new entries are stored in jv_commit even when there is no change in the object. Since these commits are not referenced from other tables, they are kind of useless.

@bartoszwalacik

This comment has been minimized.

Member

bartoszwalacik commented Mar 1, 2017

Hi, please follow the https://github.com/javers/javers/blob/master/CONTRIBUTING.md

Bug Reports have to contain:

- Clear description of your expectations versus reality
- Runnable test case which isolates the bug and allows us to easily reproduce it on our laptops. You can push this test case to your fork of this repository.
@mdii

This comment has been minimized.

Contributor

mdii commented Mar 6, 2017

As you can see in the following example, the second commit creates an entry in jv_commit and no entries in jv_snapshots. When we auto-audit repositories this leads to a lot of entries in jv_commit that are not referenced from other tables because usually we don't check if the entity has been updated before calling repository.save(entity).

Javers javers = JaversBuilder.javers().build();
Person robert = new Person("bob", "Robert Martin");
javers.commit("user", robert); 

//creates an entry in jv_commit even though nothing has changed on the object
javers.commit("user", robert); 

The expected behavior would be that commits are persisted only when the commit has snapshots.

Sorry, I couldn't come up with a runnable test case.

@bartoszwalacik

This comment has been minimized.

Member

bartoszwalacik commented Mar 7, 2017

You mean this is not OK that empty commits are persisted?

@mdii

This comment has been minimized.

Contributor

mdii commented Mar 8, 2017

Yes, exactly.

@bartoszwalacik

This comment has been minimized.

Member

bartoszwalacik commented Mar 9, 2017

Feel free to create a pull request. This issue is easy to implement so perfect for contribution.

@bartoszwalacik bartoszwalacik changed the title from New commit entries even when nothing has changed to Skip persisting empty commits in SQL repository (with 0 snapshots) Mar 9, 2017

@bartoszwalacik bartoszwalacik changed the title from Skip persisting empty commits in SQL repository (with 0 snapshots) to Skip persisting empty commits (with 0 snapshots) in SQL repository Mar 9, 2017

@mdii

This comment has been minimized.

Contributor

mdii commented Mar 10, 2017

I can create a pull request.
Should this be applied only to SQL repository or to all repositories?

@bartoszwalacik

This comment has been minimized.

Member

bartoszwalacik commented Mar 11, 2017

PR should be created from your fork. Looks like the problem could be solved in javerscore, before calling JaversRepository

mdii added a commit to mdii/javers that referenced this issue Mar 12, 2017

mdii added a commit to mdii/javers that referenced this issue Mar 12, 2017

@bartoszwalacik

This comment has been minimized.

Member

bartoszwalacik commented Mar 13, 2017

PR merged to master

@bartoszwalacik

This comment has been minimized.

Member

bartoszwalacik commented Mar 13, 2017

@mdii your commits are not bounded to your user. If you fix it, you will be listed on javers.org as a contributor

d81eb3c

@mdii

This comment has been minimized.

Contributor

mdii commented Mar 13, 2017

It's done, thanks!

@bartoszwalacik

This comment has been minimized.

Member

bartoszwalacik commented Mar 13, 2017

released in 3.0.4

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