Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix: Fix the table population problem when the skip outliers switch is activated/deactivated #16037

Merged
merged 5 commits into from
Dec 28, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -438,6 +438,11 @@ public enum ErrorCode {
"Query failed because a referenced table does not exist. Please ensure analytics job was run"),
E7145("Query failed because of a syntax error"),

/* Analytics outliers */

E7180(
"The analytics outliers data does not exist. Please ensure analytics job was run and did not skip the outliers"),

/* Event analytics */
E7200(Constants.AT_LEAST_ONE_ORGANISATION_UNIT_MUST_BE_SPECIFIED),
E7201("Dimensions cannot be specified more than once: `{0}`"),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,12 +59,15 @@
import lombok.AllArgsConstructor;
import lombok.extern.slf4j.Slf4j;
import org.hisp.dhis.analytics.cache.OutliersCache;
import org.hisp.dhis.analytics.common.TableInfoReader;
import org.hisp.dhis.analytics.outlier.data.Outlier;
import org.hisp.dhis.analytics.outlier.data.OutlierRequest;
import org.hisp.dhis.common.ExecutionPlan;
import org.hisp.dhis.common.Grid;
import org.hisp.dhis.common.GridHeader;
import org.hisp.dhis.common.IllegalQueryException;
import org.hisp.dhis.feedback.ErrorCode;
import org.hisp.dhis.feedback.ErrorMessage;
import org.hisp.dhis.organisationunit.OrganisationUnit;
import org.hisp.dhis.organisationunit.OrganisationUnitService;
import org.hisp.dhis.system.grid.GridUtils;
Expand All @@ -86,6 +89,8 @@ public class AnalyticsOutlierService {

private final CurrentUserService currentUserService;

private final TableInfoReader tableInfoReader;

/**
* Transform the incoming request into api response (json).
*
Expand Down Expand Up @@ -163,6 +168,19 @@ public void getOutliersAsHtmlCss(OutlierRequest request, Writer writer)
GridUtils.toHtmlCss(getOutliers(request), writer);
}

/**
* The inclusion of outliers is an optional aspect of the analytics table. The outliers API entry
* point can generate a proper response only when outliers are exported along with the analytics
* table. The 'sourceid' column serves as a reliable indicator for successful outliers export. Its
* absence implies that the outliers were not exported.
*/
public void checkAnalyticsTableForOutliers() {
if (tableInfoReader.getInfo("analytics").getColumns().stream()
.noneMatch("sourceid"::equalsIgnoreCase)) {
throw new IllegalQueryException(new ErrorMessage(ErrorCode.E7180));
}
}

private void setHeaders(Grid grid, OutlierRequest request) {
boolean isModifiedZScore = request.getAlgorithm() == MOD_Z_SCORE;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -163,9 +163,9 @@ public AnalyticsTableType getAnalyticsTableType() {
public List<AnalyticsTable> getAnalyticsTables(AnalyticsTableUpdateParams params) {
AnalyticsTable table =
params.isLatestUpdate()
? getLatestAnalyticsTable(params, getDimensionColumns(), getValueColumns())
? getLatestAnalyticsTable(params, getDimensionColumns(params), getValueColumns())
: getRegularAnalyticsTable(
params, getDataYears(params), getDimensionColumns(), getValueColumns());
params, getDataYears(params), getDimensionColumns(params), getValueColumns());

return table.hasPartitionTables() ? List.of(table) : List.of();
}
Expand Down Expand Up @@ -432,8 +432,8 @@ private String getApprovalJoinClause(Integer year) {
return StringUtils.EMPTY;
}

private List<AnalyticsTableColumn> getDimensionColumns() {
return getDimensionColumns(null, null);
private List<AnalyticsTableColumn> getDimensionColumns(AnalyticsTableUpdateParams params) {
return getDimensionColumns(null, params);
}

private List<AnalyticsTableColumn> getDimensionColumns(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
import org.apache.commons.lang3.ObjectUtils;
import org.hisp.dhis.analytics.AnalyticsTableGenerator;
import org.hisp.dhis.analytics.AnalyticsTableUpdateParams;
import org.hisp.dhis.analytics.common.TableInfoReader;
import org.hisp.dhis.scheduling.Job;
import org.hisp.dhis.scheduling.JobConfiguration;
import org.hisp.dhis.scheduling.JobProgress;
Expand Down Expand Up @@ -69,6 +70,8 @@ public class ContinuousAnalyticsTableJob implements Job {

private final SystemSettingManager systemSettingManager;

private final TableInfoReader tableInfoReader;

@Override
public JobType getJobType() {
return JobType.CONTINUOUS_ANALYTICS_TABLE;
Expand All @@ -79,6 +82,12 @@ public void execute(JobConfiguration jobConfiguration, JobProgress progress) {
ContinuousAnalyticsJobParameters parameters =
(ContinuousAnalyticsJobParameters) jobConfiguration.getJobParameters();

if (!checkJobOutliersConsistency(parameters)) {
log.info(
"Updating the analytics table is currently not feasible. The incoming job parameters do not align with the existing outliers data structure, preventing a successful update.");
return;
}

Integer fullUpdateHourOfDay =
ObjectUtils.firstNonNull(parameters.getFullUpdateHourOfDay(), DEFAULT_HOUR_OF_DAY);

Expand Down Expand Up @@ -132,4 +141,14 @@ public void execute(JobConfiguration jobConfiguration, JobProgress progress) {
analyticsTableGenerator.generateTables(params, progress);
}
}

private boolean checkJobOutliersConsistency(ContinuousAnalyticsJobParameters parameters) {
boolean analyticsTableWithOutliers =
tableInfoReader.getInfo("analytics").getColumns().stream()
.anyMatch("sourceid"::equalsIgnoreCase);
boolean outliersRequired = !parameters.getSkipOutliers();

return outliersRequired && analyticsTableWithOutliers
|| !outliersRequired && !analyticsTableWithOutliers;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,8 @@
* missing analytics tables. For this reason they have to run first (before analytics tables are
* created), hence @Order(1).
*/

//TODO do not forget add e2e for DHIS2-16375
@Order(1)
@ExtendWith(ConfigurationExtension.class)
@Tag("analytics")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,13 +74,15 @@ public class AnalyticsOutlierDetectionController {
value = RESOURCE_PATH + "/explain",
produces = {APPLICATION_JSON_VALUE, "application/javascript"})
public @ResponseBody Grid getExplainOutliersJson(OutlierQueryParams query) {
outlierService.checkAnalyticsTableForOutliers();
OutlierRequest request = getFromQuery(query, true);

return outlierService.getOutliersPerformanceMetrics(request);
}

@GetMapping(value = RESOURCE_PATH, produces = APPLICATION_JSON_VALUE)
public Grid getOutliersJson(OutlierQueryParams queryParams) {
outlierService.checkAnalyticsTableForOutliers();
OutlierRequest request = getFromQuery(queryParams, false);

Grid grid = outlierService.getOutliers(request);
Expand All @@ -95,6 +97,7 @@ public Grid getOutliersJson(OutlierQueryParams queryParams) {
@GetMapping(value = RESOURCE_PATH + ".csv")
public void getOutliersCsv(OutlierQueryParams queryParams, HttpServletResponse response)
throws IOException {
outlierService.checkAnalyticsTableForOutliers();
OutlierRequest request = getFromQuery(queryParams, false);
contextUtils.configureResponse(response, CONTENT_TYPE_CSV, NO_CACHE, "outlierdata.csv", true);

Expand All @@ -104,6 +107,7 @@ public void getOutliersCsv(OutlierQueryParams queryParams, HttpServletResponse r
@GetMapping(value = RESOURCE_PATH + ".xml")
public void getOutliersXml(OutlierQueryParams queryParams, HttpServletResponse response)
throws IOException {
outlierService.checkAnalyticsTableForOutliers();
OutlierRequest request = getFromQuery(queryParams, false);
contextUtils.configureResponse(response, CONTENT_TYPE_XML, NO_CACHE);

Expand All @@ -113,6 +117,7 @@ public void getOutliersXml(OutlierQueryParams queryParams, HttpServletResponse r
@GetMapping(value = RESOURCE_PATH + ".xls")
public void getOutliersXls(OutlierQueryParams queryParams, HttpServletResponse response)
throws IOException {
outlierService.checkAnalyticsTableForOutliers();
OutlierRequest request = getFromQuery(queryParams, false);
contextUtils.configureResponse(response, CONTENT_TYPE_EXCEL, NO_CACHE, "outlierdata.xls", true);

Expand All @@ -122,6 +127,7 @@ public void getOutliersXls(OutlierQueryParams queryParams, HttpServletResponse r
@GetMapping(value = RESOURCE_PATH + ".html")
public void getOutliersHtml(OutlierQueryParams queryParams, HttpServletResponse response)
throws IOException {
outlierService.checkAnalyticsTableForOutliers();
OutlierRequest request = getFromQuery(queryParams, false);

contextUtils.configureResponse(response, CONTENT_TYPE_HTML, NO_CACHE);
Expand All @@ -132,6 +138,7 @@ public void getOutliersHtml(OutlierQueryParams queryParams, HttpServletResponse
@GetMapping(value = RESOURCE_PATH + ".html+css")
public void getOutliersHtmlCss(OutlierQueryParams queryParams, HttpServletResponse response)
throws IOException {
outlierService.checkAnalyticsTableForOutliers();
OutlierRequest request = getFromQuery(queryParams, false);
contextUtils.configureResponse(response, CONTENT_TYPE_HTML, NO_CACHE);

Expand Down
Loading