Skip to content

Commit 57acec4

Browse files
adibhannaadibhanna
committed
Fix(workflows): a step that edits or creates a note leaves the note's creation date alone
Since creation dates moved into sidecars (b6c3b9d, 2.51), a note the app has never saved has no date file and shows its file's birth time. Saving replaces the file (temp file plus rename), so writeNote writes the date down first. The applier's text ops (add-tag, remove-tag, set-frontmatter, append, prepend, write-section, write, apply-template) replaced the file the same way without doing that: every such note they touched took the run's time as its creation date, and undo, which writes the bytes back as a new file, reset it again to the undo's time. Found while giving the applier sidecars (1cdb0e8), confirmed in a throwaway test (the date moved by exactly the wait before the run) and reproduced in the built app before this change. keepCreationDate writes the date down through prepareNoteCreation when the note has no date file, and both writers call it before they journal: applyTextOpToVault for an existing note, and sidecarsToCarry for a move, where the block that did this inline moves into the helper. A date file already there is left alone, readable or not, so a corrupt one cannot fail a run the way it fails a save; nothing is written in a temporary folder session, which writeNote never writes app state into either. It is the one write not journalled first, safely: it records a date the note already had, where the app already looks for it. A note the run creates keeps its own birth time. A date file left behind by an earlier note of that name is journalled as a sidecar entry and removed, as createNote removes it, so undo puts it back with the rest. journalPathOp becomes journalOp and absorbs journalTouch, since a text op now journals a sidecar too. Eleven tests cover six text ops through the run and the undo, the date written down before the note, a date file left alone, a temporary folder session, a created note over a leftover and its undo, and a created note with no date file of its own; eight fail on 25f4040.
1 parent 25f4040 commit 57acec4

2 files changed

Lines changed: 170 additions & 42 deletions

File tree

‎apps/desktop/src/main/workflow-apply.test.ts‎

Lines changed: 111 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -971,6 +971,117 @@ describe("a note's sidecars travel with it", () => {
971971
})
972972
})
973973

