diff --git a/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/6_8_0/5014-negative-partitions-not-allowed.yaml b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/6_8_0/5014-negative-partitions-not-allowed.yaml new file mode 100644 index 00000000000..55cb2291042 --- /dev/null +++ b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/6_8_0/5014-negative-partitions-not-allowed.yaml @@ -0,0 +1,4 @@ +--- +type: add +issue: 5014 +title: "There was a bug with automatic partition ID selection where it was not bound to positive integers and could return a value that was out of range. This has been fixed." diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/partition/PartitionLookupSvcImpl.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/partition/PartitionLookupSvcImpl.java index c924fa1f12e..4c1b1792e55 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/partition/PartitionLookupSvcImpl.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/partition/PartitionLookupSvcImpl.java @@ -123,14 +123,14 @@ public class PartitionLookupSvcImpl implements IPartitionLookupSvc { } /** - * Generate a random int which is guaranteed to be unused by an existing partition. + * Generate a random postive integer between 1 and Integer.MAX_VALUE, 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(); + int candidate = ThreadLocalRandom.current().nextInt(1, Integer.MAX_VALUE); Optional partition = myPartitionDao.findById(candidate); while (partition.isPresent()) { - candidate = ThreadLocalRandom.current().nextInt(); + candidate = ThreadLocalRandom.current().nextInt(1, Integer.MAX_VALUE); partition = myPartitionDao.findById(candidate); } return candidate; diff --git a/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/partition/PartitionLookupSvcImplTest.java b/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/partition/PartitionLookupSvcImplTest.java new file mode 100644 index 00000000000..7bbdbaa8d92 --- /dev/null +++ b/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/partition/PartitionLookupSvcImplTest.java @@ -0,0 +1,55 @@ +package ca.uhn.fhir.jpa.partition; + +import ca.uhn.fhir.context.FhirContext; +import ca.uhn.fhir.interceptor.api.IInterceptorService; +import ca.uhn.fhir.jpa.cache.ResourceChangeListenerRegistryInterceptor; +import ca.uhn.fhir.jpa.dao.data.IPartitionDao; +import ca.uhn.fhir.jpa.model.config.PartitionSettings; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.Mock; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.test.mock.mockito.MockBean; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Configuration; +import org.springframework.test.context.junit.jupiter.SpringExtension; +import org.springframework.transaction.PlatformTransactionManager; + +import java.util.Optional; + +import static org.junit.jupiter.api.Assertions.*; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.when; + +@ExtendWith(SpringExtension.class) +class PartitionLookupSvcImplTest { + + @Autowired + private PartitionLookupSvcImpl myPartitionLookupSvc; + @MockBean + PartitionSettings myPartitionSettings; + @MockBean + IInterceptorService myInterceptorBroadcaster; + @MockBean + IPartitionDao myPartitionDao; + @MockBean + private FhirContext myFhirCtx; + @MockBean + private PlatformTransactionManager myTxManager; + + @Configuration + static class SpringContext { + @Bean + public PartitionLookupSvcImpl partitionLookupSvcImplTest() { + return new PartitionLookupSvcImpl(); + } + } + @Test + void generateRandomUnusedPartitionId() { + when(myPartitionDao.findById(any())).thenReturn(Optional.empty()); + for (int i = 0; i<10000; i++) { + int randomUnusedPartitionId = myPartitionLookupSvc.generateRandomUnusedPartitionId(); + assertTrue(randomUnusedPartitionId >= 1); + } + } +}