fix: allow loading custom format definition correctly from templates

- also fixes a bug with possible CF overwrites because we loaded all CF definitions even if not needed

Author: BlackDark

src/custom-formats.test.ts

@@ -82,7 +82,7 @@ describe("CustomFormats", () => {
         cfNameToCarrConfig: new Map([["CF2", { configarr_id: "id2", name: "CF2" }]]),
       };
 
-      const result = mergeCfSources([source1, source2, null]);
+      const result = mergeCfSources(new Set(["id1", "id2"]), [source1, source2, null]);
 
       expect(result.carrIdMapping.size).toBe(2);
       expect(result.cfNameToCarrConfig.size).toBe(2);
@@ -113,54 +113,50 @@ describe("CustomFormats", () => {
   describe("loadCustomFormatDefinitions", () => {
     it("should load and merge (trash CFDs", async () => {
       const mockTrashCFs: CFProcessing = {
-        carrIdMapping: new Map([["trash1", { carrConfig: { configarr_id: "trash1" }, requestConfig: {} }]]),
+        carrIdMapping: new Map([["trash1", { carrConfig: { configarr_id: "trash1", name: "trash1" }, requestConfig: {} }]]),
         cfNameToCarrConfig: new Map(),
       };
 
       vi.mock("./trash-guide");
       vi.mocked(loadTrashCFs).mockResolvedValue(mockTrashCFs);
       vi.spyOn(config, "getConfig").mockReturnValue({ localCustomFormatsPath: undefined });
 
-      const result = await loadCustomFormatDefinitions("RADARR", []);
+      const result = await loadCustomFormatDefinitions(new Set(["trash1"]), "RADARR", []);
 
       expect(result.carrIdMapping.size).toBe(1);
       expect(result.carrIdMapping.has("trash1")).toBeTruthy();
     });
 
     it("should load and merge (additional CFDs)", async () => {
       const mockTrashCFs: CFProcessing = {
-        carrIdMapping: new Map([["trash1", { carrConfig: { configarr_id: "trash1" }, requestConfig: {} }]]),
+        carrIdMapping: new Map([["trash1", { carrConfig: { configarr_id: "trash1", name: "trash1" }, requestConfig: {} }]]),
         cfNameToCarrConfig: new Map(),
       };
 
       vi.mock("./trash-guide");
       vi.mocked(loadTrashCFs).mockResolvedValue(mockTrashCFs);
       vi.spyOn(config, "getConfig").mockReturnValue({ localCustomFormatsPath: undefined });
 
-      const result = await loadCustomFormatDefinitions("RADARR", [customCF]);
+      const result = await loadCustomFormatDefinitions(new Set(["trash1", customCF.trash_id]), "RADARR", [customCF]);
 
       expect(result.carrIdMapping.size).toBe(2);
       expect(result.carrIdMapping.has("trash1")).toBeTruthy();
     });
 
-    it("should load and merge (config CFDs)", async () => {
+    it("should ignore not managed CFs", async () => {
       const mockTrashCFs: CFProcessing = {
-        carrIdMapping: new Map(),
+        carrIdMapping: new Map([["trash1", { carrConfig: { configarr_id: "trash1", name: "trash1" }, requestConfig: {} }]]),
         cfNameToCarrConfig: new Map(),
       };
 
-      const clonedCFD: TrashCF = JSON.parse(JSON.stringify(customCF));
-      clonedCFD.trash_id = "trash2";
-      clonedCFD.name = "Trash2";
-
       vi.mock("./trash-guide");
       vi.mocked(loadTrashCFs).mockResolvedValue(mockTrashCFs);
-      vi.spyOn(config, "getConfig").mockReturnValue({ localCustomFormatsPath: undefined, customFormatDefinitions: [customCF] });
+      vi.spyOn(config, "getConfig").mockReturnValue({ localCustomFormatsPath: undefined });
 
-      const result = await loadCustomFormatDefinitions("RADARR", [clonedCFD]);
+      const result = await loadCustomFormatDefinitions(new Set(["trash1"]), "RADARR", [customCF]);
 
-      expect(result.carrIdMapping.size).toBe(2);
-      expect(result.carrIdMapping.has("trash2")).toBeTruthy();
+      expect(result.carrIdMapping.size).toBe(1);
+      expect(result.carrIdMapping.has("trash1")).toBeTruthy();
     });
   });
 });

src/custom-formats.ts

@@ -195,16 +195,16 @@ export const mapCustomFormatDefinitions = (customFormatDefinitions: CustomFormat
   };
 };
 
-export const loadCustomFormatDefinitions = async (arrType: ArrType, additionalCFDs: CustomFormatDefinitions) => {
+export const loadCustomFormatDefinitions = async (idsToMange: Set<string>, arrType: ArrType, additionalCFDs: CustomFormatDefinitions) => {
+  // TODO: the object CFProcessing is only needed as result from this method. All other should only work with ID -> object
   const trashCFs = await loadTrashCFs(arrType);
   const localFileCFs = await loadLocalCfs();
-  const configCFDs = loadCFFromConfig();
 
   logger.debug(
-    `Loaded ${trashCFs.carrIdMapping.size} TrashCFs, ${localFileCFs?.carrIdMapping.size} LocalCFs, ${configCFDs?.carrIdMapping.size} ConfigCFs, ${additionalCFDs.length} AdditionalCFs`,
+    `Total loaded CF definitions: ${trashCFs.carrIdMapping.size} TrashCFs, ${localFileCFs?.carrIdMapping.size == null ? 0 : localFileCFs?.carrIdMapping.size} LocalCFs, ${additionalCFDs.length} ConfigCFs`,
   );
 
-  return mergeCfSources([trashCFs, localFileCFs, configCFDs, mapCustomFormatDefinitions(additionalCFDs)]);
+  return mergeCfSources(idsToMange, [trashCFs, localFileCFs, mapCustomFormatDefinitions(additionalCFDs)]);
 };
 
 export const calculateCFsToManage = (yaml: ConfigCustomFormatList) => {
@@ -219,25 +219,29 @@ export const calculateCFsToManage = (yaml: ConfigCustomFormatList) => {
   return cfTrashToManage;
 };
 
-export const mergeCfSources = (listOfCfs: (CFProcessing | null)[]): CFProcessing => {
+export const mergeCfSources = (idsToManage: Set<string>, listOfCfs: (CFProcessing | null)[]): CFProcessing => {
   return listOfCfs.reduce<CFProcessing>(
     (p, c) => {
       if (!c) {
         return p;
       }
 
-      for (const [key, value] of c.carrIdMapping.entries()) {
-        if (p.carrIdMapping.has(key)) {
-          logger.info(`Overwriting ${key} during CF merge`);
-        }
-        p.carrIdMapping.set(key, value);
-      }
+      for (const test of idsToManage) {
+        const value = c.carrIdMapping.get(test);
+        const cfName = value?.carrConfig.name!;
+
+        if (value) {
+          if (p.carrIdMapping.has(test)) {
+            logger.warn(`Overwriting CF with id '${test}' during merge.`);
+          }
+
+          if (p.cfNameToCarrConfig.has(cfName)) {
+            logger.warn(`Overwriting CF with name '${cfName}' (ID: ${test}) during merge.`);
+          }
 
-      for (const [key, value] of c.cfNameToCarrConfig.entries()) {
-        if (p.cfNameToCarrConfig.has(key)) {
-          logger.info(`Overwriting ${key} during CF merge`);
+          p.carrIdMapping.set(test, value);
+          p.cfNameToCarrConfig.set(value.carrConfig.name!, value.carrConfig);
         }
-        p.cfNameToCarrConfig.set(key, value);
       }
 
       return p;

src/index.ts

@@ -21,26 +21,18 @@ import {
   transformTrashQPCFs,
   transformTrashQPToTemplate,
 } from "./trash-guide";
-import { ArrType, CFProcessing, MappedMergedTemplates } from "./types/common.types";
-import {
-  ConfigQualityProfile,
-  CustomFormatDefinitions,
-  InputConfigArrInstance,
-  InputConfigIncludeItem,
-  MergedConfigInstance,
-} from "./types/config.types";
+import { ArrType, MappedMergedTemplates } from "./types/common.types";
+import { ConfigQualityProfile, InputConfigArrInstance, InputConfigIncludeItem, MergedConfigInstance } from "./types/config.types";
 import { TrashQualityDefintion } from "./types/trashguide.types";
 
 /**
  * Load data from trash, recyclarr, custom configs and merge.
  * Afterwards do sanitize and check against required configuration.
+ * TODO: probably move to config.ts and write tests for it for different merge scenarios
  * @param value
  * @param arrType
  */
-const mergeConfigsAndTemplates = async (
-  value: InputConfigArrInstance,
-  arrType: ArrType,
-): Promise<{ mergedCFs: CFProcessing; config: MergedConfigInstance }> => {
+const mergeConfigsAndTemplates = async (value: InputConfigArrInstance, arrType: ArrType): Promise<{ config: MergedConfigInstance }> => {
   const recyclarrTemplateMap = loadRecyclarrTemplates(arrType);
   const localTemplateMap = loadLocalRecyclarrTemplate(arrType);
   const trashTemplates = await loadQPFromTrash(arrType);
@@ -54,7 +46,7 @@ const mergeConfigsAndTemplates = async (
     quality_profiles: [],
   };
 
-  // TODO: customFormatDefinitions not supported in templates yet
+  // HINT: we assume customFormatDefinitions only exist in RECYCLARR
   if (value.include) {
     const mappedIncludes = value.include.reduce<{ recyclarr: InputConfigIncludeItem[]; trash: InputConfigIncludeItem[] }>(
       (previous, current) => {
@@ -108,6 +100,17 @@ const mergeConfigsAndTemplates = async (
         recyclarrMergedTemplates.media_naming = { ...recyclarrMergedTemplates.media_naming, ...template.media_naming };
       }
 
+      if (template.customFormatDefinitions) {
+        if (Array.isArray(template.customFormatDefinitions)) {
+          recyclarrMergedTemplates.customFormatDefinitions = [
+            ...(recyclarrMergedTemplates.customFormatDefinitions || []),
+            ...template.customFormatDefinitions,
+          ];
+        } else {
+          logger.warn(`CustomFormatDefinitions in template must be an array. Ignoring.`);
+        }
+      }
+
       // TODO Ignore recursive include for now
       if (template.include) {
         logger.warn(`Recursive includes not supported at the moment. Ignoring.`);
@@ -128,6 +131,7 @@ const mergeConfigsAndTemplates = async (
     });
   }
 
+  // Config values overwrite template values
   if (value.custom_formats) {
     recyclarrMergedTemplates.custom_formats.push(...value.custom_formats);
   }
@@ -148,6 +152,17 @@ const mergeConfigsAndTemplates = async (
     recyclarrMergedTemplates.quality_definition = { ...recyclarrMergedTemplates.quality_definition, ...value.quality_definition };
   }
 
+  if (value.customFormatDefinitions) {
+    if (Array.isArray(value.customFormatDefinitions)) {
+      recyclarrMergedTemplates.customFormatDefinitions = [
+        ...(recyclarrMergedTemplates.customFormatDefinitions || []),
+        ...value.customFormatDefinitions,
+      ];
+    } else {
+      logger.warn(`CustomFormatDefinitions in config file must be an array. Ignoring.`);
+    }
+  }
+
   const recyclarrProfilesMerged = recyclarrMergedTemplates.quality_profiles.reduce<Map<string, ConfigQualityProfile>>((p, c) => {
     const profile = p.get(c.name);
 
@@ -175,21 +190,6 @@ const mergeConfigsAndTemplates = async (
 
   recyclarrMergedTemplates.quality_profiles = filterInvalidQualityProfiles(recyclarrMergedTemplates.quality_profiles);
 
-  /* 
-  TODO: do we want to load all available local templates or only the included ones in the instance?
-  Example: we have a local template folder which we can always traverse. So we could load every CF defined there.
-  But then we could also have in theory conflicted CF IDs if user want to define same CF in different templates.
-  How to handle overwrite? Maybe also support overriding CFs defined in Trash or something?
-  */
-  const localTemplateCFDs = Array.from(localTemplateMap.values()).reduce((p, c) => {
-    if (c.customFormatDefinitions) {
-      p.push(...c.customFormatDefinitions);
-    }
-    return p;
-  }, [] as CustomFormatDefinitions);
-
-  const mergedCFs = await loadCustomFormatDefinitions(arrType, localTemplateCFDs);
-
   // merge profiles from recyclarr templates into one
   const qualityProfilesMerged = recyclarrMergedTemplates.quality_profiles.reduce((p, c) => {
     let existingQp = p.get(c.name);
@@ -218,18 +218,33 @@ const mergeConfigsAndTemplates = async (
 
   const validatedConfig = validateConfig(recyclarrMergedTemplates);
   logger.debug(`Merged config: '${JSON.stringify(validatedConfig)}'`);
-  return { mergedCFs: mergedCFs, config: validatedConfig };
+
+  /* 
+  TODO: do we want to load all available local templates or only the included ones in the instance?
+  Example: we have a local template folder which we can always traverse. So we could load every CF defined there.
+  But then we could also have in theory conflicted CF IDs if user want to define same CF in different templates.
+  How to handle overwrite? Maybe also support overriding CFs defined in Trash or something?
+  */
+  // const localTemplateCFDs = Array.from(localTemplateMap.values()).reduce((p, c) => {
+  //   if (c.customFormatDefinitions) {
+  //     p.push(...c.customFormatDefinitions);
+  //   }
+  //   return p;
+  // }, [] as CustomFormatDefinitions);
+
+  return { config: validatedConfig };
 };
 
 const pipeline = async (value: InputConfigArrInstance, arrType: ArrType) => {
   const api = getUnifiedClient();
 
-  const { config, mergedCFs } = await mergeConfigsAndTemplates(value, arrType);
+  const { config } = await mergeConfigsAndTemplates(value, arrType);
 
   const idsToManage = calculateCFsToManage(config);
-
   logger.debug(Array.from(idsToManage), `CustomFormats to manage`);
 
+  const mergedCFs = await loadCustomFormatDefinitions(idsToManage, arrType, config.customFormatDefinitions || []);
+
   let serverCFs = await loadServerCustomFormats();
   logger.info(`CustomFormats on server: ${serverCFs.length}`);
 

src/types/common.types.ts

@@ -42,7 +42,7 @@ export type TCM = TC1 | TC2;
 
 export type ImportCF = OmitTyped<MergedCustomFormatResource, "specifications"> & {
   specifications?: TCM[] | null;
-};
+} & Required<Pick<MergedCustomFormatResource, "name">>;
 
 export type ConfigarrCFMeta = {
   configarr_id: string;

src/types/config.types.ts

@@ -1,7 +1,7 @@
-import { ConfigarrCFMeta } from "./common.types";
-import { TrashCFMeta, TrashScores } from "./trashguide.types";
+import { ConfigarrCF } from "./common.types";
+import { TrashCF, TrashScores } from "./trashguide.types";
 
-export type CustomFormatDefinitions = (TrashCFMeta | ConfigarrCFMeta)[];
+export type CustomFormatDefinitions = (TrashCF | ConfigarrCF)[];
 
 export type InputConfigSchema = {
   trashGuideUrl?: string;