Skip to content

Commit

Permalink
feat: remove program prefix from TEAs in CPL [DHIS2-16067] (#15579)
Browse files Browse the repository at this point in the history
* feat: no programUid prefix in dimension endpoint [DHIS2-16067]

* feat: program prefix is not required for TEAs in CPL endpoint params [DHIS2-16067]

* fix: sonarcloud warning [DHIS2-16067]

* fix: e2e dimensions tests [DHIS2-16067]

* fix: unit test [DHIS2-16067]
  • Loading branch information
gnespolino authored Nov 5, 2023
1 parent 0b88bbb commit 1253d7a
Show file tree
Hide file tree
Showing 16 changed files with 314 additions and 295 deletions.
10 changes: 10 additions & 0 deletions dhis-2/dhis-api/src/main/java/org/hisp/dhis/common/QueryItem.java
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,16 @@ public class QueryItem implements GroupableItem {
// Constructors
// -------------------------------------------------------------------------

public QueryItem(TrackedEntityAttribute item, LegendSet legendSet) {
this(
item,
null,
legendSet,
legendSet != null ? ValueType.TEXT : item.getValueType(),
item.getAggregationType(),
item.getOptionSet());
}

public QueryItem(DimensionalItemObject item) {
this.item = item;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,6 @@

import static java.util.Collections.emptySet;
import static java.util.Collections.unmodifiableList;
import static java.util.function.Function.identity;
import static java.util.stream.Collectors.toList;
import static org.hisp.dhis.analytics.EventOutputType.TRACKED_ENTITY_INSTANCE;
import static org.hisp.dhis.analytics.common.params.dimension.DimensionParam.isStaticDimensionIdentifier;
import static org.hisp.dhis.analytics.common.params.dimension.DimensionParamType.DATE_FILTERS;
Expand All @@ -54,15 +52,13 @@
import java.util.Date;
import java.util.LinkedHashSet;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.Optional;
import java.util.Set;
import java.util.UUID;
import java.util.stream.Collectors;
import java.util.stream.Stream;
import lombok.RequiredArgsConstructor;
import org.apache.commons.lang3.tuple.Pair;
import org.hisp.dhis.analytics.DataQueryService;
import org.hisp.dhis.analytics.common.CommonQueryRequest;
import org.hisp.dhis.analytics.common.params.AnalyticsPagingParams;
Expand All @@ -81,6 +77,7 @@
import org.hisp.dhis.organisationunit.OrganisationUnit;
import org.hisp.dhis.program.Program;
import org.hisp.dhis.program.ProgramService;
import org.hisp.dhis.trackedentity.TrackedEntityAttribute;
import org.hisp.dhis.webapi.controller.event.mapper.SortDirection;
import org.springframework.stereotype.Component;

Expand Down Expand Up @@ -111,29 +108,13 @@ public CommonParams map(CommonQueryRequest request) {
dataQueryService.getUserOrgUnits(null, request.getUserOrgUnit());
List<Program> programs = getPrograms(request);

Map<String, String> dimensionsByUid =
request.getDimension().stream().collect(Collectors.toMap(identity(), identity()));

programs.stream()
.flatMap(
program ->
getProgramAttributes(List.of(program))
.map(IdentifiableObject::getUid)
// We need fully qualified dimension identifiers.
.map(attributeUid -> Pair.of(program, attributeUid)))
.forEach(
fullyQualifiedDimension ->
dimensionsByUid.put(
fullyQualifiedDimension.getRight(),
fullyQualifiedDimension.getLeft().getUid()
+ "."
+ fullyQualifiedDimension.getRight()));

// Removes all items already existing for which exists a fully qualified dimension name.
request.getDimension().removeIf(dimensionsByUid::containsKey);

// Adds all dimensions from all programs.
request.getDimension().addAll(dimensionsByUid.values());
// Adds all program attributes from all applicable programs as dimensions
request
.getDimension()
.addAll(
getProgramAttributes(programs)
.map(IdentifiableObject::getUid)
.collect(Collectors.toSet()));

return CommonParams.builder()
.programs(programs)
Expand Down Expand Up @@ -199,7 +180,7 @@ private List<AnalyticsSortingParams> getSortingParams(
SORTING.getUidsGetter().apply(request).stream(),
(sortRequest, index) ->
toSortParam(index, sortRequest, request, programs, userOrgUnits))
.collect(toList());
.toList();
}

/**
Expand Down Expand Up @@ -240,12 +221,12 @@ private List<Program> getPrograms(CommonQueryRequest queryRequest) {
boolean programsCouldNotBeRetrieved = programs.size() != queryRequest.getProgram().size();

if (programsCouldNotBeRetrieved) {
List<String> foundProgramUids = programs.stream().map(Program::getUid).collect(toList());
List<String> foundProgramUids = programs.stream().map(Program::getUid).toList();

List<String> missingProgramUids =
Optional.of(queryRequest).map(CommonQueryRequest::getProgram).orElse(emptySet()).stream()
.filter(uidFromRequest -> !foundProgramUids.contains(uidFromRequest))
.collect(toList());
.toList();

throw new IllegalQueryException(E7129, missingProgramUids);
}
Expand Down Expand Up @@ -276,19 +257,14 @@ private List<DimensionIdentifier<DimensionParam>> retrieveDimensionParams(
dimensionParamType.getUidsGetter().apply(queryRequest);

dimensionParams.addAll(
unmodifiableList(
dimensionsOrFilter.stream()
.map(CommonQueryRequestMapper::splitOnOrIfNecessary)
.map(
dof ->
toDimensionIdentifier(
dof,
dimensionParamType,
queryRequest,
programs,
userOrgUnits))
.flatMap(Collection::stream)
.collect(toList())));
dimensionsOrFilter.stream()
.map(CommonQueryRequestMapper::splitOnOrIfNecessary)
.map(
dof ->
toDimensionIdentifier(
dof, dimensionParamType, queryRequest, programs, userOrgUnits))
.flatMap(Collection::stream)
.toList());
});

return unmodifiableList(dimensionParams);
Expand Down Expand Up @@ -320,7 +296,7 @@ private List<DimensionIdentifier<DimensionParam>> toDimensionIdentifier(
toDimensionIdentifier(
dimensionAsString, dimensionParamType, queryRequest, programs, userOrgUnits))
.map(dimensionIdentifier -> dimensionIdentifier.withGroupId(groupId))
.collect(toList());
.toList();
}

/**
Expand All @@ -330,7 +306,7 @@ private List<DimensionIdentifier<DimensionParam>> toDimensionIdentifier(
* @return the {@link List} of String.
*/
private static List<String> splitOnOrIfNecessary(String dimensionAsString) {
return Arrays.stream(DIMENSION_OR_SEPARATOR.split(dimensionAsString)).collect(toList());
return Arrays.stream(DIMENSION_OR_SEPARATOR.split(dimensionAsString)).toList();
}

/**
Expand Down Expand Up @@ -409,24 +385,38 @@ public DimensionIdentifier<DimensionParam> toDimensionIdentifier(
dimensionIdentifier.getProgram(), dimensionIdentifier.getProgramStage(), dimensionParam);
}

// If we reach here, it should be a queryItem. Objects of type queryItem
// need to be prefixed by programUid (program attributes, program
// indicators) and optionally by a programStageUid (Data Element).
if (dimensionIdentifier.hasProgram()) {
QueryItem queryItem =
QueryItem queryItem;

if (!dimensionIdentifier.hasProgram() && !dimensionIdentifier.hasProgramStage()) {
// If we reach here, it should be a trackedEntityAttribute.
queryItem =
eventDataQueryService.getQueryItem(
dimensionIdentifier.getDimension().getUid(), null, TRACKED_ENTITY_INSTANCE);

if (Objects.isNull(queryItem)) {
throw new IllegalQueryException(E7250, dimensionId);
}

} else {
// If we reach here, it should be a queryItem. In this case it can be either
// a program indicator (with programUid prefix) or a Data Element
// (both program and program stage prefixes)
queryItem =
eventDataQueryService.getQueryItem(
dimensionIdentifier.getDimension().getUid(),
dimensionIdentifier.getProgram().getElement(),
TRACKED_ENTITY_INSTANCE);

// The fully qualified dimension identification is required here.
DimensionParam dimensionParam = DimensionParam.ofObject(queryItem, dimensionParamType, items);

return DimensionIdentifier.of(
dimensionIdentifier.getProgram(), dimensionIdentifier.getProgramStage(), dimensionParam);
// TEA should only be specified without program prefix
if (queryItem.getItem() instanceof TrackedEntityAttribute) {
throw new IllegalQueryException(E7250, dimensionId);
}
}

throw new IllegalQueryException(E7250, dimensionId);
return DimensionIdentifier.of(
dimensionIdentifier.getProgram(),
dimensionIdentifier.getProgramStage(),
DimensionParam.ofObject(queryItem, dimensionParamType, items));
}

private static DimensionIdentifier<DimensionParam> parseAsStaticDimension(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
*/
package org.hisp.dhis.analytics.event;

import java.util.Optional;
import org.hisp.dhis.analytics.EventOutputType;
import org.hisp.dhis.common.QueryItem;
import org.hisp.dhis.program.Program;
Expand All @@ -49,12 +50,22 @@ public interface QueryItemLocator {
* uid}] - Relationship Type + Program Indicator [{rt uid}.{pi uid}]
*
* <p>If the provided dimension String is not matching any of the above elements, then a {@link
* IllegalQueryException} is thrown
* org.hisp.dhis.common.IllegalQueryException} is thrown
*
* @param dimension the dimension string.
* @param program the {@link Program}.
* @param type the {@link EventOutputType}.
* @return a {@link QueryItem}.
*/
QueryItem getQueryItemFromDimension(String dimension, Program program, EventOutputType type);

/**
* Same as {@link #getQueryItemFromDimension(String, Program, EventOutputType)} but without
* program. Used by Cross program linelisting application to get the query item for the tracked
* entity attribute.
*
* @param dimension the dimension string representing a tracked entity attribute uid.
* @return an Optional {@link QueryItem}. Empty if not found.
*/
Optional<QueryItem> getQueryItemForTrackedEntityAttribute(String dimension);
}
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@
import static org.hisp.dhis.analytics.event.data.DefaultEventCoordinateService.COL_NAME_TEI_GEOMETRY;
import static org.hisp.dhis.analytics.event.data.DefaultEventDataQueryService.SortableItems.isSortable;
import static org.hisp.dhis.analytics.event.data.DefaultEventDataQueryService.SortableItems.translateItemIfNecessary;
import static org.hisp.dhis.analytics.util.AnalyticsUtils.illegalQueryExSupplier;
import static org.hisp.dhis.analytics.util.AnalyticsUtils.throwIllegalQueryEx;
import static org.hisp.dhis.common.DimensionalObject.DIMENSION_NAME_SEP;
import static org.hisp.dhis.common.DimensionalObject.PERIOD_DIM_ID;
Expand Down Expand Up @@ -515,7 +516,16 @@ public QueryItem getQueryItem(String dimensionString, Program program, EventOutp
throwIllegalQueryEx(ErrorCode.E7222, dimensionString);
}

