From 6386c81872c4ea115cd03efef72fbc359e831388 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lars=20Helge=20=C3=98verland?= Date: Tue, 3 Dec 2024 14:53:46 +0100 Subject: [PATCH 1/8] refactor: Centralize isGeoSpatial property logic [DHIS2-16705] (#19369) --- .../dhis/analytics/config/ServiceConfig.java | 3 -- .../table/AbstractEventJdbcTableManager.java | 7 ++--- .../table/AbstractJdbcTableManager.java | 22 ++++++-------- .../table/JdbcAnalyticsTableManager.java | 3 -- .../table/JdbcCompletenessTableManager.java | 3 -- .../JdbcCompletenessTargetTableManager.java | 3 -- .../JdbcEnrollmentAnalyticsTableManager.java | 7 ++--- .../table/JdbcEventAnalyticsTableManager.java | 9 ++---- .../table/JdbcOrgUnitTargetTableManager.java | 3 -- .../JdbcOwnershipAnalyticsTableManager.java | 7 ++--- ...dbcTrackedEntityAnalyticsTableManager.java | 3 -- ...ntityEnrollmentsAnalyticsTableManager.java | 3 -- ...ckedEntityEventsAnalyticsTableManager.java | 3 -- .../JdbcValidationResultTableManager.java | 3 -- .../table/setting/AnalyticsTableSettings.java | 14 +++++++++ .../AbstractEventJdbcTableManagerTest.java | 12 ++++---- .../table/JdbcAnalyticsTableManagerTest.java | 2 -- ...bcEnrollmentAnalyticsTableManagerTest.java | 9 +----- ...bcEventAnalyticsTableManagerDorisTest.java | 11 ++----- .../JdbcEventAnalyticsTableManagerTest.java | 23 ++++----------- ...rackedEntityAnalyticsTableManagerTest.java | 29 ++++++++++--------- ...EntityEventsAnalyticsTableManagerTest.java | 17 +++++++++-- .../conf/DhisConfigurationProvider.java | 3 +- 23 files changed, 77 insertions(+), 122 deletions(-) diff --git a/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/config/ServiceConfig.java b/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/config/ServiceConfig.java index b9e50dde9f10..4ebb20a480eb 100644 --- a/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/config/ServiceConfig.java +++ b/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/config/ServiceConfig.java @@ -46,7 +46,6 @@ import org.hisp.dhis.period.PeriodDataProvider; import org.hisp.dhis.resourcetable.ResourceTableService; import org.hisp.dhis.setting.SystemSettingsProvider; -import org.hisp.dhis.system.database.DatabaseInfoProvider; import org.hisp.dhis.trackedentity.TrackedEntityTypeService; import org.springframework.beans.factory.annotation.Qualifier; import org.springframework.context.annotation.Bean; @@ -78,7 +77,6 @@ public AnalyticsTableManager jdbcTrackedEntityEventsAnalyticsTableManager( ResourceTableService resourceTableService, AnalyticsTableHookService tableHookService, PartitionManager partitionManager, - DatabaseInfoProvider databaseInfoProvider, @Qualifier("analyticsJdbcTemplate") JdbcTemplate jdbcTemplate, TrackedEntityTypeService trackedEntityTypeService, AnalyticsTableSettings analyticsTableSettings, @@ -94,7 +92,6 @@ public AnalyticsTableManager jdbcTrackedEntityEventsAnalyticsTableManager( resourceTableService, tableHookService, partitionManager, - databaseInfoProvider, jdbcTemplate, trackedEntityTypeService, analyticsTableSettings, diff --git a/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/table/AbstractEventJdbcTableManager.java b/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/table/AbstractEventJdbcTableManager.java index 1669e52bd535..5179b999407f 100644 --- a/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/table/AbstractEventJdbcTableManager.java +++ b/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/table/AbstractEventJdbcTableManager.java @@ -57,7 +57,6 @@ import org.hisp.dhis.period.PeriodDataProvider; import org.hisp.dhis.resourcetable.ResourceTableService; import org.hisp.dhis.setting.SystemSettingsProvider; -import org.hisp.dhis.system.database.DatabaseInfoProvider; import org.hisp.dhis.trackedentity.TrackedEntityAttribute; import org.springframework.jdbc.core.JdbcTemplate; @@ -74,9 +73,8 @@ public AbstractEventJdbcTableManager( ResourceTableService resourceTableService, AnalyticsTableHookService tableHookService, PartitionManager partitionManager, - DatabaseInfoProvider databaseInfoProvider, JdbcTemplate jdbcTemplate, - AnalyticsTableSettings analyticsExportSettings, + AnalyticsTableSettings analyticsTableSettings, PeriodDataProvider periodDataProvider, SqlBuilder sqlBuilder) { super( @@ -88,9 +86,8 @@ public AbstractEventJdbcTableManager( resourceTableService, tableHookService, partitionManager, - databaseInfoProvider, jdbcTemplate, - analyticsExportSettings, + analyticsTableSettings, periodDataProvider, sqlBuilder); } diff --git a/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/table/AbstractJdbcTableManager.java b/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/table/AbstractJdbcTableManager.java index d21a187ed63c..7f735ce1074b 100644 --- a/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/table/AbstractJdbcTableManager.java +++ b/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/table/AbstractJdbcTableManager.java @@ -83,7 +83,6 @@ import org.hisp.dhis.resourcetable.ResourceTableService; import org.hisp.dhis.setting.SystemSettings; import org.hisp.dhis.setting.SystemSettingsProvider; -import org.hisp.dhis.system.database.DatabaseInfoProvider; import org.hisp.dhis.system.util.MathUtils; import org.hisp.dhis.util.DateUtils; import org.springframework.dao.DataAccessException; @@ -139,8 +138,6 @@ public abstract class AbstractJdbcTableManager implements AnalyticsTableManager protected final PartitionManager partitionManager; - protected final DatabaseInfoProvider databaseInfoProvider; - protected final JdbcTemplate jdbcTemplate; protected final AnalyticsTableSettings analyticsTableSettings; @@ -149,16 +146,6 @@ public abstract class AbstractJdbcTableManager implements AnalyticsTableManager protected final SqlBuilder sqlBuilder; - protected Boolean spatialSupport; - - protected boolean isSpatialSupport() { - if (spatialSupport == null) { - spatialSupport = databaseInfoProvider.getDatabaseInfo().isSpatialSupport(); - } - - return spatialSupport && sqlBuilder.supportsGeospatialData(); - } - /** * Encapsulates the SQL logic to get the correct date column based on the event status. If new * statuses need to be loaded into the analytics events tables, they have to be supported/added @@ -344,6 +331,15 @@ private boolean tableExists(String name) { // Protected supportive methods // ------------------------------------------------------------------------- + /** + * Indicates whether spatial support is available. + * + * @return true if spatial support is available. + */ + protected boolean isSpatialSupport() { + return analyticsTableSettings.isSpatialSupport() && sqlBuilder.supportsGeospatialData(); + } + /** Returns the analytics table name. */ protected String getTableName() { return getAnalyticsTableType().getTableName(); diff --git a/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/table/JdbcAnalyticsTableManager.java b/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/table/JdbcAnalyticsTableManager.java index 48bdf885800c..359bba2ce1d2 100644 --- a/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/table/JdbcAnalyticsTableManager.java +++ b/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/table/JdbcAnalyticsTableManager.java @@ -77,7 +77,6 @@ import org.hisp.dhis.resourcetable.ResourceTableService; import org.hisp.dhis.setting.SystemSettings; import org.hisp.dhis.setting.SystemSettingsProvider; -import org.hisp.dhis.system.database.DatabaseInfoProvider; import org.hisp.dhis.util.DateUtils; import org.hisp.dhis.util.ObjectUtils; import org.springframework.beans.factory.annotation.Qualifier; @@ -171,7 +170,6 @@ public JdbcAnalyticsTableManager( ResourceTableService resourceTableService, AnalyticsTableHookService tableHookService, PartitionManager partitionManager, - DatabaseInfoProvider databaseInfoProvider, @Qualifier("analyticsJdbcTemplate") JdbcTemplate jdbcTemplate, AnalyticsTableSettings analyticsTableSettings, PeriodDataProvider periodDataProvider, @@ -185,7 +183,6 @@ public JdbcAnalyticsTableManager( resourceTableService, tableHookService, partitionManager, - databaseInfoProvider, jdbcTemplate, analyticsTableSettings, periodDataProvider, diff --git a/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/table/JdbcCompletenessTableManager.java b/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/table/JdbcCompletenessTableManager.java index ea052fd5de76..fe98336a13f9 100644 --- a/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/table/JdbcCompletenessTableManager.java +++ b/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/table/JdbcCompletenessTableManager.java @@ -64,7 +64,6 @@ import org.hisp.dhis.period.PeriodDataProvider; import org.hisp.dhis.resourcetable.ResourceTableService; import org.hisp.dhis.setting.SystemSettingsProvider; -import org.hisp.dhis.system.database.DatabaseInfoProvider; import org.hisp.dhis.util.DateUtils; import org.springframework.beans.factory.annotation.Qualifier; import org.springframework.jdbc.core.JdbcTemplate; @@ -102,7 +101,6 @@ public JdbcCompletenessTableManager( ResourceTableService resourceTableService, AnalyticsTableHookService tableHookService, PartitionManager partitionManager, - DatabaseInfoProvider databaseInfoProvider, @Qualifier("analyticsJdbcTemplate") JdbcTemplate jdbcTemplate, AnalyticsTableSettings analyticsTableSettings, PeriodDataProvider periodDataProvider, @@ -116,7 +114,6 @@ public JdbcCompletenessTableManager( resourceTableService, tableHookService, partitionManager, - databaseInfoProvider, jdbcTemplate, analyticsTableSettings, periodDataProvider, diff --git a/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/table/JdbcCompletenessTargetTableManager.java b/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/table/JdbcCompletenessTargetTableManager.java index 086efebaf47b..4b74d8ccee2a 100644 --- a/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/table/JdbcCompletenessTargetTableManager.java +++ b/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/table/JdbcCompletenessTargetTableManager.java @@ -56,7 +56,6 @@ import org.hisp.dhis.period.PeriodDataProvider; import org.hisp.dhis.resourcetable.ResourceTableService; import org.hisp.dhis.setting.SystemSettingsProvider; -import org.hisp.dhis.system.database.DatabaseInfoProvider; import org.springframework.beans.factory.annotation.Qualifier; import org.springframework.jdbc.core.JdbcTemplate; import org.springframework.stereotype.Service; @@ -113,7 +112,6 @@ public JdbcCompletenessTargetTableManager( ResourceTableService resourceTableService, AnalyticsTableHookService tableHookService, PartitionManager partitionManager, - DatabaseInfoProvider databaseInfoProvider, @Qualifier("analyticsJdbcTemplate") JdbcTemplate jdbcTemplate, AnalyticsTableSettings analyticsTableSettings, PeriodDataProvider periodDataProvider, @@ -127,7 +125,6 @@ public JdbcCompletenessTargetTableManager( resourceTableService, tableHookService, partitionManager, - databaseInfoProvider, jdbcTemplate, analyticsTableSettings, periodDataProvider, diff --git a/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/table/JdbcEnrollmentAnalyticsTableManager.java b/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/table/JdbcEnrollmentAnalyticsTableManager.java index b7c0ca957a65..59bdf500f0a5 100644 --- a/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/table/JdbcEnrollmentAnalyticsTableManager.java +++ b/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/table/JdbcEnrollmentAnalyticsTableManager.java @@ -53,7 +53,6 @@ import org.hisp.dhis.program.Program; import org.hisp.dhis.resourcetable.ResourceTableService; import org.hisp.dhis.setting.SystemSettingsProvider; -import org.hisp.dhis.system.database.DatabaseInfoProvider; import org.springframework.beans.factory.annotation.Qualifier; import org.springframework.jdbc.core.JdbcTemplate; import org.springframework.stereotype.Service; @@ -76,9 +75,8 @@ public JdbcEnrollmentAnalyticsTableManager( ResourceTableService resourceTableService, AnalyticsTableHookService tableHookService, PartitionManager partitionManager, - DatabaseInfoProvider databaseInfoProvider, @Qualifier("analyticsJdbcTemplate") JdbcTemplate jdbcTemplate, - AnalyticsTableSettings analyticsExportSettings, + AnalyticsTableSettings analyticsTableSettings, PeriodDataProvider periodDataProvider, SqlBuilder sqlBuilder) { super( @@ -90,9 +88,8 @@ public JdbcEnrollmentAnalyticsTableManager( resourceTableService, tableHookService, partitionManager, - databaseInfoProvider, jdbcTemplate, - analyticsExportSettings, + analyticsTableSettings, periodDataProvider, sqlBuilder); fixedColumns = EnrollmentAnalyticsColumn.getColumns(sqlBuilder); diff --git a/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/table/JdbcEventAnalyticsTableManager.java b/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/table/JdbcEventAnalyticsTableManager.java index 6ba48fd9cfb4..4b0b572b390c 100644 --- a/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/table/JdbcEventAnalyticsTableManager.java +++ b/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/table/JdbcEventAnalyticsTableManager.java @@ -82,7 +82,6 @@ import org.hisp.dhis.resourcetable.ResourceTableService; import org.hisp.dhis.setting.SystemSettings; import org.hisp.dhis.setting.SystemSettingsProvider; -import org.hisp.dhis.system.database.DatabaseInfoProvider; import org.hisp.dhis.trackedentity.TrackedEntityAttribute; import org.hisp.dhis.util.DateUtils; import org.springframework.beans.factory.annotation.Qualifier; @@ -100,7 +99,7 @@ public class JdbcEventAnalyticsTableManager extends AbstractEventJdbcTableManage static final String[] EXPORTABLE_EVENT_STATUSES = {"'COMPLETED'", "'ACTIVE'", "'SCHEDULE'"}; - protected final List fixedColumns; + private final List fixedColumns; public JdbcEventAnalyticsTableManager( IdentifiableObjectManager idObjectManager, @@ -111,9 +110,8 @@ public JdbcEventAnalyticsTableManager( ResourceTableService resourceTableService, AnalyticsTableHookService tableHookService, PartitionManager partitionManager, - DatabaseInfoProvider databaseInfoProvider, @Qualifier("analyticsJdbcTemplate") JdbcTemplate jdbcTemplate, - AnalyticsTableSettings analyticsExportSettings, + AnalyticsTableSettings analyticsTableSettings, PeriodDataProvider periodDataProvider, SqlBuilder sqlBuilder) { super( @@ -125,9 +123,8 @@ public JdbcEventAnalyticsTableManager( resourceTableService, tableHookService, partitionManager, - databaseInfoProvider, jdbcTemplate, - analyticsExportSettings, + analyticsTableSettings, periodDataProvider, sqlBuilder); fixedColumns = EventAnalyticsColumn.getColumns(sqlBuilder); diff --git a/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/table/JdbcOrgUnitTargetTableManager.java b/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/table/JdbcOrgUnitTargetTableManager.java index d7ca3423ded1..5874ec70477f 100644 --- a/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/table/JdbcOrgUnitTargetTableManager.java +++ b/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/table/JdbcOrgUnitTargetTableManager.java @@ -57,7 +57,6 @@ import org.hisp.dhis.period.PeriodDataProvider; import org.hisp.dhis.resourcetable.ResourceTableService; import org.hisp.dhis.setting.SystemSettingsProvider; -import org.hisp.dhis.system.database.DatabaseInfoProvider; import org.springframework.beans.factory.annotation.Qualifier; import org.springframework.jdbc.core.JdbcTemplate; import org.springframework.stereotype.Service; @@ -88,7 +87,6 @@ public JdbcOrgUnitTargetTableManager( ResourceTableService resourceTableService, AnalyticsTableHookService tableHookService, PartitionManager partitionManager, - DatabaseInfoProvider databaseInfoProvider, @Qualifier("analyticsJdbcTemplate") JdbcTemplate jdbcTemplate, AnalyticsTableSettings analyticsTableSettings, PeriodDataProvider periodDataProvider, @@ -102,7 +100,6 @@ public JdbcOrgUnitTargetTableManager( resourceTableService, tableHookService, partitionManager, - databaseInfoProvider, jdbcTemplate, analyticsTableSettings, periodDataProvider, diff --git a/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/table/JdbcOwnershipAnalyticsTableManager.java b/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/table/JdbcOwnershipAnalyticsTableManager.java index 7b882a0e7c30..2d3b77847e19 100644 --- a/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/table/JdbcOwnershipAnalyticsTableManager.java +++ b/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/table/JdbcOwnershipAnalyticsTableManager.java @@ -64,7 +64,6 @@ import org.hisp.dhis.program.Program; import org.hisp.dhis.resourcetable.ResourceTableService; import org.hisp.dhis.setting.SystemSettingsProvider; -import org.hisp.dhis.system.database.DatabaseInfoProvider; import org.hisp.quick.JdbcConfiguration; import org.springframework.beans.factory.annotation.Qualifier; import org.springframework.jdbc.core.JdbcTemplate; @@ -120,10 +119,9 @@ public JdbcOwnershipAnalyticsTableManager( ResourceTableService resourceTableService, AnalyticsTableHookService tableHookService, PartitionManager partitionManager, - DatabaseInfoProvider databaseInfoProvider, @Qualifier("analyticsJdbcTemplate") JdbcTemplate jdbcTemplate, JdbcConfiguration jdbcConfiguration, - AnalyticsTableSettings analyticsExportSettings, + AnalyticsTableSettings analyticsTableSettings, PeriodDataProvider periodDataProvider, SqlBuilder sqlBuilder) { super( @@ -135,9 +133,8 @@ public JdbcOwnershipAnalyticsTableManager( resourceTableService, tableHookService, partitionManager, - databaseInfoProvider, jdbcTemplate, - analyticsExportSettings, + analyticsTableSettings, periodDataProvider, sqlBuilder); this.jdbcConfiguration = jdbcConfiguration; diff --git a/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/table/JdbcTrackedEntityAnalyticsTableManager.java b/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/table/JdbcTrackedEntityAnalyticsTableManager.java index 503bd434e512..e2d0bdb9e5ac 100644 --- a/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/table/JdbcTrackedEntityAnalyticsTableManager.java +++ b/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/table/JdbcTrackedEntityAnalyticsTableManager.java @@ -75,7 +75,6 @@ import org.hisp.dhis.program.Program; import org.hisp.dhis.resourcetable.ResourceTableService; import org.hisp.dhis.setting.SystemSettingsProvider; -import org.hisp.dhis.system.database.DatabaseInfoProvider; import org.hisp.dhis.trackedentity.TrackedEntityAttribute; import org.hisp.dhis.trackedentity.TrackedEntityAttributeService; import org.hisp.dhis.trackedentity.TrackedEntityType; @@ -104,7 +103,6 @@ public JdbcTrackedEntityAnalyticsTableManager( ResourceTableService resourceTableService, AnalyticsTableHookService tableHookService, PartitionManager partitionManager, - DatabaseInfoProvider databaseInfoProvider, @Qualifier("analyticsJdbcTemplate") JdbcTemplate jdbcTemplate, TrackedEntityTypeService trackedEntityTypeService, TrackedEntityAttributeService trackedEntityAttributeService, @@ -120,7 +118,6 @@ public JdbcTrackedEntityAnalyticsTableManager( resourceTableService, tableHookService, partitionManager, - databaseInfoProvider, jdbcTemplate, analyticsTableSettings, periodDataProvider, diff --git a/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/table/JdbcTrackedEntityEnrollmentsAnalyticsTableManager.java b/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/table/JdbcTrackedEntityEnrollmentsAnalyticsTableManager.java index fc9f6f1b510a..70428872148d 100644 --- a/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/table/JdbcTrackedEntityEnrollmentsAnalyticsTableManager.java +++ b/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/table/JdbcTrackedEntityEnrollmentsAnalyticsTableManager.java @@ -67,7 +67,6 @@ import org.hisp.dhis.period.PeriodDataProvider; import org.hisp.dhis.resourcetable.ResourceTableService; import org.hisp.dhis.setting.SystemSettingsProvider; -import org.hisp.dhis.system.database.DatabaseInfoProvider; import org.hisp.dhis.trackedentity.TrackedEntityTypeService; import org.springframework.beans.factory.annotation.Qualifier; import org.springframework.jdbc.core.JdbcTemplate; @@ -176,7 +175,6 @@ public JdbcTrackedEntityEnrollmentsAnalyticsTableManager( ResourceTableService resourceTableService, AnalyticsTableHookService tableHookService, PartitionManager partitionManager, - DatabaseInfoProvider databaseInfoProvider, @Qualifier("analyticsJdbcTemplate") JdbcTemplate jdbcTemplate, TrackedEntityTypeService trackedEntityTypeService, AnalyticsTableSettings analyticsTableSettings, @@ -191,7 +189,6 @@ public JdbcTrackedEntityEnrollmentsAnalyticsTableManager( resourceTableService, tableHookService, partitionManager, - databaseInfoProvider, jdbcTemplate, analyticsTableSettings, periodDataProvider, diff --git a/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/table/JdbcTrackedEntityEventsAnalyticsTableManager.java b/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/table/JdbcTrackedEntityEventsAnalyticsTableManager.java index f3055b6db7bc..a89213becd23 100644 --- a/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/table/JdbcTrackedEntityEventsAnalyticsTableManager.java +++ b/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/table/JdbcTrackedEntityEventsAnalyticsTableManager.java @@ -78,7 +78,6 @@ import org.hisp.dhis.period.PeriodType; import org.hisp.dhis.resourcetable.ResourceTableService; import org.hisp.dhis.setting.SystemSettingsProvider; -import org.hisp.dhis.system.database.DatabaseInfoProvider; import org.hisp.dhis.trackedentity.TrackedEntityType; import org.hisp.dhis.trackedentity.TrackedEntityTypeService; import org.springframework.beans.factory.annotation.Qualifier; @@ -206,7 +205,6 @@ public JdbcTrackedEntityEventsAnalyticsTableManager( ResourceTableService resourceTableService, AnalyticsTableHookService tableHookService, PartitionManager partitionManager, - DatabaseInfoProvider databaseInfoProvider, @Qualifier("analyticsJdbcTemplate") JdbcTemplate jdbcTemplate, TrackedEntityTypeService trackedEntityTypeService, AnalyticsTableSettings analyticsTableSettings, @@ -222,7 +220,6 @@ public JdbcTrackedEntityEventsAnalyticsTableManager( resourceTableService, tableHookService, partitionManager, - databaseInfoProvider, jdbcTemplate, analyticsTableSettings, periodDataProvider, diff --git a/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/table/JdbcValidationResultTableManager.java b/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/table/JdbcValidationResultTableManager.java index e25bd5756486..26614fdc3c12 100644 --- a/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/table/JdbcValidationResultTableManager.java +++ b/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/table/JdbcValidationResultTableManager.java @@ -62,7 +62,6 @@ import org.hisp.dhis.period.PeriodDataProvider; import org.hisp.dhis.resourcetable.ResourceTableService; import org.hisp.dhis.setting.SystemSettingsProvider; -import org.hisp.dhis.system.database.DatabaseInfoProvider; import org.hisp.dhis.util.DateUtils; import org.springframework.beans.factory.annotation.Qualifier; import org.springframework.jdbc.core.JdbcTemplate; @@ -109,7 +108,6 @@ public JdbcValidationResultTableManager( ResourceTableService resourceTableService, AnalyticsTableHookService tableHookService, PartitionManager partitionManager, - DatabaseInfoProvider databaseInfoProvider, @Qualifier("analyticsJdbcTemplate") JdbcTemplate jdbcTemplate, AnalyticsTableSettings analyticsTableSettings, PeriodDataProvider periodDataProvider, @@ -123,7 +121,6 @@ public JdbcValidationResultTableManager( resourceTableService, tableHookService, partitionManager, - databaseInfoProvider, jdbcTemplate, analyticsTableSettings, periodDataProvider, diff --git a/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/table/setting/AnalyticsTableSettings.java b/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/table/setting/AnalyticsTableSettings.java index 75fbb8d6b103..645b1aef8154 100644 --- a/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/table/setting/AnalyticsTableSettings.java +++ b/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/table/setting/AnalyticsTableSettings.java @@ -53,6 +53,8 @@ import org.hisp.dhis.period.PeriodDataProvider.PeriodSource; import org.hisp.dhis.setting.SystemSettings; import org.hisp.dhis.setting.SystemSettingsProvider; +import org.hisp.dhis.system.database.DatabaseInfo; +import org.hisp.dhis.system.database.DatabaseInfoProvider; import org.springframework.stereotype.Component; /** @@ -68,6 +70,8 @@ public class AnalyticsTableSettings { private final SystemSettingsProvider settingsProvider; + private final DatabaseInfoProvider databaseInfoProvider; + /** * Returns the setting indicating whether resource and analytics tables should be logged or * unlogged. @@ -130,6 +134,16 @@ public Set getSkipColumnDimensions() { return toSet(config.getProperty(ANALYTICS_TABLE_SKIP_COLUMN)); } + /** + * Indicates whether spatial database support is available. + * + * @return true if spatial database support is available. + */ + public boolean isSpatialSupport() { + DatabaseInfo info = databaseInfoProvider.getDatabaseInfo(); + return info != null && info.isSpatialSupport(); + } + /** * Returns the {@link Database} matching the given value. * diff --git a/dhis-2/dhis-services/dhis-service-analytics/src/test/java/org/hisp/dhis/analytics/table/AbstractEventJdbcTableManagerTest.java b/dhis-2/dhis-services/dhis-service-analytics/src/test/java/org/hisp/dhis/analytics/table/AbstractEventJdbcTableManagerTest.java index 68decf6d5200..05f4337cc10a 100644 --- a/dhis-2/dhis-services/dhis-service-analytics/src/test/java/org/hisp/dhis/analytics/table/AbstractEventJdbcTableManagerTest.java +++ b/dhis-2/dhis-services/dhis-service-analytics/src/test/java/org/hisp/dhis/analytics/table/AbstractEventJdbcTableManagerTest.java @@ -30,28 +30,26 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.mockito.Mockito.when; +import org.hisp.dhis.analytics.table.setting.AnalyticsTableSettings; import org.hisp.dhis.common.ValueType; import org.hisp.dhis.db.sql.PostgreSqlBuilder; import org.hisp.dhis.db.sql.SqlBuilder; -import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; import org.mockito.InjectMocks; +import org.mockito.Mock; import org.mockito.Spy; import org.mockito.junit.jupiter.MockitoExtension; @ExtendWith(MockitoExtension.class) class AbstractEventJdbcTableManagerTest { + @Mock private AnalyticsTableSettings settings; + @Spy private SqlBuilder sqlBuilder = new PostgreSqlBuilder(); @InjectMocks private JdbcEventAnalyticsTableManager manager; - @BeforeEach - public void beforeEach() { - manager.spatialSupport = true; - } - @Test void testGetCastExpression() { String expected = @@ -124,7 +122,7 @@ void testGetSelectExpressionText() { @Test void testGetSelectExpressionGeometry() { - when(manager.isSpatialSupport()).thenReturn(Boolean.TRUE); + when(manager.isSpatialSupport()).thenReturn(true); String expected = """ diff --git a/dhis-2/dhis-services/dhis-service-analytics/src/test/java/org/hisp/dhis/analytics/table/JdbcAnalyticsTableManagerTest.java b/dhis-2/dhis-services/dhis-service-analytics/src/test/java/org/hisp/dhis/analytics/table/JdbcAnalyticsTableManagerTest.java index 25d7db4f7e31..76a6eb4bd460 100644 --- a/dhis-2/dhis-services/dhis-service-analytics/src/test/java/org/hisp/dhis/analytics/table/JdbcAnalyticsTableManagerTest.java +++ b/dhis-2/dhis-services/dhis-service-analytics/src/test/java/org/hisp/dhis/analytics/table/JdbcAnalyticsTableManagerTest.java @@ -62,7 +62,6 @@ import org.hisp.dhis.resourcetable.ResourceTableService; import org.hisp.dhis.setting.SystemSettings; import org.hisp.dhis.setting.SystemSettingsProvider; -import org.hisp.dhis.system.database.DatabaseInfoProvider; import org.joda.time.DateTime; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.DisplayName; @@ -111,7 +110,6 @@ public void setUp() { mock(ResourceTableService.class), mock(AnalyticsTableHookService.class), mock(PartitionManager.class), - mock(DatabaseInfoProvider.class), jdbcTemplate, analyticsTableSettings, periodDataProvider, diff --git a/dhis-2/dhis-services/dhis-service-analytics/src/test/java/org/hisp/dhis/analytics/table/JdbcEnrollmentAnalyticsTableManagerTest.java b/dhis-2/dhis-services/dhis-service-analytics/src/test/java/org/hisp/dhis/analytics/table/JdbcEnrollmentAnalyticsTableManagerTest.java index 7a355952e8be..d1b9f4443599 100644 --- a/dhis-2/dhis-services/dhis-service-analytics/src/test/java/org/hisp/dhis/analytics/table/JdbcEnrollmentAnalyticsTableManagerTest.java +++ b/dhis-2/dhis-services/dhis-service-analytics/src/test/java/org/hisp/dhis/analytics/table/JdbcEnrollmentAnalyticsTableManagerTest.java @@ -62,8 +62,6 @@ import org.hisp.dhis.resourcetable.ResourceTableService; import org.hisp.dhis.setting.SystemSettings; import org.hisp.dhis.setting.SystemSettingsProvider; -import org.hisp.dhis.system.database.DatabaseInfo; -import org.hisp.dhis.system.database.DatabaseInfoProvider; import org.hisp.dhis.trackedentity.TrackedEntityAttribute; import org.joda.time.DateTime; import org.junit.jupiter.api.BeforeEach; @@ -81,8 +79,6 @@ class JdbcEnrollmentAnalyticsTableManagerTest { @Mock private IdentifiableObjectManager idObjectManager; - @Mock private DatabaseInfoProvider databaseInfoProvider; - @Mock private JdbcTemplate jdbcTemplate; @Mock private AnalyticsTableSettings analyticsTableSettings; @@ -97,7 +93,7 @@ class JdbcEnrollmentAnalyticsTableManagerTest { @BeforeEach public void setUp() { - when(databaseInfoProvider.getDatabaseInfo()).thenReturn(DatabaseInfo.builder().build()); + when(analyticsTableSettings.isSpatialSupport()).thenReturn(true); SystemSettingsProvider settingsProvider = mock(SystemSettingsProvider.class); lenient().when(settingsProvider.getCurrentSettings()).thenReturn(SystemSettings.of(Map.of())); subject = @@ -110,7 +106,6 @@ public void setUp() { mock(ResourceTableService.class), mock(AnalyticsTableHookService.class), mock(PartitionManager.class), - databaseInfoProvider, jdbcTemplate, analyticsTableSettings, periodDataProvider, @@ -120,8 +115,6 @@ public void setUp() { @Test void verifyTeiTypeOrgUnitFetchesOuUidWhenPopulatingEventAnalyticsTable() { ArgumentCaptor sql = ArgumentCaptor.forClass(String.class); - when(databaseInfoProvider.getDatabaseInfo()) - .thenReturn(DatabaseInfo.builder().spatialSupport(true).build()); Program p1 = createProgram('A'); TrackedEntityAttribute tea = createTrackedEntityAttribute('a', ValueType.ORGANISATION_UNIT); diff --git a/dhis-2/dhis-services/dhis-service-analytics/src/test/java/org/hisp/dhis/analytics/table/JdbcEventAnalyticsTableManagerDorisTest.java b/dhis-2/dhis-services/dhis-service-analytics/src/test/java/org/hisp/dhis/analytics/table/JdbcEventAnalyticsTableManagerDorisTest.java index 54bdc761b617..9f952e8807b3 100644 --- a/dhis-2/dhis-services/dhis-service-analytics/src/test/java/org/hisp/dhis/analytics/table/JdbcEventAnalyticsTableManagerDorisTest.java +++ b/dhis-2/dhis-services/dhis-service-analytics/src/test/java/org/hisp/dhis/analytics/table/JdbcEventAnalyticsTableManagerDorisTest.java @@ -71,8 +71,6 @@ import org.hisp.dhis.resourcetable.ResourceTableService; import org.hisp.dhis.setting.SystemSettings; import org.hisp.dhis.setting.SystemSettingsProvider; -import org.hisp.dhis.system.database.DatabaseInfo; -import org.hisp.dhis.system.database.DatabaseInfoProvider; import org.joda.time.DateTime; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -98,16 +96,14 @@ class JdbcEventAnalyticsTableManagerDorisTest { @Mock private SystemSettings settings; - @Mock private DatabaseInfoProvider databaseInfoProvider; - @Mock private JdbcTemplate jdbcTemplate; + @Mock private AnalyticsTableSettings analyticsTableSettings; + @Mock private ResourceTableService resourceTableService; @Mock private PeriodDataProvider periodDataProvider; - @Mock private AnalyticsTableSettings analyticsTableSettings; - @Spy private SqlBuilder sqlBuilder = new DorisSqlBuilder("dhis2", "driver"); @InjectMocks private JdbcEventAnalyticsTableManager subject; @@ -141,7 +137,6 @@ private String quote(String relation) { void setUp() { today = Date.from(LocalDate.of(2019, 7, 6).atStartOfDay(ZoneId.systemDefault()).toInstant()); - when(databaseInfoProvider.getDatabaseInfo()).thenReturn(DatabaseInfo.builder().build()); when(settingsProvider.getCurrentSettings()).thenReturn(settings); when(settings.getLastSuccessfulResourceTablesUpdate()).thenReturn(new Date(0L)); when(analyticsTableSettings.getPeriodSource()).thenReturn(PeriodSource.DATABASE); @@ -149,8 +144,6 @@ void setUp() { @Test void verifyGetTableWithDataElements() { - when(databaseInfoProvider.getDatabaseInfo()) - .thenReturn(DatabaseInfo.builder().spatialSupport(true).build()); Program program = createProgram('A'); DataElement deA = createDataElement('A', ValueType.TEXT, AggregationType.SUM); diff --git a/dhis-2/dhis-services/dhis-service-analytics/src/test/java/org/hisp/dhis/analytics/table/JdbcEventAnalyticsTableManagerTest.java b/dhis-2/dhis-services/dhis-service-analytics/src/test/java/org/hisp/dhis/analytics/table/JdbcEventAnalyticsTableManagerTest.java index dc9b0903986c..b7da2447b4ac 100644 --- a/dhis-2/dhis-services/dhis-service-analytics/src/test/java/org/hisp/dhis/analytics/table/JdbcEventAnalyticsTableManagerTest.java +++ b/dhis-2/dhis-services/dhis-service-analytics/src/test/java/org/hisp/dhis/analytics/table/JdbcEventAnalyticsTableManagerTest.java @@ -99,8 +99,6 @@ import org.hisp.dhis.resourcetable.ResourceTableService; import org.hisp.dhis.setting.SystemSettings; import org.hisp.dhis.setting.SystemSettingsProvider; -import org.hisp.dhis.system.database.DatabaseInfo; -import org.hisp.dhis.system.database.DatabaseInfoProvider; import org.hisp.dhis.test.random.BeanRandomizer; import org.hisp.dhis.trackedentity.TrackedEntityAttribute; import org.joda.time.DateTime; @@ -130,8 +128,6 @@ class JdbcEventAnalyticsTableManagerTest { @Mock private SystemSettings settings; - @Mock private DatabaseInfoProvider databaseInfoProvider; - @Mock private JdbcTemplate jdbcTemplate; @Mock private ResourceTableService resourceTableService; @@ -174,7 +170,6 @@ class JdbcEventAnalyticsTableManagerTest { public void setUp() { today = Date.from(LocalDate.of(2019, 7, 6).atStartOfDay(ZoneId.systemDefault()).toInstant()); - when(databaseInfoProvider.getDatabaseInfo()).thenReturn(DatabaseInfo.builder().build()); when(settingsProvider.getCurrentSettings()).thenReturn(settings); when(settings.getLastSuccessfulResourceTablesUpdate()).thenReturn(new Date(0L)); } @@ -362,8 +357,7 @@ private AnalyticsTableColumn getColumn(String column, AnalyticsTable analyticsTa @Test void verifyGetTableWithDataElements() { - when(databaseInfoProvider.getDatabaseInfo()) - .thenReturn(DatabaseInfo.builder().spatialSupport(true).build()); + when(analyticsTableSettings.isSpatialSupport()).thenReturn(true); Program program = createProgram('A'); DataElement d1 = createDataElement('Z', ValueType.TEXT, AggregationType.SUM); @@ -486,8 +480,7 @@ void verifyGetTableWithDataElements() { @Test void verifyGetTableWithTrackedEntityAttribute() { - when(databaseInfoProvider.getDatabaseInfo()) - .thenReturn(DatabaseInfo.builder().spatialSupport(true).build()); + when(analyticsTableSettings.isSpatialSupport()).thenReturn(true); Program program = createProgram('A'); TrackedEntityAttribute tea1 = rnd.nextObject(TrackedEntityAttribute.class); @@ -570,8 +563,7 @@ void verifyGetTableWithTrackedEntityAttribute() { @Test void verifyDataElementTypeOrgUnitFetchesOuNameWhenPopulatingEventAnalyticsTable() { ArgumentCaptor sql = ArgumentCaptor.forClass(String.class); - when(databaseInfoProvider.getDatabaseInfo()) - .thenReturn(DatabaseInfo.builder().spatialSupport(true).build()); + when(analyticsTableSettings.isSpatialSupport()).thenReturn(true); Program programA = createProgram('A'); DataElement d5 = createDataElement('G', ValueType.ORGANISATION_UNIT, AggregationType.NONE); @@ -624,8 +616,7 @@ void verifyDataElementTypeOrgUnitFetchesOuNameWhenPopulatingEventAnalyticsTable( @Test void verifyTeiTypeOrgUnitFetchesOuNameWhenPopulatingEventAnalyticsTable() { ArgumentCaptor sql = ArgumentCaptor.forClass(String.class); - when(databaseInfoProvider.getDatabaseInfo()) - .thenReturn(DatabaseInfo.builder().spatialSupport(true).build()); + when(analyticsTableSettings.isSpatialSupport()).thenReturn(true); Program programA = createProgram('A'); TrackedEntityAttribute tea = createTrackedEntityAttribute('a', ValueType.ORGANISATION_UNIT); @@ -681,8 +672,7 @@ void verifyTeiTypeOrgUnitFetchesOuNameWhenPopulatingEventAnalyticsTable() { @Test void verifyOrgUnitOwnershipJoinsWhenPopulatingEventAnalyticsTable() { ArgumentCaptor sql = ArgumentCaptor.forClass(String.class); - when(databaseInfoProvider.getDatabaseInfo()) - .thenReturn(DatabaseInfo.builder().spatialSupport(true).build()); + when(analyticsTableSettings.isSpatialSupport()).thenReturn(true); Program programA = createProgram('A'); TrackedEntityAttribute tea = createTrackedEntityAttribute('a', ValueType.ORGANISATION_UNIT); @@ -899,8 +889,7 @@ private void match(AnalyticsTableColumn col) { @Test void verifyTeaTypeOrgUnitFetchesOuNameWhenPopulatingEventAnalyticsTable() { ArgumentCaptor sql = ArgumentCaptor.forClass(String.class); - when(databaseInfoProvider.getDatabaseInfo()) - .thenReturn(DatabaseInfo.builder().spatialSupport(true).build()); + when(analyticsTableSettings.isSpatialSupport()).thenReturn(true); Program programA = createProgram('A'); TrackedEntityAttribute tea = createTrackedEntityAttribute('a', ValueType.ORGANISATION_UNIT); diff --git a/dhis-2/dhis-services/dhis-service-analytics/src/test/java/org/hisp/dhis/analytics/table/JdbcTrackedEntityAnalyticsTableManagerTest.java b/dhis-2/dhis-services/dhis-service-analytics/src/test/java/org/hisp/dhis/analytics/table/JdbcTrackedEntityAnalyticsTableManagerTest.java index d9f1dc4038a9..b80504d26b7e 100644 --- a/dhis-2/dhis-services/dhis-service-analytics/src/test/java/org/hisp/dhis/analytics/table/JdbcTrackedEntityAnalyticsTableManagerTest.java +++ b/dhis-2/dhis-services/dhis-service-analytics/src/test/java/org/hisp/dhis/analytics/table/JdbcTrackedEntityAnalyticsTableManagerTest.java @@ -44,15 +44,12 @@ import org.hisp.dhis.common.ValueType; import org.hisp.dhis.dataapproval.DataApprovalLevelService; import org.hisp.dhis.db.model.Column; -import org.hisp.dhis.db.model.Logged; import org.hisp.dhis.db.sql.SqlBuilder; import org.hisp.dhis.organisationunit.OrganisationUnitService; import org.hisp.dhis.period.PeriodDataProvider; import org.hisp.dhis.program.Program; import org.hisp.dhis.resourcetable.ResourceTableService; import org.hisp.dhis.setting.SystemSettingsProvider; -import org.hisp.dhis.system.database.DatabaseInfo; -import org.hisp.dhis.system.database.DatabaseInfoProvider; import org.hisp.dhis.system.util.SqlUtils; import org.hisp.dhis.trackedentity.TrackedEntityAttribute; import org.hisp.dhis.trackedentity.TrackedEntityAttributeService; @@ -68,20 +65,34 @@ @ExtendWith(MockitoExtension.class) class JdbcTrackedEntityAnalyticsTableManagerTest { @Mock private JdbcTemplate jdbcTemplate; + @Mock private AnalyticsTableSettings analyticsTableSettings; + @Mock private PeriodDataProvider periodDataProvider; + @Mock private SqlBuilder sqlBuilder; + @Mock private PartitionManager partitionManager; + @Mock private SystemSettingsProvider systemSettingsProvider; + @Mock private IdentifiableObjectManager identifiableObjectManager; + @Mock private TrackedEntityTypeService trackedEntityTypeService; + @Mock private TrackedEntityAttributeService trackedEntityAttributeService; + @Mock private CategoryService categoryService; + + @Mock private AnalyticsTableSettings settings; + @Mock private DataApprovalLevelService dataApprovalLevelService; + @Mock private OrganisationUnitService organisationUnitService; + @Mock private ResourceTableService resourceTableService; + @Mock private AnalyticsTableHookService analyticsTableHookService; - @Mock private DatabaseInfoProvider databaseInfoProvider; @InjectMocks private JdbcTrackedEntityAnalyticsTableManager tableManager; @@ -123,18 +134,8 @@ void verifyNonConfidentialTeasAreSkipped() { when(trackedEntityAttributeService.getProgramTrackedEntityAttributes(List.of(program))) .thenReturn(List.of()); - when(analyticsTableSettings.getTableLogged()).thenReturn(Logged.LOGGED); - when(identifiableObjectManager.getAllNoAcl(Program.class)).thenReturn(List.of(program)); - when(analyticsTableSettings.getTableLogged()).thenReturn(Logged.LOGGED); - - when(identifiableObjectManager.getAllNoAcl(Program.class)).thenReturn(List.of(program)); - - DatabaseInfo databaseInfo = mock(DatabaseInfo.class); - when(databaseInfo.isSpatialSupport()).thenReturn(false); - when(databaseInfoProvider.getDatabaseInfo()).thenReturn(databaseInfo); - List analyticsTables = tableManager.getAnalyticsTables(params); assertEquals(1, analyticsTables.size()); diff --git a/dhis-2/dhis-services/dhis-service-analytics/src/test/java/org/hisp/dhis/analytics/table/JdbcTrackedEntityEventsAnalyticsTableManagerTest.java b/dhis-2/dhis-services/dhis-service-analytics/src/test/java/org/hisp/dhis/analytics/table/JdbcTrackedEntityEventsAnalyticsTableManagerTest.java index 353a3be397c0..bcc61a915cad 100644 --- a/dhis-2/dhis-services/dhis-service-analytics/src/test/java/org/hisp/dhis/analytics/table/JdbcTrackedEntityEventsAnalyticsTableManagerTest.java +++ b/dhis-2/dhis-services/dhis-service-analytics/src/test/java/org/hisp/dhis/analytics/table/JdbcTrackedEntityEventsAnalyticsTableManagerTest.java @@ -58,7 +58,6 @@ import org.hisp.dhis.period.PeriodDataProvider; import org.hisp.dhis.resourcetable.ResourceTableService; import org.hisp.dhis.setting.SystemSettingsProvider; -import org.hisp.dhis.system.database.DatabaseInfoProvider; import org.hisp.dhis.trackedentity.TrackedEntityAttributeService; import org.hisp.dhis.trackedentity.TrackedEntityType; import org.hisp.dhis.trackedentity.TrackedEntityTypeService; @@ -76,21 +75,34 @@ class JdbcTrackedEntityEventsAnalyticsTableManagerTest { @Mock private JdbcTemplate jdbcTemplate; + @Mock private AnalyticsTableSettings analyticsTableSettings; + @Mock private PeriodDataProvider periodDataProvider; + @Spy private PostgreSqlBuilder sqlBuilder; + @Spy private PostgresAnalyticsSqlBuilder analyticsSqlBuilder; + @Mock private PartitionManager partitionManager; + @Mock private SystemSettingsProvider systemSettingsProvider; + @Mock private IdentifiableObjectManager identifiableObjectManager; + @Mock private TrackedEntityTypeService trackedEntityTypeService; + @Mock private TrackedEntityAttributeService trackedEntityAttributeService; + @Mock private CategoryService categoryService; + @Mock private DataApprovalLevelService dataApprovalLevelService; + @Mock private OrganisationUnitService organisationUnitService; + @Mock private ResourceTableService resourceTableService; + @Mock private AnalyticsTableHookService analyticsTableHookService; - @Mock private DatabaseInfoProvider databaseInfoProvider; @InjectMocks private JdbcTrackedEntityEventsAnalyticsTableManager tableManager; @@ -103,7 +115,6 @@ void testTableName() { @Test void testPopulate() { - TrackedEntityType tet = mock(TrackedEntityType.class); when(tet.getUid()).thenReturn("tetUid"); when(trackedEntityTypeService.getAllTrackedEntityType()).thenReturn(List.of(tet)); diff --git a/dhis-2/dhis-support/dhis-support-external/src/main/java/org/hisp/dhis/external/conf/DhisConfigurationProvider.java b/dhis-2/dhis-support/dhis-support-external/src/main/java/org/hisp/dhis/external/conf/DhisConfigurationProvider.java index 3c1cebe2f81f..a8d1932e152d 100644 --- a/dhis-2/dhis-support/dhis-support-external/src/main/java/org/hisp/dhis/external/conf/DhisConfigurationProvider.java +++ b/dhis-2/dhis-support/dhis-support-external/src/main/java/org/hisp/dhis/external/conf/DhisConfigurationProvider.java @@ -38,7 +38,8 @@ import org.hisp.dhis.external.conf.model.GoogleAccessToken; /** - * Provider of DHIS 2 configuration properties specified in the {@code dhis.conf} file. + * Provider of DHIS 2 configuration properties specified in the {@code dhis.conf} file. The property + * values are loaded on application startup and can be assumed to not change during runtime. * * @author Lars Helge Overland */ From dc4e5d05cb9ec5996e8d9ce3f4105775a1dd138e Mon Sep 17 00:00:00 2001 From: Jan Bernitt Date: Tue, 3 Dec 2024 15:12:41 +0100 Subject: [PATCH 2/8] chore: OpenAPI - exclude user property from BaseIdentifiableObject (#19371) --- .../main/java/org/hisp/dhis/common/BaseIdentifiableObject.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dhis-2/dhis-api/src/main/java/org/hisp/dhis/common/BaseIdentifiableObject.java b/dhis-2/dhis-api/src/main/java/org/hisp/dhis/common/BaseIdentifiableObject.java index 55ece67aa490..6eb4166a55c0 100644 --- a/dhis-2/dhis-api/src/main/java/org/hisp/dhis/common/BaseIdentifiableObject.java +++ b/dhis-2/dhis-api/src/main/java/org/hisp/dhis/common/BaseIdentifiableObject.java @@ -360,7 +360,7 @@ public User getCreatedBy() { } @Override - @OpenApi.Property(UserPropertyTransformer.UserDto.class) + @OpenApi.Ignore @JsonProperty @JsonSerialize(using = UserPropertyTransformer.JacksonSerialize.class) @JsonDeserialize(using = UserPropertyTransformer.JacksonDeserialize.class) From 890be9667faca170de8d553122d314e6ce6f2504 Mon Sep 17 00:00:00 2001 From: marc Date: Wed, 4 Dec 2024 09:21:22 +0100 Subject: [PATCH 3/8] task: Rename eventProperty to eventField [DHIS2-18549] (#19373) --- .../EventChangeLog.hbm.xml | 2 +- .../event/DefaultEventChangeLogService.java | 21 ++-- .../tracker/export/event/EventChangeLog.java | 14 +-- .../export/event/EventChangeLogService.java | 2 +- .../event/HibernateEventChangeLogStore.java | 12 +- .../DefaultTrackedEntityChangeLogService.java | 2 +- .../bundle/persister/EventPersister.java | 2 +- ...ame_event_change_log_property_to_field.sql | 3 + .../event/EventChangeLogServiceTest.java | 107 ++++++++---------- .../OrderAndFilterEventChangeLogTest.java | 71 ++++++------ .../tracker/JsonEventChangeLog.java | 10 +- .../EventsExportChangeLogsControllerTest.java | 59 +++++----- .../export/event/EventChangeLogMapper.java | 19 ++-- .../tracker/view/EventChangeLog.java | 6 +- 14 files changed, 156 insertions(+), 174 deletions(-) create mode 100644 dhis-2/dhis-support/dhis-support-db-migration/src/main/resources/org/hisp/dhis/db/migration/2.42/V2_42_33__Rename_event_change_log_property_to_field.sql diff --git a/dhis-2/dhis-services/dhis-service-core/src/main/resources/org/hisp/dhis/eventchangelog.hiberante/EventChangeLog.hbm.xml b/dhis-2/dhis-services/dhis-service-core/src/main/resources/org/hisp/dhis/eventchangelog.hiberante/EventChangeLog.hbm.xml index 7c5376e0273a..830eaab43d77 100644 --- a/dhis-2/dhis-services/dhis-service-core/src/main/resources/org/hisp/dhis/eventchangelog.hiberante/EventChangeLog.hbm.xml +++ b/dhis-2/dhis-services/dhis-service-core/src/main/resources/org/hisp/dhis/eventchangelog.hiberante/EventChangeLog.hbm.xml @@ -18,7 +18,7 @@ - + diff --git a/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/export/event/DefaultEventChangeLogService.java b/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/export/event/DefaultEventChangeLogService.java index e7bb9d8fa0d7..12d76daba7bc 100644 --- a/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/export/event/DefaultEventChangeLogService.java +++ b/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/export/event/DefaultEventChangeLogService.java @@ -99,7 +99,7 @@ public void addDataValueChangeLog( @Override @Transactional - public void addPropertyChangeLog( + public void addFieldChangeLog( @Nonnull Event currentEvent, @Nonnull Event event, @Nonnull String username) { logIfChanged( "occurredAt", Event::getOccurredDate, this::formatDate, currentEvent, event, username); @@ -133,7 +133,7 @@ public Set>> getFilterableFields() { } private void logIfChanged( - String propertyName, + String field, Function valueExtractor, Function formatter, Event currentEvent, @@ -148,34 +148,27 @@ private void logIfChanged( EventChangeLog eventChangeLog = new EventChangeLog( - event, - null, - propertyName, - currentValue, - newValue, - changeLogType, - new Date(), - userName); + event, null, field, currentValue, newValue, changeLogType, new Date(), userName); hibernateEventChangeLogStore.addEventChangeLog(eventChangeLog); } } private ChangeLogType getChangeLogType(String oldValue, String newValue) { - if (isNewProperty(oldValue, newValue)) { + if (isFieldCreated(oldValue, newValue)) { return ChangeLogType.CREATE; - } else if (isUpdateProperty(oldValue, newValue)) { + } else if (isFieldUpdated(oldValue, newValue)) { return ChangeLogType.UPDATE; } else { return ChangeLogType.DELETE; } } - private boolean isNewProperty(String originalValue, String payloadValue) { + private boolean isFieldCreated(String originalValue, String payloadValue) { return originalValue == null && payloadValue != null; } - private boolean isUpdateProperty(String originalValue, String payloadValue) { + private boolean isFieldUpdated(String originalValue, String payloadValue) { return originalValue != null && payloadValue != null; } diff --git a/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/export/event/EventChangeLog.java b/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/export/event/EventChangeLog.java index 676d43e0cee6..31418e879875 100644 --- a/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/export/event/EventChangeLog.java +++ b/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/export/event/EventChangeLog.java @@ -46,7 +46,7 @@ public class EventChangeLog { private DataElement dataElement; - private String eventProperty; + private String eventField; private String previousValue; @@ -63,40 +63,40 @@ public class EventChangeLog { public EventChangeLog( Event event, DataElement dataElement, - String eventProperty, + String eventField, String previousValue, String currentValue, ChangeLogType changeLogType, Date created, String createdByUsername) { - this(event, dataElement, eventProperty, previousValue, currentValue, changeLogType, created); + this(event, dataElement, eventField, previousValue, currentValue, changeLogType, created); this.createdByUsername = createdByUsername; } public EventChangeLog( Event event, DataElement dataElement, - String eventProperty, + String eventField, String previousValue, String currentValue, ChangeLogType changeLogType, Date created, UserInfoSnapshot createdBy) { - this(event, dataElement, eventProperty, previousValue, currentValue, changeLogType, created); + this(event, dataElement, eventField, previousValue, currentValue, changeLogType, created); this.createdBy = createdBy; } private EventChangeLog( Event event, DataElement dataElement, - String eventProperty, + String eventField, String previousValue, String currentValue, ChangeLogType changeLogType, Date created) { this.event = event; this.dataElement = dataElement; - this.eventProperty = eventProperty; + this.eventField = eventField; this.previousValue = previousValue; this.currentValue = currentValue; this.changeLogType = changeLogType; diff --git a/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/export/event/EventChangeLogService.java b/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/export/event/EventChangeLogService.java index e38ffee19a23..032255fb69e4 100644 --- a/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/export/event/EventChangeLogService.java +++ b/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/export/event/EventChangeLogService.java @@ -58,7 +58,7 @@ void addDataValueChangeLog( ChangeLogType changeLogType, String userName); - void addPropertyChangeLog( + void addFieldChangeLog( @Nonnull Event currentEvent, @Nonnull Event event, @Nonnull String userName); void deleteTrackedEntityDataValueChangeLog(Event event); diff --git a/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/export/event/HibernateEventChangeLogStore.java b/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/export/event/HibernateEventChangeLogStore.java index b06bf4523e9d..7f762d67f06c 100644 --- a/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/export/event/HibernateEventChangeLogStore.java +++ b/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/export/event/HibernateEventChangeLogStore.java @@ -56,7 +56,7 @@ public class HibernateEventChangeLogStore { private static final String COLUMN_CHANGELOG_CREATED = "ecl.created"; private static final String COLUMN_CHANGELOG_USER = "ecl.createdByUsername"; private static final String COLUMN_CHANGELOG_DATA_ELEMENT = "d.uid"; - private static final String COLUMN_CHANGELOG_PROPERTY = "ecl.eventProperty"; + private static final String COLUMN_CHANGELOG_FIELD = "ecl.eventField"; private static final String DEFAULT_ORDER = COLUMN_CHANGELOG_CREATED + " " + SortDirection.DESC.getValue(); @@ -73,13 +73,13 @@ public class HibernateEventChangeLogStore { entry("createdAt", COLUMN_CHANGELOG_CREATED), entry("username", COLUMN_CHANGELOG_USER), entry("dataElement", COLUMN_CHANGELOG_DATA_ELEMENT), - entry("property", COLUMN_CHANGELOG_PROPERTY)); + entry("field", COLUMN_CHANGELOG_FIELD)); private static final Map>, String> FILTERABLE_FIELDS = Map.ofEntries( entry(Pair.of("username", String.class), COLUMN_CHANGELOG_USER), entry(Pair.of("dataElement", UID.class), COLUMN_CHANGELOG_DATA_ELEMENT), - entry(Pair.of("property", String.class), COLUMN_CHANGELOG_PROPERTY)); + entry(Pair.of("field", String.class), COLUMN_CHANGELOG_FIELD)); private final EntityManager entityManager; @@ -102,7 +102,7 @@ public Page getEventChangeLogs( """ select ecl.event, ecl.dataElement, - ecl.eventProperty, + ecl.eventField, ecl.previousValue, ecl.currentValue, ecl.changeLogType, @@ -147,7 +147,7 @@ public Page getEventChangeLogs( row -> { Event e = (Event) row[0]; DataElement dataElement = (DataElement) row[1]; - String eventProperty = (String) row[2]; + String eventField = (String) row[2]; String previousValue = (String) row[3]; String currentValue = (String) row[4]; ChangeLogType changeLogType = (ChangeLogType) row[5]; @@ -160,7 +160,7 @@ public Page getEventChangeLogs( return new EventChangeLog( e, dataElement, - eventProperty, + eventField, previousValue, currentValue, changeLogType, diff --git a/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/export/trackedentity/DefaultTrackedEntityChangeLogService.java b/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/export/trackedentity/DefaultTrackedEntityChangeLogService.java index b8e62d53d466..95e2dbaf6edb 100644 --- a/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/export/trackedentity/DefaultTrackedEntityChangeLogService.java +++ b/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/export/trackedentity/DefaultTrackedEntityChangeLogService.java @@ -215,6 +215,6 @@ private Set validateTrackedEntityAttributes(TrackedEntity trackedEntity) throw new NotFoundException(TrackedEntity.class, trackedEntity.getUid()); } - return attributes.stream().map(a -> UID.of(a.getUid())).collect(Collectors.toSet()); + return attributes.stream().map(UID::of).collect(Collectors.toSet()); } } diff --git a/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/imports/bundle/persister/EventPersister.java b/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/imports/bundle/persister/EventPersister.java index db746f8c4953..12d9cdcc009e 100644 --- a/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/imports/bundle/persister/EventPersister.java +++ b/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/imports/bundle/persister/EventPersister.java @@ -170,7 +170,7 @@ protected void updateDataValues( Event currentEntity, UserDetails user) { handleDataValues(entityManager, preheat, event.getDataValues(), payloadEntity, user); - eventChangeLogService.addPropertyChangeLog(currentEntity, payloadEntity, user.getUsername()); + eventChangeLogService.addFieldChangeLog(currentEntity, payloadEntity, user.getUsername()); } private void handleDataValues( diff --git a/dhis-2/dhis-support/dhis-support-db-migration/src/main/resources/org/hisp/dhis/db/migration/2.42/V2_42_33__Rename_event_change_log_property_to_field.sql b/dhis-2/dhis-support/dhis-support-db-migration/src/main/resources/org/hisp/dhis/db/migration/2.42/V2_42_33__Rename_event_change_log_property_to_field.sql new file mode 100644 index 000000000000..524d6c15a009 --- /dev/null +++ b/dhis-2/dhis-support/dhis-support-db-migration/src/main/resources/org/hisp/dhis/db/migration/2.42/V2_42_33__Rename_event_change_log_property_to_field.sql @@ -0,0 +1,3 @@ +-- DHIS2-18549 + +alter table eventchangelog rename eventproperty to eventfield; \ No newline at end of file diff --git a/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/tracker/export/event/EventChangeLogServiceTest.java b/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/tracker/export/event/EventChangeLogServiceTest.java index b94c64b7a610..4d38c31842b3 100644 --- a/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/tracker/export/event/EventChangeLogServiceTest.java +++ b/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/tracker/export/event/EventChangeLogServiceTest.java @@ -292,26 +292,25 @@ void shouldReturnOnlyUserNameWhenUserDoesNotExistInDatabase() } @Test - void shouldReturnEventPropertiesChangeLogWhenNewDatePropertyValueAdded() + void shouldReturnEventFieldChangeLogWhenNewDateFieldValueAdded() throws ForbiddenException, NotFoundException { String event = "QRYjLTiJTrA"; Page changeLogs = eventChangeLogService.getEventChangeLog( UID.of(event), defaultOperationParams, defaultPageParams); - List scheduledAtLogs = getChangeLogsByProperty(changeLogs, "scheduledAt"); - List occurredAtLogs = getChangeLogsByProperty(changeLogs, "occurredAt"); + List scheduledAtLogs = getChangeLogsByField(changeLogs, "scheduledAt"); + List occurredAtLogs = getChangeLogsByField(changeLogs, "occurredAt"); assertNumberOfChanges(1, scheduledAtLogs); assertNumberOfChanges(1, occurredAtLogs); assertAll( - () -> - assertPropertyCreate("scheduledAt", "2022-04-22 06:00:38.343", scheduledAtLogs.get(0)), - () -> assertPropertyCreate("occurredAt", "2022-04-20 06:00:38.343", occurredAtLogs.get(0))); + () -> assertFieldCreate("scheduledAt", "2022-04-22 06:00:38.343", scheduledAtLogs.get(0)), + () -> assertFieldCreate("occurredAt", "2022-04-20 06:00:38.343", occurredAtLogs.get(0))); } @Test - void shouldReturnEventPropertiesChangeLogWhenExistingDatePropertyUpdated() + void shouldReturnEventFieldChangeLogWhenExistingDateFieldUpdated() throws IOException, ForbiddenException, NotFoundException { UID event = UID.of("QRYjLTiJTrA"); LocalDateTime currentTime = LocalDateTime.now(); @@ -320,31 +319,30 @@ void shouldReturnEventPropertiesChangeLogWhenExistingDatePropertyUpdated() Page changeLogs = eventChangeLogService.getEventChangeLog(event, defaultOperationParams, defaultPageParams); - List scheduledAtLogs = getChangeLogsByProperty(changeLogs, "scheduledAt"); - List occurredAtLogs = getChangeLogsByProperty(changeLogs, "occurredAt"); + List scheduledAtLogs = getChangeLogsByField(changeLogs, "scheduledAt"); + List occurredAtLogs = getChangeLogsByField(changeLogs, "occurredAt"); assertNumberOfChanges(2, scheduledAtLogs); assertNumberOfChanges(2, occurredAtLogs); assertAll( () -> - assertPropertyUpdate( + assertFieldUpdate( "scheduledAt", "2022-04-22 06:00:38.343", currentTime.toString(formatter), scheduledAtLogs.get(0)), + () -> assertFieldCreate("scheduledAt", "2022-04-22 06:00:38.343", scheduledAtLogs.get(1)), () -> - assertPropertyCreate("scheduledAt", "2022-04-22 06:00:38.343", scheduledAtLogs.get(1)), - () -> - assertPropertyUpdate( + assertFieldUpdate( "occurredAt", "2022-04-20 06:00:38.343", currentTime.toString(formatter), occurredAtLogs.get(0)), - () -> assertPropertyCreate("occurredAt", "2022-04-20 06:00:38.343", occurredAtLogs.get(1))); + () -> assertFieldCreate("occurredAt", "2022-04-20 06:00:38.343", occurredAtLogs.get(1))); } @Test - void shouldReturnEventPropertiesChangeLogWhenExistingDatePropertyDeleted() + void shouldReturnEventFieldChangeLogWhenExistingDateFieldDeleted() throws ForbiddenException, NotFoundException { UID event = UID.of("QRYjLTiJTrA"); @@ -352,56 +350,53 @@ void shouldReturnEventPropertiesChangeLogWhenExistingDatePropertyDeleted() Page changeLogs = eventChangeLogService.getEventChangeLog(event, defaultOperationParams, defaultPageParams); - List scheduledAtLogs = getChangeLogsByProperty(changeLogs, "scheduledAt"); - List occurredAtLogs = getChangeLogsByProperty(changeLogs, "occurredAt"); + List scheduledAtLogs = getChangeLogsByField(changeLogs, "scheduledAt"); + List occurredAtLogs = getChangeLogsByField(changeLogs, "occurredAt"); assertNumberOfChanges(2, scheduledAtLogs); assertNumberOfChanges(1, occurredAtLogs); assertAll( - () -> - assertPropertyDelete("scheduledAt", "2022-04-22 06:00:38.343", scheduledAtLogs.get(0)), - () -> - assertPropertyCreate("scheduledAt", "2022-04-22 06:00:38.343", scheduledAtLogs.get(1)), - () -> assertPropertyCreate("occurredAt", "2022-04-20 06:00:38.343", occurredAtLogs.get(0))); + () -> assertFieldDelete("scheduledAt", "2022-04-22 06:00:38.343", scheduledAtLogs.get(0)), + () -> assertFieldCreate("scheduledAt", "2022-04-22 06:00:38.343", scheduledAtLogs.get(1)), + () -> assertFieldCreate("occurredAt", "2022-04-20 06:00:38.343", occurredAtLogs.get(0))); } @Test - void shouldReturnEventPropertiesChangeLogWhenNewGeometryPointPropertyValueAdded() + void shouldReturnEventFieldChangeLogWhenNewGeometryPointFieldValueAdded() throws ForbiddenException, NotFoundException { String event = "QRYjLTiJTrA"; Page changeLogs = eventChangeLogService.getEventChangeLog( UID.of(event), defaultOperationParams, defaultPageParams); - List geometryChangeLogs = getChangeLogsByProperty(changeLogs, "geometry"); + List geometryChangeLogs = getChangeLogsByField(changeLogs, "geometry"); assertNumberOfChanges(1, geometryChangeLogs); assertAll( - () -> - assertPropertyCreate("geometry", "(-11.419700, 8.103900)", geometryChangeLogs.get(0))); + () -> assertFieldCreate("geometry", "(-11.419700, 8.103900)", geometryChangeLogs.get(0))); } @Test - void shouldReturnEventPropertiesChangeLogWhenNewGeometryPolygonPropertyValueAdded() + void shouldReturnEventFieldChangeLogWhenNewGeometryPolygonFieldValueAdded() throws ForbiddenException, NotFoundException { String event = "YKmfzHdjUDL"; Page changeLogs = eventChangeLogService.getEventChangeLog( UID.of(event), defaultOperationParams, defaultPageParams); - List geometryChangeLogs = getChangeLogsByProperty(changeLogs, "geometry"); + List geometryChangeLogs = getChangeLogsByField(changeLogs, "geometry"); assertNumberOfChanges(1, geometryChangeLogs); assertAll( () -> - assertPropertyCreate( + assertFieldCreate( "geometry", "(-11.416855, 8.132308), (-11.445351, 8.089312), (-11.383896, 8.089652), (-11.416855, 8.132308)", geometryChangeLogs.get(0))); } @Test - void shouldReturnEventPropertiesChangeLogWhenExistingGeometryPointPropertyUpdated() + void shouldReturnEventFieldChangeLogWhenExistingGeometryPointFieldUpdated() throws ForbiddenException, NotFoundException { UID event = UID.of("QRYjLTiJTrA"); @@ -410,22 +405,21 @@ void shouldReturnEventPropertiesChangeLogWhenExistingGeometryPointPropertyUpdate Page changeLogs = eventChangeLogService.getEventChangeLog(event, defaultOperationParams, defaultPageParams); - List geometryChangeLogs = getChangeLogsByProperty(changeLogs, "geometry"); + List geometryChangeLogs = getChangeLogsByField(changeLogs, "geometry"); assertNumberOfChanges(2, geometryChangeLogs); assertAll( () -> - assertPropertyUpdate( + assertFieldUpdate( "geometry", "(-11.419700, 8.103900)", "(16.435547, 49.264220)", geometryChangeLogs.get(0)), - () -> - assertPropertyCreate("geometry", "(-11.419700, 8.103900)", geometryChangeLogs.get(1))); + () -> assertFieldCreate("geometry", "(-11.419700, 8.103900)", geometryChangeLogs.get(1))); } @Test - void shouldReturnEventPropertiesChangeLogWhenExistingGeometryPointPropertyDeleted() + void shouldReturnEventFieldChangeLogWhenExistingGeometryPointFieldDeleted() throws ForbiddenException, NotFoundException { UID event = UID.of("QRYjLTiJTrA"); @@ -433,13 +427,12 @@ void shouldReturnEventPropertiesChangeLogWhenExistingGeometryPointPropertyDelete Page changeLogs = eventChangeLogService.getEventChangeLog(event, defaultOperationParams, defaultPageParams); - List geometryChangeLogs = getChangeLogsByProperty(changeLogs, "geometry"); + List geometryChangeLogs = getChangeLogsByField(changeLogs, "geometry"); assertNumberOfChanges(2, geometryChangeLogs); assertAll( - () -> assertPropertyDelete("geometry", "(-11.419700, 8.103900)", geometryChangeLogs.get(0)), - () -> - assertPropertyCreate("geometry", "(-11.419700, 8.103900)", geometryChangeLogs.get(1))); + () -> assertFieldDelete("geometry", "(-11.419700, 8.103900)", geometryChangeLogs.get(0)), + () -> assertFieldCreate("geometry", "(-11.419700, 8.103900)", geometryChangeLogs.get(1))); } private void updateDataValue(String event, String dataElementUid, String newValue) { @@ -554,12 +547,11 @@ private void assertDataElementCreate( () -> assertDataElementChange(dataElement, null, currentValue, changeLog)); } - private void assertPropertyCreate( - String property, String currentValue, EventChangeLog changeLog) { + private void assertFieldCreate(String field, String currentValue, EventChangeLog changeLog) { assertAll( () -> assertUser(importUser, changeLog), () -> assertEquals("CREATE", changeLog.getChangeLogType().name()), - () -> assertPropertyChange(property, null, currentValue, changeLog)); + () -> assertFieldChange(field, null, currentValue, changeLog)); } private void assertDataElementUpdate( @@ -567,14 +559,14 @@ private void assertDataElementUpdate( assertUpdate(dataElement, null, previousValue, currentValue, changeLog, importUser); } - private void assertPropertyUpdate( - String property, String previousValue, String currentValue, EventChangeLog changeLog) { - assertUpdate(null, property, previousValue, currentValue, changeLog, importUser); + private void assertFieldUpdate( + String field, String previousValue, String currentValue, EventChangeLog changeLog) { + assertUpdate(null, field, previousValue, currentValue, changeLog, importUser); } private void assertUpdate( String dataElement, - String property, + String field, String previousValue, String currentValue, EventChangeLog changeLog, @@ -586,7 +578,7 @@ private void assertUpdate( if (dataElement != null) { assertDataElementChange(dataElement, previousValue, currentValue, changeLog); } else { - assertPropertyChange(property, previousValue, currentValue, changeLog); + assertFieldChange(field, previousValue, currentValue, changeLog); } }); } @@ -596,13 +588,12 @@ private void assertDataElementDelete( assertDelete(dataElement, null, previousValue, changeLog); } - private void assertPropertyDelete( - String property, String previousValue, EventChangeLog changeLog) { - assertDelete(null, property, previousValue, changeLog); + private void assertFieldDelete(String field, String previousValue, EventChangeLog changeLog) { + assertDelete(null, field, previousValue, changeLog); } private void assertDelete( - String dataElement, String property, String previousValue, EventChangeLog changeLog) { + String dataElement, String field, String previousValue, EventChangeLog changeLog) { assertAll( () -> assertUser(importUser, changeLog), () -> assertEquals("DELETE", changeLog.getChangeLogType().name()), @@ -610,7 +601,7 @@ private void assertDelete( if (dataElement != null) { assertDataElementChange(dataElement, previousValue, null, changeLog); } else { - assertPropertyChange(property, previousValue, null, changeLog); + assertFieldChange(field, previousValue, null, changeLog); } }); } @@ -624,9 +615,9 @@ private static void assertDataElementChange( assertEquals(currentValue, changeLog.getCurrentValue()); } - private static void assertPropertyChange( - String property, String previousValue, String currentValue, EventChangeLog changeLog) { - assertEquals(property, changeLog.getEventProperty()); + private static void assertFieldChange( + String field, String previousValue, String currentValue, EventChangeLog changeLog) { + assertEquals(field, changeLog.getEventField()); assertEquals(previousValue, changeLog.getPreviousValue()); assertEquals(currentValue, changeLog.getCurrentValue()); } @@ -652,10 +643,10 @@ private List getDataElementChangeLogs(Page chang return changeLogs.getItems().stream().filter(cl -> cl.getDataElement() != null).toList(); } - private List getChangeLogsByProperty( - Page changeLogs, String propertyName) { + private List getChangeLogsByField( + Page changeLogs, String fieldName) { return changeLogs.getItems().stream() - .filter(cl -> cl.getEventProperty() != null && cl.getEventProperty().equals(propertyName)) + .filter(cl -> cl.getEventField() != null && cl.getEventField().equals(fieldName)) .toList(); } diff --git a/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/tracker/export/event/OrderAndFilterEventChangeLogTest.java b/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/tracker/export/event/OrderAndFilterEventChangeLogTest.java index 2a8c3b5ca371..943e3318264c 100644 --- a/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/tracker/export/event/OrderAndFilterEventChangeLogTest.java +++ b/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/tracker/export/event/OrderAndFilterEventChangeLogTest.java @@ -198,71 +198,71 @@ void shouldSortChangeLogsWhenOrderingByDataElementDesc() } @Test - void shouldSortChangeLogsWhenOrderingByPropertyAsc() + void shouldSortChangeLogsWhenOrderingByFieldAsc() throws ForbiddenException, NotFoundException, IOException { EventChangeLogOperationParams params = - EventChangeLogOperationParams.builder().orderBy("property", SortDirection.ASC).build(); + EventChangeLogOperationParams.builder().orderBy("field", SortDirection.ASC).build(); UID event = UID.of("QRYjLTiJTrA"); LocalDateTime currentTime = LocalDateTime.now(); updateEventDates(event, currentTime.toDate().toInstant()); List changeLogs = - getAllPropertyChangeLogs( + getAllFieldChangeLogs( eventChangeLogService.getEventChangeLog( UID.of("QRYjLTiJTrA"), params, defaultPageParams)); assertNumberOfChanges(5, changeLogs); assertAll( - () -> assertPropertyCreate("geometry", "(-11.419700, 8.103900)", changeLogs.get(0)), + () -> assertFieldCreate("geometry", "(-11.419700, 8.103900)", changeLogs.get(0)), () -> - assertPropertyUpdate( + assertFieldUpdate( "occurredAt", "2022-04-20 06:00:38.343", currentTime.toString(formatter), changeLogs.get(1)), - () -> assertPropertyCreate("occurredAt", "2022-04-20 06:00:38.343", changeLogs.get(2)), + () -> assertFieldCreate("occurredAt", "2022-04-20 06:00:38.343", changeLogs.get(2)), () -> - assertPropertyUpdate( + assertFieldUpdate( "scheduledAt", "2022-04-22 06:00:38.343", currentTime.toString(formatter), changeLogs.get(3)), - () -> assertPropertyCreate("scheduledAt", "2022-04-22 06:00:38.343", changeLogs.get(4))); + () -> assertFieldCreate("scheduledAt", "2022-04-22 06:00:38.343", changeLogs.get(4))); } @Test - void shouldSortChangeLogsWhenOrderingByPropertyDesc() + void shouldSortChangeLogsWhenOrderingByFieldDesc() throws ForbiddenException, NotFoundException, IOException { EventChangeLogOperationParams params = - EventChangeLogOperationParams.builder().orderBy("property", SortDirection.DESC).build(); + EventChangeLogOperationParams.builder().orderBy("field", SortDirection.DESC).build(); UID event = UID.of("QRYjLTiJTrA"); LocalDateTime currentTime = LocalDateTime.now(); updateEventDates(event, currentTime.toDate().toInstant()); List changeLogs = - getAllPropertyChangeLogs( + getAllFieldChangeLogs( eventChangeLogService.getEventChangeLog( UID.of("QRYjLTiJTrA"), params, defaultPageParams)); assertNumberOfChanges(5, changeLogs); assertAll( () -> - assertPropertyUpdate( + assertFieldUpdate( "scheduledAt", "2022-04-22 06:00:38.343", currentTime.toString(formatter), changeLogs.get(0)), - () -> assertPropertyCreate("scheduledAt", "2022-04-22 06:00:38.343", changeLogs.get(1)), + () -> assertFieldCreate("scheduledAt", "2022-04-22 06:00:38.343", changeLogs.get(1)), () -> - assertPropertyUpdate( + assertFieldUpdate( "occurredAt", "2022-04-20 06:00:38.343", currentTime.toString(formatter), changeLogs.get(2)), - () -> assertPropertyCreate("occurredAt", "2022-04-20 06:00:38.343", changeLogs.get(3)), - () -> assertPropertyCreate("geometry", "(-11.419700, 8.103900)", changeLogs.get(4))); + () -> assertFieldCreate("occurredAt", "2022-04-20 06:00:38.343", changeLogs.get(3)), + () -> assertFieldCreate("geometry", "(-11.419700, 8.103900)", changeLogs.get(4))); } @Test @@ -302,28 +302,28 @@ void shouldFilterChangeLogsWhenFilteringByDataElement() assertContainsOnly(List.of(dataElement), changeLogDataElements); } - private Stream provideEventProperties() { + private Stream provideEventField() { return Stream.of( Arguments.of("occurredAt"), Arguments.of("scheduledAt"), Arguments.of("geometry")); } @ParameterizedTest - @MethodSource("provideEventProperties") - void shouldFilterChangeLogsWhenFilteringByProperties(String filterValue) + @MethodSource("provideEventField") + void shouldFilterChangeLogsWhenFilteringByField(String filterValue) throws ForbiddenException, NotFoundException { EventChangeLogOperationParams params = EventChangeLogOperationParams.builder() - .filterBy("property", new QueryFilter(QueryOperator.EQ, filterValue)) + .filterBy("field", new QueryFilter(QueryOperator.EQ, filterValue)) .build(); Page changeLogs = eventChangeLogService.getEventChangeLog(UID.of("QRYjLTiJTrA"), params, defaultPageParams); - Set changeLogOccurredAtProperties = + Set changeLogOccurredAtFields = changeLogs.getItems().stream() - .map(EventChangeLog::getEventProperty) + .map(EventChangeLog::getEventField) .collect(Collectors.toSet()); - assertContainsOnly(List.of(filterValue), changeLogOccurredAtProperties); + assertContainsOnly(List.of(filterValue), changeLogOccurredAtFields); } private void updateDataValue(String event, String dataElementUid, String newValue) { @@ -385,12 +385,11 @@ private void assertDataElementCreate( () -> assertDataElementChange(dataElement, null, currentValue, changeLog)); } - private void assertPropertyCreate( - String property, String currentValue, EventChangeLog changeLog) { + private void assertFieldCreate(String field, String currentValue, EventChangeLog changeLog) { assertAll( () -> assertUser(importUser, changeLog), () -> assertEquals("CREATE", changeLog.getChangeLogType().name()), - () -> assertPropertyChange(property, null, currentValue, changeLog)); + () -> assertFieldChange(field, null, currentValue, changeLog)); } private void assertDataElementUpdate( @@ -398,14 +397,14 @@ private void assertDataElementUpdate( assertUpdate(dataElement, null, previousValue, currentValue, changeLog, importUser); } - private void assertPropertyUpdate( - String property, String previousValue, String currentValue, EventChangeLog changeLog) { - assertUpdate(null, property, previousValue, currentValue, changeLog, importUser); + private void assertFieldUpdate( + String field, String previousValue, String currentValue, EventChangeLog changeLog) { + assertUpdate(null, field, previousValue, currentValue, changeLog, importUser); } private void assertUpdate( String dataElement, - String property, + String field, String previousValue, String currentValue, EventChangeLog changeLog, @@ -417,7 +416,7 @@ private void assertUpdate( if (dataElement != null) { assertDataElementChange(dataElement, previousValue, currentValue, changeLog); } else { - assertPropertyChange(property, previousValue, currentValue, changeLog); + assertFieldChange(field, previousValue, currentValue, changeLog); } }); } @@ -431,9 +430,9 @@ private static void assertDataElementChange( assertEquals(currentValue, changeLog.getCurrentValue()); } - private static void assertPropertyChange( - String property, String previousValue, String currentValue, EventChangeLog changeLog) { - assertEquals(property, changeLog.getEventProperty()); + private static void assertFieldChange( + String field, String previousValue, String currentValue, EventChangeLog changeLog) { + assertEquals(field, changeLog.getEventField()); assertEquals(previousValue, changeLog.getPreviousValue()); assertEquals(currentValue, changeLog.getCurrentValue()); } @@ -459,8 +458,8 @@ private List getDataElementChangeLogs(Page chang return changeLogs.getItems().stream().filter(cl -> cl.getDataElement() != null).toList(); } - private List getAllPropertyChangeLogs(Page changeLogs) { - return changeLogs.getItems().stream().filter(cl -> cl.getEventProperty() != null).toList(); + private List getAllFieldChangeLogs(Page changeLogs) { + return changeLogs.getItems().stream().filter(cl -> cl.getEventField() != null).toList(); } private void updateDataValues(Event event, String dataElementUid, String... values) { diff --git a/dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/controller/tracker/JsonEventChangeLog.java b/dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/controller/tracker/JsonEventChangeLog.java index 638a7949365d..1b40f86c4d52 100644 --- a/dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/controller/tracker/JsonEventChangeLog.java +++ b/dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/controller/tracker/JsonEventChangeLog.java @@ -49,8 +49,8 @@ default JsonDataValue getDataValue() { return get("dataValue").as(JsonDataValue.class); } - default JsonEventProperty getEventProperty() { - return get("eventProperty").as(JsonEventProperty.class); + default JsonEventField getEventField() { + return get("eventField").as(JsonEventField.class); } } @@ -68,9 +68,9 @@ default String getCurrentValue() { } } - interface JsonEventProperty extends JsonObject { - default String getProperty() { - return getString("property").string(); + interface JsonEventField extends JsonObject { + default String getField() { + return getString("field").string(); } default String getPreviousValue() { diff --git a/dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/controller/tracker/export/event/EventsExportChangeLogsControllerTest.java b/dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/controller/tracker/export/event/EventsExportChangeLogsControllerTest.java index 6a87881c6a73..99c1a84e95c4 100644 --- a/dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/controller/tracker/export/event/EventsExportChangeLogsControllerTest.java +++ b/dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/controller/tracker/export/event/EventsExportChangeLogsControllerTest.java @@ -174,24 +174,23 @@ void shouldGetEventChangeLogInDescOrderByDefault() { changeLogs.stream() .filter(log -> log.getChange().getDataValue().getDataElement() != null) .toList(); - List eventPropertyChangeLogs = + List eventFieldChangeLogs = changeLogs.stream() - .filter(log -> log.getChange().getEventProperty().getProperty() != null) + .filter(log -> log.getChange().getEventField().getField() != null) .toList(); assertHasSize(3, dataValueChangeLogs); - assertHasSize(2, eventPropertyChangeLogs); + assertHasSize(2, eventFieldChangeLogs); assertAll( () -> assertDelete(dataElement, "value 3", dataValueChangeLogs.get(0)), () -> assertUpdate(dataElement, "value 2", "value 3", dataValueChangeLogs.get(1)), () -> assertUpdate(dataElement, "value 1", "value 2", dataValueChangeLogs.get(2)), () -> - assertPropertyCreateExists( - "occurredAt", "2023-01-10 00:00:00.000", eventPropertyChangeLogs), + assertFieldCreateExists("occurredAt", "2023-01-10 00:00:00.000", eventFieldChangeLogs), () -> - assertPropertyCreateExists( - "scheduledAt", "2023-01-10 00:00:00.000", eventPropertyChangeLogs)); + assertFieldCreateExists( + "scheduledAt", "2023-01-10 00:00:00.000", eventFieldChangeLogs)); } @Test @@ -204,41 +203,39 @@ void shouldGetEventChangeLogInAscOrder() { changeLogs.stream() .filter(log -> log.getChange().getDataValue().getDataElement() != null) .toList(); - List eventPropertyChangeLogs = + List eventFieldChangeLogs = changeLogs.stream() - .filter(log -> log.getChange().getEventProperty().getProperty() != null) + .filter(log -> log.getChange().getEventField().getField() != null) .toList(); assertHasSize(3, dataValueChangeLogs); - assertHasSize(2, eventPropertyChangeLogs); + assertHasSize(2, eventFieldChangeLogs); assertAll( () -> assertUpdate(dataElement, "value 1", "value 2", dataValueChangeLogs.get(0)), () -> assertUpdate(dataElement, "value 2", "value 3", dataValueChangeLogs.get(1)), () -> assertDelete(dataElement, "value 3", dataValueChangeLogs.get(2)), () -> - assertPropertyCreateExists( - "occurredAt", "2023-01-10 00:00:00.000", eventPropertyChangeLogs), + assertFieldCreateExists("occurredAt", "2023-01-10 00:00:00.000", eventFieldChangeLogs), () -> - assertPropertyCreateExists( - "scheduledAt", "2023-01-10 00:00:00.000", eventPropertyChangeLogs)); + assertFieldCreateExists( + "scheduledAt", "2023-01-10 00:00:00.000", eventFieldChangeLogs)); } @Test - void shouldGetEventChangeLogsWhenFilteringByProperty() { + void shouldGetEventChangeLogsWhenFilteringByField() { JsonList changeLogs = - GET("/tracker/events/{id}/changeLogs?filter=property:eq:occurredAt", event.getUid()) + GET("/tracker/events/{id}/changeLogs?filter=field:eq:occurredAt", event.getUid()) .content(HttpStatus.OK) .getList("changeLogs", JsonEventChangeLog.class); - List eventPropertyChangeLogs = + List eventFieldChangeLogs = changeLogs.stream() - .filter(log -> log.getChange().getEventProperty().getProperty() != null) + .filter(log -> log.getChange().getEventField().getField() != null) .toList(); assertAll( - () -> assertHasSize(1, eventPropertyChangeLogs), + () -> assertHasSize(1, eventFieldChangeLogs), () -> - assertPropertyCreateExists( - "occurredAt", "2023-01-10 00:00:00.000", eventPropertyChangeLogs)); + assertFieldCreateExists("occurredAt", "2023-01-10 00:00:00.000", eventFieldChangeLogs)); } @Test @@ -479,7 +476,7 @@ private static void assertChange( assertEquals(dataElement.getUid(), actual.getChange().getDataValue().getDataElement()), () -> assertEquals(previousValue, actual.getChange().getDataValue().getPreviousValue()), () -> assertEquals(currentValue, actual.getChange().getDataValue().getCurrentValue()), - () -> JsonAssertions.assertHasNoMember(actual.getChange(), "eventProperty")); + () -> JsonAssertions.assertHasNoMember(actual.getChange(), "eventField")); } private static void assertPagerLink(String actual, int page, int pageSize, String start) { @@ -490,12 +487,12 @@ private static void assertPagerLink(String actual, int page, int pageSize, Strin () -> assertContains("pageSize=" + pageSize, actual)); } - private static void assertPropertyCreateExists( - String property, String currentValue, List changeLogs) { + private static void assertFieldCreateExists( + String field, String currentValue, List changeLogs) { assertTrue( - changeLogs.stream().anyMatch(cl -> isEventPropertyCreate(cl, property, currentValue)), + changeLogs.stream().anyMatch(cl -> isEventFieldCreate(cl, field, currentValue)), "Expected a " - + property + + field + " change with value " + currentValue + " among the change log entries."); @@ -504,11 +501,11 @@ private static void assertPropertyCreateExists( "Data value change not expected to be present, but it was"); } - private static boolean isEventPropertyCreate( - JsonEventChangeLog actual, String property, String currentValue) { + private static boolean isEventFieldCreate( + JsonEventChangeLog actual, String field, String currentValue) { return actual.getType().equals(ChangeLogType.CREATE.name()) - && actual.getChange().getEventProperty().getProperty().equals(property) - && actual.getChange().getEventProperty().getCurrentValue().equals(currentValue) - && actual.getChange().getEventProperty().getPreviousValue() == null; + && actual.getChange().getEventField().getField().equals(field) + && actual.getChange().getEventField().getCurrentValue().equals(currentValue) + && actual.getChange().getEventField().getPreviousValue() == null; } } diff --git a/dhis-2/dhis-web-api/src/main/java/org/hisp/dhis/webapi/controller/tracker/export/event/EventChangeLogMapper.java b/dhis-2/dhis-web-api/src/main/java/org/hisp/dhis/webapi/controller/tracker/export/event/EventChangeLogMapper.java index 9600f3d6d927..96fbf2a93d78 100644 --- a/dhis-2/dhis-web-api/src/main/java/org/hisp/dhis/webapi/controller/tracker/export/event/EventChangeLogMapper.java +++ b/dhis-2/dhis-web-api/src/main/java/org/hisp/dhis/webapi/controller/tracker/export/event/EventChangeLogMapper.java @@ -29,7 +29,7 @@ import org.hisp.dhis.webapi.controller.tracker.view.EventChangeLog; import org.hisp.dhis.webapi.controller.tracker.view.EventChangeLog.DataValueChange; -import org.hisp.dhis.webapi.controller.tracker.view.EventChangeLog.PropertyChange; +import org.hisp.dhis.webapi.controller.tracker.view.EventChangeLog.FieldChange; import org.hisp.dhis.webapi.controller.tracker.view.UIDMapper; import org.hisp.dhis.webapi.controller.tracker.view.User; import org.mapstruct.Mapper; @@ -47,9 +47,9 @@ public interface EventChangeLogMapper { source = "eventChangeLog", qualifiedByName = "mapIfDataValueChangeExists") @Mapping( - target = "change.eventProperty", + target = "change.eventField", source = "eventChangeLog", - qualifiedByName = "mapIfEventPropertyChangeExists") + qualifiedByName = "mapIfEventFieldChangeExists") EventChangeLog map(org.hisp.dhis.tracker.export.event.EventChangeLog eventChangeLog); @Mapping(target = "uid", source = "createdBy.uid") @@ -73,18 +73,17 @@ default DataValueChange mapIfDataValueChangeExists( return mapDataValueChange(eventChangeLog); } - @Mapping(target = "property", source = "eventProperty") + @Mapping(target = "field", source = "eventField") @Mapping(target = "previousValue", source = "previousValue") @Mapping(target = "currentValue", source = "currentValue") - PropertyChange mapEventPropertyChange( - org.hisp.dhis.tracker.export.event.EventChangeLog eventChangeLog); + FieldChange mapEventFieldChange(org.hisp.dhis.tracker.export.event.EventChangeLog eventChangeLog); - @Named("mapIfEventPropertyChangeExists") - default PropertyChange mapIfEventPropertyExists( + @Named("mapIfEventFieldChangeExists") + default FieldChange mapIfEventFieldExists( org.hisp.dhis.tracker.export.event.EventChangeLog eventChangeLog) { - if (eventChangeLog.getEventProperty() == null) { + if (eventChangeLog.getEventField() == null) { return null; } - return mapEventPropertyChange(eventChangeLog); + return mapEventFieldChange(eventChangeLog); } } diff --git a/dhis-2/dhis-web-api/src/main/java/org/hisp/dhis/webapi/controller/tracker/view/EventChangeLog.java b/dhis-2/dhis-web-api/src/main/java/org/hisp/dhis/webapi/controller/tracker/view/EventChangeLog.java index 22cc0fbbf563..73af417eb76b 100644 --- a/dhis-2/dhis-web-api/src/main/java/org/hisp/dhis/webapi/controller/tracker/view/EventChangeLog.java +++ b/dhis-2/dhis-web-api/src/main/java/org/hisp/dhis/webapi/controller/tracker/view/EventChangeLog.java @@ -38,15 +38,15 @@ public record EventChangeLog( @JsonProperty Change change) { public record Change( - @JsonProperty DataValueChange dataValue, @JsonProperty PropertyChange eventProperty) {} + @JsonProperty DataValueChange dataValue, @JsonProperty FieldChange eventField) {} public record DataValueChange( @JsonProperty UID dataElement, @JsonProperty String previousValue, @JsonProperty String currentValue) {} - public record PropertyChange( - @JsonProperty String property, + public record FieldChange( + @JsonProperty String field, @JsonProperty String previousValue, @JsonProperty String currentValue) {} } From e1f4f6e766326ddaa4dfae81b3d39d0fe1bd0be5 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Wed, 4 Dec 2024 09:36:57 +0100 Subject: [PATCH 4/8] chore(deps-dev): bump com.nimbusds:nimbus-jose-jwt in /dhis-2 (#19376) Bumps [com.nimbusds:nimbus-jose-jwt](https://bitbucket.org/connect2id/nimbus-jose-jwt) from 9.46 to 9.47. - [Changelog](https://bitbucket.org/connect2id/nimbus-jose-jwt/src/master/CHANGELOG.txt) - [Commits](https://bitbucket.org/connect2id/nimbus-jose-jwt/branches/compare/9.47..9.46) --- updated-dependencies: - dependency-name: com.nimbusds:nimbus-jose-jwt dependency-type: direct:development update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- dhis-2/pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dhis-2/pom.xml b/dhis-2/pom.xml index 538fe525a6cd..84c9480bc34e 100644 --- a/dhis-2/pom.xml +++ b/dhis-2/pom.xml @@ -98,7 +98,7 @@ 1.1.1.RELEASE 1.70 1.9.3 - 9.46 + 9.47 3.5.3 3.5.3 From df4ea0718fd180a275a80d701160eda54e1c7c2d Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Wed, 4 Dec 2024 09:37:32 +0100 Subject: [PATCH 5/8] chore(deps): bump org.flywaydb:flyway-database-postgresql in /dhis-2 (#19375) Bumps org.flywaydb:flyway-database-postgresql from 11.0.0 to 11.0.1. --- updated-dependencies: - dependency-name: org.flywaydb:flyway-database-postgresql dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- dhis-2/pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dhis-2/pom.xml b/dhis-2/pom.xml index 84c9480bc34e..ba7158e0a26c 100644 --- a/dhis-2/pom.xml +++ b/dhis-2/pom.xml @@ -115,7 +115,7 @@ 6.5.1.RELEASE - 11.0.0 + 11.0.1 5.6.15.Final 3.10.8 4.0.5 From 3784ef642547d27a6f08600a58d682c6e85ca753 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lars=20Helge=20=C3=98verland?= Date: Wed, 4 Dec 2024 10:46:38 +0100 Subject: [PATCH 6/8] fix: Revert commit dc0fa5e (#19378) --- .../dhis/common/DimensionalItemObject.java | 4 - .../org/hisp/dhis/feedback/ErrorCode.java | 3 - .../java/org/hisp/dhis/program/Program.java | 34 +++- .../org/hisp/dhis/program/ProgramTest.java | 5 + .../analytics/event/EventQueryParams.java | 5 - ...tEnrollmentAnalyticsDimensionsService.java | 41 ++--- ...efaultEventAnalyticsDimensionsService.java | 15 +- .../data/DefaultEventDataQueryService.java | 69 -------- .../table/AbstractEventJdbcTableManager.java | 2 +- .../JdbcEnrollmentAnalyticsTableManager.java | 2 +- .../table/JdbcEventAnalyticsTableManager.java | 8 +- ...dbcTrackedEntityAnalyticsTableManager.java | 12 +- .../DefaultEventDataQueryServiceTest.java | 151 ------------------ ...rackedEntityAnalyticsTableManagerTest.java | 7 +- .../analytics/AnalyticsDimensionsTest.java | 2 +- 15 files changed, 89 insertions(+), 271 deletions(-) delete mode 100644 dhis-2/dhis-services/dhis-service-analytics/src/test/java/org/hisp/dhis/analytics/event/data/DefaultEventDataQueryServiceTest.java diff --git a/dhis-2/dhis-api/src/main/java/org/hisp/dhis/common/DimensionalItemObject.java b/dhis-2/dhis-api/src/main/java/org/hisp/dhis/common/DimensionalItemObject.java index 4134b7314387..c12c4e954df7 100644 --- a/dhis-2/dhis-api/src/main/java/org/hisp/dhis/common/DimensionalItemObject.java +++ b/dhis-2/dhis-api/src/main/java/org/hisp/dhis/common/DimensionalItemObject.java @@ -87,8 +87,4 @@ public interface DimensionalItemObject extends NameableObject { default int getPeriodOffset() { return (getQueryMods() != null) ? getQueryMods().getPeriodOffset() : 0; } - - default boolean isOfType(DimensionItemType type) { - return type == getDimensionItemType(); - } } diff --git a/dhis-2/dhis-api/src/main/java/org/hisp/dhis/feedback/ErrorCode.java b/dhis-2/dhis-api/src/main/java/org/hisp/dhis/feedback/ErrorCode.java index 9dcf3008b561..1c1cf44d0afe 100644 --- a/dhis-2/dhis-api/src/main/java/org/hisp/dhis/feedback/ErrorCode.java +++ b/dhis-2/dhis-api/src/main/java/org/hisp/dhis/feedback/ErrorCode.java @@ -505,9 +505,6 @@ public enum ErrorCode { E7236("Program stage '{0}' is not associated to program '{0}'"), E7237("Sorting must have a valid dimension and a direction"), E7238("Sorting dimension ‘{0}’ is not a column"), - E7239( - "Tracked Entity Attributes marked as 'confidential' can only be used in aggregate analytics: `{0}`"), - E7240("Data Elements marked as 'skipAnalytics' can only be used in aggregate analytics: `{0}`"), /* TE analytics */ E7250("Dimension is not a fully qualified: `{0}`"), diff --git a/dhis-2/dhis-api/src/main/java/org/hisp/dhis/program/Program.java b/dhis-2/dhis-api/src/main/java/org/hisp/dhis/program/Program.java index 8d9b8da95009..d6ca3575963d 100644 --- a/dhis-2/dhis-api/src/main/java/org/hisp/dhis/program/Program.java +++ b/dhis-2/dhis-api/src/main/java/org/hisp/dhis/program/Program.java @@ -37,9 +37,11 @@ import com.fasterxml.jackson.dataformat.xml.annotation.JacksonXmlProperty; import com.fasterxml.jackson.dataformat.xml.annotation.JacksonXmlRootElement; import java.util.ArrayList; +import java.util.Collection; import java.util.HashSet; import java.util.List; import java.util.Map; +import java.util.Objects; import java.util.Set; import java.util.stream.Collectors; import org.hisp.dhis.category.CategoryCombo; @@ -260,12 +262,26 @@ public Set getDataElements() { return programStages.stream().flatMap(ps -> ps.getDataElements().stream()).collect(toSet()); } + /** + * Returns all data elements which are part of the stages of this program and is not skipped in + * analytics. + */ + public Set getAnalyticsDataElements() { + return programStages.stream() + .map(ProgramStage::getProgramStageDataElements) + .flatMap(Collection::stream) + .filter(Objects::nonNull) + .filter(psde -> !psde.getSkipAnalytics()) + .map(ProgramStageDataElement::getDataElement) + .collect(toSet()); + } + /** * Returns data elements which are part of the stages of this program which have a legend set and * is of numeric value type. */ - public Set getDataElementsWithLegendSet() { - return getDataElements().stream() + public Set getAnalyticsDataElementsWithLegendSet() { + return getAnalyticsDataElements().stream() .filter(de -> de.hasLegendSet() && de.isNumericType()) .collect(toSet()); } @@ -280,13 +296,23 @@ public List getTrackedEntityAttributes() { .collect(Collectors.toList()); } + /** + * Returns non-confidential TrackedEntityAttributes from ProgramTrackedEntityAttributes. Use + * getAttributes() to access the persisted attribute list. + */ + public List getNonConfidentialTrackedEntityAttributes() { + return getTrackedEntityAttributes().stream() + .filter(a -> !a.isConfidentialBool()) + .collect(Collectors.toList()); + } + /** * Returns TrackedEntityAttributes from ProgramTrackedEntityAttributes which have a legend set and * is of numeric value type. */ - public List getTrackedEntityAttributesWithLegendSet() { + public List getNonConfidentialTrackedEntityAttributesWithLegendSet() { return getTrackedEntityAttributes().stream() - .filter(a -> a.hasLegendSet() && a.isNumericType()) + .filter(a -> !a.isConfidentialBool() && a.hasLegendSet() && a.isNumericType()) .collect(Collectors.toList()); } diff --git a/dhis-2/dhis-api/src/test/java/org/hisp/dhis/program/ProgramTest.java b/dhis-2/dhis-api/src/test/java/org/hisp/dhis/program/ProgramTest.java index 7f7cbff8216b..3555686ada7e 100644 --- a/dhis-2/dhis-api/src/test/java/org/hisp/dhis/program/ProgramTest.java +++ b/dhis-2/dhis-api/src/test/java/org/hisp/dhis/program/ProgramTest.java @@ -85,6 +85,7 @@ void testGetAnalyticsDataElements() { assertEquals(2, prA.getDataElements().size()); assertTrue(prA.getDataElements().contains(deA)); assertTrue(prA.getDataElements().contains(deB)); + assertEquals(1, prA.getAnalyticsDataElements().size()); assertTrue(prA.getDataElements().contains(deA)); } @@ -105,6 +106,7 @@ void testCopyOfWithPropertyValuesSet() { // check equal assertEquals(original.getAccess(), copy.getAccess()); assertEquals(original.getAccessLevel(), copy.getAccessLevel()); + assertEquals(original.getAnalyticsDataElements(), copy.getAnalyticsDataElements()); assertEquals(original.getCategoryCombo(), copy.getCategoryCombo()); assertEquals(original.getCompleteEventsExpiryDays(), copy.getCompleteEventsExpiryDays()); assertEquals(original.getDataElements(), copy.getDataElements()); @@ -178,7 +180,10 @@ void testCopyOfWithNulls() { assertEquals("copynull", copy.getName()); assertEquals(original.getAccessLevel(), copy.getAccessLevel()); assertEquals(original.getDescription(), copy.getDescription()); + assertTrue(copy.getAnalyticsDataElements().isEmpty()); assertTrue(copy.getDataElements().isEmpty()); + assertTrue(copy.getNonConfidentialTrackedEntityAttributes().isEmpty()); + assertTrue(copy.getNonConfidentialTrackedEntityAttributesWithLegendSet().isEmpty()); assertTrue(copy.getNotificationTemplates().isEmpty()); assertTrue(copy.getOrganisationUnits().isEmpty()); assertTrue(copy.getProgramAttributes().isEmpty()); diff --git a/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/event/EventQueryParams.java b/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/event/EventQueryParams.java index 7848156e434d..4d6f0683f691 100644 --- a/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/event/EventQueryParams.java +++ b/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/event/EventQueryParams.java @@ -37,7 +37,6 @@ import static org.hisp.dhis.common.DimensionalObject.PERIOD_DIM_ID; import static org.hisp.dhis.common.DimensionalObjectUtils.asList; import static org.hisp.dhis.common.DimensionalObjectUtils.asTypedList; -import static org.hisp.dhis.common.RequestTypeAware.EndpointAction.AGGREGATE; import static org.hisp.dhis.common.RequestTypeAware.EndpointAction.QUERY; import com.google.common.base.MoreObjects; @@ -999,10 +998,6 @@ public boolean isRowContext() { return rowContext; } - public boolean includeConfidentialOrSkipAnalyticsItems() { - return endpointAction == AGGREGATE; - } - // ------------------------------------------------------------------------- // Builder of immutable instances // ------------------------------------------------------------------------- diff --git a/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/event/data/DefaultEnrollmentAnalyticsDimensionsService.java b/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/event/data/DefaultEnrollmentAnalyticsDimensionsService.java index c13b40d3d4df..2894d3771eb6 100644 --- a/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/event/data/DefaultEnrollmentAnalyticsDimensionsService.java +++ b/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/event/data/DefaultEnrollmentAnalyticsDimensionsService.java @@ -31,26 +31,22 @@ import static org.hisp.dhis.analytics.common.DimensionsServiceCommon.OperationType.QUERY; import static org.hisp.dhis.analytics.common.DimensionsServiceCommon.collectDimensions; import static org.hisp.dhis.analytics.common.DimensionsServiceCommon.filterByValueType; -import static org.hisp.dhis.analytics.event.data.DefaultEventAnalyticsDimensionsService.getTeasIfRegistration; import static org.hisp.dhis.common.PrefixedDimensions.ofItemsWithProgram; import static org.hisp.dhis.common.PrefixedDimensions.ofProgramStageDataElements; import java.util.Collection; import java.util.List; import java.util.Optional; -import java.util.Set; -import java.util.function.Predicate; import java.util.stream.Collectors; import lombok.RequiredArgsConstructor; import org.hisp.dhis.analytics.common.DimensionsServiceCommon; -import org.hisp.dhis.analytics.common.DimensionsServiceCommon.OperationType; import org.hisp.dhis.analytics.event.EnrollmentAnalyticsDimensionsService; import org.hisp.dhis.common.PrefixedDimension; import org.hisp.dhis.program.Program; import org.hisp.dhis.program.ProgramService; import org.hisp.dhis.program.ProgramStage; -import org.hisp.dhis.program.ProgramStageDataElement; import org.hisp.dhis.security.acl.AclService; +import org.hisp.dhis.trackedentity.TrackedEntityAttribute; import org.hisp.dhis.user.CurrentUserUtil; import org.hisp.dhis.user.UserDetails; import org.springframework.stereotype.Service; @@ -81,7 +77,9 @@ public List getQueryDimensionsByProgramId(String programId) { .collect(Collectors.toSet())), getProgramStageDataElements(QUERY, program), filterByValueType( - QUERY, ofItemsWithProgram(program, getTeasIfRegistration(program)))))) + QUERY, + ofItemsWithProgram( + program, getTeasIfRegistrationAndNotConfidential(program)))))) .orElse(List.of()); } @@ -89,22 +87,14 @@ private Collection getProgramStageDataElements( DimensionsServiceCommon.OperationType operationType, Program program) { return program.getProgramStages().stream() .map(ProgramStage::getProgramStageDataElements) - .map(psdes -> excludeIfSkipAnalytics(operationType, psdes)) - .map(psdes -> filterByValueType(operationType, ofProgramStageDataElements(psdes))) + .map( + programStageDataElements -> + filterByValueType( + operationType, ofProgramStageDataElements(programStageDataElements))) .flatMap(Collection::stream) .collect(Collectors.toList()); } - private Set excludeIfSkipAnalytics( - OperationType operationType, Set programStageDataElements) { - if (operationType == QUERY) { - return programStageDataElements.stream() - .filter(Predicate.not(ProgramStageDataElement::getSkipAnalytics)) - .collect(Collectors.toSet()); - } - return programStageDataElements; - } - @Override public List getAggregateDimensionsByProgramStageId(String programId) { return Optional.of(programId) @@ -119,4 +109,19 @@ public List getAggregateDimensionsByProgramStageId(String pro ofItemsWithProgram(program, program.getTrackedEntityAttributes()))))) .orElse(List.of()); } + + private Collection getTeasIfRegistrationAndNotConfidential( + Program program) { + return Optional.of(program) + .filter(Program::isRegistration) + .map(Program::getTrackedEntityAttributes) + .orElse(List.of()) + .stream() + .filter(this::isNotConfidential) + .collect(Collectors.toList()); + } + + private boolean isNotConfidential(TrackedEntityAttribute trackedEntityAttribute) { + return !trackedEntityAttribute.isConfidentialBool(); + } } diff --git a/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/event/data/DefaultEventAnalyticsDimensionsService.java b/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/event/data/DefaultEventAnalyticsDimensionsService.java index a67b341b61df..76086a2c4b27 100644 --- a/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/event/data/DefaultEventAnalyticsDimensionsService.java +++ b/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/event/data/DefaultEventAnalyticsDimensionsService.java @@ -135,7 +135,9 @@ private List dimensions(ProgramStage programStage) { .filter(pi -> aclService.canRead(currentUserDetails, pi)) .collect(Collectors.toSet())), filterByValueType(QUERY, ofDataElements(programStage)), - filterByValueType(QUERY, ofItemsWithProgram(p, getTeasIfRegistration(p))), + filterByValueType( + QUERY, + ofItemsWithProgram(p, getTeasIfRegistrationAndNotConfidential(p))), ofItemsWithProgram(p, getCategories(p)), ofItemsWithProgram(p, getAttributeCategoryOptionGroupSetsIfNeeded(p))))) .orElse(List.of()); @@ -185,10 +187,17 @@ private List getCategories(Program program) { .orElse(List.of()); } - static List getTeasIfRegistration(Program program) { + private List getTeasIfRegistrationAndNotConfidential(Program program) { return Optional.of(program) .filter(Program::isRegistration) .map(Program::getTrackedEntityAttributes) - .orElse(List.of()); + .orElse(List.of()) + .stream() + .filter(this::isNotConfidential) + .collect(Collectors.toList()); + } + + private boolean isNotConfidential(TrackedEntityAttribute trackedEntityAttribute) { + return !trackedEntityAttribute.isConfidentialBool(); } } 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 3d20c79d5a61..b08af886985a 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 @@ -36,8 +36,6 @@ 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.DimensionItemType.DATA_ELEMENT; -import static org.hisp.dhis.common.DimensionItemType.PROGRAM_ATTRIBUTE; import static org.hisp.dhis.common.DimensionalObject.DIMENSION_NAME_SEP; import static org.hisp.dhis.common.DimensionalObject.PERIOD_DIM_ID; import static org.hisp.dhis.common.DimensionalObjectUtils.getDimensionFromParam; @@ -54,7 +52,6 @@ import java.util.Set; import java.util.UUID; import java.util.stream.Collectors; -import java.util.stream.Stream; import lombok.Getter; import lombok.RequiredArgsConstructor; import org.apache.commons.lang3.StringUtils; @@ -70,7 +67,6 @@ import org.hisp.dhis.analytics.table.EnrollmentAnalyticsColumnName; import org.hisp.dhis.analytics.table.EventAnalyticsColumnName; import org.hisp.dhis.common.BaseDimensionalItemObject; -import org.hisp.dhis.common.BaseIdentifiableObject; import org.hisp.dhis.common.DimensionalItemObject; import org.hisp.dhis.common.DimensionalObject; import org.hisp.dhis.common.EventAnalyticalObject; @@ -94,7 +90,6 @@ import org.hisp.dhis.program.Program; import org.hisp.dhis.program.ProgramService; import org.hisp.dhis.program.ProgramStage; -import org.hisp.dhis.program.ProgramStageDataElement; import org.hisp.dhis.program.ProgramStageService; import org.hisp.dhis.setting.UserSettings; import org.hisp.dhis.trackedentity.TrackedEntityAttribute; @@ -228,73 +223,9 @@ public EventQueryParams getFromRequest(EventDataQueryRequest request, boolean an eventQueryParams = builder.build(); } - validateQueryParamsForConfidentialAndSkipAnalytics(eventQueryParams); - return eventQueryParams; } - static void validateQueryParamsForConfidentialAndSkipAnalytics( - EventQueryParams eventQueryParams) { - if (eventQueryParams.includeConfidentialOrSkipAnalyticsItems()) { - return; - } - Set confidentialAttributes = - Stream.concat( - eventQueryParams.getItems().stream(), eventQueryParams.getItemFilters().stream()) - .map(QueryItem::getItem) - .filter(Objects::nonNull) - .filter(dimObj -> dimObj.isOfType(PROGRAM_ATTRIBUTE)) - .map(TrackedEntityAttribute.class::cast) - .filter(TrackedEntityAttribute::isConfidentialBool) - .collect(Collectors.toSet()); - - if (!confidentialAttributes.isEmpty()) { - throw new IllegalQueryException( - new ErrorMessage( - ErrorCode.E7239, - confidentialAttributes.stream() - .map(TrackedEntityAttribute::getUid) - .collect(Collectors.joining(", ")))); - } - - Set skipAnalyticsDataElements = - Stream.concat( - eventQueryParams.getItems().stream(), eventQueryParams.getItemFilters().stream()) - .filter(item -> item.getItem().isOfType(DATA_ELEMENT)) - .filter(DefaultEventDataQueryService::isSkipAnalytics) - .map(item -> (DataElement) item.getItem()) - .collect(Collectors.toSet()); - - if (!skipAnalyticsDataElements.isEmpty()) { - throw new IllegalQueryException( - new ErrorMessage( - ErrorCode.E7240, - skipAnalyticsDataElements.stream() - .map(DataElement::getUid) - .collect(Collectors.joining(", ")))); - } - } - - /** - * Checks if the data element is marked as skip analytics by looking at the program stage data - * elements associated with the program stage. If any of the program stage data elements has the - * skip analytics flag set to true, the data element in it is considered to be skipAnalytics. - * - * @param item the query item - * @return true if the data element is marked as skip analytics, false otherwise - */ - static boolean isSkipAnalytics(QueryItem item) { - return Optional.of(item) - .map(QueryItem::getProgramStage) - .map(ProgramStage::getProgramStageDataElements) - .orElse(Set.of()) - .stream() - .filter(ProgramStageDataElement::getSkipAnalytics) - .map(ProgramStageDataElement::getDataElement) - .map(BaseIdentifiableObject::getUid) - .anyMatch(uid -> uid.equals(item.getItem().getUid())); - } - private boolean hasPeriodDimension(EventQueryParams eventQueryParams) { return Objects.nonNull(getPeriodDimension(eventQueryParams)); } diff --git a/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/table/AbstractEventJdbcTableManager.java b/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/table/AbstractEventJdbcTableManager.java index 5179b999407f..c164dc6d6989 100644 --- a/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/table/AbstractEventJdbcTableManager.java +++ b/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/table/AbstractEventJdbcTableManager.java @@ -214,7 +214,7 @@ protected void populateTableInternal(AnalyticsTablePartition partition, String f * Returns a list of columns based on the given attribute. * * @param attribute the {@link TrackedEntityAttribute}. - * @return a list of {@link AnalyticsTableColumn}. + * @return a list of {@link AnaylyticsTableColumn}. */ protected List getColumnForAttribute(TrackedEntityAttribute attribute) { List columns = new ArrayList<>(); diff --git a/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/table/JdbcEnrollmentAnalyticsTableManager.java b/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/table/JdbcEnrollmentAnalyticsTableManager.java index 59bdf500f0a5..d18fbdffd80b 100644 --- a/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/table/JdbcEnrollmentAnalyticsTableManager.java +++ b/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/table/JdbcEnrollmentAnalyticsTableManager.java @@ -187,7 +187,7 @@ private List getColumns(Program program) { * @return a list of {@link AnalyticsTableColumn}. */ private List getTrackedEntityAttributeColumns(Program program) { - return program.getTrackedEntityAttributes().stream() + return program.getNonConfidentialTrackedEntityAttributes().stream() .map(this::getColumnForAttribute) .flatMap(Collection::stream) .toList(); diff --git a/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/table/JdbcEventAnalyticsTableManager.java b/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/table/JdbcEventAnalyticsTableManager.java index 4b0b572b390c..7a4ff2e0ad6c 100644 --- a/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/table/JdbcEventAnalyticsTableManager.java +++ b/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/table/JdbcEventAnalyticsTableManager.java @@ -461,12 +461,12 @@ private List getAttributeCategoryColumns(Program program) private List getDataElementColumns(Program program) { List columns = new ArrayList<>(); columns.addAll( - program.getDataElements().stream() + program.getAnalyticsDataElements().stream() .map(de -> getColumnForDataElement(de, false)) .flatMap(Collection::stream) .toList()); columns.addAll( - program.getDataElementsWithLegendSet().stream() + program.getAnalyticsDataElementsWithLegendSet().stream() .map(de -> getColumnForDataElement(de, true)) .flatMap(Collection::stream) .toList()); @@ -567,12 +567,12 @@ private List getColumnForOrgUnitDataElement( private List getAttributeColumns(Program program) { List columns = new ArrayList<>(); columns.addAll( - program.getTrackedEntityAttributes().stream() + program.getNonConfidentialTrackedEntityAttributes().stream() .map(this::getColumnForAttribute) .flatMap(Collection::stream) .toList()); columns.addAll( - program.getTrackedEntityAttributesWithLegendSet().stream() + program.getNonConfidentialTrackedEntityAttributesWithLegendSet().stream() .map(this::getColumnForAttributeWithLegendSet) .flatMap(Collection::stream) .toList()); diff --git a/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/table/JdbcTrackedEntityAnalyticsTableManager.java b/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/table/JdbcTrackedEntityAnalyticsTableManager.java index e2d0bdb9e5ac..4a483d156d37 100644 --- a/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/table/JdbcTrackedEntityAnalyticsTableManager.java +++ b/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/table/JdbcTrackedEntityAnalyticsTableManager.java @@ -88,7 +88,8 @@ public class JdbcTrackedEntityAnalyticsTableManager extends AbstractJdbcTableManager { private static final String PROGRAMS_BY_TET_KEY = "programsByTetUid"; - private static final String ALL_TET_ATTRIBUTES = "allTetAttributes"; + private static final String ALL_NON_CONFIDENTIAL_TET_ATTRIBUTES = + "allNonConfidentialTetAttributes"; private final TrackedEntityTypeService trackedEntityTypeService; @@ -199,9 +200,12 @@ private List getColumns( .build())); List trackedEntityAttributes = - getAllTrackedEntityAttributes(trackedEntityType, programsByTetUid).toList(); + getAllTrackedEntityAttributes(trackedEntityType, programsByTetUid) + .filter(tea -> !tea.isConfidentialBool()) + .toList(); - params.addExtraParam(trackedEntityType.getUid(), ALL_TET_ATTRIBUTES, trackedEntityAttributes); + params.addExtraParam( + trackedEntityType.getUid(), ALL_NON_CONFIDENTIAL_TET_ATTRIBUTES, trackedEntityAttributes); columns.addAll( trackedEntityAttributes.stream() @@ -351,7 +355,7 @@ public void populateTable(AnalyticsTableUpdateParams params, AnalyticsTableParti Map.of())); ((List) - params.getExtraParam(trackedEntityType.getUid(), ALL_TET_ATTRIBUTES)) + params.getExtraParam(trackedEntityType.getUid(), ALL_NON_CONFIDENTIAL_TET_ATTRIBUTES)) .forEach( tea -> sql.append( diff --git a/dhis-2/dhis-services/dhis-service-analytics/src/test/java/org/hisp/dhis/analytics/event/data/DefaultEventDataQueryServiceTest.java b/dhis-2/dhis-services/dhis-service-analytics/src/test/java/org/hisp/dhis/analytics/event/data/DefaultEventDataQueryServiceTest.java deleted file mode 100644 index 0a54a25dc6aa..000000000000 --- a/dhis-2/dhis-services/dhis-service-analytics/src/test/java/org/hisp/dhis/analytics/event/data/DefaultEventDataQueryServiceTest.java +++ /dev/null @@ -1,151 +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.analytics.event.data; - -import static org.hisp.dhis.analytics.event.data.DefaultEventDataQueryService.validateQueryParamsForConfidentialAndSkipAnalytics; -import static org.hisp.dhis.common.DimensionItemType.DATA_ELEMENT; -import static org.hisp.dhis.common.DimensionItemType.PROGRAM_ATTRIBUTE; -import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; -import static org.junit.jupiter.api.Assertions.assertThrows; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.when; - -import java.util.List; -import java.util.Set; -import java.util.function.Consumer; -import org.hisp.dhis.analytics.event.EventQueryParams; -import org.hisp.dhis.common.DimensionItemType; -import org.hisp.dhis.common.DimensionalItemObject; -import org.hisp.dhis.common.IllegalQueryException; -import org.hisp.dhis.common.QueryItem; -import org.hisp.dhis.dataelement.DataElement; -import org.hisp.dhis.program.ProgramStage; -import org.hisp.dhis.program.ProgramStageDataElement; -import org.hisp.dhis.trackedentity.TrackedEntityAttribute; -import org.junit.jupiter.api.Test; - -class DefaultEventDataQueryServiceTest { - - private EventQueryParams mockEventQueryParams( - boolean includeConfidentialOrSkipAnalyticsItems, List queryItems) { - EventQueryParams mock = mock(EventQueryParams.class); - when(mock.includeConfidentialOrSkipAnalyticsItems()) - .thenReturn(includeConfidentialOrSkipAnalyticsItems); - when(mock.getItems()).thenReturn(queryItems); - when(mock.getItemFilters()).thenReturn(List.of()); - return mock; - } - - private QueryItem mockDimensionalItemObject( - Class clazz, DimensionItemType type, List> behavious) { - T mock = mock(clazz); - when(mock.isOfType(type)).thenReturn(true); - QueryItem queryItem = mock(QueryItem.class); - when(queryItem.getItem()).thenReturn(mock); - behavious.forEach(c -> c.accept(mock)); - return queryItem; - } - - @Test - void testAggregateDontThrowExceptionForConfidential() { - QueryItem queryItem = - mockDimensionalItemObject( - TrackedEntityAttribute.class, - PROGRAM_ATTRIBUTE, - List.of(t -> when(t.isConfidentialBool()).thenReturn(true))); - EventQueryParams eventQueryParams = mockEventQueryParams(true, List.of(queryItem)); - assertDoesNotThrow(() -> validateQueryParamsForConfidentialAndSkipAnalytics(eventQueryParams)); - } - - @Test - void testAggregateDontThrowExceptionForSkipAnalytics() { - final String dataElementUid = "dataElementUid"; - - ProgramStage programStage = mock(ProgramStage.class); - ProgramStageDataElement programStageDataElement = mock(ProgramStageDataElement.class); - - when(programStage.getProgramStageDataElements()).thenReturn(Set.of(programStageDataElement)); - when(programStageDataElement.getSkipAnalytics()).thenReturn(true); - - DataElement dataElement = mock(DataElement.class); - when(dataElement.getUid()).thenReturn(dataElementUid); - when(programStageDataElement.getDataElement()).thenReturn(dataElement); - - QueryItem queryItem = - mockDimensionalItemObject( - DataElement.class, - DATA_ELEMENT, - List.of(t -> when(t.getUid()).thenReturn(dataElementUid))); - when(queryItem.getProgramStage()).thenReturn(programStage); - - EventQueryParams eventQueryParams = mockEventQueryParams(true, List.of(queryItem)); - - assertDoesNotThrow(() -> validateQueryParamsForConfidentialAndSkipAnalytics(eventQueryParams)); - } - - @Test - void testQueryThrowsForConfidential() { - QueryItem queryItem = - mockDimensionalItemObject( - TrackedEntityAttribute.class, - PROGRAM_ATTRIBUTE, - List.of(t -> when(t.isConfidentialBool()).thenReturn(true))); - EventQueryParams eventQueryParams = mockEventQueryParams(false, List.of(queryItem)); - assertThrows( - IllegalQueryException.class, - () -> validateQueryParamsForConfidentialAndSkipAnalytics(eventQueryParams)); - } - - @Test - void testQueryThrowsForSkipAnalytics() { - final String dataElementUid = "dataElementUid"; - - ProgramStage programStage = mock(ProgramStage.class); - ProgramStageDataElement programStageDataElement = mock(ProgramStageDataElement.class); - - when(programStage.getProgramStageDataElements()).thenReturn(Set.of(programStageDataElement)); - when(programStageDataElement.getSkipAnalytics()).thenReturn(true); - - DataElement dataElement = mock(DataElement.class); - when(dataElement.getUid()).thenReturn(dataElementUid); - when(programStageDataElement.getDataElement()).thenReturn(dataElement); - - QueryItem queryItem = - mockDimensionalItemObject( - DataElement.class, - DATA_ELEMENT, - List.of(t -> when(t.getUid()).thenReturn(dataElementUid))); - when(queryItem.getProgramStage()).thenReturn(programStage); - - EventQueryParams eventQueryParams = mockEventQueryParams(false, List.of(queryItem)); - - assertThrows( - IllegalQueryException.class, - () -> validateQueryParamsForConfidentialAndSkipAnalytics(eventQueryParams)); - } -} diff --git a/dhis-2/dhis-services/dhis-service-analytics/src/test/java/org/hisp/dhis/analytics/table/JdbcTrackedEntityAnalyticsTableManagerTest.java b/dhis-2/dhis-services/dhis-service-analytics/src/test/java/org/hisp/dhis/analytics/table/JdbcTrackedEntityAnalyticsTableManagerTest.java index b80504d26b7e..0d537f2e63da 100644 --- a/dhis-2/dhis-services/dhis-service-analytics/src/test/java/org/hisp/dhis/analytics/table/JdbcTrackedEntityAnalyticsTableManagerTest.java +++ b/dhis-2/dhis-services/dhis-service-analytics/src/test/java/org/hisp/dhis/analytics/table/JdbcTrackedEntityAnalyticsTableManagerTest.java @@ -28,6 +28,7 @@ package org.hisp.dhis.analytics.table; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.Mockito.mock; @@ -143,13 +144,13 @@ void verifyNonConfidentialTeasAreSkipped() { AnalyticsTable analyticsTable = analyticsTables.get(0); assertContainsNonConfidentialTeaColumns(analyticsTable); - assertContainsConfidentialTeaColumns(analyticsTable); + assertDoesntContainConfidentialTeaColumns(analyticsTable); } - private void assertContainsConfidentialTeaColumns(AnalyticsTable analyticsTable) { + private void assertDoesntContainConfidentialTeaColumns(AnalyticsTable analyticsTable) { List columns = analyticsTable.getColumns(); - assertTrue(columns.stream().map(Column::getName).anyMatch("confidentialTeaUid"::equals)); + assertFalse(columns.stream().map(Column::getName).anyMatch("confidentialTeaUid"::equals)); } private void assertContainsNonConfidentialTeaColumns(AnalyticsTable analyticsTable) { 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 821adf4c0230..08811f890abf 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 @@ -183,7 +183,7 @@ public void shouldOnlyReturnConfidentialAttributeInAggregateDimensions() { .query() .getDimensionsByDimensionType(trackerProgram.getUid(), "PROGRAM_ATTRIBUTE") .validate() - .body("dimensions.uid", CoreMatchers.hasItem(confidentialAttribute)); + .body("dimensions.uid", not(CoreMatchers.hasItem(confidentialAttribute))); analyticsEnrollmentsActions .aggregate() From 8fcf3bfb5cc80b474cf5fd8b37f2e538878f2801 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lars=20Helge=20=C3=98verland?= Date: Wed, 4 Dec 2024 12:33:41 +0100 Subject: [PATCH 7/8] refactor: Join attribute value table for attributes [DHIS2-18417] (#19299) --- .../table/AbstractEventJdbcTableManager.java | 51 +++++++++++++++---- .../JdbcEnrollmentAnalyticsTableManager.java | 3 ++ .../table/JdbcEventAnalyticsTableManager.java | 31 ++++++----- .../AbstractEventJdbcTableManagerTest.java | 10 ++-- ...bcEnrollmentAnalyticsTableManagerTest.java | 8 +-- .../JdbcEventAnalyticsTableManagerTest.java | 27 ++-------- 6 files changed, 73 insertions(+), 57 deletions(-) diff --git a/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/table/AbstractEventJdbcTableManager.java b/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/table/AbstractEventJdbcTableManager.java index c164dc6d6989..543c686379ac 100644 --- a/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/table/AbstractEventJdbcTableManager.java +++ b/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/table/AbstractEventJdbcTableManager.java @@ -38,6 +38,7 @@ import java.util.ArrayList; import java.util.List; import java.util.Map; +import java.util.stream.Collectors; import org.hisp.dhis.analytics.AnalyticsTableHookService; import org.hisp.dhis.analytics.partition.PartitionManager; import org.hisp.dhis.analytics.table.model.AnalyticsDimensionType; @@ -46,6 +47,7 @@ import org.hisp.dhis.analytics.table.model.Skip; import org.hisp.dhis.analytics.table.setting.AnalyticsTableSettings; import org.hisp.dhis.category.CategoryService; +import org.hisp.dhis.common.IdentifiableObject; import org.hisp.dhis.common.IdentifiableObjectManager; import org.hisp.dhis.common.ValueType; import org.hisp.dhis.commons.util.TextUtils; @@ -55,6 +57,7 @@ import org.hisp.dhis.db.sql.SqlBuilder; import org.hisp.dhis.organisationunit.OrganisationUnitService; import org.hisp.dhis.period.PeriodDataProvider; +import org.hisp.dhis.program.Program; import org.hisp.dhis.resourcetable.ResourceTableService; import org.hisp.dhis.setting.SystemSettingsProvider; import org.hisp.dhis.trackedentity.TrackedEntityAttribute; @@ -117,14 +120,13 @@ protected Skip skipIndex(ValueType valueType, boolean hasOptionSet) { } /** - * Returns a select expression, potentially with a cast statement, based on the given value type. - * Handles data element and tracked entity attribute select expressions. + * Returns a column expression, potentially with a cast statement, based on the given value type. * * @param valueType the {@link ValueType} to represent as database column type. * @param columnExpression the expression or name of the column to be selected. * @return a select expression appropriate for the given value type and context. */ - protected String getSelectExpression(ValueType valueType, String columnExpression) { + protected String getColumnExpression(ValueType valueType, String columnExpression) { if (valueType.isDecimal()) { return getCastExpression(columnExpression, NUMERIC_REGEXP, sqlBuilder.dataTypeDouble()); } else if (valueType.isInteger()) { @@ -138,7 +140,8 @@ protected String getSelectExpression(ValueType valueType, String columnExpressio } else if (valueType.isGeo() && isSpatialSupport()) { return String.format( """ - ST_GeomFromGeoJSON('{"type":"Point", "coordinates":' || (%s) || ', "crs":{"type":"name", "properties":{"name":"EPSG:4326"}}}')""", + ST_GeomFromGeoJSON('{"type":"Point", "coordinates":' || (%s) || \ + ', "crs":{"type":"name", "properties":{"name":"EPSG:4326"}}}')""", columnExpression); } else { return columnExpression; @@ -219,14 +222,14 @@ protected void populateTableInternal(AnalyticsTablePartition partition, String f protected List getColumnForAttribute(TrackedEntityAttribute attribute) { List columns = new ArrayList<>(); + String valueColumn = String.format("%s.%s", quote(attribute.getUid()), "value"); DataType dataType = getColumnType(attribute.getValueType(), isSpatialSupport()); - String selectExpression = getSelectExpression(attribute.getValueType(), "value"); + String selectExpression = getColumnExpression(attribute.getValueType(), valueColumn); String dataFilterClause = getDataFilterClause(attribute); - String sql = getSelectSubquery(attribute, selectExpression, dataFilterClause); Skip skipIndex = skipIndex(attribute.getValueType(), attribute.hasOptionSet()); if (attribute.getValueType().isOrganisationUnit()) { - columns.addAll(getColumnForOrgUnitTrackedEntityAttribute(attribute, dataFilterClause)); + columns.addAll(getColumnForOrgUnitAttribute(attribute, dataFilterClause)); } columns.add( @@ -234,7 +237,7 @@ protected List getColumnForAttribute(TrackedEntityAttribut .name(attribute.getUid()) .dimensionType(AnalyticsDimensionType.DYNAMIC) .dataType(dataType) - .selectExpression(sql) + .selectExpression(selectExpression) .skipIndex(skipIndex) .build()); @@ -248,7 +251,7 @@ protected List getColumnForAttribute(TrackedEntityAttribut * @param dataFilterClause the data filter clause. * @return a list of {@link AnalyticsTableColumn}. */ - private List getColumnForOrgUnitTrackedEntityAttribute( + private List getColumnForOrgUnitAttribute( TrackedEntityAttribute attribute, String dataFilterClause) { List columns = new ArrayList<>(); @@ -306,4 +309,34 @@ private String getSelectSubquery( "closingParentheses", getClosingParentheses(selectExpression), "attributeUid", quote(attribute.getUid()))); } + + /** + * Returns a join clause for attribute value for every attribute of the given program. + * + * @param program the {@link Program}. + * @return a join clause. + */ + protected String getAttributeValueJoinClause(Program program) { + String template = + """ + left join ${trackedentityattributevalue} as ${uid} \ + on en.trackedentityid=${uid}.trackedentityid \ + and ${uid}.trackedentityattributeid = ${id}\s"""; + + return program.getNonConfidentialTrackedEntityAttributes().stream() + .map(attribute -> replaceQualify(template, toVariableMap(attribute))) + .collect(Collectors.joining()); + } + + /** + * Returns a map of identifiable properties and values. + * + * @param object the {@link IdentifiableObject}. + * @return a {@link Map}. + */ + protected Map toVariableMap(IdentifiableObject object) { + return Map.of( + "id", String.valueOf(object.getId()), + "uid", quote(object.getUid())); + } } diff --git a/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/table/JdbcEnrollmentAnalyticsTableManager.java b/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/table/JdbcEnrollmentAnalyticsTableManager.java index d18fbdffd80b..172745e793d3 100644 --- a/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/table/JdbcEnrollmentAnalyticsTableManager.java +++ b/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/table/JdbcEnrollmentAnalyticsTableManager.java @@ -136,6 +136,7 @@ protected List getPartitionChecks(Integer year, Date endDate) { @Override public void populateTable(AnalyticsTableUpdateParams params, AnalyticsTablePartition partition) { Program program = partition.getMasterTable().getProgram(); + String attributeJoinClause = getAttributeValueJoinClause(program); String fromClause = replaceQualify( @@ -148,6 +149,7 @@ public void populateTable(AnalyticsTableUpdateParams params, AnalyticsTableParti left join analytics_rs_dateperiodstructure dps on cast(en.enrollmentdate as date)=dps.dateperiod \ left join analytics_rs_orgunitstructure ous on en.organisationunitid=ous.organisationunitid \ left join analytics_rs_organisationunitgroupsetstructure ougs on en.organisationunitid=ougs.organisationunitid \ + ${attributeJoinClause}\ where pr.programid=${programId} \ and en.organisationunitid is not null \ and (ougs.startdate is null or dps.monthstartdate=ougs.startdate) \ @@ -155,6 +157,7 @@ left join analytics_rs_dateperiodstructure dps on cast(en.enrollmentdate as date and en.occurreddate is not null \ and en.deleted = false\s""", Map.of( + "attributeJoinClause", attributeJoinClause, "programId", String.valueOf(program.getId()), "startTime", toLongDate(params.getStartTime()))); diff --git a/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/table/JdbcEventAnalyticsTableManager.java b/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/table/JdbcEventAnalyticsTableManager.java index 7a4ff2e0ad6c..95a45283c8a6 100644 --- a/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/table/JdbcEventAnalyticsTableManager.java +++ b/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/table/JdbcEventAnalyticsTableManager.java @@ -328,6 +328,7 @@ public void populateTable(AnalyticsTableUpdateParams params, AnalyticsTableParti Integer latestDataYear = availableDataYears.get(availableDataYears.size() - 1); Program program = partition.getMasterTable().getProgram(); String partitionClause = getPartitionClause(partition); + String attributeJoinClause = getAttributeValueJoinClause(program); String fromClause = replaceQualify( @@ -344,6 +345,7 @@ left join analytics_rs_dateperiodstructure dps on cast(${eventDateExpression} as left join analytics_rs_organisationunitgroupsetstructure ougs on ev.organisationunitid=ougs.organisationunitid \ left join ${organisationunit} enrollmentou on en.organisationunitid=enrollmentou.organisationunitid \ inner join analytics_rs_categorystructure acs on ev.attributeoptioncomboid=acs.categoryoptioncomboid \ + ${attributeJoinClause}\ where ev.lastupdated < '${startTime}' ${partitionClause} \ and pr.programid=${programId} \ and ev.organisationunitid is not null \ @@ -356,6 +358,7 @@ and ev.status in (${exportableEventStatues}) \ Map.of( "eventDateExpression", eventDateExpression, "partitionClause", partitionClause, + "attributeJoinClause", attributeJoinClause, "startTime", toLongDate(params.getStartTime()), "programId", String.valueOf(program.getId()), "firstDataYear", String.valueOf(firstDataYear), @@ -486,15 +489,16 @@ private List getColumnForDataElement( List columns = new ArrayList<>(); DataType dataType = getColumnType(dataElement.getValueType(), isSpatialSupport()); - String columnExpression = + String jsonExpression = sqlBuilder.jsonExtractNested("eventdatavalues", dataElement.getUid(), "value"); - String selectExpression = getSelectExpression(dataElement.getValueType(), columnExpression); + String columnExpression = getColumnExpression(dataElement.getValueType(), jsonExpression); String dataFilterClause = getDataFilterClause(dataElement); - String sql = String.format("%s as %s", selectExpression, quote(dataElement.getUid())); + String selectExpression = + String.format("%s as %s", columnExpression, quote(dataElement.getUid())); Skip skipIndex = skipIndex(dataElement.getValueType(), dataElement.hasOptionSet()); if (withLegendSet) { - return getColumnFromDataElementWithLegendSet(dataElement, selectExpression, dataFilterClause); + return getColumnFromDataElementWithLegendSet(dataElement, columnExpression, dataFilterClause); } if (dataElement.getValueType().isOrganisationUnit()) { @@ -506,7 +510,7 @@ private List getColumnForDataElement( .name(dataElement.getUid()) .dimensionType(AnalyticsDimensionType.DYNAMIC) .dataType(dataType) - .selectExpression(sql) + .selectExpression(selectExpression) .skipIndex(skipIndex) .build()); @@ -587,7 +591,7 @@ private List getAttributeColumns(Program program) { */ private List getColumnForAttributeWithLegendSet( TrackedEntityAttribute attribute) { - String selectClause = getSelectExpression(attribute.getValueType(), "value"); + String columnExpression = getColumnExpression(attribute.getValueType(), "value"); String numericClause = getNumericClause(); String query = """ @@ -602,11 +606,11 @@ private List getColumnForAttributeWithLegendSet( .map( ls -> { String column = attribute.getUid() + PartitionUtils.SEP + ls.getUid(); - String sql = + String selectExpression = replaceQualify( query, Map.of( - "selectClause", selectClause, + "selectClause", columnExpression, "legendSetId", String.valueOf(ls.getId()), "column", column, "attributeId", String.valueOf(attribute.getId()), @@ -615,14 +619,14 @@ private List getColumnForAttributeWithLegendSet( return AnalyticsTableColumn.builder() .name(column) .dataType(CHARACTER_11) - .selectExpression(sql) + .selectExpression(selectExpression) .build(); }) .toList(); } /** - * Returns a select statement for the given data element with value type org unit. + * Returns a select statement for the given select expression. * * @param dataElement the data element to create the select statement for. * @param selectExpression the select expression. @@ -717,15 +721,14 @@ private List getDataYears( Program program, Integer firstDataYear, Integer lastDataYear) { + String fromDate = toMediumDate(params.getFromDate()); String fromDateClause = params.getFromDate() != null ? replace( "and (${eventDateExpression}) >= '${fromDate}'", Map.of( - "eventDateExpression", - eventDateExpression, - "fromDate", - toMediumDate(params.getFromDate()))) + "eventDateExpression", eventDateExpression, + "fromDate", fromDate)) : EMPTY; String sql = diff --git a/dhis-2/dhis-services/dhis-service-analytics/src/test/java/org/hisp/dhis/analytics/table/AbstractEventJdbcTableManagerTest.java b/dhis-2/dhis-services/dhis-service-analytics/src/test/java/org/hisp/dhis/analytics/table/AbstractEventJdbcTableManagerTest.java index 05f4337cc10a..ddb386b7bd0b 100644 --- a/dhis-2/dhis-services/dhis-service-analytics/src/test/java/org/hisp/dhis/analytics/table/AbstractEventJdbcTableManagerTest.java +++ b/dhis-2/dhis-services/dhis-service-analytics/src/test/java/org/hisp/dhis/analytics/table/AbstractEventJdbcTableManagerTest.java @@ -76,7 +76,7 @@ then cast(eventdatavalues #>> '{GieVkTxp4HH, value}' as double precision) \ else null end"""; String actual = - manager.getSelectExpression(ValueType.NUMBER, "eventdatavalues #>> '{GieVkTxp4HH, value}'"); + manager.getColumnExpression(ValueType.NUMBER, "eventdatavalues #>> '{GieVkTxp4HH, value}'"); assertEquals(expected, actual); } @@ -88,7 +88,7 @@ void testGetSelectExpressionBoolean() { case when eventdatavalues #>> '{Xl3voRRcmpo, value}' = 'true' then 1 when eventdatavalues #>> '{Xl3voRRcmpo, value}' = 'false' then 0 else null end"""; String actual = - manager.getSelectExpression( + manager.getColumnExpression( ValueType.BOOLEAN, "eventdatavalues #>> '{Xl3voRRcmpo, value}'"); assertEquals(expected, actual); @@ -103,7 +103,7 @@ then cast(eventdatavalues #>> '{AL04Wbutskk, value}' as timestamp) \ else null end"""; String actual = - manager.getSelectExpression(ValueType.DATE, "eventdatavalues #>> '{AL04Wbutskk, value}'"); + manager.getColumnExpression(ValueType.DATE, "eventdatavalues #>> '{AL04Wbutskk, value}'"); assertEquals(expected, actual); } @@ -115,7 +115,7 @@ void testGetSelectExpressionText() { eventdatavalues #>> '{FwUzmc49Pcr, value}'"""; String actual = - manager.getSelectExpression(ValueType.TEXT, "eventdatavalues #>> '{FwUzmc49Pcr, value}'"); + manager.getColumnExpression(ValueType.TEXT, "eventdatavalues #>> '{FwUzmc49Pcr, value}'"); assertEquals(expected, actual); } @@ -129,7 +129,7 @@ void testGetSelectExpressionGeometry() { ST_GeomFromGeoJSON('{"type":"Point", "coordinates":' || (eventdatavalues #>> '{C6bh7GevJfH, value}') || ', "crs":{"type":"name", "properties":{"name":"EPSG:4326"}}}')"""; String actual = - manager.getSelectExpression( + manager.getColumnExpression( ValueType.GEOJSON, "eventdatavalues #>> '{C6bh7GevJfH, value}'"); assertEquals(expected, actual); diff --git a/dhis-2/dhis-services/dhis-service-analytics/src/test/java/org/hisp/dhis/analytics/table/JdbcEnrollmentAnalyticsTableManagerTest.java b/dhis-2/dhis-services/dhis-service-analytics/src/test/java/org/hisp/dhis/analytics/table/JdbcEnrollmentAnalyticsTableManagerTest.java index d1b9f4443599..f2f2d642cf42 100644 --- a/dhis-2/dhis-services/dhis-service-analytics/src/test/java/org/hisp/dhis/analytics/table/JdbcEnrollmentAnalyticsTableManagerTest.java +++ b/dhis-2/dhis-services/dhis-service-analytics/src/test/java/org/hisp/dhis/analytics/table/JdbcEnrollmentAnalyticsTableManagerTest.java @@ -137,13 +137,7 @@ void verifyTeiTypeOrgUnitFetchesOuUidWhenPopulatingEventAnalyticsTable() { subject.populateTable(params, partition); verify(jdbcTemplate).execute(sql.capture()); - String ouQuery = - format( - """ - (select value from "trackedentityattributevalue" \ - where trackedentityid=en.trackedentityid and \ - trackedentityattributeid=9999) as %s""", - quote(tea.getUid())); + String ouQuery = format("%s.value", quote(tea.getUid())); assertThat(sql.getValue(), containsString(ouQuery)); } diff --git a/dhis-2/dhis-services/dhis-service-analytics/src/test/java/org/hisp/dhis/analytics/table/JdbcEventAnalyticsTableManagerTest.java b/dhis-2/dhis-services/dhis-service-analytics/src/test/java/org/hisp/dhis/analytics/table/JdbcEventAnalyticsTableManagerTest.java index b7da2447b4ac..b2da976c129e 100644 --- a/dhis-2/dhis-services/dhis-service-analytics/src/test/java/org/hisp/dhis/analytics/table/JdbcEventAnalyticsTableManagerTest.java +++ b/dhis-2/dhis-services/dhis-service-analytics/src/test/java/org/hisp/dhis/analytics/table/JdbcEventAnalyticsTableManagerTest.java @@ -501,10 +501,7 @@ void verifyGetTableWithTrackedEntityAttribute() { String aliasD1 = """ eventdatavalues #>> '{deabcdefghZ, value}' as "deabcdefghZ\""""; - String aliasTeaUid = - """ - (select value from "trackedentityattributevalue" where trackedentityid=en.trackedentityid \ - and trackedentityattributeid=%d) as "%s\""""; + String aliasTeaUid = "%s.value"; String aliasTea1 = """ @@ -537,12 +534,8 @@ void verifyGetTableWithTrackedEntityAttribute() { .withTableType(AnalyticsTableType.EVENT) .withColumnSize(59 + OU_NAME_HIERARCHY_COUNT) .addColumns(periodColumns) - .addColumn( - d1.getUid(), - TEXT, - toSelectExpression(aliasD1, d1.getUid()), - Skip.SKIP) // ValueType.TEXT - .addColumn(tea1.getUid(), TEXT, String.format(aliasTeaUid, tea1.getId(), tea1.getUid())) + .addColumn(d1.getUid(), TEXT, toSelectExpression(aliasD1, d1.getUid()), Skip.SKIP) + .addColumn(tea1.getUid(), TEXT, String.format(aliasTeaUid, quote(tea1.getUid()))) // Org unit geometry column .addColumn( tea1.getUid() + "_geom", @@ -650,12 +643,7 @@ void verifyTeiTypeOrgUnitFetchesOuNameWhenPopulatingEventAnalyticsTable() { subject.populateTable(params, partition); verify(jdbcTemplate).execute(sql.capture()); - String ouUidQuery = - String.format( - """ - (select value from "trackedentityattributevalue" where trackedentityid=en.trackedentityid and \ - trackedentityattributeid=9999) as %s""", - quote(tea.getUid())); + String ouUidQuery = String.format("%s.value", quote(tea.getUid())); String ouNameQuery = String.format( @@ -944,12 +932,7 @@ void verifyTeaTypeOrgUnitFetchesOuNameWhenPopulatingEventAnalyticsTable() { verify(jdbcTemplate).execute(sql.capture()); - String ouUidQuery = - String.format( - """ - select value from "trackedentityattributevalue" where trackedentityid=en.trackedentityid \ - and trackedentityattributeid=9999) as %s""", - quote(tea.getUid())); + String ouUidQuery = String.format("%s.value", quote(tea.getUid())); String ouNameQuery = String.format( From 43d41753969b26350a714ecc35898fe66483c15f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lars=20Helge=20=C3=98verland?= Date: Wed, 4 Dec 2024 14:29:24 +0100 Subject: [PATCH 8/8] refactor: Remove unnecessary data filter clause [DHIS2-16705] (#19381) --- .../table/AbstractEventJdbcTableManager.java | 45 ++++--------------- .../table/JdbcEventAnalyticsTableManager.java | 31 ++++++++----- ...bcEventAnalyticsTableManagerDorisTest.java | 2 +- .../JdbcEventAnalyticsTableManagerTest.java | 8 ++-- .../analytics/util/AnalyticsUtilsTest.java | 18 -------- 5 files changed, 31 insertions(+), 73 deletions(-) diff --git a/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/table/AbstractEventJdbcTableManager.java b/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/table/AbstractEventJdbcTableManager.java index 543c686379ac..f8d9f1bd6f72 100644 --- a/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/table/AbstractEventJdbcTableManager.java +++ b/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/table/AbstractEventJdbcTableManager.java @@ -27,18 +27,17 @@ */ package org.hisp.dhis.analytics.table; -import static org.apache.commons.lang3.StringUtils.EMPTY; import static org.hisp.dhis.analytics.table.model.Skip.SKIP; import static org.hisp.dhis.analytics.util.AnalyticsUtils.getClosingParentheses; import static org.hisp.dhis.analytics.util.AnalyticsUtils.getColumnType; import static org.hisp.dhis.db.model.DataType.GEOMETRY; import static org.hisp.dhis.db.model.DataType.TEXT; -import static org.hisp.dhis.system.util.MathUtils.NUMERIC_LENIENT_REGEXP; import java.util.ArrayList; import java.util.List; import java.util.Map; import java.util.stream.Collectors; +import org.apache.commons.lang3.Validate; import org.hisp.dhis.analytics.AnalyticsTableHookService; import org.hisp.dhis.analytics.partition.PartitionManager; import org.hisp.dhis.analytics.table.model.AnalyticsDimensionType; @@ -99,14 +98,6 @@ public AbstractEventJdbcTableManager( public static final String OU_NAME_COL_SUFFIX = "_name"; - protected final String getNumericClause() { - return " and " + sqlBuilder.regexpMatch("value", "'" + NUMERIC_LENIENT_REGEXP + "'"); - } - - protected final String getDateClause() { - return " and " + sqlBuilder.regexpMatch("value", DATE_REGEXP); - } - /** * Indicates whether creating an index should be skipped. * @@ -148,22 +139,6 @@ protected String getColumnExpression(ValueType valueType, String columnExpressio } } - /** - * For numeric and date value types, returns a data filter clause for checking whether the value - * is valid according to the value type. For other value types, returns the empty string. - * - * @param attribute the {@link TrackedEntityAttribute}. - * @return a data filter clause. - */ - protected String getDataFilterClause(TrackedEntityAttribute attribute) { - if (attribute.isNumericType()) { - return getNumericClause(); - } else if (attribute.isDateType()) { - return getDateClause(); - } - return EMPTY; - } - /** * Returns a cast expression which includes a value filter for the given value type. * @@ -225,11 +200,10 @@ protected List getColumnForAttribute(TrackedEntityAttribut String valueColumn = String.format("%s.%s", quote(attribute.getUid()), "value"); DataType dataType = getColumnType(attribute.getValueType(), isSpatialSupport()); String selectExpression = getColumnExpression(attribute.getValueType(), valueColumn); - String dataFilterClause = getDataFilterClause(attribute); Skip skipIndex = skipIndex(attribute.getValueType(), attribute.hasOptionSet()); if (attribute.getValueType().isOrganisationUnit()) { - columns.addAll(getColumnForOrgUnitAttribute(attribute, dataFilterClause)); + columns.addAll(getColumnForOrgUnitAttribute(attribute)); } columns.add( @@ -248,11 +222,11 @@ protected List getColumnForAttribute(TrackedEntityAttribut * Returns a list of columns based on the given attribute. * * @param attribute the {@link TrackedEntityAttribute}. - * @param dataFilterClause the data filter clause. * @return a list of {@link AnalyticsTableColumn}. */ private List getColumnForOrgUnitAttribute( - TrackedEntityAttribute attribute, String dataFilterClause) { + TrackedEntityAttribute attribute) { + Validate.isTrue(attribute.getValueType().isOrganisationUnit()); List columns = new ArrayList<>(); String fromClause = @@ -260,7 +234,7 @@ private List getColumnForOrgUnitAttribute( if (isSpatialSupport()) { String selectExpression = "ou.geometry " + fromClause; - String ouGeoSql = getSelectSubquery(attribute, selectExpression, dataFilterClause); + String ouGeoSql = getSelectSubquery(attribute, selectExpression); columns.add( AnalyticsTableColumn.builder() .name((attribute.getUid() + OU_GEOMETRY_COL_SUFFIX)) @@ -272,7 +246,7 @@ private List getColumnForOrgUnitAttribute( } String selectExpression = "ou.name " + fromClause; - String ouNameSql = getSelectSubquery(attribute, selectExpression, dataFilterClause); + String ouNameSql = getSelectSubquery(attribute, selectExpression); columns.add( AnalyticsTableColumn.builder() @@ -291,20 +265,17 @@ private List getColumnForOrgUnitAttribute( * * @param attribute the {@link TrackedEntityAttribute}. * @param selectExpression the select expression. - * @param dataFilterClause the data filter clause. * @return a select statement. */ - private String getSelectSubquery( - TrackedEntityAttribute attribute, String selectExpression, String dataFilterClause) { + private String getSelectSubquery(TrackedEntityAttribute attribute, String selectExpression) { return replaceQualify( """ (select ${selectExpression} from ${trackedentityattributevalue} \ where trackedentityid=en.trackedentityid \ - and trackedentityattributeid=${attributeId}${dataFilterClause})\ + and trackedentityattributeid=${attributeId})\ ${closingParentheses} as ${attributeUid}""", Map.of( "selectExpression", selectExpression, - "dataFilterClause", dataFilterClause, "attributeId", String.valueOf(attribute.getId()), "closingParentheses", getClosingParentheses(selectExpression), "attributeUid", quote(attribute.getUid()))); diff --git a/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/table/JdbcEventAnalyticsTableManager.java b/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/table/JdbcEventAnalyticsTableManager.java index 95a45283c8a6..4474f2d4c97a 100644 --- a/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/table/JdbcEventAnalyticsTableManager.java +++ b/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/table/JdbcEventAnalyticsTableManager.java @@ -39,6 +39,7 @@ import static org.hisp.dhis.db.model.DataType.GEOMETRY; import static org.hisp.dhis.db.model.DataType.INTEGER; import static org.hisp.dhis.db.model.DataType.TEXT; +import static org.hisp.dhis.system.util.MathUtils.NUMERIC_LENIENT_REGEXP; import static org.hisp.dhis.util.DateUtils.toLongDate; import static org.hisp.dhis.util.DateUtils.toMediumDate; @@ -502,7 +503,7 @@ private List getColumnForDataElement( } if (dataElement.getValueType().isOrganisationUnit()) { - columns.addAll(getColumnForOrgUnitDataElement(dataElement, dataFilterClause)); + columns.addAll(getColumnForOrgUnitDataElement(dataElement)); } columns.add( @@ -524,8 +525,7 @@ private List getColumnForDataElement( * @param dataFilterClause the data filter SQL clause. * @return a list of {@link AnalyticsTableColumn}. */ - private List getColumnForOrgUnitDataElement( - DataElement dataElement, String dataFilterClause) { + private List getColumnForOrgUnitDataElement(DataElement dataElement) { List columns = new ArrayList<>(); String columnExpression = @@ -535,20 +535,20 @@ private List getColumnForOrgUnitDataElement( if (isSpatialSupport()) { String fromType = "ou.geometry " + fromClause; - String geoSql = getOrgUnitSelectExpression(dataElement, fromType, dataFilterClause); + String geoExpression = getOrgUnitSelectExpression(dataElement, fromType); columns.add( AnalyticsTableColumn.builder() .name((dataElement.getUid() + OU_GEOMETRY_COL_SUFFIX)) .dimensionType(AnalyticsDimensionType.DYNAMIC) .dataType(GEOMETRY) - .selectExpression(geoSql) + .selectExpression(geoExpression) .indexType(IndexType.GIST) .build()); } String fromTypeSql = "ou.name " + fromClause; - String ouNameSql = getOrgUnitSelectExpression(dataElement, fromTypeSql, dataFilterClause); + String ouNameSql = getOrgUnitSelectExpression(dataElement, fromTypeSql); columns.add( AnalyticsTableColumn.builder() @@ -592,7 +592,7 @@ private List getAttributeColumns(Program program) { private List getColumnForAttributeWithLegendSet( TrackedEntityAttribute attribute) { String columnExpression = getColumnExpression(attribute.getValueType(), "value"); - String numericClause = getNumericClause(); + String numericClause = getNumericClause("value"); String query = """ \s(select l.uid from ${maplegend} l \ @@ -630,18 +630,15 @@ private List getColumnForAttributeWithLegendSet( * * @param dataElement the data element to create the select statement for. * @param selectExpression the select expression. - * @param dataFilterClause the data filter clause. * @return a select expression. */ - private String getOrgUnitSelectExpression( - DataElement dataElement, String selectExpression, String dataFilterClause) { + private String getOrgUnitSelectExpression(DataElement dataElement, String selectExpression) { Validate.isTrue(dataElement.getValueType().isOrganisationUnit()); String prts = getClosingParentheses(selectExpression); return replaceQualify( - "(select ${selectExpression} ${dataFilterClause})${closingParentheses} as ${uid}", + "(select ${selectExpression})${closingParentheses} as ${uid}", Map.of( "selectExpression", selectExpression, - "dataFilterClause", dataFilterClause, "closingParentheses", prts, "uid", quote(dataElement.getUid()))); } @@ -756,6 +753,16 @@ private List getDataYears( return jdbcTemplate.queryForList(sql, Integer.class); } + /** + * Returns a numeric regexp match expression for the given value. + * + * @param value the value. + * @return a numeric regexp match expression. + */ + private final String getNumericClause(String value) { + return " and " + sqlBuilder.regexpMatch(value, "'" + NUMERIC_LENIENT_REGEXP + "'"); + } + /** * Retrieve years for partition tables. Year will become a partition key. The default return value * is the list with the recent year. diff --git a/dhis-2/dhis-services/dhis-service-analytics/src/test/java/org/hisp/dhis/analytics/table/JdbcEventAnalyticsTableManagerDorisTest.java b/dhis-2/dhis-services/dhis-service-analytics/src/test/java/org/hisp/dhis/analytics/table/JdbcEventAnalyticsTableManagerDorisTest.java index 9f952e8807b3..5b2bca0f3ec1 100644 --- a/dhis-2/dhis-services/dhis-service-analytics/src/test/java/org/hisp/dhis/analytics/table/JdbcEventAnalyticsTableManagerDorisTest.java +++ b/dhis-2/dhis-services/dhis-service-analytics/src/test/java/org/hisp/dhis/analytics/table/JdbcEventAnalyticsTableManagerDorisTest.java @@ -169,7 +169,7 @@ void verifyGetTableWithDataElements() { "case when json_unquote(json_extract(eventdatavalues, '$.%s.value')) regexp '^\\d{4}-\\d{2}-\\d{2}(\\s|T)?((\\d{2}:)(\\d{2}:)?(\\d{2}))?(|.(\\d{3})|.(\\d{3})Z)?$' then cast(json_unquote(json_extract(eventdatavalues, '$.%s.value')) as datetime) else null end as `%s`"; String aliasE = "json_unquote(json_extract(eventdatavalues, '$.%s.value')) as `%s`"; String aliasF = - "(select ou.name from dhis2.public.`organisationunit` ou where ou.uid = json_unquote(json_extract(eventdatavalues, '$.%s.value')) ) as `%s`"; + "(select ou.name from dhis2.public.`organisationunit` ou where ou.uid = json_unquote(json_extract(eventdatavalues, '$.%s.value'))) as `%s`"; String aliasG = "case when json_unquote(json_extract(eventdatavalues, '$.%s.value')) regexp '^(-?[0-9]+)(\\.[0-9]+)?$' then cast(json_unquote(json_extract(eventdatavalues, '$.%s.value')) as bigint) else null end as `%s`"; diff --git a/dhis-2/dhis-services/dhis-service-analytics/src/test/java/org/hisp/dhis/analytics/table/JdbcEventAnalyticsTableManagerTest.java b/dhis-2/dhis-services/dhis-service-analytics/src/test/java/org/hisp/dhis/analytics/table/JdbcEventAnalyticsTableManagerTest.java index b2da976c129e..9a9db09b4f99 100644 --- a/dhis-2/dhis-services/dhis-service-analytics/src/test/java/org/hisp/dhis/analytics/table/JdbcEventAnalyticsTableManagerTest.java +++ b/dhis-2/dhis-services/dhis-service-analytics/src/test/java/org/hisp/dhis/analytics/table/JdbcEventAnalyticsTableManagerTest.java @@ -399,15 +399,13 @@ void verifyGetTableWithDataElements() { String aliasD5_geo = "(select ou.geometry from \"organisationunit\" ou where ou.uid = eventdatavalues #>> '{" + d5.getUid() - + ", value}' " - + ") as \"" + + ", value}') as \"" + d5.getUid() + "\""; String aliasD5_name = "(select ou.name from \"organisationunit\" ou where ou.uid = eventdatavalues #>> '{" + d5.getUid() - + ", value}' " - + ") as \"" + + ", value}') as \"" + d5.getUid() + "\""; AnalyticsTableUpdateParams params = @@ -599,7 +597,7 @@ void verifyDataElementTypeOrgUnitFetchesOuNameWhenPopulatingEventAnalyticsTable( String.format( """ (select ou.name from "organisationunit" ou where ou.uid = \ - eventdatavalues #>> '{%s, value}' ) as %s""", + eventdatavalues #>> '{%s, value}') as %s""", d5.getUid(), quote(d5.getUid())); assertThat(sql.getValue(), containsString(ouUidQuery)); diff --git a/dhis-2/dhis-services/dhis-service-analytics/src/test/java/org/hisp/dhis/analytics/util/AnalyticsUtilsTest.java b/dhis-2/dhis-services/dhis-service-analytics/src/test/java/org/hisp/dhis/analytics/util/AnalyticsUtilsTest.java index 78424fe9c735..dbbb563aa2ac 100644 --- a/dhis-2/dhis-services/dhis-service-analytics/src/test/java/org/hisp/dhis/analytics/util/AnalyticsUtilsTest.java +++ b/dhis-2/dhis-services/dhis-service-analytics/src/test/java/org/hisp/dhis/analytics/util/AnalyticsUtilsTest.java @@ -93,7 +93,6 @@ import org.hisp.dhis.test.TestBase; import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Test; -import org.springframework.jdbc.BadSqlGrammarException; import org.springframework.jdbc.UncategorizedSQLException; /** @@ -763,31 +762,14 @@ void whenUncategorizedSQLException_withTableNotExisting_thenThrowException() { () -> AnalyticsUtils.withExceptionHandling(supplier, false)); } - @Test - void whenBadSqlGrammarException_withMultipleQueries_thenReturnEmpty() { - SQLException sqlException = new SQLException("syntax error"); - BadSqlGrammarException badSqlException = - new BadSqlGrammarException("task", DUMMY_SQL, sqlException); - Supplier supplier = - () -> { - throw badSqlException; - }; - - Optional result = AnalyticsUtils.withExceptionHandling(supplier, true); - - assertFalse(result.isPresent()); - } - @Test void whenQueryRuntimeException_thenRethrow() { - // Arrange QueryRuntimeException queryException = new QueryRuntimeException("Test error"); Supplier supplier = () -> { throw queryException; }; - // Act & Assert QueryRuntimeException thrown = assertThrows( QueryRuntimeException.class,