Skip to content

[MS-929] Fix 1:N pool count for CoSync#1128

Merged
BurningAXE merged 2 commits into
mainfrom
MS-929-1-N-pool-count-is-incorrect-for-CoSync
Mar 6, 2025
Merged

[MS-929] Fix 1:N pool count for CoSync#1128
BurningAXE merged 2 commits into
mainfrom
MS-929-1-N-pool-count-is-incorrect-for-CoSync

Conversation

@BurningAXE
Copy link
Copy Markdown
Contributor

@BurningAXE BurningAXE commented Mar 5, 2025

MS-929

Problem
Both the fingerprint and face matcher get the pool size (recorded in OneToManyMatch.MatchPool.count) by calling:
val totalCandidates = enrolmentRecordRepository.count(queryWithSupportedFormat, dataSource = matchParams.biometricDataSource)
This is fine when using the local DB. However, when using CommCare as the biometric data source, we cannot use any of the query’s filtering criteria so instead we just count the whole ContentProvider URI. This means we get the count of all cases in CommCare, regardless of their module or even whether they have biometric data attached or not. This might often lead to incorrect pool size reported in the OneToManyMatch event.

Fix
The fix contains two parts:

  1. In the matchers count the number of actually loaded candidates and report that upstream so it ends in the OneToManyMatch event
  2. In CommCareIdentityProvider move the onCandidateLoaded callback after filtering so that it only count actually loaded candidates (similar to how the local DB works)

@cla-bot cla-bot Bot added the ... label Mar 5, 2025
@BurningAXE BurningAXE marked this pull request as ready for review March 5, 2025 17:43
@BurningAXE BurningAXE requested review from a team, TristramN, alex-vt, alexandr-simprints, luhmirin-s, meladRaouf and ybourgery and removed request for a team March 5, 2025 17:43
@BurningAXE BurningAXE force-pushed the MS-929-1-N-pool-count-is-incorrect-for-CoSync branch from ba2fc33 to c496c93 Compare March 6, 2025 14:31
@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud Bot commented Mar 6, 2025

@BurningAXE BurningAXE merged commit 406aa43 into main Mar 6, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants