fix(firestore): enforce write preconditions and transaction conflicts - #114
Open
tdewitt wants to merge 3 commits into
Open
fix(firestore): enforce write preconditions and transaction conflicts#114tdewitt wants to merge 3 commits into
tdewitt wants to merge 3 commits into
Conversation
|
| Filename | Overview |
|---|---|
| src/main/java/io/floci/gcp/services/firestore/FirestoreService.java | Adds synchronized precondition validation, atomic validation-before-application, transaction read-set tracking, conflict detection, and lifecycle cleanup without an accepted defect. |
| src/main/java/io/floci/gcp/services/firestore/FirestoreController.java | Routes preconditions and transaction selectors through the service, records returned snapshot versions, and emits per-write BatchWrite statuses. |
| src/main/java/io/floci/gcp/core/common/GcpException.java | Adds the expected HTTP and gRPC mapping for transaction abort errors. |
| src/test/java/io/floci/gcp/services/firestore/FirestoreServiceTest.java | Adds focused coverage for preconditions, atomic validation, conflicts, racing reads, retries, rollback, and malformed stored timestamps. |
| compatibility-tests/sdk-test-java/src/test/java/io/floci/gcp/test/FirestoreTest.java | Adds Java SDK compatibility coverage for precondition errors and automatic transaction retry behavior. |
| docs/services/firestore.md | Documents preconditions, optimistic concurrency, atomicity semantics, and intentional emulator deviations. |
Sequence Diagram
sequenceDiagram
participant Client
participant Controller as FirestoreController
participant Service as FirestoreService
participant Store as DocumentStore
Client->>Controller: BeginTransaction
Controller->>Service: beginTransaction()
Service-->>Client: transaction ID
Client->>Controller: Transactional read
Controller->>Store: Read document snapshot
Controller->>Service: recordTransactionRead(id, name, version)
Controller-->>Client: Snapshot
Client->>Controller: Commit(writes, transaction ID)
Controller->>Service: commit(writes, id)
Service->>Store: Validate recorded versions
alt Read version changed
Service-->>Client: ABORTED
else Versions and preconditions valid
Service->>Store: Apply all writes
Service-->>Client: CommitResponse
end
Reviews (1): Last reviewed commit: "fix(firestore): record transaction reads..." | Re-trigger Greptile
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #113
The Firestore emulator applied every write unconditionally:
Write.current_documentpreconditions were ignored andCommitnever validated its transaction, so double-creates succeeded, stale preconditions passed silently, and conflicting transactions all committed (losing updates).current_documentpreconditions on all write paths —exists=false→ALREADY_EXISTS,exists=true→NOT_FOUND, staleupdate_time→FAILED_PRECONDITION;CreateDocument/UpdateDocument/DeleteDocumentpass their preconditions throughBeginTransactionregisters server-side state, transactional reads (GetDocument,BatchGetDocuments,RunQuery, incl.new_transaction) record the returned snapshot's version, andCommitaborts withABORTEDif any read document changed;Rollbackdiscards stateCommitand theWritestream are atomic per request (preconditions validated before any write lands);BatchWriteis non-atomic with per-writestatus, matching real FirestoreGcpException.aborted()(gRPCABORTED, HTTP 409)Intentional deviations (documented in
docs/services/firestore.md): query read sets cover returned documents only (no phantom-read detection),RunAggregationQueryignores transactions, transaction state is in-memory with a 15-minute TTL.Validated with the Java SDK against a running emulator: create/update precondition failures surface as SDK exceptions, and a transaction with a conflicting concurrent write is aborted and retried by the SDK exactly once with no lost update.
Type of change
fix:)feat:)feat!:orfix!:)Checklist
./mvnw testpasses locally