Merge pull request #21418 from Snuffleupagus/getAttachments-Map

[api-minor] Convert `getAttachments` to return data in a `Map`

src/core/catalog.js

@@ -1136,21 +1136,21 @@ class Catalog {
   /**
    * Get attachments.
    *
-   * @returns {Record<string, CatalogAttachment> | null}
+   * @returns {Map<string, CatalogAttachment> | null}
    *   Attachments.
    */
   get attachments() {
     const obj = this.#catDict.get("Names");
-    /** @type {Record<string, CatalogAttachment> | null} */
+    /** @type {Map<string, CatalogAttachment> | null} */
     let attachments = null;
 
     if (obj instanceof Dict && obj.has("EmbeddedFiles")) {
       const nameTree = new NameTree(obj.getRaw("EmbeddedFiles"), this.xref);
       for (const [key, value] of nameTree.getAll()) {
-        const fs = new FileSpec(value);
-        attachments ??= Object.create(null);
-        attachments[stringToPDFString(key, /* keepEscapeSequence = */ true)] =
-          fs.serializable;
+        (attachments ??= new Map()).set(
+          stringToPDFString(key, /* keepEscapeSequence = */ true),
+          new FileSpec(value).serializable
+        );
       }
     }
     return shadow(this, "attachments", attachments);
@@ -1880,7 +1880,7 @@ class Catalog {
 
           if (docAttachments && id) {
             resultObj.attachmentId = id;
-            resultObj.attachment = docAttachments[id];
+            resultObj.attachment = docAttachments.get(id);
 
             // NOTE: the destination is relative to the *attachment*.
             const attachmentDest = fetchRemoteDest(action);

src/display/api.js

@@ -838,7 +838,7 @@ class PDFDocumentProxy {
   }
 
   /**
-   * @returns {Promise<Record<string, CatalogAttachment> | null>}
+   * @returns {Promise<Map<string, CatalogAttachment> | null>}
    *   Promise that is resolved with a lookup table for mapping named
    *   attachments to their content.
    */
@@ -3077,7 +3077,7 @@ class WorkerTransport {
   }
 
   /**
-   * @returns {Promise<Record<string, CatalogAttachment> | null>}
+   * @returns {Promise<Map<string, CatalogAttachment> | null>}
    *   Promise that is resolved with a lookup table for mapping named
    *   attachments to their content.
    */

test/unit/api_spec.js

@@ -1778,7 +1778,7 @@ describe("api", function () {
       const pdfDoc = await loadingTask.promise;
       const attachments = await pdfDoc.getAttachments();
 
-      expect(attachments["foo.txt"]).toEqual({
+      expect(attachments.get("foo.txt")).toEqual({
         rawFilename: "foo.txt",
         filename: "foo.txt",
         description: "",
@@ -1797,7 +1797,8 @@ describe("api", function () {
       const pdfDoc = await loadingTask.promise;
       const attachments = await pdfDoc.getAttachments();
 
-      const { rawFilename, filename, description } = attachments["empty.pdf"];
+      const { rawFilename, filename, description } =
+        attachments.get("empty.pdf");
       expect(rawFilename).toEqual("Empty page.pdf");
       expect(filename).toEqual("Empty page.pdf");
       expect(description).toEqual(
@@ -1827,11 +1828,11 @@ describe("api", function () {
 
       const attachments = await pdfDoc.getAttachments();
       const { description, filename, rawFilename } =
-        attachments["attachment.pdf"] || {};
+        attachments.get("attachment.pdf");
       expect(rawFilename).toEqual("attachment.pdf");
       expect(filename).toEqual("attachment.pdf");
       expect(description).toEqual("");
-      expect(attachments["attachment.pdf"].content).toBeUndefined();
+      expect(attachments.get("attachment.pdf").content).toBeUndefined();
 
       const content = await pdfDoc.getAttachmentContent("attachment.pdf");
       expect(passwordRequests).toEqual(1);
@@ -1861,11 +1862,11 @@ describe("api", function () {
 
       const attachments = await pdfDoc.getAttachments();
       const { description, filename, rawFilename } =
-        attachments["attachment.pdf"] || {};
+        attachments.get("attachment.pdf");
       expect(rawFilename).toEqual("attachment.pdf");
       expect(filename).toEqual("attachment.pdf");
       expect(description).toEqual("");
-      expect(attachments["attachment.pdf"].content).toBeUndefined();
+      expect(attachments.get("attachment.pdf").content).toBeUndefined();
 
       const content = await pdfDoc.getAttachmentContent("attachment.pdf");
       expect(reasons).toEqual([
@@ -1888,7 +1889,7 @@ describe("api", function () {
       try {
         const pdfDoc = await loadingTask.promise;
         const attachments = await pdfDoc.getAttachments();
-        const attachment = attachments?.["attachment.pdf"];
+        const attachment = attachments.get("attachment.pdf");
 
         expect(attachment).toBeDefined();
         expect(attachment.filename).toEqual("attachment.pdf");
@@ -6996,7 +6997,7 @@ small scripts as well as for`);
 
         // Verify the original document has the expected attachment.
         const originalAttachments = await pdfDoc.getAttachments();
-        expect(originalAttachments["foo.txt"]).toBeDefined();
+        expect(originalAttachments.get("foo.txt")).toBeDefined();
 
         const data = await pdfDoc.extractPages([
           { document: null, includePages: [0] },
@@ -7008,7 +7009,7 @@ small scripts as well as for`);
 
         const attachments = await pdfDoc.getAttachments();
         expect(attachments).not.toBeNull();
-        expect(attachments["foo.txt"]).toEqual({
+        expect(attachments.get("foo.txt")).toEqual({
           rawFilename: "foo.txt",
           filename: "foo.txt",
           description: "",
@@ -7043,12 +7044,12 @@ small scripts as well as for`);
         const expectedContent = new Uint8Array([
           98, 97, 114, 32, 98, 97, 122, 32, 10,
         ]);
-        expect(attachments["foo.txt"]).toEqual({
+        expect(attachments.get("foo.txt")).toEqual({
           rawFilename: "foo.txt",
           filename: "foo.txt",
           description: "",
         });
-        expect(attachments["foo.txt_1"]).toEqual({
+        expect(attachments.get("foo.txt_1")).toEqual({
           rawFilename: "foo.txt",
           filename: "foo.txt",
           description: "",

web/pdf_attachment_viewer.js

@@ -39,7 +39,7 @@ import { waitOnEventOrTimeout } from "./event_utils.js";
 
 /**
  * @typedef PDFAttachmentViewerRenderParameters
- * @property {Record<string, CatalogAttachment> | null} attachments - A lookup
+ * @property {Map<string, CatalogAttachment> | null} attachments - A lookup
  *   table of attachment objects.
  * @property {boolean} [keepRenderedCapability]
  */
@@ -154,8 +154,7 @@ class PDFAttachmentViewer extends BaseTreeViewer {
     const ul = document.createElement("ul");
     fragment.append(ul);
     let attachmentsCount = 0;
-    for (const name in attachments) {
-      const item = attachments[name];
+    for (const [name, item] of attachments) {
       const li = document.createElement("li");
       ul.append(li);
       const element = document.createElement("a");
@@ -182,14 +181,12 @@ class PDFAttachmentViewer extends BaseTreeViewer {
       if (renderedPromise !== this._renderedCapability.promise) {
         return; // The FileAttachment annotation belongs to a previous document.
       }
-      const attachments = this._attachments || Object.create(null);
+      const attachments = new Map(this._attachments);
 
-      for (const name in attachments) {
-        if (item.filename === name) {
-          return; // Ignore the new attachment if it already exists.
-        }
+      if (attachments.has(item.filename)) {
+        return; // Ignore the new attachment if it already exists.
       }
-      attachments[item.filename] = item;
+      attachments.set(item.filename, item);
 
       this.render({
         attachments,