diff --git a/dhis-2/dhis-api/src/main/java/org/hisp/dhis/reservedvalue/ReservedValueStore.java b/dhis-2/dhis-api/src/main/java/org/hisp/dhis/reservedvalue/ReservedValueStore.java index 671b26bd163e..a86d3f897706 100644 --- a/dhis-2/dhis-api/src/main/java/org/hisp/dhis/reservedvalue/ReservedValueStore.java +++ b/dhis-2/dhis-api/src/main/java/org/hisp/dhis/reservedvalue/ReservedValueStore.java @@ -42,8 +42,6 @@ public interface ReservedValueStore extends GenericStore { List getAvailableValues( ReservedValue reservedValue, List values, String ownerObject); - List reserveValuesJpa(ReservedValue reservedValue, List values); - int getNumberOfUsedValues(ReservedValue reservedValue); boolean useReservedValue(String ownerUID, String value); diff --git a/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/reservedvalue/hibernate/HibernateReservedValueStore.java b/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/reservedvalue/hibernate/HibernateReservedValueStore.java index c56eb2bd19d1..93fa430b2734 100644 --- a/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/reservedvalue/hibernate/HibernateReservedValueStore.java +++ b/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/reservedvalue/hibernate/HibernateReservedValueStore.java @@ -29,9 +29,9 @@ import static com.google.common.base.Preconditions.checkNotNull; import static org.hisp.dhis.common.Objects.TRACKEDENTITYATTRIBUTE; +import static org.hisp.dhis.commons.collection.CollectionUtils.isEmpty; import java.util.List; -import java.util.Optional; import java.util.stream.Collectors; import lombok.extern.slf4j.Slf4j; import org.hibernate.SessionFactory; @@ -71,7 +71,10 @@ public HibernateReservedValueStore( @Override public List getAvailableValues( ReservedValue reservedValue, List values, String ownerObject) { - List availableValues = getIfAvailable(reservedValue, values, ownerObject); + if (isEmpty(values) || !reservedValue.getOwnerObject().equals(ownerObject)) { + return List.of(); + } + List availableValues = getIfAvailable(reservedValue, values); return availableValues.stream() .map(value -> reservedValue.toBuilder().value(value).build()) @@ -87,28 +90,22 @@ public void bulkInsertReservedValues(List toAdd) { batchHandler.flush(); } - private List getIfAvailable( - ReservedValue reservedValue, List values, String ownerObject) { - // FIX manipulating a collection argument is not ideal, make copy - Optional.of(values) - .filter(v -> !v.isEmpty() && reservedValue.getOwnerObject().equals(ownerObject)) - .ifPresent( - v -> - values.removeAll( - getSession() - .createNamedQuery("getRandomGeneratedAvailableValuesNamedQuery") - .setParameter("teaId", reservedValue.getTrackedEntityAttributeId()) - .setParameter("ownerObject", reservedValue.getOwnerObject()) - .setParameter("ownerUid", reservedValue.getOwnerUid()) - .setParameter("key", reservedValue.getKey()) - .setParameter( - "values", - v.parallelStream() - .map(String::toLowerCase) - .collect(Collectors.toList())) - .list())); - - return values; + private List getIfAvailable(ReservedValue reservedValue, List values) { + + List teavOrReservedValues = + getSession() + .createNamedQuery("getRandomGeneratedValuesNotAvailableNamedQuery") + .setParameter("teaId", reservedValue.getTrackedEntityAttributeId()) + .setParameter("ownerObject", reservedValue.getOwnerObject()) + .setParameter("ownerUid", reservedValue.getOwnerUid()) + .setParameter("key", reservedValue.getKey()) + .setParameter( + "values", values.stream().map(String::toLowerCase).collect(Collectors.toList())) + .list(); + + return values.stream() + .filter(rv -> !teavOrReservedValues.contains(rv)) + .collect(Collectors.toList()); } @Override @@ -120,17 +117,6 @@ public void reserveValues(List reservedValues) { batchHandler.flush(); } - @Override - public List reserveValuesJpa(ReservedValue reservedValue, List values) { - List toAdd = - values.stream() - .map(value -> reservedValue.toBuilder().value(value).build()) - .collect(Collectors.toList()); - - toAdd.forEach(this::save); - return toAdd; - } - @Override public int getNumberOfUsedValues(ReservedValue reservedValue) { Query query = diff --git a/dhis-2/dhis-services/dhis-service-core/src/main/resources/org/hisp/dhis/reservedvalue/hibernate/ReservedValue.hbm.xml b/dhis-2/dhis-services/dhis-service-core/src/main/resources/org/hisp/dhis/reservedvalue/hibernate/ReservedValue.hbm.xml index cf4f071ba09b..9715d05c5274 100644 --- a/dhis-2/dhis-services/dhis-service-core/src/main/resources/org/hisp/dhis/reservedvalue/hibernate/ReservedValue.hbm.xml +++ b/dhis-2/dhis-services/dhis-service-core/src/main/resources/org/hisp/dhis/reservedvalue/hibernate/ReservedValue.hbm.xml @@ -24,7 +24,7 @@ - + res = - reservedValueStore.reserveValuesJpa(rv, Lists.newArrayList(rv.getValue())); - assertEquals(1, res.size()); + saveReservedValue(reservedValue.value(prog002).build()); assertEquals(reservedValueStore.getCount(), count + 1); } @@ -120,46 +114,39 @@ void isReservedShouldBeFalse() { @Test void reserveValuesMultipleValues() { - ReservedValue rv = reservedValue.value(prog001).build(); - reservedValueStore.save(rv); + saveReservedValue(reservedValue.value(prog001).build()); int count = reservedValueStore.getCount(); - ArrayList values = new ArrayList<>(); + int counter = 0; int n = 10; for (int i = 0; i < n; i++) { - values.add(String.format("%03d", counter++)); + saveReservedValue( + ReservedValue.builder() + .ownerObject(Objects.TRACKEDENTITYATTRIBUTE.name()) + .created(new Date()) + .ownerUid("FREE") + .key("00X") + .value(String.format("%03d", counter++)) + .expiryDate(futureDate) + .build()); } - List res = reservedValueStore.reserveValuesJpa(getFreeReservedValue(), values); - assertEquals(n, res.size()); - assertEquals((count + n), reservedValueStore.getCount()); - } - @Test - void reserveValuesMultipleValuesAlreadyReservedAndUsed() { - ReservedValue rv = reservedValue.value(prog001).build(); - reservedValueStore.save(rv); - int count = reservedValueStore.getCount(); - List res = - reservedValueStore.reserveValuesJpa(rv, Lists.newArrayList("002", "003", "004")); - assertEquals(1, count); - assertEquals(3, res.size()); - assertEquals((count + 3), reservedValueStore.getCount()); + assertEquals((count + counter), reservedValueStore.getCount()); } @Test void getIfReservedValuesReturnsReservedValue() { ReservedValue rv = reservedValue.value(prog001).build(); - reservedValueStore.save(rv); + saveReservedValue(rv); List res = reservedValueStore.getAvailableValues( rv, Lists.newArrayList(rv.getValue()), rv.getOwnerObject()); - assertEquals(rv, res.get(0)); - assertEquals(1, res.size()); + assertEquals(0, res.size()); } @Test void getAvailableValuesWhenNotReserved() { ReservedValue rv = reservedValue.value(prog001).build(); - reservedValueStore.save(rv); + saveReservedValue(rv); assertEquals(1, reservedValueStore.getAll().size()); List res = reservedValueStore.getAvailableValues( @@ -203,7 +190,7 @@ void removeExpiredReservations() { pastDate.add(Calendar.DATE, -1); reservedValue.expiryDate(pastDate.getTime()); ReservedValue rv = reservedValue.value(prog001).build(); - reservedValueStore.reserveValuesJpa(rv, Lists.newArrayList(rv.getValue())); + saveReservedValue(rv); assertTrue( reservedValueStore.isReserved(Objects.TRACKEDENTITYATTRIBUTE.name(), teaUid, prog001)); reservedValueStore.removeUsedOrExpiredReservations(); @@ -212,8 +199,7 @@ void removeExpiredReservations() { @Test void removeExpiredReservationsDoesNotRemoveAnythingIfNothingHasExpired() { - ReservedValue rv = reservedValue.value(prog001).build(); - reservedValueStore.save(rv); + saveReservedValue(reservedValue.value(prog001).build()); int num = reservedValueStore.getCount(); reservedValueStore.removeUsedOrExpiredReservations(); assertEquals(num, reservedValueStore.getCount()); @@ -221,8 +207,7 @@ void removeExpiredReservationsDoesNotRemoveAnythingIfNothingHasExpired() { @Test void shouldNotAddAlreadyReservedValues() { - ReservedValue rv = reservedValue.value(prog001).build(); - reservedValueStore.save(rv); + saveReservedValue(reservedValue.value(prog001).build()); OrganisationUnit ou = createOrganisationUnit("OU"); organisationUnitStore.save(ou); TrackedEntityInstance tei = createTrackedEntityInstance(ou); @@ -238,8 +223,7 @@ void shouldNotAddAlreadyReservedValues() { @Test void shouldRemoveAlreadyUsedReservedValues() { - ReservedValue rv = reservedValue.value(prog001).build(); - reservedValueStore.save(rv); + saveReservedValue(reservedValue.value(prog001).build()); OrganisationUnit ou = createOrganisationUnit("OU"); organisationUnitStore.save(ou); TrackedEntityInstance tei = createTrackedEntityInstance(ou); @@ -261,9 +245,8 @@ void shouldRemoveAlreadyUsedOrExpiredReservedValues() { // expired value Calendar pastDate = Calendar.getInstance(); pastDate.add(Calendar.DATE, -1); - reservedValueStore.reserveValuesJpa( - reservedValue.expiryDate(pastDate.getTime()).value(prog002).build(), - Lists.newArrayList(prog002)); + saveReservedValue(reservedValue.value(prog002).expiryDate(pastDate.getTime()).build()); + // used value OrganisationUnit ou = createOrganisationUnit("OU"); organisationUnitStore.save(ou); @@ -285,14 +268,14 @@ void shouldRemoveAlreadyUsedOrExpiredReservedValues() { assertEquals(0, reservedValueStore.getCount()); } - private ReservedValue getFreeReservedValue() { - return ReservedValue.builder() - .ownerObject(Objects.TRACKEDENTITYATTRIBUTE.name()) - .created(new Date()) - .ownerUid("FREE") - .key("00X") - .value(String.format("%03d", counter++)) - .expiryDate(futureDate) - .build(); + /** + * Save reserved value and clear session to persist. In the reserved value store, the save method + * is not transactional + * + * @param reservedValue + */ + private void saveReservedValue(ReservedValue reservedValue) { + reservedValueStore.save(reservedValue); + dbmsManager.clearSession(); } }