Fix same-size local updates being skipped during folder sync

[RubenSDev10]

Fixes #39.

Problem

Folder sync's database fast path only checked that the remote file still matched the stored remote metadata and that the current local size matched the remote size.

Replacing a local file with different content at the same path and with the same size was therefore treated as unchanged, even when its local modification time had changed. The remote copy remained stale.

Fix

• Persist the local size and modification time alongside the remote sync baseline.
• Use the fast path only when both the local and remote sides still match the last successful sync.
• Upload when the remote side is unchanged but the local metadata changed, including same-size updates.
• Preserve the existing size/timestamp and newer-side fallback when there is no conclusive baseline.
• Add a Room migration from schema version 1 to 2 without discarding existing remote state.

This remains a metadata-based comparison. A source that changes content while preserving both its size and exact modification time remains indistinguishable without content hashing.

Validation

• Added focused tests for unchanged state, same-size local updates, local size changes, remote changes, timestamp tolerance, and legacy rows.
• Added a real Room 1-to-2 migration test.
• Updated SyncStateStoreTest to verify both local and remote metadata.
• spotlessCheck, debug compilation, and lintDebug pass.
• The focused sync and migration tests pass.
• The remaining unit/integration suite and jacocoTestReport pass when partitioned to avoid Testcontainers churn. SmbRepositoryTest also passes in full when run separately after one transient container port timeout.

The repository's existing GuestLoginIntegrationTest.guestLogin_withMapToGuestBadUser_succeeds currently fails identically on an untouched main checkout because the mutable dperson/samba:latest container does not expose the expected guestshare.

[egdels]

Thank you.

The approach looks reasonable to me. One case I'd like to see covered explicitly is the common SAF scenario where preserving the remote modification time after a download fails. In that case, the local and remote timestamps can legitimately differ after a successful sync. Please add a regression test showing that a baseline such as local mtime=5000 / remote mtime=1000 is treated as unchanged on the next sync, so no transfer is triggered.

[egdels]

I reproduced the failing GuestLoginIntegrationTest on an unchanged older revision. Pinning the test image from dockurr/samba:latest to dockurr/samba:4.23.8 makes the test pass again, so this appears to be caused by a change in the upstream container image rather than by this PR.

[egdels]

Hi @RubenSDev10, just curious - was there a particular reason you decided to close this PR after adding the requested SAF regression test? The approach looked like it was close to being ready. Thanks!

[RubenSDev10]

@egdels I closed it temporarily while fixing the branch history. That's sorted now, so I've reopened it and marked it ready for review. Thanks for checking.

[egdels]

@RubenSDev10 Thanks a lot. It looks good! I’ve got a few other things going on at the moment, so I won’t be able to take a proper look at it right away. I’ll need a little time.

[egdels]

This branch uploads unconditionally whenever local metadata differs from the
stored baseline, without the isLocalNewer/isRemoteNewer check every other
branch here uses. That's a direct deviation from the documented behavior in
the User Guide:

Local → Remote: "New or modified local files overwrite older remote
versions."
Conflict Resolution: "the file with the more recent modification timestamp
overwrites the older one."

Concretely: if a local file is reverted to an older revision (restored from
a backup, trash-restore, etc.) so its metadata no longer matches the
baseline, while remote hasn't changed, this branch uploads the older local
content over the newer remote content - silently losing the remote version.

I wrote a unit test against SyncStateComparator that reproduces this and
fails on the current code:

@Test                                                                                                                                                                                                                       
public void localChangedWhileRemoteMatches_mustNotAuthorizeOverwritingWithOlderLocalContent() {                                                                                                                             
    long olderLocalModified = INITIAL_MODIFIED - 10_000;                                                                                                                                                                      
    assertFalse(                                                                                                                                                                                                              
        SyncStateComparator.localChangedWhileRemoteMatches(                                                                                                                                                                   
            state, 512, olderLocalModified, 1024, INITIAL_MODIFIED));                                                                                                                                                         
}    

Since BIDIRECTIONAL calls syncLocalToRemote too, this also affects
bidirectional syncs, not just one-way Local→Remote.

To be clear, this wouldn't conflict with #39: the issue's own repro steps state
"Its local modification time should be newer" after replacing the file, so
adding an isLocalNewer check here doesn't narrow the fix - it only excludes
a case the bug report never asked for (local becoming older), while still
covering the exact same-size/newer-content scenario from #39.

Could you add a timestamp-direction guard here (or fold it into
SyncStateComparator) before merging?

[egdels]

localMatches() requires exact equality on localLastModified, while
remoteMatches() (a few lines below) applies the 3s
DEFAULT_TIMESTAMP_TOLERANCE_MS. The User Guide explains that tolerance as
accounting for "network jitter and filesystem differences" in general - and
we already know locally-read SAF timestamps aren't always stable across
reads either (that's why we added the bothMatch_whenSafCannotPreserveRemoteTimestampAfterDownload
test for the post-download case).

Without the same tolerance here, a local SAF provider that returns a
slightly different lastModified() for the very same untouched file will
trigger an unnecessary re-upload every sync cycle (and, combined with the
issue above, feed into that unconditional-upload branch on every jitter
occurrence rather than only on rare restores).

Test that currently fails:

@Test                                                                                                                                                                                                                       
public void localMatches_mustToleratesTimestampJitterLikeRemoteMatchesDoes() {                                                                                                                                              
    long jitteredLocalModified =                                                                                                                                                                                              
        INITIAL_MODIFIED + SyncComparator.DEFAULT_TIMESTAMP_TOLERANCE_MS - 1;                                                                                                                                                 
    assertFalse(                                                                                                                                                                                                              
        SyncStateComparator.localChangedWhileRemoteMatches(                                                                                                                                                                   
            state, 1024, jitteredLocalModified, 1024, INITIAL_MODIFIED));                                                                                                                                                     
}    

Should localMatches() apply the same tolerance as remoteMatches()?

[egdels]

Thanks again for this - the underlying fix (not skipping same-size content
changes) is correct and the SAF-timestamp-divergence handling you added is
exactly right.

I found two issues in the new fast-path logic while reviewing against the
sync behavior documented in our User Guide (conflict resolution / "Newer
Wins"), left as inline comments:

1. The localChangedWhileRemoteMatches upload branch skips the
   newer-than-remote check that every other branch enforces, which can
   overwrite a newer remote file with older local content.
2. localMatches() has no timestamp tolerance while remoteMatches() does,
   which can cause spurious re-uploads and increases how often issue (1)
   can fire.

Note that neither of these would conflict with #39 - its repro steps explicitly
assume the local modification time is newer after the edit, so both
guards are purely additive on top of the current fix.

I've added two unit tests (in the inline comments) that reproduce both
issues against the current SyncStateComparator - happy to see them land in
the PR if useful. Marking as request changes for now since (1) is a
potential silent data-loss path; let me know if you'd like to discuss
approaches before you dig in.