Skip to content

fix: added batch collection adding of media for Jellyfin#2494

Merged
enoch85 merged 7 commits into
Maintainerr:mainfrom
AlexS54:fix/jellyfin-batch-add-to-collection
Mar 24, 2026
Merged

fix: added batch collection adding of media for Jellyfin#2494
enoch85 merged 7 commits into
Maintainerr:mainfrom
AlexS54:fix/jellyfin-batch-add-to-collection

Conversation

@AlexS54

@AlexS54 AlexS54 commented Mar 15, 2026

Copy link
Copy Markdown
Contributor

Description & Design

This fixes potential Jellyfin errors caused by collection mutations requested by Maintainerr in quick succession.
The solution implemented here is taking advantage of Jellyfin's support for sending multiple media ids at the same time in one collection creation/media addition/media removal request.

The new batch methods are exposed in the media server abstractization and leverage the support described above for the Jellyfin implementation, and makes sequential calls in the Plex implementation.

This PR introduces a new MediaServerFeature: COLLECTION_CREATION_WITH_ITEMS, which represents whether the Media Server supports sending a list of media ids to be automatically added on collection creation.

Related issue

Fixes #2489

AI-Assisted Development

I used Copilot for tab autocomplete in a few parts of the code, but I read each suggestion before accepting it. Did not have the AI do any complex/agent work.

Checklist

  • I have read the CONTRIBUTING.md document.
  • I understand the code I am submitting and can explain how it works
  • I have performed a self-review of my code
  • I have linted and formatted my code
  • My changes generate no new warnings
  • New and existing unit tests pass locally with my changes

How to test

  1. Set up a Jellyfin server with multiple media items and connect it to the Maintainerr instance.
  2. Create a rule that matches multiple media items from Jellyfin and run it so a collection is created in Jellyfin.
  3. Delete the collection IN Jellyfin (not in Maintainerr).
  4. In Maintainerr, add a new media item to the collection.
  5. The Syncing ${collectionMedia.length} existing items to newly created media server collection message will not be displayed, but the collection will be recreated properly with all the items in Jellyfin.

@AlexS54

AlexS54 commented Mar 15, 2026

Copy link
Copy Markdown
Contributor Author

I didn't yet go into addChildToCollection and removeChildFromCollection because these pefrorm more processing on the media items one-at-a-time, including db operations and metadata fetching for logs.

If we redo these operations to use a bulk call to the media server, the other operations will also have do be done in bulk before/after this call, so I wanted to ask for input first.

Also if the MediaServerFeature approach is not good, let me know how to change it.

This comment was marked as resolved.

@AlexS54 AlexS54 force-pushed the fix/jellyfin-batch-add-to-collection branch from 4be1841 to be9571c Compare March 16, 2026 20:47
@AlexS54 AlexS54 marked this pull request as ready for review March 16, 2026 20:47
@AlexS54 AlexS54 requested a review from ydkmlt84 as a code owner March 16, 2026 20:47
Comment on lines 265 to +297
@@ -277,6 +286,15 @@ export class PlexAdapterService implements IMediaServerService {
}
}

async removeBatchFromCollection(
collectionId: string,
itemIds: string[],
): Promise<void> {
for (const itemId of itemIds) {
await this.removeFromCollection(collectionId, itemId);
}
}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right now the batch methods throw in the Plex adapter on any failed operation, I don't know if we should do this another way

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be fixed now.

Instead of throwing on the first Plex failure, the batch methods now return the failed item IDs. That lets the collection service handle partial success properly, while still treating Plex 404 removes as success and Jellyfin failures as all-or-nothing.

enoch85 added 2 commits March 21, 2026 19:57
- Guard against empty arrays in all batch methods (both adapters + service layer)
- Plex batch: continue on per-item failures instead of aborting the whole batch
- Plex batch remove: restore 404 tolerance (item already gone is not an error)
- Only pass itemIds to createCollection when COLLECTION_CREATION_WITH_ITEMS is supported
- Improve error messages to include the specific mediaServerId that failed
- Fix contract doc: reference feature flag instead of hardcoding "Jellyfin only"
@enoch85

enoch85 commented Mar 21, 2026

Copy link
Copy Markdown
Collaborator

Hey, nice work on the batch approach - using Jellyfin's native multi-id support is the right call. I went ahead and pushed a commit with some hardening fixes:

What I changed

  • Empty array guards everywhere — all batch methods (both adapters + the service layer helpers) now bail early on empty input instead of firing off pointless API calls

  • Plex batch methods don't blow up on partial failures anymore — if one item fails to add/remove, we keep going with the rest and only throw if everything failed. Previously a single bad item would abort the whole batch and leave the DB out of sync

  • Restored 404 tolerance on Plex remove — the old removeChildFromCollection had special handling for 404s (item already gone = fine). That got lost in the refactor, brought it back in removeBatchFromCollection

  • itemIds only sent when the server actually supports it — before this, createCollection always passed itemIds from collectionMedia, even for Plex which just silently ignores it. Now we check COLLECTION_CREATION_WITH_ITEMS first

  • Better error messages — the catch blocks in addChildrenToCollection / removeChildrenFromCollection now log which specific mediaServerId failed instead of just "An error occurred"

  • Contract cleanup — changed the itemIds comment from "Jellyfin only" to reference the feature flag properly, removed a "not tested on Plex" comment from the constants (Plex doesn't support this — it's not a matter of testing)

enoch85 added a commit that referenced this pull request Mar 21, 2026
PR #2494 - fix: added batch collection adding of media for Jellyfin
- Batch collection add/remove using Jellyfin's native multi-id API support
- New COLLECTION_CREATION_WITH_ITEMS feature flag to skip redundant resync
- Plex fallback via sequential calls with per-item error resilience
- Empty array guards on all batch methods
- Restored 404 tolerance on Plex batch remove
- itemIds only passed to createCollection when feature is supported
@enoch85 enoch85 self-assigned this Mar 21, 2026
@enoch85

enoch85 commented Mar 21, 2026

Copy link
Copy Markdown
Collaborator

Tightened the batch methods a bit — they now return failed IDs instead of assuming success, so the service layer only writes DB rows for items that actually landed on the server. Also wired up create-with-items properly: Jellyfin gets items at creation time, Plex and manual collections fall back to the sequential add path.

enoch85 added a commit that referenced this pull request Mar 21, 2026
PR #2494 - fix: added batch collection adding of media for Jellyfin
- Batch collection add/remove using Jellyfin's native multi-id API support
- New COLLECTION_CREATION_WITH_ITEMS feature flag to skip redundant resync
- Batch methods return failed IDs so the service layer only persists DB rows for items that actually landed on the server
- Create-with-items wired into createCollection: Jellyfin gets items at creation time, Plex and manual collections fall back to the sequential add path
- Plex sequential fallback with per-item error handling and 404 tolerance on remove
@enoch85

enoch85 commented Mar 21, 2026

Copy link
Copy Markdown
Collaborator

Tested, and works beautifully!

Add

[maintainerr] | 21/03/2026 22:25:29  [INFO] [RuleExecutorService] Executing rules for 'Test'
[maintainerr] | 21/03/2026 22:25:39  [DEBUG] [CollectionsService] [checkAutomaticMediaServerLink] Collection "Test" (DB id: 5, mediaServerId: 6ec5b089a18a56c8a21371a37d94d2fe)
[maintainerr] | 21/03/2026 22:25:39  [DEBUG] [CollectionsService] [checkAutomaticMediaServerLink] getCollection(6ec5b089a18a56c8a21371a37d94d2fe) returned: id=6ec5b089a18a56c8a21371a37d94d2fe, childCount=1
[maintainerr] | 21/03/2026 22:25:39  [INFO] [RuleExecutorService] Adding 36 media items to 'Test'.
[maintainerr] | 21/03/2026 22:25:39  [DEBUG] [CollectionsService] [checkAutomaticMediaServerLink] Collection "Test" (DB id: 5, mediaServerId: 6ec5b089a18a56c8a21371a37d94d2fe)
[maintainerr] | 21/03/2026 22:25:39  [DEBUG] [CollectionsService] [checkAutomaticMediaServerLink] getCollection(6ec5b089a18a56c8a21371a37d94d2fe) returned: id=6ec5b089a18a56c8a21371a37d94d2fe, childCount=1
[maintainerr] | 21/03/2026 22:25:39  [INFO] [CollectionsService] Adding 36 media items to collection..  <<<<-------------
[maintainerr] | 21/03/2026 22:26:05  [DEBUG] [CollectionsService] [checkAutomaticMediaServerLink] Collection "Test" (DB id: 5, mediaServerId: 6ec5b089a18a56c8a21371a37d94d2fe)
[maintainerr] | 21/03/2026 22:26:05  [DEBUG] [CollectionsService] [checkAutomaticMediaServerLink] getCollection(6ec5b089a18a56c8a21371a37d94d2fe) returned: id=6ec5b089a18a56c8a21371a37d94d2fe, childCount=37
[maintainerr] | 21/03/2026 22:26:05  [INFO] [RuleExecutorService] Execution of rules for 'Test' done.
[maintainerr] | 21/03/2026 22:26:05  [INFO] [RuleExecutorService] Synced collection 'Test' with media server

