mirror of
https://github.com/jorenn92/Maintainerr.git
synced 2026-06-01 18:48:13 +02:00
fix(collections): create collections empty, then batch-add (avoid HTTP 414) (#3001)
Creating a new collection from a large rule seeded every item id into the create request's query string (BULK_COLLECTION_CREATE, #2911), overflowing the URL and failing with "Failed to create collection" / HTTP 414 URI Too Long. Item ids can only travel in the query string on Plex/Jellyfin/Emby (no request body), so the seed is unbounded. Revert to the pre-#2911 / 2.x behaviour: create the collection empty and populate it via the existing batched add path (addBatchToCollection), for all media servers. Removes the now-unused BULK_COLLECTION_CREATE capability and the initialItemIds plumbing.
This commit is contained in:
@@ -1,6 +1,4 @@
|
||||
import { MediaServerFeature } from '@maintainerr/contracts';
|
||||
import { AxiosError } from 'axios';
|
||||
import cacheManager from '../../lib/cache';
|
||||
import { EmbyAdapterService } from './emby-adapter.service';
|
||||
|
||||
jest.mock('../../lib/cache', () => ({
|
||||
@@ -80,37 +78,30 @@ describe('EmbyAdapterService', () => {
|
||||
setHttp();
|
||||
});
|
||||
|
||||
it('reports bulk-collection-create capability for Emby', () => {
|
||||
expect(
|
||||
service.supportsFeature(MediaServerFeature.BULK_COLLECTION_CREATE),
|
||||
).toBe(true);
|
||||
expect(cacheManager.getCache).toHaveBeenCalledWith('emby');
|
||||
});
|
||||
|
||||
describe('createCollection', () => {
|
||||
it('passes initial item ids on create and hydrates the created collection', async () => {
|
||||
it('creates the collection empty without seeding item ids', async () => {
|
||||
// Item ids must never be sent on create (they go in the query string →
|
||||
// HTTP 414 at scale); items are added via addBatchToCollection.
|
||||
http.post.mockResolvedValueOnce({ data: { Id: 'collection-1' } });
|
||||
http.get.mockResolvedValueOnce({
|
||||
data: {
|
||||
Id: 'collection-1',
|
||||
Name: 'Seeded Collection',
|
||||
Name: 'New Collection',
|
||||
Overview: 'summary',
|
||||
ChildCount: 2,
|
||||
ChildCount: 0,
|
||||
},
|
||||
});
|
||||
|
||||
const result = await service.createCollection({
|
||||
libraryId: 'library-1',
|
||||
title: 'Seeded Collection',
|
||||
title: 'New Collection',
|
||||
type: 'show',
|
||||
initialItemIds: ['item-1', 'item-2'],
|
||||
});
|
||||
|
||||
expect(http.post).toHaveBeenCalledWith('/Collections', null, {
|
||||
params: {
|
||||
Name: 'Seeded Collection',
|
||||
Name: 'New Collection',
|
||||
ParentId: 'library-1',
|
||||
Ids: 'item-1,item-2',
|
||||
IsLocked: true,
|
||||
},
|
||||
});
|
||||
@@ -118,7 +109,7 @@ describe('EmbyAdapterService', () => {
|
||||
expect(result).toEqual(
|
||||
expect.objectContaining({
|
||||
id: 'collection-1',
|
||||
title: 'Seeded Collection',
|
||||
title: 'New Collection',
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
@@ -827,6 +827,7 @@ export class EmbyAdapterService implements IMediaServerService {
|
||||
): Promise<MediaCollection> {
|
||||
if (!this.http) throw new Error('Emby not initialized');
|
||||
try {
|
||||
// Created empty; items are added afterwards via addBatchToCollection.
|
||||
const { data } = await this.http.post<EmbyBaseItemDto>(
|
||||
'/Collections',
|
||||
null,
|
||||
@@ -834,7 +835,6 @@ export class EmbyAdapterService implements IMediaServerService {
|
||||
params: {
|
||||
Name: params.title,
|
||||
ParentId: params.libraryId,
|
||||
Ids: params.initialItemIds?.join(','),
|
||||
// IsLocked enables composite image generation from items, matching
|
||||
// the Jellyfin adapter; without it, Emby may skip the auto-cover.
|
||||
IsLocked: true,
|
||||
|
||||
@@ -1456,37 +1456,15 @@ describe('JellyfinAdapterService', () => {
|
||||
isLocked: true,
|
||||
}),
|
||||
);
|
||||
// When no initialItemIds are supplied, `ids` is forwarded as undefined
|
||||
// rather than omitted — the SDK signature accepts undefined and the
|
||||
// server treats it the same as absent.
|
||||
expect(collectionApiMocks.createCollection).toHaveBeenCalledWith(
|
||||
expect.objectContaining({ ids: undefined }),
|
||||
// Collections are created empty — item ids must never be sent on create
|
||||
// (they go in the query string → HTTP 414 at scale); items are added via
|
||||
// addBatchToCollection.
|
||||
expect(collectionApiMocks.createCollection).not.toHaveBeenCalledWith(
|
||||
expect.objectContaining({ ids: expect.anything() }),
|
||||
);
|
||||
expect(result.id).toBe('collection-1');
|
||||
});
|
||||
|
||||
it('forwards initialItemIds to the Jellyfin SDK as ids', async () => {
|
||||
collectionApiMocks.createCollection.mockResolvedValueOnce({
|
||||
data: { Id: 'collection-2' },
|
||||
});
|
||||
|
||||
await service.createCollection({
|
||||
libraryId: 'library-1',
|
||||
title: 'Seeded',
|
||||
type: 'movie',
|
||||
initialItemIds: ['item-1', 'item-2'],
|
||||
});
|
||||
|
||||
expect(collectionApiMocks.createCollection).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
name: 'Seeded',
|
||||
parentId: 'library-1',
|
||||
isLocked: true,
|
||||
ids: ['item-1', 'item-2'],
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it('should add a batch of items in one Jellyfin request', async () => {
|
||||
await expect(
|
||||
service.addBatchToCollection('collection-1', ['item-1', 'item-2']),
|
||||
|
||||
@@ -1399,9 +1399,9 @@ export class JellyfinAdapterService implements IMediaServerService {
|
||||
}
|
||||
|
||||
try {
|
||||
// Created empty; items are added afterwards via addBatchToCollection.
|
||||
const response = await getCollectionApi(this.api).createCollection({
|
||||
name: params.title,
|
||||
ids: params.initialItemIds,
|
||||
parentId: params.libraryId,
|
||||
// isLocked enables composite image generation from collection items
|
||||
isLocked: true,
|
||||
|
||||
@@ -618,13 +618,15 @@ describe('PlexAdapterService', () => {
|
||||
).rejects.toThrow('Failed to create collection');
|
||||
});
|
||||
|
||||
it('forwards initialItemIds to PlexApiService for bulk create', async () => {
|
||||
it('creates the collection empty without forwarding item ids', async () => {
|
||||
// Items are added afterwards via the batched add path; seeding them into
|
||||
// the create request overflows the URL (HTTP 414).
|
||||
plexApi.createCollection.mockResolvedValue(
|
||||
createPlexCollection({
|
||||
ratingKey: 'col456',
|
||||
key: '/library/collections/col456',
|
||||
guid: 'plex://collection/col456',
|
||||
title: 'Seeded',
|
||||
title: 'New',
|
||||
subtype: 'movie',
|
||||
summary: '',
|
||||
index: 0,
|
||||
@@ -632,7 +634,7 @@ describe('PlexAdapterService', () => {
|
||||
thumb: '/thumb/col456',
|
||||
addedAt: 1609459200,
|
||||
updatedAt: 1609459200,
|
||||
childCount: '2',
|
||||
childCount: '0',
|
||||
maxYear: '2021',
|
||||
minYear: '2021',
|
||||
}),
|
||||
@@ -640,14 +642,13 @@ describe('PlexAdapterService', () => {
|
||||
|
||||
await service.createCollection({
|
||||
libraryId: 'lib1',
|
||||
title: 'Seeded',
|
||||
title: 'New',
|
||||
type: 'movie',
|
||||
initialItemIds: ['item-1', 'item-2'],
|
||||
});
|
||||
|
||||
expect(plexApi.createCollection).toHaveBeenCalledWith(
|
||||
expect(plexApi.createCollection).not.toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
initialItemIds: ['item-1', 'item-2'],
|
||||
initialItemIds: expect.anything(),
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
@@ -299,7 +299,6 @@ export class PlexAdapterService implements IMediaServerService {
|
||||
title: params.title,
|
||||
summary: params.summary,
|
||||
sortTitle: params.sortTitle,
|
||||
initialItemIds: params.initialItemIds,
|
||||
});
|
||||
|
||||
if (!result) {
|
||||
|
||||
@@ -28,7 +28,6 @@ export interface CreateUpdateCollection {
|
||||
summary?: string;
|
||||
child?: string;
|
||||
sortTitle?: string;
|
||||
initialItemIds?: string[];
|
||||
}
|
||||
|
||||
export interface PlexPlaylist {
|
||||
|
||||
@@ -862,19 +862,13 @@ export class PlexApiService {
|
||||
|
||||
public async createCollection(params: CreateUpdateCollection) {
|
||||
try {
|
||||
// When initial items are supplied, seed them at create time using the
|
||||
// canonical server-URI form that python-plexapi's Collection.create()
|
||||
// uses. Saves a round trip and avoids a half-created empty collection
|
||||
// if the follow-up add were to fail.
|
||||
const itemsUri = params.initialItemIds?.length
|
||||
? `&uri=${this.buildCollectionItemsUri(params.initialItemIds)}`
|
||||
: '';
|
||||
// Created empty; items are added afterwards via the batched add path.
|
||||
const response = await this.plexClient.postQuery<any>({
|
||||
uri: `/library/collections?type=${
|
||||
params.type
|
||||
}&title=${encodeURIComponent(params.title)}§ionId=${
|
||||
params.libraryId
|
||||
}${itemsUri}`,
|
||||
}`,
|
||||
});
|
||||
const collection: PlexCollection = response.MediaContainer
|
||||
.Metadata[0] as PlexCollection;
|
||||
|
||||
@@ -1807,7 +1807,10 @@ describe('CollectionsService', () => {
|
||||
expect(result[0].mediaData?.title).toBe('Fallback Movie');
|
||||
});
|
||||
|
||||
it('passes initial item ids on create for servers that support seeded collection creation', async () => {
|
||||
it('creates a new media server collection empty, then batch-adds items', async () => {
|
||||
// Regression: item ids must never be seeded into the create request (they
|
||||
// travel in the query string → HTTP 414 at scale). Create empty, then add
|
||||
// via the batched path.
|
||||
const collection = createCollection({
|
||||
id: 21,
|
||||
mediaServerId: null,
|
||||
@@ -1820,9 +1823,6 @@ describe('CollectionsService', () => {
|
||||
collectionRepo.save.mockImplementation(async (entity) => entity as any);
|
||||
collectionMediaRepo.find.mockResolvedValue([]);
|
||||
collectionPosterService.loadStoredPoster.mockResolvedValue(null);
|
||||
mediaServer.supportsFeature.mockImplementation(
|
||||
(feature) => feature === MediaServerFeature.BULK_COLLECTION_CREATE,
|
||||
);
|
||||
jest
|
||||
.spyOn(service as any, 'checkAutomaticMediaServerLink')
|
||||
.mockResolvedValue(collection);
|
||||
@@ -1836,16 +1836,16 @@ describe('CollectionsService', () => {
|
||||
]);
|
||||
|
||||
expect(mediaServer.createCollection).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
initialItemIds: ['episode-1', 'episode-2'],
|
||||
expect.not.objectContaining({
|
||||
initialItemIds: expect.anything(),
|
||||
}),
|
||||
);
|
||||
expect(mediaServer.addBatchToCollection).not.toHaveBeenCalled();
|
||||
// Not seeded on create, so the batched add path runs (skipMediaServerAdd=false).
|
||||
expect(addChildrenToCollection).toHaveBeenCalledWith(
|
||||
{ mediaServerId: 'remote-collection', dbId: collection.id },
|
||||
[{ mediaServerId: 'episode-1' }, { mediaServerId: 'episode-2' }],
|
||||
false,
|
||||
true,
|
||||
false,
|
||||
CollectionMediaManualMembershipSource.LOCAL,
|
||||
);
|
||||
});
|
||||
|
||||
@@ -1451,7 +1451,6 @@ export class CollectionsService {
|
||||
async createCollection(
|
||||
collection: ICollection,
|
||||
empty = true,
|
||||
initialItemIds?: string[],
|
||||
): Promise<
|
||||
| {
|
||||
dbCollection: addCollectionDbResponse;
|
||||
@@ -1474,7 +1473,6 @@ export class CollectionsService {
|
||||
summary: collection?.description,
|
||||
sortTitle: collection?.sortTitle,
|
||||
type: collection.type,
|
||||
initialItemIds,
|
||||
});
|
||||
|
||||
// Store the media server ID from the created collection
|
||||
@@ -1562,19 +1560,7 @@ export class CollectionsService {
|
||||
| undefined
|
||||
> {
|
||||
try {
|
||||
const mediaServer = await this.getMediaServer();
|
||||
const createWithItems = mediaServer.supportsFeature(
|
||||
MediaServerFeature.BULK_COLLECTION_CREATE,
|
||||
);
|
||||
const initialItemIds =
|
||||
createWithItems && media?.length
|
||||
? media.map((item) => item.mediaServerId)
|
||||
: undefined;
|
||||
const createdCollection = await this.createCollection(
|
||||
collection,
|
||||
false,
|
||||
initialItemIds,
|
||||
);
|
||||
const createdCollection = await this.createCollection(collection, false);
|
||||
|
||||
if (!createdCollection?.dbCollection) {
|
||||
return undefined;
|
||||
@@ -1590,7 +1576,7 @@ export class CollectionsService {
|
||||
},
|
||||
media,
|
||||
false,
|
||||
createWithItems && Boolean(initialItemIds?.length),
|
||||
false,
|
||||
);
|
||||
}
|
||||
|
||||
@@ -2011,21 +1997,6 @@ export class CollectionsService {
|
||||
!collection.mediaServerId &&
|
||||
(newMedia.length > 0 || collectionMedia.length > 0);
|
||||
|
||||
const createWithItems = mediaServer.supportsFeature(
|
||||
MediaServerFeature.BULK_COLLECTION_CREATE,
|
||||
);
|
||||
const initialItemIds = createWithItems
|
||||
? [
|
||||
...new Set([
|
||||
...collectionMedia.map(
|
||||
(existingMedia) => existingMedia.mediaServerId,
|
||||
),
|
||||
...newMedia.map((pendingMedia) => pendingMedia.mediaServerId),
|
||||
]),
|
||||
]
|
||||
: undefined;
|
||||
let seededOnCreate = false;
|
||||
|
||||
// Create media server collection if needed
|
||||
if (needsMediaServerCollection) {
|
||||
let newColl: MediaCollection | undefined = undefined;
|
||||
@@ -2047,9 +2018,7 @@ export class CollectionsService {
|
||||
summary: collection.description,
|
||||
sortTitle: collection.sortTitle,
|
||||
type: collection.type,
|
||||
initialItemIds,
|
||||
});
|
||||
seededOnCreate = Boolean(initialItemIds?.length);
|
||||
}
|
||||
}
|
||||
if (newColl?.id) {
|
||||
@@ -2087,8 +2056,9 @@ export class CollectionsService {
|
||||
);
|
||||
}
|
||||
|
||||
// Check if we need to sync existing items to a newly created collection
|
||||
const needsResync = collectionMedia.length > 0 && !seededOnCreate;
|
||||
// Sync existing collection_media rows to the freshly created
|
||||
// (empty) media server collection via the batched add path.
|
||||
const needsResync = collectionMedia.length > 0;
|
||||
|
||||
// If we had existing collection_media items, sync them to the new media server collection
|
||||
if (needsResync) {
|
||||
@@ -2178,7 +2148,7 @@ export class CollectionsService {
|
||||
{ mediaServerId: collection.mediaServerId, dbId: collection.id },
|
||||
newMedia,
|
||||
manual,
|
||||
skipMediaServerAdd || seededOnCreate,
|
||||
skipMediaServerAdd,
|
||||
manualMembershipSource,
|
||||
);
|
||||
|
||||
|
||||
@@ -46,8 +46,6 @@ export function isValidMediaItemType(type: string): type is MediaItemType {
|
||||
export enum MediaServerFeature {
|
||||
/** Ability to set collection visibility (home/recommended) */
|
||||
COLLECTION_VISIBILITY = 'collection_visibility',
|
||||
/** Adapter creates a collection and seeds initial items in one API call */
|
||||
BULK_COLLECTION_CREATE = 'bulk_collection_create',
|
||||
/** Watchlist functionality via external API (Plex.tv) */
|
||||
WATCHLIST = 'watchlist',
|
||||
/** Central watch history endpoint (vs per-user iteration) */
|
||||
|
||||
@@ -9,7 +9,6 @@ export const MEDIA_SERVER_FEATURES: Record<
|
||||
ReadonlySet<MediaServerFeature>
|
||||
> = {
|
||||
[MediaServerType.PLEX]: new Set([
|
||||
MediaServerFeature.BULK_COLLECTION_CREATE,
|
||||
MediaServerFeature.COLLECTION_VISIBILITY,
|
||||
MediaServerFeature.WATCHLIST,
|
||||
MediaServerFeature.CENTRAL_WATCH_HISTORY,
|
||||
@@ -19,7 +18,6 @@ export const MEDIA_SERVER_FEATURES: Record<
|
||||
MediaServerFeature.COLLECTION_SORT,
|
||||
]),
|
||||
[MediaServerType.JELLYFIN]: new Set([
|
||||
MediaServerFeature.BULK_COLLECTION_CREATE,
|
||||
MediaServerFeature.LABELS, // Tags in Jellyfin
|
||||
MediaServerFeature.PLAYLISTS,
|
||||
MediaServerFeature.COLLECTION_POSTER,
|
||||
@@ -29,7 +27,6 @@ export const MEDIA_SERVER_FEATURES: Record<
|
||||
// Note: COLLECTION_SORT not supported — no boxset reorder API; ForcedSortName has global side-effects.
|
||||
]),
|
||||
[MediaServerType.EMBY]: new Set([
|
||||
MediaServerFeature.BULK_COLLECTION_CREATE,
|
||||
MediaServerFeature.LABELS,
|
||||
MediaServerFeature.PLAYLISTS,
|
||||
MediaServerFeature.COLLECTION_POSTER,
|
||||
|
||||
@@ -227,7 +227,6 @@ export interface CreateCollectionParams {
|
||||
summary?: string
|
||||
type: MediaItemType
|
||||
sortTitle?: string
|
||||
initialItemIds?: string[]
|
||||
}
|
||||
|
||||
/** Plex-only visibility settings */
|
||||
|
||||
Reference in New Issue
Block a user