Skip to content

Commit 22657e2

Browse files
SnuffleupagusSnuffleupagus
authored
Merge pull request #19778 from Snuffleupagus/replace-getAll
[api-minor] Replace various `getAll` methods with iterators
2 parents 12c7c7b + 2c593b0 commit 22657e2

File tree

7 files changed

+53
-92
lines changed

7 files changed

+53
-92
lines changed

‎src/display/annotation_storage.js‎

Lines changed: 16 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@
1313
* limitations under the License.
1414
*/
1515

16-
import { objectFromMap, shadow, unreachable } from "../shared/util.js";
16+
import { shadow, unreachable } from "../shared/util.js";
1717
import { AnnotationEditor } from "./editor/editor.js";
1818
import { MurmurHash3_64 } from "../shared/murmurhash3.js";
1919

@@ -41,6 +41,17 @@ class AnnotationStorage {
4141
this.onSetModified = null;
4242
this.onResetModified = null;
4343
this.onAnnotationEditor = null;
44+
45+
if (typeof PDFJSDev === "undefined" || PDFJSDev.test("TESTING")) {
46+
// For testing purposes.
47+
Object.defineProperty(this, "_setValues", {
48+
value: obj => {
49+
for (const [key, val] of Object.entries(obj)) {
50+
this.setValue(key, val);
51+
}
52+
},
53+
});
54+
}
4455
}
4556

4657
/**
@@ -128,22 +139,6 @@ class AnnotationStorage {
128139
return this.#storage.has(key);
129140
}
130141

131-
/**
132-
* @returns {Object | null}
133-
*/
134-
getAll() {
135-
return this.#storage.size > 0 ? objectFromMap(this.#storage) : null;
136-
}
137-
138-
/**
139-
* @param {Object} obj
140-
*/
141-
setAll(obj) {
142-
for (const [key, val] of Object.entries(obj)) {
143-
this.setValue(key, val);
144-
}
145-
}
146-
147142
get size() {
148143
return this.#storage.size;
149144
}
@@ -278,6 +273,10 @@ class AnnotationStorage {
278273
hash: ids.join(","),
279274
});
280275
}
276+
277+
[Symbol.iterator]() {
278+
return this.#storage.entries();
279+
}
281280
}
282281

283282
/**

‎src/display/metadata.js‎

Lines changed: 5 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -13,15 +13,13 @@
1313
* limitations under the License.
1414
*/
1515

