From 22bd9471dae81d99cf765810051223d304ce83d1 Mon Sep 17 00:00:00 2001 From: Kir_Antipov Date: Mon, 15 Jan 2024 15:49:16 +0300 Subject: [PATCH] Refactored metadata readers - `ZippedLoaderMetadataReader` was removed - Metadata readers now throw instead of returning `undefined` --- src/loaders/fabric/fabric-metadata-reader.ts | 15 ++- src/loaders/forge/forge-metadata-reader.ts | 15 ++- src/loaders/loader-metadata-reader.ts | 8 +- src/loaders/quilt/quilt-metadata-reader.ts | 15 ++- src/loaders/zipped-loader-metadata-reader.ts | 115 ------------------ src/program.ts | 4 +- .../fabric/fabric-metadata-reader.spec.ts | 12 +- .../forge/forge-metadata-reader.spec.ts | 23 +--- .../loaders/loader-metadata-reader.spec.ts | 20 ++- .../quilt/quilt-metadata-reader.spec.ts | 12 +- .../zipped-loader-metadata-reader.spec.ts | 100 --------------- 11 files changed, 55 insertions(+), 284 deletions(-) delete mode 100644 src/loaders/zipped-loader-metadata-reader.ts delete mode 100644 tests/unit/loaders/zipped-loader-metadata-reader.spec.ts diff --git a/src/loaders/fabric/fabric-metadata-reader.ts b/src/loaders/fabric/fabric-metadata-reader.ts index 36011dc..1d7c22b 100644 --- a/src/loaders/fabric/fabric-metadata-reader.ts +++ b/src/loaders/fabric/fabric-metadata-reader.ts @@ -1,15 +1,18 @@ -import { ZippedTextLoaderMetadataReader } from "@/loaders/zipped-loader-metadata-reader"; +import { PathLike } from "node:fs"; +import { readAllZippedText } from "@/utils/io/file-info"; +import { LoaderMetadataReader } from "../loader-metadata-reader"; import { FabricMetadata } from "./fabric-metadata"; -import { FABRIC_MOD_JSON, RawFabricMetadata } from "./raw-fabric-metadata"; +import { FABRIC_MOD_JSON } from "./raw-fabric-metadata"; /** * A metadata reader that is able to read Fabric mod metadata from a zipped file. */ -export class FabricMetadataReader extends ZippedTextLoaderMetadataReader { +export class FabricMetadataReader implements LoaderMetadataReader { /** - * Constructs a new {@link FabricMetadataReader} instance. + * @inheritdoc */ - constructor() { - super(FABRIC_MOD_JSON, FabricMetadata.from, JSON.parse); + async readMetadataFile(path: PathLike): Promise { + const metadataText = await readAllZippedText(path, FABRIC_MOD_JSON); + return FabricMetadata.from(JSON.parse(metadataText)); } } diff --git a/src/loaders/forge/forge-metadata-reader.ts b/src/loaders/forge/forge-metadata-reader.ts index 8172baa..89588df 100644 --- a/src/loaders/forge/forge-metadata-reader.ts +++ b/src/loaders/forge/forge-metadata-reader.ts @@ -1,16 +1,19 @@ +import { PathLike } from "node:fs"; import { parse as parseToml } from "toml"; -import { ZippedTextLoaderMetadataReader } from "@/loaders/zipped-loader-metadata-reader"; -import { MODS_TOML, RawForgeMetadata } from "./raw-forge-metadata"; +import { readAllZippedText } from "@/utils/io/file-info"; +import { LoaderMetadataReader } from "../loader-metadata-reader"; import { ForgeMetadata } from "./forge-metadata"; +import { MODS_TOML } from "./raw-forge-metadata"; /** * A metadata reader that is able to read Forge mod metadata from a zipped file. */ -export class ForgeMetadataReader extends ZippedTextLoaderMetadataReader { +export class ForgeMetadataReader implements LoaderMetadataReader { /** - * Constructs a new {@link ForgeMetadataReader} instance. + * @inheritdoc */ - constructor() { - super(MODS_TOML, ForgeMetadata.from, parseToml); + async readMetadataFile(path: PathLike): Promise { + const metadataText = await readAllZippedText(path, MODS_TOML); + return ForgeMetadata.from(parseToml(metadataText)); } } diff --git a/src/loaders/loader-metadata-reader.ts b/src/loaders/loader-metadata-reader.ts index 07c1261..0fd968f 100644 --- a/src/loaders/loader-metadata-reader.ts +++ b/src/loaders/loader-metadata-reader.ts @@ -17,9 +17,11 @@ export interface LoaderMetadataReader * * @param path - The path to the metadata file. * - * @returns The metadata object, or `undefined` if the file cannot be read. + * @returns The metadata object. + * + * @throws {Error} - If the file cannot be read. */ - readMetadataFile(path: PathLike): Promise; + readMetadataFile(path: PathLike): Promise; } /** @@ -40,7 +42,7 @@ export function combineLoaderMetadataReaders(readers: Iterable { +export class QuiltMetadataReader implements LoaderMetadataReader { /** - * Constructs a new {@link QuiltMetadataReader} instance. + * @inheritdoc */ - constructor() { - super(QUILT_MOD_JSON, QuiltMetadata.from, JSON.parse); + async readMetadataFile(path: PathLike): Promise { + const metadataText = await readAllZippedText(path, QUILT_MOD_JSON); + return QuiltMetadata.from(JSON.parse(metadataText)); } } diff --git a/src/loaders/zipped-loader-metadata-reader.ts b/src/loaders/zipped-loader-metadata-reader.ts deleted file mode 100644 index e3fe123..0000000 --- a/src/loaders/zipped-loader-metadata-reader.ts +++ /dev/null @@ -1,115 +0,0 @@ -import { Awaitable } from "@/utils/types"; -import { StreamZipAsync, async as ZipArchive } from "node-stream-zip"; -import { PathLike } from "node:fs"; -import { LoaderMetadata } from "./loader-metadata"; -import { LoaderMetadataReader } from "./loader-metadata-reader"; - -/** - * Provides a base for reading metadata from zipped files for various loaders. - * - * @template TMetadata - Represents the processed metadata object. - * @template TRawMetadata - Represents the raw metadata object to be transformed. - */ -export abstract class ZippedLoaderMetadataReader implements LoaderMetadataReader { - /** - * The name of the entry inside the zipped file to read. - */ - private readonly _entry: string; - - /** - * Constructs a new {@link ZippedLoaderMetadataReader} instance. - * - * @param entry - The name of the entry inside the zipped file to read. - */ - protected constructor(entry: string) { - this._entry = entry; - } - - /** - * Reads the metadata file from a zipped file at the given path. - * - * @param path - The path to the zipped file. - * - * @returns The metadata object, or `undefined` if the zipped file cannot be read. - */ - async readMetadataFile(path: PathLike): Promise { - let zip = undefined as StreamZipAsync; - try { - zip = new ZipArchive({ file: path as string }); - const buffer = await zip.entryData(this._entry); - if (!buffer) { - return undefined; - } - - const rawMetadata = await this.readRawMetadata(buffer); - return await this.createMetadata(rawMetadata); - } catch { - return undefined; - } finally { - await zip?.close().catch(() => undefined); - } - } - - /** - * Reads the raw metadata from a buffer. - * - * @param buffer - The buffer containing the raw metadata. - * - * @returns The raw metadata object. - */ - protected abstract readRawMetadata(buffer: Buffer): Promise; - - /** - * Creates a metadata object from the raw metadata. - * - * @param config - The raw metadata object. - * - * @returns The metadata object. - */ - protected abstract createMetadata(config: TRawMetadata): Promise; -} - -/** - * Provides a base for reading metadata from text-based files within zipped files. - * - * @template TMetadata - Represents the processed metadata object. - * @template TRawMetadata - Represents the raw metadata object to be transformed. - */ -export abstract class ZippedTextLoaderMetadataReader extends ZippedLoaderMetadataReader { - /** - * A function to transform the raw metadata into a processed metadata object. - */ - private readonly _factory: (raw: TRawMetadata) => Awaitable; - - /** - * A function to parse the text content into a raw metadata object. - */ - private readonly _parser: (text: string) => Awaitable; - - /** - * Constructs a new {@link ZippedTextLoaderMetadataReader} instance. - * - * @param entry - The name of the entry inside the zipped file to read. - * @param factory - A function to transform the raw metadata into a processed metadata object. - * @param parser - A function to parse the text content into a raw metadata object. - */ - protected constructor(entry: string, factory: (raw: TRawMetadata) => Awaitable, parser: (text: string) => Awaitable) { - super(entry); - this._factory = factory; - this._parser = parser; - } - - /** - * @inheritdoc - */ - protected async readRawMetadata(buffer: Buffer): Promise { - return await this._parser(buffer.toString()); - } - - /** - * @inheritdoc - */ - protected async createMetadata(config: TRawMetadata): Promise { - return await this._factory(config); - } -} diff --git a/src/program.ts b/src/program.ts index a3a1b62..3eebba8 100644 --- a/src/program.ts +++ b/src/program.ts @@ -1,7 +1,7 @@ import { McPublishInput, McPublishOutput } from "@/action"; import { GameVersionFilter, getGameVersionProviderByName } from "@/games"; import { MINECRAFT } from "@/games/minecraft"; -import { LoaderMetadataReader, createDefaultLoaderMetadataReader } from "@/loaders"; +import { LoaderMetadata, LoaderMetadataReader, createDefaultLoaderMetadataReader } from "@/loaders"; import { PlatformType, createPlatformUploader } from "@/platforms"; import { GitHubContext } from "@/platforms/github"; import { SPLIT_BY_WORDS_AND_GROUP_ACTION_PARAMETER_PATH_PARSER, createActionOutputControllerUsingMetadata, getActionOutput, getAllActionInputsAsObjectUsingMetadata, parseActionMetadataFromFile, setActionOutput } from "@/utils/actions"; @@ -103,7 +103,7 @@ async function fillInDefaultValues undefined as LoaderMetadata); const gameVersionProvider = getGameVersionProviderByName(metadata?.gameName || MINECRAFT); const wrappedGameVersions = options.gameVersions?.length ? options.gameVersions : (metadata?.gameVersions || []); diff --git a/tests/unit/loaders/fabric/fabric-metadata-reader.spec.ts b/tests/unit/loaders/fabric/fabric-metadata-reader.spec.ts index cda508a..fe9ba28 100644 --- a/tests/unit/loaders/fabric/fabric-metadata-reader.spec.ts +++ b/tests/unit/loaders/fabric/fabric-metadata-reader.spec.ts @@ -23,19 +23,15 @@ describe("FabricMetadataReader", () => { expect(metadata).toBeInstanceOf(FabricMetadata); }); - test("returns undefined if file is not a Fabric mod", async () => { + test("throws if file is not a Fabric mod", async () => { const reader = new FabricMetadataReader(); - const metadata = await reader.readMetadataFile("text.txt"); - - expect(metadata).toBeUndefined(); + await expect(reader.readMetadataFile("text.txt")).rejects.toThrow(); }); - test("returns undefined if file does not exist", async () => { + test("throws if file does not exist", async () => { const reader = new FabricMetadataReader(); - const metadata = await reader.readMetadataFile("text.json"); - - expect(metadata).toBeUndefined(); + await expect(reader.readMetadataFile("text.json")).rejects.toThrow(); }); }); diff --git a/tests/unit/loaders/forge/forge-metadata-reader.spec.ts b/tests/unit/loaders/forge/forge-metadata-reader.spec.ts index 5f435c5..ad57214 100644 --- a/tests/unit/loaders/forge/forge-metadata-reader.spec.ts +++ b/tests/unit/loaders/forge/forge-metadata-reader.spec.ts @@ -6,7 +6,6 @@ import { ForgeMetadataReader } from "@/loaders/forge/forge-metadata-reader"; beforeEach(async () => { mockFs({ "forge.mod.jar": await zipFile([__dirname, "../../../content/forge/mods.toml"], "META-INF/mods.toml"), - "neoforge.mod.jar": await zipFile([__dirname, "../../../content/neoforge/mods.toml"], "META-INF/mods.toml"), "text.txt": "", }); }); @@ -16,7 +15,7 @@ afterEach(() => { }); describe("ForgeMetadataReader", () => { - test("successfully reads forge/mods.toml", async () => { + test("successfully reads mods.toml", async () => { const reader = new ForgeMetadataReader(); const metadata = await reader.readMetadataFile("forge.mod.jar"); @@ -24,27 +23,15 @@ describe("ForgeMetadataReader", () => { expect(metadata).toBeInstanceOf(ForgeMetadata); }); - test("successfully reads neoforge/mods.toml", async () => { + test("throws if file is not a Forge mod", async () => { const reader = new ForgeMetadataReader(); - const metadata = await reader.readMetadataFile("neoforge.mod.jar"); - - expect(metadata).toBeInstanceOf(ForgeMetadata); + await expect(reader.readMetadataFile("text.txt")).rejects.toThrow(); }); - test("returns undefined if file is not a Forge mod", async () => { + test("throws if file does not exist", async () => { const reader = new ForgeMetadataReader(); - const metadata = await reader.readMetadataFile("text.txt"); - - expect(metadata).toBeUndefined(); - }); - - test("returns undefined if file does not exist", async () => { - const reader = new ForgeMetadataReader(); - - const metadata = await reader.readMetadataFile("text.json"); - - expect(metadata).toBeUndefined(); + await expect(reader.readMetadataFile("text.json")).rejects.toThrow(); }); }); diff --git a/tests/unit/loaders/loader-metadata-reader.spec.ts b/tests/unit/loaders/loader-metadata-reader.spec.ts index 1ffb24a..f86954b 100644 --- a/tests/unit/loaders/loader-metadata-reader.spec.ts +++ b/tests/unit/loaders/loader-metadata-reader.spec.ts @@ -43,20 +43,18 @@ describe("combineLoaderMetadataReaders", () => { expect(reader2.readMetadataFile).toHaveBeenCalledWith("2"); }); - test("combined reader returns undefined when no reader can read the metadata", async () => { + test("combined reader throws when no reader can read the metadata", async () => { const combined = combineLoaderMetadataReaders([]); - const metadata = await combined.readMetadataFile("test"); - expect(metadata).toBeUndefined(); + await expect(combined.readMetadataFile("test")).rejects.toThrow(); }); - test("combined reader returns undefined instead of throwing", async () => { + test("combined reader throws if all underlying readers throw", async () => { const reader1 = { readMetadataFile: jest.fn().mockRejectedValue(new Error("Cannot read the metadata file")) } as LoaderMetadataReader; const combined = combineLoaderMetadataReaders([reader1]); - const metadata = await combined.readMetadataFile("test"); - expect(metadata).toBeUndefined(); + await expect(combined.readMetadataFile("test")).rejects.toThrow(); }); }); @@ -77,21 +75,19 @@ describe("createLoaderMetadataReader", () => { } }); - test("created reader returns undefined for unsupported metadata files", async () => { + test("created reader throws for unsupported metadata files", async () => { for (const loader of LoaderType.values()) { const reader = createLoaderMetadataReader(loader); - const metadata = await reader.readMetadataFile("text.txt"); - expect(metadata).toBeUndefined(); + await expect(reader.readMetadataFile("text.txt")).rejects.toThrow(); } }); - test("created reader returns undefined for non-existing files", async () => { + test("created reader throws for non-existing files", async () => { for (const loader of LoaderType.values()) { const reader = createLoaderMetadataReader(loader); - const metadata = await reader.readMetadataFile("text.json"); - expect(metadata).toBeUndefined(); + await expect(reader.readMetadataFile("text.json")).rejects.toThrow(); } }); diff --git a/tests/unit/loaders/quilt/quilt-metadata-reader.spec.ts b/tests/unit/loaders/quilt/quilt-metadata-reader.spec.ts index ecb6e08..bca50ae 100644 --- a/tests/unit/loaders/quilt/quilt-metadata-reader.spec.ts +++ b/tests/unit/loaders/quilt/quilt-metadata-reader.spec.ts @@ -23,19 +23,15 @@ describe("QuiltMetadataReader", () => { expect(metadata).toBeInstanceOf(QuiltMetadata); }); - test("returns undefined if file is not a Quilt mod", async () => { + test("throws if file is not a Quilt mod", async () => { const reader = new QuiltMetadataReader(); - const metadata = await reader.readMetadataFile("text.txt"); - - expect(metadata).toBeUndefined(); + await expect(reader.readMetadataFile("text.txt")).rejects.toThrow(); }); - test("returns undefined if file does not exist", async () => { + test("throws if file does not exist", async () => { const reader = new QuiltMetadataReader(); - const metadata = await reader.readMetadataFile("text.json"); - - expect(metadata).toBeUndefined(); + await expect(reader.readMetadataFile("text.json")).rejects.toThrow(); }); }); diff --git a/tests/unit/loaders/zipped-loader-metadata-reader.spec.ts b/tests/unit/loaders/zipped-loader-metadata-reader.spec.ts deleted file mode 100644 index bbaa959..0000000 --- a/tests/unit/loaders/zipped-loader-metadata-reader.spec.ts +++ /dev/null @@ -1,100 +0,0 @@ -import { zipContent } from "@/../tests/utils/zip-utils"; -import { LoaderMetadata } from "@/loaders/loader-metadata"; -import mockFs from "mock-fs"; -import { ZippedLoaderMetadataReader, ZippedTextLoaderMetadataReader } from "@/loaders/zipped-loader-metadata-reader"; - -class MockZippedLoaderMetadataReader extends ZippedLoaderMetadataReader { - constructor(entry: string) { - super(entry); - } - - protected readRawMetadata(buffer: Buffer): Promise { - return Promise.resolve(buffer.toString()); - } - - protected createMetadata(config: string): Promise { - return Promise.resolve({ id: config } as LoaderMetadata); - } -} - -class MockZippedTextLoaderMetadataReader extends ZippedTextLoaderMetadataReader { - constructor(entry: string, factory: (raw: string) => LoaderMetadata, parser: (text: string) => string) { - super(entry, factory, parser); - } -} - -beforeEach(async () => { - mockFs({ - "test.zip": await zipContent("Test", "test.txt"), - }); -}); - -afterEach(() => { - mockFs.restore(); -}); - -describe("ZippedLoaderMetadataReader", () => { - test("reads metadata file from a zipped file at the given path", async () => { - const reader = new MockZippedLoaderMetadataReader("test.txt"); - - const metadata = await reader.readMetadataFile("test.zip"); - - expect(metadata).toMatchObject({ id: "Test" }); - }); - - test("returns undefined if the given path does not exist", async () => { - const reader = new MockZippedLoaderMetadataReader("test.txt"); - - const metadata = await reader.readMetadataFile(""); - - expect(metadata).toBeUndefined(); - }); - - test("returns undefined if the zip entry does not exist", async () => { - const reader = new MockZippedLoaderMetadataReader("test"); - - const metadata = await reader.readMetadataFile("test.zip"); - - expect(metadata).toBeUndefined(); - }); -}); - -describe("ZippedTextLoaderMetadataReader", () => { - test("reads metadata file from a zipped file at the given path", async () => { - const factory = jest.fn().mockImplementation(x => ({ id: x })); - const parse = jest.fn().mockImplementation(x => [...String(x)].reverse().join("")); - const reader = new MockZippedTextLoaderMetadataReader("test.txt", factory, parse); - - const metadata = await reader.readMetadataFile("test.zip"); - - expect(metadata).toMatchObject({ id: "tseT" }); - expect(factory).toHaveBeenCalledTimes(1); - expect(factory).toHaveBeenCalledWith("tseT"); - expect(parse).toHaveBeenCalledTimes(1); - expect(parse).toHaveBeenCalledWith("Test"); - }); - - test("returns undefined if the given path does not exist", async () => { - const factory = jest.fn(); - const parse = jest.fn(); - const reader = new MockZippedTextLoaderMetadataReader("test.txt", factory, parse); - - const metadata = await reader.readMetadataFile(""); - - expect(metadata).toBeUndefined(); - expect(factory).not.toHaveBeenCalled(); - expect(parse).not.toHaveBeenCalled(); - }); - - test("returns undefined if the zip entry does not exist", async () => { - const factory = jest.fn(); - const parse = jest.fn(); - const reader = new MockZippedTextLoaderMetadataReader("test", factory, parse); - - const metadata = await reader.readMetadataFile("test.zip"); - - expect(metadata).toBeUndefined(); - expect(factory).not.toHaveBeenCalled(); - expect(parse).not.toHaveBeenCalled(); - }); -});