mirror of
https://github.com/runbear-io/beardrive.git
synced 2026-08-25 08:08:08 +02:00
fix(cli): bdrive log sorts by when a change arrived (BEA-112) (#174)
`mv a.md b.md` produced two rows minutes apart: the delete carried the rename's time, the put carried the original file's mtime, so a file that appeared seconds ago sorted below the fold of "what changed since yesterday" — the question `bdrive log` exists to answer. SortForDisplay now orders by CommitTime (when the change was journaled), tie-breaking on DisplayTime so one scan still reads by the files' own edit times. The write time is still shown, appended as `written <time>` when it lags the commit by more than a minute — a rename, or an old document added today — rather than silently replacing the column. DisplayTime and both of its security clamps are untouched. CommitTime carries the same clamp: an op stamped after this machine's clock cannot date itself, so it sorts last rather than first. One deviation from the plan: it assumed a scan shares one commit time, but nextOp stamped time.Now() per op, so the tie-break never engaged and one scan sorted in walk order. A scan is now one commit instant — order inside the batch is already carried by Lamport and Seq, which journal.Less reads first, so replay is unaffected. journal.Less, Replay, LogEntries' causal order and the op format are unchanged. The hub's History is a separate path and still orders a rename by write time.
This commit is contained in:
@@ -649,11 +649,18 @@ func (s *Session) scan(cache map[string]store.CachedFile, st *store.SyncState, s
|
||||
if note == "" {
|
||||
note = s.Store.LoadNote()
|
||||
}
|
||||
// One scan is one commit: every op it produces carries the same Time. Op
|
||||
// order inside the batch is already carried by Lamport and Seq, which
|
||||
// journal.Less consults first and second — so replay is unaffected — while
|
||||
// a shared Time is what lets `bdrive log` recognise a batch and order it by
|
||||
// the files' own write times (SortForDisplay). Stamping each op separately
|
||||
// made a rename's two halves two different instants, which is the bug.
|
||||
committed := time.Now().UTC()
|
||||
nextOp := func(kind, rel string) journal.Op {
|
||||
st.Lamport = tickLamport(st.Lamport)
|
||||
seqBase++
|
||||
return journal.Op{
|
||||
Seq: seqBase, Lamport: st.Lamport, Time: time.Now().UTC(),
|
||||
Seq: seqBase, Lamport: st.Lamport, Time: committed,
|
||||
Device: s.Device.ID, DeviceName: s.Device.Name, Author: s.Device.Author,
|
||||
User: s.Account.Email, UserName: s.Account.Name,
|
||||
Kind: kind, Path: rel, Note: note, Session: s.SessionID,
|
||||
@@ -1700,17 +1707,40 @@ func DisplayTime(op journal.Op) time.Time {
|
||||
return op.Time
|
||||
}
|
||||
|
||||
// SortForDisplay orders ops newest-first by DisplayTime — the timestamp the
|
||||
// user actually sees, so the list reads as a timeline. Ties fall back to
|
||||
// reversed journal.Less to stay deterministic. This is deliberately NOT the
|
||||
// replay order: LogEntries keeps returning causal order because
|
||||
// bdrive restore walks it to find a file's previous version.
|
||||
// CommitTime is when a change entered the project, which is the question
|
||||
// `bdrive log` answers. It is not DisplayTime: `mv` preserves mtime, so the put
|
||||
// half of a rename carries the original file's write time and sorts away from
|
||||
// the delete half of the same rename — a file that appeared seconds ago lands
|
||||
// below the fold.
|
||||
//
|
||||
// Same clamp as DisplayTime, for the same reason: Op.Time is a peer's JSON, and
|
||||
// the one clock a peer does not own is this machine's. An op we cannot date
|
||||
// sorts last rather than first, which is the direction that cannot be aimed.
|
||||
func CommitTime(op journal.Op) time.Time {
|
||||
if op.Time.After(time.Now()) {
|
||||
return time.Time{}
|
||||
}
|
||||
return op.Time
|
||||
}
|
||||
|
||||
// SortForDisplay orders ops newest-first by CommitTime — when the change
|
||||
// entered the project — so the two halves of a rename sit together and an old
|
||||
// file added today sorts by when it arrived. DisplayTime breaks ties and is
|
||||
// load-bearing: everything from one scan shares a commit second, and the files'
|
||||
// own write times are the only thing that orders it inside that second. Ties
|
||||
// beyond that fall back to reversed journal.Less to stay deterministic. This is
|
||||
// deliberately NOT the replay order: LogEntries keeps returning causal order
|
||||
// because bdrive restore walks it to find a file's previous version.
|
||||
func SortForDisplay(ops []journal.Op) {
|
||||
sort.SliceStable(ops, func(i, j int) bool {
|
||||
ti, tj := DisplayTime(ops[i]), DisplayTime(ops[j])
|
||||
ti, tj := CommitTime(ops[i]), CommitTime(ops[j])
|
||||
if !ti.Equal(tj) {
|
||||
return ti.After(tj)
|
||||
}
|
||||
di, dj := DisplayTime(ops[i]), DisplayTime(ops[j])
|
||||
if !di.Equal(dj) {
|
||||
return di.After(dj)
|
||||
}
|
||||
return journal.Less(ops[j], ops[i])
|
||||
})
|
||||
}
|
||||
|
||||
@@ -682,10 +682,16 @@ func touch(t *testing.T, folder, rel string, when time.Time) {
|
||||
}
|
||||
}
|
||||
|
||||
// TestLogDisplayOrder builds exactly the skew that made `bdrive log`
|
||||
// unreadable: device B commits after pulling A, so B's op carries the HIGHER
|
||||
// lamport, while B's file was written hours EARLIER on the wall clock. Causal
|
||||
// order and clock order disagree — the display sort must follow the clock.
|
||||
// TestLogDisplayOrder builds the skew that made `bdrive log` unreadable:
|
||||
// device B commits after pulling A, so B's op carries the HIGHER lamport,
|
||||
// while B's file was written hours EARLIER on the wall clock.
|
||||
//
|
||||
// The display sort follows the commit clock: early.md was written two hours
|
||||
// ago but only ARRIVED in the project on B's cycle, and "what changed" means
|
||||
// what arrived. Its own write time still reaches the reader — `bdrive log`
|
||||
// prints it alongside — but it is not what orders the list. Before BEA-112
|
||||
// the write time was the sort key, which is how the two halves of one rename
|
||||
// landed a minute apart.
|
||||
func TestLogDisplayOrder(t *testing.T) {
|
||||
be := sharedRemote(t)
|
||||
a, b := newDevice(t, "deva", be), newDevice(t, "devb", be)
|
||||
@@ -712,8 +718,12 @@ func TestLogDisplayOrder(t *testing.T) {
|
||||
}
|
||||
|
||||
SortForDisplay(entries)
|
||||
if entries[0].Path != "late.md" {
|
||||
t.Fatalf("display order should lead with the newest file late.md, got %q", entries[0].Path)
|
||||
if entries[0].Path != "early.md" {
|
||||
t.Fatalf("display order should lead with the most recently journaled file early.md, got %q", entries[0].Path)
|
||||
}
|
||||
// Its write time is two hours old and still available to print.
|
||||
if gap := entries[0].Time.Sub(DisplayTime(entries[0])); gap < time.Hour {
|
||||
t.Fatalf("early.md's write time was lost: commit %v, display %v", entries[0].Time, DisplayTime(entries[0]))
|
||||
}
|
||||
assertNonIncreasing(t, entries)
|
||||
}
|
||||
@@ -780,7 +790,10 @@ func TestSortForDisplayFallsBackToTime(t *testing.T) {
|
||||
}
|
||||
SortForDisplay(ops)
|
||||
|
||||
want := []string{"legacy-new.md", "gone.md", "fresh.md", "legacy-old.md"}
|
||||
// Ordered by commit time. legacy ops and deletes carry no mtime, and the
|
||||
// point of the fallback is that they sort on their commit time like
|
||||
// everything else rather than sinking to the bottom as zero-time rows.
|
||||
want := []string{"fresh.md", "legacy-new.md", "gone.md", "legacy-old.md"}
|
||||
for i, w := range want {
|
||||
if ops[i].Path != w {
|
||||
t.Fatalf("order[%d] = %q, want %q (full: %v)", i, ops[i].Path, w, paths(ops))
|
||||
@@ -797,12 +810,16 @@ func paths(ops []journal.Op) []string {
|
||||
return out
|
||||
}
|
||||
|
||||
// assertNonIncreasing pins BEA-40's guarantee on the key the display sort now
|
||||
// uses: `bdrive log` reads strictly newest-first by commit time, the column it
|
||||
// prints. DisplayTime is deliberately not monotone down the list — an old file
|
||||
// journaled today belongs at the top wearing its old write time.
|
||||
func assertNonIncreasing(t *testing.T, ops []journal.Op) {
|
||||
t.Helper()
|
||||
for i := 1; i < len(ops); i++ {
|
||||
if DisplayTime(ops[i]).After(DisplayTime(ops[i-1])) {
|
||||
if CommitTime(ops[i]).After(CommitTime(ops[i-1])) {
|
||||
t.Fatalf("display order not newest-first at %d: %v then %v",
|
||||
i, DisplayTime(ops[i-1]), DisplayTime(ops[i]))
|
||||
i, CommitTime(ops[i-1]), CommitTime(ops[i]))
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user