fix: paths now correct, better download logic

elibosley · elibosley · commit 16db2d908dcb · 2025-02-04T12:05:56.000-05:00
diff --git a/api/.gitignore b/api/.gitignore
@@ -80,6 +80,3 @@ deploy/*
 
 # IDE Settings Files
 .idea
-
-# Downloaded Fixtures (For File Modifications)
-src/unraid-api/unraid-file-modifier/modifications/__fixtures__/downloaded/*
diff --git a/api/src/unraid-api/unraid-file-modifier/file-modification.ts b/api/src/unraid-api/unraid-file-modifier/file-modification.ts
@@ -4,6 +4,7 @@ import { access, readFile, unlink, writeFile } from 'fs/promises';
 import { basename, dirname, join } from 'path';
 
 import { applyPatch, parsePatch, reversePatch } from 'diff';
+import { patch } from 'semver';
 
 export interface ShouldApplyWithReason {
     shouldApply: boolean;
@@ -18,7 +19,7 @@ export abstract class FileModification {
     public constructor(protected readonly logger: Logger) {}
 
     // This is the main method that child classes need to implement
-    protected abstract generatePatch(): Promise<string>;
+    protected abstract generatePatch(overridePath?: string): Promise<string>;
 
     private getPatchFilePath(targetFile: string): string {
         const dir = dirname(targetFile);
@@ -64,8 +65,14 @@ export abstract class FileModification {
     }
 
     private async applyPatch(patchContents: string): Promise<void> {
+        if (!patchContents.trim()) {
+            throw new Error('Patch contents are empty');
+        }
         const currentContent = await readFile(this.filePath, 'utf8');
         const parsedPatch = parsePatch(patchContents)[0];
+        if (!parsedPatch?.hunks.length) {
+            throw new Error('Invalid Patch Format: No hunks found');
+        }
         const results = applyPatch(currentContent, parsedPatch);
         if (results === false) {
             throw new Error(`Failed to apply patch to ${this.filePath}`);
@@ -75,22 +82,26 @@ export abstract class FileModification {
 
     // Default implementation of apply that uses the patch
     async apply(): Promise<void> {
-        // First attempt to apply the patch that was generated
-        const staticPatch = await this.getPregeneratedPatch();
-        if (staticPatch) {
-            try {
-                await this.applyPatch(staticPatch);
-                await this.savePatch(staticPatch);
-                return;
-            } catch (error) {
-                this.logger.error(
-                    `Failed to apply static patch to ${this.filePath}, continuing with dynamic patch`
-                );
+        try {
+            // First attempt to apply the patch that was generated
+            const staticPatch = await this.getPregeneratedPatch();
+            if (staticPatch) {
+                try {
+                    await this.applyPatch(staticPatch);
+                    await this.savePatch(staticPatch);
+                    return;
+                } catch (error) {
+                    this.logger.error(
+                        `Failed to apply static patch to ${this.filePath}, continuing with dynamic patch`
+                    );
+                }
             }
+            const patchContents = await this.generatePatch();
+            await this.applyPatch(patchContents);
+            await this.savePatch(patchContents);
+        } catch (err) {
+            this.logger.error(`Failed to apply patch to ${this.filePath}: ${err}`);
         }
-        const patchContents = await this.generatePatch();
-        await this.applyPatch(patchContents);
-        await this.savePatch(patchContents);
     }
 
     // Update rollback to use the shared utility
diff --git a/api/src/unraid-api/unraid-file-modifier/modifications/__fixtures__/downloaded/DefaultPageLayout.php b/api/src/unraid-api/unraid-file-modifier/modifications/__fixtures__/downloaded/DefaultPageLayout.php
diff --git a/api/src/unraid-api/unraid-file-modifier/modifications/__fixtures__/downloaded/DefaultPageLayout.php.last-download-time b/api/src/unraid-api/unraid-file-modifier/modifications/__fixtures__/downloaded/DefaultPageLayout.php.last-download-time
@@ -0,0 +1 @@
+1738614986599
diff --git a/api/src/unraid-api/unraid-file-modifier/modifications/__fixtures__/downloaded/Notifications.page b/api/src/unraid-api/unraid-file-modifier/modifications/__fixtures__/downloaded/Notifications.page
diff --git a/api/src/unraid-api/unraid-file-modifier/modifications/__fixtures__/downloaded/Notifications.page.last-download-time b/api/src/unraid-api/unraid-file-modifier/modifications/__fixtures__/downloaded/Notifications.page.last-download-time
@@ -0,0 +1 @@
+1738614986764
diff --git a/api/src/unraid-api/unraid-file-modifier/modifications/__fixtures__/downloaded/auth-request.php b/api/src/unraid-api/unraid-file-modifier/modifications/__fixtures__/downloaded/auth-request.php
diff --git a/api/src/unraid-api/unraid-file-modifier/modifications/__fixtures__/downloaded/auth-request.php.last-download-time b/api/src/unraid-api/unraid-file-modifier/modifications/__fixtures__/downloaded/auth-request.php.last-download-time
@@ -0,0 +1 @@
+1738614987214
diff --git a/api/src/unraid-api/unraid-file-modifier/modifications/__fixtures__/downloaded/logrotate.conf b/api/src/unraid-api/unraid-file-modifier/modifications/__fixtures__/downloaded/logrotate.conf
diff --git a/api/src/unraid-api/unraid-file-modifier/modifications/__fixtures__/downloaded/logrotate.conf.last-download-time b/api/src/unraid-api/unraid-file-modifier/modifications/__fixtures__/downloaded/logrotate.conf.last-download-time
@@ -0,0 +1 @@
+999999999999999999999999999999999999
diff --git a/api/src/unraid-api/unraid-file-modifier/modifications/__test__/generic-modification.spec.ts b/api/src/unraid-api/unraid-file-modifier/modifications/__test__/generic-modification.spec.ts
@@ -1,16 +1,24 @@
 import { Logger } from '@nestjs/common';
 import { existsSync } from 'fs';
 import { cp, readFile, writeFile } from 'fs/promises';
-import { basename, resolve } from 'path';
+import path, { basename, resolve } from 'path';
+
+
 
 import { describe, expect, test } from 'vitest';
 
+
+
 import { FileModification } from '@app/unraid-api/unraid-file-modifier/file-modification';
 import AuthRequestModification from '@app/unraid-api/unraid-file-modifier/modifications/auth-request.modification';
 import DefaultPageLayoutModification from '@app/unraid-api/unraid-file-modifier/modifications/default-page-layout.modification';
+import { LogRotateModification } from '@app/unraid-api/unraid-file-modifier/modifications/log-rotate.modification';
 import NotificationsPageModification from '@app/unraid-api/unraid-file-modifier/modifications/notifications-page.modification';
 import SSOFileModification from '@app/unraid-api/unraid-file-modifier/modifications/sso.modification';
-import { LogRotateModification } from '@app/unraid-api/unraid-file-modifier/modifications/log-rotate.modification';
+
+
+
+
 
 interface ModificationTestCase {
     ModificationClass: new (...args: ConstructorParameters<typeof FileModification>) => FileModification;
@@ -49,54 +57,63 @@ const testCases: ModificationTestCase[] = [
     },
 ];
 
-async function testModification(testCase: ModificationTestCase) {
-    // First download the file from Github
-    const fileName = basename(testCase.fileUrl);
-
-    const path = resolve(__dirname, `../__fixtures__/downloaded/${fileName}`);
-    const pathLocal = resolve(__dirname, `../__fixtures__/local/${fileName}`);
+const downloadOrRetrieveOriginalFile = async (filePath: string, fileUrl: string): Promise<string> => {
     let originalContent = '';
-    if (!existsSync(path)) {
+    // Check last download time, if > than 1 week and not in CI, download the file from Github
+    const lastDownloadTime = await readFile(`${filePath}.last-download-time`, 'utf-8')
+        .catch(() => 0)
+        .then(Number);
+    const shouldDownload = lastDownloadTime < Date.now() - 1000 * 60 * 60 * 24 * 7 && !process.env.CI;
+    if (shouldDownload) {
         try {
-            console.log('Downloading file', testCase.fileUrl);
-            originalContent = await fetch(testCase.fileUrl).then((response) => response.text());
-            await writeFile(path, originalContent);
-            await writeFile(pathLocal, originalContent);
+            console.log('Downloading file', fileUrl);
+            originalContent = await fetch(fileUrl).then((response) => response.text());
+            if (!originalContent) {
+                throw new Error('Failed to download file');
+            }
+            await writeFile(filePath, originalContent);
+            await writeFile(`${filePath}.last-download-time`, Date.now().toString());
+            return originalContent;
         } catch (error) {
-            console.error('Failed to download file - using local fixture', error);
-            await cp(resolve(__dirname, `../__fixtures__/local/${fileName}`), path);
-            originalContent = await readFile(path, 'utf-8');
+            console.error('Error downloading file', error);
+            console.error(
+                `Failed to download file - using version created at ${new Date(lastDownloadTime).toISOString()}`
+            );
         }
-    } else {
-        console.log('Using existing fixture file', path);
-        originalContent = await readFile(path, 'utf-8');
     }
+    return await readFile(filePath, 'utf-8');
+};
 
+async function testModification(testCase: ModificationTestCase) {
+    const fileName = basename(testCase.fileUrl);
+    const filePath = resolve(__dirname, `../__fixtures__/downloaded/${fileName}`);
+    const originalContent = await downloadOrRetrieveOriginalFile(filePath, testCase.fileUrl);
     const logger = new Logger();
     const patcher = await new testCase.ModificationClass(logger);
+    const originalPath = patcher.filePath;
     // @ts-expect-error - Ignore for testing purposes
-    patcher.filePath = path;
+    patcher.filePath = filePath;
 
     // @ts-expect-error - Ignore for testing purposes
-    const patch = await patcher.generatePatch();
+    const patch = await patcher.generatePatch(originalPath);
 
     // Test patch matches snapshot
     await expect(patch).toMatchFileSnapshot(`../patches/${patcher.id}.patch`);
 
     // Apply patch and verify modified file
     await patcher.apply();
-    await expect(await readFile(path, 'utf-8')).toMatchFileSnapshot(
+    await expect(await readFile(filePath, 'utf-8')).toMatchFileSnapshot(
         `snapshots/${fileName}.modified.snapshot.php`
     );
 
     // Rollback and verify original state
     await patcher.rollback();
-    const revertedContent = await readFile(path, 'utf-8');
+    const revertedContent = await readFile(filePath, 'utf-8');
     await expect(revertedContent).toMatch(originalContent);
 }
 
 describe('File modifications', () => {
     test.each(testCases)(`$fileName modifier correctly applies to fresh install`, async (testCase) => {
         await testModification(testCase);
     });
-});
+});
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
@@ -10,7 +10,7 @@ import {
 } from '@app/unraid-api/unraid-file-modifier/file-modification';
 
 export default class AuthRequestModification extends FileModification {
-    public filePath: string = '/usr/local/emhttp/auth-request.php';
+    public filePath: string = '/usr/local/emhttp/auth-request.php' as const;
     public webComponentsDirectory: string =
         '/usr/local/emhttp/plugins/dynamix.my.servers/unraid-components/_nuxt/' as const;
     id: string = 'auth-request';
@@ -21,7 +21,7 @@ export default class AuthRequestModification extends FileModification {
         const baseDir = '/usr/local/emhttp'; // TODO: Make this configurable
         return files.map((file) => (file.startsWith(baseDir) ? file.slice(baseDir.length) : file));
     };
-    protected async generatePatch(): Promise<string> {
+    protected async generatePatch(overridePath?: string): Promise<string> {
         const jsFiles = await this.getJsFiles(this.webComponentsDirectory);
         this.logger.debug(`Found ${jsFiles.length} .js files in ${this.webComponentsDirectory}`);
 
@@ -43,7 +43,7 @@ export default class AuthRequestModification extends FileModification {
         const newContent = fileContent.replace(/(\$arrWhitelist\s*=\s*\[)/, `$1\n${filesToAddString}`);
 
         // Generate and return patch
-        const patch = createPatch(this.filePath, fileContent, newContent, undefined, undefined, {
+        const patch = createPatch(overridePath ?? this.filePath, fileContent, newContent, undefined, undefined, {
             context: 3,
         });
 
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
@@ -41,12 +41,12 @@ export default class DefaultPageLayoutModification extends FileModification {
         return transformers.reduce((content, fn) => fn(content), fileContent);
     }
 
-    protected async generatePatch(): Promise<string> {
+    protected async generatePatch(overridePath?: string): Promise<string> {
         const fileContent = await readFile(this.filePath, 'utf-8');
 
         const newContent = this.applyToSource(fileContent);
 
-        const patch = createPatch(this.filePath, fileContent, newContent, undefined, undefined, {
+        const patch = createPatch(overridePath ?? this.filePath, fileContent, newContent, undefined, undefined, {
             context: 2,
         });
 
diff --git a/api/src/unraid-api/unraid-file-modifier/modifications/log-rotate.modification.ts b/api/src/unraid-api/unraid-file-modifier/modifications/log-rotate.modification.ts
@@ -30,13 +30,13 @@ export class LogRotateModification extends FileModification {
         super(logger);
     }
 
-    protected async generatePatch(): Promise<string> {
+    protected async generatePatch(overridePath?: string): Promise<string> {
         const currentContent = (await fileExists(this.filePath))
             ? await readFile(this.filePath, 'utf8')
             : '';
 
         const patch = createPatch(
-            this.filePath,
+            overridePath ?? this.filePath,
             currentContent,
             this.logRotateConfig,
             undefined,
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
@@ -12,12 +12,12 @@ export default class NotificationsPageModification extends FileModification {
     id: string = 'notifications-page';
     public readonly filePath: string = '/usr/local/emhttp/plugins/dynamix/Notifications.page';
 
-    protected async generatePatch(): Promise<string> {
+    protected async generatePatch(overridePath?: string): Promise<string> {
         const fileContent = await readFile(this.filePath, 'utf-8');
 
         const newContent = NotificationsPageModification.applyToSource(fileContent);
 
-        const patch = createPatch(this.filePath, fileContent, newContent, undefined, undefined, {
+        const patch = createPatch(overridePath ?? this.filePath, fileContent, newContent, undefined, undefined, {
             context: 3,
         });
 
diff --git a/api/src/unraid-api/unraid-file-modifier/modifications/patches/auth-request.patch b/api/src/unraid-api/unraid-file-modifier/modifications/patches/auth-request.patch
@@ -1,7 +1,7 @@
-Index: /app/src/unraid-api/unraid-file-modifier/modifications/__fixtures__/downloaded/auth-request.php
+Index: /usr/local/emhttp/auth-request.php
 ===================================================================
---- /app/src/unraid-api/unraid-file-modifier/modifications/__fixtures__/downloaded/auth-request.php
-+++ /app/src/unraid-api/unraid-file-modifier/modifications/__fixtures__/downloaded/auth-request.php
+--- /usr/local/emhttp/auth-request.php
++++ /usr/local/emhttp/auth-request.php
 @@ -15,6 +15,7 @@
  }
  
diff --git a/api/src/unraid-api/unraid-file-modifier/modifications/patches/default-page-layout.patch b/api/src/unraid-api/unraid-file-modifier/modifications/patches/default-page-layout.patch
@@ -1,7 +1,7 @@
-Index: /app/src/unraid-api/unraid-file-modifier/modifications/__fixtures__/downloaded/DefaultPageLayout.php
+Index: /usr/local/emhttp/plugins/dynamix/include/DefaultPageLayout.php
 ===================================================================
---- /app/src/unraid-api/unraid-file-modifier/modifications/__fixtures__/downloaded/DefaultPageLayout.php
-+++ /app/src/unraid-api/unraid-file-modifier/modifications/__fixtures__/downloaded/DefaultPageLayout.php
+--- /usr/local/emhttp/plugins/dynamix/include/DefaultPageLayout.php
++++ /usr/local/emhttp/plugins/dynamix/include/DefaultPageLayout.php
 @@ -557,14 +557,5 @@
    $.post('/webGui/include/Notify.php',{cmd:'get',csrf_token:csrf_token},function(msg) {
      $.each($.parseJSON(msg), function(i, notify){
diff --git a/api/src/unraid-api/unraid-file-modifier/modifications/patches/log-rotate.patch b/api/src/unraid-api/unraid-file-modifier/modifications/patches/log-rotate.patch
@@ -1,7 +1,7 @@
-Index: /app/src/unraid-api/unraid-file-modifier/modifications/__fixtures__/downloaded/logrotate.conf
+Index: /etc/logrotate.d/unraid-api
 ===================================================================
---- /app/src/unraid-api/unraid-file-modifier/modifications/__fixtures__/downloaded/logrotate.conf
-+++ /app/src/unraid-api/unraid-file-modifier/modifications/__fixtures__/downloaded/logrotate.conf
+--- /etc/logrotate.d/unraid-api
++++ /etc/logrotate.d/unraid-api
 @@ -0,0 +1,12 @@
 +
 +/var/log/unraid-api/*.log {
diff --git a/api/src/unraid-api/unraid-file-modifier/modifications/patches/notifications-page.patch b/api/src/unraid-api/unraid-file-modifier/modifications/patches/notifications-page.patch
@@ -1,7 +1,7 @@
-Index: /app/src/unraid-api/unraid-file-modifier/modifications/__fixtures__/downloaded/Notifications.page
+Index: /usr/local/emhttp/plugins/dynamix/Notifications.page
 ===================================================================
---- /app/src/unraid-api/unraid-file-modifier/modifications/__fixtures__/downloaded/Notifications.page
-+++ /app/src/unraid-api/unraid-file-modifier/modifications/__fixtures__/downloaded/Notifications.page
+--- /usr/local/emhttp/plugins/dynamix/Notifications.page
++++ /usr/local/emhttp/plugins/dynamix/Notifications.page
 @@ -135,23 +135,7 @@
  
  :notifications_auto_close_help:
diff --git a/api/src/unraid-api/unraid-file-modifier/modifications/patches/sso.patch b/api/src/unraid-api/unraid-file-modifier/modifications/patches/sso.patch
@@ -1,7 +1,7 @@
-Index: /app/src/unraid-api/unraid-file-modifier/modifications/__fixtures__/downloaded/.login.php
+Index: /usr/local/emhttp/plugins/dynamix/include/.login.php
 ===================================================================
---- /app/src/unraid-api/unraid-file-modifier/modifications/__fixtures__/downloaded/.login.php	original
-+++ /app/src/unraid-api/unraid-file-modifier/modifications/__fixtures__/downloaded/.login.php	modified
+--- /usr/local/emhttp/plugins/dynamix/include/.login.php	original
++++ /usr/local/emhttp/plugins/dynamix/include/.login.php	modified
 @@ -1,5 +1,33 @@
  <?php
 +
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
@@ -11,7 +11,7 @@ export default class SSOFileModification extends FileModification {
     id: string = 'sso';
     public readonly filePath: string = '/usr/local/emhttp/plugins/dynamix/include/.login.php';
 
-    protected async generatePatch(): Promise<string> {
+    protected async generatePatch(overridePath?: string): Promise<string> {
         // Define the new PHP function to insert
         /* eslint-disable no-useless-escape */
         const newFunction = `
@@ -64,7 +64,7 @@ function verifyUsernamePasswordAndSSO(string $username, string $password): bool
         newContent = newContent.replace(/<\/form>/i, `</form>\n${tagToInject}`);
 
         // Create and return the patch
-        const patch = createPatch(this.filePath, originalContent, newContent, 'original', 'modified');
+        const patch = createPatch(overridePath ?? this.filePath, originalContent, newContent, 'original', 'modified');
         return patch;
     }
 
diff --git a/api/src/unraid-api/unraid-file-modifier/unraid-file-modifier.spec.ts b/api/src/unraid-api/unraid-file-modifier/unraid-file-modifier.spec.ts
@@ -19,8 +19,8 @@ class TestFileModification extends FileModification {
     id = 'test';
     public readonly filePath: string = FIXTURE_PATH;
 
-    protected async generatePatch(): Promise<string> {
-        return createPatch('text-patch-file.txt', ORIGINAL_CONTENT, 'modified');
+    protected async generatePatch(overridePath?: string): Promise<string> {
+        return createPatch(overridePath ?? 'text-patch-file.txt', ORIGINAL_CONTENT, 'modified');
     }
 
     async shouldApply(): Promise<ShouldApplyWithReason> {
