From bc45e09dbd01de18046e35ec544f925f9dd507b8 Mon Sep 17 00:00:00 2001 From: Kir_Antipov Date: Thu, 30 Sep 2021 17:01:53 +0300 Subject: [PATCH] Refactoring: made `options` an argument instead of a property --- src/index.ts | 9 ++++++-- src/publishing/github-publisher.ts | 24 ++++++-------------- src/publishing/mod-publisher.ts | 34 ++++++++++------------------- src/publishing/publisher-factory.ts | 8 +++---- src/publishing/publisher.ts | 16 ++++++++------ src/utils/file-utils.ts | 27 +++++++++++++---------- test/file-utils.test.ts | 12 +++++++++- test/publisher-factory.test.ts | 20 +++-------------- 8 files changed, 69 insertions(+), 81 deletions(-) diff --git a/src/index.ts b/src/index.ts index ad0fd88..637ea33 100644 --- a/src/index.ts +++ b/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); } diff --git a/src/publishing/github-publisher.ts b/src/publishing/github-publisher.ts index 33599f4..062b212 100644 --- a/src/publishing/github-publisher.ts +++ b/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 { public get target(): PublisherTarget { return PublisherTarget.GitHub; } - public async publish(): Promise { + public async publish(files: File[], options: GitHubPublisherOptions): Promise { + this.validateOptions(options); let releaseId = 0; const repo = github.context.repo; - const octokit = github.getOctokit(this.options.token); - if (this.options.tag) { - const response = await octokit.rest.repos.getReleaseByTag({ ...repo, tag: this.options.tag }); + const octokit = github.getOctokit(options.token); + if (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 { releaseId = github.context.payload.release?.id; } if (!releaseId) { - throw new Error(`Couldn't find release #${this.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`); + throw new Error(`Couldn't find release #${options.tag || releaseId}`); } const existingAssets = (await octokit.rest.repos.listReleaseAssets({ ...repo, release_id: releaseId })).data; diff --git a/src/publishing/mod-publisher.ts b/src/publishing/mod-publisher.ts index d4085da..35a6a57 100644 --- a/src/publishing/mod-publisher.ts +++ b/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 extends Publisher { - public async publish(): Promise { +export default abstract class ModPublisher extends Publisher { + public async publish(files: File[], options: ModPublisherOptions): Promise { + this.validateOptions(options); const releaseInfo = 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 files = await getFiles(fileSelector); - if (!files.length) { - throw new Error(`Specified files (${typeof fileSelector === "string" ? fileSelector : fileSelector.primary}) were not found`); - } + const version = (typeof options.version === "string" && options.version) || releaseInfo?.tag_name || parseVersionFromFilename(files[0].name); + const versionType = options.versionType?.toLowerCase() || parseVersionTypeFromFilename(files[0].name); + const name = (typeof options.name === "string" && options.name) || releaseInfo?.name || version; + const changelog = ((typeof options.changelog === "string" || options.changelog?.file) ? (await readChangelog(options.changelog)) : releaseInfo?.body) || ""; - const version = (typeof this.options.version === "string" && this.options.version) || releaseInfo?.tag_name || parseVersionFromFilename(files[0].name); - const versionType = this.options.versionType?.toLowerCase() || parseVersionTypeFromFilename(files[0].name); - const name = (typeof this.options.name === "string" && this.options.name) || releaseInfo?.name || version; - const changelog = ((typeof this.options.changelog === "string" || this.options.changelog?.file) ? (await readChangelog(this.options.changelog)) : releaseInfo?.body) || ""; - - const loaders = processMultilineInput(this.options.loaders, /\s+/); + const loaders = processMultilineInput(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 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); } diff --git a/src/publishing/publisher-factory.ts b/src/publishing/publisher-factory.ts index edcbd19..3637adb 100644 --- a/src/publishing/publisher-factory.ts +++ b/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, logger?: Logger): Publisher { + public create(target: PublisherTarget, logger?: Logger): Publisher { switch(target) { case PublisherTarget.GitHub: - return new GitHubPublisher(options, logger); + return new GitHubPublisher(logger); case PublisherTarget.Modrinth: - return new ModrinthPublisher(options, logger); + return new ModrinthPublisher(logger); case PublisherTarget.CurseForge: - return new CurseForgePublisher(options, logger); + return new CurseForgePublisher(logger); default: throw new Error(`Unknown target "${PublisherTarget.toString(target)}"`); diff --git a/src/publishing/publisher.ts b/src/publishing/publisher.ts index 2826fa9..fb1b521 100644 --- a/src/publishing/publisher.ts +++ b/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 { - protected readonly options: TOptions; protected readonly logger: Logger; - public constructor(options: TOptions, logger?: Logger) { - if (!options || typeof options !== "object") { - throw new Error(`Expected options to be an object, got ${options ? typeof options : options}`); - } - this.options = options; + public constructor(logger?: Logger) { this.logger = logger || getEmptyLogger(); } public abstract get target(): PublisherTarget; - public abstract publish(): Promise; + public abstract publish(files: File[], options: TOptions): Promise; + + 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}`); + } + } } diff --git a/src/utils/file-utils.ts b/src/utils/file-utils.ts index 71fde6c..c3a2db6 100644 --- a/src/utils/file-utils.ts +++ b/src/utils/file-utils.ts @@ -24,22 +24,27 @@ export class File { }); } - public getStats(): Promise { - 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 { +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 { + 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 { if (!files || typeof files !== "string" && !files.primary && !files.secondary) { return []; } diff --git a/test/file-utils.test.ts b/test/file-utils.test.ts index aa9aab8..04ff47d 100644 --- a/test/file-utils.test.ts +++ b/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"); diff --git a/test/publisher-factory.test.ts b/test/publisher-factory.test.ts index 7569510..d7f322b 100644 --- a/test/publisher-factory.test.ts +++ b/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((publisher).options).toStrictEqual(options); expect((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, options); + const publisher = factory.create(target); expect(publisher.target).toStrictEqual(target); - expect((publisher).options).toStrictEqual(options); expect((publisher).logger).toBeTruthy(); } }); test("the method throws on invalid PublisherTarget value", () => { const factory = new PublisherFactory(); - 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, options)).toThrow(); - } - } + expect(() => factory.create(-1)).toThrow(); }); });