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

[UNDER TESTING] fix: Use stored path for org units [DHIS2-18234] #19635

Open
wants to merge 3 commits into
base: 2.40
Choose a base branch
from
Open
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 @@ -28,7 +28,9 @@
package org.hisp.dhis.organisationunit;

import static org.apache.commons.collections4.CollectionUtils.isEmpty;
import static org.apache.commons.lang3.StringUtils.isNotEmpty;

import com.fasterxml.jackson.annotation.JsonIgnore;
import com.fasterxml.jackson.annotation.JsonProperty;
import com.fasterxml.jackson.databind.annotation.JsonSerialize;
import com.fasterxml.jackson.dataformat.xml.annotation.JacksonXmlElementWrapper;
Expand Down Expand Up @@ -708,6 +710,13 @@ public void setChildren(Set<OrganisationUnit> children) {
this.children = children;
}

/**
* Note that the {@code path} property is mapped with the "property access" mode. This method will
* calculate and return the path property value based on the org unit ancestors. To access the
* {@code path} property directly, use {@link OrganisationUnit#getStoredPath}.
*
* @return the path.
*/
@JsonProperty
@JacksonXmlProperty(namespace = DxfNamespaces.DXF_2_0)
public String getPath() {
Expand All @@ -729,19 +738,34 @@ public String getPath() {

Collections.reverse(pathList);

this.path = PATH_SEP + StringUtils.join(pathList, PATH_SEP);
return PATH_SEP + StringUtils.join(pathList, PATH_SEP);
}

return this.path;
/**
* Note that the {@code path} property is mapped with the "property access" mode. This method will
* return the persisted {@code path} property value directly. If the path is not defined,
* typically as part of an integration test where the state is not yet flushed to the database,
* the calculated path based on the org unit ancestors is returned. To get the calculated path
* value explicitly, use {@link OrganisationUnit#getPath}.
*
* @return the path.
*/
@JsonIgnore
public String getStoredPath() {
return isNotEmpty(path) ? path : getPath();
}

/** Do not set directly, managed by persistence layer. */
/**
* Note that the {@code path} property is mapped with the "property access" mode. Do not set
* directly, this property is managed by the persistence layer.
*/
public void setPath(String path) {
this.path = path;
}

/**
* Note that the {@code path} is mapped with the "property access" mode. This method is for unit
* testing purposes only.
* Note that the {@code path} property is mapped with the "property access" mode. This method is
* for unit testing purposes only.
*/
public void updatePath() {
setPath(getPath());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@ private String getSelectStatement(DataQueryParams params, List<DimensionalObject
for (DimensionalItemObject item : dim.getItems()) {
OrganisationUnit unit = (OrganisationUnit) item;

sql += DIM_NAME_OU + " like '" + unit.getPath() + "%' or ";
sql += DIM_NAME_OU + " like '" + unit.getStoredPath() + "%' or ";
}

sql = TextUtils.removeLastOr(sql) + ") ";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ public boolean checkOrganisationUnitsAssociations(String program, String orgUnit
private Set<String> getUserOrgUnitPaths() {
Set<String> allUserOrgUnitPaths =
currentUserService.getCurrentUserOrganisationUnits().stream()
.map(OrganisationUnit::getPath)
.map(OrganisationUnit::getStoredPath)
.collect(Collectors.toSet());

return allUserOrgUnitPaths.stream()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -315,7 +315,7 @@ private String getDataValuesHql(

hql.append(
params.getOrganisationUnits().stream()
.map(OrganisationUnit::getPath)
.map(OrganisationUnit::getStoredPath)
.map(p -> "ou.path like '" + p + "%'")
.collect(joining(" or ")));

Expand Down Expand Up @@ -546,7 +546,7 @@ private void getDdvOrgUnits(
where
.append(sqlHelper.or())
.append("ou.path like '")
.append(parent.getPath())
.append(parent.getStoredPath())
.append("%'");
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ public Object evaluate(ExpressionParser.ExprContext ctx, CommonExpressionVisitor

if (orgUnit != null) {
for (TerminalNode uid : ctx.UID()) {
if (orgUnit.getPath().contains(uid.getText() + "/")) {
if (orgUnit.getStoredPath().contains(uid.getText() + "/")) {
return true;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,7 @@ public Long getOrganisationUnitHierarchyMemberCount(
+ ")";

Query<Long> query = getTypedQuery(hql);
query.setParameter("path", parent.getPath() + "%").setParameter("object", member);
query.setParameter("path", parent.getStoredPath() + "%").setParameter("object", member);

return query.getSingleResult();
}
Expand Down Expand Up @@ -216,7 +216,7 @@ public List<OrganisationUnit> getOrganisationUnits(OrganisationUnitQueryParams p

if (params.hasParents()) {
for (OrganisationUnit parent : params.getParents()) {
query.setParameter(parent.getUid(), parent.getPath() + "%");
query.setParameter(parent.getUid(), parent.getStoredPath() + "%");
}
}

Expand Down Expand Up @@ -261,7 +261,7 @@ public Map<String, Set<String>> getOrganisationUnitDataSetAssocationMap(
sql += hlp.whereAnd() + " (";

for (OrganisationUnit unit : organisationUnits) {
sql += "ou.path like '" + unit.getPath() + "%' or ";
sql += "ou.path like '" + unit.getStoredPath() + "%' or ";
}

sql = TextUtils.removeLastOr(sql) + ") ";
Expand Down Expand Up @@ -385,7 +385,7 @@ private String buildOrganisationUnitDistinctUidsSql(OrganisationUnitQueryParams
sql += hlp.whereAnd() + " (";

for (OrganisationUnit parent : params.getParents()) {
sql += "o.path like '" + escapeSql(parent.getPath()) + "%'" + " or ";
sql += "o.path like '" + escapeSql(parent.getStoredPath()) + "%'" + " or ";
}

sql = TextUtils.removeLastOr(sql) + ") ";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,10 @@ private QueryWithOrderBy buildProgramInstanceHql(ProgramInstanceQueryParams para

for (OrganisationUnit organisationUnit : params.getOrganisationUnits()) {
ouClause +=
orHlp.or() + "pi.organisationUnit.path LIKE '" + organisationUnit.getPath() + "%'";
orHlp.or()
+ "pi.organisationUnit.path LIKE '"
+ organisationUnit.getStoredPath()
+ "%'";
}

ouClause += ")";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -880,7 +880,11 @@ private String getFromSubQueryJoinOrgUnitConditions(TrackedEntityInstanceQueryPa

OrganisationUnit ou = organisationUnitStore.getByUid(organisationUnit.getUid());
if (ou != null) {
orgUnits.append(orHlp.or()).append("OU.path LIKE '").append(ou.getPath()).append("%'");
orgUnits
.append(orHlp.or())
.append("OU.path LIKE '")
.append(ou.getStoredPath())
.append("%'");
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,7 @@ private List<Predicate> getTrackedEntityDataValueAuditCriteria(
List<Predicate> orgUnitPredicates = new ArrayList<>();

for (OrganisationUnit orgUnit : params.getOrgUnits()) {
orgUnitPredicates.add(builder.like(ou.get("path"), (orgUnit.getPath() + "%")));
orgUnitPredicates.add(builder.like(ou.get("path"), (orgUnit.getStoredPath() + "%")));
}

predicates.add(builder.or(orgUnitPredicates.toArray(Predicate[]::new)));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -272,7 +272,7 @@ private static String createOrgUnitClause(

clause +=
params.getOrganisationUnits().stream()
.map(o -> " ou.path LIKE '" + o.getPath() + "%' OR ")
.map(o -> " ou.path LIKE '" + o.getStoredPath() + "%' OR ")
.collect(Collectors.joining());

return TextUtils.removeLastOr(clause) + " ) ";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -290,7 +290,7 @@ private String getDataValueSql(DataExportParams params) {
sql += "and (";

for (OrganisationUnit parent : params.getOrganisationUnits()) {
sql += "ou.path like '" + parent.getPath() + "%' or ";
sql += "ou.path like '" + parent.getStoredPath() + "%' or ";
}

sql = TextUtils.removeLastOr(sql) + ") ";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1401,7 +1401,7 @@ private String createAccessibleSql(

private String createDescendantsSql(
User user, EventQueryParams params, MapSqlParameterSource mapSqlParameterSource) {
mapSqlParameterSource.addValue(COLUMN_ORG_UNIT_PATH, params.getOrgUnit().getPath());
mapSqlParameterSource.addValue(COLUMN_ORG_UNIT_PATH, params.getOrgUnit().getStoredPath());

if (isProgramRestricted(params.getProgram())) {
return createCaptureScopeQuery(
Expand All @@ -1414,7 +1414,7 @@ private String createDescendantsSql(

private String createChildrenSql(
User user, EventQueryParams params, MapSqlParameterSource mapSqlParameterSource) {
mapSqlParameterSource.addValue(COLUMN_ORG_UNIT_PATH, params.getOrgUnit().getPath());
mapSqlParameterSource.addValue(COLUMN_ORG_UNIT_PATH, params.getOrgUnit().getStoredPath());

String customChildrenQuery =
" AND (ou.hierarchylevel = "
Expand All @@ -1437,7 +1437,7 @@ private String createChildrenSql(

private String createSelectedSql(
User user, EventQueryParams params, MapSqlParameterSource mapSqlParameterSource) {
mapSqlParameterSource.addValue(COLUMN_ORG_UNIT_PATH, params.getOrgUnit().getPath());
mapSqlParameterSource.addValue(COLUMN_ORG_UNIT_PATH, params.getOrgUnit().getStoredPath());

String orgUnitPathEqualsMatchQuery =
" ou.path = :"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,8 @@ public void init(
this.outputDataElementOperand = outputDataElementOperand;

orgUnitLookup =
orgUnits.stream().collect(Collectors.toMap(OrganisationUnit::getPath, Function.identity()));
orgUnits.stream()
.collect(Collectors.toMap(OrganisationUnit::getStoredPath, Function.identity()));
dataElementLookup =
dataElements.stream()
.collect(toMap(DataElement::getId, Function.identity(), (de1, de2) -> de1));
Expand Down Expand Up @@ -359,7 +360,7 @@ private PredictionData getPredictionData(
addValueToMap(dv, map);
}

if (ddv.getSourcePath().equals(dv.getSource().getPath())
if (ddv.getSourcePath().equals(dv.getSource().getStoredPath())
&& ddv.getDataElementId() == outputDataElementOperand.getDataElement().getId()
&& (outputDataElementOperand.getCategoryOptionCombo() == null
|| ddv.getCategoryOptionComboId()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,7 @@ public void delete(Collection<DataElement> dataElements, OrganisationUnit parent

getQuery(hql)
.setParameterList("dataElements", dataElements)
.setParameter("path", parent.getPath() + "%")
.setParameter("path", parent.getStoredPath() + "%")
.executeUpdate();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ public static String getOrgUnitPathClause(List<OrganisationUnit> orgUnits) {
String sql = "(";

for (OrganisationUnit ou : orgUnits) {
sql += "ou.\"path\" like '" + ou.getPath() + "%' or ";
sql += "ou.\"path\" like '" + ou.getStoredPath() + "%' or ";
}

return StringUtils.trim(TextUtils.removeLastOr(sql)) + ")";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -201,7 +201,8 @@ public List<ValidationResult> getValidationResults(
+ "vr.validationRule in :validationRules and vr.period in :periods ");

if (orgUnit != null) {
query.setParameter("orgUnitPath", orgUnit.getPath() + (includeOrgUnitDescendants ? "%" : ""));
query.setParameter(
"orgUnitPath", orgUnit.getStoredPath() + (includeOrgUnitDescendants ? "%" : ""));
}

query.setParameter("validationRules", validationRules);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -671,7 +671,7 @@ private List<ValidationResultView> validationResultsListToResponse(
if (organisationUnit != null) {
validationResultView.setOrganisationUnitId(organisationUnit.getUid());
validationResultView.setOrganisationUnitDisplayName(organisationUnit.getDisplayName());
validationResultView.setOrganisationUnitPath(organisationUnit.getPath());
validationResultView.setOrganisationUnitPath(organisationUnit.getStoredPath());
validationResultView.setOrganisationUnitAncestorNames(organisationUnit.getAncestorNames());
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -307,6 +307,6 @@ public void deleteLockException(
private boolean canCapture(OrganisationUnit captureTarget) {
return currentUserService.currentUserIsSuper()
|| currentUserService.getCurrentUserOrganisationUnits().stream()
.anyMatch(ou -> captureTarget.getPath().startsWith(ou.getPath()));
.anyMatch(ou -> captureTarget.getPath().startsWith(ou.getStoredPath()));
}
}
Loading