Code review changes

src/APL/fileAPL.test.ts

@@ -1,7 +1,7 @@
 import { promises as fsPromises } from "fs";
 import { afterEach, describe, expect, it, vi } from "vitest";
 
-import { FileAPL, loadDataFromFile, saveDataToFile } from "./fileAPL";
+import { FileAPL } from "./fileAPL";
 
 const stubAuthData = {
   domain: "example.com",
@@ -9,10 +9,20 @@ const stubAuthData = {
 };
 
 describe("APL", () => {
+  afterEach(() => {
+    vi.clearAllMocks();
+  });
+
   describe("FileAPL", () => {
     describe("get", () => {
-      afterEach(() => {
+      it("Should throw error when JSON parse fails", async () => {
-        vi.clearAllMocks();
+        vi.spyOn(fsPromises, "access").mockResolvedValue();
+        vi.spyOn(fsPromises, "readFile").mockResolvedValue("Not a valid JSON");
+
+        const apl = new FileAPL();
+        await expect(apl.get(stubAuthData.domain)).rejects.toThrow(
+          "File APL could not read auth data from the .saleor-app-auth.json file"
+        );
       });
 
       it("Returns auth data for existing domain", async () => {
@@ -34,11 +44,32 @@ describe("APL", () => {
       });
     });
 
-    describe("delete", () => {
+    describe("set", () => {
-      afterEach(() => {
+      it("Handle write file errors", async () => {
-        vi.clearAllMocks();
+        const spyWriteFile = vi.spyOn(fsPromises, "writeFile").mockImplementation(() => {
-      });
+          throw Error("Write error");
+        });
 
+        const apl = new FileAPL();
+
+        await expect(apl.set(stubAuthData)).rejects.toThrow(
+          "File APL was unable to save auth data"
+        );
+        expect(spyWriteFile).toBeCalledWith(".saleor-app-auth.json", JSON.stringify(stubAuthData));
+      });
+    });
+
+    it("Successfully save to file", async () => {
+      const spyWriteFile = vi.spyOn(fsPromises, "writeFile").mockResolvedValue();
+
+      const apl = new FileAPL();
+
+      await expect(apl.set(stubAuthData));
+
+      expect(spyWriteFile).toBeCalledWith(".saleor-app-auth.json", JSON.stringify(stubAuthData));
+    });
+
+    describe("delete", () => {
       it("Should override file when called with known domain", async () => {
         vi.spyOn(fsPromises, "access").mockResolvedValue();
         vi.spyOn(fsPromises, "readFile").mockResolvedValue(JSON.stringify(stubAuthData));
@@ -66,10 +97,6 @@ describe("APL", () => {
     });
 
     describe("getAll", () => {
-      afterEach(() => {
-        vi.clearAllMocks();
-      });
-
       it("Should return list with one item when auth data are existing", async () => {
         vi.spyOn(fsPromises, "access").mockResolvedValue();
         vi.spyOn(fsPromises, "readFile").mockResolvedValue(JSON.stringify(stubAuthData));
@@ -89,69 +116,4 @@ describe("APL", () => {
       });
     });
   });
-
-  describe("FileAPL utils", () => {
-    describe("saveDataToFile", () => {
-      afterEach(() => {
-        vi.clearAllMocks();
-      });
-
-      it("Save existing auth data to file", async () => {
-        const spyWriteFile = vi.spyOn(fsPromises, "writeFile").mockResolvedValue();
-        await saveDataToFile("test.json", stubAuthData);
-
-        expect(spyWriteFile).toBeCalledWith("test.json", JSON.stringify(stubAuthData));
-      });
-
-      it("Save empty file when no auth data provided", async () => {
-        const spyWriteFile = vi.spyOn(fsPromises, "writeFile").mockResolvedValue();
-        await saveDataToFile("test.json");
-        expect(spyWriteFile).toBeCalledWith("test.json", "{}");
-      });
-
-      it("Handle write file errors", async () => {
-        const spyWriteFile = vi.spyOn(fsPromises, "writeFile").mockImplementation(() => {
-          throw Error("Write error");
-        });
-        await saveDataToFile("test.json");
-        expect(spyWriteFile).toBeCalledWith("test.json", "{}");
-      });
-    });
-
-    describe("loadDataFromFile", () => {
-      afterEach(() => {
-        vi.clearAllMocks();
-      });
-
-      it("Load existing auth data", async () => {
-        vi.spyOn(fsPromises, "access").mockResolvedValue();
-        vi.spyOn(fsPromises, "readFile").mockResolvedValue(JSON.stringify(stubAuthData));
-        expect(await loadDataFromFile("test.json")).toStrictEqual(stubAuthData);
-      });
-
-      it("Should return undefined when JSON parse fails", async () => {
-        vi.spyOn(fsPromises, "access").mockResolvedValue();
-        vi.spyOn(fsPromises, "readFile").mockResolvedValue("Not a valid JSON");
-        expect(await loadDataFromFile("test.json")).toBe(undefined);
-      });
-
-      it("Should return undefined when auth token is missing", async () => {
-        vi.spyOn(fsPromises, "access").mockResolvedValue();
-        const stubInvalidAuthData = {
-          domain: "example.com",
-        };
-        vi.spyOn(fsPromises, "readFile").mockResolvedValue(JSON.stringify(stubInvalidAuthData));
-        expect(await loadDataFromFile("test.json")).toBe(undefined);
-      });
-
-      it("Should return undefined when domain is missing", async () => {
-        vi.spyOn(fsPromises, "access").mockResolvedValue();
-        const stubInvalidAuthData = {
-          token: "token",
-        };
-        vi.spyOn(fsPromises, "readFile").mockResolvedValue(JSON.stringify(stubInvalidAuthData));
-        expect(await loadDataFromFile("test.json")).toBe(undefined);
-      });
-    });
-  });
 });

src/APL/fileAPL.ts

@@ -5,46 +5,6 @@ import { APL, AuthData } from "./apl";
 
 const debug = debugPkg.debug("app-sdk:FileAPL");
 
-/**
- * Load auth data from a file and return it as AuthData format.
- * In case of incomplete or invalid data, return `undefined`.
- *
- * @param {string} fileName
- */
-export const loadDataFromFile = async (fileName: string): Promise<AuthData | undefined> => {
-  debug(`Load auth data from the ${fileName} file`);
-  let parsedData: Record<string, string> = {};
-  try {
-    await fsPromises.access(fileName);
-    parsedData = JSON.parse(await fsPromises.readFile(fileName, "utf-8"));
-  } catch (err) {
-    debug(`Could not read auth data from the ${fileName} file`, err);
-    return undefined;
-  }
-  const { token, domain } = parsedData;
-  if (token && domain) {
-    return { token, domain };
-  }
-  return undefined;
-};
-
-/**
- * Save auth data to file.
- * When `authData` argument is empty, will overwrite file with empty values.
- *
- * @param {string} fileName
- * @param {AuthData} [authData]
- */
-export const saveDataToFile = async (fileName: string, authData?: AuthData) => {
-  debug(`Save auth data to the ${fileName} file`);
-  const newData = authData ? JSON.stringify(authData) : "{}";
-  try {
-    await fsPromises.writeFile(fileName, newData);
-  } catch (err) {
-    debug(`Could not save auth data to the ${fileName} file`, err);
-  }
-};
-
 export type FileAPLConfig = {
   fileName?: string;
 };
@@ -68,8 +28,49 @@ export class FileAPL implements APL {
     this.fileName = config?.fileName || ".saleor-app-auth.json";
   }
 
+  /**
+   * Load auth data from a file and return it as AuthData format.
+   * In case of incomplete or invalid data, return `undefined`.
+   *
+   * @param {string} fileName
+   */
+  private async loadDataFromFile(): Promise<AuthData | undefined> {
+    debug(`Load auth data from the ${this.fileName} file`);
+    let parsedData: Record<string, string> = {};
+    try {
+      await fsPromises.access(this.fileName);
+      parsedData = JSON.parse(await fsPromises.readFile(this.fileName, "utf-8"));
+    } catch (err) {
+      debug(`Could not read auth data from the ${this.fileName} file`, err);
+      throw new Error(`File APL could not read auth data from the ${this.fileName} file`);
+    }
+    const { token, domain } = parsedData;
+    if (token && domain) {
+      return { token, domain };
+    }
+    return undefined;
+  }
+
+  /**
+   * Save auth data to file.
+   * When `authData` argument is empty, will overwrite file with empty values.
+   *
+   * @param {string} fileName
+   * @param {AuthData} [authData]
+   */
+  private async saveDataToFile(authData?: AuthData) {
+    debug(`Save auth data to the ${this.fileName} file`);
+    const newData = authData ? JSON.stringify(authData) : "{}";
+    try {
+      await fsPromises.writeFile(this.fileName, newData);
+    } catch (err) {
+      debug(`Could not save auth data to the ${this.fileName} file`, err);
+      throw new Error("File APL was unable to save auth data");
+    }
+  }
+
   async get(domain: string) {
-    const authData = await loadDataFromFile(this.fileName);
+    const authData = await this.loadDataFromFile();
     if (domain === authData?.domain) {
       return authData;
     }
@@ -77,19 +78,19 @@ export class FileAPL implements APL {
   }
 
   async set(authData: AuthData) {
-    await saveDataToFile(this.fileName, authData);
+    await this.saveDataToFile(authData);
   }
 
   async delete(domain: string) {
-    const authData = await loadDataFromFile(this.fileName);
+    const authData = await this.loadDataFromFile();
 
     if (domain === authData?.domain) {
-      await saveDataToFile(this.fileName);
+      await this.saveDataToFile();
     }
   }
 
   async getAll() {
-    const authData = await loadDataFromFile(this.fileName);
+    const authData = await this.loadDataFromFile();
 
     if (!authData) {
       return [];

src/APL/vercelAPL.test.ts

@@ -1,12 +1,6 @@
 import { afterEach, describe, expect, it } from "vitest";
 
-import {
+import { VercelAPL, VercelAPLVariables } from "./vercelAPL";
-  DOMAIN_VARIABLE_NAME,
-  SALEOR_DEPLOYMENT_TOKEN,
-  SALEOR_REGISTER_APP_URL,
-  TOKEN_VARIABLE_NAME,
-  VercelAPL,
-} from "./vercelAPL";
 
 const aplConfig = {
   deploymentToken: "token",
@@ -19,58 +13,63 @@ const stubAuthData = {
 };
 
 describe("APL", () => {
+  const initialEnv = { ...process.env };
+
+  afterEach(() => {
+    process.env = { ...initialEnv };
+  });
+
   describe("VercelAPL", () => {
     describe("constructor", () => {
-      const initialEnv = { ...process.env };
-
-      afterEach(() => {
-        process.env = { ...initialEnv };
-      });
-
       it("Raise an error when configuration is missing", async () => {
-        delete process.env[SALEOR_REGISTER_APP_URL];
+        delete process.env[VercelAPLVariables.SALEOR_REGISTER_APP_URL];
-        process.env[SALEOR_DEPLOYMENT_TOKEN] = "token";
+        process.env[VercelAPLVariables.SALEOR_DEPLOYMENT_TOKEN] = "token";
 
         expect(() => new VercelAPL()).toThrow();
 
-        process.env[SALEOR_REGISTER_APP_URL] = "http://example.com";
+        process.env[VercelAPLVariables.SALEOR_REGISTER_APP_URL] = "http://example.com";
-        delete process.env[SALEOR_DEPLOYMENT_TOKEN];
+        delete process.env[VercelAPLVariables.SALEOR_DEPLOYMENT_TOKEN];
 
         expect(() => new VercelAPL()).toThrow();
       });
     });
 
-    it("Constructor with config values", async () => {
+    it("Constructs VercelAPL instance when deploymentToken and registerAppURL provided", async () => {
       expect(() => new VercelAPL(aplConfig)).not.toThrow();
     });
 
-    it("Constructor with config values from environment variables", async () => {
+    it("Constructs VercelAPL instance with config values from environment variables", async () => {
-      process.env[SALEOR_REGISTER_APP_URL] = aplConfig.registerAppURL;
+      process.env[VercelAPLVariables.SALEOR_REGISTER_APP_URL] = aplConfig.registerAppURL;
-      process.env[SALEOR_DEPLOYMENT_TOKEN] = aplConfig.deploymentToken;
+      process.env[VercelAPLVariables.SALEOR_DEPLOYMENT_TOKEN] = aplConfig.deploymentToken;
 
       expect(() => new VercelAPL()).not.toThrow();
     });
 
+    it("Test if constructor use options over environment variables", async () => {
+      process.env[VercelAPLVariables.SALEOR_REGISTER_APP_URL] = "environment";
+      process.env[VercelAPLVariables.SALEOR_DEPLOYMENT_TOKEN] = "environment";
+
+      const apl = await new VercelAPL({ deploymentToken: "option", registerAppURL: "option" });
+      // eslint-disable-next-line dot-notation
+      expect(apl["deploymentToken"]).toBe("option");
+      // eslint-disable-next-line dot-notation
+      expect(apl["registerAppURL"]).toBe("option");
+    });
+
     describe("get", () => {
       describe("Read existing auth data from env", () => {
-        const initialEnv = { ...process.env };
-
-        afterEach(() => {
-          process.env = { ...initialEnv };
-        });
-
         it("Read existing auth data", async () => {
-          process.env[TOKEN_VARIABLE_NAME] = stubAuthData.token;
+          process.env[VercelAPLVariables.TOKEN_VARIABLE_NAME] = stubAuthData.token;
-          process.env[DOMAIN_VARIABLE_NAME] = stubAuthData.domain;
+          process.env[VercelAPLVariables.DOMAIN_VARIABLE_NAME] = stubAuthData.domain;
 
-          const apl = new VercelAPL();
+          const apl = new VercelAPL(aplConfig);
 
           expect(await apl.get(stubAuthData.domain)).toStrictEqual(stubAuthData);
         });
 
         it("Return undefined when unknown domain requested", async () => {
-          process.env[TOKEN_VARIABLE_NAME] = stubAuthData.token;
+          process.env[VercelAPLVariables.TOKEN_VARIABLE_NAME] = stubAuthData.token;
-          process.env[DOMAIN_VARIABLE_NAME] = stubAuthData.domain;
+          process.env[VercelAPLVariables.DOMAIN_VARIABLE_NAME] = stubAuthData.domain;
 
           const apl = new VercelAPL(aplConfig);
 
@@ -78,8 +77,8 @@ describe("APL", () => {
         });
 
         it("Return undefined when no data is defined", async () => {
-          delete process.env[TOKEN_VARIABLE_NAME];
+          delete process.env[VercelAPLVariables.TOKEN_VARIABLE_NAME];
-          delete process.env[DOMAIN_VARIABLE_NAME];
+          delete process.env[VercelAPLVariables.DOMAIN_VARIABLE_NAME];
 
           const apl = new VercelAPL(aplConfig);
 
@@ -90,15 +89,9 @@ describe("APL", () => {
 
     describe("getAll", () => {
       describe("Read existing auth data from env", () => {
-        const initialEnv = { ...process.env };
-
-        afterEach(() => {
-          process.env = { ...initialEnv };
-        });
-
         it("Read existing auth data", async () => {
-          process.env[TOKEN_VARIABLE_NAME] = stubAuthData.token;
+          process.env[VercelAPLVariables.TOKEN_VARIABLE_NAME] = stubAuthData.token;
-          process.env[DOMAIN_VARIABLE_NAME] = stubAuthData.domain;
+          process.env[VercelAPLVariables.DOMAIN_VARIABLE_NAME] = stubAuthData.domain;
 
           const apl = new VercelAPL(aplConfig);
 
@@ -106,8 +99,8 @@ describe("APL", () => {
         });
 
         it("Return empty list when no auth data are existing", async () => {
-          delete process.env[TOKEN_VARIABLE_NAME];
+          delete process.env[VercelAPLVariables.TOKEN_VARIABLE_NAME];
-          delete process.env[DOMAIN_VARIABLE_NAME];
+          delete process.env[VercelAPLVariables.DOMAIN_VARIABLE_NAME];
 
           const apl = new VercelAPL(aplConfig);
 

src/APL/vercelAPL.ts

@@ -6,14 +6,16 @@ import { APL, AuthData } from "./apl";
 
 const debug = debugPkg.debug("app-sdk:VercelAPL");
 
-export const TOKEN_VARIABLE_NAME = "SALEOR_AUTH_TOKEN";
+export const VercelAPLVariables = {
-export const DOMAIN_VARIABLE_NAME = "SALEOR_DOMAIN";
+  TOKEN_VARIABLE_NAME: "SALEOR_AUTH_TOKEN",
-export const SALEOR_REGISTER_APP_URL = "SALEOR_REGISTER_APP_URL";
+  DOMAIN_VARIABLE_NAME: "SALEOR_DOMAIN",
-export const SALEOR_DEPLOYMENT_TOKEN = "SALEOR_DEPLOYMENT_TOKEN";
+  SALEOR_REGISTER_APP_URL: "SALEOR_REGISTER_APP_URL",
+  SALEOR_DEPLOYMENT_TOKEN: "SALEOR_DEPLOYMENT_TOKEN",
+};
 
 const getEnvAuth = (): AuthData | undefined => {
-  const token = process.env[TOKEN_VARIABLE_NAME];
+  const token = process.env[VercelAPLVariables.TOKEN_VARIABLE_NAME];
-  const domain = process.env[DOMAIN_VARIABLE_NAME];
+  const domain = process.env[VercelAPLVariables.DOMAIN_VARIABLE_NAME];
   if (!token || !domain) {
     return undefined;
   }
@@ -23,30 +25,6 @@ const getEnvAuth = (): AuthData | undefined => {
   };
 };
 
-const saveDataToVercel = async (
-  registerAppURL: string,
-  deploymentToken: string,
-  authData?: AuthData
-) => {
-  debug("Saving data to Vercel");
-
-  try {
-    await fetch(registerAppURL, {
-      method: "POST",
-      headers: { "Content-Type": "application/json" },
-      body: JSON.stringify({
-        token: deploymentToken,
-        envs: {
-          [TOKEN_VARIABLE_NAME]: authData?.token || "",
-          [DOMAIN_VARIABLE_NAME]: authData?.domain || "",
-        },
-      }),
-    });
-  } catch (error) {
-    debug("Error during saving the data:", error);
-  }
-};
-
 export type VercelAPLConfig = {
   registerAppURL?: string;
   deploymentToken?: string;
@@ -71,14 +49,14 @@ export class VercelAPL implements APL {
   private deploymentToken: string;
 
   constructor(config?: VercelAPLConfig) {
-    const registerAppURL = config?.registerAppURL || process.env.SALEOR_REGISTER_APP_URL;
+    const registerAppURL =
+      config?.registerAppURL || process.env[VercelAPLVariables.SALEOR_REGISTER_APP_URL];
     if (!registerAppURL) {
-      debug("Missing registerAppURL");
       throw new Error("Misconfiguration: please provide registerAppUrl");
     }
-    const deploymentToken = config?.deploymentToken || process.env.SALEOR_DEPLOYMENT_TOKEN;
+    const deploymentToken =
+      config?.deploymentToken || process.env[VercelAPLVariables.SALEOR_DEPLOYMENT_TOKEN];
     if (!deploymentToken) {
-      debug("Missing deploymentToken");
       throw new Error("Misconfiguration: please provide deploymentToken");
     }
 
@@ -86,6 +64,26 @@ export class VercelAPL implements APL {
     this.deploymentToken = deploymentToken;
   }
 
+  private async saveDataToVercel(authData?: AuthData) {
+    debug(`saveDataToVercel with: ${authData}`);
+    try {
+      await fetch(this.registerAppURL, {
+        method: "POST",
+        headers: { "Content-Type": "application/json" },
+        body: JSON.stringify({
+          token: this.deploymentToken,
+          envs: {
+            [VercelAPLVariables.TOKEN_VARIABLE_NAME]: authData?.token || "",
+            [VercelAPLVariables.DOMAIN_VARIABLE_NAME]: authData?.domain || "",
+          },
+        }),
+      });
+    } catch (error) {
+      debug("Error during saving the data:", error);
+      throw new Error(`VercelAPL was not able to save auth data${error}`);
+    }
+  }
+
   async get(domain: string) {
     const authData = getEnvAuth();
 
@@ -96,13 +94,13 @@ export class VercelAPL implements APL {
   }
 
   async set(authData: AuthData) {
-    await saveDataToVercel(this.registerAppURL, this.deploymentToken, authData);
+    await this.saveDataToVercel(authData);
   }
 
   async delete(domain: string) {
     if (domain === getEnvAuth()?.domain) {
       // Override existing data with the empty values
-      await saveDataToVercel(this.registerAppURL, this.deploymentToken);
+      await this.saveDataToVercel();
     }
   }