Refactoring: made options an argument instead of a property

src/index.ts

@@ -1,3 +1,4 @@
+import { getRequiredFiles, gradleOutputSelector } from "./utils/file-utils";
 import PublisherFactory from "./publishing/publisher-factory";
 import PublisherTarget from "./publishing/publisher-target";
 import { getInputAsObject } from "./utils/input-utils";
@@ -16,10 +17,14 @@ async function main() {
             continue;
         }
 
-        const publisher = publisherFactory.create(target, { ...commonOptions, ...publisherOptions }, logger);
+        const options = { ...commonOptions, ...publisherOptions };
+        const fileSelector = options.files && (typeof(options.files) === "string" || options.files.primary) ? options.files : gradleOutputSelector;
+        const files = await getRequiredFiles(fileSelector);
+
+        const publisher = publisherFactory.create(target, logger);
         logger.info(`Publishing assets to ${targetName}...`);
         const start = new Date();
-        await publisher.publish();
+        await publisher.publish(files, options);
         logger.info(`Successfully published assets to ${targetName} (in ${new Date().getTime() - start.getTime()}ms)`);
         publishedTo.push(targetName);
     }

src/publishing/github-publisher.ts

@@ -1,7 +1,7 @@
 import Publisher from "./publisher";
 import PublisherTarget from "./publisher-target";
 import * as github from "@actions/github";
