mirror of
https://github.com/4gray/iptvnator.git
synced 2026-10-16 14:16:16 -08:00
fix(server-storage): the move uploads playlists create-only and merges into a copy moved first
Two tabs or devices moving browser copies with the same playlist id both planned an upload; the browser documents carry no served version, so both PUTs were unconditional and the later one overwrote the first copy (and any change made on it since), with both moves reporting success. The backend's playlist PUT now honours `If-None-Match: *` (412 when the playlist exists; checked and written in one synchronous handler). The move uploads with it through ServerPlaylistDocumentStore .dbCreateAppPlaylist, and an upload that finds its id taken merges its favorites and history into that copy instead, counted as merged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
1 parent
f6a216f59a
commit
954a335f31
7 files changed
+129
-12
No files matched your search
@@ -22,6 +22,7 @@ describe('ServerMigrationService', () => {
|
||||
dbGetAppPlaylist: jest.Mock;
|
||||
dbGetAppPlaylistMetas: jest.Mock;
|
||||
dbUpsertAppPlaylist: jest.Mock;
|
||||
dbCreateAppPlaylist: jest.Mock;
|
||||
};
|
||||
api: { getJson: jest.Mock; sendJson: jest.Mock };
|
||||
userData: {
|
||||
@@ -52,6 +53,7 @@ describe('ServerMigrationService', () => {
|
||||
dbUpsertAppPlaylist: jest
|
||||
.fn()
|
||||
.mockResolvedValue({ success: true }),
|
||||
dbCreateAppPlaylist: jest.fn().mockResolvedValue(true),
|
||||
},
|
||||
api: {
|
||||
getJson: jest.fn().mockResolvedValue({ entries: {} }),
|
||||
@@ -117,7 +119,7 @@ describe('ServerMigrationService', () => {
|
||||
const result = await migration.moveBrowserData();
|
||||
|
||||
expect(result).toEqual({ imported: 2, merged: 0, failed: 0 });
|
||||
expect(session.documentStore.dbUpsertAppPlaylist).toHaveBeenCalledTimes(
|
||||
expect(session.documentStore.dbCreateAppPlaylist).toHaveBeenCalledTimes(
|
||||
2
|
||||
);
|
||||
expect(session.api.sendJson).toHaveBeenCalledWith(
|
||||
@@ -353,10 +355,38 @@ describe('ServerMigrationService', () => {
|
||||
});
|
||||
});
|
||||
|
||||
it('merges into a copy another tab or device uploaded first', async () => {
|
||||
TestBed.overrideProvider(NgxIndexedDBService, {
|
||||
useValue: {
|
||||
getAll: () =>
|
||||
of([{ ...browserPlaylists[0], favorites: ['http://s/2'] }]),
|
||||
},
|
||||
});
|
||||
// The id was free when planned, taken when uploaded.
|
||||
session.documentStore.dbCreateAppPlaylist.mockResolvedValue(false);
|
||||
const first = {
|
||||
...browserPlaylists[0],
|
||||
favorites: ['http://s/1'],
|
||||
recentlyViewed: [],
|
||||
};
|
||||
session.documentStore.dbGetAppPlaylist.mockResolvedValue(first);
|
||||
|
||||
const result = await TestBed.inject(
|
||||
ServerMigrationService
|
||||
).moveBrowserData();
|
||||
|
||||
expect(result).toEqual({ imported: 0, merged: 1, failed: 0 });
|
||||
// Never overwritten: this browser's favorites join the first copy's.
|
||||
expect(session.documentStore.dbUpsertAppPlaylist).toHaveBeenCalledWith({
|
||||
...first,
|
||||
favorites: ['http://s/1', 'http://s/2'],
|
||||
});
|
||||
});
|
||||
|
||||
it('keeps the move on offer when a playlist could not be uploaded', async () => {
|
||||
session.documentStore.dbUpsertAppPlaylist
|
||||
session.documentStore.dbCreateAppPlaylist
|
||||
.mockRejectedValueOnce(new Error('413'))
|
||||
.mockResolvedValue({ success: true });
|
||||
.mockResolvedValue(true);
|
||||
const migration = TestBed.inject(ServerMigrationService);
|
||||
|
||||
const result = await migration.moveBrowserData();
|
||||
@@ -382,7 +412,7 @@ describe('ServerMigrationService', () => {
|
||||
|
||||
await expect(migration.moveBrowserData()).rejects.toThrow('Unsent');
|
||||
expect(
|
||||
session.documentStore.dbUpsertAppPlaylist
|
||||
session.documentStore.dbCreateAppPlaylist
|
||||
).not.toHaveBeenCalled();
|
||||
expect(session.api.sendJson).not.toHaveBeenCalled();
|
||||
expect(migration.browserDataMoved()).toBe(false);
|
||||
@@ -398,7 +428,7 @@ describe('ServerMigrationService', () => {
|
||||
]),
|
||||
},
|
||||
});
|
||||
session.documentStore.dbUpsertAppPlaylist.mockRejectedValue(
|
||||
session.documentStore.dbCreateAppPlaylist.mockRejectedValue(
|
||||
new Error('500')
|
||||
);
|
||||
localStorage.setItem(
|
||||
@@ -501,7 +531,7 @@ describe('ServerMigrationService', () => {
|
||||
|
||||
await TestBed.inject(ServerMigrationService).moveBrowserData();
|
||||
|
||||
expect(session.documentStore.dbUpsertAppPlaylist).toHaveBeenCalledWith(
|
||||
expect(session.documentStore.dbCreateAppPlaylist).toHaveBeenCalledWith(
|
||||
expect.objectContaining({ _id: 's1', isFullStalkerPortal: true })
|
||||
);
|
||||
});
|
||||
|
||||
@@ -130,10 +130,21 @@ export class ServerMigrationService {
|
||||
merged += 1;
|
||||
targets.set(step.browserId, step.serverId);
|
||||
} else {
|
||||
await this.session.documentStore.dbUpsertAppPlaylist(
|
||||
step.playlist
|
||||
);
|
||||
imported += 1;
|
||||
// Create-only: another tab or device that moved the
|
||||
// same playlist first keeps its copy, merged into.
|
||||
const created =
|
||||
await this.session.documentStore.dbCreateAppPlaylist(
|
||||
step.playlist
|
||||
);
|
||||
if (created) {
|
||||
imported += 1;
|
||||
} else {
|
||||
await this.mergeDocumentUserData(
|
||||
step.playlist,
|
||||
step.playlist._id
|
||||
);
|
||||
merged += 1;
|
||||
}
|
||||
onServer.add(step.playlist._id);
|
||||
targets.set(step.playlist._id, step.playlist._id);
|
||||
}
|
||||
|
||||
@@ -170,6 +170,19 @@ describe('ServerPlaylistDocumentStore', () => {
|
||||
expect(api.sendJson.mock.calls[1][3]).toEqual({});
|
||||
});
|
||||
|
||||
it('creates a playlist only while the server has none of that id', async () => {
|
||||
const { api, store } = setup();
|
||||
api.sendJson.mockResolvedValueOnce(putResponse('"b.0.1"', ''));
|
||||
|
||||
await expect(store.dbCreateAppPlaylist(doc)).resolves.toBe(true);
|
||||
expect(api.sendJson.mock.calls[0][3]).toEqual({ 'If-None-Match': '*' });
|
||||
|
||||
api.sendJson.mockRejectedValueOnce(
|
||||
new ServerApiError(412, 'conflict', 'exists')
|
||||
);
|
||||
await expect(store.dbCreateAppPlaylist(doc)).resolves.toBe(false);
|
||||
});
|
||||
|
||||
it('writes a portal document on the version it read', async () => {
|
||||
const { api, store } = setup();
|
||||
const portal = { _id: 'p2', title: 'Portal', macAddress: 'm' };
|
||||
|
||||
@@ -191,6 +191,29 @@ export class ServerPlaylistDocumentStore implements PlaylistDocumentStore {
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* A create-only upload (the move from browser storage): false when the
|
||||
* server has a playlist with that id already, uploaded meanwhile by
|
||||
* another tab or device, which the caller merges into instead.
|
||||
*/
|
||||
async dbCreateAppPlaylist(playlist: Playlist): Promise<boolean> {
|
||||
try {
|
||||
const response = await this.api.sendJson(
|
||||
'PUT',
|
||||
`/playlists/${encodeURIComponent(playlist._id)}`,
|
||||
playlist,
|
||||
{ 'If-None-Match': '*' }
|
||||
);
|
||||
this.remember(playlist._id, response, playlist);
|
||||
return true;
|
||||
} catch (error) {
|
||||
if (error instanceof ServerApiError && error.status === 412) {
|
||||
return false;
|
||||
}
|
||||
throw error;
|
||||
}
|
||||
}
|
||||
|
||||
/** `null` when the server has no payload to keep (write it in full). */
|
||||
private async keepPayload(
|
||||
playlist: Playlist,
|
||||
|
||||
Reference in new issue
Block a user