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

ElementValueChange not using @Id annotation in Levenstein #167

Closed
tchegito opened this issue Jun 1, 2015 · 11 comments
Closed

ElementValueChange not using @Id annotation in Levenstein #167

tchegito opened this issue Jun 1, 2015 · 11 comments
Labels
bug

Comments

@tchegito
Copy link

@tchegito tchegito commented Jun 1, 2015

Hi people,

not sure that's an issue, maybe an enhancement, but I'm surprised of a different behavior between simple method and Levenshtein method for comparing.

I isolate it in a very simple usecase, which I resume here with words instead of code.
Consider two class A and B with each one an @id declared on a field. Class A contains a field which is a list of object B.
We create two instances A1 and A2 of A with same value I1 for Id. We create two instances B1 and B2 of B with same value I2 for Id. We assign a singleton list of B1 to object A1. And the same for A2 with a singleton list of B2.

Now, let's compare.

Simple method returns no differences at all.
And Levenshtein returns one difference of type ElementValueChange. I check the code, and that's because of this method calls 'equals' method which I haven't defined. Okay. But why this method can't compare fields defined with @id ?

Thanks for help. And by the way, this is a great library !

@bartoszwalacik
Copy link
Member

@bartoszwalacik bartoszwalacik commented Jun 1, 2015

thanks @tchegito, well, both list comparing algorithms should produce similar results.
If you have a list of objects with @id - you have the list of Entities.
In this case, entities should be compared only by Id.

There could be a bug in Levenshtein. Could you help us and isolate this case to the new, failing test, pushed to your forked JaVers github repository?

@tchegito
Copy link
Author

@tchegito tchegito commented Jun 2, 2015

Hi Bartosz

Yes I can do that ! I'll try today, but you will see, this is very simple.
I hope my report isn't confused by my whole process (this may be), so I'll
make it simplest as possible.

Thank you for quick answer

On Mon, Jun 1, 2015 at 11:14 PM, Bartosz Walacik notifications@github.com
wrote:

thanks @tchegito https://github.com/tchegito, well, both list comparing
algorithms should produce similar results.
If you have a list of objects with @id https://github.com/Id - you have
the list of Entities.
In this case, entities should be compared only by Id.

There could be a bug in Levenshtein. Could you help us and isolate this
case to the new, failing test, pushed to your forked JaVers github
repository?


Reply to this email directly or view it on GitHub
#167 (comment).

@bartoszwalacik
Copy link
Member

@bartoszwalacik bartoszwalacik commented Jun 3, 2015

ok, I am waiting for the isolated test

@tchegito
Copy link
Author

@tchegito tchegito commented Jun 3, 2015

Hi Bartosz

I just pushed unit test in my forked repo here:
https://github.com/tchegito/javers

On Wed, Jun 3, 2015 at 9:11 AM, Bartosz Walacik notifications@github.com
wrote:

ok, I am waiting for the isolated test


Reply to this email directly or view it on GitHub
#167 (comment).

@bartoszwalacik
Copy link
Member

@bartoszwalacik bartoszwalacik commented Jun 3, 2015

ok, I'll come back to it after weekend

@bartoszwalacik bartoszwalacik added bug and removed question labels Jun 3, 2015
@tchegito
Copy link
Author

@tchegito tchegito commented Jun 8, 2015

Hi Bartosz,

first of all, sorry to reuse this mail about another question, but I don't
have other one.

Using Javers, my main goal was to :

  1. analyse diff between 2 objects
    ==> it's ok
  2. propagate these diff from the first object
    ==> I get trouble there

The main problem I encounter is: how reflect the changes detected on the
first element ? I tried many approaches:
-change your source code to store a reference to the original node,
(because you only provide "right" node) in each kind of Change (this is
huge, and I don't really like fork your work, that suggests I'm using it
the wrong way)
-commit changes on subpart of the graph and retrieve toplevel by the method
you provided there : #133

And my last idea was to iterate over the graph, looking for an object with
given GlobalId: Class and Id. But I'm sure there have to be a way, using
your code to do that. I would rather avoid to reinvent the wheel, starting
with BeanUtils.

Not sure this is really clear, but your help would be really appreciated !

Thank you

On Wed, Jun 3, 2015 at 10:30 PM, Bartosz Walacik notifications@github.com
wrote:

ok, I'll come back to it after weekend


Reply to this email directly or view it on GitHub
#167 (comment).

@bartoszwalacik
Copy link
Member

@bartoszwalacik bartoszwalacik commented Jun 8, 2015

Hi,
what do you mean by propagate? Your goal is to synchronize both objects by aplying changes calculated by diff to one of those objects (left I guess)?

@tchegito
Copy link
Author

@tchegito tchegito commented Jun 9, 2015

Yes you're right !

The goal is to synchronize graph objects between two databases.

On Mon, Jun 8, 2015 at 10:36 PM, Bartosz Walacik notifications@github.com
wrote:

Hi,
what do you mean by propagate? Your goal is to synchronize both objects by
aplying changes calculated by diff to one of those objects (left I guess)?


Reply to this email directly or view it on GitHub
#167 (comment).

@bartoszwalacik
Copy link
Member

@bartoszwalacik bartoszwalacik commented Jun 9, 2015

So having the left reference would solve your issue?
What about this 'apply changes' feature? It could be useful. For now I don't have time to write it but I can accept PR.
Using javers-core engine (Cdo abstraction) it shouldn't be very hard to implement.

bartoszwalacik added a commit that referenced this issue Jun 11, 2015
@bartoszwalacik
Copy link
Member

@bartoszwalacik bartoszwalacik commented Jun 12, 2015

fixed in 1.2.10

@tchegito
Copy link
Author

@tchegito tchegito commented Jun 12, 2015

Thanks Bartosz !

On Fri, Jun 12, 2015 at 10:29 AM, Bartosz Walacik notifications@github.com
wrote:

Closed #167 #167.


Reply to this email directly or view it on GitHub
#167 (comment).

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

Successfully merging a pull request may close this issue.

None yet
2 participants
You can’t perform that action at this time.