improve attachment processing

allure-framework · Jan 9, 2025 · ee53949 · ee53949
1 parent 013aec4
commit ee53949
Show file tree

Hide file tree

Showing 11 changed files with 268 additions and 19 deletions.
diff --git a/packages/core/src/store/convert.ts b/packages/core/src/store/convert.ts
@@ -26,9 +26,9 @@ import type {
   RawTestResult,
   ReaderContext,
 } from "@allurereport/reader-api";
+import { extension, lookupContentType } from "@allurereport/reader-api";
 import MarkdownIt from "markdown-it";
 import { randomUUID } from "node:crypto";
-import { extname } from "node:path";
 
 const defaultStatus: TestStatus = "unknown";
 
@@ -171,12 +171,14 @@ const processAttachmentLink = (
   const previous: AttachmentLink | undefined = attachments.get(id);
 
   if (!previous) {
+    const contentType: string | undefined = attach.contentType ?? lookupContentType(attach.originalFileName);
+    const ext = extension(attach.originalFileName, contentType) ?? "";
     const linkExpected: AttachmentLinkExpected = {
       id,
       originalFileName: attach.originalFileName,
-      ext: extname(attach.originalFileName),
+      ext,
       name: attach.name ?? attach.originalFileName,
-      contentType: attach.contentType,
+      contentType,
       used: true,
       missed: true,
     };
@@ -204,6 +206,9 @@ const processAttachmentLink = (
     id,
     name: attach.name ?? previous.originalFileName,
     contentType: attach.contentType ?? previous.contentType,
+    // if no extension is specified for originalFileName, we should use provided
+    // contentType in the link first, and only then rely on detected content type.
+    ext: extension(previous.originalFileName, attach.contentType) ?? previous.ext ?? "",
     contentLength: previous.contentLength,
     used: true,
     missed: false,

diff --git a/packages/core/src/store/store.ts b/packages/core/src/store/store.ts
@@ -143,7 +143,7 @@ export class DefaultAllureStore implements AllureStore, ResultsVisitor {
       // we need to preserve the same object since it's referenced in steps
       const link = maybeLink as AttachmentLinkLinked;
       link.missed = false;
-      link.ext = link?.ext ?? resultFile.getExtension();
+      link.ext = link.ext === undefined || link.ext === "" ? resultFile.getExtension() : link.ext;
       link.contentType = link.contentType ?? resultFile.getContentType();
       link.contentLength = resultFile.getContentLength();
     } else {

diff --git a/packages/core/test/store/convert.test.ts b/packages/core/test/store/convert.test.ts
@@ -120,4 +120,38 @@ describe("testResultRawToState", () => {
       historyId: `${md5(testId)}.${md5("")}`,
     });
   });
+
+  it("should detect attachment link content type based on file extension if specified", () => {
+    const result = testResultRawToState(
+      emptyStateData,
+      { steps: [{ type: "attachment", originalFileName: "some-file.txt" }] },
+      { readerId },
+    );
+
+    const [attachment] = result.steps;
+    expect(attachment).toMatchObject({
+      type: "attachment",
+      link: {
+        originalFileName: "some-file.txt",
+        contentType: "text/plain",
+      },
+    });
+  });
+
+  it("should set extension based on file name", () => {
+    const result = testResultRawToState(
+      emptyStateData,
+      { steps: [{ type: "attachment", originalFileName: "some-file.txt" }] },
+      { readerId },
+    );
+
+    const [attachment] = result.steps;
+    expect(attachment).toMatchObject({
+      type: "attachment",
+      link: {
+        originalFileName: "some-file.txt",
+        ext: ".txt",
+      },
+    });
+  });
 });
diff --git a/packages/core/test/store/store.test.ts b/packages/core/test/store/store.test.ts
@@ -892,6 +892,172 @@ describe("attachments", () => {
       type: "attachment",
     });
   });
