Make Partition ID non-mandatory during partition creation. (#5016)
* closes #5014, fix, changelog, docs * documentationf fixes * checkstyle fix
This commit is contained in:
parent
ddf3b72f2d
commit
20a9ad0985
|
@ -0,0 +1,5 @@
|
||||||
|
---
|
||||||
|
type: add
|
||||||
|
issue: 5014
|
||||||
|
title: "It is no longer mandatory to provide a partition ID when executing the `$partition-management-create-partition` operation. If you omit the partition ID, one will be randomly selected
|
||||||
|
from the available pool of values. Note that this may incur a small performance cost when creating partitions."
|
|
@ -21,9 +21,9 @@ The `$partition-management-create-partition` operation can be used to create a n
|
||||||
<tr>
|
<tr>
|
||||||
<td>id</td>
|
<td>id</td>
|
||||||
<td>Integer</td>
|
<td>Integer</td>
|
||||||
<td>1..1</td>
|
<td>0..1</td>
|
||||||
<td>
|
<td>
|
||||||
The numeric ID for the partition. This value can be any integer, positive or negative or zero. It must not be a value that has already been used.
|
The numeric ID for the partition. This value can be any integer, positive or negative or zero. It must not be a value that has already been used. If omitted, a random unused integer will be selected.
|
||||||
</td>
|
</td>
|
||||||
</tr>
|
</tr>
|
||||||
<tr>
|
<tr>
|
||||||
|
|
|
@ -46,6 +46,13 @@ public interface IPartitionLookupSvc {
|
||||||
|
|
||||||
void clearCaches();
|
void clearCaches();
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Will generate a random unused partition ID. Validates that no partition with that ID exists before returning.
|
||||||
|
*
|
||||||
|
* @return an integer, representing a random unused partition ID.
|
||||||
|
*/
|
||||||
|
int generateRandomUnusedPartitionId();
|
||||||
|
|
||||||
PartitionEntity createPartition(PartitionEntity thePartition, RequestDetails theRequestDetails);
|
PartitionEntity createPartition(PartitionEntity thePartition, RequestDetails theRequestDetails);
|
||||||
|
|
||||||
PartitionEntity updatePartition(PartitionEntity thePartition);
|
PartitionEntity updatePartition(PartitionEntity thePartition);
|
||||||
|
|
|
@ -52,6 +52,7 @@ import javax.annotation.Nonnull;
|
||||||
import javax.annotation.PostConstruct;
|
import javax.annotation.PostConstruct;
|
||||||
import java.util.List;
|
import java.util.List;
|
||||||
import java.util.Optional;
|
import java.util.Optional;
|
||||||
|
import java.util.concurrent.ThreadLocalRandom;
|
||||||
import java.util.concurrent.TimeUnit;
|
import java.util.concurrent.TimeUnit;
|
||||||
import java.util.regex.Pattern;
|
import java.util.regex.Pattern;
|
||||||
|
|
||||||
|
@ -120,9 +121,23 @@ public class PartitionLookupSvcImpl implements IPartitionLookupSvc {
|
||||||
myIdToPartitionCache.invalidateAll();
|
myIdToPartitionCache.invalidateAll();
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Generate a random int which is guaranteed to be unused by an existing partition.
|
||||||
|
* @return an integer representing a partition ID that is not currently in use by the system.
|
||||||
|
*/
|
||||||
|
public int generateRandomUnusedPartitionId() {
|
||||||
|
int candidate = ThreadLocalRandom.current().nextInt();
|
||||||
|
Optional<PartitionEntity> partition = myPartitionDao.findById(candidate);
|
||||||
|
while (partition.isPresent()) {
|
||||||
|
candidate = ThreadLocalRandom.current().nextInt();
|
||||||
|
partition = myPartitionDao.findById(candidate);
|
||||||
|
}
|
||||||
|
return candidate;
|
||||||
|
}
|
||||||
@Override
|
@Override
|
||||||
@Transactional
|
@Transactional
|
||||||
public PartitionEntity createPartition(PartitionEntity thePartition, RequestDetails theRequestDetails) {
|
public PartitionEntity createPartition(PartitionEntity thePartition, RequestDetails theRequestDetails) {
|
||||||
|
|
||||||
validateNotInUnnamedPartitionMode();
|
validateNotInUnnamedPartitionMode();
|
||||||
validateHaveValidPartitionIdAndName(thePartition);
|
validateHaveValidPartitionIdAndName(thePartition);
|
||||||
validatePartitionNameDoesntAlreadyExist(thePartition.getName());
|
validatePartitionNameDoesntAlreadyExist(thePartition.getName());
|
||||||
|
|
|
@ -21,6 +21,7 @@ package ca.uhn.fhir.jpa.partition;
|
||||||
|
|
||||||
import ca.uhn.fhir.context.FhirContext;
|
import ca.uhn.fhir.context.FhirContext;
|
||||||
import ca.uhn.fhir.jpa.entity.PartitionEntity;
|
import ca.uhn.fhir.jpa.entity.PartitionEntity;
|
||||||
|
import ca.uhn.fhir.model.primitive.IntegerDt;
|
||||||
import ca.uhn.fhir.rest.annotation.Operation;
|
import ca.uhn.fhir.rest.annotation.Operation;
|
||||||
import ca.uhn.fhir.rest.annotation.OperationParam;
|
import ca.uhn.fhir.rest.annotation.OperationParam;
|
||||||
import ca.uhn.fhir.rest.annotation.ResourceParam;
|
import ca.uhn.fhir.rest.annotation.ResourceParam;
|
||||||
|
@ -30,6 +31,8 @@ import ca.uhn.fhir.util.ParametersUtil;
|
||||||
import org.hl7.fhir.instance.model.api.IBase;
|
import org.hl7.fhir.instance.model.api.IBase;
|
||||||
import org.hl7.fhir.instance.model.api.IBaseParameters;
|
import org.hl7.fhir.instance.model.api.IBaseParameters;
|
||||||
import org.hl7.fhir.instance.model.api.IPrimitiveType;
|
import org.hl7.fhir.instance.model.api.IPrimitiveType;
|
||||||
|
import org.slf4j.Logger;
|
||||||
|
import org.slf4j.LoggerFactory;
|
||||||
import org.springframework.beans.factory.annotation.Autowired;
|
import org.springframework.beans.factory.annotation.Autowired;
|
||||||
|
|
||||||
import javax.annotation.Nonnull;
|
import javax.annotation.Nonnull;
|
||||||
|
@ -51,6 +54,7 @@ import static org.hl7.fhir.instance.model.api.IPrimitiveType.toValueOrNull;
|
||||||
* </ul>
|
* </ul>
|
||||||
*/
|
*/
|
||||||
public class PartitionManagementProvider {
|
public class PartitionManagementProvider {
|
||||||
|
private static final Logger ourLog = LoggerFactory.getLogger(PartitionLookupSvcImpl.class);
|
||||||
|
|
||||||
@Autowired
|
@Autowired
|
||||||
private FhirContext myCtx;
|
private FhirContext myCtx;
|
||||||
|
@ -71,6 +75,11 @@ public class PartitionManagementProvider {
|
||||||
@OperationParam(name = ProviderConstants.PARTITION_MANAGEMENT_PARTITION_DESC, min = 0, max = 1, typeName = "string") IPrimitiveType<String> thePartitionDescription,
|
@OperationParam(name = ProviderConstants.PARTITION_MANAGEMENT_PARTITION_DESC, min = 0, max = 1, typeName = "string") IPrimitiveType<String> thePartitionDescription,
|
||||||
RequestDetails theRequestDetails
|
RequestDetails theRequestDetails
|
||||||
) {
|
) {
|
||||||
|
|
||||||
|
if (thePartitionId == null && thePartitionName!= null && thePartitionName.hasValue()) {
|
||||||
|
thePartitionId = requestRandomPartitionId(thePartitionName);
|
||||||
|
}
|
||||||
|
|
||||||
validatePartitionIdSupplied(myCtx, toValueOrNull(thePartitionId));
|
validatePartitionIdSupplied(myCtx, toValueOrNull(thePartitionId));
|
||||||
|
|
||||||
PartitionEntity input = parseInput(thePartitionId, thePartitionName, thePartitionDescription);
|
PartitionEntity input = parseInput(thePartitionId, thePartitionName, thePartitionDescription);
|
||||||
|
@ -83,6 +92,13 @@ public class PartitionManagementProvider {
|
||||||
return retVal;
|
return retVal;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@Nonnull
|
||||||
|
private IPrimitiveType<Integer> requestRandomPartitionId(IPrimitiveType<String> thePartitionName) {
|
||||||
|
int unusedPartitionId = myPartitionLookupSvc.generateRandomUnusedPartitionId();
|
||||||
|
ourLog.info("Request to create partition came in without a partition ID. Auto-assigning an available ID.[partition_id={}, partition_name={}]", unusedPartitionId, thePartitionName);
|
||||||
|
return new IntegerDt(unusedPartitionId);
|
||||||
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Add Partition:
|
* Add Partition:
|
||||||
* <code>
|
* <code>
|
||||||
|
|
|
@ -17,6 +17,8 @@ import org.junit.jupiter.api.BeforeEach;
|
||||||
import org.junit.jupiter.api.Test;
|
import org.junit.jupiter.api.Test;
|
||||||
import org.junit.jupiter.api.extension.ExtendWith;
|
import org.junit.jupiter.api.extension.ExtendWith;
|
||||||
import org.junit.jupiter.api.extension.RegisterExtension;
|
import org.junit.jupiter.api.extension.RegisterExtension;
|
||||||
|
import org.mockito.ArgumentCaptor;
|
||||||
|
import org.mockito.Captor;
|
||||||
import org.mockito.stubbing.Answer;
|
import org.mockito.stubbing.Answer;
|
||||||
import org.slf4j.Logger;
|
import org.slf4j.Logger;
|
||||||
import org.slf4j.LoggerFactory;
|
import org.slf4j.LoggerFactory;
|
||||||
|
@ -33,7 +35,9 @@ import java.util.List;
|
||||||
import java.util.stream.Collectors;
|
import java.util.stream.Collectors;
|
||||||
|
|
||||||
import static org.hamcrest.MatcherAssert.assertThat;
|
import static org.hamcrest.MatcherAssert.assertThat;
|
||||||
|
import static org.hamcrest.Matchers.equalTo;
|
||||||
import static org.hamcrest.Matchers.hasSize;
|
import static org.hamcrest.Matchers.hasSize;
|
||||||
|
import static org.hamcrest.Matchers.instanceOf;
|
||||||
import static org.hamcrest.Matchers.is;
|
import static org.hamcrest.Matchers.is;
|
||||||
import static org.junit.jupiter.api.Assertions.assertEquals;
|
import static org.junit.jupiter.api.Assertions.assertEquals;
|
||||||
import static org.junit.jupiter.api.Assertions.fail;
|
import static org.junit.jupiter.api.Assertions.fail;
|
||||||
|
@ -94,6 +98,34 @@ public class PartitionManagementProviderTest {
|
||||||
assertEquals("a description", ((StringType) response.getParameterValue(ProviderConstants.PARTITION_MANAGEMENT_PARTITION_DESC)).getValue());
|
assertEquals("a description", ((StringType) response.getParameterValue(ProviderConstants.PARTITION_MANAGEMENT_PARTITION_DESC)).getValue());
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
public void testCreatePartitionWithNameOnlyAutogeneratesId() {
|
||||||
|
|
||||||
|
when(myPartitionConfigSvc.createPartition(any(), any())).thenAnswer(createAnswer());
|
||||||
|
|
||||||
|
Parameters input = createInputPartitionWithoutId();;
|
||||||
|
ourLog.debug("Input:\n{}", ourCtx.newJsonParser().setPrettyPrint(true).encodeResourceToString(input));
|
||||||
|
|
||||||
|
Parameters response = myClient
|
||||||
|
.operation()
|
||||||
|
.onServer()
|
||||||
|
.named(ProviderConstants.PARTITION_MANAGEMENT_CREATE_PARTITION)
|
||||||
|
.withParameters(input)
|
||||||
|
.encodedXml()
|
||||||
|
.execute();
|
||||||
|
|
||||||
|
ourLog.debug("Response:\n{}", ourCtx.newJsonParser().setPrettyPrint(true).encodeResourceToString(response));
|
||||||
|
ArgumentCaptor<PartitionEntity> entityCaptor = ArgumentCaptor.forClass(PartitionEntity.class);
|
||||||
|
verify(myPartitionConfigSvc, times(1)).createPartition(entityCaptor.capture(), any());
|
||||||
|
verify(myPartitionConfigSvc, times(1)).generateRandomUnusedPartitionId();
|
||||||
|
verifyNoMoreInteractions(myPartitionConfigSvc);
|
||||||
|
PartitionEntity value = entityCaptor.getValue();
|
||||||
|
|
||||||
|
assertThat(value.getName(), is(equalTo("PARTITION-123")));
|
||||||
|
assertThat(value.getId(), is(instanceOf(Integer.class)));
|
||||||
|
assertThat(value.getDescription(), is(equalTo("a description")));
|
||||||
|
}
|
||||||
|
|
||||||
@Nonnull
|
@Nonnull
|
||||||
private Parameters createInputPartition() {
|
private Parameters createInputPartition() {
|
||||||
Parameters input = new Parameters();
|
Parameters input = new Parameters();
|
||||||
|
@ -103,6 +135,14 @@ public class PartitionManagementProviderTest {
|
||||||
return input;
|
return input;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@Nonnull
|
||||||
|
private Parameters createInputPartitionWithoutId() {
|
||||||
|
Parameters input = new Parameters();
|
||||||
|
input.addParameter(ProviderConstants.PARTITION_MANAGEMENT_PARTITION_NAME, new CodeType("PARTITION-123"));
|
||||||
|
input.addParameter(ProviderConstants.PARTITION_MANAGEMENT_PARTITION_DESC, new StringType("a description"));
|
||||||
|
return input;
|
||||||
|
}
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
public void testCreatePartition_InvalidInput() {
|
public void testCreatePartition_InvalidInput() {
|
||||||
try {
|
try {
|
||||||
|
|
Loading…
Reference in New Issue