Skip to content

Commit a87e653

Browse files
SQiShERSQiShER
committed
Refactored internal logic of InclusionService into InclusionResolvers
This made it possible to further improve performance by only activating the InclusionResolvers that have been actually configured to include or exclude nodes. It also made it possible for custom inclusion resolvers to be every bit as powerful as the built-in ones, because they are all treated the same. The InclusionResolver mechanism also allowed me to get rid of the InclusionNode and replace it with ValueNode. The original logic that justified the creation of the InclusionNode has been migrated to the NodePathInclusionResolver.
1 parent 2c343a7 commit a87e653

27 files changed

Lines changed: 1598 additions & 824 deletions

‎CHANGELOG.md‎

Lines changed: 7 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -3,21 +3,14 @@
33
## 0.91
44

55
### Features
6-
- **Inclusion:** Implemented a mechanism to control the inclusion of properties of
7-
specific object types.
8-
- **Inclusion:** Implemented a mechanism to register custom inclusion resolvers to
9-
support inclusion rules far beyond the built-in ones.
10-
- **Introspection:** Implemented a mechanism to register a custom instance factory,
11-
which will be used by `DiffNode#canonicalSet` in order to create instances of missing
12-
objects
6+
- **Inclusion:** Implemented a mechanism to control the inclusion of properties of specific object types.
7+
- **Inclusion:** Implemented a mechanism to register custom inclusion resolvers to support inclusion rules far beyond the built-in ones.
8+
- **Introspection:** Implemented a mechanism to register a custom instance factory, which is used by `DiffNode#canonicalSet` in order to create instances of missing objects
139

1410
### Improvements
15-
- **Inclusion:** Performance improvements (via @Deipher)
16-
- **DiffNode:** `canonicalSet` now automatically creates missing objects along the
17-
path to the root object
11+
- **Inclusion:** Performance improvements (Thanks [@Deipher](https://github.com/Deipher))
12+
- **Inclusion:** `InclusionService` has been split into several `InclusionResolver`s
13+
- **DiffNode:** `canonicalSet` now automatically creates missing objects along the path to the root object
1814

1915
### Bugfixes
20-
- **Circular Reference Detection:** Fixed 'Detected inconsistency in
21-
enter/leave sequence. Must always be LIFO.' bug that could occur
22-
due to inconsistent cleanup of the instance memory when a circular
23-
reference has been detected.
16+
- **Circular Reference Detection:** Fixed 'Detected inconsistency in enter/leave sequence. Must always be LIFO.' bug that could occur due to inconsistent cleanup of the instance memory when a circular reference has been detected.

‎src/integration-test/java/de/danielbechler/diff/ObjectDifferIT.java‎

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -299,13 +299,17 @@ public void testCompareWithDifferentTypes()
299299
@Test(groups = INTEGRATION)
300300
public void testCompareWithIgnoredProperty()
301301
{
302-
final ObjectDifferBuilder objectDifferBuilder = ObjectDifferBuilder.startBuilding();
303-
objectDifferBuilder.inclusion().exclude().node(NodePath.withRoot());
304-
objectDiffer = objectDifferBuilder.build();
305-
306-
final DiffNode node = objectDiffer.compare("foo", "bar");
302+
objectDiffer = ObjectDifferBuilder.startBuilding()
303+
.inclusion()
304+
.exclude().node(NodePath.with("value")).and()
305+
.filtering().returnNodesWithState(DiffNode.State.IGNORED).and()
306+
.build();
307+
308+
final ObjectWithIdentityAndValue working = new ObjectWithIdentityAndValue("1", "foo");
309+
final ObjectWithIdentityAndValue base = new ObjectWithIdentityAndValue("1", "bar");
310+
final DiffNode node = objectDiffer.compare(working, base);
307311

308-
NodeAssertions.assertThat(node).self().hasState(DiffNode.State.IGNORED);
312+
NodeAssertions.assertThat(node).child("value").hasState(DiffNode.State.IGNORED);
309313
}
310314

311315
@Test(groups = INTEGRATION)

‎src/integration-test/java/de/danielbechler/diff/inclusion/ExclusiveInclusionIT.groovy‎

Lines changed: 142 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,142 @@
1+
/*
2+
* Copyright 2014 Daniel Bechler
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
package de.danielbechler.diff.inclusion
18+
19+
import de.danielbechler.diff.ObjectDifferBuilder
20+
import de.danielbechler.diff.node.DiffNode
21+
import de.danielbechler.diff.path.NodePath
22+
import spock.lang.Specification
23+
24+
class ExclusiveInclusionIT extends Specification {
25+
26+
def objectDifferBuilder = ObjectDifferBuilder.startBuilding()
27+
.filtering()
28+
.returnNodesWithState(DiffNode.State.IGNORED).and()
29+
30+
def 'inclusion via type based property does NOT exclude nodes that are not explicitly included'() {
31+
given:
32+
def base = new A(b: new B(foo: 'base', bar: 'base'), c: new C(baz: 'base'))
33+
def working = new A(b: new B(foo: 'working', bar: 'base'), c: new C(baz: 'working'))
34+
35+
when:
36+
def node = objectDifferBuilder
37+
.inclusion()
38+
.include().propertyNameOfType(B, 'foo').and()
39+
.build()
40+
.compare(working, base)
41+
42+
then:
43+
node.getChild('b').getChild('foo').isChanged()
44+
node.getChild('b').getChild('bar').isIgnored()
45+
node.getChild('c').getChild('baz').isChanged()
46+
}
47+
48+
class A {
49+
B b
50+
C c
51+
}
52+
53+
class B {
54+
String foo
55+
String bar
56+
}
57+
58+
class C {
59+
String baz
60+
}
61+
62+
def 'inclusion via property name does exclude nodes that are not explicitly included'() {
63+
given:
64+
def base = new A(b: new B(foo: 'base', bar: 'base'), c: new C(baz: 'base'))
65+
def working = new A(b: new B(foo: 'working', bar: 'working'), c: new C(baz: 'working'))
66+
67+
when:
68+
def node = objectDifferBuilder
69+
.inclusion()
70+
.include().propertyName('b').and()
71+
.build()
72+
.compare(working, base)
73+
74+
then:
75+
node.getChild('b').hasChanges()
76+
node.getChild('b').getChild('foo').isChanged()
77+
node.getChild('b').getChild('bar').isChanged()
78+
node.getChild('c').isIgnored()
79+
}
80+
81+
def 'inclusion via node path does exclude nodes that are not explicitly included'() {
82+
given:
83+
def base = new A(b: new B(foo: 'base', bar: 'base'), c: new C(baz: 'base'))
84+
def working = new A(b: new B(foo: 'working', bar: 'working'), c: new C(baz: 'working'))
85+
86+
when:
87+
def node = objectDifferBuilder
88+
.inclusion()
89+
.include().node(NodePath.with('b')).and()
90+
.build()
91+
.compare(working, base)
92+
93+
then:
94+
node.getChild('b').hasChanges()
95+
node.getChild('b').getChild('foo').isChanged()
96+
node.getChild('b').getChild('bar').isChanged()
97+
node.getChild('c').isIgnored()
98+
}
99+
100+
def 'inclusion via type does exclude nodes that are not explicitly included - even its children'() {
101+
given:
102+
def base = new A(b: new B(foo: 'base', bar: 'base'), c: new C(baz: 'base'))
103+
def working = new A(b: new B(foo: 'working', bar: 'working'), c: new C(baz: 'working'))
104+
105+
when: 'the property with type B is explicitly included'
106+
def node = objectDifferBuilder
107+
.inclusion()
108+
.include().type(B).and()
109+
.build()
110+
.compare(working, base)
111+
112+
then: 'the included type should not be ignored'
113+
node.getChild('b').isIgnored() == false
114+
115+
and: 'its sibling with a different type schould be ignored'
116+
node.getChild('c').isIgnored()
117+
118+
and: 'the children are also ignored, even though the parent is included'
119+
node.getChild('b').getChild('foo').isIgnored()
120+
node.getChild('b').getChild('bar').isIgnored()
121+
}
122+
123+
def 'inclusion via category does exclude nodes that are not explicitly included'() {
124+
given:
125+
def base = new A(b: new B(foo: 'base', bar: 'base'), c: new C(baz: 'base'))
126+
def working = new A(b: new B(foo: 'working', bar: 'working'), c: new C(baz: 'working'))
127+
128+
when:
129+
def node = objectDifferBuilder
130+
.categories().ofNode(NodePath.with('b')).toBe('included-category').and()
131+
.inclusion()
132+
.include().category('included-category').and()
133+
.build()
134+
.compare(working, base)
135+
136+
then:
137+
node.getChild('b').hasChanges()
138+
node.getChild('b').getChild('foo').isChanged()
139+
node.getChild('b').getChild('bar').isChanged()
140+
node.getChild('c').isIgnored()
141+
}
142+
}

‎src/integration-test/java/de/danielbechler/diff/inclusion/InclusionAT.groovy‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -116,7 +116,7 @@ class InclusionAT extends Specification {
116116
def 'excludes always win over includes'() {
117117
given:
118118
objectDifferBuilder.inclusion()
119-
.exclude().node(NodePath.with('songs'))
119+
.exclude().node(NodePath.with('songs')).also()
120120
.include().node(NodePath.startBuildingFrom(NodePath.with('songs')).collectionItem('Happy').build())
121121
when:
122122
def node = objectDifferBuilder.build().compare(working, base)
@@ -146,8 +146,10 @@ class InclusionAT extends Specification {
146146

147147
def 'including an element by path implicitly includes its parents'() {
148148
given:
149-
objectDifferBuilder.inclusion()
150-
.include().node(NodePath.startBuilding().propertyName('songs').collectionItem(new Song(title: 'Happy')).build())
149+
def includedNodePath = NodePath.startBuilding()
150+
.propertyName('songs')
151+
.collectionItem(new Song(title: 'Happy'))
152+
objectDifferBuilder.inclusion().include().node(includedNodePath.build())
151153
when:
152154
def node = objectDifferBuilder.build().compare(working, base)
153155
then:

‎src/integration-test/java/de/danielbechler/diff/inclusion/PropertyInclusionViaInclusionResolverAT.groovy‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,11 @@ class PropertyInclusionViaInclusionResolverAT extends Specification {
3030

3131
given: "custom inclusion resolver that excludes all properties named 'foo'"
3232
def inclusionResolver = new InclusionResolver() {
33+
@Override
34+
boolean enablesStrictIncludeMode() {
35+
return false
36+
}
37+
3338
@Override
3439
Inclusion getInclusion(DiffNode node) {
3540
if (node.propertyAware && node.propertyName == 'foo') {

‎src/main/java/de/danielbechler/diff/inclusion/CategoryInclusionResolver.java‎

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,83 @@
1+
/*
2+
* Copyright 2014 Daniel Bechler
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
package de.danielbechler.diff.inclusion;
18+
19+
import de.danielbechler.diff.category.CategoryResolver;
20+
import de.danielbechler.diff.node.DiffNode;
21+
import de.danielbechler.util.Assert;
22+
23+
import java.util.HashMap;
24+
import java.util.Map;
25+
import java.util.Set;
26+
27+
import static de.danielbechler.diff.inclusion.Inclusion.DEFAULT;
28+
import static de.danielbechler.diff.inclusion.Inclusion.EXCLUDED;
29+
import static de.danielbechler.diff.inclusion.Inclusion.INCLUDED;
30+
31+
class CategoryInclusionResolver implements InclusionResolver
32+
{
33+
private final CategoryResolver categoryResolver;
34+
private final Map<String, Inclusion> categoryInclusions = new HashMap<String, Inclusion>();
35+
private boolean containsIncluded;
36+
private boolean containsExcluded;
37+
38+
public CategoryInclusionResolver(final CategoryResolver categoryResolver)
39+
{
40+
Assert.notNull(categoryResolver, "categoryResolver");
41+
this.categoryResolver = categoryResolver;
42+
}
43+
44+
public Inclusion getInclusion(final DiffNode node)
45+
{
46+
if (isInactive())
47+
{
48+
return DEFAULT;
49+
}
50+
final Set<String> categories = categoryResolver.resolveCategories(node);
51+
Inclusion resolvedInclusion = DEFAULT;
52+
for (final String category : categories)
53+
{
54+
final Inclusion configuredInclusion = categoryInclusions.get(category);
55+
if (configuredInclusion == EXCLUDED)
56+
{
57+
return EXCLUDED;
58+
}
59+
if (configuredInclusion == INCLUDED)
60+
{
61+
resolvedInclusion = INCLUDED;
62+
}
63+
}
64+
return resolvedInclusion;
65+
}
66+
67+
private boolean isInactive()
68+
{
69+
return !containsIncluded && !containsExcluded;
70+
}
71+
72+
public boolean enablesStrictIncludeMode()
73+
{
74+
return containsIncluded;
75+
}
76+
77+
public void setInclusion(final String category, final Inclusion inclusion)
78+
{
79+
categoryInclusions.put(category, inclusion);
80+
containsIncluded = categoryInclusions.containsValue(INCLUDED);
81+
containsExcluded = categoryInclusions.containsValue(EXCLUDED);
82+
}
83+
}

‎src/main/java/de/danielbechler/diff/inclusion/InclusionConfigurer.java‎

Lines changed: 13 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -56,13 +56,13 @@ public interface InclusionConfigurer
5656

5757
public interface ToInclude
5858
{
59-
ToIncludeAndReturn category(String category);
59+
ToInclude category(String category);
6060

61-
ToIncludeAndReturn type(Class<?> type);
61+
ToInclude type(Class<?> type);
6262

63-
ToIncludeAndReturn node(NodePath nodePath);
63+
ToInclude node(NodePath nodePath);
6464

65-
ToIncludeAndReturn propertyName(String propertyName);
65+
ToInclude propertyName(String propertyName);
6666

6767
/**
6868
* Include one or more properties of the given type. This automatically causes all other properties of that
@@ -73,25 +73,22 @@ public interface ToInclude
7373
* @param propertyNames One or more property names to include.
7474
* @see de.danielbechler.diff.introspection.ObjectDiffProperty#inclusion()
7575
*/
76-
ToIncludeAndReturn propertyNameOfType(Class<?> type, String... propertyNames);
76+
ToInclude propertyNameOfType(Class<?> type, String... propertyNames);
7777

78-
ToExclude exclude();
79-
}
78+
InclusionConfigurer also();
8079

81-
public interface ToIncludeAndReturn extends ToInclude
82-
{
8380
ObjectDifferBuilder and();
8481
}
8582

8683
public interface ToExclude
8784
{
88-
ToExcludeAndReturn category(String category);
85+
ToExclude category(String category);
8986

90-
ToExcludeAndReturn type(Class<?> type);
87+
ToExclude type(Class<?> type);
9188

92-
ToExcludeAndReturn node(NodePath nodePath);
89+
ToExclude node(NodePath nodePath);
9390

94-
ToExcludeAndReturn propertyName(String property);
91+
ToExclude propertyName(String property);
9592

9693
/**
9794
* Excludes one or more properties of the given type.
@@ -101,13 +98,11 @@ public interface ToExclude
10198
* @see de.danielbechler.diff.introspection.ObjectDiffProperty#inclusion()
10299
* @see de.danielbechler.diff.introspection.ObjectDiffProperty#excluded()
103100
*/
104-
ToExcludeAndReturn propertyNameOfType(Class<?> type, String... propertyNames);
101+
ToExclude propertyNameOfType(Class<?> type, String... propertyNames);
105102

106-
ToInclude include();
107-
}
103+
InclusionConfigurer also();
108104

109-
public interface ToExcludeAndReturn extends ToExclude
110-
{
111105
ObjectDifferBuilder and();
112106
}
107+
113108
}

0 commit comments

Comments
 (0)