Address review comments. Added tests.

hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/term/BaseHapiTerminologySvcImpl.java

@@ -882,10 +882,18 @@ public abstract class BaseHapiTerminologySvcImpl implements IHapiTerminologySvc,
 				break;
 			case "parent":
 			case "child":
-				handleFilterLoincParentChild(theSystem, theQb, theBool, theFilter);
+				if (isCodeSystemLoinc(theSystem)) {
+					handleFilterLoincParentChild(theQb, theBool, theFilter);
+				} else {
+					throw new InvalidRequestException("Invalid filter, property " + theFilter.getProperty() + " is LOINC-specific and cannot be used with system: " + theSystem);
+				}
 				break;
 			case "copyright":
-				handleFilterLoincCopyright(theSystem, theQb, theBool, theFilter);
+				if (isCodeSystemLoinc(theSystem)) {
+					handleFilterLoincCopyright(theQb, theBool, theFilter);
+				} else {
+					throw new InvalidRequestException("Invalid filter, property " + theFilter.getProperty() + " is LOINC-specific and cannot be used with system: " + theSystem);
+				}
 				break;
 			default:
 				handleFilterRegex(theBool, theFilter);
@@ -893,6 +901,10 @@ public abstract class BaseHapiTerminologySvcImpl implements IHapiTerminologySvc,
 		}
 	}
 
+	private boolean isCodeSystemLoinc(String theSystem) {
+		return IHapiTerminologyLoaderSvc.LOINC_URI.equals(theSystem);
+	}
+
 	private void handleFilterDisplay(QueryBuilder theQb, BooleanJunction<?> theBool, ValueSet.ConceptSetFilterComponent theFilter) {
 		if (theFilter.getProperty().equals("display:exact") && theFilter.getOp() == ValueSet.FilterOperator.EQUAL) {
 			addDisplayFilterExact(theQb, theBool, theFilter);
@@ -917,11 +929,7 @@ public abstract class BaseHapiTerminologySvcImpl implements IHapiTerminologySvc,
 		}
 	}
 
