Skip to content

Commit

Permalink
feat: Add filtering capabilities in events change log [DHIS2-18012] (#…
Browse files Browse the repository at this point in the history
…19268)

* feat: Add filtering capabilities in events change log [DHIS2-18012]

* feat: Add filtering capabilities in events change log [DHIS2-18012]

* feat: Add filtering capabilities in events change log [DHIS2-18012]

* feat: Address PR comments [DHIS2-18012]

* feat: Address PR comments [DHIS2-18012]
  • Loading branch information
muilpp authored Nov 25, 2024
1 parent 29e8f40 commit 2313c47
Show file tree
Hide file tree
Showing 11 changed files with 292 additions and 17 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@
import java.util.stream.Stream;
import javax.annotation.Nonnull;
import lombok.RequiredArgsConstructor;
import org.apache.commons.lang3.tuple.Pair;
import org.hisp.dhis.changelog.ChangeLogType;
import org.hisp.dhis.common.UID;
import org.hisp.dhis.dataelement.DataElement;
Expand Down Expand Up @@ -69,8 +70,7 @@ public Page<EventChangeLog> getEventChangeLog(
// check existence and access
eventService.getEvent(event);

return hibernateEventChangeLogStore.getEventChangeLogs(
event, operationParams.getOrder(), pageParams);
return hibernateEventChangeLogStore.getEventChangeLogs(event, operationParams, pageParams);
}

@Transactional
Expand Down Expand Up @@ -165,6 +165,11 @@ public Set<String> getOrderableFields() {
return hibernateEventChangeLogStore.getOrderableFields();
}

@Override
public Set<Pair<String, Class<?>>> getFilterableFields() {
return hibernateEventChangeLogStore.getFilterableFields();
}