Remove

Maintainerr

[maintainerr] | 21/03/2026 22:26:05  [INFO] [RuleExecutorService] Execution of rules for 'Test' done.
[maintainerr] | 21/03/2026 22:26:05  [INFO] [RuleExecutorService] Synced collection 'Test' with media server
[maintainerr] | 21/03/2026 22:29:08  [DEBUG] [CollectionsService] [checkAutomaticMediaServerLink] Collection "Test" (DB id: 5, mediaServerId: 6ec5b089a18a56c8a21371a37d94d2fe)
[maintainerr] | 21/03/2026 22:29:08  [DEBUG] [CollectionsService] [checkAutomaticMediaServerLink] getCollection(6ec5b089a18a56c8a21371a37d94d2fe) returned: id=6ec5b089a18a56c8a21371a37d94d2fe, childCount=37
[maintainerr] | 21/03/2026 22:29:09  [INFO] [CollectionsService] Removing collection from database..
[maintainerr] | 21/03/2026 22:29:09  [INFO] [CollectionsService] Collection with id 5 has been removed from the database.
[maintainerr] | 21/03/2026 22:29:09  [INFO] [RulesService] Removed rulegroup with id 11 from the database

Jellyfin

[22:29:08] [INF] [46] Emby.Server.Implementations.Library.LibraryManager: Removing item, Type: BoxSet, Name: Test, Path: /config/data/data/collections/Test [boxset], Id: 6ec5b089-a18a-56c8-a213-71a37d94d2fe
[22:29:08] [INF] [46] Emby.Server.Implementations.Library.LibraryManager: Deleting item path, Type: BoxSet, Name: Test, Path: /config/data/data/collections/Test [boxset], Id: 6ec5b089-a18a-56c8-a213-71a37d94d2fe

Result: No locks at all in the Jellyfin logs! 🥳

enoch85
enoch85 previously approved these changes Mar 22, 2026
@enoch85 enoch85 self-requested a review March 24, 2026 21:07
@enoch85 enoch85 enabled auto-merge (squash) March 24, 2026 21:08
@enoch85 enoch85 merged commit 5ad01d3 into Maintainerr:main Mar 24, 2026
9 checks passed
@AlexS54 AlexS54 deleted the fix/jellyfin-batch-add-to-collection branch March 24, 2026 21:17
enoch85 added a commit that referenced this pull request Mar 24, 2026
Apply null-guard for mediaServerId to main's batch
removeChildrenFromCollection method, preserving the fix
from #2510 while adopting the batch API from #2494.
@github-actions

Copy link
Copy Markdown
Contributor

🎉 This PR is included in version 3.2.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

enoch85 added a commit that referenced this pull request Mar 28, 2026
…ellyfin

Fixes a regression introduced in v3.2.0 where rule execution would crash
with "Cannot read properties of undefined (reading 'id')" for Jellyfin users.

The COLLECTION_CREATION_WITH_ITEMS optimisation (PR #2494, fixing #2489) attempted
to create a Jellyfin collection with all existing item IDs in a single atomic API
call. If Jellyfin rejected any ID (e.g. stale/unavailable media), the entire
createCollection call threw, addToCollectionInternal returned undefined, and the
caller in rule-executor then accessed collection.id on that undefined — crashing
the rule run for every affected collection.

The original lag problem that motivated PR #2494 was independently solved by
PR #2511 (issue #2493), so the optimisation no longer provides any benefit.

Fix:
- Remove itemIds from all createCollection call sites; collections are always
  created empty and items are added via the existing addBatchToCollection path
- Remove the COLLECTION_CREATION_WITH_ITEMS feature flag (enum, constants,
  CreateCollectionParams.itemIds) as it is now unused
- Add null guards in addToCollectionWithResolvedLink and
  removeFromCollectionWithResolvedLink to prevent crashes if collection creation
  fails for any other reason
- Add collection && guard in rule-executor before the remove block for the same
  defensive reason
- Update tests to reflect the new invariant: createCollection never receives
  itemIds regardless of media server type

Removed implementation introduced by: PR #2494
PR #2494 fixed: issue #2489
Related later Jellyfin collection creation fix: PR #2511 for issue #2493
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Jellyfin collection sync can trigger collection.xml file lock errors

2 participants