diff --git a/server/src/db/repositories/JourneyRecordedDays.repository.ts b/server/src/db/repositories/JourneyRecordedDays.repository.ts index 54bd745ae..94e1a0cef 100644 --- a/server/src/db/repositories/JourneyRecordedDays.repository.ts +++ b/server/src/db/repositories/JourneyRecordedDays.repository.ts @@ -112,6 +112,22 @@ export class JourneyRecordedDaysRepository extends TrekRepository { + 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 { const result = await this.db() diff --git a/server/src/nest/journey-recordings/journey-recordings.service.ts b/server/src/nest/journey-recordings/journey-recordings.service.ts index cca1b18b5..2641f9815 100644 --- a/server/src/nest/journey-recordings/journey-recordings.service.ts +++ b/server/src/nest/journey-recordings/journey-recordings.service.ts @@ -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' }); +} diff --git a/server/src/nest/journey/journey-recorded-days.service.ts b/server/src/nest/journey/journey-recorded-days.service.ts index 52d33faba..4795406aa 100644 --- a/server/src/nest/journey/journey-recorded-days.service.ts +++ b/server/src/nest/journey/journey-recorded-days.service.ts @@ -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 { + /** + * 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 { 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; diff --git a/server/tests/unit/db/repositories/JourneyRecordedDays.repository.test.ts b/server/tests/unit/db/repositories/JourneyRecordedDays.repository.test.ts index c6c5be5a1..e73809d16 100644 --- a/server/tests/unit/db/repositories/JourneyRecordedDays.repository.test.ts +++ b/server/tests/unit/db/repositories/JourneyRecordedDays.repository.test.ts @@ -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); diff --git a/server/tests/unit/nest/journey-recordings.service.test.ts b/server/tests/unit/nest/journey-recordings.service.test.ts index 6bd59083b..cfafdf91e 100644 --- a/server/tests/unit/nest/journey-recordings.service.test.ts +++ b/server/tests/unit/nest/journey-recordings.service.test.ts @@ -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());