974+
/* -------------------------------------------------------------------------- */
975+
/* A note's creation date */
976+
/* -------------------------------------------------------------------------- */
977+
978+
// A saved note keeps its creation date in a date file; one ZenNotes never saved
979+
// shows its file's birth time, and an atomic write replaces that file with one
980+
// born now. The editor's save writes the date down first. The applier did not,
981+
// so every text op gave a note the run's time as its creation date.
982+
describe('a note keeps its creation date', () => {
983+
/** What the app would be left with and no date file: the fallback. */
984+
const NO_DATE_FILE = -1
985+
986+
const textOps: Array<[string, WorkflowOp]> = [
987+
['append', { kind: 'append', path: 'inbox/A.md', text: 'more' }],
988+
['prepend', { kind: 'prepend', path: 'inbox/A.md', text: 'first' }],
989+
['add-tag', { kind: 'add-tag', path: 'inbox/A.md', tag: 'filed' }],
990+
['set-frontmatter', { kind: 'set-frontmatter', path: 'inbox/A.md', field: 'status', value: 'done' }],
991+
['write-section', { kind: 'write-section', path: 'inbox/A.md', heading: 'Log', text: 'entry' }],
992+
['write-note', { kind: 'write-note', path: 'inbox/A.md', text: 'replaced\n' }]
993+
]
994+
995+
it.each(textOps)('through %s with no date file yet, and through the undo', async (_kind, op) => {
996+
const root = await makeVault()
997+
await seed(root, 'inbox/A.md', 'a\n')
998+
const born = await stat(path.join(root, 'inbox', 'A.md'))
999+
const createdAt = Math.trunc(born.birthtimeMs || born.ctimeMs)
1000+
1001+
const receipt = await apply(root, [op])
1002+
expect(receipt.rolledBack).toBeUndefined()
1003+
expect(await readOrNull(root, 'inbox/A.md')).not.toBe('a\n')
1004+
expect(await readNoteCreatedAt(root, 'inbox/A.md', NO_DATE_FILE)).toBe(createdAt)
1005+
1006+
await undoWorkflowRun(root, receipt.runId)
1007+
1008+
expect(await readOrNull(root, 'inbox/A.md')).toBe('a\n')
1009+
expect(await readNoteCreatedAt(root, 'inbox/A.md', NO_DATE_FILE)).toBe(createdAt)
1010+
})
1011+
1012+
it('has the date written down before the note is', async () => {
1013+
const root = await makeVault()
1014+
await seed(root, 'inbox/A.md', 'a\n')
1015+
let dateFileAtWrite: string | null = null
1016+
injected.beforeWrite = async (abs) => {
1017+
if (abs.endsWith(path.join('inbox', 'A.md')) && dateFileAtWrite === null) {
1018+
dateFileAtWrite = await readOrNull(root, metadataRel('inbox/A.md'))
1019+
}
1020+
}
1021+
1022+
await apply(root, [{ kind: 'append', path: 'inbox/A.md', text: 'more' }])
1023+
1024+
expect(dateFileAtWrite).not.toBeNull()
1025+
})
1026+
1027+
it('leaves a date file that is already there alone, one it cannot read included', async () => {
1028+
const root = await makeVault()
1029+
await seed(root, 'inbox/A.md', 'a\n')
1030+
await seed(root, 'inbox/B.md', 'b\n')
1031+
await seed(root, metadataRel('inbox/A.md'), CREATED)
1032+
await seed(root, metadataRel('inbox/B.md'), 'not a date\n')
1033+
1034+
const receipt = await apply(root, [
1035+
{ kind: 'append', path: 'inbox/A.md', text: 'more' },
1036+
{ kind: 'append', path: 'inbox/B.md', text: 'more' }
1037+
])
1038+
1039+
// A save refuses a date file it cannot read; a run does not fail over one.
1040+
expect(receipt.rolledBack).toBeUndefined()
1041+
expect(await readOrNull(root, metadataRel('inbox/A.md'))).toBe(CREATED)
1042+
expect(await readOrNull(root, metadataRel('inbox/B.md'))).toBe('not a date\n')
1043+
})
1044+
1045+
it('writes no date file in a temporary folder session', async () => {
1046+
const root = await makeVault()
1047+
registerEphemeralRoot(root)
1048+
try {
1049+
await seed(root, 'inbox/A.md', 'a\n')
1050+
await apply(root, [{ kind: 'append', path: 'inbox/A.md', text: 'more' }])
1051+
expect(await readOrNull(root, metadataRel('inbox/A.md'))).toBeNull()
1052+
} finally {
1053+
unregisterEphemeralRoot(root)
1054+
}
1055+
})
1056+
1057+
it('a created note has its own birth time, not a date left behind, and undo brings that back', async () => {
1058+
const root = await makeVault()
1059+
const leftover = '{"version":1,"createdAt":1600000000000}\n'
1060+
await seed(root, metadataRel('inbox/New.md'), leftover)
1061+
1062+
const receipt = await apply(root, [{ kind: 'create-note', path: 'inbox/New.md', body: 'new' }])
1063+
const ledger = await readLedger(root, receipt.runId)
1064+
1065+
expect(await readOrNull(root, metadataRel('inbox/New.md'))).toBeNull()
1066+
expect(await readNoteCreatedAt(root, 'inbox/New.md', NO_DATE_FILE)).toBe(NO_DATE_FILE)
1067+
expect(ledger.sidecars).toEqual([
1068+
{ note: 'inbox/New.md', sidecar: 'metadata', before: leftover, after: null }
1069+
])
1070+
1071+
await undoWorkflowRun(root, receipt.runId)
1072+
1073+
expect(await readOrNull(root, 'inbox/New.md')).toBeNull()
1074+
expect(await readOrNull(root, metadataRel('inbox/New.md'))).toBe(leftover)
1075+
})
1076+
1077+
it('a created note gets no date file of its own', async () => {
1078+
const root = await makeVault()
1079+
await apply(root, [{ kind: 'create-note', path: 'inbox/New.md', body: 'new' }])
1080+
expect(await readOrNull(root, 'inbox/New.md')).not.toBeNull()
1081+
expect(await readOrNull(root, metadataRel('inbox/New.md'))).toBeNull()
1082+
})
1083+
})
1084+
9741085
/* -------------------------------------------------------------------------- */
9751086
/* Rollback */
9761087
/* -------------------------------------------------------------------------- */

