diff --git a/dhis-2/dhis-services/dhis-service-dxf2/src/main/java/org/hisp/dhis/dxf2/datavalueset/DefaultDataExportService.java b/dhis-2/dhis-services/dhis-service-dxf2/src/main/java/org/hisp/dhis/dxf2/datavalueset/DefaultDataExportService.java index 6f8814a9e058..531f00aa597d 100644 --- a/dhis-2/dhis-services/dhis-service-dxf2/src/main/java/org/hisp/dhis/dxf2/datavalueset/DefaultDataExportService.java +++ b/dhis-2/dhis-services/dhis-service-dxf2/src/main/java/org/hisp/dhis/dxf2/datavalueset/DefaultDataExportService.java @@ -39,6 +39,7 @@ import static org.hisp.dhis.user.CurrentUserUtil.getCurrentUserDetails; import static org.hisp.dhis.util.DateUtils.toLongGmtDate; +import java.time.Duration; import java.util.ArrayList; import java.util.Collection; import java.util.Iterator; @@ -345,7 +346,7 @@ private static String emptyAsNull(String value) { return value == null || value.isEmpty() ? null : value; } - private DataExportParams decodeParams(DataExportParams.Input params) { + private DataExportParams decodeParams(DataExportParams.Input params) throws ConflictException { boolean codeFallback = Boolean.TRUE.equals(params.getInputUseCodeFallback()); IdentifiableProperty anyIn = params.getInputIdScheme(); IdentifiableProperty dsIn = params.getInputDataSetIdScheme(); @@ -358,24 +359,6 @@ private DataExportParams decodeParams(DataExportParams.Input params) { if (ouIn == null) ouIn = anyIn; if (degIn == null) degIn = anyIn; - List attributeOptionCombos = decodeIds(COC, anyIn, params.getAttributeOptionCombo()); - if (attributeOptionCombos.isEmpty() && params.getAttributeOptions() != null) { - UID aoc = - store.getAttributeOptionCombo( - params.getAttributeCombo(), params.getAttributeOptions().stream()); - if (aoc != null) attributeOptionCombos = List.of(aoc); - } - - List orders = params.getOrder(); - if (params.isOrderByPeriod()) { - if (orders == null) { - orders = List.of(Order.PE); - } else if (!orders.contains(Order.PE)) { - orders = new ArrayList<>(orders); - orders.add(0, Order.PE); - } - } - return DataExportParams.builder() .dataSets(decodeIds(DS, dsIn, params.getDataSet())) .dataElementGroups(decodeIds(DEG, degIn, params.getDataElementGroup())) @@ -384,20 +367,54 @@ private DataExportParams decodeParams(DataExportParams.Input params) { .organisationUnitGroups(decodeIds(OUG, anyIn, params.getOrgUnitGroup())) .orgUnitLevel(params.getLevel()) .categoryOptionCombos(decodeIds(COC, anyIn, params.getCategoryOptionCombo())) - .attributeOptionCombos(attributeOptionCombos) + .attributeOptionCombos(resolveAttributeOptionCombos(anyIn, params)) .periods(decodePeriods(params.getPeriod())) .startDate(params.getStartDate()) .endDate(params.getEndDate()) .includeDescendants(params.isChildren()) .includeDeleted(params.isIncludeDeleted()) .lastUpdated(params.getLastUpdated()) - .lastUpdatedDuration(DateUtils.getDuration(params.getLastUpdatedDuration())) + .lastUpdatedDuration(resolveLastUpdatedDuration(params)) .limit(params.getLimit()) .offset(params.getOffset()) - .orders(orders) + .orders(resolveOrders(params)) .build(); } + @Nonnull + private List resolveAttributeOptionCombos( + IdentifiableProperty anyIn, DataExportParams.Input params) { + List attributeOptionCombos = decodeIds(COC, anyIn, params.getAttributeOptionCombo()); + if (!attributeOptionCombos.isEmpty() || params.getAttributeOptions() == null) { + return attributeOptionCombos; + } + UID aoc = + store.getAttributeOptionCombo( + params.getAttributeCombo(), params.getAttributeOptions().stream()); + return aoc == null ? attributeOptionCombos : List.of(aoc); + } + + private List resolveOrders(DataExportParams.Input params) { + List orders = params.getOrder(); + if (!params.isOrderByPeriod()) return orders; + if (orders == null) return List.of(Order.PE); + if (orders.contains(Order.PE)) return orders; + orders = new ArrayList<>(orders); + orders.add(0, Order.PE); + return orders; + } + + private Duration resolveLastUpdatedDuration(DataExportParams.Input params) + throws ConflictException { + String lastUpdatedDurationIn = params.getLastUpdatedDuration(); + Duration lastUpdatedDuration = DateUtils.getDuration(lastUpdatedDurationIn); + if (lastUpdatedDurationIn != null + && !lastUpdatedDurationIn.isBlank() + && lastUpdatedDuration == null) + throw new ConflictException(ErrorCode.E2005, lastUpdatedDurationIn); + return lastUpdatedDuration; + } + @Nonnull private List decodeIds( IdCoder.ObjectType type, @CheckForNull IdentifiableProperty from, Collection ids) { diff --git a/dhis-2/dhis-services/dhis-service-dxf2/src/test/java/org/hisp/dhis/dxf2/datavalueset/DefaultDataExportServiceTest.java b/dhis-2/dhis-services/dhis-service-dxf2/src/test/java/org/hisp/dhis/dxf2/datavalueset/DefaultDataExportServiceTest.java new file mode 100644 index 000000000000..a75966062a47 --- /dev/null +++ b/dhis-2/dhis-services/dhis-service-dxf2/src/test/java/org/hisp/dhis/dxf2/datavalueset/DefaultDataExportServiceTest.java @@ -0,0 +1,118 @@ +/* + * 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: + * + * 1. Redistributions of source code must retain the above copyright notice, this + * list of conditions and the following disclaimer. + * + * 2. 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. + * + * 3. Neither the name of the copyright holder 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.dxf2.datavalueset; + +import static org.hisp.dhis.test.TestBase.clearSecurityContext; +import static org.hisp.dhis.test.TestBase.injectSecurityContextNoSettings; +import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; + +import java.util.Set; +import org.hisp.dhis.common.IdCoder; +import org.hisp.dhis.common.UID; +import org.hisp.dhis.datavalue.DataExportParams; +import org.hisp.dhis.datavalue.DataExportStore; +import org.hisp.dhis.feedback.ConflictException; +import org.hisp.dhis.feedback.ErrorCode; +import org.hisp.dhis.user.SystemUser; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; + +@ExtendWith(MockitoExtension.class) +class DefaultDataExportServiceTest { + + @Mock private DataExportStore store; + + @Mock private IdCoder idCoder; + + @BeforeEach + void setUp() { + injectSecurityContextNoSettings(new SystemUser()); + } + + @AfterEach + void tearDown() { + clearSecurityContext(); + } + + private DataExportParams.Input.InputBuilder validFiltersBuilder() { + return DataExportParams.Input.builder() + .dataSet(Set.of(UID.generate().getValue())) + .orgUnit(Set.of(UID.generate().getValue())) + .period(Set.of("202201")); + } + + private DefaultDataExportService service() { + return new DefaultDataExportService(store, idCoder); + } + + @Test + void testExportValues_InvalidLastUpdatedDurationThrowsE2005() { + DataExportParams.Input params = validFiltersBuilder().lastUpdatedDuration("-5d").build(); + + ConflictException ex = + assertThrows(ConflictException.class, () -> service().exportValues(params)); + + assertEquals(ErrorCode.E2005, ex.getCode()); + } + + @Test + void testExportValues_MalformedLastUpdatedDurationThrowsE2005() { + DataExportParams.Input params = validFiltersBuilder().lastUpdatedDuration("abc").build(); + + ConflictException ex = + assertThrows(ConflictException.class, () -> service().exportValues(params)); + + assertEquals(ErrorCode.E2005, ex.getCode()); + } + + @Test + void testExportValues_ZeroDurationIsValid() { + // 0d parses successfully (Duration.ZERO); it's a degenerate "as of right now" filter, + // not a parse failure, so it must not be rejected the same way "-5d"/"abc" are. + DataExportParams.Input params = validFiltersBuilder().lastUpdatedDuration("0d").build(); + + assertDoesNotThrow(() -> service().exportValues(params)); + } + + @Test + void testExportValues_BlankLastUpdatedDurationIsIgnored() { + // a blank (but non-null) value never reaches the parser, so it must not be + // treated as unparseable and rejected like "-5d"/"abc" are. + DataExportParams.Input params = validFiltersBuilder().lastUpdatedDuration(" ").build(); + + assertDoesNotThrow(() -> service().exportValues(params)); + } +}