-import { getFiles } from "../utils/file-utils";
+import { File } from "../utils/file-utils";
 
 interface GitHubPublisherOptions {
     tag?: string;
@@ -9,22 +9,18 @@ interface GitHubPublisherOptions {
     files?: string | { primary?: string, secondary?: string };
 }
 
-const defaultFiles = {
-    primary: "build/libs/!(*-@(dev|sources)).jar",
-    secondary: "build/libs/*-@(dev|sources).jar"
-};
-
 export default class GitHubPublisher extends Publisher<GitHubPublisherOptions> {
     public get target(): PublisherTarget {
         return PublisherTarget.GitHub;
     }
 
-    public async publish(): Promise<void> {
+    public async publish(files: File[], options: GitHubPublisherOptions): Promise<void> {
+        this.validateOptions(options);
         let releaseId = 0;
         const repo = github.context.repo;
-        const octokit = github.getOctokit(this.options.token);
+        const octokit = github.getOctokit(options.token);
-        if (this.options.tag) {
+        if (options.tag) {
-            const response = await octokit.rest.repos.getReleaseByTag({ ...repo, tag: this.options.tag });
+            const response = await octokit.rest.repos.getReleaseByTag({ ...repo, tag: options.tag });
             if (response.status >= 200 && response.status < 300) {
                 releaseId = response.data.id;
             }
@@ -32,13 +28,7 @@ export default class GitHubPublisher extends Publisher<GitHubPublisherOptions> {
             releaseId = github.context.payload.release?.id;
         }
         if (!releaseId) {
-            throw new Error(`Couldn't find release #${this.options.tag || releaseId}`);
+            throw new Error(`Couldn't find release #${options.tag || releaseId}`);
-        }
-
-        const fileSelector = this.options.files && (typeof(this.options.files) === "string" || this.options.files.primary) ? this.options.files : defaultFiles;
-        const files = await getFiles(fileSelector);
-        if (!files.length) {
-            throw new Error(`Specified files (${typeof fileSelector === "string" ? fileSelector : fileSelector.primary}) were not found`);
         }
 
         const existingAssets = (await octokit.rest.repos.listReleaseAssets({ ...repo, release_id: releaseId })).data;

src/publishing/mod-publisher.ts

@@ -17,11 +17,6 @@ interface ModPublisherOptions {
     files?: string | { primary?: string, secondary?: string };
 }
 
-const defaultFiles = {
-    primary: "build/libs/!(*-@(dev|sources)).jar",
-    secondary: "build/libs/*-@(dev|sources).jar"
-};
-
 const defaultLoaders = ["fabric"];
 
 function processMultilineInput(input: string | string[], splitter?: RegExp): string[] {
@@ -43,37 +38,32 @@ async function readChangelog(changelog: string | { file?: string }): Promise<str
     return (await file.getBuffer()).toString("utf8");
 }
 
-export default abstract class ModPublisher<TUniqueOptions = unknown> extends Publisher<ModPublisherOptions & TUniqueOptions> {
+export default abstract class ModPublisher extends Publisher<ModPublisherOptions> {
-    public async publish(): Promise<void> {
+    public async publish(files: File[], options: ModPublisherOptions): Promise<void> {
+        this.validateOptions(options);
         const releaseInfo = <any>context.payload.release;
 
-        const id = this.options.id;
+        const id = options.id;
         if (!id) {
             throw new Error(`Project id is required to publish your assets to ${PublisherTarget.toString(this.target)}`);
         }
 
-        const token = this.options.token;
+        const token = options.token;
         if (!token) {
             throw new Error(`Token is required to publish your assets to ${PublisherTarget.toString(this.target)}`);
         }
 
-        const fileSelector = this.options.files && (typeof(this.options.files) === "string" || this.options.files.primary) ? this.options.files : defaultFiles;
+        const version = (typeof options.version === "string" && options.version) || <string>releaseInfo?.tag_name || parseVersionFromFilename(files[0].name);
-        const files = await getFiles(fileSelector);
+        const versionType = options.versionType?.toLowerCase() || parseVersionTypeFromFilename(files[0].name);
-        if (!files.length) {
+        const name = (typeof options.name === "string" && options.name) || <string>releaseInfo?.name || version;
-            throw new Error(`Specified files (${typeof fileSelector === "string" ? fileSelector : fileSelector.primary}) were not found`);
+        const changelog = ((typeof options.changelog === "string" || options.changelog?.file) ? (await readChangelog(options.changelog)) : <string>releaseInfo?.body) || "";
-        }
 
-        const version = (typeof this.options.version === "string" && this.options.version) || <string>releaseInfo?.tag_name || parseVersionFromFilename(files[0].name);
+        const loaders = processMultilineInput(options.loaders, /\s+/);
-        const versionType = this.options.versionType?.toLowerCase() || parseVersionTypeFromFilename(files[0].name);
-        const name = (typeof this.options.name === "string" && this.options.name) || <string>releaseInfo?.name || version;
-        const changelog = ((typeof this.options.changelog === "string" || this.options.changelog?.file) ? (await readChangelog(this.options.changelog)) : <string>releaseInfo?.body) || "";
-
-        const loaders = processMultilineInput(this.options.loaders, /\s+/);
         if (!loaders.length) {
             loaders.push(...defaultLoaders);
         }
 
-        const gameVersions = processMultilineInput(this.options.gameVersions);
+        const gameVersions = processMultilineInput(options.gameVersions);
         if (!gameVersions.length) {
             const minecraftVersion = parseVersionNameFromFileVersion(version);
             if (minecraftVersion) {
@@ -84,7 +74,7 @@ export default abstract class ModPublisher<TUniqueOptions = unknown> extends Pub
             }
         }
 
-        const java = processMultilineInput(this.options.java);
+        const java = processMultilineInput(options.java);
 
         await this.publishMod(id, token, name, version, versionType, loaders, gameVersions, java, changelog, files);
     }

src/publishing/publisher-factory.ts

@@ -6,16 +6,16 @@ import CurseForgePublisher from "./curseforge-publisher";
 import Logger from "../utils/logger";
 
 export default class PublisherFactory {
-    public create(target: PublisherTarget, options: Record<string, unknown>, logger?: Logger): Publisher<unknown> {
+    public create(target: PublisherTarget, logger?: Logger): Publisher<unknown> {
         switch(target) {
             case PublisherTarget.GitHub:
-                return new GitHubPublisher(<any>options, logger);
+                return new GitHubPublisher(logger);
 
             case PublisherTarget.Modrinth:
-                return new ModrinthPublisher(<any>options, logger);
+                return new ModrinthPublisher(logger);
 
             case PublisherTarget.CurseForge:
-                return new CurseForgePublisher(<any>options, logger);
+                return new CurseForgePublisher(logger);
 
             default:
                 throw new Error(`Unknown target "${PublisherTarget.toString(target)}"`);

src/publishing/publisher.ts

@@ -1,20 +1,22 @@
+import { File } from "utils/file-utils";
 import Logger from "../utils/logger";
 import { getEmptyLogger } from "../utils/logger-utils";
 import PublisherTarget from "./publisher-target";
 
 export default abstract class Publisher<TOptions> {
-    protected readonly options: TOptions;
     protected readonly logger: Logger;
 
-    public constructor(options: TOptions, logger?: Logger) {
+    public constructor(logger?: Logger) {
-        if (!options || typeof options !== "object") {
-            throw new Error(`Expected options to be an object, got ${options ? typeof options : options}`);
-        }
-        this.options = options;
         this.logger = logger || getEmptyLogger();
     }
 
     public abstract get target(): PublisherTarget;
 
-    public abstract publish(): Promise<void>;
+    public abstract publish(files: File[], options: TOptions): Promise<void>;
+
+    protected validateOptions(options: TOptions): void | never {
+        if (!options || typeof options !== "object") {
+            throw new Error(`Expected options to be an object, got ${options ? typeof options : options}`);
+        }
+    }
 }

src/utils/file-utils.ts

@@ -24,22 +24,27 @@ export class File {
         });
     }
 
-    public getStats(): Promise<fs.Stats> {
-        return new Promise((resolve, reject) => fs.stat(this.path, (error, stats) => {
-            if (error) {
-                reject(error);
-            } else {
-                resolve(stats);
-            }
-        }));
-    }
-
     public equals(file: unknown): boolean {
         return file instanceof File && file.path === this.path;
     }
 }
 
-export async function getFiles(files: string | { primary?: string, secondary?: string }): Promise<File[]> {
+export type FileSelector = string | { primary?: string, secondary?: string };
+
+export const gradleOutputSelector: FileSelector = {
+    primary: "build/libs/!(*-@(dev|sources)).jar",
+    secondary: "build/libs/*-@(dev|sources).jar"
+};
+
+export async function getRequiredFiles(files: FileSelector): Promise<File[] | never> {
+    const foundFiles = await getFiles(files);
+    if (foundFiles && foundFiles.length) {
+        return foundFiles;
+    }
+    throw new Error(`Specified files ('${typeof files === "string" ? files : [files.primary, files.secondary].filter(x => x).join(", ")}') were not found`);
+}
+
+export async function getFiles(files: FileSelector): Promise<File[]> {
     if (!files || typeof files !== "string" && !files.primary && !files.secondary) {
         return [];
     }

test/file-utils.test.ts

@@ -1,5 +1,5 @@
 import { describe, test, expect } from "@jest/globals";
-import { getFiles, parseVersionFromFilename, parseVersionTypeFromFilename } from "../src/utils/file-utils";
+import { getFiles, getRequiredFiles, parseVersionFromFilename, parseVersionTypeFromFilename } from "../src/utils/file-utils";
 
 describe("getFiles", () => {
     test("all files matching the given pattern are returned", async () => {
@@ -17,6 +17,16 @@ describe("getFiles", () => {
     });
 });
 
+describe("getRequiredFiles", () => {
+    test("all files matching the given pattern are returned", async () => {
+        expect(await getRequiredFiles("(README|LICENSE|FOO).md")).toHaveLength(2);
+    });
+
+    test("an error is thrown if no files are found", async () => {
+        await expect(getRequiredFiles("FOO.md")).rejects.toBeInstanceOf(Error);
+    });
+});
+
 describe("parseVersionFromFilename", () => {
     test("file version is correctly extracted from the filename", () => {
         expect(parseVersionFromFilename("sodium-fabric-mc1.17.1-0.3.2+build.7.jar")).toStrictEqual("mc1.17.1-0.3.2+build.7");

test/publisher-factory.test.ts

@@ -7,11 +7,9 @@ describe("PublisherFactory.create", () => {
     test("factory can create publisher for every PublisherTarget value", () => {
         const factory = new PublisherFactory();
         for (const target of PublisherTarget.getValues()) {
-            const options = {};
             const logger = getConsoleLogger();
-            const publisher = factory.create(target, options, logger);
+            const publisher = factory.create(target, logger);
             expect(publisher.target).toStrictEqual(target);
-            expect((<any>publisher).options).toStrictEqual(options);
             expect((<any>publisher).logger).toStrictEqual(logger);
         }
     });
@@ -19,26 +17,14 @@ describe("PublisherFactory.create", () => {
     test("every publisher has logger object", () => {
         const factory = new PublisherFactory();
         for (const target of PublisherTarget.getValues()) {
-            const options = {};
+            const publisher = factory.create(target);
-            const publisher = factory.create(target, options);
             expect(publisher.target).toStrictEqual(target);
-            expect((<any>publisher).options).toStrictEqual(options);
             expect((<any>publisher).logger).toBeTruthy();
         }
     });
 
     test("the method throws on invalid PublisherTarget value", () => {
         const factory = new PublisherFactory();
-        expect(() => factory.create(-1, {})).toThrow();
+        expect(() => factory.create(-1)).toThrow();
-    });
-
-    test("the method throws on invalid options", () => {
-        const factory = new PublisherFactory();
-        const invalidOptions = [null, undefined, "", true, false, () => {}];
-        for (const target of PublisherTarget.getValues()) {
-            for (const options of invalidOptions) {
-                expect(() => factory.create(target, <any>options)).toThrow();
-            }
-        }
     });
 });