honeymoose
/
hapi-fhir
mirror of https://github.com/hapifhir/hapi-fhir.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Add remove method to jobregistry (#4187) 

* Add remove method to jobregistry

* Add changelog
This commit is contained in:
James Agnew 2022-10-24 17:07:32 -04:00 committed by GitHub
parent fb6bf5b6a4
commit cbd65c794d
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
8 changed files with 74 additions and 18 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/6_2_0/4187-add-jobregistry-remove.yaml Normal file
View File

@ -0,0 +1,5 @@
---
type: add
issue: 4187
title: "A remove method has been added to the Batch2 job registry. This will allow for dynamic job registration
in the future."

3
hapi-fhir-jpaserver-uhnfhirtest/src/main/java/ca/uhn/fhirtest/config/TestDstu2Config.java
View File

@ -106,8 +106,7 @@ public class TestDstu2Config {
}
retVal.setUsername(myDbUsername);
retVal.setPassword(myDbPassword);
retVal.setDefaultQueryTimeout(20);
retVal.setTestOnBorrow(true);
TestR5Config.applyCommonDatasourceParams(retVal);
DataSource dataSource = ProxyDataSourceBuilder
.create(retVal)

3
hapi-fhir-jpaserver-uhnfhirtest/src/main/java/ca/uhn/fhirtest/config/TestDstu3Config.java
View File

@ -115,8 +115,7 @@ public class TestDstu3Config {
}
retVal.setUsername(myDbUsername);
retVal.setPassword(myDbPassword);
retVal.setDefaultQueryTimeout(20);
retVal.setTestOnBorrow(true);
TestR5Config.applyCommonDatasourceParams(retVal);
DataSource dataSource = ProxyDataSourceBuilder
.create(retVal)

3
hapi-fhir-jpaserver-uhnfhirtest/src/main/java/ca/uhn/fhirtest/config/TestR4BConfig.java
View File

@ -102,8 +102,7 @@ public class TestR4BConfig {
}
retVal.setUsername(myDbUsername);
retVal.setPassword(myDbPassword);
retVal.setDefaultQueryTimeout(20);
retVal.setTestOnBorrow(true);
TestR5Config.applyCommonDatasourceParams(retVal);
DataSource dataSource = ProxyDataSourceBuilder
.create(retVal)

3
hapi-fhir-jpaserver-uhnfhirtest/src/main/java/ca/uhn/fhirtest/config/TestR4Config.java
View File

@ -102,8 +102,7 @@ public class TestR4Config {
}
retVal.setUsername(myDbUsername);
retVal.setPassword(myDbPassword);
retVal.setDefaultQueryTimeout(20);
retVal.setTestOnBorrow(true);
TestR5Config.applyCommonDatasourceParams(retVal);
DataSource dataSource = ProxyDataSourceBuilder
.create(retVal)

10
hapi-fhir-jpaserver-uhnfhirtest/src/main/java/ca/uhn/fhirtest/config/TestR5Config.java
View File