QueryItem queryItem = queryItemLocator.getQueryItemFromDimension(split[0], program, type);
QueryItem queryItem;
if (Objects.isNull(program)) {
// support for querying program attributes by uid without passing the program
queryItem =
queryItemLocator
.getQueryItemForTrackedEntityAttribute(split[0])
.orElseThrow(illegalQueryExSupplier(ErrorCode.E7224, dimensionString));
} else {
queryItem = queryItemLocator.getQueryItemFromDimension(split[0], program, type);
}

if (split.length > 1) // Filters specified
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,11 @@
package org.hisp.dhis.analytics.event.data;

import static com.google.common.base.Preconditions.checkNotNull;
import static org.hisp.dhis.analytics.util.AnalyticsUtils.illegalQueryExSupplier;
import static org.hisp.dhis.analytics.util.AnalyticsUtils.throwIllegalQueryEx;
import static org.hisp.dhis.common.DimensionalObject.DIMENSION_IDENTIFIER_SEP;
import static org.hisp.dhis.common.DimensionalObject.ITEM_SEP;
import static org.hisp.dhis.feedback.ErrorCode.E7224;

import java.util.Collections;
import java.util.Date;
Expand Down Expand Up @@ -109,10 +111,7 @@ public QueryItem getQueryItemFromDimension(
() ->
getDynamicDimension(dimension)
.orElseThrow(
() ->
new IllegalQueryException(
new ErrorMessage(
ErrorCode.E7224, dimension))))));
illegalQueryExSupplier(E7224, dimension)))));
}

