Feature/rfc 6578 - #100
Open
f1-outsourcing wants to merge 12 commits into
Open
Conversation
docs: mark sync-collection suites green, drop RED-phase notes - testcases.md: add implementation-status banner - test javadocs: replace "expected to FAIL" notes with green-status/regression-guard wording - changelog-design: append implementation status + open persistence-verification item
Closes the open item from synccollection-changelog-design.md: the sync-collection change log was so far only exercised through the mock DAO stack. Now verified against a real Hibernate/JPA session + MariaDB: - HibernateModificationDaoTest: field round-trip, revision ordering, findSince exclusive lower bound, nresults truncation keeps oldest pending changes, per-collection isolation, currentRevision monotonicity - AbstractSpringDaoTestCase: allow skipping embedded MariaDB4j and using an external database via COSMO_TEST_EXTERNAL_DB plus the standard SPRING_DATASOURCE_* overrides
add Group A discovery integration tests for sync-collection (RFC 6578)
New SyncCollectionDiscoveryIntegrationTest exercises the capability-discovery
path over the full StandardRequestHandler pipeline (test-only, no production
changes):
- A1: PROPFIND Depth 0 for DAV:supported-report-set on a calendar collection
must return 207 with the property in a 200 propstat advertising
<D:report><D:sync-collection/></D:report> (RFC 6578 sec. 5, RFC 4918
sec. 14.14 discovery mechanism).
- A2: OPTIONS on a calendar collection stays 200 with REPORT in the Allow
header and the existing DAV compliance class (1, calendar-access, ...)
unchanged; pins that RFC 6578 adds no new compliance token and no
OPTIONS extension.
- A2b: regression guard for a behavior discovered along the way: the home
collection's Allow header intentionally omits REPORT (see
DavHomeCollection#getSupportedMethods); the test pins this design
decision so it does not silently change.
All three tests are green against the committed implementation, acting as a
regression net for the previously-untested discovery surface.
test: automate R6578 E1-E4 property-selection cases for sync-collection
Add SyncCollectionPropertySelectionIntegrationTest (5 tests, drives the
full StandardRequestHandler pipeline):
- E1: multiple requested live properties (getetag + getlastmodified)
both appear in a single 200 propstat per member, non-empty.
- E2: an unknown property (urn:example:nope) surfaces as its own 404
propstat and is NOT listed in the 200 propstat, while known
properties are unaffected (overall status stays 207).
- E3 + companion: omitting DAV:prop and sending an empty <D:prop/>
both yield 207 with bare href+status responses (no propstat).
- E4: DAV:allprop is outside the RFC 6578 sync-collection DTD; pinned
the observed behavior - the element is ignored and the request is
treated as an empty property selection (207, href only, no
propstat, no 4xx/5xx).
No production code changed. Update synccollection-testcases.md: E1-E4
listed as automated and the observed property-selection behavior block
added to the implementation-status header.
Verified: mvn -pl cosmo-core -am -Dtest=SyncCollectionPropertySelectionIntegrationTest
-Dsurefire.failIfNoSpecifiedTests=false test -> 5/5 green.
test: automate R6578 F1/F3/F5 limit & truncation cases for sync-collection
Add SyncCollectionLimitIntegrationTest (3 tests, full
StandardRequestHandler pipeline; complements the committed F2
convergence test):
- F1: 10 changes, nresults=4 -> rounds list exactly 4 + 4 + 2 entries,
each truncated round issues a continuation DAV:sync-token, the
union of pages contains all 10 changes with no loss and no
duplication across the truncation boundary, and a final round
after draining reports none.
- F3: nresults=1000 exceeding the 10 pending changes -> all 10
returned in one round; the issued token is fully drained
(re-query reports 0 entries).
- F5: invalid DAV:nresults (negative -5, non-numeric "abc") ->
400 Bad Request.
F4 (server MAY ignore limits) is covered behaviorally by F1, which
pins Cosmo's "honors limits" policy.
No production code changed. Update synccollection-testcases.md
implementation-status block with F1/F3/F5.
Verified: mvn -pl cosmo-core -am -Dtest=SyncCollectionLimitIntegrationTest
-Dsurefire.failIfNoSpecifiedTests=false test -> 3/3 green.
test: G1-G8 DAV:sync-collection error-path integration tests Add SyncCollectionErrorPathIntegrationTest covering seven negative-path cases (G1, G2, G3, G5, G6, G7, G8) from synccollection-testcases.md. Each test drives the full StandardRequestHandler pipeline against a logged-in fixture user and pins the exact 4xx status the production code emits: G1: unsupported report root → 422 (DavResourceBase#getReport) G2: nonexistent target → 404 (NotFoundException) G3: non-collection member → 4xx (doQuerySelf guard) G5: malformed XML body → 400 (getSafeRequestDocument) G6: missing DAV:sync-level → 400 (parseReport) G7: DAV:sync-level=2 → 400 (level-1 only) G8: DAV:sync-level=infinity → 400 (level-1 only) G4 (missing body), G9 (stale token), G10 (cross-collection token), G11 (garbage token), G12 (401), G13 (no read privilege) are documented as already covered or out-of-scope in the class Javadoc
fix: entity converter NPE on notes without triage code; test: B3/B5
EntityConverter: triage comparison auto-unboxed a null Integer when a
note has no triage code (TriageStatus.getCode() is Integer, CODE_DONE
is a primitive int), surfacing as a 500 on any report that materialized
member calendars. Use a null-safe Integer equals instead.
Add SyncCollectionScopeAndCalendarDataIntegrationTest covering:
B3: initial sync on a collection with a nested sub-collection lists
the sub-collection but not its grand-child (DAV:sync-level 1 scope)
B5: CALDAV:calendar-data on the sync-collection path surfaces as a
404 propstat per member while DAV:getetag still 200s and the
overall response stays 207 with a usable sync-token (SyncCollec-
tionReport extends plain MultiStatusReport, deliberately not
CaldavMultiStatusReport; see class Javadoc for the design note)
Update synccollection-testcases.md with the B3/B5 acceptance criteria.
test: D2/D3 incremental sync-collection cases; spec: D-group annotations
cosmo-core/src/main/java/.../dav/report/SyncCollectionReport.java (tombstone dedup — the one production change) cosmo-core/src/test/.../SyncCollectionIncrementalSyncIntegrationTest.java (D2/D3, C5/C6/C7) cosmo-core/src/test/.../SyncCollectionConcurrencyRobustnessIntegrationTest.java (H-group + both fixes above) synccollection-testcases.md (spec, C5 deviation)
test(sync-collection): add I-group (Cosmo-specific) integration tests + finalize spec
fix: emit RFC 6578 change-log entries (D+C) on cross-collection MOVE
StandardContentService#moveItem now writes a DAV:sync-collection
tombstone (D-row) at the source collection and a live member (C-row)
at the destination, per RFC 6578 §3.2/§5 (a rename/move is reported
as delete at the old href + create at the new href).
- D-row logged before removeItemFromCollection (source tombstone)
- C-row logged after addItemToCollection (destination live member)
- Guarded by crossCollection so same-parent renames (single M-row via
updateContent) do not double-log
- Applied in both the locking ContentItem branch and the plain branch
Test (I2):
- Rewrote crossCollectionMoveEmitsTombstoneInSourceAndLiveMemberInTarget
to lock the RFC-compliant shape: exactly one bare-404 tombstone in
A's incremental sync, exactly one live 200 getetag entry in B's
- Fixed three test-bug helpers:
* OutputKeys is a top-level class (not Transformer.OutputKeys)
* hrefStartsWithPrefix now decodes the percent-encoded collectionUri
* B-side 200 lives INSIDE the propstat (Cosmo/RFC 4918 shape), not
as a response-level DAV:status
Docs: rfc-compliance.md (Known Limitations mam-dev#1, Roadmap mam-dev#5) and
synccollection-testcases.md (I2 rows, notes, status, traceability)
updated to reflect the implemented behavior.
Full cosmo-core suite: 395/395 green.
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
2 participants
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.
Hi @danielgrigore Don't merge this pull request. I am just curious what you think about this ai generated code. Is it good, sort of ok or just crap. Maybe this is an interesting solution to add rfc support on the existing code?