mirror of
https://github.com/mauriceboe/TREK.git
synced 2026-10-19 13:36:48 +00:00
939452f9d8
* refactor(server): route every trip-access check through DatabaseService
Twelve Nest services carry the same verifyTripAccess/canEdit pair. Six already
asked DatabaseService.canAccessTrip; the other six went through
services/tripAccess, a nine-line re-export of the identical function. Same
query, same result, two ways to reach it.
All twelve now take the injected route, so services/tripAccess is gone.
The bigger duplication was in the types. Thirteen files declared their own
`Trip` for the same return value, in three different shapes:
NonNullable<ReturnType<typeof verifyTripAccess>> 6x
{ user_id: number } 4x
TripAccess 3x
The narrow one is why assignments, days, places and accommodations all wrote
`as Trip | null | undefined` on the call — the cast existed to shrink
TripAccess down to a local type that was never meant to differ from it. All
thirteen now say TripAccess, and the four casts are gone with no other change.
middleware/tripAccess.ts goes too: it was already dead in production — nothing
under src/ imported requireTripAccess or requireTripOwner, only comments in the
services that had absorbed them. Its test went with it.
* refactor(server): fold the query helpers and trip joins into the container
Two more legacy files whose only reason for staying outside the container was
that they predated it.
services/queryHelpers.ts held five exports doing two different jobs. Three are
the batch loaders that keep the list endpoints off N+1 queries — they hit the
database and are now QueryHelpersService over the injected connection, imported
by assignments, days, places and share. The other two, formatAssignmentWithPlace
and ratingAggregate, touch no database at all; they sat next to the loaders only
because the loaders were free functions too. Those stay free functions, in
common/rowShape.ts.
The README note on the assignments service claimed the loaders had to stay in
services/ because the day, share and place services were unmigrated. All three
have since moved, so the reason is gone.
services/tripMembership.ts becomes TripMembershipService in its own module. It
could have folded into auth or trip-invite, but both call it — as do oidc and
the plugin RPC host — and putting it in either would have made AuthModule and
TripInviteModule import each other.
Neither module is @Global, matching PermissionsModule: e2e TestingModules
resolve them transitively through each consumer's explicit import.
Test seams changed shape accordingly. Two path mocks of services/tripMembership
became injected stubs; the trip-invite e2e overrides the provider in the
container instead. The oidc test keeps the real implementation, which is what
OIDC-SVC-045 was always asserting.
ratingAggregate moved out of the ungated services/ tree into the coverage-gated
one, so it gets the direct tests it never had — as does the tags/participants
pass-through, which two long-dead fixture variables in the moved test file had
been waiting for.
Also drops four stale comments that outlived their subject: the requireTripAccess
middleware they pointed at was deleted in the previous commit.
138 lines
5.7 KiB
TypeScript
138 lines
5.7 KiB
TypeScript
/**
|
|
* Optimistic-concurrency / 409 conflict tests (#1135) for the place + packing
|
|
* update services. A matching If-Match token (or none) updates as before; a
|
|
* stale token returns the conflict sentinel carrying the server's current row.
|
|
*/
|
|
import { describe, it, expect, vi, beforeAll, beforeEach, afterAll } from 'vitest';
|
|
|
|
const { testDb, dbMock } = vi.hoisted(() => {
|
|
const Database = require('better-sqlite3');
|
|
const db = new Database(':memory:');
|
|
db.exec('PRAGMA journal_mode = WAL');
|
|
db.exec('PRAGMA foreign_keys = ON');
|
|
db.exec('PRAGMA busy_timeout = 5000');
|
|
function getPlaceWithTags(placeId: number | string) {
|
|
const p = db.prepare('SELECT * FROM places WHERE id = ?').get(placeId);
|
|
if (!p) return null;
|
|
return { ...(p as object), category: null, tags: [] };
|
|
}
|
|
const mock = {
|
|
db,
|
|
closeDb: () => {},
|
|
reinitialize: () => {},
|
|
getPlaceWithTags,
|
|
canAccessTrip: () => null,
|
|
isOwner: () => false,
|
|
};
|
|
return { testDb: db, dbMock: mock };
|
|
});
|
|
|
|
vi.mock('../../../src/db/database', () => dbMock);
|
|
vi.mock('../../../src/config', () => ({
|
|
JWT_SECRET: 'test-secret',
|
|
ENCRYPTION_KEY: 'a1b2c3d4e5f6a7b8c9d0e1f2a3b4c5d6a7b8c9d0e1f2a3b4c5d6a7b8c9d0e1f2',
|
|
updateJwtSecret: () => {},
|
|
}));
|
|
|
|
import { createTables } from '../../../src/db/schema';
|
|
import { runMigrations } from '../../../src/db/migrations';
|
|
import { resetTestDb } from '../../helpers/test-db';
|
|
import { createUser, createTrip } from '../../helpers/factories';
|
|
import { isUpdateConflict } from '../../../src/nest/common/conflictResult';
|
|
import { DatabaseService } from '../../../src/nest/database/database.service';
|
|
import { PackingService } from '../../../src/nest/packing/packing.service';
|
|
import { PlacesService } from '../../../src/nest/places/places.service';
|
|
import { MapsService } from '../../../src/nest/maps/maps.service';
|
|
import { PermissionsService } from '../../../src/nest/permissions/permissions.service';
|
|
import { RealtimeService } from '../../../src/nest/realtime/realtime.service';
|
|
import { QueryHelpersService } from '../../../src/nest/query-helpers/query-helpers.service';
|
|
|
|
const packing = new PackingService(new DatabaseService(testDb), new PermissionsService(new DatabaseService(testDb)), new RealtimeService());
|
|
const places = new PlacesService(new DatabaseService(testDb), new PermissionsService(new DatabaseService(testDb)), new RealtimeService(), new MapsService(new DatabaseService(testDb)), new QueryHelpersService(new DatabaseService(testDb)));
|
|
|
|
beforeAll(() => {
|
|
createTables(testDb);
|
|
runMigrations(testDb);
|
|
});
|
|
|
|
beforeEach(() => {
|
|
resetTestDb(testDb);
|
|
});
|
|
|
|
afterAll(() => {
|
|
testDb.close();
|
|
});
|
|
|
|
function freshPlace(tripId: number) {
|
|
const place = places.create(String(tripId), { name: 'Original' }) as unknown as { id: number; updated_at: string };
|
|
return place;
|
|
}
|
|
|
|
describe('PlacesService.update — optimistic concurrency', () => {
|
|
it('updates normally when no If-Match token is sent (back-compat)', () => {
|
|
const { user } = createUser(testDb);
|
|
const trip = createTrip(testDb, user.id);
|
|
const place = freshPlace(trip.id);
|
|
|
|
const result = places.update(String(trip.id), String(place.id), { name: 'Edited' });
|
|
expect(isUpdateConflict(result)).toBe(false);
|
|
expect((result as { name: string }).name).toBe('Edited');
|
|
});
|
|
|
|
it('updates when the If-Match token matches the current updated_at', () => {
|
|
const { user } = createUser(testDb);
|
|
const trip = createTrip(testDb, user.id);
|
|
const place = freshPlace(trip.id);
|
|
|
|
const result = places.update(String(trip.id), String(place.id), { name: 'Edited' }, place.updated_at);
|
|
expect(isUpdateConflict(result)).toBe(false);
|
|
expect((result as { name: string }).name).toBe('Edited');
|
|
});
|
|
|
|
it('returns a conflict (with the server row) when the token is stale', () => {
|
|
const { user } = createUser(testDb);
|
|
const trip = createTrip(testDb, user.id);
|
|
const place = freshPlace(trip.id);
|
|
|
|
const result = places.update(String(trip.id), String(place.id), { name: 'Mine' }, '1999-01-01 00:00:00');
|
|
expect(isUpdateConflict(result)).toBe(true);
|
|
if (isUpdateConflict(result)) {
|
|
expect((result.server as { name: string }).name).toBe('Original');
|
|
}
|
|
// The row must NOT have been overwritten.
|
|
const row = testDb.prepare('SELECT name FROM places WHERE id = ?').get(place.id) as { name: string };
|
|
expect(row.name).toBe('Original');
|
|
});
|
|
|
|
it('returns null for a place that does not exist', () => {
|
|
const { user } = createUser(testDb);
|
|
const trip = createTrip(testDb, user.id);
|
|
expect(places.update(String(trip.id), '999999', { name: 'x' }, 'whatever')).toBeNull();
|
|
});
|
|
});
|
|
|
|
describe('updateItem (packing) — optimistic concurrency', () => {
|
|
it('migration added updated_at and createItem stamps it', () => {
|
|
const cols = testDb.prepare("PRAGMA table_info('packing_items')").all() as { name: string }[];
|
|
expect(cols.map(c => c.name)).toContain('updated_at');
|
|
|
|
const { user } = createUser(testDb);
|
|
const trip = createTrip(testDb, user.id);
|
|
const item = packing.createItem(trip.id, { name: 'Socks' }) as { id: number; updated_at: string | null };
|
|
expect(item.updated_at).toBeTruthy();
|
|
});
|
|
|
|
it('returns a conflict when the packing token is stale', () => {
|
|
const { user } = createUser(testDb);
|
|
const trip = createTrip(testDb, user.id);
|
|
const item = packing.createItem(trip.id, { name: 'Socks' }) as { id: number; updated_at: string };
|
|
|
|
const stale = packing.updateItem(trip.id, item.id, { name: 'Mine' }, ['name'], '1999-01-01 00:00:00');
|
|
expect(isUpdateConflict(stale)).toBe(true);
|
|
|
|
const fresh = packing.updateItem(trip.id, item.id, { name: 'Edited' }, ['name'], item.updated_at);
|
|
expect(isUpdateConflict(fresh)).toBe(false);
|
|
expect((fresh as { name: string }).name).toBe('Edited');
|
|
});
|
|
})
|