‎apps/desktop/src/main/workflow-apply.ts‎

Lines changed: 59 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -943,21 +943,6 @@ function journalKey(rel: string): string {
943943
return process.platform === 'darwin' || process.platform === 'win32' ? rel.toLowerCase() : rel
944944
}
945945

946-
/**
947-
* Record a path's pre-run bytes, on disk before it is recorded in memory.
948-
*
949-
* The order is the durability guarantee: every caller awaits this before the
950-
* write it describes, so a process killed at any point leaves a journal that
951-
* covers at least every file it had begun to change.
952-
*/
953-
async function journalTouch(state: RunState, rel: string, before: string | null): Promise<void> {
954-
const key = journalKey(rel)
955-
if (state.journal.has(key)) return
956-
const entry = await withLinkText(state.root, { path: rel, before })
957-
await appendJournalEntries(state.journalFile, [entry])
958-
state.journal.set(key, entry)
959-
}
960-
961946
/**
962947
* The entry, plus the text of the symlink its path is, when it is one. Read at
963948
* the first touch, like the bytes, so it describes the path before the run.
@@ -982,13 +967,18 @@ function sidecarKey(note: string, sidecar: SidecarKind): string {
982967
}
983968

984969
/**
985-
* `journalTouch` for everything one path op is about to change: both ends of
986-
* the note and of each sidecar it carries, first touch still winning, on disk
987-
* in ONE append and ONE sync. A sync per line made a bulk move several times
988-
* slower for no extra safety, since the op writes none of these files until
989-
* all of them are durable either way.
970+
* Record the pre-run state of everything one op is about to change, on disk
971+
* before it is recorded in memory: the note (both ends of it, for a path op)
972+
* and each sidecar the op touches, first touch winning throughout.
973+
*
974+
* The order is the durability guarantee: every caller awaits this before the
975+
* write it describes, so a process killed at any point leaves a journal that
976+
* covers at least every file it had begun to change. It is ONE append and ONE
977+
* sync for all of them: a sync per line made a bulk move several times slower
978+
* for no extra safety, since the op writes none of these files until all of
979+
* them are durable either way.
990980
*/
991-
async function journalPathOp(
981+
async function journalOp(
992982
state: RunState,
993983
notes: WorkflowJournalEntry[],
994984
sidecars: SidecarJournalEntry[]
@@ -1081,11 +1071,47 @@ async function applyTextOpToVault(state: RunState, op: TextOp): Promise<void> {
10811071
// for a change that did not happen. Only for a file that already exists;
10821072
// creating an empty note is a real change even though '' equals ''.
10831073
if (live !== null && next === live) return
1084-
await journalTouch(state, rel, live)
1074+
const metadata = await sidecarPathOf(state.root, rel, 'metadata')
1075+
// Creating a note where a date file waits with no note beside it: that date
1076+
// belongs to nobody (#839), and `createNote` discards it too. Journalled,
1077+
// so undo puts it back; the new note's date is its own birth time.
1078+
const leftover = live === null ? await readIfExists(metadata) : null
1079+
if (live !== null) await keepCreationDate(state, rel)
1080+
await journalOp(
1081+
state,
1082+
[{ path: rel, before: live }],
1083+
leftover === null ? [] : [{ note: rel, sidecar: 'metadata', before: leftover }]
1084+
)
1085+
if (leftover !== null) {
1086+
await fs.rm(metadata, { force: true })
1087+
state.sidecarsWritten.set(sidecarKey(rel, 'metadata'), null)
1088+
}
10851089
await writeNoteThroughLinks(abs, next)
10861090
recordWritten(state, rel, hashText(next))
10871091
}
10881092

1093+
/**
1094+
* Write a note's creation date down before its file is replaced.
1095+
*
1096+
* A note ZenNotes has never saved (one from before 2.51, or from git, sync or
1097+
* a file manager) has no date file: its date is the file's own birth time,
1098+
* and an atomic write, temp file plus rename, replaces that file with one
1099+
* born now. Saving from the editor writes the date down first (`writeNote`),
1100+
* and so must every write here, and every move: the rename keeps the file,
1101+
* but undo writes it back as a new one. A date file already there is left
1102+
* alone, valid or not, so a corrupt one cannot fail a run the way it fails a
1103+
* save. This is the one write that is not journalled first, safely: it
1104+
* records a date the note already had, where the app already looks for it, so
1105+
* nothing anyone can see changes, even if the run dies right after it. Not in
1106+
* a temporary folder session, which `writeNote` never writes app state into
1107+
* either.
1108+
*/
1109+
async function keepCreationDate(state: RunState, rel: string): Promise<void> {
1110+
if (isEphemeralRoot(state.root)) return
1111+
if ((await readIfExists(await sidecarPathOf(state.root, rel, 'metadata'))) !== null) return
1112+
await prepareNoteCreation(state.root, rel)
1113+
}
1114+
10891115
/** A sidecar a path op is about to carry. */
10901116
interface SidecarMove {
10911117
kind: SidecarKind
@@ -1117,29 +1143,20 @@ async function sidecarsToCarry(
11171143
toAbs: string
11181144
): Promise<SidecarMove[]> {
11191145
const moves: SidecarMove[] = []
1146+
// Refused before anything is written down, so a refusal leaves no trace.
1147+
const toComments = await sidecarPathOf(state.root, to, 'comments')
1148+
if ((await readIfExists(toComments)) !== null) {
1149+
throw new Error(leftoverCommentsMessage(state.root, toAbs))
1150+
}
1151+
// The rename keeps the file's birth time, but undo cannot: it writes the
1152+
// note back as a new file. Written down now, the date travels and comes back
1153+
// like any other sidecar.
1154+
await keepCreationDate(state, from)
11201155
for (const kind of SIDECAR_KINDS) {
11211156
const fromAbs = await sidecarPathOf(state.root, from, kind)
11221157
const toSidecar = await sidecarPathOf(state.root, to, kind)
11231158
const toBytes = await readIfExists(toSidecar)
1124-
if (kind === 'comments' && toBytes !== null) {
1125-
throw new Error(leftoverCommentsMessage(state.root, toAbs))
1126-
}
1127-
let fromBytes = await readIfExists(fromAbs)
1128-
// Not in a temporary folder session, which `writeNote` never writes app
1129-
// state into either.
1130-
if (kind === 'metadata' && fromBytes === null && !isEphemeralRoot(state.root)) {
1131-
// A note ZenNotes has never saved (one from before 2.51, or from git, sync
1132-
// or a file manager) has no date file: its date is the file's own birth
1133-
// time. The rename below keeps that, but undo cannot, because it writes
1134-
// the bytes back as a new file, born at the moment of the undo. So the
1135-
// date is written down first, as the editor's first save does, and then
1136-
// travels and comes back like any other. This is the one write that is
1137-
// not journalled first, safely: it records a date the note already had,
1138-
// where the app already looks for it, so nothing anyone can see changes,
1139-
// even if the run dies right after it.
1140-
await prepareNoteCreation(state.root, from)
1141-
fromBytes = await readIfExists(fromAbs)
1142-
}
1159+
const fromBytes = await readIfExists(fromAbs)
11431160
if (fromBytes === null && toBytes === null) continue
11441161
moves.push({ kind, fromAbs, toAbs: toSidecar, fromBytes, toBytes })
11451162
}
@@ -1171,7 +1188,7 @@ async function movePathInVault(
11711188
// Before the first journal line, so an op refused over its sidecars leaves
11721189
// nothing of itself to take back.
11731190
const sidecars = await sidecarsToCarry(state, from, to, toAbs)
1174-
await journalPathOp(
1191+
await journalOp(
11751192
state,
11761193
[
11771194
{ path: from, before: live },

0 commit comments

Comments
 (0)