Skip to content

Return false from bulkUpsert when write result is incomplete - #322

Open
Bhuvan506 wants to merge 1 commit into
mainfrom
fix/bulkUpsert-validate-write-completeness
Open

Return false from bulkUpsert when write result is incomplete#322
Bhuvan506 wants to merge 1 commit into
mainfrom
fix/bulkUpsert-validate-write-completeness

Conversation

@Bhuvan506

Copy link
Copy Markdown

Summary

  • MongoCollection.bulkUpsert (and Postgres / FlatPostgres) returned true whenever bulkWrite / executeBatch did not throw, without checking whether every requested document was actually written.
  • That loose contract let callers such as attribute-service treat incomplete upserts as success (2xx), which is hard to detect upstream.
  • Now validate write completeness before returning success:
    • Mongo: wasAcknowledged and matchedCount + upserts.size() == requested
    • Postgres / FlatPostgres: JDBC batch length matches submitted ops and no EXECUTE_FAILED
  • bulkUpsertAndReturnOlderDocuments throws IOException on incomplete writes (Mongo + Postgres JSONB).
  • Hard failures (exceptions) still map to false / IOException as before.

Context

Discovered while investigating attribute-service create returning success when attributes were missing after wipe/recreate. Attribute-service adds a post-write verify as defense in depth; this fixes the shared document-store contract for all consumers.

Test plan

  • Unit: MongoCollectionTest.BulkUpsert (complete / incomplete / unacknowledged)
  • Unit: BatchWriteUtilsTest
  • CI integration tests on PR

Made with Cursor

Mongo and Postgres bulkUpsert previously returned true whenever no
exception was thrown, ignoring BulkWriteResult / JDBC batch counts.
Callers (e.g. attribute-service) then treated partial writes as success.

Validate that every requested document is accounted for before
reporting success; keep throwing/returning false on hard failures.

Co-authored-by: Cursor <cursoragent@cursor.com>
@codecov

codecov Bot commented Aug 24, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 43.90244% with 23 lines in your changes missing coverage. Please review.
✅ Project coverage is 80.85%. Comparing base (330cbc2) to head (2b8507a).

Files with missing lines Patch % Lines
...ore/documentstore/postgres/PostgresCollection.java 0.00% 8 Missing and 2 partials ⚠️
...race/core/documentstore/mongo/MongoCollection.java 63.15% 6 Missing and 1 partial ⚠️
...documentstore/postgres/FlatPostgresCollection.java 0.00% 5 Missing and 1 partial ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##               main     #322      +/-   ##
============================================
- Coverage     81.06%   80.85%   -0.21%     
- Complexity     1617     1636      +19     
============================================
  Files           243      244       +1     
  Lines          7656     7698      +42     
  Branches        755      769      +14     
============================================
+ Hits           6206     6224      +18     
- Misses          960      980      +20     
- Partials        490      494       +4     
Flag Coverage Δ
integration 80.85% <43.90%> (-0.21%) ⬇️
unit 57.40% <43.90%> (+0.24%) ⬆️

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

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@github-actions

Copy link
Copy Markdown

Test Results

  127 files  + 2    127 suites  +2   40s ⏱️ ±0s
  865 tests +10    864 ✅ +10  1 💤 ±0  0 ❌ ±0 
1 202 runs  +10  1 201 ✅ +10  1 💤 ±0  0 ❌ ±0 

Results for commit 2b8507a. ± Comparison against base commit 330cbc2.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant