Skip to content

Commit aa0f9a0

Browse files
committed
Let a rule read a named set from the test resources
semantic-tag-hierarchies.txt is loaded as a generic key=value,value,value store, but isSemanticTagCompatibleWithinHierarchy is the only way a rule can read it - and that method extracts a semantic tag from a term before looking it up, so the store is only reachable for values that are semantic tags. This exposes the same store for values that are not, so a rule can be driven by per-edition configuration. The case that prompted it is a set of module ids: an editorial policy that some editions have adopted and others have not cannot be expressed as a semantic tag, and hardcoding either the policy or its exceptions into a rule in common-authoring makes that rule wrong for somebody. Defaulted rather than abstract, so implementations outside this project - the authoring platform's, in particular - keep compiling untouched. The default answers false for every key, meaning "no configuration present", which obliges a rule using it to behave on false exactly as it did before the method existed. NamedSetLookupTest asserts that against an implementation declaring only the methods the interface required before this change. TestDescriptionService implements it too. Without that the rules test rig would always see the default, so a rule driven by a named set could only ever be tested unconfigured - the set would look empty however test-cases.json and the dummy test resources were written. No behaviour change on its own: no rule calls it yet.
1 parent 84d511b commit aa0f9a0

5 files changed

Lines changed: 245 additions & 0 deletions

File tree

snomed-drools-engine/src/main/java/org/ihtsdo/drools/service/DescriptionService.java

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,4 +25,29 @@ public interface DescriptionService {
2525

2626
boolean isSemanticTagCompatibleWithinHierarchy(String term, Set<String> topLevelSemanticTags);
2727

28+
/**
29+
* Is {@code value} a member of the named set {@code setKey} in the test
30+
* resources?
31+
*
32+
* <p>semantic-tag-hierarchies.txt is loaded as a generic
33+
* {@code key=value,value,value} store, and
34+
* {@link #isSemanticTagCompatibleWithinHierarchy} is currently the only way a
35+
* rule can read it - which restricts it to values that are semantic tags
36+
* extracted from a term. This exposes the same store for values that are not,
37+
* so that a rule can be driven by per-edition configuration such as a set of
38+
* module ids.
39+
*
40+
* <p>Defaulted rather than abstract so that existing implementations outside
41+
* this project keep compiling. The default answers {@code false} for every
42+
* key, which means "no configuration present" - a rule written against this
43+
* must therefore behave, when it gets {@code false}, exactly as it did before
44+
* the configuration existed.
45+
*
46+
* @param setKey the key on the left of the {@code =} in the resource file
47+
* @param value the value to look for among that key's members
48+
*/
49+
default boolean isInNamedSet(String setKey, String value) {
50+
return false;
51+
}
52+
2853
}

