From 41ca7107d5c7810d861d323331575b7a3e4393f0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lars=20Helge=20=C3=98verland?= Date: Mon, 2 Dec 2024 18:17:20 +0100 Subject: [PATCH] fix: Update code --- .../table/JdbcEventAnalyticsTableManager.java | 32 ++--- .../JdbcEventAnalyticsTableManagerTest.java | 111 +++++++++++------- 2 files changed, 82 insertions(+), 61 deletions(-) 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 8c652952993..976872fecff 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 @@ -51,6 +51,7 @@ import java.util.Map; import java.util.Objects; import lombok.extern.slf4j.Slf4j; +import org.apache.commons.lang3.Validate; import org.hisp.dhis.analytics.AnalyticsTableHookService; import org.hisp.dhis.analytics.AnalyticsTableType; import org.hisp.dhis.analytics.AnalyticsTableUpdateParams; @@ -492,7 +493,7 @@ private List getColumnForDataElement( sqlBuilder.jsonExtractNested("eventdatavalues", dataElement.getUid(), "value"); String selectExpression = getSelectExpression(dataElement.getValueType(), columnExpression); String dataFilterClause = getDataFilterClause(dataElement); - String sql = getSelectForInsert(dataElement, selectExpression, dataFilterClause); + String sql = String.format("%s as %s", selectExpression, quote(dataElement.getUid())); Skip skipIndex = skipIndex(dataElement.getValueType(), dataElement.hasOptionSet()); if (withLegendSet) { @@ -533,7 +534,7 @@ private List getColumnForOrgUnitDataElement( if (isSpatialSupport()) { String fromType = "ou.geometry " + fromClause; - String geoSql = getSelectForInsert(dataElement, fromType, dataFilterClause); + String geoSql = getOrgUnitSelectExpression(dataElement, fromType, dataFilterClause); columns.add( AnalyticsTableColumn.builder() @@ -546,7 +547,7 @@ private List getColumnForOrgUnitDataElement( } String fromTypeSql = "ou.name " + fromClause; - String ouNameSql = getSelectForInsert(dataElement, fromTypeSql, dataFilterClause); + String ouNameSql = getOrgUnitSelectExpression(dataElement, fromTypeSql, dataFilterClause); columns.add( AnalyticsTableColumn.builder() @@ -624,31 +625,24 @@ private List getColumnForAttributeWithLegendSet( } /** - * Retyrns a select statement for the given select expression. + * Returns a select statement for the given data element with value type org unit. * * @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 getSelectForInsert( + private String getOrgUnitSelectExpression( DataElement dataElement, String selectExpression, String dataFilterClause) { - String sqlTemplate = - dataElement.getValueType().isOrganisationUnit() - ? "(select ${selectExpression} ${dataClause})${closingParentheses} as ${uid}" - : "${selectExpression}${closingParentheses} as ${uid}"; - + Validate.isTrue(dataElement.getValueType().isOrganisationUnit()); + String prts = getClosingParentheses(selectExpression); return replaceQualify( - sqlTemplate, + "(select ${selectExpression} ${dataFilterClause})${closingParentheses} as ${uid}", Map.of( - "selectExpression", - selectExpression, - "dataClause", - dataFilterClause, - "closingParentheses", - getClosingParentheses(selectExpression), - "uid", - quote(dataElement.getUid()))); + "selectExpression", selectExpression, + "dataFilterClause", dataFilterClause, + "closingParentheses", prts, + "uid", quote(dataElement.getUid()))); } /** 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 4e64792c854..662fd57c685 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 @@ -316,10 +316,15 @@ void verifyClientSideTimestampsColumns() { assertThat( lastUpdated.getSelectExpression(), is( - "case when ev.lastupdatedatclient is not null then ev.lastupdatedatclient else ev.lastupdated end")); + """ + case when ev.lastupdatedatclient is not null \ + then ev.lastupdatedatclient else ev.lastupdated end""")); assertThat( created.getSelectExpression(), - is("case when ev.createdatclient is not null then ev.createdatclient else ev.created end")); + is( + """ + case when ev.createdatclient is not null \ + then ev.createdatclient else ev.created end""")); } @Test @@ -389,13 +394,7 @@ void verifyGetTableWithDataElements() { case when eventdatavalues #>> '{deabcdefghW, value}' ~* '^\\d{4}-\\d{2}-\\d{2}(\\s|T)?((\\d{2}:)(\\d{2}:)?(\\d{2}))?(|.(\\d{3})|.(\\d{3})Z)?$' \ then cast(eventdatavalues #>> '{deabcdefghW, value}' as timestamp) else null end as "deabcdefghW\""""; String aliasD5 = - "(select ou.uid from \"organisationunit\" ou where ou.uid = " - + "eventdatavalues #>> '{" - + d5.getUid() - + ", value}' " - + ") as \"" - + d5.getUid() - + "\""; + "eventdatavalues #>> '{" + d5.getUid() + ", value}' as \"" + d5.getUid() + "\""; String aliasD6 = """ case when eventdatavalues #>> '{deabcdefghH, value}' ~* '^(-?[0-9]+)(\\.[0-9]+)?$' \ @@ -509,10 +508,16 @@ 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 aliasTea1 = - "(select %s from \"organisationunit\" ou where ou.uid = (select value from " - + "\"trackedentityattributevalue\" where trackedentityid=en.trackedentityid and " - + "trackedentityattributeid=%d)) as \"%s\""; + """ + (select %s from "organisationunit" ou where ou.uid = \ + (select value from "trackedentityattributevalue" where trackedentityid=en.trackedentityid \ + and trackedentityattributeid=%d)) as "%s\""""; AnalyticsTableUpdateParams params = AnalyticsTableUpdateParams.newBuilder() @@ -544,14 +549,14 @@ void verifyGetTableWithTrackedEntityAttribute() { TEXT, toSelectExpression(aliasD1, d1.getUid()), Skip.SKIP) // ValueType.TEXT - .addColumn( - tea1.getUid(), TEXT, String.format(aliasTea1, "ou.uid", tea1.getId(), tea1.getUid())) - // Second Geometry column created from the OU column above + .addColumn(tea1.getUid(), TEXT, String.format(aliasTeaUid, tea1.getId(), tea1.getUid())) + // Org unit geometry column .addColumn( tea1.getUid() + "_geom", GEOMETRY, String.format(aliasTea1, "ou.geometry", tea1.getId(), tea1.getUid()), IndexType.GIST) + // Org unit name column .addColumn( tea1.getUid() + "_name", TEXT, @@ -599,16 +604,21 @@ void verifyDataElementTypeOrgUnitFetchesOuNameWhenPopulatingEventAnalyticsTable( subject.populateTable(params, partition); verify(jdbcTemplate).execute(sql.capture()); - String ouQuery = - "(select ou.%s from \"organisationunit\" ou where ou.uid = " - + "eventdatavalues #>> '{" - + d5.getUid() - + ", value}' ) as \"" - + d5.getUid() - + "\""; - - assertThat(sql.getValue(), containsString(String.format(ouQuery, "uid"))); - assertThat(sql.getValue(), containsString(String.format(ouQuery, "name"))); + String ouUidQuery = + String.format( + """ + eventdatavalues #>> '{%s, value}' as %s""", + d5.getUid(), quote(d5.getUid())); + + String ouNameQuery = + String.format( + """ + (select ou.name from "organisationunit" ou where ou.uid = \ + eventdatavalues #>> '{%s, value}' ) as %s""", + d5.getUid(), quote(d5.getUid())); + + assertThat(sql.getValue(), containsString(ouUidQuery)); + assertThat(sql.getValue(), containsString(ouNameQuery)); } @Test @@ -649,15 +659,23 @@ void verifyTeiTypeOrgUnitFetchesOuNameWhenPopulatingEventAnalyticsTable() { subject.populateTable(params, partition); verify(jdbcTemplate).execute(sql.capture()); - String ouQuery = - "(select ou.%s from \"organisationunit\" ou where ou.uid = " - + "(select value from \"trackedentityattributevalue\" where trackedentityid=en.trackedentityid and " - + "trackedentityattributeid=9999)) as \"" - + tea.getUid() - + "\""; - - assertThat(sql.getValue(), containsString(String.format(ouQuery, "uid"))); - assertThat(sql.getValue(), containsString(String.format(ouQuery, "name"))); + String ouUidQuery = + String.format( + """ + (select value from "trackedentityattributevalue" where trackedentityid=en.trackedentityid and \ + trackedentityattributeid=9999) as %s""", + quote(tea.getUid())); + + String ouNameQuery = + String.format( + """ + (select ou.name from "organisationunit" ou where ou.uid = \ + (select value from "trackedentityattributevalue" where trackedentityid=en.trackedentityid and \ + trackedentityattributeid=9999)) as %s""", + quote(tea.getUid())); + + assertThat(sql.getValue(), containsString(ouUidQuery)); + assertThat(sql.getValue(), containsString(ouNameQuery)); } @Test @@ -937,14 +955,23 @@ void verifyTeaTypeOrgUnitFetchesOuNameWhenPopulatingEventAnalyticsTable() { verify(jdbcTemplate).execute(sql.capture()); - String ouQuery = - """ - (select ou.%s from \"organisationunit\" ou where ou.uid = \ - (select value from \"trackedentityattributevalue\" where trackedentityid=en.trackedentityid and \ - trackedentityattributeid=9999)) as %s"""; - - assertThat(sql.getValue(), containsString(String.format(ouQuery, "uid", quote(tea.getUid())))); - assertThat(sql.getValue(), containsString(String.format(ouQuery, "name", quote(tea.getUid())))); + String ouUidQuery = + String.format( + """ + select value from "trackedentityattributevalue" where trackedentityid=en.trackedentityid \ + and trackedentityattributeid=9999) as %s""", + quote(tea.getUid())); + + String ouNameQuery = + String.format( + """ + (select ou.name from "organisationunit" ou where ou.uid = \ + (select value from "trackedentityattributevalue" where trackedentityid=en.trackedentityid \ + and trackedentityattributeid=9999)) as %s""", + quote(tea.getUid())); + + assertThat(sql.getValue(), containsString(ouUidQuery)); + assertThat(sql.getValue(), containsString(ouNameQuery)); } private String toSelectExpression(String template, String uid) {