Question about categories in DiffNode

[scompo]

Hi, I've succesfully usedObjectDifferBuilder to configure exclusion with categories, only the children with the specified categories are returned to the Visitorby the visitChildrenmethod of DiffNode.
I'm pretty happy with it cause it helps dealing with only parts of the object I'm diffing and merging I'm interested in.
I've noticed that calling DiffNode.getCategories() returns an empty Set, is this the expected beheaviour?

[SQiShER]

Yes, but I agree that's a bit confusing. The DiffNode only returns the categories discovered during introspection (from the annotation). But more can be assigned to specific nodes at runtime via configuration API. Internally the ObjectDiffer will look at both, the compile time categories and the runtime categories and make a decision based on all of them.

The DiffNode doesn't represent the whole truth, because I didn't want to introduce a dependency to the category resolver mechanism. However, I'm not opposed to the idea of having the DiffNode represent the actual categories. It just wasn't high enough on my todo list, to look for possible solutions. It may turn out to be very easy to do. Or it may be super hard. Who knows? 😄 If you want to give it a shot, I'd be happy to assist you on a pull request.

If not, I'll probably do it myself someday. 😉

[scompo]

Oh I understand, I would like to help with that, I'll provide a pull request as soon as I can.
Would exporting the code for resolveCategories in CategoryService in a strategy and use it in the getCategoriesmethod of DiffNodebe an ok solution in your opinion?

[SQiShER]

Awesome! Ok, let me give you some pointers: the basic idea of the CategoryService was to move the configuration (regarding categories) into a single place. The DiffNode on the other side is meant to be as stupid and decoupled from the rest of the system as possible. If the code would be moved into a strategy, the node would still need to access the service to retrieve the configuration and therefore introduce an unwanted dependency.

I would prefer a solution, where the service simply assigns the resolved categories as property of the node sometime during the comparison process. That way the dependency remains unidirectional and the node doesn't become much more complex than it already is.

For the public API it makes sense, that getCategories() behaves just as you initially expected. Internally however, it may make sense to introduce different properties for the hardcoded, inherited and configured categories, depending on how you adjust the logic in the CategoryService.

[scompo]

Great, makes sense to keep DiffNode decoupled.
By hardcoded you mean the ones on the annotations, right?

[SQiShER]

Yup

[scompo]

Thanks, just wanted to make sure I got it ;)
I didn't know anything about groovy and spock so I had to learn them, but I think I've got an integration test working.
I pushed the code I wrote this evening here: 1d15505.
I need to clean it up a bit and probably add some unit test, of course if you have any suggestion or critic I'd be glad to hear them.

[SQiShER]

Sorry, I didn't get to it tonight. But I'll have a closer look at it tomorrow!

[SQiShER]

Alright, I finally found the time to take a closer look and have some small suggestions. Could you open a Pull Request, so we can easily comment on specific lines of code? That would make it easier to discuss. PRs can always be amended later, so don't worry if you're not 100% done yet.

[scompo]

Hi, my last days have been pretty busy too 😄
Wich branch of the main repo should I use to create the pull request from mine?