fix(journey): refuse the second of two saves of a recorded day at once unless it asked to replace

This commit is contained in:
Maurice
2026-10-21 15:18:01 +02:00
parent 4f60cc5c98
commit 5116f7be2c
5 changed files with 89 additions and 17 deletions
@@ -112,6 +112,22 @@ export class JourneyRecordedDaysRepository extends TrekRepository<JourneyRecorde
return written.id;
}
/**
* Keeps a day only while the journey keeps none for that day and source, and
* answers its id; undefined when another version was there first. Two takeovers
* of the same day at once cannot both win this way.
*/
async insertDayIfAbsent(day: JourneyRecordedDayWrite): Promise<number | undefined> {
const now = dbNow();
const written = await this.db()
.insertInto('journey_recorded_days')
.values({ ...day, created_at: now, updated_at: now })
.onConflict((oc) => oc.columns(['journey_id', 'local_date', 'source']).doNothing())
.returning('id')
.executeTakeFirst();
return written?.id;
}
/** Drops the day a journey keeps for one source; false when it kept none. */
async deleteDay(journeyId: number, localDate: string, source: string): Promise<boolean> {
const result = await this.db()
@@ -100,20 +100,25 @@ export class JourneyRecordingsService {
const stored = await this.recordedDays.find(journeyId, date);
if (stored?.contentHash === contentHash) return { day: stored, outcome: 'unchanged' };
if (stored && !request.replace) {
throw conflict('The journey already keeps another recording of that day', { code: 'recording_exists' });
}
if (stored && !request.replace) throw recordingExists();
const kept = await this.recordedDays.keep(journeyId, userId, {
date,
sourceKind: day.sourceKind,
utcOffsetMinutes: day.utcOffsetMinutes,
window: day.window,
segments: day.segments,
truncated: day.truncated,
fetchedAt: day.fetchedAt,
contentHash,
});
const kept = await this.recordedDays.keep(
journeyId,
userId,
{
date,
sourceKind: day.sourceKind,
utcOffsetMinutes: day.utcOffsetMinutes,
window: day.window,
segments: day.segments,
truncated: day.truncated,
fetchedAt: day.fetchedAt,
contentHash,
},
request.replace === true,
);
// Another editor kept the day between the read above and this write.
if (!kept) throw recordingExists();
await this.journeys.broadcastJourneyEvent(journeyId, 'journey:recording:updated', { date }, socketId);
return { day: kept, outcome: stored ? 'replaced' : 'created' };
}
@@ -138,3 +143,7 @@ export class JourneyRecordingsService {
}
}
}
function recordingExists(): DomainError {
return conflict('The journey already keeps another recording of that day', { code: 'recording_exists' });
}
@@ -60,10 +60,19 @@ export class JourneyRecordedDaysService {
return row ? toRecordedDay(row) : null;
}
/** Keeps a day's recording, replacing what the journey kept for that day, and answers it as kept. */
async keep(journeyId: number, userId: number, recording: RecordingToKeep): Promise<JourneyRecordedDay> {
/**
* Keeps a day's recording and answers it as kept. With `replace` it takes the
* place of what the journey kept for that day; without, it is kept only while
* the journey keeps none, and null says another version got there first.
*/
async keep(
journeyId: number,
userId: number,
recording: RecordingToKeep,
replace: boolean,
): Promise<JourneyRecordedDay | null> {
const stats = recordingStats(recording.segments);
await this.days.upsertDay({
const row = {
journey_id: journeyId,
local_date: recording.date,
source: SOURCE,
@@ -78,7 +87,9 @@ export class JourneyRecordedDaysService {
truncated: recording.truncated ? 1 : 0,
fetched_at: recording.fetchedAt,
imported_by: userId,
});
};
if (replace) await this.days.upsertDay(row);
else if ((await this.days.insertDayIfAbsent(row)) === undefined) return null;
const kept = await this.find(journeyId, recording.date);
if (!kept) throw new Error(`Recorded day ${recording.date} of journey ${journeyId} vanished after its write`);
return kept;
@@ -104,6 +104,19 @@ describe('JourneyRecordedDaysRepository', () => {
});
});
it('JRDAYS-007: a write that keeps only while none is kept leaves the version there first and says so', async () => {
const { user, journey: j } = await journey();
const first = await days.insertDayIfAbsent(day(j.id, user.id));
expect(first).toEqual(expect.any(Number));
expect(await days.insertDayIfAbsent(day(j.id, user.id, { content_hash: 'b'.repeat(64) }))).toBeUndefined();
expect(await countRows(t, JourneyRecordedDays, {})).toBe(1);
expect(await days.findDay(j.id, '2026-05-14', 'dawarich')).toMatchObject({
id: first,
content_hash: 'a'.repeat(64),
});
});
it('JRDAYS-003: lists the days of one journey earliest first, and only its own', async () => {
const { user, journey: j } = await journey();
const other = await makeJourney(t, user.id);
@@ -259,6 +259,29 @@ describe('JourneyRecordingsService.save', () => {
});
});
it('JRNREC-SVC-013: of two editors saving the day at once without Replace, the second is refused, not silently kept', async () => {
const f = await fixture();
fetchDay.mockResolvedValue(recorded());
await svc.save(f.editor.id, f.journey.id, '2026-05-14', await previewed(f.editor.id, f.journey.id));
t.realtime.reset();
fetchDay.mockResolvedValue(recorded({ segments: [WALK, { ...WALK, startedAt: '2026-05-14T15:00:00+02:00' }] }));
const body = await previewed(f.owner.id, f.journey.id);
// The owner read the day before the editor's write landed.
const staleRead = vi.spyOn(kept, 'find').mockResolvedValueOnce(null);
expect(await refusal(svc.save(f.owner.id, f.journey.id, '2026-05-14', body))).toEqual({
status: 409,
body: { error: 'The journey already keeps another recording of that day', code: 'recording_exists' },
});
staleRead.mockRestore();
expect(await kept.find(f.journey.id, '2026-05-14')).toMatchObject({
importedBy: { id: f.editor.id, username: 'jonas' },
stats: { segmentCount: 1 },
});
expect(t.realtime.broadcastToUserMock).not.toHaveBeenCalled();
});
it('JRNREC-SVC-008: a recording that changed since the preview is not kept, and the kept version stays', async () => {
const f = await fixture();
fetchDay.mockResolvedValue(recorded());