From 5bef071419d624525803628abfc1f4a405769e13 Mon Sep 17 00:00:00 2001 From: Eduard Marbach Date: Sat, 12 Oct 2024 17:23:10 +0200 Subject: [PATCH] fix: improve comparison for custom format changes * fixes a bug where added specifications (total length > on server than what you want) have been ignored --- src/custom-formats.ts | 4 ++-- src/util.test.ts | 53 +++++++++++++++++++++++++++++++++++-------- src/util.ts | 32 ++++++++++++++++---------- 3 files changed, 66 insertions(+), 23 deletions(-) diff --git a/src/custom-formats.ts b/src/custom-formats.ts index 4b5f9d4..ca326f0 100644 --- a/src/custom-formats.ts +++ b/src/custom-formats.ts @@ -5,7 +5,7 @@ import { getArrApi } from "./api"; import { getConfig } from "./config"; import { logger } from "./logger"; import { CFProcessing, ConfigCustomFormatList, ConfigarrCF, TrashCF } from "./types"; -import { IS_DRY_RUN, IS_LOCAL_SAMPLE_MODE, compareObjectsCarr, loadJsonFile, mapImportCfToRequestCf, toCarrCF } from "./util"; +import { IS_DRY_RUN, IS_LOCAL_SAMPLE_MODE, compareCustomFormats, loadJsonFile, mapImportCfToRequestCf, toCarrCF } from "./util"; export const deleteAllCustomFormats = async () => { const api = getArrApi(); @@ -52,7 +52,7 @@ export const manageCf = async ( if (existingCf) { // Update if necessary - const comparison = compareObjectsCarr(existingCf, tr.requestConfig); + const comparison = compareCustomFormats(existingCf, tr.requestConfig); if (!comparison.equal) { logger.info(`Found mismatch for ${tr.requestConfig.name}: ${comparison.changes}`); diff --git a/src/util.test.ts b/src/util.test.ts index f1016b7..8e31ed0 100644 --- a/src/util.test.ts +++ b/src/util.test.ts @@ -2,7 +2,7 @@ import path from "path"; import { describe, expect, test } from "vitest"; import { MergedCustomFormatResource } from "./__generated__/mergedTypes"; import { TrashCF, TrashCFSpF } from "./types"; -import { cloneWithJSON, compareObjectsCarr, loadJsonFile, mapImportCfToRequestCf, toCarrCF } from "./util"; +import { cloneWithJSON, compareCustomFormats, loadJsonFile, mapImportCfToRequestCf, toCarrCF } from "./util"; const exampleCFImplementations = { name: "TestSpec", @@ -140,7 +140,7 @@ describe("SizeSpecification", async () => { test("equal", async () => { const copied: typeof custom = JSON.parse(JSON.stringify(custom)); - const result = compareObjectsCarr(serverResponse, mapImportCfToRequestCf(toCarrCF(copied))); + const result = compareCustomFormats(serverResponse, mapImportCfToRequestCf(toCarrCF(copied))); expect(result.equal).toBe(true); }); @@ -148,7 +148,7 @@ describe("SizeSpecification", async () => { const copied = JSON.parse(JSON.stringify(custom)); copied.specifications![0].negate = true; - const result = compareObjectsCarr(serverResponse, mapImportCfToRequestCf(toCarrCF(copied))); + const result = compareCustomFormats(serverResponse, mapImportCfToRequestCf(toCarrCF(copied))); expect(result.equal).toBe(false); }); @@ -156,7 +156,7 @@ describe("SizeSpecification", async () => { const copied: typeof custom = JSON.parse(JSON.stringify(custom)); copied.specifications![0].required = true; - const result = compareObjectsCarr(serverResponse, mapImportCfToRequestCf(toCarrCF(copied))); + const result = compareCustomFormats(serverResponse, mapImportCfToRequestCf(toCarrCF(copied))); expect(result.equal).toBe(false); }); @@ -164,12 +164,12 @@ describe("SizeSpecification", async () => { const copied: typeof custom = JSON.parse(JSON.stringify(custom)); (copied.specifications![0].fields as TrashCFSpF).max = 100; - const result = compareObjectsCarr(serverResponse, mapImportCfToRequestCf(toCarrCF(copied))); + const result = compareCustomFormats(serverResponse, mapImportCfToRequestCf(toCarrCF(copied))); expect(result.equal).toBe(false); }); }); -describe("compareObjectsCarr - general", async () => { +describe("compareImportCFs - general", async () => { const filePath = path.resolve(__dirname, "../tests/samples/20240930_cf_exceptLanguage.json"); const serverResponse = loadJsonFile(filePath); @@ -193,7 +193,7 @@ describe("compareObjectsCarr - general", async () => { test("should not diff for fields length bigger on remote", async () => { const copied: typeof custom = JSON.parse(JSON.stringify(custom)); - const result = compareObjectsCarr(serverResponse, mapImportCfToRequestCf(toCarrCF(copied))); + const result = compareCustomFormats(serverResponse, mapImportCfToRequestCf(toCarrCF(copied))); expect(result.equal).toBe(true); }); @@ -204,7 +204,7 @@ describe("compareObjectsCarr - general", async () => { expect(clonedServer.specifications![0].fields.length).toBe(1); - const result = compareObjectsCarr(clonedServer, mapImportCfToRequestCf(toCarrCF(copied))); + const result = compareCustomFormats(clonedServer, mapImportCfToRequestCf(toCarrCF(copied))); expect(result.equal).toBe(true); }); @@ -217,7 +217,42 @@ describe("compareObjectsCarr - general", async () => { expect(clonedServer.specifications![0].fields.length).toBe(1); - const result = compareObjectsCarr(clonedServer, mapImportCfToRequestCf(toCarrCF(copied))); + const result = compareCustomFormats(clonedServer, mapImportCfToRequestCf(toCarrCF(copied))); expect(result.equal).toBe(false); }); + + test("should diff for specifications length bigger on remote", async () => { + const copied: typeof custom = JSON.parse(JSON.stringify(custom)); + const clonedServer = cloneWithJSON(serverResponse); + clonedServer.specifications![0].fields = [clonedServer.specifications![0].fields![0]]; + clonedServer.specifications?.push(clonedServer.specifications![0]); + + expect(clonedServer.specifications![0].fields.length).toBe(1); + + const result = compareCustomFormats(clonedServer, mapImportCfToRequestCf(toCarrCF(copied))); + expect(result.equal).toBe(false); + }); + + test("should diff for specifications length smaller on remote", async () => { + const copied: typeof custom = JSON.parse(JSON.stringify(custom)); + const clonedServer = cloneWithJSON(serverResponse); + clonedServer.specifications![0].fields = [clonedServer.specifications![0].fields![0]]; + copied.specifications?.push(copied.specifications[0]); + + expect(clonedServer.specifications![0].fields.length).toBe(1); + + const result = compareCustomFormats(clonedServer, mapImportCfToRequestCf(toCarrCF(copied))); + expect(result.equal).toBe(false); + }); + + test("should not diff for specifications length equal", async () => { + const copied: typeof custom = JSON.parse(JSON.stringify(custom)); + const clonedServer = cloneWithJSON(serverResponse); + clonedServer.specifications![0].fields = [clonedServer.specifications![0].fields![0]]; + + expect(clonedServer.specifications![0].fields.length).toBe(1); + + const result = compareCustomFormats(clonedServer, mapImportCfToRequestCf(toCarrCF(copied))); + expect(result.equal).toBe(true); + }); }); diff --git a/src/util.ts b/src/util.ts index 8a8ed61..d2b1e8c 100644 --- a/src/util.ts +++ b/src/util.ts @@ -107,7 +107,14 @@ export const mapImportCfToRequestCf = (cf: TrashCF | ConfigarrCF): MergedCustomF return { ...rest, specifications: specs }; }; -export function compareObjectsCarr(serverObject: any, localObject: any): { equal: boolean; changes: string[] } { +export function compareCustomFormats( + serverObject: MergedCustomFormatResource, + localObject: MergedCustomFormatResource, +): ReturnType { + return compareObjectsCarr(serverObject, localObject); +} + +function compareObjectsCarr(serverObject: any, localObject: any, parent?: string): { equal: boolean; changes: string[] } { const changes: string[] = []; for (const key in localObject) { @@ -116,30 +123,31 @@ export function compareObjectsCarr(serverObject: any, localObject: any): { equal const serverProperty = serverObject[key]; let localProperty = localObject[key]; - // Todo remove should be already handled - if (key === "fields") { - if (!Array.isArray(localProperty)) { - localProperty = [localProperty]; - } - } - if (Array.isArray(serverProperty)) { if (!Array.isArray(localProperty)) { changes.push(`Expected array for key ${key} in localProperty`); continue; } - if (serverProperty.length < localProperty.length) { + let arrayLengthMismatch = false; + + if (key === "fields") { // Only if server does provide less props as we have -> assume change required. // For example if radarr introduces new fields for custom formats and we do not have them included this would result in always changed results. + arrayLengthMismatch = serverProperty.length < localProperty.length; + } else if (serverProperty.length != localProperty.length) { + arrayLengthMismatch = true; + } + + if (arrayLengthMismatch) { changes.push( - `Array length mismatch for key ${key}: serverProperty length ${serverProperty.length}, localProperty length ${localProperty.length}`, + `Array length mismatch for key ${key} (parent: ${parent}): serverProperty length ${serverProperty.length}, localProperty length ${localProperty.length}`, ); continue; } for (let i = 0; i < serverProperty.length; i++) { - const { equal: isEqual, changes: subChanges } = compareObjectsCarr(serverProperty[i], localProperty[i]); + const { equal: isEqual, changes: subChanges } = compareObjectsCarr(serverProperty[i], localProperty[i], key); // changes.push( // ...subChanges.map((subChange) => `${key}[${i}].${subChange}`) // ); @@ -158,7 +166,7 @@ export function compareObjectsCarr(serverObject: any, localObject: any): { equal continue; } - const { equal: isEqual, changes: subChanges } = compareObjectsCarr(serverProperty, localProperty); + const { equal: isEqual, changes: subChanges } = compareObjectsCarr(serverProperty, localProperty, key); changes.push(...subChanges.map((subChange) => `${key}.${subChange}`)); if (!isEqual) { changes.push(`Mismatch found for key '${key}'`);