Removing gaps from concept order when pre-expanding ValueSets.

hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/data/ITermValueSetConceptDao.java

@@ -46,4 +46,11 @@ public interface ITermValueSetConceptDao extends JpaRepository<TermValueSetConce
 
 	@Query("SELECT vsc FROM TermValueSetConcept vsc WHERE vsc.myValueSet.myResourcePid = :resource_pid AND vsc.mySystem = :system_url AND vsc.myCode = :codeval")
 	Optional<TermValueSetConcept> findByValueSetResourcePidSystemAndCode(@Param("resource_pid") Long theValueSetId, @Param("system_url") String theSystem, @Param("codeval") String theCode);
+
+	@Query("SELECT vsc.myId FROM TermValueSetConcept vsc WHERE vsc.myValueSetPid = :pid ORDER BY vsc.myId")
+	List<Long> findIdsByTermValueSetId(@Param("pid") Long theValueSetId);
+
+	@Query("UPDATE TermValueSetConcept vsc SET vsc.myOrder = :order WHERE vsc.myId = :pid")
+	@Modifying
+	void updateOrderById(@Param("pid") Long theId, @Param("order") int theOrder);
 }

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

@@ -607,10 +607,10 @@ public abstract class BaseHapiTerminologySvcImpl implements IHapiTerminologySvc,
 		ourLog.debug("Handling includes");
 		for (ValueSet.ConceptSetComponent include : theValueSetToExpand.getCompose().getInclude()) {
 			for (int i = 0; ; i++) {
-				int finalI = i;
+				int queryIndex = i;
 				Boolean shouldContinue = myTxTemplate.execute(t -> {
 					boolean add = true;
-					return expandValueSetHandleIncludeOrExclude(theValueSetCodeAccumulator, addedCodes, include, add, theCodeCounter, finalI);
+					return expandValueSetHandleIncludeOrExclude(theValueSetCodeAccumulator, addedCodes, include, add, theCodeCounter, queryIndex);
 				});
 				if (!shouldContinue) {
 					break;
@@ -622,10 +622,10 @@ public abstract class BaseHapiTerminologySvcImpl implements IHapiTerminologySvc,
 		ourLog.debug("Handling excludes");
 		for (ValueSet.ConceptSetComponent exclude : theValueSetToExpand.getCompose().getExclude()) {
 			for (int i = 0; ; i++) {
-				int finalI = i;
+				int queryIndex = i;
 				Boolean shouldContinue = myTxTemplate.execute(t -> {
 					boolean add = false;
-					return expandValueSetHandleIncludeOrExclude(theValueSetCodeAccumulator, addedCodes, exclude, add, theCodeCounter, finalI);
+					return expandValueSetHandleIncludeOrExclude(theValueSetCodeAccumulator, addedCodes, exclude, add, theCodeCounter, queryIndex);
 				});
 				if (!shouldContinue) {
 					break;
@@ -633,7 +633,9 @@ public abstract class BaseHapiTerminologySvcImpl implements IHapiTerminologySvc,
 			}
 		}
 
-		// FIXME: DM 2019-09-06 - After handling excludes, we need to adjust TermValueSetConcept.myOrder to account for any gaps.
+		if (theValueSetCodeAccumulator instanceof ValueSetConceptAccumulator) {
+			myTxTemplate.execute(t -> ((ValueSetConceptAccumulator) theValueSetCodeAccumulator).removeGapsFromConceptOrder());
+		}
 
 		ourLog.info("Done working with {} in {}ms", valueSetInfo, sw.getMillis());
 	}

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

@@ -31,6 +31,7 @@ import ca.uhn.fhir.util.ValidateUtil;
 
 import javax.annotation.Nonnull;
 import java.util.Collection;
+import java.util.List;
 import java.util.Optional;
 
 import static org.apache.commons.lang3.StringUtils.*;
@@ -44,6 +45,7 @@ public class ValueSetConceptAccumulator implements IValueSetConceptAccumulator {
 	private ITermValueSetConceptDesignationDao myValueSetConceptDesignationDao;
 	private int myConceptsSaved;
 	private int myDesignationsSaved;
+	private int myConceptsExcluded;
 
 	public ValueSetConceptAccumulator(@Nonnull TermValueSet theTermValueSet, @Nonnull ITermValueSetDao theValueSetDao, @Nonnull ITermValueSetConceptDao theValueSetConceptDao, @Nonnull ITermValueSetConceptDesignationDao theValueSetConceptDesignationDao) {
 		myTermValueSet = theTermValueSet;
@@ -52,6 +54,7 @@ public class ValueSetConceptAccumulator implements IValueSetConceptAccumulator {
 		myValueSetConceptDesignationDao = theValueSetConceptDesignationDao;
 		myConceptsSaved = 0;
 		myDesignationsSaved = 0;
+		myConceptsExcluded = 0;
 	}
 
 	@Override
@@ -88,6 +91,10 @@ public class ValueSetConceptAccumulator implements IValueSetConceptAccumulator {
 			myTermValueSet.decrementTotalConcepts();
 			myValueSetDao.save(myTermValueSet);
 			ourLog.info("Done excluding [{}|{}] from ValueSet[{}]", concept.getSystem(), concept.getCode(), myTermValueSet.getUrl());
+
+			if (++myConceptsExcluded % 250 == 0) {
+				ourLog.info("Have excluded {} concepts from ValueSet[{}]", myConceptsExcluded, myTermValueSet.getUrl());
+			}
 		}
 	}
 
@@ -138,7 +145,23 @@ public class ValueSetConceptAccumulator implements IValueSetConceptAccumulator {
 		return designation;
 	}
 
-	// TODO: DM 2019-07-16 - We need TermValueSetConceptProperty, similar to TermConceptProperty.
+	public Boolean removeGapsFromConceptOrder() {
-	// TODO: DM 2019-07-16 - We should also populate TermValueSetConceptProperty entities here.
+		if (myConceptsExcluded <= 0) {
-	// TODO: DM 2019-07-30 - Expansions don't include the properties themselves; they are needed to facilitate filters and parameterized expansions.
+			return false;
+		}
+
+		ourLog.info("Removing gaps from concept order for ValueSet[{}]", myTermValueSet.getUrl());
+		int order = 0;
+		List<Long> conceptIds = myValueSetConceptDao.findIdsByTermValueSetId(myTermValueSet.getId());
+		for (Long conceptId : conceptIds) {
+			myValueSetConceptDao.updateOrderById(conceptId, order++);
+		}
+		ourLog.info("Have remove gaps from concept order for {} concepts in ValueSet[{}]", conceptIds.size(), myTermValueSet.getUrl());
+
+		return true;
+	}
+
+	// TODO: DM 2019-07-16 - We may need TermValueSetConceptProperty, similar to TermConceptProperty.
+	// TODO: DM 2019-07-16 - If so, we should also populate TermValueSetConceptProperty entities here.
+	// TODO: DM 2019-07-30 - Expansions don't include the properties themselves; they may be needed to facilitate filters and parameterized expansions.
 }

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

@@ -2332,13 +2332,13 @@ public class TerminologySvcImplR4Test extends BaseJpaR4Test {
 
 			// ...
 
-			concept = termValueSet.getConcepts().get(22 - 2);
+			concept = termValueSet.getConcepts().get(20);
 			ourLog.info("Concept:\n" + concept.toString());
 			assertEquals("http://acme.org", concept.getSystem());
 			assertEquals("8491-3", concept.getCode());
 			assertEquals("Systolic blood pressure 1 hour minimum", concept.getDisplay());
 			assertEquals(1, concept.getDesignations().size());
-			assertEquals(22, concept.getOrder());
+			assertEquals(20, concept.getOrder());
 
 			designation = concept.getDesignations().get(0);
 			assertEquals("nl", designation.getLanguage());
@@ -2347,13 +2347,13 @@ public class TerminologySvcImplR4Test extends BaseJpaR4Test {
 			assertEquals("Synonym", designation.getUseDisplay());
 			assertEquals("Systolische bloeddruk minimaal 1 uur", designation.getValue());
 
-			concept = termValueSet.getConcepts().get(23 - 2);
+			concept = termValueSet.getConcepts().get(21);
 			ourLog.info("Concept:\n" + concept.toString());
 			assertEquals("http://acme.org", concept.getSystem());
 			assertEquals("8492-1", concept.getCode());
 			assertEquals("Systolic blood pressure 8 hour minimum", concept.getDisplay());
 			assertEquals(0, concept.getDesignations().size());
-			assertEquals(23, concept.getOrder());
+			assertEquals(21, concept.getOrder());
 		});
 	}
 
@@ -2434,13 +2434,13 @@ public class TerminologySvcImplR4Test extends BaseJpaR4Test {
 
 			// ...
 
-			concept = termValueSet.getConcepts().get(22 - 2);
+			concept = termValueSet.getConcepts().get(20);
 			ourLog.info("Concept:\n" + concept.toString());
 			assertEquals("http://acme.org", concept.getSystem());
 			assertEquals("8491-3", concept.getCode());
 			assertEquals("Systolic blood pressure 1 hour minimum", concept.getDisplay());
 			assertEquals(1, concept.getDesignations().size());
-			assertEquals(22, concept.getOrder());
+			assertEquals(20, concept.getOrder());
 
 			designation = concept.getDesignations().get(0);
 			assertEquals("nl", designation.getLanguage());
@@ -2449,13 +2449,13 @@ public class TerminologySvcImplR4Test extends BaseJpaR4Test {
 			assertEquals("Synonym", designation.getUseDisplay());
 			assertEquals("Systolische bloeddruk minimaal 1 uur", designation.getValue());
 
-			concept = termValueSet.getConcepts().get(23 - 2);
+			concept = termValueSet.getConcepts().get(21);
 			ourLog.info("Concept:\n" + concept.toString());
 			assertEquals("http://acme.org", concept.getSystem());
 			assertEquals("8492-1", concept.getCode());
 			assertEquals("Systolic blood pressure 8 hour minimum", concept.getDisplay());
 			assertEquals(0, concept.getDesignations().size());
-			assertEquals(23, concept.getOrder());
+			assertEquals(21, concept.getOrder());
 		});
 	}