fix: Remove unnecessary calls to the DB during DataValueSet import [DHIS2-16138]

[david-mackessy]

Summary

During a DataValueSet import, calls are made to the DB to get OrganisationUnits. When there are large amounts of OrganisationUnits this can lead to the import taking a long time.
v38 did not experience these issues.

Cause

During a large refactor PR the implementation that was working well was changed.

• In v38 code we check OrganisationUnits that are already in-memory (ImportContext).
• Post v38 we check OrganisationUnits using a service call which in turn calls the DB.

Fix

Revert back to original implementation, checking the OrganisationUnits in memory.
Needs backporting to:

• 2.39
• 2.40

Testing

The data below is based on following the steps provided in the Jira issue.
Running locally on a Mac.

Version	DB	Org Check	Import time
2.38	SL	In memory	~5 seconds
2.39	SL	In memory	~5 seconds
2.39	SL	DB	~5 minutes
2.40	SL	In memory	~5 seconds
2.40	SL	DB	~5 minutes
dev (master)	SL	In memory	~5 seconds
dev (master)	SL	DB	~5 minutes

Also confirmed in the DB logs that the DB query which was being called for OrganisationUnit was not called anymore during the import, after the fix.

The test setup in DataValueSetImportValidatorTest had to be updated to reflect the change. Instead of using a mock response to always return true no matter what, the test now checks the actual OrganisationUnit path, mirroring actual behaviour in the code.

Notes

During the large refactor PR there was a similar change made to this method:

  private boolean isOrgUnitValidForAttrOptionCombo(DataValueContext valueContext) {
    Set<OrganisationUnit> aocOrgUnits = valueContext.getAttrOptionCombo().getOrganisationUnits();
    return aocOrgUnits == null
        || organisationUnitService.isDescendant(valueContext.getOrgUnit(), aocOrgUnits);
  }

As the reported issue seems to have been resolved we won't include any changes in this method. We will look into this however to prove if similar changes are required or not (even though it seems like an obvious change).
I did test changing it to use the in-memory & it had no difference with the execution time but that may be due to the data being used.

[codecov]

Codecov Report

Merging #15639 (73d1d89) into master (ae674bb) will decrease coverage by 0.01%.
Report is 2 commits behind head on master.
The diff coverage is 100.00%.

@@             Coverage Diff              @@
##             master   #15639      +/-   ##
============================================
- Coverage     66.20%   66.20%   -0.01%     
+ Complexity    31264    31260       -4     
============================================
  Files          3487     3487              
  Lines        129902   129901       -1     
  Branches      15170    15170              
============================================
- Hits          86007    85997      -10     
- Misses        36813    36819       +6     
- Partials       7082     7085       +3     

Flag	Coverage Δ
integration	49.77% <100.00%> (-0.01%)	⬇️
integration-h2	32.40% <100.00%> (+<0.01%)	⬆️
unit	30.32% <100.00%> (-0.01%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files	Coverage Δ
...dxf2/datavalueset/DataValueSetImportValidator.java	92.76% <100.00%> (-0.02%)	⬇️

... and 6 files with indirect coverage changes

---

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update dae3308...73d1d89. Read the comment docs.

[sonarcloud]

Kudos, SonarCloud Quality Gate passed!   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

0.0% Coverage
0.0% Duplication