Add Readarr root folder sync support #365
Closed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Reviewer's GuideImplements Readarr-specific root folder synchronization that resolves profile names, tags, and optional Calibre settings into the Readarr API shape, and wires it into the existing root folder sync factory and config typing alongside Lidarr and the generic implementation. Sequence diagram for resolving a Readarr root folder configsequenceDiagram
actor Operator
participant Orchestrator as RootFolderSyncOrchestrator
participant Factory as RootFolderSyncFactory
participant Sync as ReadarrRootFolderSync
participant Cache as ServerCache
participant QC as QualityProfileLoader
participant RC as ReadarrClient
participant ReadarrAPI
Operator->>Orchestrator: runSync(readarr, config)
Orchestrator->>Factory: createRootFolderSync(READARR)
Factory-->>Orchestrator: ReadarrRootFolderSync instance
Orchestrator->>Sync: resolveRootFolderConfig(configFolder, serverCache)
par Load profiles
Sync->>QC: loadQualityProfilesFromServer()
QC->>ReadarrAPI: GET /qualityprofile
ReadarrAPI-->>QC: QualityProfileResource[]
QC-->>Sync: QualityProfileResource[]
Sync->>RC: getMetadataProfiles()
RC->>ReadarrAPI: GET /metadataprofile
ReadarrAPI-->>RC: MetadataProfileResource[]
RC-->>Sync: MetadataProfileResource[]
end
Sync->>Sync: build name->id maps
Sync->>Sync: lookup metadata_profile and quality_profile
alt any profile missing
Sync-->>Orchestrator: throw Error
Orchestrator-->>Operator: report configuration error
else profiles found
alt tags configured
loop for each tag in config.tags
Sync->>Cache: find tag by label
alt tag exists in cache
Cache-->>Sync: existing TagResource
else tag not in cache
Sync->>RC: createTag(label)
RC->>ReadarrAPI: POST /tag
ReadarrAPI-->>RC: TagResource
RC-->>Sync: TagResource
Sync->>Cache: add TagResource to tags
end
end
end
Sync->>Sync: construct RootFolderResource
Sync-->>Orchestrator: RootFolderResource
Orchestrator-->>Operator: sync continues with resolved root folder
end
Class diagram for ReadarrRootFolderSync and Readarr config typesclassDiagram
class InputConfigRootFolderGeneric {
}
class InputConfigRootFolderLidarr {
+string path
+string name
+string metadata_profile
+string quality_profile
+string[] tags
}
class InputConfigRootFolderReadarr {
+string path
+string name
+string metadata_profile
+string quality_profile
+MonitorOption monitor
+NewItemMonitorOption monitor_new_items
+string[] tags
+boolean is_calibre_library
+string calibre_host
+number calibre_port
+string calibre_url_base
+string calibre_username
+string calibre_password
+string calibre_library
+string calibre_output_format
+string calibre_output_profile
+boolean calibre_use_ssl
}
class InputConfigRootFolder {
}
class ServerCache {
+TagResource[] tags
}
class TagResource {
+number id
+string label
}
class RootFolderResource {
+number id
+string path
+string name
+number defaultMetadataProfileId
+number defaultQualityProfileId
+MonitorTypes defaultMonitorOption
+NewItemMonitorTypes defaultNewItemMonitorOption
+number[] defaultTags
+boolean isCalibreLibrary
+string host
+number port
+string urlBase
+string username
+string password
+string library
+string outputFormat
+string outputProfile
+boolean useSsl
}
class BaseRootFolderSync {
+logger logger
+Promise~RootFolderResource[]~ loadRootFoldersFromServer()
}
class ReadarrClient {
+Promise~MetadataProfileResource[]~ getMetadataProfiles()
+Promise~TagResource~ createTag(label)
}
class ReadarrRootFolderSync {
+ReadarrClient api
+Promise~RootFolderResource~ resolveRootFolderConfig(config, serverCache)
+Promise~RootFolderDiff~ calculateDiff(rootFolders, serverCache)
-boolean isRootFolderConfigEqual(resolvedConfig, serverFolder)
+string getArrType()
}
class RootFolderDiff {
+InputConfigRootFolder[] missingOnServer
+RootFolderResource[] notAvailableAnymore
+ChangedRootFolder[] changed
}
class ChangedRootFolder {
+InputConfigRootFolder config
+RootFolderResource server
}
class RootFolderSyncFactory {
+BaseRootFolderSync createRootFolderSync(arrType)
}
class ArrType {
<<enumeration>>
LIDARR
READARR
OTHER
}
class QualityProfileResource {
+number id
+string name
}
class MetadataProfileResource {
+number id
+string name
}
InputConfigRootFolderGeneric <|.. InputConfigRootFolder
InputConfigRootFolderLidarr <|.. InputConfigRootFolder
InputConfigRootFolderReadarr <|.. InputConfigRootFolder
BaseRootFolderSync <|-- ReadarrRootFolderSync
ReadarrRootFolderSync o-- ReadarrClient
ReadarrRootFolderSync o-- ServerCache
ReadarrRootFolderSync ..> RootFolderResource
ReadarrRootFolderSync ..> RootFolderDiff
ReadarrRootFolderSync ..> TagResource
ReadarrRootFolderSync ..> QualityProfileResource
ReadarrRootFolderSync ..> MetadataProfileResource
RootFolderSyncFactory ..> BaseRootFolderSync
RootFolderSyncFactory ..> ArrType
RootFolderSyncFactory ..> ReadarrRootFolderSync
RootFolderSyncFactory ..> InputConfigRootFolder
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 3 issues, and left some high level feedback:
- The
isRootFolderConfigEqualcomparison includespasswordand the rawdefaultTagsarray, which may cause persistent diffs if the server obscures credentials or returns tags in a different order; consider excluding sensitive fields and/or normalizing tag IDs before comparison. - In
resolveRootFolderConfig, quality and metadata profiles are fetched on each call; sincecalculateDiffcan call this per root folder, you could reduce API chatter by resolving and caching these profiles once per sync (e.g., viaServerCacheor a local memoization).
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The `isRootFolderConfigEqual` comparison includes `password` and the raw `defaultTags` array, which may cause persistent diffs if the server obscures credentials or returns tags in a different order; consider excluding sensitive fields and/or normalizing tag IDs before comparison.
- In `resolveRootFolderConfig`, quality and metadata profiles are fetched on each call; since `calculateDiff` can call this per root folder, you could reduce API chatter by resolving and caching these profiles once per sync (e.g., via `ServerCache` or a local memoization).
## Individual Comments
### Comment 1
<location> `src/rootFolder/rootFolderReadarr.ts:31` </location>
<code_context>
+ }
+
+ // Load quality profiles and metadata profiles for Readarr
+ const [qualityProfiles, metadataProfiles] = await Promise.all([loadQualityProfilesFromServer(), this.api.getMetadataProfiles()]);
+
+ const qualityProfileMap = new Map<string, number>();
</code_context>
<issue_to_address>
**suggestion (performance):** Avoid reloading profiles on every `resolveRootFolderConfig` call by reusing cached data.
This method refetches quality and metadata profiles for every root folder, which can add overhead and extra load on the Readarr server when many folders are processed. Since these profiles are global and stable during a sync run, cache them (e.g., in `ServerCache` or a private field on the syncer) and reuse them across `resolveRootFolderConfig` calls within `calculateDiff`.
Suggested implementation:
```typescript
// Load quality profiles and metadata profiles for Readarr, cached per server
let qualityProfileMap: Map<string, number> | undefined =
(serverCache as any).readarrQualityProfilesByName;
let metadataProfileMap: Map<string, number> | undefined =
(serverCache as any).readarrMetadataProfilesByName;
if (!qualityProfileMap || !metadataProfileMap) {
const [qualityProfiles, metadataProfiles] = await Promise.all([
loadQualityProfilesFromServer(),
this.api.getMetadataProfiles(),
]);
qualityProfileMap = new Map<string, number>();
metadataProfileMap = new Map<string, number>();
qualityProfiles.forEach((profile: QualityProfileResource) => {
if (profile.name && profile.id !== undefined) {
qualityProfileMap!.set(profile.name, profile.id);
}
});
metadataProfiles.forEach((profile: MetadataProfileResource) => {
if (profile.id !== undefined && profile.name) {
metadataProfileMap!.set(profile.name, profile.id);
}
});
(serverCache as any).readarrQualityProfilesByName = qualityProfileMap;
(serverCache as any).readarrMetadataProfilesByName = metadataProfileMap;
}
// At this point both maps are guaranteed to be populated
qualityProfileMap = qualityProfileMap!;
metadataProfileMap = metadataProfileMap!;
```
To make this compile cleanly with TypeScript and fit your existing conventions, you should also:
1. Extend the `ServerCache` type/interface to include the two new cached maps:
- `readarrQualityProfilesByName?: Map<string, number>;`
- `readarrMetadataProfilesByName?: Map<string, number>;`
This will allow you to remove the `(serverCache as any)` casts and use `serverCache.readarrQualityProfilesByName` directly.
2. If you have similar caching patterns for other *arr syncers (e.g., Sonarr/Radarr), align the property names and caching strategy with those for consistency (potentially extracting a shared helper if one exists).
</issue_to_address>
### Comment 2
<location> `src/rootFolder/rootFolderReadarr.ts:133-142` </location>
<code_context>
+ password: resolvedConfig.password,
</code_context>
<issue_to_address>
**issue (bug_risk):** Comparing the Calibre password field may cause perpetual diffs if the server does not return the raw password.
If the Readarr API omits, masks, or normalizes this field, the local config and server values will never compare equal, causing the resource to appear changed on every run. Consider excluding `password` from equality checks or explicitly treating empty/masked server values as “don’t care” to avoid perpetual noisy diffs while still allowing real updates to be applied.
</issue_to_address>
### Comment 3
<location> `src/rootFolder/rootFolderReadarr.ts:126-135` </location>
<code_context>
+ defaultTags: resolvedConfig.defaultTags,
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Tag comparison likely depends on array ordering, which may cause false positives in diffs.
If `compareObjectsCarr` does positional array comparison, the same tag IDs in a different order will appear as changes. Unless the API guarantees tag order, consider normalizing (e.g., sort IDs) before comparison or compare them as sets instead of ordered arrays.
Suggested implementation:
```typescript
// Normalize tag ordering so comparison is not sensitive to array order
defaultTags: resolvedConfig.defaultTags
? [...resolvedConfig.defaultTags].sort()
: resolvedConfig.defaultTags,
```
To fully implement the suggestion, you should also:
1. Apply the same normalization to the corresponding `defaultTags` field for `serverFolder` wherever its config fields are assembled for comparison.
2. If tag IDs are not simple primitives (e.g., objects), adjust the sort to use a stable key (e.g., `sort((a, b) => a.id - b.id)` or `String(a).localeCompare(String(b))`).
3. If `compareObjectsCarr` is used elsewhere for tag arrays, consider centralizing this “sort before compare” behavior to avoid future ordering-related diffs.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
ghostwriternr
force-pushed
the
feat/readarr-root-folder-sync
branch
from
c96cef8 to
e4ccbd1
Compare
Readarr's API requires defaultMetadataProfileId and defaultQualityProfileId when creating root folders, but the generic implementation only passes the path. This adds a dedicated ReadarrRootFolderSync class following the LidarrRootFolderSync pattern. The new implementation supports: - Required fields: name, metadata_profile, quality_profile - Optional fields: monitor, monitor_new_items, tags - Calibre library integration (Readarr-specific) Resolves part of raydak-labs#264
ghostwriternr
force-pushed
the
feat/readarr-root-folder-sync
branch
from
e4ccbd1 to
bbd676b
Compare
Member
|
Hi @ghostwriternr thx for the PR. Cherry picked your commit and removed the |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
Readarr's API requires
defaultMetadataProfileIdanddefaultQualityProfileIdwhen creating root folders, but the generic implementation only passes the path. This adds a dedicatedReadarrRootFolderSyncclass following the existingLidarrRootFolderSyncpattern.Changes
InputConfigRootFolderReadarrtype with Calibre integration fieldsReadarrRootFolderSyncclass with profile name-to-ID resolutionExample Config
Related
Summary by Sourcery
Add dedicated Readarr root folder synchronization with support for profile resolution and Calibre-specific settings.
New Features:
Enhancements:
Tests: