-
Notifications
You must be signed in to change notification settings - Fork 180
added category resolution on compare #159
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
Changes from 8 commits
1d15505
7b8b206
ac8a359
8cf1557
316602f
4f03e4a
b459cf7
b8c3267
1944cae
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,49 @@ | ||
| package de.danielbechler.diff.categories | ||
|
|
||
| import de.danielbechler.diff.ObjectDifferBuilder | ||
| import de.danielbechler.diff.introspection.ObjectDiffProperty | ||
| import de.danielbechler.diff.node.DiffNode | ||
| import de.danielbechler.diff.node.Visit | ||
| import de.danielbechler.diff.path.NodePath | ||
| import spock.lang.Specification | ||
|
|
||
| class CategoriesTestIT extends Specification{ | ||
|
|
||
| def obj1 = new MyObject("aaa","aaa", "aaa") | ||
| def obj2 = new MyObject("bbb","bbb", "bbb") | ||
| def differ = ObjectDifferBuilder.startBuilding() | ||
| .categories() | ||
| .ofNode(NodePath.with("firstString")).toBe("cat1") | ||
| .ofNode(NodePath.with("secondString")).toBe("cat1") | ||
| .ofNode(NodePath.with("thirdString")).toBe("cat1") | ||
| .and() | ||
| .build() | ||
|
|
||
| def "should return all categories"(){ | ||
| given: | ||
| def node = differ.compare(obj1,obj2) | ||
| expect : | ||
| node.getChild("firstString").getCategories() == ["cat1"] as Set | ||
| node.getChild("secondString").getCategories() == ["cat1"] as Set | ||
|
||
| node.getChild("thirdString").getCategories() == ["cat1", "catAnnotation"] as Set | ||
| } | ||
|
|
||
| class MyObject{ | ||
|
|
||
| def firstString | ||
| def secondString | ||
| def thirdString | ||
|
|
||
| MyObject(firstString,secondString,thirdString) { | ||
|
|
||
| this.firstString = firstString | ||
| this.secondString = secondString | ||
| this.thirdString = thirdString | ||
| } | ||
|
|
||
| @ObjectDiffProperty(categories = ["catAnnotation"]) | ||
| def getThirdString() { | ||
| return thirdString | ||
| } | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -24,6 +24,7 @@ | |
|
|
||
| import java.util.Collection; | ||
| import java.util.LinkedList; | ||
| import java.util.Set; | ||
|
||
|
|
||
| import static de.danielbechler.diff.inclusion.Inclusion.DEFAULT; | ||
| import static de.danielbechler.diff.inclusion.Inclusion.EXCLUDED; | ||
|
|
@@ -279,4 +280,5 @@ public ObjectDifferBuilder and() | |
| { | ||
| return rootConfiguration; | ||
| } | ||
|
|
||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -32,12 +32,7 @@ | |
| import de.danielbechler.util.Assert; | ||
|
|
||
| import java.lang.annotation.Annotation; | ||
| import java.util.Collections; | ||
| import java.util.LinkedHashMap; | ||
| import java.util.List; | ||
| import java.util.Map; | ||
| import java.util.Set; | ||
| import java.util.TreeSet; | ||
| import java.util.*; | ||
|
||
| import java.util.concurrent.atomic.AtomicBoolean; | ||
|
|
||
| import static java.util.Collections.unmodifiableSet; | ||
|
|
@@ -67,6 +62,7 @@ public class DiffNode | |
| private Class<?> valueType; | ||
| private TypeInfo valueTypeInfo; | ||
| private IdentityStrategy childIdentityStrategy; | ||
| private final Set<String> additionalCategories = new TreeSet<String>(); | ||
|
|
||
| public void setChildIdentityStrategy(final IdentityStrategy identityStrategy) | ||
| { | ||
|
|
@@ -573,7 +569,10 @@ public boolean isExcluded() | |
| return false; | ||
| } | ||
|
|
||
| // TODO These categories should also contain the ones configured via CategoryService | ||
| /** | ||
| * Returns a {@link java.util.Set} of {@link java.lang.String} | ||
| * @return | ||
| */ | ||
| public final Set<String> getCategories() | ||
| { | ||
| final Set<String> categories = new TreeSet<String>(); | ||
|
|
@@ -589,7 +588,9 @@ public final Set<String> getCategories() | |
| categories.addAll(categoriesFromAccessor); | ||
| } | ||
| } | ||
| return categories; | ||
| categories.addAll(additionalCategories); | ||
|
|
||
| return Collections.unmodifiableSet(categories); | ||
| } | ||
|
|
||
| /** | ||
|
|
@@ -732,6 +733,12 @@ else if (childCount() > 1) | |
| return sb.toString(); | ||
| } | ||
|
|
||
| public void addCategories(final Collection<String> additionalCategories) | ||
| { | ||
| Assert.notNull(additionalCategories, "additionalCategories"); | ||
| this.additionalCategories.addAll(additionalCategories); | ||
| } | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Unfortunately there is no way to avoid making this method part of the public API, so people will eventually start (ab)using it. That's one of the reasons why I'd prefer a different name for the field, as mentioned above. Although I wouldn't even expose the field via getter or setter at all. If we can't avoid leaking this detail to the outside, let's make it a cool feature: let's say we call the method Calling it I imagine something like this: public final void addCategories(final Collection<String> additionalCategories)
{
Assert.notNull(additionalCategories, "additionalCategories");
this.additionalCategories.addAll(additionalCategories);
}In this example I changed the parameter type from |
||
|
|
||
| /** | ||
| * @return Returns the path to the first node in the hierarchy that represents the same object instance as | ||
| * this one. (Only if {@link #isCircular()} returns <code>true</code>. | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -245,4 +245,32 @@ class DiffNodeTest extends Specification { | |
|
|
||
| doesOrDoesNotImplement = expectedResult ? 'implements' : 'does not implement' | ||
| } | ||
|
|
||
| def "should return categories added when visiting"(){ | ||
|
||
| given: | ||
| def node = new DiffNode(null, Mock(Accessor), Object) | ||
| node.addCategories(["addedCategory"] as List) | ||
| expect : | ||
| node.getCategories() == ["addedCategory"] as Set | ||
| } | ||
|
|
||
| def "categories should not be modifiable by a client directly"(){ | ||
|
|
||
| when: | ||
| def node = new DiffNode(null, Mock(Accessor), Object) | ||
| def cats = node.getCategories() | ||
| cats.removeAll() | ||
| then : | ||
| thrown UnsupportedOperationException | ||
| } | ||
|
|
||
| def "should throw exception when added a null collection"(){ | ||
|
|
||
| when: | ||
| def node = new DiffNode(null, Mock(Accessor), Object) | ||
| node.addCategories(null) | ||
| then : | ||
| def ex = thrown(IllegalArgumentException) | ||
| ex.message == "'additionalCategories' must not be null" | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since there is only one test left, the setup can be moved into the test method, so it becomes self-contained.