snomed-drools-engine/src/test/java/org/ihtsdo/drools/rulestestrig/service/TestDescriptionService.java

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -162,4 +162,19 @@ public boolean isSemanticTagCompatibleWithinHierarchy(String testTerm, Set<Strin
162162

163163
return false;
164164
}
165+
166+
/**
167+
* Without this the rules test rig always sees the interface default,
168+
* {@code false}, so a rule driven by a named set could only ever be tested in
169+
* its unconfigured state - the set would appear empty however the test
170+
* resources were written.
171+
*/
172+
@Override
173+
public boolean isInNamedSet(String setKey, String value) {
174+
if (setKey == null || value == null) {
175+
return false;
176+
}
177+
Set<String> members = testResourceProvider.getSemanticHierarchyMap().get(setKey);
178+
return !CollectionUtils.isEmpty(members) && members.contains(value);
179+
}
165180
}
Lines changed: 99 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,99 @@
1+
package org.ihtsdo.drools.unittest;
2+
3+
import org.ihtsdo.drools.domain.Concept;
4+
import org.ihtsdo.drools.domain.Description;
5+
import org.ihtsdo.drools.service.DescriptionService;
6+
import org.junit.Test;
7+
8+
import java.util.Set;
9+
10+
import static org.junit.Assert.assertFalse;
11+
12+
/**
13+
* The guarantee that makes {@link DescriptionService#isInNamedSet} safe to add:
14+
* an implementation written before it existed keeps compiling, and answers in a
15+
* way that leaves a rule using it behaving exactly as it did before.
16+
*
17+
* <p>{@link LegacyDescriptionService} below is such an implementation - it
18+
* implements every method the interface had previously and does not mention the
19+
* new one. That this file compiles is half the test; the assertions are the
20+
* other half.
21+
*/
22+
public class NamedSetLookupTest {
23+
24+
private final DescriptionService legacy = new LegacyDescriptionService();
25+
26+
@Test
27+
public void anImplementationThatDoesNotOverrideItAnswersFalse() {
28+
assertFalse(legacy.isInNamedSet("redundant-isa-exempt-modules", "32506021000036107"));
29+
}
30+
31+
@Test
32+
public void theDefaultDoesNotDistinguishBetweenKeys() {
33+
// "No configuration present" has to be the answer for every key, not only
34+
// for keys nobody has defined, or a rule would behave differently
35+
// depending on which implementation it ran against.
36+
assertFalse(legacy.isInNamedSet("anything-at-all", "any-value"));
37+
assertFalse(legacy.isInNamedSet("", ""));
38+
}
39+
40+
@Test
41+
public void theDefaultToleratesNulls() {
42+
assertFalse(legacy.isInNamedSet(null, "32506021000036107"));
43+
assertFalse(legacy.isInNamedSet("redundant-isa-exempt-modules", null));
44+
assertFalse(legacy.isInNamedSet(null, null));
45+
}
46+
47+
/**
48+
* Stands in for an implementation outside this project - the authoring
49+
* platform's, in particular. Only the methods the interface required before
50+
* this change are declared.
51+
*/
52+
private static final class LegacyDescriptionService implements DescriptionService {
53+
54+
@Override
55+
public Set<String> getFSNs(Set<String> conceptIds, String... languageRefsetIds) {
56+
throw new UnsupportedOperationException();
57+
}
58+
59+
@Override
60+
public Set<Description> findActiveDescriptionByExactTerm(String exactTerm) {
61+
throw new UnsupportedOperationException();
62+
}
63+
64+
@Override
65+
public Set<Description> findInactiveDescriptionByExactTerm(String exactTerm) {
66+
throw new UnsupportedOperationException();
67+
}
68+
69+
@Override
70+
public Set<Description> findMatchingDescriptionInHierarchy(Concept concept, Description description) {
71+
throw new UnsupportedOperationException();
72+
}
73+
74+
@Override
75+
public String getLanguageSpecificErrorMessage(Description description) {
76+
throw new UnsupportedOperationException();
77+
}
78+
79+
@Override
80+
public String getCaseSensitiveWordsErrorMessage(Description description) {
81+
throw new UnsupportedOperationException();
82+
}
83+
84+
@Override
85+
public Set<String> findParentsNotContainingSemanticTag(Concept concept, String termSematicTag, String... languageRefsetIds) {
86+
throw new UnsupportedOperationException();
87+
}
88+
89+
@Override
90+
public boolean isRecognisedSemanticTag(String termSemanticTag, String language) {
91+
throw new UnsupportedOperationException();
92+
}
93+
94+
@Override
95+
public boolean isSemanticTagCompatibleWithinHierarchy(String term, Set<String> topLevelSemanticTags) {
96+
throw new UnsupportedOperationException();
97+
}
98+
}
99+
}

snomed-drools-rf2-validator/src/main/java/org/ihtsdo/drools/validator/rf2/service/DroolsDescriptionService.java

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -191,6 +191,15 @@ public boolean isSemanticTagCompatibleWithinHierarchy(String testTerm, Set<Strin
191191
return false;
192192
}
193193

