Rel 6 2 4 mergeback

hapi-fhir-base/src/main/java/ca/uhn/fhir/util/VersionEnum.java

@@ -107,6 +107,10 @@ public enum VersionEnum {
  V6_1_4,
  V6_2_0,
  V6_2_1,
+ V6_2_2,
+ V6_2_3,
+ V6_2_4,
+ V6_2_5,
  // Dev Build
  V6_3_0,
  V6_4_0

hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/6_2_2/upgrade.md

@@ -0,0 +1 @@
+This version fixes a bug with 6.2.0 and previous releases wherein batch jobs that created very large chunk counts could occasionally fail to submit a small proportion of chunks.

hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/6_2_2/version.yaml

@@ -0,0 +1,3 @@
+---
+release-date: "2022-11-25"
+codename: "Vishwa"

hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/6_2_3/upgrade.md

@@ -0,0 +1 @@
+

hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/6_2_3/version.yaml

@@ -0,0 +1,3 @@
+---
+release-date: "2023-01-05"
+codename: "Vishwa"

hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/6_2_4/upgrade.md

@@ -0,0 +1 @@
+

hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/6_2_4/version.yaml

@@ -0,0 +1,3 @@
+---
+release-date: "2023-01-04"
+codename: "Vishwa"

hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/6_4_0/4291-r4b-package-install.yaml

@@ -1,5 +1,6 @@
 ---
 type: add
 issue: 4291
+backport: 6.2.2
 title: "The NPM package installer did not support installing on R4B repositories. Thanks to Jens Kristian Villadsen
  for the pull request!"

hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/6_4_0/4388-npe-in-binary-storage.yaml

@@ -2,4 +2,5 @@
 type: fix
 issue: 4388
 jira: SMILE-5834
+backport: 6.2.4
 title: "Fixed an edge case during a Read operation where hooks could be invoked with a null resource. This could cause a NullPointerException in some cases."

hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/6_4_0/4398-retry-on-mdm-clear-conflicts.yaml

@@ -0,0 +1,5 @@
+---
+type: fix
+backport: 6.2.3
+title: "The $mdm-clear operation sometimes failed with a constraint error when running in a heavily
+ multithreaded environment. This has been fixed."

hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/6_4_0/4400-dont_fail_on_batch2_double_delivery.yaml

@@ -0,0 +1,5 @@
+---
+type: fix
+issue: 4400
+title: "When Batch2 work notifications are received twice (e.g. because the notification engine double delivered)
+ an unrecoverable failure could occur. This has been corrected."

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

@@ -156,6 +156,7 @@ public abstract class BaseHapiFhirResourceDao<T extends IBaseResource> extends B
 
  public static final String BASE_RESOURCE_NAME = "resource";
  private static final org.slf4j.Logger ourLog = org.slf4j.LoggerFactory.getLogger(BaseHapiFhirResourceDao.class);
+
  @Autowired
  protected PlatformTransactionManager myPlatformTransactionManager;
  @Autowired(required = false)

hapi-fhir-jpaserver-mdm/src/test/java/ca/uhn/fhir/jpa/mdm/svc/MdmControllerSvcImplTest.java

@@ -4,6 +4,7 @@
 import ca.uhn.fhir.interceptor.model.RequestPartitionId;
 import ca.uhn.fhir.jpa.entity.MdmLink;
 import ca.uhn.fhir.jpa.entity.PartitionEntity;
+import ca.uhn.fhir.jpa.interceptor.UserRequestRetryVersionConflictsInterceptor;
 import ca.uhn.fhir.jpa.mdm.provider.BaseLinkR4Test;
 import ca.uhn.fhir.jpa.partition.IRequestPartitionHelperSvc;
 import ca.uhn.fhir.rest.api.server.SystemRequestDetails;
@@ -14,8 +15,10 @@
 import ca.uhn.fhir.mdm.api.MdmMatchResultEnum;
 import ca.uhn.fhir.mdm.api.MdmQuerySearchParameters;
 import ca.uhn.fhir.mdm.api.paging.MdmPageRequest;
+import ca.uhn.fhir.mdm.batch2.clear.MdmClearStep;
 import ca.uhn.fhir.mdm.model.MdmTransactionContext;
 import ca.uhn.fhir.mdm.rules.config.MdmSettings;
+import ca.uhn.fhir.rest.server.exceptions.ResourceVersionConflictException;
 import ca.uhn.fhir.rest.server.interceptor.partition.RequestTenantPartitionInterceptor;
 import ca.uhn.fhir.rest.server.servlet.ServletRequestDetails;
 import org.hl7.fhir.instance.model.api.IBaseParameters;
@@ -37,6 +40,7 @@
 import java.math.BigDecimal;
 import java.util.ArrayList;
 import java.util.List;
+import java.util.concurrent.atomic.AtomicBoolean;
 
 import static ca.uhn.fhir.mdm.provider.MdmProviderDstu3Plus.DEFAULT_PAGE_SIZE;
 import static ca.uhn.fhir.mdm.provider.MdmProviderDstu3Plus.MAX_PAGE_SIZE;
@@ -59,6 +63,7 @@ public class MdmControllerSvcImplTest extends BaseLinkR4Test {
  private Batch2JobHelper myBatch2JobHelper;
  @Autowired
  private MdmSettings myMdmSettings;
+ private UserRequestRetryVersionConflictsInterceptor myUserRequestRetryVersionConflictsInterceptor;
  private final RequestTenantPartitionInterceptor myPartitionInterceptor = new RequestTenantPartitionInterceptor();
 
  @Override
@@ -70,12 +75,16 @@ public void before() throws Exception {
  myPartitionLookupSvc.createPartition(new PartitionEntity().setId(2).setName(PARTITION_2), null);
  myInterceptorService.registerInterceptor(myPartitionInterceptor);
  myMdmSettings.setEnabled(true);
+
+ myUserRequestRetryVersionConflictsInterceptor = new UserRequestRetryVersionConflictsInterceptor();
+ myInterceptorService.registerInterceptor(myUserRequestRetryVersionConflictsInterceptor);
  }
 
  @Override
  @AfterEach
  public void after() throws IOException {
  myMdmSettings.setEnabled(false);
+ myInterceptorService.unregisterInterceptor(myUserRequestRetryVersionConflictsInterceptor);
  myPartitionSettings.setPartitioningEnabled(false);
  myInterceptorService.unregisterInterceptor(myPartitionInterceptor);
  super.after();
@@ -160,6 +169,35 @@ public void testMdmClearWithProvidedResources() {
  assertLinkCount(2);
  }
 
+ @Test
+ public void testMdmClearWithWriteConflict() {
+ AtomicBoolean haveFired = new AtomicBoolean(false);
+ MdmClearStep.setClearCompletionCallbackForUnitTest(()->{
+ if (haveFired.getAndSet(true) == false) {
+ throw new ResourceVersionConflictException("Conflict");
+ }
+ });
+
+ assertLinkCount(1);
+
+ RequestPartitionId requestPartitionId1 = RequestPartitionId.fromPartitionId(1);
+ RequestPartitionId requestPartitionId2 = RequestPartitionId.fromPartitionId(2);
+ createPractitionerAndUpdateLinksOnPartition(buildJanePractitioner(), requestPartitionId1);
+ createPractitionerAndUpdateLinksOnPartition(buildJanePractitioner(), requestPartitionId2);
+ assertLinkCount(3);
+
+ List<String> urls = new ArrayList<>();
+ urls.add("Practitioner");
+ IPrimitiveType<BigDecimal> batchSize = new DecimalType(new BigDecimal(100));
+ ServletRequestDetails details = new ServletRequestDetails();
+ details.setTenantId(PARTITION_2);
+ IBaseParameters clearJob = myMdmControllerSvc.submitMdmClearJob(urls, batchSize, details);
+ String jobId = ((StringType) ((Parameters) clearJob).getParameterValue("jobId")).getValueAsString();
+ myBatch2JobHelper.awaitJobCompletion(jobId);
+
+ assertLinkCount(2);
+ }
+
  private class PartitionIdMatcher implements ArgumentMatcher<RequestPartitionId> {
  private RequestPartitionId myRequestPartitionId;
 

hapi-fhir-jpaserver-mdm/src/test/resources/logback-test.xml

@@ -1,7 +1,7 @@
 <configuration>
  <appender name="STDOUT" class="ch.qos.logback.core.ConsoleAppender">
  <filter class="ch.qos.logback.classic.filter.ThresholdFilter">
- <level>TRACE</level>
+ <level>INFO</level>
  </filter>
  <encoder>
 <!--N.B use this pattern to remove timestamp/thread/level/logger information from logs during testing.<pattern>[%file:%line] %msg%n</pattern>-->
@@ -49,25 +49,6 @@
  <appender-ref ref="STDOUT" />
  </logger>
 
- <!--
- Configuration for MDM troubleshooting log
- -->
- <appender name="MDM_TROUBLESHOOTING" class="ch.qos.logback.core.rolling.RollingFileAppender">
- <filter class="ch.qos.logback.classic.filter.ThresholdFilter"><level>DEBUG</level></filter>
- <file>${smile.basedir}/log/mdm-troubleshooting.log</file>
- <rollingPolicy class="ch.qos.logback.core.rolling.FixedWindowRollingPolicy">
- <fileNamePattern>${smile.basedir}/log/mdm-troubleshooting.log.%i.gz</fileNamePattern>
- <minIndex>1</minIndex>
- <maxIndex>9</maxIndex>
- </rollingPolicy>
- <triggeringPolicy class="ch.qos.logback.core.rolling.SizeBasedTriggeringPolicy">
- <maxFileSize>5MB</maxFileSize>
- </triggeringPolicy>
- <encoder>
- <pattern>%d{yyyy-MM-dd HH:mm:ss.SSS} [%thread] %-5level %logger{36} - %msg%n</pattern>
- </encoder>
- </appender>
-
  <logger name="ca.uhn.fhir.log.mdm_troubleshooting" level="TRACE">
  <appender-ref ref="MDM_TROUBLESHOOTING"/>
  </logger>

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

@@ -41,6 +41,8 @@
 import javax.persistence.EntityManager;
 import java.util.List;
 
+import java.util.List;
+
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertNotNull;
 import static org.junit.jupiter.api.Assertions.fail;

hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/provider/r4/ResourceProviderR4Test.java

@@ -166,6 +166,7 @@
 import org.springframework.transaction.support.TransactionTemplate;
 
 import javax.annotation.Nonnull;
+import javax.sql.DataSource;
 import java.io.BufferedReader;
 import java.io.IOException;
 import java.io.InputStreamReader;

hapi-fhir-storage-batch2-jobs/src/main/java/ca/uhn/fhir/batch2/jobs/export/FetchResourceIdsStep.java

@@ -31,6 +31,7 @@
 import ca.uhn.fhir.batch2.jobs.models.BatchResourceId;
 import ca.uhn.fhir.i18n.Msg;
 import ca.uhn.fhir.jpa.api.config.DaoConfig;
+import ca.uhn.fhir.jpa.api.dao.DaoRegistry;
 import ca.uhn.fhir.jpa.bulk.export.api.IBulkExportProcessor;
 import ca.uhn.fhir.jpa.bulk.export.model.ExportPIDIteratorParameters;
 import ca.uhn.fhir.rest.api.server.storage.IResourcePersistentId;

hapi-fhir-storage-batch2/src/main/java/ca/uhn/fhir/batch2/coordinator/WorkChunkProcessor.java

@@ -39,6 +39,10 @@
 
 import javax.annotation.Nullable;
 
+import java.util.Optional;
+
+import static org.apache.commons.lang3.StringUtils.isBlank;
+
 public class WorkChunkProcessor {
  private static final Logger ourLog = Logs.getBatchTroubleshootingLog();
 
@@ -113,7 +117,12 @@ public WorkChunkProcessor(IJobPersistence theJobPersistence,
  } else {
  // all other kinds of steps
  Validate.notNull(theWorkChunk);
- StepExecutionDetails<PT, IT> stepExecutionDetails = getExecutionDetailsForNonReductionStep(theWorkChunk, theInstance, inputType, parameters);
+ Optional<StepExecutionDetails<PT, IT>> stepExecutionDetailsOpt = getExecutionDetailsForNonReductionStep(theWorkChunk, theInstance, inputType, parameters);
+ if (!stepExecutionDetailsOpt.isPresent()) {
+ return new JobStepExecutorOutput<>(false, dataSink);
+ }
+
+ StepExecutionDetails<PT, IT> stepExecutionDetails = stepExecutionDetailsOpt.get();
 
  // execute the step
  boolean success = myStepExecutor.executeStep(stepExecutionDetails, worker, dataSink);
@@ -146,7 +155,7 @@ protected <PT extends IModelJson, IT extends IModelJson, OT extends IModelJson>
  /**
  * Construct execution details for non-reduction step
  */
- private <PT extends IModelJson, IT extends IModelJson> StepExecutionDetails<PT, IT> getExecutionDetailsForNonReductionStep(
+ private <PT extends IModelJson, IT extends IModelJson> Optional<StepExecutionDetails<PT, IT>> getExecutionDetailsForNonReductionStep(
  WorkChunk theWorkChunk,
  JobInstance theInstance,
  Class<IT> theInputType,
@@ -155,11 +164,15 @@ private <PT extends IModelJson, IT extends IModelJson> StepExecutionDetails<PT,
  IT inputData = null;
 
  if (!theInputType.equals(VoidModel.class)) {
+ if (isBlank(theWorkChunk.getData())) {
+ ourLog.info("Ignoring chunk[{}] for step[{}] in status[{}] because it has no data", theWorkChunk.getId(), theWorkChunk.getTargetStepId(), theWorkChunk.getStatus());
+ return Optional.empty();
+ }
  inputData = theWorkChunk.getData(theInputType);
  }
 
  String chunkId = theWorkChunk.getId();
 
- return new StepExecutionDetails<>(theParameters, inputData, theInstance, chunkId);
+ return Optional.of(new StepExecutionDetails<>(theParameters, inputData, theInstance, chunkId));
  }
 }

hapi-fhir-storage-batch2/src/test/java/ca/uhn/fhir/batch2/coordinator/JobCoordinatorImplTest.java

@@ -454,6 +454,31 @@ public void testPerformStep_ChunkNotKnown() {
 
  }
 
+ /**
+ * If a notification is received for a chunk that should have data but doesn't, we can just ignore that
+ * (just caused by double delivery of a chunk notification message)
+ */
+ @Test
+ public void testPerformStep_ChunkAlreadyComplete() {
+
+ // Setup
+
+ WorkChunk chunk = createWorkChunkStep2();
+ chunk.setData((String)null);
+ setupMocks(createJobDefinition(), chunk);
+ mySvc.start();
+
+ // Execute
+
+ myWorkChannelReceiver.send(new JobWorkNotificationJsonMessage(createWorkNotification(STEP_2)));
+
+ // Verify
+ verifyNoMoreInteractions(myStep1Worker);
+ verifyNoMoreInteractions(myStep2Worker);
+ verifyNoMoreInteractions(myStep3Worker);
+
+ }
+
  @Test
  public void testStartInstance() {
 

hapi-fhir-storage-mdm/src/main/java/ca/uhn/fhir/mdm/batch2/clear/MdmClearStep.java

@@ -41,6 +41,7 @@
 import ca.uhn.fhir.rest.api.server.storage.TransactionDetails;
 import ca.uhn.fhir.rest.server.provider.ProviderConstants;
 import ca.uhn.fhir.util.StopWatch;
+import com.google.common.annotations.VisibleForTesting;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import org.springframework.beans.factory.annotation.Autowired;
@@ -54,6 +55,7 @@
 public class MdmClearStep implements IJobStepWorker<MdmClearJobParameters, ResourceIdListWorkChunkJson, VoidModel> {
 
  private static final Logger ourLog = LoggerFactory.getLogger(MdmClearStep.class);
+ private static Runnable ourClearCompletionCallbackForUnitTest;
 
  @Autowired
  HapiTransactionService myHapiTransactionService;
@@ -71,6 +73,8 @@ public class MdmClearStep implements IJobStepWorker<MdmClearJobParameters, Resou
  public RunOutcome run(@Nonnull StepExecutionDetails<MdmClearJobParameters, ResourceIdListWorkChunkJson> theStepExecutionDetails, @Nonnull IJobDataSink<VoidModel> theDataSink) throws JobExecutionFailedException {
 
  SystemRequestDetails requestDetails = new SystemRequestDetails();
+ requestDetails.setRetry(true);
+ requestDetails.setMaxRetries(100);
  requestDetails.setRequestPartitionId(theStepExecutionDetails.getParameters().getRequestPartitionId());
  TransactionDetails transactionDetails = new TransactionDetails();
  myHapiTransactionService.execute(requestDetails, transactionDetails, buildJob(requestDetails, transactionDetails, theStepExecutionDetails));
@@ -119,7 +123,18 @@ public Void doInTransaction(@Nonnull TransactionStatus theStatus) {
 
  ourLog.info("Finished removing {} golden resources in {} - {}/sec - Instance[{}] Chunk[{}]", persistentIds.size(), sw, sw.formatThroughput(persistentIds.size(), TimeUnit.SECONDS), myInstanceId, myChunkId);
 
+ if (ourClearCompletionCallbackForUnitTest != null) {
+ ourClearCompletionCallbackForUnitTest.run();
+ }
+
  return null;
  }
  }
+
+
+ @VisibleForTesting
+ public static void setClearCompletionCallbackForUnitTest(Runnable theClearCompletionCallbackForUnitTest) {
+ ourClearCompletionCallbackForUnitTest = theClearCompletionCallbackForUnitTest;
+ }
+
 }

src/checkstyle/checkstyle.xml

@@ -7,9 +7,9 @@
  <property name="charset" value="UTF-8"/>
  <property name="cacheFile" value="target/cache_non_main_files"/>
 
- <module name="SuppressionFilter">
- <property name="file" value="src/checkstyle/checkstyle_suppressions.xml" />
- </module>
+<!-- <module name="SuppressionFilter">-->
+<!-- <property name="file" value="${basedir}/src/checkstyle/checkstyle_suppressions.xml" />-->
+<!-- </module> TODO GGG propagate this to master -->
  <module name="TreeWalker">
  <module name="RegexpSinglelineJava">
  <property name="format" value="System\.out\.println"/>