Preparatory refactors and deprecations for ValidationContext split

org.hl7.fhir.validation.cli/src/main/java/org/hl7/fhir/validation/cli/ValidatorCli.java

@@ -362,14 +362,10 @@ private void readParamsAndExecuteTask(ValidationContext validationContext, Strin
 
     final CliTask cliTask = selectCliTask(validationContext, params);
 
-
-
     if (cliTask instanceof ValidationEngineTask) {
       TimeTracker tt = new TimeTracker();
       TimeTracker.Session tts = tt.start("Loading");
-      if (validationContext.getSv() == null) {
-        validationContext.setSv(myValidationService.determineVersion(validationContext));
-      }
+
       ValidationEngine validationEngine = getValidationEngine(tt, validationContext);
       tts.end();
       ((ValidationEngineTask) cliTask).executeTask(myValidationService, validationEngine, validationContext, params);
@@ -405,8 +401,7 @@ private ValidationEngine getValidationEngine(TimeTracker tt, ValidationContext v
     }
 
     log.info("Loading");
-    String definitions = "dev".equals(validationContext.getSv()) ? "hl7.fhir.r5.core#current" : VersionUtilities.packageForVersion(validationContext.getSv()) + "#" + VersionUtilities.getCurrentVersion(validationContext.getSv());
-    validationEngine = myValidationService.initializeValidator(validationContext, definitions, tt);
+    validationEngine = myValidationService.getValidationEngineFromValidationContext(validationContext, tt);
     return validationEngine;
   }
 

org.hl7.fhir.validation.cli/src/test/java/org/hl7/fhir/validation/cli/ValidatorCliTests.java

@@ -2,7 +2,6 @@
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.mockito.ArgumentMatchers.any;
-import static org.mockito.ArgumentMatchers.anyString;
 import static org.mockito.ArgumentMatchers.eq;
 import static org.mockito.ArgumentMatchers.same;
 import static org.mockito.Mockito.mock;
@@ -153,8 +152,7 @@ protected List<CliTask> getCliTasks() {
     return validatorCli;
   }
   public ValidatorCli mockValidatorCliWithService(ValidationContext validationContext) throws Exception {
-    when(validationService.determineVersion(Mockito.same(validationContext))).thenReturn("5.0.1");
-    when(validationService.initializeValidator(Mockito.same(validationContext), anyString(), any(org.hl7.fhir.utilities.TimeTracker.class))).thenReturn(validationEngine);
+    when(validationService.getValidationEngineFromValidationContext(Mockito.same(validationContext), any(org.hl7.fhir.utilities.TimeTracker.class))).thenReturn(validationEngine);
     return mockValidatorCli();
   }
 
@@ -178,7 +176,6 @@ public void transformTest() throws Exception {
     ValidationContext validationContext = Params.loadValidationContext(args);
     ValidatorCli cli = mockValidatorCliWithService(validationContext);
     cli.readGlobalParamsAndExecuteTask(validationContext, args);
-    Mockito.verify(validationService).determineVersion(same(validationContext));
     Mockito.verify(validationService).transform(same(validationContext), same(validationEngine));
   }
 
@@ -188,7 +185,6 @@ public void narrativeTest() throws Exception {
     ValidationContext validationContext = Params.loadValidationContext(args);
     ValidatorCli cli = mockValidatorCliWithService(validationContext);
     cli.readGlobalParamsAndExecuteTask(validationContext, args);
-    Mockito.verify(validationService).determineVersion(same(validationContext));
     Mockito.verify(validationService).generateNarrative(same(validationContext), same(validationEngine));
   }
 
@@ -198,7 +194,6 @@ public void compileTest() throws Exception {
     ValidationContext validationContext = Params.loadValidationContext(args);
     ValidatorCli cli = mockValidatorCliWithService(validationContext);
     cli.readGlobalParamsAndExecuteTask(validationContext, args);
-    Mockito.verify(validationService).determineVersion(same(validationContext));
     Mockito.verify(validationService).compile(same(validationContext), same(validationEngine));
   }
 
@@ -208,7 +203,6 @@ public void convertTest() throws Exception {
     ValidationContext validationContext = Params.loadValidationContext(args);
     ValidatorCli cli = mockValidatorCliWithService(validationContext);
     cli.readGlobalParamsAndExecuteTask(validationContext, args);
-    Mockito.verify(validationService).determineVersion(same(validationContext));
     Mockito.verify(validationService).convertSources(validationContext,validationEngine);
   }
   @Test
@@ -217,7 +211,6 @@ public void snapshotTest() throws Exception {
     ValidationContext validationContext = Params.loadValidationContext(args);
     ValidatorCli cli = mockValidatorCliWithService(validationContext);
     cli.readGlobalParamsAndExecuteTask(validationContext, args);
-    Mockito.verify(validationService).determineVersion(same(validationContext));
     Mockito.verify(validationService).generateSnapshot(same(validationContext), same(validationEngine));
   }
 
@@ -227,7 +220,6 @@ public void installTest() throws Exception {
     ValidationContext validationContext = Params.loadValidationContext(args);
     ValidatorCli cli = mockValidatorCliWithService(validationContext);
     cli.readGlobalParamsAndExecuteTask(validationContext, args);
-    Mockito.verify(validationService).determineVersion(same(validationContext));
     Mockito.verify(validationService).install(same(validationContext), same(validationEngine));
   }
 
@@ -237,7 +229,6 @@ public void spreadsheetTest() throws Exception {
     ValidationContext validationContext = Params.loadValidationContext(args);
     ValidatorCli cli = mockValidatorCliWithService(validationContext);
     cli.readGlobalParamsAndExecuteTask(validationContext, args);
-    Mockito.verify(validationService).determineVersion(same(validationContext));
     Mockito.verify(validationService).generateSpreadsheet(same(validationContext), same(validationEngine));
   }
 
@@ -247,7 +238,6 @@ public void fhirpathTest() throws Exception {
     ValidationContext validationContext = Params.loadValidationContext(args);
     ValidatorCli cli = mockValidatorCliWithService(validationContext);
     cli.readGlobalParamsAndExecuteTask(validationContext, args);
-    Mockito.verify(validationService).determineVersion(same(validationContext));
     Mockito.verify(validationService).evaluateFhirpath(same(validationContext), same(validationEngine));
   }
 
@@ -257,7 +247,6 @@ public void versionTest() throws Exception {
     ValidationContext validationContext = Params.loadValidationContext(args);
     ValidatorCli cli = mockValidatorCliWithService(validationContext);
     cli.readGlobalParamsAndExecuteTask(validationContext, args);
-    Mockito.verify(validationService).determineVersion(same(validationContext));
     Mockito.verify(validationService).transformVersion(same(validationContext), same(validationEngine));
   }
 
@@ -267,7 +256,6 @@ public void langTransformTest() throws Exception {
     ValidationContext validationContext = Params.loadValidationContext(args);
     ValidatorCli cli = mockValidatorCliWithService(validationContext);
     cli.readGlobalParamsAndExecuteTask(validationContext, args);
-    Mockito.verify(validationService).determineVersion(same(validationContext));
     Mockito.verify(validationService).transformLang(same(validationContext), same(validationEngine));
   }
 
@@ -280,7 +268,6 @@ public void defaultTest() throws Exception {
     int watchScanDelay = 1000;
     int watchSettleTime = 100;
     cli.readGlobalParamsAndExecuteTask(validationContext, args);
-    Mockito.verify(validationService).determineVersion(same(validationContext));
     Mockito.verify(validationService).validateSources(same(validationContext), same(validationEngine), eq(watchMode), eq(watchScanDelay), eq(watchSettleTime));
   }
 
@@ -290,7 +277,6 @@ public void scanTest() throws Exception {
     ValidationContext validationContext = Params.loadValidationContext(args);
     ValidatorCli cli = mockValidatorCliWithService(validationContext);
     cli.readGlobalParamsAndExecuteTask(validationContext, args);
-    Mockito.verify(validationService).determineVersion(same(validationContext));
     Mockito.verify(scanTask).executeTask(same(validationService), same(validationEngine), same(validationContext), eq(args));
   }
 
@@ -300,7 +286,6 @@ public void specialTest() throws Exception {
     ValidationContext validationContext = Params.loadValidationContext(args);
     ValidatorCli cli = mockValidatorCli();
     cli.readGlobalParamsAndExecuteTask(validationContext, args);
-
     Mockito.verify(specialTask).executeTask(same(validationContext), eq(args));
   }
 
@@ -310,7 +295,6 @@ public void compareTest() throws Exception {
     ValidationContext validationContext = Params.loadValidationContext(args);
     ValidatorCli cli = mockValidatorCliWithService(validationContext);
     cli.readGlobalParamsAndExecuteTask(validationContext, args);
-    Mockito.verify(validationService).determineVersion(same(validationContext));
     Mockito.verify(compareTask).executeTask(same(validationService), same(validationEngine), same(validationContext), eq(args));
   }
 
@@ -320,7 +304,6 @@ public void preloadCacheTest() throws Exception {
     ValidationContext validationContext = Params.loadValidationContext(args);
     ValidatorCli cli = mockValidatorCli();
     cli.readGlobalParamsAndExecuteTask(validationContext, args);
-
     Mockito.verify(preloadCacheTask).executeTask(same(validationContext), eq(args));
   }
 
@@ -330,7 +313,6 @@ public void txTestsTest() throws Exception {
     ValidationContext validationContext = Params.loadValidationContext(args);
     ValidatorCli cli = mockValidatorCli();
     cli.readGlobalParamsAndExecuteTask(validationContext, args);
-
     Mockito.verify(txTestsTask).executeTask(same(validationContext), eq(args));
   }
 
@@ -341,7 +323,6 @@ public void testsTest() throws Exception {
     ValidationContext validationContext = Params.loadValidationContext(args);
     ValidatorCli cli = mockValidatorCli();
     cli.readGlobalParamsAndExecuteTask(validationContext, args);
-
     Mockito.verify(testsTask).executeTask(same(validationContext), eq(args));
   }
 }