@ -107,8 +107,7 @@ public class TestR5Config {
}
retVal.setUsername(myDbUsername);
retVal.setPassword(myDbPassword);
retVal.setDefaultQueryTimeout(20);
retVal.setTestOnBorrow(true);
applyCommonDatasourceParams(retVal);
DataSource dataSource = ProxyDataSourceBuilder
.create(retVal)
@ -121,6 +120,13 @@ public class TestR5Config {
return dataSource;
}
static void applyCommonDatasourceParams(BasicDataSource retVal) {
retVal.setDefaultQueryTimeout(20);
retVal.setTestOnBorrow(true);
retVal.setMaxTotal(50);
retVal.setMaxWaitMillis(25000);
}
// TODO KHS there is code duplication between this and the other Test*Config classes in this directory
@Bean
public DatabaseBackedPagingProvider databaseBackedPagingProvider() {

51
hapi-fhir-storage-batch2/src/main/java/ca/uhn/fhir/batch2/coordinator/JobDefinitionRegistry.java
View File

@ -28,14 +28,17 @@ import ca.uhn.fhir.i18n.Msg;
import ca.uhn.fhir.jpa.batch.log.Logs;
import ca.uhn.fhir.model.api.IModelJson;
import ca.uhn.fhir.rest.server.exceptions.InternalErrorException;
import com.google.common.collect.ImmutableSortedMap;
import org.apache.commons.lang3.Validate;
import org.slf4j.Logger;
import javax.annotation.Nonnull;
import java.util.Collection;
import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.NavigableMap;
import java.util.Optional;
import java.util.Set;
import java.util.TreeMap;
@ -44,15 +47,15 @@ import java.util.stream.Collectors;
public class JobDefinitionRegistry {
private static final Logger ourLog = Logs.getBatchTroubleshootingLog();
private final Map<String, TreeMap<Integer, JobDefinition<?>>> myJobs = new HashMap<>();
private volatile Map<String, NavigableMap<Integer, JobDefinition<?>>> myJobs = new HashMap<>();
/**
* Add a job definition only if it is not registered
* @param theDefinition
* @return true if it did not already exist and was registered
*
* @param <PT> the job parameter type for the definition
* @return true if it did not already exist and was registered
*/
public <PT extends IModelJson> boolean addJobDefinitionIfNotRegistered(@Nonnull JobDefinition<PT> theDefinition) {
public synchronized <PT extends IModelJson> boolean addJobDefinitionIfNotRegistered(@Nonnull JobDefinition<PT> theDefinition) {
Optional<JobDefinition<?>> orig = getJobDefinition(theDefinition.getJobDefinitionId(), theDefinition.getJobDefinitionVersion());
if (orig.isPresent()) {
return false;
@ -61,7 +64,7 @@ public class JobDefinitionRegistry {
return true;
}
public <PT extends IModelJson> void addJobDefinition(@Nonnull JobDefinition<PT> theDefinition) {
public synchronized <PT extends IModelJson> void addJobDefinition(@Nonnull JobDefinition<PT> theDefinition) {
Validate.notNull(theDefinition);
String jobDefinitionId = theDefinition.getJobDefinitionId();
Validate.notBlank(jobDefinitionId);
@ -75,7 +78,8 @@ public class JobDefinitionRegistry {
}
}
TreeMap<Integer, JobDefinition<?>> versionMap = myJobs.computeIfAbsent(jobDefinitionId, t -> new TreeMap<>());
Map<String, NavigableMap<Integer, JobDefinition<?>>> newJobsMap = cloneJobsMap();
NavigableMap<Integer, JobDefinition<?>> versionMap = newJobsMap.computeIfAbsent(jobDefinitionId, t -> new TreeMap<>());
if (versionMap.containsKey(theDefinition.getJobDefinitionVersion())) {
if (versionMap.get(theDefinition.getJobDefinitionVersion()) == theDefinition) {
ourLog.warn("job[{}] version: {} already registered. Not registering again.", jobDefinitionId, theDefinition.getJobDefinitionVersion());
@ -84,10 +88,37 @@ public class JobDefinitionRegistry {
throw new ConfigurationException(Msg.code(2047) + "Multiple definitions for job[" + jobDefinitionId + "] version: " + theDefinition.getJobDefinitionVersion());
}
versionMap.put(theDefinition.getJobDefinitionVersion(), theDefinition);
myJobs = newJobsMap;
}
public synchronized void removeJobDefinition(@Nonnull String theDefinitionId, int theVersion) {
Validate.notBlank(theDefinitionId);
Validate.isTrue(theVersion >= 1);
Map<String, NavigableMap<Integer, JobDefinition<?>>> newJobsMap = cloneJobsMap();
NavigableMap<Integer, JobDefinition<?>> versionMap = newJobsMap.get(theDefinitionId);
if (versionMap != null) {
versionMap.remove(theVersion);
if (versionMap.isEmpty()) {
newJobsMap.remove(theDefinitionId);
}
}
myJobs = newJobsMap;
}
@Nonnull
private Map<String, NavigableMap<Integer, JobDefinition<?>>> cloneJobsMap() {
Map<String, NavigableMap<Integer, JobDefinition<?>>> newJobsMap = new HashMap<>();
for (Map.Entry<String, NavigableMap<Integer, JobDefinition<?>>> nextEntry : myJobs.entrySet()) {
newJobsMap.put(nextEntry.getKey(), new TreeMap<>(nextEntry.getValue()));
}
return newJobsMap;
}
public Optional<JobDefinition<?>> getLatestJobDefinition(@Nonnull String theJobDefinitionId) {
TreeMap<Integer, JobDefinition<?>> versionMap = myJobs.get(theJobDefinitionId);
NavigableMap<Integer, JobDefinition<?>> versionMap = myJobs.get(theJobDefinitionId);
if (versionMap == null || versionMap.isEmpty()) {
return Optional.empty();
}
@ -95,7 +126,7 @@ public class JobDefinitionRegistry {
}
public Optional<JobDefinition<?>> getJobDefinition(@Nonnull String theJobDefinitionId, int theJobDefinitionVersion) {
TreeMap<Integer, JobDefinition<?>> versionMap = myJobs.get(theJobDefinitionId);
NavigableMap<Integer, JobDefinition<?>> versionMap = myJobs.get(theJobDefinitionId);
if (versionMap == null || versionMap.isEmpty()) {
return Optional.empty();
}
@ -138,4 +169,8 @@ public class JobDefinitionRegistry {
public JobDefinition<?> getJobDefinitionOrThrowException(JobInstance theJobInstance) {
return getJobDefinitionOrThrowException(theJobInstance.getJobDefinitionId(), theJobInstance.getJobDefinitionVersion());
}
public Collection<Integer> getJobDefinitionVersions(String theDefinitionId) {
return myJobs.getOrDefault(theDefinitionId, ImmutableSortedMap.of()).keySet();
}
}

14
hapi-fhir-storage-batch2/src/test/java/ca/uhn/fhir/batch2/coordinator/JobDefinitionRegistryTest.java
View File

@ -11,6 +11,9 @@ import org.junit.jupiter.api.extension.ExtendWith;
import org.mockito.Mock;
import org.mockito.junit.jupiter.MockitoExtension;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.containsInAnyOrder;
import static org.hamcrest.Matchers.empty;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.fail;
@ -122,5 +125,16 @@ class JobDefinitionRegistryTest {
}
}
@Test
public void testRemoveJobDefinition() {
mySvc.removeJobDefinition("A", 1);
assertThat(mySvc.getJobDefinitionIds(), containsInAnyOrder("A"));
assertThat(mySvc.getJobDefinitionVersions("A"), containsInAnyOrder(2));
mySvc.removeJobDefinition("A", 2);
assertThat(mySvc.getJobDefinitionIds(), empty());
}
}