Skip to content

Commit

Permalink
fix: /tracker/relationships?order=createdAt (#16100)
Browse files Browse the repository at this point in the history
In https://dhis2.atlassian.net/browse/TECH-1619 we harmonized ordering
relationships in 2.41. This now allows ordering by createdAt as the
only field. Any other field will be rejected with a 400 like in 2.41.
2.41 also added ordering by createdAtClient which will not be backported.
  • Loading branch information
teleivo authored Jan 8, 2024
1 parent 4215ac0 commit ed11497
Show file tree
Hide file tree
Showing 8 changed files with 312 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,15 @@ public List<OrderCriteria> getOrder() {
return dtoNameToDatabaseNameTranslator.apply(orderCriteriaPartitionedByAllowance.get(true));
}

/**
* Get the raw {@link #order} field, without any field name translation. Added the method to
* validate the user provided order fields without affecting code relying on the existing behavior
* of {@link #getOrder()}.
*/
public List<OrderCriteria> getRawOrder() {
return order;
}

private boolean isAllowed(OrderCriteria orderCriteria) {
return getAllowedOrderingFields().contains(orderCriteria.getField());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@
import org.hisp.dhis.webapi.mvc.messageconverter.JsonMessageConverter;
import org.hisp.dhis.webapi.mvc.messageconverter.XmlDeprecationNoticeHttpMessageConverter;
import org.hisp.dhis.webapi.mvc.messageconverter.XmlMessageConverter;
import org.hisp.dhis.webapi.security.config.StringToOrderCriteriaListConverter;
import org.hisp.dhis.webapi.view.CustomPathExtensionContentNegotiationStrategy;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.beans.factory.annotation.Qualifier;
Expand Down Expand Up @@ -201,6 +202,7 @@ public void configureMessageConverters(List<HttpMessageConverter<?>> converters)

@Override
public void addFormatters(FormatterRegistry registry) {
registry.addConverter(new StringToOrderCriteriaListConverter());
registry.addConverter(new FieldPathConverter());
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,10 @@ default String getRelationshipType() {
return getString("relationshipType").string();
}

default String getCreatedAt() {
return getString("createdAt").string();
}

default JsonRelationshipItem getFrom() {
return get("from").as(JsonRelationshipItem.class);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -473,6 +473,18 @@ void getRelationshipsByTrackedEntityWithEnrollments() {
assertEquals(to.getUid(), enrollment.getTrackedEntity());
}

@Test
void getRelationshipsByTrackedEntityWithEnrollmentsOrderByIllegalField() {
TrackedEntityInstance to = trackedEntityInstance();
ProgramInstance from = programInstance(to);
relationship(from, to);

GET(
"/tracker/relationships?trackedEntity={tei}&order=created&fields=relationship,createdAt",
to.getUid())
.content(HttpStatus.BAD_REQUEST);
}

@Test
void getRelationshipsByTrackedEntityWithAttributes() {
TrackedEntityInstance to = trackedEntityInstance(orgUnit);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,131 @@
/*
* Copyright (c) 2004-2022, University of Oslo
* All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions are met:
* Redistributions of source code must retain the above copyright notice, this
* list of conditions and the following disclaimer.
*
* Redistributions in binary form must reproduce the above copyright notice,
* this list of conditions and the following disclaimer in the documentation
* and/or other materials provided with the distribution.
* Neither the name of the HISP project nor the names of its contributors may
* be used to endorse or promote products derived from this software without
* specific prior written permission.
*
* THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" AND
* ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED
* WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
* DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS BE LIABLE FOR
* ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES
* (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
* LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON
* ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
* (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS
* SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
*/
package org.hisp.dhis.webapi.controller.tracker.export;

import java.util.List;
import java.util.Map.Entry;
import java.util.Set;
import java.util.function.Function;
import java.util.stream.Collectors;
import org.hisp.dhis.common.CodeGenerator;
import org.hisp.dhis.webapi.controller.event.webrequest.OrderCriteria;
import org.hisp.dhis.webapi.controller.exception.BadRequestException;

/**
* RequestParamUtils are functions used to parse and transform tracker request parameters. This
* class is intended to only house functions without any dependencies on services or components.
*/
class RequestParamsValidator {
private RequestParamsValidator() {
throw new IllegalStateException("Utility class");
}

/**
* Validate the {@code order} request parameter in tracker exporters. Allowed order values are
* {@code supportedFieldNames} and UIDs which represent {@code uidMeaning}. Every field name or
* UID can be specified at most once.
*/
public static void validateOrderParams(
List<OrderCriteria> order, Set<String> supportedFieldNames, String uidMeaning)
throws BadRequestException {
if (order == null || order.isEmpty()) {
return;
}

Set<String> invalidOrderComponents =
order.stream().map(OrderCriteria::getField).collect(Collectors.toSet());
invalidOrderComponents.removeAll(supportedFieldNames);
Set<String> uids =
invalidOrderComponents.stream()
.filter(CodeGenerator::isValidUid)
.collect(Collectors.toSet());
invalidOrderComponents.removeAll(uids);

String errorSuffix =
String.format(
"Supported are %s UIDs and fields '%s'. All of which can at most be specified once.",
uidMeaning, supportedFieldNames.stream().sorted().collect(Collectors.joining(", ")));
if (!invalidOrderComponents.isEmpty()) {
throw new BadRequestException(
String.format(
"order parameter is invalid. Cannot order by '%s'. %s",
String.join(", ", invalidOrderComponents), errorSuffix));
}

validateOrderParamsContainNoDuplicates(order, errorSuffix);
}

private static void validateOrderParamsContainNoDuplicates(
List<OrderCriteria> order, String errorSuffix) throws BadRequestException {
Set<String> duplicateOrderComponents =
order.stream()
.map(OrderCriteria::getField)
.collect(Collectors.groupingBy(Function.identity(), Collectors.counting()))
.entrySet()
.stream()
.filter(e -> e.getValue() > 1)
.map(Entry::getKey)
.collect(Collectors.toSet());

if (!duplicateOrderComponents.isEmpty()) {
throw new BadRequestException(
String.format(
"order parameter is invalid. '%s' are repeated. %s",
String.join(", ", duplicateOrderComponents), errorSuffix));
}
}

/**
* Validate the {@code order} request parameter in tracker exporters. Allowed order values are
* {@code supportedFieldNames}. Every field name can be specified at most once. If the endpoint
* supports field names and UIDs use {@link #validateOrderParams(List, Set, String)}.
*/
public static void validateOrderParams(List<OrderCriteria> order, Set<String> supportedFieldNames)
throws BadRequestException {
if (order == null || order.isEmpty()) {
return;
}

Set<String> invalidOrderComponents =
order.stream().map(OrderCriteria::getField).collect(Collectors.toSet());
invalidOrderComponents.removeAll(supportedFieldNames);

String errorSuffix =
String.format(
"Supported fields are '%s'. All of which can at most be specified once.",
supportedFieldNames.stream().sorted().collect(Collectors.joining(", ")));
if (!invalidOrderComponents.isEmpty()) {
throw new BadRequestException(
String.format(
"order parameter is invalid. Cannot order by '%s'. %s",
String.join(", ", invalidOrderComponents), errorSuffix));
}

validateOrderParamsContainNoDuplicates(order, errorSuffix);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -27,16 +27,20 @@
*/
package org.hisp.dhis.webapi.controller.tracker.export;

import static java.util.Map.entry;
import static org.hisp.dhis.dxf2.webmessage.WebMessageUtils.notFound;
import static org.hisp.dhis.webapi.controller.tracker.TrackerControllerSupport.RESOURCE_PATH;
import static org.hisp.dhis.webapi.controller.tracker.export.RequestParamsValidator.validateOrderParams;
import static org.springframework.http.MediaType.APPLICATION_JSON_VALUE;

import com.fasterxml.jackson.databind.node.ObjectNode;
import com.google.common.collect.ImmutableMap;
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
import java.util.Map;
import java.util.Optional;
import java.util.Set;
import java.util.function.BiFunction;
import java.util.function.Function;
import java.util.function.Supplier;
Expand All @@ -58,6 +62,7 @@
import org.hisp.dhis.program.ProgramStageInstanceService;
import org.hisp.dhis.trackedentity.TrackedEntityInstance;
import org.hisp.dhis.trackedentity.TrackedEntityInstanceService;
import org.hisp.dhis.webapi.controller.event.webrequest.OrderCriteria;
import org.hisp.dhis.webapi.controller.event.webrequest.PagingAndSortingCriteriaAdapter;
import org.hisp.dhis.webapi.controller.event.webrequest.PagingWrapper;
import org.hisp.dhis.webapi.controller.event.webrequest.tracker.TrackerRelationshipCriteria;
Expand All @@ -80,6 +85,15 @@
public class TrackerRelationshipsExportController {
protected static final String RELATIONSHIPS = "relationships";

/**
* Relationships can be ordered by given fields which correspond to fields on {@link
* org.hisp.dhis.relationship.Relationship}.
*/
private static final Map<String, String> ORDERABLE_FIELDS =
Map.ofEntries(entry("createdAt", "created"));

private static final Set<String> ORDERABLE_FIELD_NAMES = ORDERABLE_FIELDS.keySet();

private static final String DEFAULT_FIELDS_PARAM =
"relationship,relationshipType,from[trackedEntity[trackedEntity],enrollment[enrollment],event[event]],to[trackedEntity[trackedEntity],enrollment[enrollment],event[event]]";

Expand Down Expand Up @@ -191,6 +205,16 @@ private List<org.hisp.dhis.tracker.domain.Relationship> tryGetRelationshipFrom(
Class<?> type,
Supplier<WebMessage> notFoundMessageSupplier,
PagingAndSortingCriteriaAdapter pagingAndSortingCriteria) {

if (pagingAndSortingCriteria != null) {
// validate and translate from user facing field names to internal field names users can order
// by avoiding field translation by PagingAndSortingCriteriaAdapter to validate user order
// fields and to not affect old tracker.
validateOrderParams(pagingAndSortingCriteria.getRawOrder(), ORDERABLE_FIELD_NAMES);
List<OrderCriteria> orders = mapOrderParam(pagingAndSortingCriteria.getRawOrder());
pagingAndSortingCriteria.setOrder(orders);
}

if (identifier != null) {
Object object = getObjectRetriever(type).apply(identifier);
if (object != null) {
Expand Down Expand Up @@ -219,4 +243,18 @@ private List<org.hisp.dhis.tracker.domain.Relationship> tryGetRelationshipFrom(
.orElseThrow(
() -> new IllegalArgumentException("Unable to detect object retriever from " + type));
}

private static List<OrderCriteria> mapOrderParam(List<OrderCriteria> orders) {
if (orders == null || orders.isEmpty()) {
return List.of();
}

List<OrderCriteria> result = new ArrayList<>();
for (OrderCriteria order : orders) {
if (ORDERABLE_FIELDS.containsKey(order.getField())) {
result.add(OrderCriteria.of(ORDERABLE_FIELDS.get(order.getField()), order.getDirection()));
}
}
return result;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@
*
* @author Giuseppe Nespolino <[email protected]>
*/
class StringToOrderCriteriaListConverter implements Converter<String, List<OrderCriteria>> {
public class StringToOrderCriteriaListConverter implements Converter<String, List<OrderCriteria>> {
@Override
public List<OrderCriteria> convert(String source) {
return OrderCriteria.fromOrderString(source);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,115 @@
/*
* Copyright (c) 2004-2022, University of Oslo
* All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions are met:
* Redistributions of source code must retain the above copyright notice, this
* list of conditions and the following disclaimer.
*
* Redistributions in binary form must reproduce the above copyright notice,
* this list of conditions and the following disclaimer in the documentation
* and/or other materials provided with the distribution.
* Neither the name of the HISP project nor the names of its contributors may
* be used to endorse or promote products derived from this software without
* specific prior written permission.
*
* THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" AND
* ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED
* WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
* DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS BE LIABLE FOR
* ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES
* (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
* LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON
* ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
* (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS
* SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
*/
package org.hisp.dhis.webapi.controller.tracker.export;

import static org.hisp.dhis.utils.Assertions.assertContains;
import static org.hisp.dhis.utils.Assertions.assertStartsWith;
import static org.hisp.dhis.webapi.controller.event.webrequest.OrderCriteria.fromOrderString;
import static org.hisp.dhis.webapi.controller.tracker.export.RequestParamsValidator.validateOrderParams;
import static org.junit.jupiter.api.Assertions.assertAll;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;

import java.util.Set;
import org.hisp.dhis.common.CodeGenerator;
import org.hisp.dhis.webapi.controller.exception.BadRequestException;
import org.junit.jupiter.api.Test;

/** Tests {@link RequestParamsValidator}. */
class RequestParamsValidatorTest {

@Test
void shouldPassOrderParamsValidationWhenGivenOrderIsOrderable() throws BadRequestException {
Set<String> supportedFieldNames = Set.of("createdAt", "scheduledAt");

validateOrderParams(fromOrderString("createdAt:asc,scheduledAt:asc"), supportedFieldNames, "");
}

@Test
void shouldFailOrderParamsValidationWhenGivenInvalidOrderComponents() {
Set<String> supportedFieldNames = Set.of("enrolledAt");
String invalidUID = "Cogn34Del";
assertFalse(CodeGenerator.isValidUid(invalidUID));

Exception exception =
assertThrows(
BadRequestException.class,
() ->
validateOrderParams(
fromOrderString(
"unsupportedProperty1:asc,enrolledAt:asc,"
+ invalidUID
+ ",unsupportedProperty2:desc"),
supportedFieldNames,
"data element and attribute"));
assertAll(
() -> assertStartsWith("order parameter is invalid", exception.getMessage()),
() ->
assertContains(
"Supported are data element and attribute UIDs and fields", exception.getMessage()),
// order of fields might not always be the same; therefore using contains
() -> assertContains(invalidUID, exception.getMessage()),
() -> assertContains("unsupportedProperty1", exception.getMessage()),
() -> assertContains("unsupportedProperty2", exception.getMessage()));
}

@Test
void shouldPassOrderParamsValidationWhenGivenInvalidOrderNameWhichIsAValidUID()
throws BadRequestException {
Set<String> supportedFieldNames = Set.of("enrolledAt");
// This test case shows that some field names are valid UIDs. We can thus not rule out all
// invalid field names and UIDs at this stage as we do not have access to data element/attribute
// services. Such invalid order values will be caught in the service (mapper).
assertTrue(CodeGenerator.isValidUid("lastUpdated"));

validateOrderParams(fromOrderString("lastUpdated:desc"), supportedFieldNames, "");
}

@Test
void shouldFailOrderParamsValidationWhenGivenRepeatedOrderComponents() {
Set<String> supportedFieldNames = Set.of("createdAt", "enrolledAt");

Exception exception =
assertThrows(
BadRequestException.class,
() ->
validateOrderParams(
fromOrderString(
"zGlzbfreTOH,createdAt:asc,enrolledAt:asc,enrolledAt,zGlzbfreTOH"),
supportedFieldNames,
""));

assertAll(
() -> assertStartsWith("order parameter is invalid", exception.getMessage()),
// order of fields might not always be the same; therefore using contains
() -> assertContains("repeated", exception.getMessage()),
() -> assertContains("enrolledAt", exception.getMessage()),
() -> assertContains("zGlzbfreTOH", exception.getMessage()));
}
}

0 comments on commit ed11497

Please sign in to comment.