From beab83b56e23eff11581c9d2e7e2f37e261d557d Mon Sep 17 00:00:00 2001 From: Pujit Mehrotra Date: Thu, 7 Aug 2025 10:03:53 -0400 Subject: [PATCH] refactor: mv `7.2.0` version check to file modifier super class (#1567) ## Summary by CodeRabbit * **Bug Fixes** * `nginx:reload` effect is no longer triggered via the nginx.conf modification on 7.2.0. * Improved consistency in determining when patches and modifications should be applied for Unraid versions 7.2.0 and above. * Removed redundant version checks from several modification modules to streamline patch application logic. * Adjusted logging for skipped modifications to reduce output verbosity. * **Refactor** * Centralized version-based logic for patch application, reducing duplication and improving maintainability. --- .../unraid-file-modifier/file-modification.ts | 7 +++++++ .../modifications/auth-request.modification.ts | 13 ------------- .../default-page-layout.modification.ts | 17 ----------------- .../notifications-page.modification.ts | 13 ------------- .../modifications/rc-nginx.modification.ts | 10 ++-------- .../set-password-modal.modification.ts | 8 +++----- .../modifications/sso.modification.ts | 10 +++------- .../unraid-file-modifier.service.ts | 2 +- 8 files changed, 16 insertions(+), 64 deletions(-) diff --git a/api/src/unraid-api/unraid-file-modifier/file-modification.ts b/api/src/unraid-api/unraid-file-modifier/file-modification.ts index b1e0329d1..af16882e6 100644 --- a/api/src/unraid-api/unraid-file-modifier/file-modification.ts +++ b/api/src/unraid-api/unraid-file-modifier/file-modification.ts @@ -214,6 +214,13 @@ export abstract class FileModification { // Default implementation that can be overridden if needed async shouldApply(): Promise { try { + if (await this.isUnraidVersionGreaterThanOrEqualTo('7.2.0')) { + return { + shouldApply: false, + reason: 'Patch unnecessary for Unraid 7.2 or later because the Unraid API is integrated.', + }; + } + if (!this.filePath || !this.id) { throw new Error('Invalid file modification configuration'); } diff --git a/api/src/unraid-api/unraid-file-modifier/modifications/auth-request.modification.ts b/api/src/unraid-api/unraid-file-modifier/modifications/auth-request.modification.ts index a039dd2ed..a19ad750a 100644 --- a/api/src/unraid-api/unraid-file-modifier/modifications/auth-request.modification.ts +++ b/api/src/unraid-api/unraid-file-modifier/modifications/auth-request.modification.ts @@ -62,17 +62,4 @@ export default class AuthRequestModification extends FileModification { return this.createPatchWithDiff(overridePath ?? this.filePath, fileContent, newContent); } - - async shouldApply(): Promise { - if (await this.isUnraidVersionGreaterThanOrEqualTo('7.2.0')) { - return { - shouldApply: false, - reason: 'Skipping for Unraid 7.2 or later, where the Unraid API is integrated.', - }; - } - return { - shouldApply: true, - reason: 'Unraid version is less than 7.2.0, applying the patch.', - }; - } } diff --git a/api/src/unraid-api/unraid-file-modifier/modifications/default-page-layout.modification.ts b/api/src/unraid-api/unraid-file-modifier/modifications/default-page-layout.modification.ts index f4dba13cb..034fb79eb 100644 --- a/api/src/unraid-api/unraid-file-modifier/modifications/default-page-layout.modification.ts +++ b/api/src/unraid-api/unraid-file-modifier/modifications/default-page-layout.modification.ts @@ -99,21 +99,4 @@ if (is_localhost() && !is_good_session()) { return this.createPatchWithDiff(overridePath ?? this.filePath, fileContent, newContent); } - - async shouldApply(): Promise { - // Check if system is running Unraid 7.2 or later, where the Unraid API is integrated - const isUnraidVersionGreaterThanOrEqualTo72 = - await this.isUnraidVersionGreaterThanOrEqualTo('7.2.0'); - if (isUnraidVersionGreaterThanOrEqualTo72) { - return { - shouldApply: false, - reason: 'Skipping for Unraid 7.2 or later, where the Unraid API is integrated.', - }; - } - - return { - shouldApply: true, - reason: 'Always apply the allowed file changes to ensure compatibility.', - }; - } } diff --git a/api/src/unraid-api/unraid-file-modifier/modifications/notifications-page.modification.ts b/api/src/unraid-api/unraid-file-modifier/modifications/notifications-page.modification.ts index d792af1f8..98f16fa61 100644 --- a/api/src/unraid-api/unraid-file-modifier/modifications/notifications-page.modification.ts +++ b/api/src/unraid-api/unraid-file-modifier/modifications/notifications-page.modification.ts @@ -17,19 +17,6 @@ export default class NotificationsPageModification extends FileModification { return this.createPatchWithDiff(overridePath ?? this.filePath, fileContent, newContent); } - async shouldApply(): Promise { - if (await this.isUnraidVersionGreaterThanOrEqualTo('7.2.0')) { - return { - shouldApply: false, - reason: 'Skipping for Unraid 7.2 or later, where the Unraid API is integrated.', - }; - } - return { - shouldApply: true, - reason: 'Always apply the allowed file changes to ensure compatibility.', - }; - } - private static applyToSource(fileContent: string): string { return ( fileContent diff --git a/api/src/unraid-api/unraid-file-modifier/modifications/rc-nginx.modification.ts b/api/src/unraid-api/unraid-file-modifier/modifications/rc-nginx.modification.ts index a19b01737..6b1c717a3 100644 --- a/api/src/unraid-api/unraid-file-modifier/modifications/rc-nginx.modification.ts +++ b/api/src/unraid-api/unraid-file-modifier/modifications/rc-nginx.modification.ts @@ -91,16 +91,10 @@ check_remote_access(){ } async shouldApply(): Promise { - if (await this.isUnraidVersionGreaterThanOrEqualTo('7.2.0')) { - return { - shouldApply: false, - reason: 'Patch unnecessary for Unraid 7.2 or later because the Unraid API is integrated.', - }; - } const { shouldApply, reason } = await super.shouldApply(); return { - shouldApply: shouldApply, - reason: shouldApply ? 'Unraid version is less than 7.2.0, applying the patch.' : reason, + shouldApply, + reason, effects: ['nginx:reload'], }; } diff --git a/api/src/unraid-api/unraid-file-modifier/modifications/set-password-modal.modification.ts b/api/src/unraid-api/unraid-file-modifier/modifications/set-password-modal.modification.ts index 210025024..58b885400 100644 --- a/api/src/unraid-api/unraid-file-modifier/modifications/set-password-modal.modification.ts +++ b/api/src/unraid-api/unraid-file-modifier/modifications/set-password-modal.modification.ts @@ -19,11 +19,9 @@ export default class SetPasswordModalModification extends FileModification { } async shouldApply(): Promise { - if (await this.isUnraidVersionGreaterThanOrEqualTo('7.2.0')) { - return { - shouldApply: false, - reason: 'Skipping for Unraid 7.2 or later, where the Unraid API is integrated.', - }; + const superShouldApply = await super.shouldApply(); + if (!superShouldApply.shouldApply) { + return superShouldApply; } const fileContent = await readFile(this.filePath, 'utf-8'); const injectString = diff --git a/api/src/unraid-api/unraid-file-modifier/modifications/sso.modification.ts b/api/src/unraid-api/unraid-file-modifier/modifications/sso.modification.ts index ee24b1f2e..0146cca46 100644 --- a/api/src/unraid-api/unraid-file-modifier/modifications/sso.modification.ts +++ b/api/src/unraid-api/unraid-file-modifier/modifications/sso.modification.ts @@ -89,13 +89,9 @@ function verifyUsernamePasswordAndSSO(string $username, string $password): bool } async shouldApply(): Promise { - const isUnraidVersionGreaterThanOrEqualTo72 = - await this.isUnraidVersionGreaterThanOrEqualTo('7.2.0'); - if (isUnraidVersionGreaterThanOrEqualTo72) { - return { - shouldApply: false, - reason: 'Skipping for Unraid 7.2 or later, where the Unraid API is integrated.', - }; + const superShouldApply = await super.shouldApply(); + if (!superShouldApply.shouldApply) { + return superShouldApply; } if (!this.configService) { diff --git a/api/src/unraid-api/unraid-file-modifier/unraid-file-modifier.service.ts b/api/src/unraid-api/unraid-file-modifier/unraid-file-modifier.service.ts index f86ac5aa0..cca3cae83 100644 --- a/api/src/unraid-api/unraid-file-modifier/unraid-file-modifier.service.ts +++ b/api/src/unraid-api/unraid-file-modifier/unraid-file-modifier.service.ts @@ -124,7 +124,7 @@ export class UnraidFileModificationService shouldApplyWithReason.effects?.forEach((effect) => this.effects.add(effect)); this.logger.log(`Modification applied successfully: ${modification.id}`); } else { - this.logger.log( + this.logger.debug( `Skipping modification: ${modification.id} - ${shouldApplyWithReason.reason}` ); }