Partitioning: creating two partitions with the same ID overwrites the first (#5012)
* Added unit test and bug fix to ensure no two partitions with the same ID can be created. * Added changelogs, and changed the equals operator. * Made required changes to ensure updating of partition is not affected * Changed unit test and its location to ensure functionality. * changing for loop to findById to avoid iteration over all partitions. * Only allows findById when list is not empty, deals with that edge case. * Replaced null with isPresent in if statement to fix issues regarding the creation of links on different partitions * Removed the findAll command within the function. To avoid searching twice. * Edited the changelogs to properly describe the issue that we have at hand * Edited the changelogs to properly describe the issue that we have at hand pt 2, wording changes.
This commit is contained in:
parent
d7bdabb91b
commit
a59eb1ace3
|
@ -188,6 +188,7 @@ ca.uhn.fhir.jpa.dao.index.IdHelperService.nonUniqueForcedId=Non-unique ID specif
|
||||||
|
|
||||||
ca.uhn.fhir.jpa.partition.PartitionLookupSvcImpl.noIdSupplied=No Partition ID supplied
|
ca.uhn.fhir.jpa.partition.PartitionLookupSvcImpl.noIdSupplied=No Partition ID supplied
|
||||||
ca.uhn.fhir.jpa.partition.PartitionLookupSvcImpl.missingPartitionIdOrName=Partition must have an ID and a Name
|
ca.uhn.fhir.jpa.partition.PartitionLookupSvcImpl.missingPartitionIdOrName=Partition must have an ID and a Name
|
||||||
|
ca.uhn.fhir.jpa.partition.PartitionLookupSvcImpl.duplicatePartitionId=Partition ID already exists
|
||||||
ca.uhn.fhir.jpa.partition.PartitionLookupSvcImpl.unknownPartitionId=No partition exists with ID {0}
|
ca.uhn.fhir.jpa.partition.PartitionLookupSvcImpl.unknownPartitionId=No partition exists with ID {0}
|
||||||
ca.uhn.fhir.jpa.partition.PartitionLookupSvcImpl.invalidName=Partition name "{0}" is not valid
|
ca.uhn.fhir.jpa.partition.PartitionLookupSvcImpl.invalidName=Partition name "{0}" is not valid
|
||||||
ca.uhn.fhir.jpa.partition.PartitionLookupSvcImpl.cantCreateDuplicatePartitionName=Partition name "{0}" is already defined
|
ca.uhn.fhir.jpa.partition.PartitionLookupSvcImpl.cantCreateDuplicatePartitionName=Partition name "{0}" is already defined
|
||||||
|
|
|
@ -0,0 +1,5 @@
|
||||||
|
---
|
||||||
|
type: fix
|
||||||
|
issue: 5011
|
||||||
|
title: "Previously, if a partition was created with an ID that was already in use, it would overwrite the partition that
|
||||||
|
was using that ID. Now attempting to overwrite will return a 400"
|
|
@ -51,6 +51,7 @@ import org.springframework.transaction.support.TransactionTemplate;
|
||||||
import javax.annotation.Nonnull;
|
import javax.annotation.Nonnull;
|
||||||
import javax.annotation.PostConstruct;
|
import javax.annotation.PostConstruct;
|
||||||
import java.util.List;
|
import java.util.List;
|
||||||
|
import java.util.ListIterator;
|
||||||
import java.util.Optional;
|
import java.util.Optional;
|
||||||
import java.util.concurrent.ThreadLocalRandom;
|
import java.util.concurrent.ThreadLocalRandom;
|
||||||
import java.util.concurrent.TimeUnit;
|
import java.util.concurrent.TimeUnit;
|
||||||
|
@ -141,7 +142,7 @@ public class PartitionLookupSvcImpl implements IPartitionLookupSvc {
|
||||||
validateNotInUnnamedPartitionMode();
|
validateNotInUnnamedPartitionMode();
|
||||||
validateHaveValidPartitionIdAndName(thePartition);
|
validateHaveValidPartitionIdAndName(thePartition);
|
||||||
validatePartitionNameDoesntAlreadyExist(thePartition.getName());
|
validatePartitionNameDoesntAlreadyExist(thePartition.getName());
|
||||||
|
validIdUponCreation(thePartition);
|
||||||
ourLog.info("Creating new partition with ID {} and Name {}", thePartition.getId(), thePartition.getName());
|
ourLog.info("Creating new partition with ID {} and Name {}", thePartition.getId(), thePartition.getName());
|
||||||
|
|
||||||
PartitionEntity retVal = myPartitionDao.save(thePartition);
|
PartitionEntity retVal = myPartitionDao.save(thePartition);
|
||||||
|
@ -212,6 +213,13 @@ public class PartitionLookupSvcImpl implements IPartitionLookupSvc {
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
private void validIdUponCreation(PartitionEntity thePartition){
|
||||||
|
if (myPartitionDao.findById(thePartition.getId()).isPresent()) {
|
||||||
|
String msg = myFhirCtx.getLocalizer().getMessageSanitized(PartitionLookupSvcImpl.class, "duplicatePartitionId");
|
||||||
|
throw new InvalidRequestException(Msg.code(2366) + msg);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
private void validateHaveValidPartitionIdAndName(PartitionEntity thePartition) {
|
private void validateHaveValidPartitionIdAndName(PartitionEntity thePartition) {
|
||||||
if (thePartition.getId() == null || isBlank(thePartition.getName())) {
|
if (thePartition.getId() == null || isBlank(thePartition.getName())) {
|
||||||
String msg = myFhirCtx.getLocalizer().getMessage(PartitionLookupSvcImpl.class, "missingPartitionIdOrName");
|
String msg = myFhirCtx.getLocalizer().getMessage(PartitionLookupSvcImpl.class, "missingPartitionIdOrName");
|
||||||
|
@ -227,7 +235,6 @@ public class PartitionLookupSvcImpl implements IPartitionLookupSvc {
|
||||||
String msg = myFhirCtx.getLocalizer().getMessageSanitized(PartitionLookupSvcImpl.class, "invalidName", thePartition.getName());
|
String msg = myFhirCtx.getLocalizer().getMessageSanitized(PartitionLookupSvcImpl.class, "invalidName", thePartition.getName());
|
||||||
throw new InvalidRequestException(Msg.code(1312) + msg);
|
throw new InvalidRequestException(Msg.code(1312) + msg);
|
||||||
}
|
}
|
||||||
|
|
||||||
}
|
}
|
||||||
|
|
||||||
private void validateNotInUnnamedPartitionMode() {
|
private void validateNotInUnnamedPartitionMode() {
|
||||||
|
|
|
@ -103,6 +103,27 @@ public class PartitionSettingsSvcImplTest extends BaseJpaR4Test {
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
public void testCreatePartition_whenPartitionIdAlreadyExists_operationNotAllowed(){
|
||||||
|
try {
|
||||||
|
PartitionEntity partition = new PartitionEntity();
|
||||||
|
partition.setId(123);
|
||||||
|
partition.setName("NAME123");
|
||||||
|
partition.setDescription("A description");
|
||||||
|
myPartitionConfigSvc.createPartition(partition, null);
|
||||||
|
|
||||||
|
partition = new PartitionEntity();
|
||||||
|
partition.setId(123);
|
||||||
|
partition.setName("NAME111");
|
||||||
|
partition.setDescription("A description");
|
||||||
|
myPartitionConfigSvc.createPartition(partition, null);
|
||||||
|
}
|
||||||
|
|
||||||
|
catch (InvalidRequestException e) {
|
||||||
|
assertEquals( Msg.code(2366) + "Partition ID already exists", e.getMessage());
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
public void testUpdatePartition_TryToRenameDefault() {
|
public void testUpdatePartition_TryToRenameDefault() {
|
||||||
PartitionEntity partition = new PartitionEntity();
|
PartitionEntity partition = new PartitionEntity();
|
||||||
|
|
Loading…
Reference in New Issue