From bf3bb25736a566051dd08fd5f8f86960090b989f Mon Sep 17 00:00:00 2001 From: Giuseppe Nespolino Date: Thu, 2 Nov 2023 14:58:13 +0100 Subject: [PATCH 1/5] feat: no programUid prefix in dimension endpoint [DHIS2-16067] --- .../tei/TeiAnalyticsController.java | 4 +- .../dimension/DimensionMapperService.java | 22 ++++- .../dimension/TeiAnalyticsPrefixStrategy.java | 43 -------- .../BaseDimensionalItemObjectMapper.java | 5 +- .../dimension/DimensionMapperServiceTest.java | 99 +++++++++++++++++++ 5 files changed, 121 insertions(+), 52 deletions(-) delete mode 100644 dhis-2/dhis-web-api/src/main/java/org/hisp/dhis/webapi/dimension/TeiAnalyticsPrefixStrategy.java create mode 100644 dhis-2/dhis-web-api/src/test/java/org/hisp/dhis/webapi/dimension/DimensionMapperServiceTest.java diff --git a/dhis-2/dhis-web-api/src/main/java/org/hisp/dhis/webapi/controller/tei/TeiAnalyticsController.java b/dhis-2/dhis-web-api/src/main/java/org/hisp/dhis/webapi/controller/tei/TeiAnalyticsController.java index 5f5734371f84..e161f1f8f2f6 100644 --- a/dhis-2/dhis-web-api/src/main/java/org/hisp/dhis/webapi/controller/tei/TeiAnalyticsController.java +++ b/dhis-2/dhis-web-api/src/main/java/org/hisp/dhis/webapi/controller/tei/TeiAnalyticsController.java @@ -65,7 +65,7 @@ import org.hisp.dhis.common.OpenApi; import org.hisp.dhis.webapi.dimension.DimensionFilteringAndPagingService; import org.hisp.dhis.webapi.dimension.DimensionMapperService; -import org.hisp.dhis.webapi.dimension.TeiAnalyticsPrefixStrategy; +import org.hisp.dhis.webapi.dimension.EnrollmentAnalyticsPrefixStrategy; import org.hisp.dhis.webapi.mvc.annotation.ApiVersion; import org.hisp.dhis.webapi.utils.ContextUtils; import org.springframework.security.access.prepost.PreAuthorize; @@ -257,7 +257,7 @@ public AnalyticsDimensionsPagingWrapper getQueryDimensions( dimensionMapperService.toDimensionResponse( teiAnalyticsDimensionsService.getQueryDimensionsByTrackedEntityTypeId( trackedEntityType, program), - TeiAnalyticsPrefixStrategy.INSTANCE), + EnrollmentAnalyticsPrefixStrategy.INSTANCE), dimensionsCriteria, fields); } diff --git a/dhis-2/dhis-web-api/src/main/java/org/hisp/dhis/webapi/dimension/DimensionMapperService.java b/dhis-2/dhis-web-api/src/main/java/org/hisp/dhis/webapi/dimension/DimensionMapperService.java index a24b00ce8825..f625fc9f5c2f 100644 --- a/dhis-2/dhis-web-api/src/main/java/org/hisp/dhis/webapi/dimension/DimensionMapperService.java +++ b/dhis-2/dhis-web-api/src/main/java/org/hisp/dhis/webapi/dimension/DimensionMapperService.java @@ -32,6 +32,10 @@ import java.util.Collection; import java.util.List; +import java.util.Set; +import java.util.concurrent.ConcurrentHashMap; +import java.util.function.Function; +import java.util.function.Predicate; import lombok.RequiredArgsConstructor; import org.hisp.dhis.common.PrefixedDimension; import org.springframework.stereotype.Service; @@ -43,9 +47,19 @@ public class DimensionMapperService { public List toDimensionResponse( Collection dimensions, PrefixStrategy prefixStrategy) { - return mapToList( - dimensions, - pDimension -> toDimensionResponse(pDimension, prefixStrategy.apply(pDimension))); + return distinctByUid( + mapToList( + dimensions, + pDimension -> toDimensionResponse(pDimension, prefixStrategy.apply(pDimension)))); + } + + private List distinctByUid(List dimensionResponses) { + return dimensionResponses.stream().filter(distinctBy(DimensionResponse::getUid)).toList(); + } + + private static Predicate distinctBy(Function keyExtractor) { + Set seen = ConcurrentHashMap.newKeySet(); + return t -> seen.add(keyExtractor.apply(t)); } private DimensionResponse toDimensionResponse(PrefixedDimension dimension, String prefix) { @@ -56,6 +70,6 @@ private DimensionResponse toDimensionResponse(PrefixedDimension dimension, Strin .orElseThrow( () -> new IllegalArgumentException( - "Unsupported dimension type: " + getRealClass(dimension))); + "Unsupported dimension type: " + getRealClass(dimension.getItem()))); } } diff --git a/dhis-2/dhis-web-api/src/main/java/org/hisp/dhis/webapi/dimension/TeiAnalyticsPrefixStrategy.java b/dhis-2/dhis-web-api/src/main/java/org/hisp/dhis/webapi/dimension/TeiAnalyticsPrefixStrategy.java deleted file mode 100644 index b0185245a112..000000000000 --- a/dhis-2/dhis-web-api/src/main/java/org/hisp/dhis/webapi/dimension/TeiAnalyticsPrefixStrategy.java +++ /dev/null @@ -1,43 +0,0 @@ -/* - * 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.dimension; - -import lombok.AccessLevel; -import lombok.NoArgsConstructor; -import org.hisp.dhis.common.PrefixedDimension; - -@NoArgsConstructor(access = AccessLevel.PRIVATE) -public class TeiAnalyticsPrefixStrategy implements PrefixStrategy { - - public static final TeiAnalyticsPrefixStrategy INSTANCE = new TeiAnalyticsPrefixStrategy(); - - @Override - public String apply(PrefixedDimension pDimension) { - return pDimension.getPrefix(); - } -} diff --git a/dhis-2/dhis-web-api/src/main/java/org/hisp/dhis/webapi/dimension/mappers/BaseDimensionalItemObjectMapper.java b/dhis-2/dhis-web-api/src/main/java/org/hisp/dhis/webapi/dimension/mappers/BaseDimensionalItemObjectMapper.java index 598a88d9378e..50876892c0ce 100644 --- a/dhis-2/dhis-web-api/src/main/java/org/hisp/dhis/webapi/dimension/mappers/BaseDimensionalItemObjectMapper.java +++ b/dhis-2/dhis-web-api/src/main/java/org/hisp/dhis/webapi/dimension/mappers/BaseDimensionalItemObjectMapper.java @@ -59,9 +59,8 @@ public DimensionResponse map(PrefixedDimension prefixedDimension, String prefix) DimensionResponse responseWithDimensionType = super.map(prefixedDimension, prefix) .withDimensionType(dimensionTypeOrElse(prefixedDimension, dimensionItemType.name())); - if (prefixedDimension.getItem() instanceof ValueTypedDimensionalItemObject) { - ValueTypedDimensionalItemObject valueTypedDimensionalItemObject = - (ValueTypedDimensionalItemObject) prefixedDimension.getItem(); + if (prefixedDimension.getItem() + instanceof ValueTypedDimensionalItemObject valueTypedDimensionalItemObject) { return responseWithDimensionType .withValueType(valueTypedDimensionalItemObject.getValueType().name()) .withOptionSet( diff --git a/dhis-2/dhis-web-api/src/test/java/org/hisp/dhis/webapi/dimension/DimensionMapperServiceTest.java b/dhis-2/dhis-web-api/src/test/java/org/hisp/dhis/webapi/dimension/DimensionMapperServiceTest.java new file mode 100644 index 000000000000..755964c4a299 --- /dev/null +++ b/dhis-2/dhis-web-api/src/test/java/org/hisp/dhis/webapi/dimension/DimensionMapperServiceTest.java @@ -0,0 +1,99 @@ +/* + * 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.dimension; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +import java.util.Collection; +import java.util.Collections; +import java.util.List; +import java.util.stream.Stream; +import org.hisp.dhis.common.BaseIdentifiableObject; +import org.hisp.dhis.common.DimensionType; +import org.hisp.dhis.common.PrefixedDimension; +import org.hisp.dhis.trackedentity.TrackedEntityAttribute; +import org.hisp.dhis.webapi.dimension.mappers.BaseDimensionalItemObjectMapper; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +class DimensionMapperServiceTest { + + private DimensionMapperService dimensionMapperService; + private BaseDimensionMapper baseDimensionMapper; + + @BeforeEach + void setUp() { + baseDimensionMapper = mock(BaseDimensionalItemObjectMapper.class); + dimensionMapperService = new DimensionMapperService(Collections.singleton(baseDimensionMapper)); + } + + @Test + void testReturnedDimensionsHaveNoDuplicates() { + + when(baseDimensionMapper.map(any(), any())) + .thenAnswer( + invocation -> { + PrefixedDimension prefixedDimension = invocation.getArgument(0); + return DimensionResponse.builder().uid(prefixedDimension.getItem().getUid()).build(); + }); + when(baseDimensionMapper.supports(any())).thenReturn(true); + + List dimensionResponse = + dimensionMapperService.toDimensionResponse( + mockDimensions(), EnrollmentAnalyticsPrefixStrategy.INSTANCE); + + assertEquals(4, dimensionResponse.size()); + + Collection dimensionResponseUids = + dimensionResponse.stream().map(DimensionResponse::getUid).toList(); + + assertEquals(List.of("uid1", "uid2", "uid3", "repeated"), dimensionResponseUids); + } + + private Collection mockDimensions() { + return Stream.of("uid1", "uid2", "uid3", "repeated", "repeated") + .map(this::asPrefixedDimension) + .toList(); + } + + private PrefixedDimension asPrefixedDimension(String dimension) { + return PrefixedDimension.builder() + .item(buildItem(dimension)) + .dimensionType(DimensionType.PROGRAM_ATTRIBUTE.name()) + .build(); + } + + private BaseIdentifiableObject buildItem(String uid) { + TrackedEntityAttribute item = new TrackedEntityAttribute(); + item.setUid(uid); + return item; + } +} From 2d3aaf93221b73f4eaaa444a7ed239a725cd5fd8 Mon Sep 17 00:00:00 2001 From: Giuseppe Nespolino Date: Thu, 2 Nov 2023 18:50:21 +0100 Subject: [PATCH 2/5] feat: program prefix is not required for TEAs in CPL endpoint params [DHIS2-16067] --- .../java/org/hisp/dhis/common/QueryItem.java | 10 + .../processing/CommonQueryRequestMapper.java | 100 ++++---- .../analytics/event/QueryItemLocator.java | 13 +- .../data/DefaultEventDataQueryService.java | 12 +- .../event/data/DefaultQueryItemLocator.java | 20 +- .../dhis/analytics/tei/query/TeiFields.java | 45 +--- .../context/querybuilder/TeiQueryBuilder.java | 2 +- .../dhis/analytics/util/AnalyticsUtils.java | 16 +- .../analytics/tei/TrackedEntityQueryTest.java | 233 ++++-------------- 9 files changed, 174 insertions(+), 277 deletions(-) diff --git a/dhis-2/dhis-api/src/main/java/org/hisp/dhis/common/QueryItem.java b/dhis-2/dhis-api/src/main/java/org/hisp/dhis/common/QueryItem.java index 62b9f4036f24..39d0e28be97a 100644 --- a/dhis-2/dhis-api/src/main/java/org/hisp/dhis/common/QueryItem.java +++ b/dhis-2/dhis-api/src/main/java/org/hisp/dhis/common/QueryItem.java @@ -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; } diff --git a/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/common/processing/CommonQueryRequestMapper.java b/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/common/processing/CommonQueryRequestMapper.java index 69c89a940579..c238510fc22d 100644 --- a/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/common/processing/CommonQueryRequestMapper.java +++ b/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/common/processing/CommonQueryRequestMapper.java @@ -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; @@ -54,7 +52,6 @@ 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; @@ -62,7 +59,6 @@ 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; @@ -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; @@ -111,29 +108,13 @@ public CommonParams map(CommonQueryRequest request) { dataQueryService.getUserOrgUnits(null, request.getUserOrgUnit()); List programs = getPrograms(request); - Map 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) @@ -199,7 +180,7 @@ private List getSortingParams( SORTING.getUidsGetter().apply(request).stream(), (sortRequest, index) -> toSortParam(index, sortRequest, request, programs, userOrgUnits)) - .collect(toList()); + .toList(); } /** @@ -240,12 +221,12 @@ private List getPrograms(CommonQueryRequest queryRequest) { boolean programsCouldNotBeRetrieved = programs.size() != queryRequest.getProgram().size(); if (programsCouldNotBeRetrieved) { - List foundProgramUids = programs.stream().map(Program::getUid).collect(toList()); + List foundProgramUids = programs.stream().map(Program::getUid).toList(); List missingProgramUids = Optional.of(queryRequest).map(CommonQueryRequest::getProgram).orElse(emptySet()).stream() .filter(uidFromRequest -> !foundProgramUids.contains(uidFromRequest)) - .collect(toList()); + .toList(); throw new IllegalQueryException(E7129, missingProgramUids); } @@ -276,19 +257,14 @@ private List> 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); @@ -320,7 +296,7 @@ private List> toDimensionIdentifier( toDimensionIdentifier( dimensionAsString, dimensionParamType, queryRequest, programs, userOrgUnits)) .map(dimensionIdentifier -> dimensionIdentifier.withGroupId(groupId)) - .collect(toList()); + .toList(); } /** @@ -330,7 +306,7 @@ private List> toDimensionIdentifier( * @return the {@link List} of String. */ private static List splitOnOrIfNecessary(String dimensionAsString) { - return Arrays.stream(DIMENSION_OR_SEPARATOR.split(dimensionAsString)).collect(toList()); + return Arrays.stream(DIMENSION_OR_SEPARATOR.split(dimensionAsString)).toList(); } /** @@ -409,21 +385,39 @@ public DimensionIdentifier 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); + // TEA should only be specified without program prefix + if (queryItem.getItem() instanceof TrackedEntityAttribute) { + throw new IllegalQueryException(E7250, dimensionId); + } + } + if (Objects.nonNull(queryItem)) { return DimensionIdentifier.of( - dimensionIdentifier.getProgram(), dimensionIdentifier.getProgramStage(), dimensionParam); + dimensionIdentifier.getProgram(), + dimensionIdentifier.getProgramStage(), + DimensionParam.ofObject(queryItem, dimensionParamType, items)); } throw new IllegalQueryException(E7250, dimensionId); diff --git a/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/event/QueryItemLocator.java b/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/event/QueryItemLocator.java index b34f871da659..6f70d674aebe 100644 --- a/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/event/QueryItemLocator.java +++ b/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/event/QueryItemLocator.java @@ -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; @@ -49,7 +50,7 @@ public interface QueryItemLocator { * uid}] - Relationship Type + Program Indicator [{rt uid}.{pi uid}] * *

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}. @@ -57,4 +58,14 @@ public interface QueryItemLocator { * @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 getQueryItemForTrackedEntityAttribute(String dimension); } diff --git a/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/event/data/DefaultEventDataQueryService.java b/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/event/data/DefaultEventDataQueryService.java index c6926134e144..27fd03bcfe69 100644 --- a/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/event/data/DefaultEventDataQueryService.java +++ b/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/event/data/DefaultEventDataQueryService.java @@ -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; @@ -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 { diff --git a/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/event/data/DefaultQueryItemLocator.java b/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/event/data/DefaultQueryItemLocator.java index a2322bd9aa27..b4872263141e 100644 --- a/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/event/data/DefaultQueryItemLocator.java +++ b/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/event/data/DefaultQueryItemLocator.java @@ -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; @@ -109,10 +111,7 @@ public QueryItem getQueryItemFromDimension( () -> getDynamicDimension(dimension) .orElseThrow( - () -> - new IllegalQueryException( - new ErrorMessage( - ErrorCode.E7224, dimension)))))); + illegalQueryExSupplier(E7224, dimension))))); } /** @@ -217,6 +216,19 @@ private Optional 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 getQueryItemForTrackedEntityAttribute(String dimension) { + return Optional.ofNullable(dimension) + .map(attributeService::getTrackedEntityAttribute) + .map(attribute -> new QueryItem(attribute, getLegendSet(dimension))); + } + private Optional getProgramIndicator( String dimension, Program program, LegendSet legendSet) { QueryItem qi = null; diff --git a/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/tei/query/TeiFields.java b/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/tei/query/TeiFields.java index 1ea7ddfdd7b7..20a02dd453ea 100644 --- a/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/tei/query/TeiFields.java +++ b/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/tei/query/TeiFields.java @@ -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; @@ -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; @@ -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 getDimensionFields(TeiQueryParams teiQueryParams) { - Set programAttributesUids = - teiQueryParams.getCommonParams().getPrograms().stream() - .map(Program::getProgramAttributes) - .flatMap(List::stream) - .map(programAttr -> programAttr.getAttribute().getUid()) - .collect(toSet()); - - Stream 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 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)); } /** diff --git a/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/tei/query/context/querybuilder/TeiQueryBuilder.java b/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/tei/query/context/querybuilder/TeiQueryBuilder.java index 4f2e7d42688b..94b085ad7ee4 100644 --- a/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/tei/query/context/querybuilder/TeiQueryBuilder.java +++ b/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/tei/query/context/querybuilder/TeiQueryBuilder.java @@ -121,7 +121,7 @@ private static boolean isTei(DimensionIdentifier dimensionIdenti dimensionIdentifier.getDimensionIdentifierType() == TEI || // Will match all dimensionIdentifiers whose type is PROGRAM_ATTRIBUTE. - // e.g. {programUid}.{attributeUid} + // e.g. {attributeUid} isOfType(dimensionIdentifier, PROGRAM_ATTRIBUTE); } diff --git a/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/util/AnalyticsUtils.java b/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/util/AnalyticsUtils.java index b32a35e43505..7a02d97d90b9 100644 --- a/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/util/AnalyticsUtils.java +++ b/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/util/AnalyticsUtils.java @@ -1011,7 +1011,21 @@ public static Double getBaseMonth(PeriodType periodType) { * @param args the arguments to provide to the error code message. */ public static void throwIllegalQueryEx(ErrorCode errorCode, Object... args) { - throw new IllegalQueryException(new ErrorMessage(errorCode, args)); + throw illegalQueryExSupplier(errorCode, args).get(); + } + + /** + * Returns a {@link Supplier} of {@link IllegalQueryException} using the given {@link ErrorCode}. + * The supplier can be used to throw an {@link IllegalQueryException} at a later point in time. + * This is useful when the exception should be thrown in a lambda expression. + * + * @param errorCode the {@link ErrorCode}. + * @param args the arguments to provide to the error code message. + * @return a {@link Supplier} of {@link IllegalQueryException}. + */ + public static Supplier illegalQueryExSupplier( + ErrorCode errorCode, Object... args) { + return () -> new IllegalQueryException(new ErrorMessage(errorCode, args)); } /** diff --git a/dhis-2/dhis-test-e2e/src/test/java/org/hisp/dhis/analytics/tei/TrackedEntityQueryTest.java b/dhis-2/dhis-test-e2e/src/test/java/org/hisp/dhis/analytics/tei/TrackedEntityQueryTest.java index bbe2006ee24d..1aec4a759b5e 100644 --- a/dhis-2/dhis-test-e2e/src/test/java/org/hisp/dhis/analytics/tei/TrackedEntityQueryTest.java +++ b/dhis-2/dhis-test-e2e/src/test/java/org/hisp/dhis/analytics/tei/TrackedEntityQueryTest.java @@ -64,10 +64,10 @@ private QueryParamsBuilder withDefaultHeaders(QueryParamsBuilder queryParamsBuil + "ouname," + "oucode," + "ounamehierarchy," - + "IpHINAT79UW.w75KJ2mc4zz," - + "IpHINAT79UW.zDhUuAYrxNC," - + "IpHINAT79UW.cejWyOfXge6," - + "IpHINAT79UW.lZGmxYbs97q"); + + "w75KJ2mc4zz," + + "zDhUuAYrxNC," + + "cejWyOfXge6," + + "lZGmxYbs97q"); } @Test @@ -77,7 +77,7 @@ void queryWithProgramAndProgramStageWhenTotalPagesIsFalse() { new QueryParamsBuilder() .add("dimension=ou:ImspTQPwCqd") .add("program=IpHINAT79UW") - .add("asc=IpHINAT79UW.w75KJ2mc4zz") + .add("asc=w75KJ2mc4zz") .add("lastUpdated=LAST_YEAR") .add("totalPages=false") .add("pageSize=100") @@ -168,34 +168,12 @@ void queryWithProgramAndProgramStageWhenTotalPagesIsFalse() { false, true); validateHeader( - response, - 10, - "IpHINAT79UW.w75KJ2mc4zz", - "First name", - "TEXT", - "java.lang.String", - false, - true); + response, 10, "w75KJ2mc4zz", "First name", "TEXT", "java.lang.String", false, true); validateHeader( - response, - 11, - "IpHINAT79UW.zDhUuAYrxNC", - "Last name", - "TEXT", - "java.lang.String", - false, - true); - validateHeader( - response, 12, "IpHINAT79UW.cejWyOfXge6", "Gender", "TEXT", "java.lang.String", false, true); + response, 11, "zDhUuAYrxNC", "Last name", "TEXT", "java.lang.String", false, true); + validateHeader(response, 12, "cejWyOfXge6", "Gender", "TEXT", "java.lang.String", false, true); validateHeader( - response, - 13, - "IpHINAT79UW.lZGmxYbs97q", - "Unique ID", - "TEXT", - "java.lang.String", - false, - true); + response, 13, "lZGmxYbs97q", "Unique ID", "TEXT", "java.lang.String", false, true); } @Test @@ -291,34 +269,12 @@ void queryWithProgramOnly() { false, true); validateHeader( - response, - 10, - "IpHINAT79UW.w75KJ2mc4zz", - "First name", - "TEXT", - "java.lang.String", - false, - true); + response, 10, "w75KJ2mc4zz", "First name", "TEXT", "java.lang.String", false, true); validateHeader( - response, - 11, - "IpHINAT79UW.zDhUuAYrxNC", - "Last name", - "TEXT", - "java.lang.String", - false, - true); - validateHeader( - response, 12, "IpHINAT79UW.cejWyOfXge6", "Gender", "TEXT", "java.lang.String", false, true); + response, 11, "zDhUuAYrxNC", "Last name", "TEXT", "java.lang.String", false, true); + validateHeader(response, 12, "cejWyOfXge6", "Gender", "TEXT", "java.lang.String", false, true); validateHeader( - response, - 13, - "IpHINAT79UW.lZGmxYbs97q", - "Unique ID", - "TEXT", - "java.lang.String", - false, - true); + response, 13, "lZGmxYbs97q", "Unique ID", "TEXT", "java.lang.String", false, true); // Validate the first three rows, as samples. validateRow( @@ -474,34 +430,12 @@ void queryWithProgramAndPagination() { false, true); validateHeader( - response, - 10, - "IpHINAT79UW.w75KJ2mc4zz", - "First name", - "TEXT", - "java.lang.String", - false, - true); - validateHeader( - response, - 11, - "IpHINAT79UW.zDhUuAYrxNC", - "Last name", - "TEXT", - "java.lang.String", - false, - true); + response, 10, "w75KJ2mc4zz", "First name", "TEXT", "java.lang.String", false, true); validateHeader( - response, 12, "IpHINAT79UW.cejWyOfXge6", "Gender", "TEXT", "java.lang.String", false, true); + response, 11, "zDhUuAYrxNC", "Last name", "TEXT", "java.lang.String", false, true); + validateHeader(response, 12, "cejWyOfXge6", "Gender", "TEXT", "java.lang.String", false, true); validateHeader( - response, - 13, - "IpHINAT79UW.lZGmxYbs97q", - "Unique ID", - "TEXT", - "java.lang.String", - false, - true); + response, 13, "lZGmxYbs97q", "Unique ID", "TEXT", "java.lang.String", false, true); // Validate the first three rows, as samples. validateRow( @@ -575,7 +509,7 @@ public void queryWithProgramAndManyParams() { .add("dimension=cejWyOfXge6") .add("lastUpdated=LAST_10_YEARS") .add( - "headers=ouname,IpHINAT79UW.cejWyOfXge6,IpHINAT79UW.w75KJ2mc4zz,trackedentityinstanceuid,lastupdated,oucode") + "headers=ouname,cejWyOfXge6,w75KJ2mc4zz,trackedentityinstanceuid,lastupdated,oucode") .add("desc=lastupdated") .add("relativePeriodDate=2022-09-27"); @@ -615,17 +549,9 @@ public void queryWithProgramAndManyParams() { // Validate headers validateHeader( response, 0, "ouname", "Organisation unit name", "TEXT", "java.lang.String", false, true); + validateHeader(response, 1, "cejWyOfXge6", "Gender", "TEXT", "java.lang.String", false, true); validateHeader( - response, 1, "IpHINAT79UW.cejWyOfXge6", "Gender", "TEXT", "java.lang.String", false, true); - validateHeader( - response, - 2, - "IpHINAT79UW.w75KJ2mc4zz", - "First name", - "TEXT", - "java.lang.String", - false, - true); + response, 2, "w75KJ2mc4zz", "First name", "TEXT", "java.lang.String", false, true); validateHeader( response, 3, @@ -683,10 +609,10 @@ public void queryWithProgramDimensionAndFilter() { QueryParamsBuilder params = new QueryParamsBuilder() .add("program=IpHINAT79UW") - .add("dimension=ouname,IpHINAT79UW.w75KJ2mc4zz:eq:James") + .add("dimension=ouname,w75KJ2mc4zz:eq:James") .add("lastUpdated=LAST_10_YEARS") .add("includeMetadataDetails=false") - .add("headers=ouname,IpHINAT79UW.w75KJ2mc4zz,lastupdated") + .add("headers=ouname,w75KJ2mc4zz,lastupdated") .add("asc=lastupdated") .add("relativePeriodDate=2022-09-27"); @@ -727,14 +653,7 @@ public void queryWithProgramDimensionAndFilter() { validateHeader( response, 0, "ouname", "Organisation unit name", "TEXT", "java.lang.String", false, true); validateHeader( - response, - 1, - "IpHINAT79UW.w75KJ2mc4zz", - "First name", - "TEXT", - "java.lang.String", - false, - true); + response, 1, "w75KJ2mc4zz", "First name", "TEXT", "java.lang.String", false, true); validateHeader( response, 2, @@ -805,8 +724,8 @@ public void queryWithProgramAndMultipleDynamicDimOrdering() { new QueryParamsBuilder() .add("program=IpHINAT79UW") .add("lastUpdated=LAST_10_YEARS") - .add("desc=IpHINAT79UW.w75KJ2mc4zz,IpHINAT79UW.zDhUuAYrxNC") - .add("headers=IpHINAT79UW.w75KJ2mc4zz,IpHINAT79UW.zDhUuAYrxNC") + .add("desc=w75KJ2mc4zz,zDhUuAYrxNC") + .add("headers=w75KJ2mc4zz,zDhUuAYrxNC") .add("relativePeriodDate=2022-09-27"); // When @@ -824,23 +743,9 @@ public void queryWithProgramAndMultipleDynamicDimOrdering() { // Validate headers validateHeader( - response, - 0, - "IpHINAT79UW.w75KJ2mc4zz", - "First name", - "TEXT", - "java.lang.String", - false, - true); + response, 0, "w75KJ2mc4zz", "First name", "TEXT", "java.lang.String", false, true); validateHeader( - response, - 1, - "IpHINAT79UW.zDhUuAYrxNC", - "Last name", - "TEXT", - "java.lang.String", - false, - true); + response, 1, "zDhUuAYrxNC", "Last name", "TEXT", "java.lang.String", false, true); // Validate the first three rows, as samples. validateRow(response, 0, List.of("Willie", "Woods")); @@ -858,7 +763,7 @@ public void queryWithProgramAndEnrollmentStaticDimOrdering() { .add("program=IpHINAT79UW") .add("lastUpdated=LAST_10_YEARS") .add("desc=lastupdated,IpHINAT79UW.A03MvHHogjR.ouname") - .add("headers=ouname,IpHINAT79UW.lZGmxYbs97q") + .add("headers=ouname,lZGmxYbs97q") .add("relativePeriodDate=2022-09-27"); // When @@ -878,14 +783,7 @@ public void queryWithProgramAndEnrollmentStaticDimOrdering() { validateHeader( response, 0, "ouname", "Organisation unit name", "TEXT", "java.lang.String", false, true); validateHeader( - response, - 1, - "IpHINAT79UW.lZGmxYbs97q", - "Unique ID", - "TEXT", - "java.lang.String", - false, - true); + response, 1, "lZGmxYbs97q", "Unique ID", "TEXT", "java.lang.String", false, true); // Validate the first three rows, as samples. validateRow(response, 0, List.of("Ngelehun CHC", "")); @@ -903,7 +801,7 @@ public void queryWithProgramAndMultipleEventDimOrdering() { .add("program=IpHINAT79UW") .add("desc=IpHINAT79UW.A03MvHHogjR.UXz7xuGCEhU,IpHINAT79UW.A03MvHHogjR.a3kGcGDCuk6") .add("lastUpdated=LAST_10_YEARS") - .add("headers=ouname,IpHINAT79UW.lZGmxYbs97q") + .add("headers=ouname,lZGmxYbs97q") .add("relativePeriodDate=2022-09-27"); // When @@ -923,14 +821,7 @@ public void queryWithProgramAndMultipleEventDimOrdering() { validateHeader( response, 0, "ouname", "Organisation unit name", "TEXT", "java.lang.String", false, true); validateHeader( - response, - 1, - "IpHINAT79UW.lZGmxYbs97q", - "Unique ID", - "TEXT", - "java.lang.String", - false, - true); + response, 1, "lZGmxYbs97q", "Unique ID", "TEXT", "java.lang.String", false, true); // Validate the first three rows, as samples. validateRow(response, 0, List.of("Ngelehun CHC", "")); @@ -948,9 +839,8 @@ public void queryWithProgramAndProgramIndicatorOrdering() { .add("program=IpHINAT79UW") .add("dimension=IpHINAT79UW.A03MvHHogjR.p2Zxg0wcPQ3") .add("lastUpdated=LAST_10_YEARS") - .add( - "asc=IpHINAT79UW.A03MvHHogjR.p2Zxg0wcPQ3,IpHINAT79UW.zDhUuAYrxNC,IpHINAT79UW.w75KJ2mc4zz") - .add("headers=ouname,IpHINAT79UW.lZGmxYbs97q,IpHINAT79UW.A03MvHHogjR.p2Zxg0wcPQ3") + .add("asc=IpHINAT79UW.A03MvHHogjR.p2Zxg0wcPQ3,zDhUuAYrxNC,w75KJ2mc4zz") + .add("headers=ouname,lZGmxYbs97q,IpHINAT79UW.A03MvHHogjR.p2Zxg0wcPQ3") .add("relativePeriodDate=2022-09-27"); // When @@ -970,14 +860,7 @@ public void queryWithProgramAndProgramIndicatorOrdering() { validateHeader( response, 0, "ouname", "Organisation unit name", "TEXT", "java.lang.String", false, true); validateHeader( - response, - 1, - "IpHINAT79UW.lZGmxYbs97q", - "Unique ID", - "TEXT", - "java.lang.String", - false, - true); + response, 1, "lZGmxYbs97q", "Unique ID", "TEXT", "java.lang.String", false, true); validateHeader( response, 2, @@ -1895,7 +1778,7 @@ public void queryWithProgramAndEnrollmentDateAndNegativeEnrollmentOffset() { .add("enrollmentDate=IpHINAT79UW[-1].LAST_YEAR") .add("desc=lastupdated") .add("relativePeriodDate=2023-04-03") - .add("headers=ouname,IpHINAT79UW.w75KJ2mc4zz,IpHINAT79UW.zDhUuAYrxNC"); + .add("headers=ouname,w75KJ2mc4zz,zDhUuAYrxNC"); // When ApiResponse response = analyticsTeiActions.query().get("nEenWmSyUEp", JSON, JSON, params); @@ -1914,23 +1797,9 @@ public void queryWithProgramAndEnrollmentDateAndNegativeEnrollmentOffset() { validateHeader( response, 0, "ouname", "Organisation unit name", "TEXT", "java.lang.String", false, true); validateHeader( - response, - 1, - "IpHINAT79UW.w75KJ2mc4zz", - "First name", - "TEXT", - "java.lang.String", - false, - true); + response, 1, "w75KJ2mc4zz", "First name", "TEXT", "java.lang.String", false, true); validateHeader( - response, - 2, - "IpHINAT79UW.zDhUuAYrxNC", - "Last name", - "TEXT", - "java.lang.String", - false, - true); + response, 2, "zDhUuAYrxNC", "Last name", "TEXT", "java.lang.String", false, true); // Validate the first three rows, as samples. validateRow(response, 0, List.of("Ngelehun CHC", "John", "Kelly")); @@ -2022,7 +1891,7 @@ public void queryWithProgramAndDimensionFilterUsingIdSchemeCode() { new QueryParamsBuilder() .add("program=IpHINAT79UW") .add("dimension=IpHINAT79UW.ZzYYXq4fJie.cYGaxwK615G:IN:Negative-Conf") - .add("desc=IpHINAT79UW.w75KJ2mc4zz,IpHINAT79UW.zDhUuAYrxNC") + .add("desc=w75KJ2mc4zz,zDhUuAYrxNC") .add("relativePeriodDate=2016-01-01") .add("outputIdScheme=CODE"); @@ -2122,7 +1991,7 @@ public void queryWithProgramAndDimensionFilterUsingIdSchemeName() { new QueryParamsBuilder() .add("program=IpHINAT79UW") .add("dimension=IpHINAT79UW.ZzYYXq4fJie.cYGaxwK615G:IN:Negative-Conf") - .add("desc=IpHINAT79UW.w75KJ2mc4zz,IpHINAT79UW.zDhUuAYrxNC") + .add("desc=w75KJ2mc4zz,zDhUuAYrxNC") .add("relativePeriodDate=2016-01-01") .add("outputIdScheme=NAME"); @@ -2504,8 +2373,8 @@ public void queryWithProgramAndDimensionFilterUsingDataIdSchemeName() { "Ruiz", "Male", "", - "Positive", - "3681")); + "3681", + "Positive")); validateRow( response, @@ -2527,8 +2396,8 @@ public void queryWithProgramAndDimensionFilterUsingDataIdSchemeName() { "Gardner", "Female", "", - "Positive", - "3945")); + "3945", + "Positive")); validateRow( response, @@ -2550,8 +2419,8 @@ public void queryWithProgramAndDimensionFilterUsingDataIdSchemeName() { "Hart", "Female", "", - "Positive", - "3104")); + "3104", + "Positive")); } @Test @@ -2609,8 +2478,8 @@ public void queryWithProgramAndDimensionFilterUsingDataIdSchemeUid() { "Ruiz", "rBvjJYbMCVx", "", - "fWI0UiNZgMy", - "3681")); + "3681", + "fWI0UiNZgMy")); validateRow( response, @@ -2632,8 +2501,8 @@ public void queryWithProgramAndDimensionFilterUsingDataIdSchemeUid() { "Gardner", "Mnp3oXrpAbK", "", - "fWI0UiNZgMy", - "3945")); + "3945", + "fWI0UiNZgMy")); validateRow( response, @@ -2655,8 +2524,8 @@ public void queryWithProgramAndDimensionFilterUsingDataIdSchemeUid() { "Hart", "Mnp3oXrpAbK", "", - "fWI0UiNZgMy", - "3104")); + "3104", + "fWI0UiNZgMy")); } @Test From 819fc25ea093d9fb9f0ded7820c508272bb4c348 Mon Sep 17 00:00:00 2001 From: Giuseppe Nespolino Date: Fri, 3 Nov 2023 10:29:36 +0100 Subject: [PATCH 3/5] fix: sonarcloud warning [DHIS2-16067] --- .../common/processing/CommonQueryRequestMapper.java | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/common/processing/CommonQueryRequestMapper.java b/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/common/processing/CommonQueryRequestMapper.java index c238510fc22d..67f76a96ee2f 100644 --- a/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/common/processing/CommonQueryRequestMapper.java +++ b/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/common/processing/CommonQueryRequestMapper.java @@ -413,14 +413,10 @@ public DimensionIdentifier toDimensionIdentifier( } } - if (Objects.nonNull(queryItem)) { - return DimensionIdentifier.of( - dimensionIdentifier.getProgram(), - dimensionIdentifier.getProgramStage(), - DimensionParam.ofObject(queryItem, dimensionParamType, items)); - } - - throw new IllegalQueryException(E7250, dimensionId); + return DimensionIdentifier.of( + dimensionIdentifier.getProgram(), + dimensionIdentifier.getProgramStage(), + DimensionParam.ofObject(queryItem, dimensionParamType, items)); } private static DimensionIdentifier parseAsStaticDimension( From c1d5b96e4db268aaa723188f253e6310080a46b1 Mon Sep 17 00:00:00 2001 From: Giuseppe Nespolino Date: Fri, 3 Nov 2023 12:50:04 +0100 Subject: [PATCH 4/5] fix: e2e dimensions tests [DHIS2-16067] --- .../analytics/AnalyticsDimensionsTest.java | 10 +++-- .../tei/TeiAnalyticsController.java | 5 ++- .../dimension/DimensionMapperService.java | 12 +++++- .../EnrollmentAnalyticsPrefixStrategy.java | 3 +- .../dimension/TeiAnalyticsPrefixStrategy.java | 43 +++++++++++++++++++ 5 files changed, 65 insertions(+), 8 deletions(-) create mode 100644 dhis-2/dhis-web-api/src/main/java/org/hisp/dhis/webapi/dimension/TeiAnalyticsPrefixStrategy.java diff --git a/dhis-2/dhis-test-e2e/src/test/java/org/hisp/dhis/analytics/AnalyticsDimensionsTest.java b/dhis-2/dhis-test-e2e/src/test/java/org/hisp/dhis/analytics/AnalyticsDimensionsTest.java index b9bef42d6b83..2a24c5e97e46 100644 --- a/dhis-2/dhis-test-e2e/src/test/java/org/hisp/dhis/analytics/AnalyticsDimensionsTest.java +++ b/dhis-2/dhis-test-e2e/src/test/java/org/hisp/dhis/analytics/AnalyticsDimensionsTest.java @@ -40,7 +40,9 @@ import static org.hamcrest.Matchers.startsWith; import java.util.Arrays; +import java.util.HashSet; import java.util.List; +import java.util.Set; import java.util.function.Consumer; import java.util.stream.Collectors; import java.util.stream.Stream; @@ -283,7 +285,7 @@ public void shouldReturnAllProgramAttributes() { "programs.programTrackedEntityAttributes.flatten().trackedEntityAttribute.id", String.class) .stream() - // .distinct() attributes can be duplicated in different programs + .distinct() .collect(Collectors.toList()); analyticsTeiActions @@ -308,6 +310,8 @@ public void shouldReturnAllDataElements() { .extractList( "programs.programStages.programStageDataElements.flatten().dataElement.id"); + Set distinctDataElements = new HashSet<>(dataElements); + analyticsTeiActions .query() .getDimensions( @@ -315,7 +319,7 @@ public void shouldReturnAllDataElements() { new QueryParamsBuilder().add("filter", "dimensionType:eq:DATA_ELEMENT")) .validate() .statusCode(200) - .body("dimensions", hasSize(equalTo(dataElements.size()))) - .body("dimensions.uid", everyItem(in(dataElements))); + .body("dimensions", hasSize(equalTo(distinctDataElements.size()))) + .body("dimensions.uid", everyItem(in(distinctDataElements))); } } diff --git a/dhis-2/dhis-web-api/src/main/java/org/hisp/dhis/webapi/controller/tei/TeiAnalyticsController.java b/dhis-2/dhis-web-api/src/main/java/org/hisp/dhis/webapi/controller/tei/TeiAnalyticsController.java index e161f1f8f2f6..17cbdee340f6 100644 --- a/dhis-2/dhis-web-api/src/main/java/org/hisp/dhis/webapi/controller/tei/TeiAnalyticsController.java +++ b/dhis-2/dhis-web-api/src/main/java/org/hisp/dhis/webapi/controller/tei/TeiAnalyticsController.java @@ -65,7 +65,7 @@ import org.hisp.dhis.common.OpenApi; import org.hisp.dhis.webapi.dimension.DimensionFilteringAndPagingService; import org.hisp.dhis.webapi.dimension.DimensionMapperService; -import org.hisp.dhis.webapi.dimension.EnrollmentAnalyticsPrefixStrategy; +import org.hisp.dhis.webapi.dimension.TeiAnalyticsPrefixStrategy; import org.hisp.dhis.webapi.mvc.annotation.ApiVersion; import org.hisp.dhis.webapi.utils.ContextUtils; import org.springframework.security.access.prepost.PreAuthorize; @@ -257,7 +257,8 @@ public AnalyticsDimensionsPagingWrapper getQueryDimensions( dimensionMapperService.toDimensionResponse( teiAnalyticsDimensionsService.getQueryDimensionsByTrackedEntityTypeId( trackedEntityType, program), - EnrollmentAnalyticsPrefixStrategy.INSTANCE), + TeiAnalyticsPrefixStrategy.INSTANCE, + true), dimensionsCriteria, fields); } diff --git a/dhis-2/dhis-web-api/src/main/java/org/hisp/dhis/webapi/dimension/DimensionMapperService.java b/dhis-2/dhis-web-api/src/main/java/org/hisp/dhis/webapi/dimension/DimensionMapperService.java index f625fc9f5c2f..6fb1adbafa31 100644 --- a/dhis-2/dhis-web-api/src/main/java/org/hisp/dhis/webapi/dimension/DimensionMapperService.java +++ b/dhis-2/dhis-web-api/src/main/java/org/hisp/dhis/webapi/dimension/DimensionMapperService.java @@ -36,6 +36,7 @@ import java.util.concurrent.ConcurrentHashMap; import java.util.function.Function; import java.util.function.Predicate; +import java.util.function.UnaryOperator; import lombok.RequiredArgsConstructor; import org.hisp.dhis.common.PrefixedDimension; import org.springframework.stereotype.Service; @@ -47,7 +48,16 @@ public class DimensionMapperService { public List toDimensionResponse( Collection dimensions, PrefixStrategy prefixStrategy) { - return distinctByUid( + return toDimensionResponse(dimensions, prefixStrategy, false); + } + + public List toDimensionResponse( + Collection dimensions, PrefixStrategy prefixStrategy, boolean distinct) { + + UnaryOperator> distinctFunction = + distinct ? this::distinctByUid : UnaryOperator.identity(); + + return distinctFunction.apply( mapToList( dimensions, pDimension -> toDimensionResponse(pDimension, prefixStrategy.apply(pDimension)))); diff --git a/dhis-2/dhis-web-api/src/main/java/org/hisp/dhis/webapi/dimension/EnrollmentAnalyticsPrefixStrategy.java b/dhis-2/dhis-web-api/src/main/java/org/hisp/dhis/webapi/dimension/EnrollmentAnalyticsPrefixStrategy.java index 8309ea5f59a6..cab79371b42a 100644 --- a/dhis-2/dhis-web-api/src/main/java/org/hisp/dhis/webapi/dimension/EnrollmentAnalyticsPrefixStrategy.java +++ b/dhis-2/dhis-web-api/src/main/java/org/hisp/dhis/webapi/dimension/EnrollmentAnalyticsPrefixStrategy.java @@ -36,8 +36,7 @@ @NoArgsConstructor(access = AccessLevel.PRIVATE) public class EnrollmentAnalyticsPrefixStrategy implements PrefixStrategy { - public static final EnrollmentAnalyticsPrefixStrategy INSTANCE = - new EnrollmentAnalyticsPrefixStrategy(); + public static final PrefixStrategy INSTANCE = new EnrollmentAnalyticsPrefixStrategy(); @Override public String apply(PrefixedDimension pDimension) { diff --git a/dhis-2/dhis-web-api/src/main/java/org/hisp/dhis/webapi/dimension/TeiAnalyticsPrefixStrategy.java b/dhis-2/dhis-web-api/src/main/java/org/hisp/dhis/webapi/dimension/TeiAnalyticsPrefixStrategy.java new file mode 100644 index 000000000000..c9a437250768 --- /dev/null +++ b/dhis-2/dhis-web-api/src/main/java/org/hisp/dhis/webapi/dimension/TeiAnalyticsPrefixStrategy.java @@ -0,0 +1,43 @@ +/* + * 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.dimension; + +import lombok.AccessLevel; +import lombok.NoArgsConstructor; +import org.hisp.dhis.common.PrefixedDimension; + +@NoArgsConstructor(access = AccessLevel.PRIVATE) +public class TeiAnalyticsPrefixStrategy implements PrefixStrategy { + + public static final PrefixStrategy INSTANCE = EnrollmentAnalyticsPrefixStrategy.INSTANCE; + + @Override + public String apply(PrefixedDimension pDimension) { + throw new UnsupportedOperationException("This method should not be called for TEI analytics"); + } +} From 733a8265f8721cb172779086fccea941ccc038f6 Mon Sep 17 00:00:00 2001 From: Giuseppe Nespolino Date: Fri, 3 Nov 2023 13:13:40 +0100 Subject: [PATCH 5/5] fix: unit test [DHIS2-16067] --- .../hisp/dhis/webapi/dimension/DimensionMapperServiceTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dhis-2/dhis-web-api/src/test/java/org/hisp/dhis/webapi/dimension/DimensionMapperServiceTest.java b/dhis-2/dhis-web-api/src/test/java/org/hisp/dhis/webapi/dimension/DimensionMapperServiceTest.java index 755964c4a299..5ec514e47256 100644 --- a/dhis-2/dhis-web-api/src/test/java/org/hisp/dhis/webapi/dimension/DimensionMapperServiceTest.java +++ b/dhis-2/dhis-web-api/src/test/java/org/hisp/dhis/webapi/dimension/DimensionMapperServiceTest.java @@ -68,7 +68,7 @@ void testReturnedDimensionsHaveNoDuplicates() { List dimensionResponse = dimensionMapperService.toDimensionResponse( - mockDimensions(), EnrollmentAnalyticsPrefixStrategy.INSTANCE); + mockDimensions(), EnrollmentAnalyticsPrefixStrategy.INSTANCE, true); assertEquals(4, dimensionResponse.size());