+
+  it("should use extension from original file name if any (link first)", async () => {
+    const store = new DefaultAllureStore();
+    const tr1: RawTestResult = {
+      name: "test result 1",
+      steps: [
+        {
+          name: "attachment 1",
+          type: "attachment",
+          originalFileName: "tr1-source1.txt",
+        },
+      ],
+    };
+
+    const buffer1 = Buffer.from("<svg xmlns=\"http://www.w3.org/2000/svg\"></svg>", "utf-8");
+    const rf1 = new BufferResultFile(buffer1, "tr1-source1.txt");
+    await store.visitTestResult(tr1, { readerId });
+    await store.visitAttachmentFile(rf1, { readerId });
+
+    const [attachment] = await store.allAttachments();
+
+    expect(attachment).toMatchObject({
+      name: "attachment 1",
+      originalFileName: "tr1-source1.txt",
+      ext: ".txt"
+    });
+  });
+
+  it("should use extension from original file name if any (file first)", async () => {
+    const store = new DefaultAllureStore();
+    const tr1: RawTestResult = {
+      name: "test result 1",
+      steps: [
+        {
+          name: "attachment 1",
+          type: "attachment",
+          originalFileName: "tr1-source1.txt",
+        },
+      ],
+    };
+
+    const buffer1 = Buffer.from("<svg xmlns=\"http://www.w3.org/2000/svg\"></svg>", "utf-8");
+    const rf1 = new BufferResultFile(buffer1, "tr1-source1.txt");
+    await store.visitAttachmentFile(rf1, { readerId });
+    await store.visitTestResult(tr1, { readerId });
+
+    const [attachment] = await store.allAttachments();
+
+    expect(attachment).toMatchObject({
+      name: "attachment 1",
+      originalFileName: "tr1-source1.txt",
+      ext: ".txt"
+    });
+  });
+
+  it("should use extension based on content type specified in link if file without extension (link first)", async () => {
+    const store = new DefaultAllureStore();
+    const tr1: RawTestResult = {
+      name: "test result 1",
+      steps: [
+        {
+          name: "attachment 1",
+          type: "attachment",
+          originalFileName: "tr1-source1",
+          contentType: "text/plain"
+        },
+      ],
+    };
+
+    const buffer1 = Buffer.from("<svg xmlns=\"http://www.w3.org/2000/svg\"></svg>", "utf-8");
+    const rf1 = new BufferResultFile(buffer1, "tr1-source1");
+    await store.visitTestResult(tr1, { readerId });
+    await store.visitAttachmentFile(rf1, { readerId });
+
+    const [attachment] = await store.allAttachments();
+
+    expect(attachment).toMatchObject({
+      name: "attachment 1",
+      originalFileName: "tr1-source1",
+      ext: ".txt"
+    });
+  });
+
+  it("should use extension based on content type specified in link if file without extension (file first)", async () => {
+    const store = new DefaultAllureStore();
+    const tr1: RawTestResult = {
+      name: "test result 1",
+      steps: [
+        {
+          name: "attachment 1",
+          type: "attachment",
+          originalFileName: "tr1-source1",
+          contentType: "text/plain"
+        },
+      ],
+    };
+
+    const buffer1 = Buffer.from("<svg xmlns=\"http://www.w3.org/2000/svg\"></svg>", "utf-8");
+    const rf1 = new BufferResultFile(buffer1, "tr1-source1");
+    await store.visitAttachmentFile(rf1, { readerId });
+    await store.visitTestResult(tr1, { readerId });
+
+    const [attachment] = await store.allAttachments();
+
+    expect(attachment).toMatchObject({
+      name: "attachment 1",
+      originalFileName: "tr1-source1",
+      ext: ".txt"
+    });
+  });
+
+  it("should use extension based on detected content type if no extension and content type is provided (link first)", async () => {
+    const store = new DefaultAllureStore();
+    const tr1: RawTestResult = {
+      name: "test result 1",
+      steps: [
+        {
+          name: "attachment 1",
+          type: "attachment",
+          originalFileName: "tr1-source1",
+        },
+      ],
+    };
+
+    const buffer1 = Buffer.from("<svg xmlns=\"http://www.w3.org/2000/svg\"></svg>", "utf-8");
+    const rf1 = new BufferResultFile(buffer1, "tr1-source1");
+    await store.visitTestResult(tr1, { readerId });
+    await store.visitAttachmentFile(rf1, { readerId });
+
+    const [attachment] = await store.allAttachments();
+
+    expect(attachment).toMatchObject({
+      name: "attachment 1",
+      originalFileName: "tr1-source1",
+      contentType: "image/svg+xml",
+      ext: ".svg"
+    });
+  });
+
+  it("should use extension based on detected content type if no extension and content type is provided (file first)", async () => {
+    const store = new DefaultAllureStore();
+    const tr1: RawTestResult = {
+      name: "test result 1",
+      steps: [
+        {
+          name: "attachment 1",
+          type: "attachment",
+          originalFileName: "tr1-source1",
+        },
+      ],
+    };
+
+    const buffer1 = Buffer.from("<svg xmlns=\"http://www.w3.org/2000/svg\"></svg>", "utf-8");
+    const rf1 = new BufferResultFile(buffer1, "tr1-source1");
+    await store.visitAttachmentFile(rf1, { readerId });
+    await store.visitTestResult(tr1, { readerId });
+
+    const [attachment] = await store.allAttachments();
+
+    expect(attachment).toMatchObject({
+      name: "attachment 1",
+      originalFileName: "tr1-source1",
+      contentType: "image/svg+xml",
+      ext: ".svg"
+    });
+  });
 });
 
 describe("history", () => {

diff --git a/packages/reader-api/src/detect.ts b/packages/reader-api/src/detect.ts
@@ -491,7 +491,7 @@ class MagicMatchClause implements Clause {
     private end: number = start,
   ) {}
   eval = (data: Uint8Array): boolean => {
-    if (data.length < this.pattern.length + this.end - this.start) {
+    if (data.length < this.pattern.length + this.start) {
       return false;
     }
 

diff --git a/packages/reader-api/src/index.ts b/packages/reader-api/src/index.ts
@@ -1,3 +1,5 @@
 export type * from "./model.js";
 export type * from "./reader.js";
 export * from "./resultFile.js";
+export { detectContentType } from "./detect.js";
+export { extension, lookupContentType } from "./utils.js";
diff --git a/packages/reader-api/src/resultFile.ts b/packages/reader-api/src/resultFile.ts
@@ -1,5 +1,5 @@
 import type { ResultFile } from "@allurereport/plugin-api";
-import { extension, lookup } from "mime-types";
+import { lookup } from "mime-types";
 import {
   ReadStream,
   closeSync,
@@ -11,13 +11,16 @@ import {
   statSync,
 } from "node:fs";
 import "node:fs/promises";
-import { basename, extname } from "node:path";
+import { basename } from "node:path";
 import type { Readable } from "node:stream";
 import { pipeline } from "node:stream/promises";
 import { detectContentType } from "./detect.js";
+import { extension } from "./utils.js";
 
 export abstract class BaseResultFile implements ResultFile {
   fileName: string;
+  extension: string | false = false;
+  contentType: string | undefined | false = false;
 
   protected constructor(fileName: string) {
     this.fileName = fileName;
@@ -30,6 +33,13 @@ export abstract class BaseResultFile implements ResultFile {
   abstract getContentLength(): number | undefined;
 
   getContentType(): string | undefined {
+    if (this.contentType === false) {
+      this.contentType = this.#detectContentType();
+    }
+    return this.contentType;
+  }
+
+  #detectContentType() {
     const res = lookup(this.getOriginalFileName());
     if (res === false) {
       const magicHeader = this.readMagicHeader();
@@ -46,19 +56,10 @@ export abstract class BaseResultFile implements ResultFile {
   }
 
   getExtension(): string {
-    const ext = extname(this.getOriginalFileName());
-    if (ext !== "") {
-      return ext;
-    }
-    const contentType = this.getContentType();
-    if (contentType) {
-      const result = extension(contentType);
-      if (result === false) {
-        return "";
-      }
-      return `.${result}`;
+    if (this.extension === false) {
+      this.extension = extension(this.getOriginalFileName()) ?? extension("", this.getContentType()) ?? "";
     }
-    return "";
+    return this.extension;
   }
 
   async asJson<T>(): Promise<T | undefined> {

diff --git a/packages/reader-api/src/utils.ts b/packages/reader-api/src/utils.ts
@@ -0,0 +1,25 @@
+import { extension as extensionFromContentType, lookup } from "mime-types";
+import { extname } from "node:path";
+
+export const extension = (fileName: string, contentType?: string) => {
+  const ext = extname(fileName);
+  if (ext !== "") {
+    return ext;
+  }
+  if (contentType) {
+    const result = extensionFromContentType(contentType);
+    if (result === false) {
+      return undefined;
+    }
+    return `.${result}`;
+  }
+  return undefined;
+};
+
+export const lookupContentType = (fileName: string) => {
+  const res = lookup(fileName);
+  if (res === false) {
+    return undefined;
+  }
+  return res;
+};
diff --git a/packages/reader-api/test/resources/short.svg b/packages/reader-api/test/resources/short.svg
diff --git a/packages/reader-api/test/utils.test.ts b/packages/reader-api/test/utils.test.ts
@@ -0,0 +1,14 @@
+import { describe, expect, it } from "vitest";
+import { extension } from "../src/utils.js";
+
+describe("extension", () => {
+  it("should return original extension if any", () => {
+    const result = extension("some.json", "text/plain");
+    expect(result).toEqual(".json");
+  });
+
+  it("should return extension based on content type if no file extension", () => {
+    const result = extension("without-extension", "text/plain");
+    expect(result).toEqual(".txt");
+  });
+});
diff --git a/packages/reader-api/test/utils.ts b/packages/reader-api/test/utils.ts
@@ -12,4 +12,5 @@ export const resources = [
   ["sample.tiff", "image/tiff"],
   ["sample.jpeg", "image/jpeg"],
   ["sample.svg", "image/svg+xml"],
+  ["short.svg", "image/svg+xml"],
 ];