Skip to content

Commit

Permalink
clean: reserved values (#16003)
Browse files Browse the repository at this point in the history
  • Loading branch information
@lucaCambi77
lucaCambi77 authored Jan 5, 2024
1 parent 4f83aa7 commit ba87bae
Show file tree
Hide file tree
Showing 5 changed files with 65 additions and 87 deletions.
2 changes: 0 additions & 2 deletions dhis-2/dhis-api/src/main/java/org/hisp/dhis/reservedvalue/ReservedValueStore.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,6 @@ public interface ReservedValueStore extends GenericStore<ReservedValue> {
List<ReservedValue> getAvailableValues(
ReservedValue reservedValue, List<String> values, String ownerObject);

List<ReservedValue> reserveValuesJpa(ReservedValue reservedValue, List<String> values);

int getNumberOfUsedValues(ReservedValue reservedValue);

boolean useReservedValue(String ownerUID, String value);
Expand Down
56 changes: 21 additions & 35 deletions ...core/src/main/java/org/hisp/dhis/reservedvalue/hibernate/HibernateReservedValueStore.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -71,7 +71,10 @@ public HibernateReservedValueStore(
@Override
public List<ReservedValue> getAvailableValues(
ReservedValue reservedValue, List<String> values, String ownerObject) {
List<String> availableValues = getIfAvailable(reservedValue, values, ownerObject);
if (isEmpty(values) || !reservedValue.getOwnerObject().equals(ownerObject)) {
return List.of();
}
List<String> availableValues = getIfAvailable(reservedValue, values);

return availableValues.stream()
.map(value -> reservedValue.toBuilder().value(value).build())
Expand All @@ -87,28 +90,22 @@ public void bulkInsertReservedValues(List<ReservedValue> toAdd) {
batchHandler.flush();
}

private List<String> getIfAvailable(
ReservedValue reservedValue, List<String> 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<String> getIfAvailable(ReservedValue reservedValue, List<String> 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
Expand All @@ -120,17 +117,6 @@ public void reserveValues(List<ReservedValue> reservedValues) {
batchHandler.flush();
}

@Override
public List<ReservedValue> reserveValuesJpa(ReservedValue reservedValue, List<String> values) {
List<ReservedValue> 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<Long> query =
Expand Down
2 changes: 1 addition & 1 deletion ...rvice-core/src/main/resources/org/hisp/dhis/reservedvalue/hibernate/ReservedValue.hbm.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@

</class>

<sql-query name="getRandomGeneratedAvailableValuesNamedQuery">
<sql-query name="getRandomGeneratedValuesNotAvailableNamedQuery">
<![CDATA[ SELECT value
FROM
trackedentityattributevalue teav
Expand Down
11 changes: 11 additions & 0 deletions ...ration/src/test/java/org/hisp/dhis/reservedvalue/ReservedValueServiceIntegrationTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -258,4 +258,15 @@ private static TrackedEntityAttribute createTextPattern(
}
return null;
}

/**
* 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();
}
}
81 changes: 32 additions & 49 deletions .../src/test/java/org/hisp/dhis/reservedvalue/hibernate/HibernateReservedValueStoreTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@
import static org.junit.jupiter.api.Assertions.assertTrue;

import com.google.common.collect.Lists;
import java.util.ArrayList;
import java.util.Calendar;
import java.util.Date;
import java.util.List;
Expand All @@ -55,8 +54,6 @@

class HibernateReservedValueStoreTest extends SingleSetupIntegrationTestBase {

private static int counter = 1;

private static final String teaUid = "tea";

private static final String prog001 = "001";
Expand Down Expand Up @@ -95,12 +92,9 @@ protected void setUpTest() {

@Test
void reserveValuesSingleValue() {
reservedValueStore.save(reservedValue.value(prog001).build());
saveReservedValue(reservedValue.value(prog001).build());
int count = reservedValueStore.getCount();
ReservedValue rv = reservedValue.value(prog002).build();
List<ReservedValue> res =
reservedValueStore.reserveValuesJpa(rv, Lists.newArrayList(rv.getValue()));
assertEquals(1, res.size());
saveReservedValue(reservedValue.value(prog002).build());
assertEquals(reservedValueStore.getCount(), count + 1);
}

Expand All @@ -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<String> 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<ReservedValue> 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<ReservedValue> 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<ReservedValue> 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<ReservedValue> res =
reservedValueStore.getAvailableValues(
Expand Down Expand Up @@ -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();
Expand All @@ -212,17 +199,15 @@ 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());
}

@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);
Expand All @@ -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);
Expand All @@ -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);
Expand All @@ -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();
}
}

0 comments on commit ba87bae

Please sign in to comment.