ValueSet Expansion Should Preserve Order (#2724)

* Add test

* FIxed

* Add changelog

* Add test

* Intermittent test failure fix

* Build fix

* Build fix

* Test fix

* Test fixes

* Fix checkstyle issue

* Test fix

* Add test logging

hapi-deployable-pom/pom.xml

@@ -140,6 +140,25 @@
 					</ignoredResourcePatterns>
 				</configuration>
 			</plugin>
+			<plugin>
+				<groupId>org.apache.maven.plugins</groupId>
+				<artifactId>maven-checkstyle-plugin</artifactId>
+				<executions>
+					<execution>
+						<phase>process-sources</phase>
+						<goals>
+							<goal>checkstyle</goal>
+						</goals>
+						<configuration>
+							<failsOnError>true</failsOnError>
+							<enableRulesSummary>true</enableRulesSummary>
+							<enableSeveritySummary>true</enableSeveritySummary>
+							<consoleOutput>true</consoleOutput>
+							<configLocation>${maven.multiModuleProjectDirectory}/src/checkstyle/checkstyle.xml</configLocation>
+						</configuration>
+					</execution>
+				</executions>
+			</plugin>
 		</plugins>
 	</build>
 

hapi-fhir-cli/hapi-fhir-cli-api/src/main/java/ca/uhn/fhir/cli/BaseApp.java

@@ -36,6 +36,7 @@ import org.fusesource.jansi.Ansi;
 import org.fusesource.jansi.AnsiConsole;
 import org.slf4j.LoggerFactory;
 
+import java.io.PrintStream;
 import java.io.PrintWriter;
 import java.lang.management.ManagementFactory;
 import java.util.ArrayList;
@@ -43,6 +44,7 @@ import java.util.Arrays;
 import java.util.Collections;
 import java.util.List;
 
+import static org.apache.commons.lang3.StringUtils.isNotBlank;
 import static org.fusesource.jansi.Ansi.ansi;
 
 @SuppressWarnings("WeakerAccess")
@@ -67,17 +69,27 @@ public abstract class BaseApp {
 
 	private void logAppHeader() {
 		System.out.flush();
-		System.out.println("------------------------------------------------------------");
+		String msg = "------------------------------------------------------------";
+		printMessageToStdout(msg);
 		logProductName();
-		System.out.println("------------------------------------------------------------");
+		printMessageToStdout("------------------------------------------------------------");
-		System.out.println("Process ID                      : " + ManagementFactory.getRuntimeMXBean().getName());
+		printMessageToStdout("Process ID                      : " + ManagementFactory.getRuntimeMXBean().getName());
-		System.out.println("Max configured JVM memory (Xmx) : " + FileHelper.getFileSizeDisplay(Runtime.getRuntime().maxMemory(), 1));
+		printMessageToStdout("Max configured JVM memory (Xmx) : " + FileHelper.getFileSizeDisplay(Runtime.getRuntime().maxMemory(), 1));
-		System.out.println("Detected Java version           : " + System.getProperty("java.version"));
+		printMessageToStdout("Detected Java version           : " + System.getProperty("java.version"));
-		System.out.println("------------------------------------------------------------");
+		printMessageToStdout("------------------------------------------------------------");
+	}
+
+	private void printMessageToStdout(String theMsg) {
+		PrintStream out = System.out;
+		if (isNotBlank(theMsg)) {
+			out.println(theMsg);
+		} else {
+			out.println();
+		}
 	}
 
 	protected void logProductName() {
-		System.out.println("\ud83d\udd25 " + ansi().bold() + " " + provideProductName() + ansi().boldOff() + " " + provideProductVersion() + " - Command Line Tool");
+		printMessageToStdout("\ud83d\udd25 " + ansi().bold() + " " + provideProductName() + ansi().boldOff() + " " + provideProductVersion() + " - Command Line Tool");
 	}
 
 	private void logCommandUsage(BaseCommand theCommand) {
@@ -99,32 +111,32 @@ public abstract class BaseApp {
 		}
 
 		// Usage
-		System.out.println("Usage:");
+		printMessageToStdout("Usage:");
-		System.out.println("  " + provideCommandName() + " " + theCommand.getCommandName() + " [options]");
+		printMessageToStdout("  " + provideCommandName() + " " + theCommand.getCommandName() + " [options]");
-		System.out.println();
+		printMessageToStdout("");
 
 		// Description
 		String wrapped = WordUtils.wrap(theCommand.getCommandDescription(), columns);
-		System.out.println(wrapped);
+		printMessageToStdout(wrapped);
-		System.out.println();
+		printMessageToStdout("");
 
 		// Usage Notes
 		List<String> usageNotes = theCommand.provideUsageNotes();
 		for (String next : usageNotes) {
 			wrapped = WordUtils.wrap(next, columns);
-			System.out.println(wrapped);
+			printMessageToStdout(wrapped);
-			System.out.println();
+			printMessageToStdout("");
 		}
 
 		// Options
-		System.out.println("Options:");
+		printMessageToStdout("Options:");
 		HelpFormatter fmt = new HelpFormatter();
 		PrintWriter pw = new PrintWriter(System.out);
 		fmt.printOptions(pw, columns, getOptions(theCommand), 2, 2);
 		pw.flush();
 
 		// That's it!
-		System.out.println();
+		printMessageToStdout("");
 	}
 
 	private Options getOptions(BaseCommand theCommand) {
@@ -135,10 +147,10 @@ public abstract class BaseApp {
 
 	private void logUsage() {
 		logAppHeader();
-		System.out.println("Usage:");
+		printMessageToStdout("Usage:");
-		System.out.println("  " + provideCommandName() + " {command} [options]");
+		printMessageToStdout("  " + provideCommandName() + " {command} [options]");
-		System.out.println();
+		printMessageToStdout("");
-		System.out.println("Commands:");
+		printMessageToStdout("Commands:");
 
 		int longestCommandLength = 0;
 		for (BaseCommand next : ourCommands) {
@@ -151,12 +163,12 @@ public abstract class BaseApp {
 			for (int i = 1; i < rightParts.length; i++) {
 				rightParts[i] = StringUtils.leftPad("", left.length() + 3) + rightParts[i];
 			}
-			System.out.println(ansi().bold().fg(Ansi.Color.GREEN) + left + ansi().boldOff().fg(Ansi.Color.WHITE) + " - " + ansi().bold() + StringUtils.join(rightParts, LINESEP));
+			printMessageToStdout(ansi().bold().fg(Ansi.Color.GREEN) + left + ansi().boldOff().fg(Ansi.Color.WHITE) + " - " + ansi().bold() + StringUtils.join(rightParts, LINESEP));
 		}
-		System.out.println();
+		printMessageToStdout("");
-		System.out.println(ansi().boldOff().fg(Ansi.Color.WHITE) + "See what options are available:");
+		printMessageToStdout(ansi().boldOff().fg(Ansi.Color.WHITE) + "See what options are available:");
-		System.out.println("  " + provideCommandName() + " help {command}");
+		printMessageToStdout("  " + provideCommandName() + " help {command}");
-		System.out.println();
+		printMessageToStdout("");
 	}
 
 	protected abstract String provideCommandName();
@@ -235,8 +247,8 @@ public abstract class BaseApp {
 
 		if (command == null) {
 			String message = "Unrecognized command: " + ansi().bold().fg(Ansi.Color.RED) + theArgs[0] + ansi().boldOff().fg(Ansi.Color.WHITE);
-			System.out.println(message);
+			printMessageToStdout(message);
-			System.out.println();
+			printMessageToStdout("");
 			logUsage();
 			exitDueToProblem(message);
 			return;

hapi-fhir-cli/hapi-fhir-cli-app/pom.xml

@@ -119,7 +119,7 @@
 					</execution>
 				</executions>
 			</plugin>
-		</plugins>
+        </plugins>
 	</build>
 
 </project>

hapi-fhir-docs/pom.xml

@@ -141,6 +141,13 @@
 					<skip>true</skip>
 				</configuration>
 			</plugin>
+            <plugin>
+                <groupId>org.apache.maven.plugins</groupId>
+                <artifactId>maven-checkstyle-plugin</artifactId>
+                <configuration>
+                    <skip>true</skip>
+                </configuration>
+            </plugin>
 		</plugins>
 		<resources>
 			<resource>

hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/5_5_0/2724-valueset-expansion-preserve-order.yaml

@@ -0,0 +1,5 @@
+---
+type: fix
+issue: 2624
+title: "ValueSet expansion did not correctly preserve the order if multiple codes were included
+  in a single inclusion block."

hapi-fhir-jacoco/pom.xml

@@ -147,9 +147,9 @@
 			<plugin>
 				<groupId>org.apache.maven.plugins</groupId>
 				<artifactId>maven-checkstyle-plugin</artifactId>
-				<executions>
+				<configuration>
-					<execution><id>validate</id><phase>none</phase></execution>
+                    <skip>true</skip>
-				</executions>
+                </configuration>
 			</plugin>
 		</plugins>
 	</build>

hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/entity/TermConceptPropertyBinder.java

@@ -26,6 +26,8 @@ import org.hibernate.search.mapper.pojo.bridge.PropertyBridge;
 import org.hibernate.search.mapper.pojo.bridge.binding.PropertyBindingContext;
 import org.hibernate.search.mapper.pojo.bridge.mapping.programmatic.PropertyBinder;
 import org.hibernate.search.mapper.pojo.bridge.runtime.PropertyBridgeWriteContext;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 import java.util.Collection;
 
@@ -38,6 +40,7 @@ public class TermConceptPropertyBinder implements PropertyBinder {
 
 
 	public static final String CONCEPT_FIELD_PROPERTY_PREFIX = "PROP";
+	private static final Logger ourLog = LoggerFactory.getLogger(TermConceptPropertyBinder.class);
 
 	@Override
 	public void bind(PropertyBindingContext thePropertyBindingContext) {
@@ -65,10 +68,10 @@ public class TermConceptPropertyBinder implements PropertyBinder {
 			if (properties != null) {
 				for (TermConceptProperty next : properties) {
 					theDocument.addValue(CONCEPT_FIELD_PROPERTY_PREFIX + next.getKey(), next.getValue());
-					System.out.println("Adding Prop: " + CONCEPT_FIELD_PROPERTY_PREFIX + next.getKey() + " -- " + next.getValue());
+					ourLog.trace("Adding Prop: {}{} -- {}", CONCEPT_FIELD_PROPERTY_PREFIX, next.getKey(), next.getValue());
 					if (next.getType() == TermConceptPropertyTypeEnum.CODING && isNotBlank(next.getDisplay())) {
-							theDocument.addValue(CONCEPT_FIELD_PROPERTY_PREFIX + next.getKey(), next.getDisplay());
+						theDocument.addValue(CONCEPT_FIELD_PROPERTY_PREFIX + next.getKey(), next.getDisplay());
-							System.out.println("Adding multivalue Prop: " + CONCEPT_FIELD_PROPERTY_PREFIX + next.getKey() + " -- " + next.getDisplay());
+						ourLog.trace("Adding multivalue Prop: {}{} -- {}", CONCEPT_FIELD_PROPERTY_PREFIX, next.getKey(), next.getDisplay());
 					}
 				}
 			}

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

@@ -132,6 +132,7 @@ import org.springframework.transaction.interceptor.NoRollbackRuleAttribute;
 import org.springframework.transaction.interceptor.RuleBasedTransactionAttribute;
 import org.springframework.transaction.support.TransactionSynchronizationManager;
 import org.springframework.transaction.support.TransactionTemplate;
+import org.springframework.util.comparator.Comparators;
 
 import javax.annotation.Nonnull;
 import javax.annotation.Nullable;
@@ -938,7 +939,15 @@ public abstract class BaseTermReadSvcImpl implements ITermReadSvc {
 			}
 		});
 
-		PredicateFinalStep expansionStep = buildExpansionPredicate(theIncludeOrExclude, predicate);
+		List<String> codes = theIncludeOrExclude
+			.getConcept()
+			.stream()
+			.filter(Objects::nonNull)
+			.map(ValueSet.ConceptReferenceComponent::getCode)
+			.filter(StringUtils::isNotBlank)
+			.collect(Collectors.toList());
+
+		PredicateFinalStep expansionStep = buildExpansionPredicate(codes, predicate);
 		final PredicateFinalStep finishedQuery;
 		if (expansionStep == null) {
 			finishedQuery = step;
@@ -973,9 +982,6 @@ public abstract class BaseTermReadSvcImpl implements ITermReadSvc {
 			}
 		}
 
-//		jpaQuery.setMaxResults(maxResultsPerBatch);
-//		jpaQuery.setFirstResult(theQueryIndex * maxResultsPerBatch);
-
 		ourLog.debug("Beginning batch expansion for {} with max results per batch: {}", (theAdd ? "inclusion" : "exclusion"), maxResultsPerBatch);
 
 		StopWatch swForBatch = new StopWatch();
@@ -984,9 +990,22 @@ public abstract class BaseTermReadSvcImpl implements ITermReadSvc {
 		SearchQuery<TermConcept> termConceptsQuery = searchSession.search(TermConcept.class)
 			.where(f -> finishedQuery).toQuery();
 
-		System.out.println("About to query:" + termConceptsQuery.queryString());
+		ourLog.trace("About to query: {}", termConceptsQuery.queryString());
 		List<TermConcept> termConcepts = termConceptsQuery.fetchHits(theQueryIndex * maxResultsPerBatch, maxResultsPerBatch);
 
+		// If the include section had multiple codes, return the codes in the same order
+		if (codes.size() > 1) {
+			termConcepts = new ArrayList<>(termConcepts);
+			Map<String, Integer> codeToIndex = new HashMap<>(codes.size());
+			for (int i = 0; i < codes.size(); i++) {
+				codeToIndex.put(codes.get(i), i);
+			}
+			termConcepts.sort(((o1, o2) -> {
+				Integer idx1 = codeToIndex.get(o1.getCode());
+				Integer idx2 = codeToIndex.get(o2.getCode());
+				return Comparators.nullsHigh().compare(idx1, idx2);
+			}));
+		}
 
 		int resultsInBatch = termConcepts.size();
 		int firstResult = theQueryIndex * maxResultsPerBatch;// TODO GGG HS we lose the ability to check the index of the first result, so just best-guessing it here.
@@ -1027,17 +1046,13 @@ public abstract class BaseTermReadSvcImpl implements ITermReadSvc {
 	/**
 	 * Helper method which builds a predicate for the expansion
 	 */
-	private PredicateFinalStep buildExpansionPredicate(ValueSet.ConceptSetComponent theTheIncludeOrExclude, SearchPredicateFactory thePredicate) {
+	private PredicateFinalStep buildExpansionPredicate(List<String> theCodes, SearchPredicateFactory thePredicate) {
 		PredicateFinalStep expansionStep;
 		/*
 		 * Include/Exclude Concepts
 		 */
-		List<Term> codes = theTheIncludeOrExclude
+		List<Term> codes = theCodes
-			.getConcept()
 			.stream()
-			.filter(Objects::nonNull)
-			.map(ValueSet.ConceptReferenceComponent::getCode)
-			.filter(StringUtils::isNotBlank)
 			.map(t -> new Term("myCode", t))
 			.collect(Collectors.toList());
 

hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4SearchOptimizedTest.java

@@ -21,6 +21,7 @@ import ca.uhn.fhir.rest.param.StringParam;
 import ca.uhn.fhir.rest.param.TokenParam;
 import ca.uhn.fhir.rest.server.exceptions.InternalErrorException;
 import org.apache.commons.lang3.StringUtils;
+import org.apache.commons.lang3.exception.ExceptionUtils;
 import org.hl7.fhir.instance.model.api.IAnyResource;
 import org.hl7.fhir.instance.model.api.IIdType;
 import org.hl7.fhir.r4.model.DateTimeType;
@@ -55,6 +56,7 @@ import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertNotEquals;
 import static org.junit.jupiter.api.Assertions.assertNotNull;
 import static org.junit.jupiter.api.Assertions.assertNull;
+import static org.junit.jupiter.api.Assertions.fail;
 
 
 public class FhirResourceDaoR4SearchOptimizedTest extends BaseJpaR4Test {
@@ -673,7 +675,8 @@ public class FhirResourceDaoR4SearchOptimizedTest extends BaseJpaR4Test {
 		for (Future<Throwable> next : futures) {
 			Throwable t = next.get();
 			if (t != null) {
-				throw t;
+				String stackTrace = ExceptionUtils.getStackTrace(t);
+				fail(t.toString() + "\n" + stackTrace);
 			}
 		}
 		executor.shutdownNow();

hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4SearchWithLuceneDisabledTest.java

@@ -2,6 +2,7 @@ package ca.uhn.fhir.jpa.dao.r4;
 
 import ca.uhn.fhir.context.FhirContext;
 import ca.uhn.fhir.context.support.IValidationSupport;
+import ca.uhn.fhir.context.support.ValueSetExpansionOptions;
 import ca.uhn.fhir.jpa.api.config.DaoConfig;
 import ca.uhn.fhir.jpa.api.dao.IFhirResourceDao;
 import ca.uhn.fhir.jpa.api.dao.IFhirResourceDaoValueSet;
@@ -13,6 +14,7 @@ import ca.uhn.fhir.jpa.dao.BaseJpaTest;
 import ca.uhn.fhir.jpa.dao.dstu2.FhirResourceDaoDstu2SearchNoFtTest;
 import ca.uhn.fhir.jpa.search.reindex.IResourceReindexingSvc;
 import ca.uhn.fhir.jpa.searchparam.SearchParameterMap;
+import ca.uhn.fhir.jpa.term.ValueSetExpansionR4Test;
 import ca.uhn.fhir.rest.server.util.ISearchParamRegistry;
 import ca.uhn.fhir.jpa.sp.ISearchParamPresenceSvc;
 import ca.uhn.fhir.jpa.term.api.ITermReadSvc;
@@ -32,6 +34,7 @@ import org.hl7.fhir.r4.model.AuditEvent;
 import org.hl7.fhir.r4.model.Bundle;
 import org.hl7.fhir.r4.model.CarePlan;
 import org.hl7.fhir.r4.model.CodeSystem;
+import org.hl7.fhir.r4.model.CodeType;
 import org.hl7.fhir.r4.model.CompartmentDefinition;
 import org.hl7.fhir.r4.model.ConceptMap;
 import org.hl7.fhir.r4.model.Condition;
@@ -60,6 +63,9 @@ import java.io.InputStream;
 import java.nio.charset.StandardCharsets;
 import java.util.List;
 
+import static ca.uhn.fhir.util.HapiExtensions.EXT_VALUESET_EXPANSION_MESSAGE;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.contains;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.fail;
 
@@ -263,6 +269,44 @@ public class FhirResourceDaoR4SearchWithLuceneDisabledTest extends BaseJpaTest {
 		}
 	}
 
+	@Test
+	public void testExpandValueSetPreservesExplicitOrder() {
+		CodeSystem cs = new CodeSystem();
+		cs.setId("cs");
+		cs.setUrl("http://cs");
+		cs.addConcept().setCode("code1");
+		cs.addConcept().setCode("code2");
+		cs.addConcept().setCode("code3");
+		cs.addConcept().setCode("code4");
+		cs.addConcept().setCode("code5");
+		myCodeSystemDao.update(cs);
+
+		// Vs in reverse order
+		ValueSet vs = new ValueSet();
+		vs.setId("vs");
+		vs.setUrl("http://vs");
+		// Add some codes in separate compose sections, and some more codes in a single compose section.
+		// Order should be preserved for all of them.
+		vs.getCompose().addInclude().setSystem("http://cs")
+			.addConcept(new ValueSet.ConceptReferenceComponent(new CodeType("code5")));
+		vs.getCompose().addInclude().setSystem("http://cs")
+			.addConcept(new ValueSet.ConceptReferenceComponent(new CodeType("code4")));
+		vs.getCompose().addInclude().setSystem("http://cs")
+			.addConcept(new ValueSet.ConceptReferenceComponent(new CodeType("code3")))
+			.addConcept(new ValueSet.ConceptReferenceComponent(new CodeType("code2")))
+			.addConcept(new ValueSet.ConceptReferenceComponent(new CodeType("code1")));
+		myValueSetDao.update(vs);
+
+		// Non Pre-Expanded
+		ValueSet outcome = myValueSetDao.expand(vs, new ValueSetExpansionOptions());
+		assertEquals("ValueSet \"ValueSet.url[http://vs]\" has not yet been pre-expanded. Performing in-memory expansion without parameters. Current status: NOT_EXPANDED | The ValueSet is waiting to be picked up and pre-expanded by a scheduled task.", outcome.getMeta().getExtensionString(EXT_VALUESET_EXPANSION_MESSAGE));
+		assertThat(ValueSetExpansionR4Test.toCodes(outcome).toString(), ValueSetExpansionR4Test.toCodes(outcome), contains(
+			"code5", "code4", "code3", "code2", "code1"
+		));
+
+
+	}
+
 	@Test
 	public void testSearchByCodeIn() {
 		CodeSystem cs = new CodeSystem();

hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/r5/ResourceProviderR5Test.java

@@ -11,6 +11,7 @@ import java.util.ArrayList;
 import java.util.List;
 import java.util.stream.Collectors;
 
+import ca.uhn.fhir.rest.server.exceptions.ResourceVersionConflictException;
 import org.apache.commons.io.IOUtils;
 import org.apache.http.client.methods.CloseableHttpResponse;
 import org.apache.http.client.methods.HttpGet;
@@ -136,13 +137,7 @@ public class ResourceProviderR5Test extends BaseResourceProviderR5Test {
 		assertEquals(response0.getId(), response1.getId());
 
 		// Pretend the search was errored out
-		runInTransaction(() -> {
+		markSearchErrored();
-			assertEquals(1L, mySearchEntityDao.count());
-			Search search = mySearchEntityDao.findAll().iterator().next();
-			search.setStatus(SearchStatusEnum.FAILED);
-			search.setFailureMessage("Some Failure Message");
-			search.setFailureCode(501);
-		});
 
 		// Perform the search again (shouldn't return the errored out search)
 		Bundle response3 = myClient.search()
@@ -155,6 +150,25 @@ public class ResourceProviderR5Test extends BaseResourceProviderR5Test {
 
 	}
 
+	private void markSearchErrored() {
+		while (true) {
+			try {
+				runInTransaction(() -> {
+					assertEquals(1L, mySearchEntityDao.count());
+					Search search = mySearchEntityDao.findAll().iterator().next();
+					search.setStatus(SearchStatusEnum.FAILED);
+					search.setFailureMessage("Some Failure Message");
+					search.setFailureCode(501);
+					mySearchEntityDao.save(search);
+				});
+				break;
+			} catch (ResourceVersionConflictException e) {
+				ourLog.warn("Conflict while updating search: " + e);
+				continue;
+			}
+		}
+	}
+
 	@Test
 	public void testErroredSearchReturnsAppropriateResponse() {
 		Patient pt1 = new Patient();
@@ -174,14 +188,11 @@ public class ResourceProviderR5Test extends BaseResourceProviderR5Test {
 			.execute();
 		assertEquals(1, response0.getEntry().size());
 
+		// Make sure it works for now
+		myClient.loadPage().next(response0).execute();
+
 		// Pretend the search was errored out
-		runInTransaction(() -> {
+		markSearchErrored();
-			assertEquals(1L, mySearchEntityDao.count());
-			Search search = mySearchEntityDao.findAll().iterator().next();
-			search.setStatus(SearchStatusEnum.FAILED);
-			search.setFailureMessage("Some Failure Message");
-			search.setFailureCode(501);
-		});
 
 		// Request the second page
 		try {

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

@@ -19,6 +19,7 @@ import ca.uhn.fhir.util.HapiExtensions;
 import com.google.common.collect.Lists;
 import org.hl7.fhir.instance.model.api.IIdType;
 import org.hl7.fhir.r4.model.CodeSystem;
+import org.hl7.fhir.r4.model.CodeType;
 import org.hl7.fhir.r4.model.Extension;
 import org.hl7.fhir.r4.model.ValueSet;
 import org.hl7.fhir.r4.model.codesystems.HttpVerb;
@@ -38,6 +39,7 @@ import java.util.List;
 import java.util.Optional;
 import java.util.stream.Collectors;
 
+import static ca.uhn.fhir.util.HapiExtensions.EXT_VALUESET_EXPANSION_MESSAGE;
 import static org.hamcrest.CoreMatchers.containsString;
 import static org.hamcrest.CoreMatchers.equalTo;
 import static org.hamcrest.CoreMatchers.is;
@@ -444,11 +446,6 @@ public class ValueSetExpansionR4Test extends BaseTermR4Test {
 		assertThat(lastSelectQuery, containsString(" like '%display value 9%'"));
 	}
 
-	@Nonnull
-	public List<String> toCodes(ValueSet theExpandedValueSet) {
-		return theExpandedValueSet.getExpansion().getContains().stream().map(t -> t.getCode()).collect(Collectors.toList());
-	}
-
 	@SuppressWarnings("SpellCheckingInspection")
 	@Test
 	public void testExpandTermValueSetAndChildren() throws Exception {
@@ -514,7 +511,6 @@ public class ValueSetExpansionR4Test extends BaseTermR4Test {
 		assertEquals(3, expandedValueSet.getExpansion().getContains().size());
 	}
 
-
 	@Test
 	public void testExpandExistingValueSetNotPreExpanded() throws Exception {
 		loadAndPersistCodeSystemAndValueSetWithDesignations(HttpVerb.POST);
@@ -870,7 +866,6 @@ public class ValueSetExpansionR4Test extends BaseTermR4Test {
 		assertEquals("Unknown CodeSystem URI \"http://unknown-system\" referenced from ValueSet", extensionByUrl.getValueAsPrimitive().getValueAsString());
 	}
 
-
 	@Test
 	public void testExpandTermValueSetAndChildrenWithOffsetAndCountWithClientAssignedId() throws Exception {
 		myDaoConfig.setPreExpandValueSets(true);
@@ -955,6 +950,54 @@ public class ValueSetExpansionR4Test extends BaseTermR4Test {
 		verify(myValueSetCodeAccumulator, times(9)).includeConceptWithDesignations(anyString(), anyString(), nullable(String.class), anyCollection(), nullable(Long.class), nullable(String.class));
 	}
 
+	@Test
+	public void testExpandValueSetPreservesExplicitOrder() {
+		CodeSystem cs = new CodeSystem();
+		cs.setId("cs");
+		cs.setUrl("http://cs");
+		cs.addConcept().setCode("code1");
+		cs.addConcept().setCode("code2");
+		cs.addConcept().setCode("code3");
+		cs.addConcept().setCode("code4");
+		cs.addConcept().setCode("code5");
+		myCodeSystemDao.update(cs);
+
+		// Vs in reverse order
+		ValueSet vs = new ValueSet();
+		vs.setId("vs");
+		vs.setUrl("http://vs");
+		// Add some codes in separate compose sections, and some more codes in a single compose section.
+		// Order should be preserved for all of them.
+		vs.getCompose().addInclude().setSystem("http://cs")
+			.addConcept(new ValueSet.ConceptReferenceComponent(new CodeType("code5")));
+		vs.getCompose().addInclude().setSystem("http://cs")
+			.addConcept(new ValueSet.ConceptReferenceComponent(new CodeType("code4")));
+		vs.getCompose().addInclude().setSystem("http://cs")
+			.addConcept(new ValueSet.ConceptReferenceComponent(new CodeType("code3")))
+			.addConcept(new ValueSet.ConceptReferenceComponent(new CodeType("code2")))
+			.addConcept(new ValueSet.ConceptReferenceComponent(new CodeType("code1")));
+		myValueSetDao.update(vs);
+
+		// Non Pre-Expanded
+		ValueSet outcome = myValueSetDao.expand(vs, new ValueSetExpansionOptions());
+		assertEquals("ValueSet \"ValueSet.url[http://vs]\" has not yet been pre-expanded. Performing in-memory expansion without parameters. Current status: NOT_EXPANDED | The ValueSet is waiting to be picked up and pre-expanded by a scheduled task.", outcome.getMeta().getExtensionString(EXT_VALUESET_EXPANSION_MESSAGE));
+		assertThat(toCodes(outcome).toString(), toCodes(outcome), contains(
+			"code5", "code4", "code3", "code2", "code1"
+		));
+
+		myTermSvc.preExpandDeferredValueSetsToTerminologyTables();
+
+		// Pre-Expanded
+		myCaptureQueriesListener.clear();
+		outcome = myValueSetDao.expand(vs, new ValueSetExpansionOptions());
+		myCaptureQueriesListener.logSelectQueriesForCurrentThread();
+		assertEquals("ValueSet was expanded using a pre-calculated expansion", outcome.getMeta().getExtensionString(EXT_VALUESET_EXPANSION_MESSAGE));
+		assertThat(toCodes(outcome).toString(), toCodes(outcome), contains(
+			"code5", "code4", "code3", "code2", "code1"
+		));
+
+	}
+
 	@Test
 	public void testStoreTermCodeSystemAndChildren() throws Exception {
 		loadAndPersistCodeSystemWithDesignations(HttpVerb.POST);
@@ -1431,5 +1474,10 @@ public class ValueSetExpansionR4Test extends BaseTermR4Test {
 		});
 	}
 
+	@Nonnull
+	public static List<String> toCodes(ValueSet theExpandedValueSet) {
+		return theExpandedValueSet.getExpansion().getContains().stream().map(t -> t.getCode()).collect(Collectors.toList());
+	}
+
 
 }

pom.xml

@@ -2233,12 +2233,9 @@
 						<dependency>
 							<groupId>com.puppycrawl.tools</groupId>
 							<artifactId>checkstyle</artifactId>
-							<version>8.42</version>
+							<version>8.43</version>
 						</dependency>
 					</dependencies>
-					<configuration>
-						<configLocation>${project.basedir}/src/checkstyle/checkstyle.xml</configLocation>
-					</configuration>
 				</plugin>
 				<plugin>
 					<groupId>org.apache.maven.plugins</groupId>
@@ -2818,7 +2815,7 @@
 								<id>validate</id>
 								<phase>generate-sources</phase>
 								<configuration>
-									<configLocation>src/checkstyle/checkstyle_config_nofixmes.xml</configLocation>
+									<configLocation>${maven.multiModuleProjectDirectory}/src/checkstyle/checkstyle_config_nofixmes.xml</configLocation>
 									<encoding>UTF-8</encoding>
 									<consoleOutput>true</consoleOutput>
 									<failOnViolation>true</failOnViolation>

src/checkstyle/checkstyle.xml

@@ -5,11 +5,16 @@
 
 <module name="Checker">
 	<property name="charset" value="UTF-8"/>
+	<property name="cacheFile" value="target/cache_non_main_files"/>
 
-	<property name="severity" value="warning"/>
+	<module name="TreeWalker">
-
+		<module name="RegexpSinglelineJava">
-	<!--<property name="fileExtensions" value="java, properties, xml"/> -->
+			<property name="format" value="System\.out\.println"/>
+			<property name="ignoreComments" value="true"/>
+		</module>
+	</module>
 
+	<!--
 	<module name="TreeWalker">
 		<property name="tabWidth" value="3"/>
 		<module name="OuterTypeFilename"/>
@@ -40,7 +45,7 @@
 			<property name="format" value="org.jetbrains.annotations.Nullable"/>
 			<property name="message" value="Incorrect Nullable annotation used: The &quot;javax.annotation.Nullable&quot; annotation should be used for nullable things"/>
 		</module>
-		<!--<module name="AvoidStarImport"/> -->
+
 		<module name="OneTopLevelClass"/>
 		<module name="NoLineWrap"/>
 		<module name="EmptyBlock">
@@ -49,28 +54,15 @@
 				value="LITERAL_TRY, LITERAL_FINALLY, LITERAL_IF, LITERAL_ELSE, LITERAL_SWITCH"/>
 		</module>
 		<module name="NeedBraces"/>
-		<!--
-		<module name="LeftCurly">
-			<property name="maxLineLength" value="300"/>
-		</module>
-		-->
 		<module name="RightCurly"/>
 		<module name="RightCurly">
 			<property name="option" value="alone"/>
 			<property name="tokens"
 				value="CLASS_DEF, METHOD_DEF, CTOR_DEF, LITERAL_FOR, LITERAL_WHILE, LITERAL_DO, STATIC_INIT, INSTANCE_INIT"/>
 		</module>
-		<!-- <module name="WhitespaceAround"> <property name="allowEmptyConstructors" 
-			value="true"/> <property name="allowEmptyMethods" value="true"/> <property 
-			name="allowEmptyTypes" value="true"/> <property name="allowEmptyLoops" value="true"/> 
-			<message key="ws.notFollowed" value="WhitespaceAround: ''{0}'' is not followed 
-			by whitespace. Empty blocks may only be represented as '{}' when not part 
-			of a multi-block statement (4.1.3)"/> <message key="ws.notPreceded" value="WhitespaceAround: 
-			''{0}'' is not preceded with whitespace."/> </module> -->
 		<module name="OneStatementPerLine"/>
 		<module name="MultipleVariableDeclarations"/>
 		<module name="ArrayTypeStyle"/>
-		<!-- <module name="MissingSwitchDefault"/> -->
 		<module name="FallThrough"/>
 		<module name="UpperEll"/>
 		<module name="ModifierOrder"/>
@@ -145,10 +137,6 @@
 		</module>
 		<module name="OverloadMethodsDeclarationOrder"/>
 		<module name="VariableDeclarationUsageDistance"/>
-		<!-- <module name="CustomImportOrder"> <property name="specialImportsRegExp" 
-			value="com.google"/> <property name="sortImportsInGroupAlphabetically" value="true"/> 
-			<property name="customImportOrderRules" value="STATIC###SPECIAL_IMPORTS###THIRD_PARTY_PACKAGE###STANDARD_JAVA_PACKAGE"/> 
-			</module> -->
 		<module name="MethodParamPad"/>
 		<module name="OperatorWrap">
 			<property name="option" value="NL"/>
@@ -174,21 +162,13 @@
 			<property name="target"
 				value="CLASS_DEF, INTERFACE_DEF, ENUM_DEF, METHOD_DEF, CTOR_DEF, VARIABLE_DEF"/>
 		</module>
-		<!-- <module name="JavadocMethod"> <module name="JavadocParagraph"/> <property 
-			name="scope" value="public"/> <property name="allowMissingParamTags" value="true"/> 
-			<property name="allowMissingThrowsTags" value="true"/> <property name="allowMissingReturnTag" 
-			value="true"/> <property name="minLineCount" value="2"/> <property name="allowedAnnotations" 
-			value="Override, Test"/> <property name="allowThrowsTagsForSubclasses" value="true"/> 
-			</module> -->
 		<module name="MethodName">
 			<property name="format" value="^[a-z][a-z0-9][a-zA-Z0-9_]*$"/>
 			<message key="name.invalidPattern"
 				value="Method name ''{0}'' must match pattern ''{1}''."/>
 		</module>
 		<module name="SingleLineJavadoc"/>
-		<!--<property name="ignoreInlineTags" value="false"/> -->
-		<!--<module name="EmptyCatchBlock"> <property name="exceptionVariableName" 
-			value="expected"/> </module> -->
 	</module>
+	-->
 </module>