194+
@Override
195+
public boolean isInNamedSet(String setKey, String value) {
196+
if (setKey == null || value == null) {
197+
return false;
198+
}
199+
Set<String> members = testResourceProvider.getSemanticHierarchyMap().get(setKey);
200+
return !CollectionUtils.isEmpty(members) && members.contains(value);
201+
}
202+
194203
private static String getTag(String term) {
195204
final Matcher matcher = DescriptionHelper.TAG_PATTERN.matcher(term);
196205
if (matcher.matches()) {
Lines changed: 97 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,97 @@
1+
package org.ihtsdo.drools.validator.rf2.service;
2+
3+
import org.ihtsdo.drools.service.TestResourceProvider;
4+
import org.ihtsdo.drools.validator.rf2.DroolsRF2Validator;
5+
import org.ihtsdo.otf.resourcemanager.ResourceManager;
6+
import org.junit.Before;
7+
import org.junit.Test;
8+
9+
import java.io.IOException;
10+
import java.util.Collections;
11+
import java.util.HashMap;
12+
import java.util.HashSet;
13+
import java.util.Map;
14+
import java.util.Set;
15+
16+
import static org.junit.Assert.assertFalse;
17+
import static org.junit.Assert.assertTrue;
18+
19+
/**
20+
* {@code isInNamedSet} against the real reference-data store.
21+
*
22+
* <p>The absent-key case is the one that matters, because it is the state every
23+
* edition is in until it adds a key: a rule that negates this method must go on
24+
* behaving as it did before the key existed.
25+
*/
26+
public class DroolsNamedSetLookupTest extends BaseServiceTest {
27+
28+
private static final String KEY = "redundant-isa-exempt-modules";
29+
private static final String AU_MODULE = "32506021000036107";
30+
private static final String CORE_MODULE = "900000000000207008";
31+
32+
private DroolsDescriptionService serviceWithNoResources;
33+
34+
@Before
35+
public void setup() throws IOException {
36+
loadConceptsIntoRepository();
37+
loadDescriptionsIntoRepository();
38+
ResourceManager blank = new ResourceManager(DroolsRF2Validator.BLANK_RESOURCES_CONFIGURATION, null);
39+
serviceWithNoResources = service(new TestResourceProvider(blank));
40+
}
41+
42+
private DroolsDescriptionService service(TestResourceProvider provider) {
43+
return new DroolsDescriptionService(repository, new DroolsConceptService(repository, null), provider);
44+
}
45+
46+
/** No reference data at all - every lookup must answer false. */
47+
@Test
48+
public void absentFileMeansNotAMember() {
49+
assertFalse(serviceWithNoResources.isInNamedSet(KEY, AU_MODULE));
50+
assertFalse(serviceWithNoResources.isInNamedSet(KEY, CORE_MODULE));
51+
}
52+
53+
@Test
54+
public void aKeyThatIsNotInTheFileMeansNotAMember() throws IOException {
55+
assertFalse(withSet("some-other-key", AU_MODULE).isInNamedSet(KEY, AU_MODULE));
56+
}
57+
58+
@Test
59+
public void aListedValueIsAMemberAndAnUnlistedOneIsNot() throws IOException {
60+
DroolsDescriptionService s = withSet(KEY, AU_MODULE);
61+
assertTrue(s.isInNamedSet(KEY, AU_MODULE));
62+
assertFalse(s.isInNamedSet(KEY, CORE_MODULE));
63+
}
64+
65+
@Test
66+
public void matchingIsExactRatherThanBySubstring() throws IOException {
67+
// Module ids are long and share prefixes; a substring match would exempt
68+
// modules nobody listed.
69+
DroolsDescriptionService s = withSet(KEY, AU_MODULE);
70+
assertFalse(s.isInNamedSet(KEY, AU_MODULE.substring(0, 8)));
71+
assertFalse(s.isInNamedSet(KEY, AU_MODULE + "9"));
72+
}
73+
74+
@Test
75+
public void nullsAreNotMembers() throws IOException {
76+
DroolsDescriptionService s = withSet(KEY, AU_MODULE);
77+
assertFalse(s.isInNamedSet(null, AU_MODULE));
78+
assertFalse(s.isInNamedSet(KEY, null));
79+
}
80+
81+
/**
82+
* The loader that reads {@code key=value,value} from
83+
* semantic-tag-hierarchies.txt is already exercised by the semantic-tag
84+
* tests, so the map is supplied directly here to keep this about the lookup.
85+
*/
86+
private DroolsDescriptionService withSet(String key, String... values) throws IOException {
87+
ResourceManager blank = new ResourceManager(DroolsRF2Validator.BLANK_RESOURCES_CONFIGURATION, null);
88+
Map<String, Set<String>> named = new HashMap<>();
89+
named.put(key, new HashSet<>(java.util.Arrays.asList(values)));
90+
return service(new TestResourceProvider(blank) {
91+
@Override
92+
public Map<String, Set<String>> getSemanticHierarchyMap() {
93+
return Collections.unmodifiableMap(named);
94+
}
95+
});
96+
}
97+
}

0 commit comments

Comments
 (0)