/**
Expand Down Expand Up @@ -217,6 +216,19 @@ private Optional<QueryItem> getTrackedEntityAttribute(
return Optional.ofNullable(qi);
}

/**
* Returns a QueryItem for a TrackedEntityAttribute
*
* @param dimension the uid of the TrackedEntityAttribute
* @return a QueryItem for a TrackedEntityAttribute
*/
@Override
public Optional<QueryItem> getQueryItemForTrackedEntityAttribute(String dimension) {
return Optional.ofNullable(dimension)
.map(attributeService::getTrackedEntityAttribute)
.map(attribute -> new QueryItem(attribute, getLegendSet(dimension)));
}

private Optional<QueryItem> getProgramIndicator(
String dimension, Program program, LegendSet legendSet) {
QueryItem qi = null;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@
import static java.lang.String.join;
import static java.util.Arrays.stream;
import static java.util.stream.Collectors.joining;
import static java.util.stream.Collectors.toSet;
import static lombok.AccessLevel.PRIVATE;
import static org.apache.commons.lang3.StringUtils.EMPTY;
import static org.hisp.dhis.analytics.tei.query.context.QueryContextConstants.TEI_ALIAS;
Expand Down Expand Up @@ -58,7 +57,6 @@
import org.hisp.dhis.analytics.tei.query.context.TeiStaticField;
import org.hisp.dhis.common.DimensionalObject;
import org.hisp.dhis.common.GridHeader;
import org.hisp.dhis.common.IdentifiableObject;
import org.hisp.dhis.common.QueryItem;
import org.hisp.dhis.common.RepeatableStageParams;
import org.hisp.dhis.common.ValueType;
Expand All @@ -79,44 +77,23 @@ public class TeiFields {
private static final String ENROLLMENT_COLUMN_PREFIX = "Enrollment";

/**
* Retrieves all object attributes from the given param encapsulating them into a stream of {@link
* Retrieves all TEAs attributes from the given param encapsulating them into a stream of {@link
* Field}.
*
* @param teiQueryParams the {@link TeiQueryParams}.
* @return a {@link Stream} of {@link Field}.
*/
public static Stream<Field> getDimensionFields(TeiQueryParams teiQueryParams) {
Set<String> programAttributesUids =
teiQueryParams.getCommonParams().getPrograms().stream()
.map(Program::getProgramAttributes)
.flatMap(List::stream)
.map(programAttr -> programAttr.getAttribute().getUid())
.collect(toSet());

Stream<Field> programAttributes =
teiQueryParams.getCommonParams().getPrograms().stream()
.map(Program::getProgramAttributes)
.flatMap(List::stream)
.map(
programAttr ->
Field.of(
TEI_ALIAS,
() -> programAttr.getAttribute().getUid(),
join(
".",
programAttr.getProgram().getUid(),
programAttr.getAttribute().getUid())));

Stream<Field> trackedEntityAttributesFromType =
getTrackedEntityAttributes(teiQueryParams.getTrackedEntityType())
.filter(
programTrackedEntityAttribute ->
!programAttributesUids.contains(programTrackedEntityAttribute.getUid()))
.map(IdentifiableObject::getUid)
.map(attr -> Field.of(TEI_ALIAS, () -> attr, attr));

// TET and program attribute uids.
return Stream.concat(trackedEntityAttributesFromType, programAttributes);
return Stream.concat(
teiQueryParams.getCommonParams().getPrograms().stream()
.map(Program::getProgramAttributes)
.flatMap(List::stream)
.map(ProgramTrackedEntityAttribute::getAttribute)
.map(TrackedEntityAttribute::getUid),
teiQueryParams.getTrackedEntityType().getTrackedEntityAttributes().stream()
.map(TrackedEntityAttribute::getUid))
.distinct()
.map(attr -> Field.of(TEI_ALIAS, () -> attr, attr));
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ private static boolean isTei(DimensionIdentifier<DimensionParam> dimensionIdenti
dimensionIdentifier.getDimensionIdentifierType() == TEI
||
// Will match all dimensionIdentifiers whose type is PROGRAM_ATTRIBUTE.
// e.g. {programUid}.{attributeUid}
// e.g. {attributeUid}
isOfType(dimensionIdentifier, PROGRAM_ATTRIBUTE);
}

Expand Down
Loading

0 comments on commit 1253d7a

Please sign in to comment.