Skip to content

Commit

Permalink
Merge pull request #86 from raydak-labs/fix/cf-specification-length
Browse files Browse the repository at this point in the history
fix: improve comparison for custom format changes
  • Loading branch information
BlackDark authored Oct 12, 2024
2 parents 581736d + 5bef071 commit d3c9c09
Show file tree
Hide file tree
Showing 3 changed files with 66 additions and 23 deletions.
4 changes: 2 additions & 2 deletions src/custom-formats.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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}`);
Expand Down
53 changes: 44 additions & 9 deletions src/util.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -140,36 +140,36 @@ 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);
});

test("mismatch negate", 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);
});

test("mismatch required", 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);
});

test("max differ", 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<MergedCustomFormatResource>(filePath);

Expand All @@ -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);
});

Expand All @@ -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);
});

Expand All @@ -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);
});
});
32 changes: 20 additions & 12 deletions src/util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<typeof compareObjectsCarr> {
return compareObjectsCarr(serverObject, localObject);
}

function compareObjectsCarr(serverObject: any, localObject: any, parent?: string): { equal: boolean; changes: string[] } {
const changes: string[] = [];

for (const key in localObject) {
Expand All @@ -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}`)
// );
Expand All @@ -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}'`);
Expand Down

0 comments on commit d3c9c09

Please sign in to comment.