refactor(sync): remove dead getOpsSince method, migrate tests to getOpsSinceWithSeq (#8601)

* refactor(sync): remove dead getOpsSince method, migrate tests to getOpsSinceWithSeq

getOpsSince (non-WithSeq) was dead code in production — routes only called
getOpsSinceWithSeq. Removed the method and its direct unit tests; migrated
all remaining test callers to getOpsSinceWithSeq with .ops accessor.

* fix(sync): add missing lastSeq to userSyncState in cleanup test

getOpsSinceWithSeq reads userSyncState.lastSeq to determine the latest
sequence. The test overwrote userSyncState without lastSeq, causing the
new method to return empty ops.
This commit is contained in:
Mohamed Abdeltawab 2026-06-26 13:14:58 +03:00 committed by GitHub
parent 2f2bf55c1a
commit 13b23c09dc
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
6 changed files with 36 additions and 149 deletions

View file

@ -106,32 +106,6 @@ const parsePersistedVectorClock = (value: unknown): VectorClock | undefined => {
};
export class OperationDownloadService {
/**
* Get operations since a given sequence number.
* Simple version without gap detection.
*/
async getOpsSince(
userId: number,
sinceSeq: number,
excludeClient?: string,
limit: number = 500,
): Promise<ServerOperation[]> {
const ops = await prisma.operation.findMany({
where: {
userId,
serverSeq: { gt: sinceSeq },
...(excludeClient ? { clientId: { not: excludeClient } } : {}),
},
orderBy: {
serverSeq: 'asc',
},
take: limit,
select: OPERATION_DOWNLOAD_SELECT,
});
return ops.map(mapOperationRow);
}
/**
* Get operations and latest sequence atomically with gap detection.
*

View file

@ -863,7 +863,7 @@ describe('Conflict Detection', () => {
// Verify the clock was pruned to MAX_VECTOR_CLOCK_SIZE (20) before storage.
// clientA ('client-a') is not in the clock, so only the MAX most active
// clients are kept (the ones with highest counters).
const ops = await operationDownloadService.getOpsSince(userId, 0);
const ops = (await operationDownloadService.getOpsSinceWithSeq(userId, 0)).ops;
const storedClock = ops[0].op.vectorClock;
expect(Object.keys(storedClock).length).toBe(MAX_VECTOR_CLOCK_SIZE);
// The most active clients should be preserved (top MAX entries by counter)
@ -893,7 +893,7 @@ describe('Conflict Detection', () => {
expect(result[0].accepted).toBe(true);
// Verify the clock was pruned to MAX_VECTOR_CLOCK_SIZE
const ops = await operationDownloadService.getOpsSince(userId, 0);
const ops = (await operationDownloadService.getOpsSinceWithSeq(userId, 0)).ops;
const storedClock = ops[0].op.vectorClock;
expect(Object.keys(storedClock).length).toBe(MAX_VECTOR_CLOCK_SIZE);
// clientA should be preserved despite having the lowest counter
@ -947,7 +947,7 @@ describe('Conflict Detection', () => {
expect(resolvedResult[0].accepted).toBe(true);
// Verify the stored clock was pruned to MAX after acceptance
const ops = await operationDownloadService.getOpsSince(userId, 0);
const ops = (await operationDownloadService.getOpsSinceWithSeq(userId, 0)).ops;
const latestOp = ops.find((o: any) => o.op.id === resolvedOp.id);
expect(latestOp).toBeDefined();
const storedClock = latestOp!.op.vectorClock;

View file

@ -1,6 +1,5 @@
import { describe, it, expect, beforeEach, vi, afterEach } from 'vitest';
import { OperationDownloadService } from '../src/sync/services/operation-download.service';
import { Operation, ServerOperation } from '../src/sync/sync.types';
// Mock prisma
vi.mock('../src/db', () => ({
@ -117,107 +116,6 @@ describe('OperationDownloadService', () => {
vi.useRealTimers();
});
describe('getOpsSince', () => {
it('should return empty array when no operations exist', async () => {
vi.mocked(prisma.operation.findMany).mockResolvedValue([]);
const result = await service.getOpsSince(1, 0);
expect(result).toEqual([]);
expect(prisma.operation.findMany).toHaveBeenCalledWith({
where: {
userId: 1,
serverSeq: { gt: 0 },
},
orderBy: { serverSeq: 'asc' },
take: 500,
select: EXPECTED_OPERATION_DOWNLOAD_SELECT,
});
});
it('should return operations mapped to ServerOperation format', async () => {
const mockOps = [createMockOpRow(1), createMockOpRow(2)];
vi.mocked(prisma.operation.findMany).mockResolvedValue(mockOps as any);
const result = await service.getOpsSince(1, 0);
expect(result).toHaveLength(2);
expect(result[0].serverSeq).toBe(1);
expect(result[0].op.id).toBe('op-1');
expect(result[0].op.opType).toBe('ADD');
expect(result[1].serverSeq).toBe(2);
});
it('should exclude operations from specified client', async () => {
vi.mocked(prisma.operation.findMany).mockResolvedValue([]);
await service.getOpsSince(1, 0, 'excluded-client');
expect(prisma.operation.findMany).toHaveBeenCalledWith({
where: {
userId: 1,
serverSeq: { gt: 0 },
clientId: { not: 'excluded-client' },
},
orderBy: { serverSeq: 'asc' },
take: 500,
select: EXPECTED_OPERATION_DOWNLOAD_SELECT,
});
});
it('should respect custom limit', async () => {
vi.mocked(prisma.operation.findMany).mockResolvedValue([]);
await service.getOpsSince(1, 0, undefined, 100);
expect(prisma.operation.findMany).toHaveBeenCalledWith(
expect.objectContaining({ take: 100 }),
);
});
it('should correctly map operation fields including optional entityId', async () => {
const opWithNullEntityId = createMockOpRow(1, 'client-1', { entityId: null });
vi.mocked(prisma.operation.findMany).mockResolvedValue([opWithNullEntityId as any]);
const result = await service.getOpsSince(1, 0);
expect(result[0].op.entityId).toBeUndefined();
});
it('should convert timestamps from bigint to number', async () => {
const mockOp = createMockOpRow(1, 'client-1', {
clientTimestamp: BigInt(1700000000000),
receivedAt: BigInt(1700000001000),
});
vi.mocked(prisma.operation.findMany).mockResolvedValue([mockOp as any]);
const result = await service.getOpsSince(1, 0);
expect(result[0].op.timestamp).toBe(1700000000000);
expect(result[0].receivedAt).toBe(1700000001000);
});
it('should handle operations with encrypted payloads', async () => {
const encryptedOp = createMockOpRow(1, 'client-1', { isPayloadEncrypted: true });
vi.mocked(prisma.operation.findMany).mockResolvedValue([encryptedOp as any]);
const result = await service.getOpsSince(1, 0);
expect(result[0].op.isPayloadEncrypted).toBe(true);
});
it('should map sync import reason when present', async () => {
const importOp = createMockOpRow(1, 'client-1', {
syncImportReason: 'recovery',
});
vi.mocked(prisma.operation.findMany).mockResolvedValue([importOp as any]);
const result = await service.getOpsSince(1, 0);
expect(result[0].op.syncImportReason).toBe('recovery');
});
});
describe('getOpsSinceWithSeq', () => {
// Helper to set up transaction mock
const setupTransactionMock = (mockFn: (tx: any) => Promise<any>) => {

View file

@ -947,6 +947,7 @@ describe('Sync Operations', () => {
const now = Date.now();
testState.userSyncStates.set(userId, {
userId,
lastSeq: 2,
lastSnapshotSeq: 2,
snapshotAt: BigInt(now),
opCount: 2,
@ -969,7 +970,7 @@ describe('Sync Operations', () => {
expect(result.affectedUserIds).toContain(userId);
// Verify correct operation was deleted
const ops = await operationDownloadService.getOpsSince(userId, 0);
const ops = (await operationDownloadService.getOpsSinceWithSeq(userId, 0)).ops;
expect(ops.length).toBe(1);
expect(ops[0].serverSeq).toBe(2);
});

View file

@ -1617,7 +1617,7 @@ describe('SyncService', () => {
expect(results[0].accepted).toBe(true);
// Verify round-trip
const ops = await operationDownloadService.getOpsSince(userId, 0);
const ops = (await operationDownloadService.getOpsSinceWithSeq(userId, 0)).ops;
expect(ops[0].op.entityId).toBe('task-日本語-émoji-🎉');
});
@ -1663,7 +1663,7 @@ describe('SyncService', () => {
await service.uploadOps(userId, clientId, [op]);
}
const ops = await operationDownloadService.getOpsSince(userId, 2);
const ops = (await operationDownloadService.getOpsSinceWithSeq(userId, 2)).ops;
expect(ops).toHaveLength(3);
expect(ops[0].serverSeq).toBe(3);
@ -1708,7 +1708,8 @@ describe('SyncService', () => {
},
]);
const ops = await operationDownloadService.getOpsSince(userId, 0, client1);
const ops = (await operationDownloadService.getOpsSinceWithSeq(userId, 0, client1))
.ops;
expect(ops).toHaveLength(1);
expect(ops[0].op.entityId).toBe('task-2');
@ -1735,13 +1736,15 @@ describe('SyncService', () => {
]);
}
const ops = await operationDownloadService.getOpsSince(userId, 0, undefined, 3);
const ops = (
await operationDownloadService.getOpsSinceWithSeq(userId, 0, undefined, 3)
).ops;
expect(ops).toHaveLength(3);
});
it('should return empty array when no operations exist', async () => {
const ops = await operationDownloadService.getOpsSince(userId, 0);
const ops = (await operationDownloadService.getOpsSinceWithSeq(userId, 0)).ops;
expect(ops).toHaveLength(0);
});
@ -2031,7 +2034,8 @@ describe('SyncService', () => {
expect(totalDeleted).toBe(2);
expect(affectedUserIds).toContain(userId);
const remaining = await operationDownloadService.getOpsSince(userId, 0);
const remaining = (await operationDownloadService.getOpsSinceWithSeq(userId, 0))
.ops;
expect(remaining).toHaveLength(3);
});
@ -2285,8 +2289,12 @@ describe('SyncService', () => {
expect(affectedUserIds).toContain(userId);
expect(affectedUserIds).toContain(user2Id);
expect((await operationDownloadService.getOpsSince(userId, 0)).length).toBe(0);
expect((await operationDownloadService.getOpsSince(user2Id, 0)).length).toBe(0);
expect(
(await operationDownloadService.getOpsSinceWithSeq(userId, 0)).ops.length,
).toBe(0);
expect(
(await operationDownloadService.getOpsSinceWithSeq(user2Id, 0)).ops.length,
).toBe(0);
});
it('should delete stale devices', async () => {
@ -2351,7 +2359,9 @@ describe('SyncService', () => {
expect(totalDeleted).toBe(0);
expect(affectedUserIds).toHaveLength(0);
expect((await operationDownloadService.getOpsSince(userId, 0)).length).toBe(3);
expect(
(await operationDownloadService.getOpsSinceWithSeq(userId, 0)).ops.length,
).toBe(3);
});
it('should not delete recent devices', async () => {
@ -2980,14 +2990,15 @@ describe('SyncService', () => {
]);
// Verify operations exist
const opsBefore = await operationDownloadService.getOpsSince(userId, 0);
const opsBefore = (await operationDownloadService.getOpsSinceWithSeq(userId, 0))
.ops;
expect(opsBefore.length).toBe(2);
// Delete all user data
await service.deleteAllUserData(userId);
// Verify operations are gone
const opsAfter = await operationDownloadService.getOpsSince(userId, 0);
const opsAfter = (await operationDownloadService.getOpsSinceWithSeq(userId, 0)).ops;
expect(opsAfter.length).toBe(0);
});
@ -3032,7 +3043,7 @@ describe('SyncService', () => {
expect(results[0].accepted).toBe(true);
// Verify only new operation exists
const ops = await operationDownloadService.getOpsSince(userId, 0);
const ops = (await operationDownloadService.getOpsSinceWithSeq(userId, 0)).ops;
expect(ops.length).toBe(1);
expect(ops[0].op.entityId).toBe('t2');
});
@ -3084,11 +3095,12 @@ describe('SyncService', () => {
await service.deleteAllUserData(userId);
// Verify user 1's data is gone
const user1Ops = await operationDownloadService.getOpsSince(userId, 0);
const user1Ops = (await operationDownloadService.getOpsSinceWithSeq(userId, 0)).ops;
expect(user1Ops.length).toBe(0);
// Verify user 2's data still exists
const user2Ops = await operationDownloadService.getOpsSince(otherUserId, 0);
const user2Ops = (await operationDownloadService.getOpsSinceWithSeq(otherUserId, 0))
.ops;
expect(user2Ops.length).toBe(1);
expect(user2Ops[0].op.entityId).toBe('t2');
});

View file

@ -816,8 +816,9 @@ describe('TIME_TRACKING Operations', () => {
await service.uploadOps(userId, clientId, [op]);
// Another client downloads - getOpsSince returns { op: Operation, serverSeq }[]
const downloadedOps = await operationDownloadService.getOpsSince(userId, 0);
// Another client downloads - getOpsSinceWithSeq returns { op: Operation, serverSeq }[]
const downloadedOps = (await operationDownloadService.getOpsSinceWithSeq(userId, 0))
.ops;
expect(downloadedOps).toHaveLength(1);
expect(downloadedOps[0].op.entityType).toBe('TIME_TRACKING');
@ -836,7 +837,8 @@ describe('TIME_TRACKING Operations', () => {
await service.uploadOps(userId, clientId, [op]);
const downloadedOps = await operationDownloadService.getOpsSince(userId, 0);
const downloadedOps = (await operationDownloadService.getOpsSinceWithSeq(userId, 0))
.ops;
expect(downloadedOps[0].op.payload).toEqual({
contextType: 'PROJECT',