private <T> void logIfChanged(
String propertyName,
Function<Event, T> valueExtractor,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@
import lombok.AllArgsConstructor;
import lombok.Builder;
import lombok.Getter;
import org.apache.commons.lang3.tuple.Pair;
import org.hisp.dhis.common.QueryFilter;
import org.hisp.dhis.common.SortDirection;
import org.hisp.dhis.tracker.export.Order;

Expand All @@ -40,20 +42,30 @@
public class EventChangeLogOperationParams {

private Order order;
private Pair<String, QueryFilter> filter;

public static class EventChangeLogOperationParamsBuilder {

// Do not remove this unused method. This hides the order field from the builder which Lombok
// does not support. The repeated order field and private order method prevent access to order
// via the builder.
// Order should be added via the orderBy builder methods.
// Do not remove these unused methods. They hide the order and filter fields from the builder
// which Lombok
// does not support.
// They should be added via their respective orderBy and filterBy builder methods.
private EventChangeLogOperationParamsBuilder order(Order order) {
return this;
}

private EventChangeLogOperationParamsBuilder filter(Pair<String, QueryFilter> filter) {
return this;
}

public EventChangeLogOperationParamsBuilder orderBy(String field, SortDirection direction) {
this.order = new Order(field, direction);
return this;
}

public EventChangeLogOperationParamsBuilder filterBy(String field, QueryFilter filter) {
this.filter = Pair.of(field, filter);
return this;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
import java.util.List;
import java.util.Set;
import javax.annotation.Nonnull;
import org.apache.commons.lang3.tuple.Pair;
import org.hisp.dhis.changelog.ChangeLogType;
import org.hisp.dhis.common.UID;
import org.hisp.dhis.dataelement.DataElement;
Expand Down Expand Up @@ -85,9 +86,17 @@ void addPropertyChangeLog(

/**
* Fields the {@link #getEventChangeLog(UID, EventChangeLogOperationParams, PageParams)} can order
* event change logs by. Ordering by fields other than these is considered a programmer error.
* event change logs by. Ordering by fields other than these, is considered a programmer error.
* Validation of user provided field names should occur before calling {@link
* #getEventChangeLog(UID, EventChangeLogOperationParams, PageParams)}.
*/
Set<String> getOrderableFields();

/**
* Fields the {@link #getEventChangeLog(UID, EventChangeLogOperationParams, PageParams)} can
* filter event change logs by. Filtering by fields other than these, is considered a programmer
* error. Validation of user provided field names should occur before calling {@link
* #getEventChangeLog(UID, EventChangeLogOperationParams, PageParams)}.
*/
Set<Pair<String, Class<?>>> getFilterableFields();
}
Original file line number Diff line number Diff line change
Expand Up @@ -34,11 +34,13 @@
import java.util.Date;
import java.util.List;
import java.util.Map;
import java.util.Map.Entry;
import java.util.Set;
import javax.annotation.Nonnull;
import javax.annotation.Nullable;
import org.apache.commons.lang3.tuple.Pair;
import org.hibernate.Session;
import org.hisp.dhis.changelog.ChangeLogType;
import org.hisp.dhis.common.QueryFilter;
import org.hisp.dhis.common.SortDirection;
import org.hisp.dhis.common.UID;
import org.hisp.dhis.dataelement.DataElement;
Expand Down Expand Up @@ -73,6 +75,12 @@ public class HibernateEventChangeLogStore {
entry("dataElement", COLUMN_CHANGELOG_DATA_ELEMENT),
entry("property", COLUMN_CHANGELOG_PROPERTY));

private static final Map<Pair<String, Class<?>>, String> FILTERABLE_FIELDS =
Map.ofEntries(
entry(Pair.of("username", String.class), COLUMN_CHANGELOG_USER),
entry(Pair.of("dataElement", UID.class), COLUMN_CHANGELOG_DATA_ELEMENT),
entry(Pair.of("property", String.class), COLUMN_CHANGELOG_PROPERTY));

private final EntityManager entityManager;

public HibernateEventChangeLogStore(EntityManager entityManager) {
Expand All @@ -84,11 +92,14 @@ public void addEventChangeLog(EventChangeLog eventChangeLog) {
}

public Page<EventChangeLog> getEventChangeLogs(
@Nonnull UID event, @Nullable Order order, @Nonnull PageParams pageParams) {
@Nonnull UID event,
@Nonnull EventChangeLogOperationParams operationParams,
@Nonnull PageParams pageParams) {

Pair<String, QueryFilter> filter = operationParams.getFilter();

String hql =
String.format(
"""
"""
select ecl.event,
ecl.dataElement,
ecl.eventProperty,
Expand All @@ -105,15 +116,30 @@ public Page<EventChangeLog> getEventChangeLogs(
left join ecl.dataElement d
left join ecl.createdBy u
where e.uid = :eventUid
order by %s
"""
.formatted(sortExpressions(order)));
""";

if (filter != null) {
String filterField =
FILTERABLE_FIELDS.entrySet().stream()
.filter(entry -> entry.getKey().getLeft().equals(filter.getKey()))
.findFirst()
.map(Entry::getValue)
.get();

hql += String.format(" and %s = :filterValue ", filterField);
}

hql += String.format("order by %s".formatted(sortExpressions(operationParams.getOrder())));

Query query = entityManager.createQuery(hql);
query.setParameter("eventUid", event.getValue());
query.setFirstResult((pageParams.getPage() - 1) * pageParams.getPageSize());
query.setMaxResults(pageParams.getPageSize() + 1);

if (filter != null) {
query.setParameter("filterValue", filter.getValue().getFilter());
}

List<Object[]> results = query.getResultList();
List<EventChangeLog> eventChangeLogs =
results.stream()
Expand Down Expand Up @@ -189,4 +215,8 @@ private static String sortExpressions(Order order) {
public Set<String> getOrderableFields() {
return ORDERABLE_FIELDS.keySet();
}

public Set<Pair<String, Class<?>>> getFilterableFields() {
return FILTERABLE_FIELDS.keySet();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
*/
package org.hisp.dhis.tracker.export.event;

import static org.hisp.dhis.test.utils.Assertions.assertContainsOnly;
import static org.hisp.dhis.tracker.Assertions.assertNoErrors;
import static org.junit.jupiter.api.Assertions.assertAll;
import static org.junit.jupiter.api.Assertions.assertEquals;
Expand All @@ -35,8 +36,12 @@
import java.io.IOException;
import java.time.Instant;
import java.util.List;
import java.util.Set;
import java.util.stream.Collectors;
import java.util.stream.Stream;
import org.hisp.dhis.common.IdentifiableObjectManager;
import org.hisp.dhis.common.QueryFilter;
import org.hisp.dhis.common.QueryOperator;
import org.hisp.dhis.common.SortDirection;
import org.hisp.dhis.common.UID;
import org.hisp.dhis.feedback.ForbiddenException;
Expand All @@ -59,7 +64,7 @@
import org.junit.jupiter.params.provider.MethodSource;
import org.springframework.beans.factory.annotation.Autowired;

class OrderEventChangeLogTest extends TrackerTest {
class OrderAndFilterEventChangeLogTest extends TrackerTest {

@Autowired private EventChangeLogService eventChangeLogService;

Expand Down Expand Up @@ -260,6 +265,67 @@ void shouldSortChangeLogsWhenOrderingByPropertyDesc()
() -> assertPropertyCreate("geometry", "(-11.419700, 8.103900)", changeLogs.get(4)));
}

@Test
void shouldFilterChangeLogsWhenFilteringByUser() throws ForbiddenException, NotFoundException {
EventChangeLogOperationParams params =
EventChangeLogOperationParams.builder()
.filterBy("username", new QueryFilter(QueryOperator.EQ, importUser.getUsername()))
.build();

Page<EventChangeLog> changeLogs =
eventChangeLogService.getEventChangeLog(UID.of("QRYjLTiJTrA"), params, defaultPageParams);

Set<String> changeLogUsers =
changeLogs.getItems().stream()
.map(cl -> cl.getCreatedBy().getUsername())
.collect(Collectors.toSet());
assertContainsOnly(List.of(importUser.getUsername()), changeLogUsers);
}

@Test
void shouldFilterChangeLogsWhenFilteringByDataElement()
throws ForbiddenException, NotFoundException {
Event event = getEvent("kWjSezkXHVp");
String dataElement = getFirstDataElement(event);
EventChangeLogOperationParams params =
EventChangeLogOperationParams.builder()
.filterBy("dataElement", new QueryFilter(QueryOperator.EQ, dataElement))
.build();

Page<EventChangeLog> changeLogs =
eventChangeLogService.getEventChangeLog(UID.of(event.getUid()), params, defaultPageParams);

Set<String> changeLogDataElements =
changeLogs.getItems().stream()
.map(cl -> cl.getDataElement().getUid())
.collect(Collectors.toSet());
assertContainsOnly(List.of(dataElement), changeLogDataElements);
}

private Stream<Arguments> provideEventProperties() {
return Stream.of(
Arguments.of("occurredAt"), Arguments.of("scheduledAt"), Arguments.of("geometry"));
}

@ParameterizedTest
@MethodSource("provideEventProperties")
void shouldFilterChangeLogsWhenFilteringByProperties(String filterValue)
throws ForbiddenException, NotFoundException {
EventChangeLogOperationParams params =
EventChangeLogOperationParams.builder()
.filterBy("property", new QueryFilter(QueryOperator.EQ, filterValue))
.build();

Page<EventChangeLog> changeLogs =
eventChangeLogService.getEventChangeLog(UID.of("QRYjLTiJTrA"), params, defaultPageParams);

Set<String> changeLogOccurredAtProperties =
changeLogs.getItems().stream()
.map(EventChangeLog::getEventProperty)
.collect(Collectors.toSet());
assertContainsOnly(List.of(filterValue), changeLogOccurredAtProperties);
}

private void updateDataValue(String event, String dataElementUid, String newValue) {
trackerObjects.getEvents().stream()
.filter(e -> e.getEvent().getValue().equalsIgnoreCase(event))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -223,6 +223,24 @@ void shouldGetEventChangeLogInAscOrder() {
"scheduledAt", "2023-01-10 00:00:00.000", eventPropertyChangeLogs));
}

@Test
void shouldGetEventChangeLogsWhenFilteringByProperty() {
JsonList<JsonEventChangeLog> changeLogs =
GET("/tracker/events/{id}/changeLogs?filter=property:eq:occurredAt", event.getUid())
.content(HttpStatus.OK)
.getList("changeLogs", JsonEventChangeLog.class);
List<JsonEventChangeLog> eventPropertyChangeLogs =
changeLogs.stream()
.filter(log -> log.getChange().getEventProperty().getProperty() != null)
.toList();

assertAll(
() -> assertHasSize(1, eventPropertyChangeLogs),
() ->
assertPropertyCreateExists(
"occurredAt", "2023-01-10 00:00:00.000", eventPropertyChangeLogs));
}

@Test
void shouldGetChangeLogPagerWithNextElementWhenMultipleElementsImportedAndFirstPageRequested() {
JsonPage changeLogs =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,4 +51,6 @@ public class ChangeLogRequestParams implements FieldsRequestParam {
private List<FieldPath> fields = FieldFilterParser.parse(DEFAULT_FIELDS_PARAM);

private List<OrderCriteria> order = new ArrayList<>();

private String filter;
}
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@
import java.util.stream.Collectors;
import java.util.stream.Stream;
import org.apache.commons.lang3.StringUtils;
import org.apache.commons.lang3.tuple.Pair;
import org.hisp.dhis.common.CodeGenerator;
import org.hisp.dhis.common.OrganisationUnitSelectionMode;
import org.hisp.dhis.common.QueryFilter;
Expand All @@ -74,6 +75,8 @@ private RequestParamsValidator() {

private static final char ESCAPE = '/';

private static final String SUPPORTED_CHANGELOG_FILTER_OPERATOR = "eq";

/**
* Negative lookahead to avoid wrong split of comma-separated list of filters when one or more
* filter value contain comma. It skips comma escaped by slash
Expand Down Expand Up @@ -271,6 +274,54 @@ public static void validateOrderParams(List<OrderCriteria> order, Set<String> su
validateOrderParamsContainNoDuplicates(order, errorSuffix);
}

/**
* Validate the {@code filter} request parameter in change log tracker exporters. Allowed filter
* values are {@code supportedFieldNames}. Only one field name at a time can be specified. If the
* endpoint supports UIDs use {@link #parseFilters(String)}.
*/
public static void validateFilter(String filter, Set<Pair<String, Class<?>>> supportedFields)
throws BadRequestException {
if (filter == null) {
return;
}

String[] split = filter.split(":");
Set<String> supportedFieldNames =
supportedFields.stream().map(Pair::getKey).collect(Collectors.toSet());

if (split.length != 3) {
throw new BadRequestException(
String.format(
"Invalid filter => %s. Expected format is [field]:eq:[value]. Supported fields are '%s'. Only one of them can be specified at a time",
filter, String.join(", ", supportedFieldNames)));
}

if (!supportedFieldNames.contains(split[0])) {
throw new BadRequestException(
String.format(
"Invalid filter field. Supported fields are '%s'. Only one of them can be specified at a time",
String.join(", ", supportedFieldNames)));
}

if (!split[1].equalsIgnoreCase(SUPPORTED_CHANGELOG_FILTER_OPERATOR)) {
throw new BadRequestException(
String.format(
"Invalid filter operator. The only supported operator is '%s'.",
SUPPORTED_CHANGELOG_FILTER_OPERATOR));
}

for (Pair<String, Class<?>> filterField : supportedFields) {
if (filterField.getKey().equalsIgnoreCase(split[0])
&& filterField.getValue() == UID.class
&& !CodeGenerator.isValidUid(filterField.getKey())) {
throw new BadRequestException(
String.format(
"Incorrect filter value provided as UID: %s. UID must be an alphanumeric string of 11 characters starting with a letter.",
split[2]));
}
}
}

/**
* Parse given {@code input} string representing a filter for an object referenced by a UID like a
* tracked entity attribute or data element. Refer to {@link #parseSanitizedFilters(Map, String)}}
Expand Down
Loading

0 comments on commit 2313c47

Please sign in to comment.