-	private void handleFilterLoincParentChild(String theSystem, QueryBuilder theQb, BooleanJunction<?> theBool, ValueSet.ConceptSetFilterComponent theFilter) {
+	private void handleFilterLoincParentChild(QueryBuilder theQb, BooleanJunction<?> theBool, ValueSet.ConceptSetFilterComponent theFilter) {
-		if (isNotCodeSystemLoinc(theSystem)) {
-			return;
-		}
-
 		if (theFilter.getOp() == ValueSet.FilterOperator.EQUAL) {
 			addLoincFilterParentChildEqual(theBool, theFilter.getProperty(), theFilter.getValue());
 		} else if (theFilter.getOp() == ValueSet.FilterOperator.IN) {
@@ -931,14 +939,6 @@ public abstract class BaseHapiTerminologySvcImpl implements IHapiTerminologySvc,
 		}
 	}
 
-	private boolean isCodeSystemLoinc(String theSystem) {
-		return IHapiTerminologyLoaderSvc.LOINC_URI.equals(theSystem);
-	}
-
-	private boolean isNotCodeSystemLoinc(String theSystem) {
-		return !isCodeSystemLoinc(theSystem);
-	}
-
 	private void addLoincFilterParentChildEqual(BooleanJunction<?> theBool, String theProperty, String theValue) {
 		logFilteringValueOnProperty(theValue, theProperty);
 		theBool.must(new TermsQuery(getPropertyTerm(theProperty, theValue)));
@@ -958,11 +958,7 @@ public abstract class BaseHapiTerminologySvcImpl implements IHapiTerminologySvc,
 		return new Term(TermConceptPropertyFieldBridge.CONCEPT_FIELD_PROPERTY_PREFIX + theProperty, theValue);
 	}
 
-	private void handleFilterLoincCopyright(String theSystem, QueryBuilder theQb, BooleanJunction<?> theBool, ValueSet.ConceptSetFilterComponent theFilter) {
+	private void handleFilterLoincCopyright(QueryBuilder theQb, BooleanJunction<?> theBool, ValueSet.ConceptSetFilterComponent theFilter) {
-		if (isNotCodeSystemLoinc(theSystem)) {
-			return;
-		}
-
 		if (theFilter.getOp() == ValueSet.FilterOperator.EQUAL) {
 
 			String copyrightFilterValue = defaultString(theFilter.getValue()).toLowerCase();

hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/term/TerminologySvcImplDstu3Test.java

@@ -473,9 +473,7 @@ public class TerminologySvcImplDstu3Test extends BaseJpaDstu3Test {
 	public void testExpandValueSetPropertyFilterLoincCopyrightWithUnsupportedOp() {
 		createLoincSystemWithSomeCodes();
 
-		List<String> codes;
 		ValueSet vs;
-		ValueSet outcome;
 		ValueSet.ConceptSetComponent include;
 
 		// Include
@@ -488,20 +486,43 @@ public class TerminologySvcImplDstu3Test extends BaseJpaDstu3Test {
 			.setOp(ValueSet.FilterOperator.ISA)
 			.setValue("LOINC");
 		try {
-			outcome = myTermSvc.expandValueSet(vs);
+			myTermSvc.expandValueSet(vs);
 		} catch (InvalidRequestException e) {
 			assertEquals(400, e.getStatusCode());
 			assertEquals("Don't know how to handle op=ISA on property copyright", e.getMessage());
 		}
 	}
 
+	@Test
+	public void testExpandValueSetPropertyFilterLoincCopyrightWithUnsupportedSystem() {
+		createCodeSystem();
+		createLoincSystemWithSomeCodes();
+
+		ValueSet vs;
+		ValueSet.ConceptSetComponent include;
+
+		// Include
+		vs = new ValueSet();
+		include = vs.getCompose().addInclude();
+		include.setSystem(LOINC_URI);
+		include
+			.addFilter()
+			.setProperty("copyright")
+			.setOp(ValueSet.FilterOperator.EQUAL)
+			.setValue("LOINC");
+		try {
+			myTermSvc.expandValueSet(vs);
+		} catch (InvalidRequestException e) {
+			assertEquals(400, e.getStatusCode());
+			assertEquals("Invalid filter, property copyright is LOINC-specific and cannot be used with system: http://example.com/my_code_system", e.getMessage());
+		}
+	}
+
 	@Test
 	public void testExpandValueSetPropertyFilterLoincCopyrightWithUnsupportedValue() {
 		createLoincSystemWithSomeCodes();
 
-		List<String> codes;
 		ValueSet vs;
-		ValueSet outcome;
 		ValueSet.ConceptSetComponent include;
 
 		// Include
@@ -514,7 +535,7 @@ public class TerminologySvcImplDstu3Test extends BaseJpaDstu3Test {
 			.setOp(ValueSet.FilterOperator.EQUAL)
 			.setValue("bogus");
 		try {
-			outcome = myTermSvc.expandValueSet(vs);
+			myTermSvc.expandValueSet(vs);
 		} catch (InvalidRequestException e) {
 			assertEquals(400, e.getStatusCode());
 			assertEquals("Don't know how to handle value=bogus on property copyright", e.getMessage());
@@ -710,6 +731,55 @@ public class TerminologySvcImplDstu3Test extends BaseJpaDstu3Test {
 		assertThat(codes, containsInAnyOrder("50015-7", "43343-3"));
 	}
 
+	@Test
+	public void testExpandValueSetPropertyFilterLoincChildWithUnsupportedOp() {
+		createLoincSystemWithSomeCodes();
+
+		ValueSet vs;
+		ValueSet.ConceptSetComponent include;
+
+		// Include
+		vs = new ValueSet();
+		include = vs.getCompose().addInclude();
+		include.setSystem(LOINC_URI);
+		include
+			.addFilter()
+			.setProperty("child")
+			.setOp(ValueSet.FilterOperator.ISA)
+			.setValue("50015-7");
+		try {
+			myTermSvc.expandValueSet(vs);
+		} catch (InvalidRequestException e) {
+			assertEquals(400, e.getStatusCode());
+			assertEquals("Don't know how to handle op=ISA on property child", e.getMessage());
+		}
+	}
+
+	@Test
+	public void testExpandValueSetPropertyFilterLoincChildWithUnsupportedSystem() {
+		createCodeSystem();
+		createLoincSystemWithSomeCodes();
+
+		ValueSet vs;
+		ValueSet.ConceptSetComponent include;
+
+		// Include
+		vs = new ValueSet();
+		include = vs.getCompose().addInclude();
+		include.setSystem(CS_URL);
+		include
+			.addFilter()
+			.setProperty("child")
+			.setOp(ValueSet.FilterOperator.EQUAL)
+			.setValue("50015-7");
+		try {
+			myTermSvc.expandValueSet(vs);
+		} catch (InvalidRequestException e) {
+			assertEquals(400, e.getStatusCode());
+			assertEquals("Invalid filter, property child is LOINC-specific and cannot be used with system: http://example.com/my_code_system", e.getMessage());
+		}
+	}
+
 	@Test
 	public void testExpandValueSetPropertyFilterLoincParentWithExcludeAndEqual() {
 		createLoincSystemWithSomeCodes();
@@ -898,6 +968,55 @@ public class TerminologySvcImplDstu3Test extends BaseJpaDstu3Test {
 		assertThat(codes, containsInAnyOrder("43343-3", "43343-4", "47239-9"));
 	}
 
+	@Test
+	public void testExpandValueSetPropertyFilterLoincParentWithUnsupportedOp() {
+		createLoincSystemWithSomeCodes();
+
+		ValueSet vs;
+		ValueSet.ConceptSetComponent include;
+
+		// Include
+		vs = new ValueSet();
+		include = vs.getCompose().addInclude();
+		include.setSystem(LOINC_URI);
+		include
+			.addFilter()
+			.setProperty("parent")
+			.setOp(ValueSet.FilterOperator.ISA)
+			.setValue("50015-7");
+		try {
+			myTermSvc.expandValueSet(vs);
+		} catch (InvalidRequestException e) {
+			assertEquals(400, e.getStatusCode());
+			assertEquals("Don't know how to handle op=ISA on property parent", e.getMessage());
+		}
+	}
+
+	@Test
+	public void testExpandValueSetPropertyFilterLoincParentWithUnsupportedSystem() {
+		createCodeSystem();
+		createLoincSystemWithSomeCodes();
+
+		ValueSet vs;
+		ValueSet.ConceptSetComponent include;
+
+		// Include
+		vs = new ValueSet();
+		include = vs.getCompose().addInclude();
+		include.setSystem(CS_URL);
+		include
+			.addFilter()
+			.setProperty("parent")
+			.setOp(ValueSet.FilterOperator.EQUAL)
+			.setValue("50015-7");
+		try {
+			myTermSvc.expandValueSet(vs);
+		} catch (InvalidRequestException e) {
+			assertEquals(400, e.getStatusCode());
+			assertEquals("Invalid filter, property parent is LOINC-specific and cannot be used with system: http://example.com/my_code_system", e.getMessage());
+		}
+	}
+
 	@Test
 	public void testExpandValueSetPropertySearchWithRegexExclude() {
 		createLoincSystemWithSomeCodes();