org.hl7.fhir.validation/src/main/java/org/hl7/fhir/validation/service/ValidationService.java

@@ -142,7 +142,7 @@ public ValidationService(SessionCache cache) {
   public ValidationResponse validateSources(ValidationRequest request) throws Exception {
 
     TimeTracker timeTracker = new TimeTracker();
-    String sessionId = initializeValidator(request.getValidationContext(), null, timeTracker, request.sessionId);
+    String sessionId = initializeValidator(request.getValidationContext(), timeTracker, request.sessionId);
     ValidationEngine validationEngine = sessionCache.fetchSessionValidatorEngine(sessionId);
 
     /* Cached validation engines already have expensive setup like loading definitions complete. But it wouldn't make
@@ -530,47 +530,112 @@ public void transformVersion(ValidationContext validationContext, ValidationEngi
     }
   }
 
+  @Deprecated
   public ValidationEngine initializeValidator(ValidationContext validationContext, String definitions, TimeTracker tt) throws Exception {
     return sessionCache.fetchSessionValidatorEngine(initializeValidator(validationContext, definitions, tt, null));
   }
+  /**
+   * Check if there is an entry in the session cache for the passed sessionId and return the key for that session if it
+   * exists. If no entry exists, one will be created from the passed validationContext with a new key, which will be
+   * returned.
+   *
+   * @param validationContext a ValidationContext to be used to generate a ValidationEngine
+   * @param timeTracker an existing TimeTracker to associate with a newly generated ValidationEngine
+   * @param sessionId a key used to look for existing ValidationEngine instances in the cache
+   * @return a sessionId associated with an appropriate ValidationEngine
+   *
+   * @deprecated This method is based on {@link ValidationContext}, which will eventually be replaced.
+   */
+  @Deprecated
+  public String initializeValidator(ValidationContext validationContext, TimeTracker timeTracker, String sessionId) throws Exception {
+    return initializeValidator(validationContext, null, timeTracker, sessionId);
+  }
 
-  public String initializeValidator(ValidationContext validationContext, String definitions, TimeTracker tt, String sessionId) throws Exception {
-    tt.milestone();
-
-    if (!sessionCache.sessionExists(sessionId)) {
+  /**
+   * This method is maintained for backward compatibility, and allows the FHIR definitions string to be explicitly set.
+   * It is strongly advised to use the
+   * {@link ValidationService#initializeValidator(ValidationContext, TimeTracker, String)} method, which sets this
+   * automatically based on the FHIR version information included in the ValidationContext.
+   *
+   * @see ValidationService#initializeValidator(ValidationContext, TimeTracker, String)
+   *
+   * @param definitions explicit FHIR definitions to be used.
+   * @return a sessionId associated with an appropriate ValidationEngine
+   *
+   * @deprecated This method is based on {@link ValidationContext}, which will eventually be replaced.
+   */
+  @Deprecated
+  public String initializeValidator(ValidationContext validationContext, String definitions, TimeTracker timeTracker, String sessionId) throws Exception {
+    timeTracker.milestone();
+
+    if (sessionCache.sessionExists(sessionId)) {
+      log.info("Cached session exists for session id " + sessionId + ", returning stored validator session id. Cache size = " + sessionCache.getSessionIds().size());
+      return sessionId;
+    }
       if (sessionId != null) {
         log.info("No such cached session exists for session id " + sessionId + ", re-instantiating validator.");
       }
       sessionCache.cleanUp();
-      if (validationContext.getSv() == null) {
-        String sv = determineVersion(validationContext);
-        validationContext.setSv(sv);
-      }
-      final String engineDefinitions = definitions != null ? definitions : VersionUtilities.packageForVersion(validationContext.getSv()) + "#" + VersionUtilities.getCurrentVersion(validationContext.getSv());
 
-      ValidationEngine validationEngine = getValidationEngineFromValidationContext(validationContext, engineDefinitions, tt);
-      sessionId = sessionCache.cacheSession(validationEngine);
-      log.info("Cached new session. Cache size = " + sessionCache.getSessionIds().size());
+      final ValidationEngine validationEngine = getValidationEngineFromValidationContext(validationContext, definitions, timeTracker);
+      final String newSessionId = sessionCache.cacheSession(validationEngine);
+      log.info("Cached new session with ID " + newSessionId + " . Cache size = " + sessionCache.getSessionIds().size());
+    return newSessionId;
+  }
 
-    } else {
-      log.info("Cached session exists for session id " + sessionId + ", returning stored validator session id. Cache size = " + sessionCache.getSessionIds().size());
-    }
-    return sessionId;
+  /**
+   * Create and return a ValidationEngine based on the ValidationContext.
+   * <p/>
+   * If this service contains an existing ValidationEngine indexed by the value passed in validationContext.baseEngine,
+   * the existing engine will be copied and returned instead.
+   *
+   * @param validationContext a ValidationContext to be used to generate the ValidationEngine
+   * @param timeTracker an existing TimeTracker to associate with a newly generated ValidationEngine
+   * @return the generated ValidationEngine
+   *
+   * @deprecated This method is based on {@link ValidationContext}, which will eventually be replaced.
+   */
+  @Deprecated
+  public ValidationEngine getValidationEngineFromValidationContext(ValidationContext validationContext, TimeTracker timeTracker) throws Exception {
+    return getValidationEngineFromValidationContext(validationContext, null, timeTracker);
   }
 
-  private ValidationEngine getValidationEngineFromValidationContext(ValidationContext validationContext, String definitions, TimeTracker tt) throws Exception {
+  /**
+   * This method is maintained for backward compatibility, and allows the FHIR definitions string to be explicitly set.
+   * It is strongly advised to use the
+   * {@link ValidationService#getValidationEngineFromValidationContext(ValidationContext, TimeTracker)} method, which
+   * sets this automatically based on the FHIR versioninformation included in the ValidationContext.
+   *
+   * @param definitions explicit FHIR definitions to be used.
+   *
+   * @see ValidationService#getValidationEngineFromValidationContext(ValidationContext, TimeTracker)
+   *
+   * @deprecated This method is based on {@link ValidationContext}, which will eventually be replaced.
+   */
+  @Deprecated
+  public ValidationEngine getValidationEngineFromValidationContext(ValidationContext validationContext, String definitions, TimeTracker timeTracker) throws Exception {
+    if (validationContext.getSv() == null) {
+      final String sv = determineVersion(validationContext);
+      validationContext.setSv(sv);
+    }
+
     ValidationEngine validationEngine;
     if (validationContext.getBaseEngine() != null && hasBaseEngineForKey(validationContext.getBaseEngine())) {
       validationEngine = new ValidationEngine(getBaseEngine(validationContext.getBaseEngine()));
     } else {
-      if (definitions == null) {
-        throw new IllegalArgumentException("Cannot create a validator engine (definitions == null)");
-      }
-      validationEngine = buildValidationEngine(validationContext, definitions, tt);
+      final String engineDefinitions = definitions != null ? definitions : getEngineDefinitionsForVersion(validationContext.getSv());
+      validationEngine = buildValidationEngine(validationContext, engineDefinitions, timeTracker);
     }
     return validationEngine;
   }
 
+  private static String getEngineDefinitionsForVersion(@Nonnull String fhirVersion) {
+    if ("dev".equals(fhirVersion)) {
+      return "hl7.fhir.r5.core#current";
+    }
+    return VersionUtilities.packageForVersion(fhirVersion) + "#" + VersionUtilities.getCurrentVersion(fhirVersion);
+  }
+
   protected ValidationEngine.ValidationEngineBuilder getValidationEngineBuilder() {
     return new ValidationEngine.ValidationEngineBuilder();
   }
@@ -689,15 +754,37 @@ protected void loadIgsAndExtensions(ValidationEngine validationEngine, Validatio
     log.info("  Package Summary: "+ validationEngine.getContext().loadedPackageSummary());
   }
 
+  /**
+   * Returns the appropriate FHIR version based on the ValidationContext.
+   * <p/>
+   *
+   * This is evaluated first on required versions for the mode specified in the evaluationContext and then the FHIR
+   * versions specified by the IGs in the context.
+   * <p/>
+   *
+   * 5.0.0 will be returned if no version information can be obtained from IGs.
+   *
+   * @param validationContext The validationContext
+   * @return An appropriate FHIR Version
+   * @throws IOException If IGs cannot be loaded as part of the evaluation.
+   */
+  public String getFhirVersionFromValidationContext(ValidationContext validationContext) throws IOException {
+    return determineVersion(validationContext);
+  }
+
+  /**
+   * @deprecated Use {@link ValidationService#getFhirVersionFromValidationContext } in future versions for the same functionality.
+   */
+  @Deprecated(forRemoval = true)
   public String determineVersion(ValidationContext validationContext) throws IOException {
-    if (validationContext.getMode() != EngineMode.VALIDATION && validationContext.getMode() != EngineMode.INSTALL) {
+    if (validationContext.getMode() != EngineMode.INSTALL) {
       return "5.0";
     }
     log.info("Scanning for versions (no -version parameter):");
     VersionSourceInformation versions = scanForVersions(validationContext);
-    for (String s : versions.getReport()) {
-      if (!s.equals("(nothing found)")) {
-        log.info("  " + s);
+    for (String reportEntry : versions.getReport()) {
+      if (!reportEntry.equals("(nothing found)")) {
+        log.info("  " + reportEntry);
       }
     }
     if (versions.isEmpty()) {

org.hl7.fhir.validation/src/main/java/org/hl7/fhir/validation/service/model/ValidationContext.java

@@ -27,7 +27,12 @@
 
 /**
  * A POJO for storing the flags/values for the CLI validator.
+ *
+ * @deprecated This class will be replaced with more a more modular set of POJOs that better separate concerns like
+ * ValidationEngine instantiation, InstanceValidator instantiation, etc. It will be maintained for backwards
+ * compatibility, but will eventually be removed.
  */
+@Deprecated
 public class ValidationContext {
 
   @JsonProperty("baseEngine")