16-
import { objectFromMap } from "../shared/util.js";
17-
1816
class Metadata {
19-
#metadataMap;
17+
#map;
2018

2119
#data;
2220

2321
constructor({ parsedData, rawData }) {
24-
this.#metadataMap = parsedData;
22+
this.#map = parsedData;
2523
this.#data = rawData;
2624
}
2725

@@ -30,15 +28,11 @@ class Metadata {
3028
}
3129

3230
get(name) {
33-
return this.#metadataMap.get(name) ?? null;
34-
}
35-
36-
getAll() {
37-
return objectFromMap(this.#metadataMap);
31+
return this.#map.get(name) ?? null;
3832
}
3933

40-
has(name) {
41-
return this.#metadataMap.has(name);
34+
[Symbol.iterator]() {
35+
return this.#map.entries();
4236
}
4337
}
4438

‎src/display/optional_content_config.js‎

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@
1515

1616
import {
1717
info,
18-
objectFromMap,
1918
RenderingIntentFlag,
2019
unreachable,
2120
warn,
@@ -301,10 +300,6 @@ class OptionalContentConfig {
301300
return [...this.#groups.keys()];
302301
}
303302

304-
getGroups() {
305-
return this.#groups.size > 0 ? objectFromMap(this.#groups) : null;
306-
}
307-
308303
getGroup(id) {
309304
return this.#groups.get(id) || null;
310305
}
@@ -320,6 +315,10 @@ class OptionalContentConfig {
320315
}
321316
return (this.#cachedGetHash = hash.hexdigest());
322317
}
318+
319+
[Symbol.iterator]() {
320+
return this.#groups.entries();
321+
}
323322
}
324323

325324
export { OptionalContentConfig };

‎src/shared/util.js‎

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -577,16 +577,6 @@ function objectSize(obj) {
577577
return Object.keys(obj).length;
578578
}
579579

580-
// Ensure that the returned Object has a `null` prototype; hence why
581-
// `Object.fromEntries(...)` is not used.
582-
function objectFromMap(map) {
583-
const obj = Object.create(null);
584-
for (const [key, value] of map) {
585-
obj[key] = value;
586-
}
587-
return obj;
588-
}
589-
590580
// Checks the endianness of the platform.
591581
function isLittleEndian() {
592582
const buffer8 = new Uint8Array(4);
@@ -1311,7 +1301,6 @@ export {
13111301
LINE_FACTOR,
13121302
MathClamp,
13131303
normalizeUnicode,
1314-
objectFromMap,
13151304
objectSize,
13161305
OPS,
13171306
PageActionEventType,

‎test/driver.js‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -738,7 +738,7 @@ class Driver {
738738
await page.getAnnotations({ intent: "display" });
739739
}
740740
}
741-
doc.annotationStorage.setAll(task.annotationStorage);
741+
doc.annotationStorage._setValues(task.annotationStorage);
742742

743743
const data = await doc.saveDocument();
744744
await loadingTask.destroy();
@@ -919,7 +919,7 @@ class Driver {
919919
pageColors = null;
920920

921921
if (task.annotationStorage) {
922-
task.pdfDoc.annotationStorage.setAll(task.annotationStorage);
922+
task.pdfDoc.annotationStorage._setValues(task.annotationStorage);
923923
}
924924

925925
let textLayerCanvas, annotationLayerCanvas, annotationLayerContext;

‎test/unit/annotation_storage_spec.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ describe("AnnotationStorage", function () {
5959
it("should set a new value in the annotation storage", function () {
6060
const annotationStorage = new AnnotationStorage();
6161
annotationStorage.setValue("123A", { value: "an other string" });
62-
const value = annotationStorage.getAll()["123A"].value;
62+
const { value } = annotationStorage.getRawValue("123A");
6363
expect(value).toEqual("an other string");
6464
});
6565

‎test/unit/metadata_spec.js‎

Lines changed: 25 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,6 @@
1616
import { Metadata } from "../../src/display/metadata.js";
1717
import { MetadataParser } from "../../src/core/metadata_parser.js";
1818

19-
const emptyObj = Object.create(null);
20-
2119
function createMetadata(data) {
2220
const metadataParser = new MetadataParser(data);
2321
return new Metadata(metadataParser.serializable);
@@ -33,13 +31,10 @@ describe("metadata", function () {
3331
"</rdf:Alt></dc:title></rdf:Description></rdf:RDF></x:xmpmeta>";
3432
const metadata = createMetadata(data);
3533

36-
expect(metadata.has("dc:title")).toBeTruthy();
37-
expect(metadata.has("dc:qux")).toBeFalsy();
38-
3934
expect(metadata.get("dc:title")).toEqual("Foo bar baz");
4035
expect(metadata.get("dc:qux")).toEqual(null);
4136

42-
expect(metadata.getAll()).toEqual({ "dc:title": "Foo bar baz" });
37+
expect([...metadata]).toEqual([["dc:title", "Foo bar baz"]]);
4338
});
4439

4540
it("should repair and handle invalid metadata", function () {
@@ -51,13 +46,10 @@ describe("metadata", function () {
5146
"</rdf:Description></rdf:RDF></x:xmpmeta>";
5247
const metadata = createMetadata(data);
5348

54-
expect(metadata.has("dc:title")).toBeTruthy();
55-
expect(metadata.has("dc:qux")).toBeFalsy();
56-
5749
expect(metadata.get("dc:title")).toEqual("PDF&");
5850
expect(metadata.get("dc:qux")).toEqual(null);
5951

60-
expect(metadata.getAll()).toEqual({ "dc:title": "PDF&" });
52+
expect([...metadata]).toEqual([["dc:title", "PDF&"]]);
6153
});
6254

6355
it("should repair and handle invalid metadata (bug 1424938)", function () {
@@ -94,19 +86,16 @@ describe("metadata", function () {
9486
"</x:xmpmeta>";
9587
const metadata = createMetadata(data);
9688

97-
expect(metadata.has("dc:title")).toBeTruthy();
98-
expect(metadata.has("dc:qux")).toBeFalsy();
99-
10089
expect(metadata.get("dc:title")).toEqual(
10190
"L'Odissee thématique logo Odisséé - décembre 2008.pub"
10291
);
10392
expect(metadata.get("dc:qux")).toEqual(null);
10493

105-
expect(metadata.getAll()).toEqual({
106-
"dc:creator": ["ODIS"],
107-
"dc:title": "L'Odissee thématique logo Odisséé - décembre 2008.pub",
108-
"xap:creatortool": "PDFCreator Version 0.9.6",
109-
});
94+
expect([...metadata].sort()).toEqual([
95+
["dc:creator", ["ODIS"]],
96+
["dc:title", "L'Odissee thématique logo Odisséé - décembre 2008.pub"],
97+
["xap:creatortool", "PDFCreator Version 0.9.6"],
98+
]);
11099
});
111100

112101
it("should gracefully handle incomplete tags (issue 8884)", function () {
@@ -137,7 +126,7 @@ describe("metadata", function () {
137126
'<?xpacket end="w"?>';
138127
const metadata = createMetadata(data);
139128

140-
expect(metadata.getAll()).toEqual(emptyObj);
129+
expect([...metadata]).toEqual([]);
141130
});
142131

143132
it('should gracefully handle "junk" before the actual metadata (issue 10395)', function () {
@@ -168,26 +157,23 @@ describe("metadata", function () {
168157
'</rdf:Description></rdf:RDF></x:xmpmeta><?xpacket end="w"?>';
169158
const metadata = createMetadata(data);
170159

171-
expect(metadata.has("dc:title")).toBeTruthy();
172-
expect(metadata.has("dc:qux")).toBeFalsy();
173-
174160
expect(metadata.get("dc:title")).toEqual("");
175161
expect(metadata.get("dc:qux")).toEqual(null);
176162

177-
expect(metadata.getAll()).toEqual({
178-
"dc:creator": [""],
179-
"dc:description": "",
180-
"dc:format": "application/pdf",
181-
"dc:subject": [],
182-
"dc:title": "",
183-
"pdf:keywords": "",
184-
"pdf:pdfversion": "1.7",
185-
"pdf:producer": "PDFKit.NET 4.0.102.0",
186-
"xap:createdate": "2018-12-27T13:50:36-08:00",
187-
"xap:creatortool": "",
188-
"xap:metadatadate": "2018-12-27T13:50:38-08:00",
189-
"xap:modifydate": "2018-12-27T13:50:38-08:00",
190-
});
163+
expect([...metadata].sort()).toEqual([
164+
["dc:creator", [""]],
165+
["dc:description", ""],
166+
["dc:format", "application/pdf"],
167+
["dc:subject", []],
168+
["dc:title", ""],
169+
["pdf:keywords", ""],
170+
["pdf:pdfversion", "1.7"],
171+
["pdf:producer", "PDFKit.NET 4.0.102.0"],
172+
["xap:createdate", "2018-12-27T13:50:36-08:00"],
173+
["xap:creatortool", ""],
174+
["xap:metadatadate", "2018-12-27T13:50:38-08:00"],
175+
["xap:modifydate", "2018-12-27T13:50:38-08:00"],
176+
]);
191177
});
192178

193179
it('should correctly handle metadata containing "&apos" (issue 10407)', function () {
@@ -200,13 +186,10 @@ describe("metadata", function () {
200186
"</rdf:Alt></dc:title></rdf:Description></rdf:RDF></x:xmpmeta>";
201187
const metadata = createMetadata(data);
202188

203-
expect(metadata.has("dc:title")).toBeTruthy();
204-
expect(metadata.has("dc:qux")).toBeFalsy();
205-
206189
expect(metadata.get("dc:title")).toEqual("'Foo bar baz'");
207190
expect(metadata.get("dc:qux")).toEqual(null);
208191

209-
expect(metadata.getAll()).toEqual({ "dc:title": "'Foo bar baz'" });
192+
expect([...metadata]).toEqual([["dc:title", "'Foo bar baz'"]]);
210193
});
211194

212195
it("should gracefully handle unbalanced end tags (issue 10410)", function () {
@@ -229,7 +212,7 @@ describe("metadata", function () {
229212
'</rdf:RDF></x:xmpmeta><?xpacket end="w"?>';
230213
const metadata = createMetadata(data);
231214

232-
expect(metadata.getAll()).toEqual(emptyObj);
215+
expect([...metadata]).toEqual([]);
233216
});
234217

235218
it("should not be vulnerable to the billion laughs attack", function () {
@@ -258,12 +241,9 @@ describe("metadata", function () {
258241
"</rdf:RDF>";
259242
const metadata = createMetadata(data);
260243

261-
expect(metadata.has("dc:title")).toBeTruthy();
262-
expect(metadata.has("dc:qux")).toBeFalsy();
263-
264244
expect(metadata.get("dc:title")).toEqual("a&lol9;b");
265245
expect(metadata.get("dc:qux")).toEqual(null);
266246

267-
expect(metadata.getAll()).toEqual({ "dc:title": "a&lol9;b" });
247+
expect([...metadata]).toEqual([["dc:title", "a&lol9;b"]]);
268248
});
269249
});

0 commit comments

Comments
 (0)