From fa7a607d336113c5ade06688d67873e795559ff4 Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Mon, 13 Sep 2021 13:34:28 +0200 Subject: [PATCH 1/3] Improve the API unit-tests by checking that `getDocument` returns a `PDFDocumentLoadingTask`-instance This is similar to existing unit-tests, which checks for `PDFDocumentProxy`- and `PDFPageProxy`-instances. --- src/display/api.js | 1 + test/unit/api_spec.js | 32 ++++++++++++++++++++++++++++++++ 2 files changed, 33 insertions(+) diff --git a/src/display/api.js b/src/display/api.js index 81cde0712b41e..f9b211f757082 100644 --- a/src/display/api.js +++ b/src/display/api.js @@ -3361,6 +3361,7 @@ export { getDocument, LoopbackPort, PDFDataRangeTransport, + PDFDocumentLoadingTask, PDFDocumentProxy, PDFPageProxy, PDFWorker, diff --git a/test/unit/api_spec.js b/test/unit/api_spec.js index 28afdb91ef91c..90faacc53e6e6 100644 --- a/test/unit/api_spec.js +++ b/test/unit/api_spec.js @@ -35,6 +35,7 @@ import { DefaultCanvasFactory, getDocument, PDFDataRangeTransport, + PDFDocumentLoadingTask, PDFDocumentProxy, PDFPageProxy, PDFWorker, @@ -75,6 +76,7 @@ describe("api", function () { it("creates pdf doc from URL-string", async function () { const urlStr = TEST_PDFS_PATH + basicApiFileName; const loadingTask = getDocument(urlStr); + expect(loadingTask instanceof PDFDocumentLoadingTask).toEqual(true); const pdfDocument = await loadingTask.promise; expect(typeof urlStr).toEqual("string"); @@ -93,6 +95,7 @@ describe("api", function () { window.location ); const loadingTask = getDocument(urlObj); + expect(loadingTask instanceof PDFDocumentLoadingTask).toEqual(true); const pdfDocument = await loadingTask.promise; expect(urlObj instanceof URL).toEqual(true); @@ -104,6 +107,7 @@ describe("api", function () { it("creates pdf doc from URL", async function () { const loadingTask = getDocument(basicApiGetDocumentParams); + expect(loadingTask instanceof PDFDocumentLoadingTask).toEqual(true); const progressReportedCapability = createPromiseCapability(); // Attach the callback that is used to report loading progress; @@ -128,6 +132,7 @@ describe("api", function () { it("creates pdf doc from URL and aborts before worker initialized", async function () { const loadingTask = getDocument(basicApiGetDocumentParams); + expect(loadingTask instanceof PDFDocumentLoadingTask).toEqual(true); const destroyed = loadingTask.destroy(); try { @@ -143,6 +148,7 @@ describe("api", function () { it("creates pdf doc from URL and aborts loading after worker initialized", async function () { const loadingTask = getDocument(basicApiGetDocumentParams); + expect(loadingTask instanceof PDFDocumentLoadingTask).toEqual(true); // This can be somewhat random -- we cannot guarantee perfect // 'Terminate' message to the worker before/after setting up pdfManager. const destroyed = loadingTask._worker.promise.then(function () { @@ -162,6 +168,7 @@ describe("api", function () { expect(typedArrayPdf.length).toEqual(basicApiFileLength); const loadingTask = getDocument(typedArrayPdf); + expect(loadingTask instanceof PDFDocumentLoadingTask).toEqual(true); const progressReportedCapability = createPromiseCapability(); loadingTask.onProgress = function (data) { @@ -181,6 +188,7 @@ describe("api", function () { it("creates pdf doc from invalid PDF file", async function () { // A severely corrupt PDF file (even Adobe Reader fails to open it). const loadingTask = getDocument(buildGetDocumentParams("bug1020226.pdf")); + expect(loadingTask instanceof PDFDocumentLoadingTask).toEqual(true); try { await loadingTask.promise; @@ -203,6 +211,7 @@ describe("api", function () { const loadingTask = getDocument( buildGetDocumentParams("non-existent.pdf") ); + expect(loadingTask instanceof PDFDocumentLoadingTask).toEqual(true); try { await loadingTask.promise; @@ -218,6 +227,7 @@ describe("api", function () { it("creates pdf doc from PDF file protected with user and owner password", async function () { const loadingTask = getDocument(buildGetDocumentParams("pr6531_1.pdf")); + expect(loadingTask instanceof PDFDocumentLoadingTask).toEqual(true); const passwordNeededCapability = createPromiseCapability(); const passwordIncorrectCapability = createPromiseCapability(); @@ -264,6 +274,10 @@ describe("api", function () { password: "", }) ); + expect( + passwordNeededLoadingTask instanceof PDFDocumentLoadingTask + ).toEqual(true); + const result1 = passwordNeededLoadingTask.promise.then( function () { // Shouldn't get here. @@ -282,6 +296,10 @@ describe("api", function () { password: "qwerty", }) ); + expect( + passwordIncorrectLoadingTask instanceof PDFDocumentLoadingTask + ).toEqual(true); + const result2 = passwordIncorrectLoadingTask.promise.then( function () { // Shouldn't get here. @@ -300,6 +318,10 @@ describe("api", function () { password: "asdfasdf", }) ); + expect( + passwordAcceptedLoadingTask instanceof PDFDocumentLoadingTask + ).toEqual(true); + const result3 = passwordAcceptedLoadingTask.promise.then(function (data) { expect(data instanceof PDFDocumentProxy).toEqual(true); return passwordAcceptedLoadingTask.destroy(); @@ -317,11 +339,18 @@ describe("api", function () { const passwordNeededLoadingTask = getDocument( buildGetDocumentParams(filename) ); + expect( + passwordNeededLoadingTask instanceof PDFDocumentLoadingTask + ).toEqual(true); + const passwordIncorrectLoadingTask = getDocument( buildGetDocumentParams(filename, { password: "qwerty", }) ); + expect( + passwordIncorrectLoadingTask instanceof PDFDocumentLoadingTask + ).toEqual(true); let passwordNeededDestroyed; passwordNeededLoadingTask.onPassword = function (callback, reason) { @@ -371,6 +400,7 @@ describe("api", function () { it("creates pdf doc from empty typed array", async function () { const loadingTask = getDocument(new Uint8Array(0)); + expect(loadingTask instanceof PDFDocumentLoadingTask).toEqual(true); try { await loadingTask.promise; @@ -389,10 +419,12 @@ describe("api", function () { it("checks that `docId`s are unique and increasing", async function () { const loadingTask1 = getDocument(basicApiGetDocumentParams); + expect(loadingTask1 instanceof PDFDocumentLoadingTask).toEqual(true); await loadingTask1.promise; const docId1 = loadingTask1.docId; const loadingTask2 = getDocument(basicApiGetDocumentParams); + expect(loadingTask2 instanceof PDFDocumentLoadingTask).toEqual(true); await loadingTask2.promise; const docId2 = loadingTask2.docId; From d854352cd5b52ae8012c5d7d90b59c9358f733af Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Mon, 13 Sep 2021 13:34:37 +0200 Subject: [PATCH 2/3] Improve the API unit-tests by checking that `PDFPageProxy.render` returns a `RenderTask`-instance This is similar to existing unit-tests, which checks for `PDFDocumentProxy`- and `PDFPageProxy`-instances. --- src/display/api.js | 1 + test/unit/api_spec.js | 18 +++++++++++++++++- 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/src/display/api.js b/src/display/api.js index f9b211f757082..0ae803035cc6d 100644 --- a/src/display/api.js +++ b/src/display/api.js @@ -3365,6 +3365,7 @@ export { PDFDocumentProxy, PDFPageProxy, PDFWorker, + RenderTask, setPDFNetworkStreamFactory, version, }; diff --git a/test/unit/api_spec.js b/test/unit/api_spec.js index 90faacc53e6e6..8dfb7eb856409 100644 --- a/test/unit/api_spec.js +++ b/test/unit/api_spec.js @@ -39,6 +39,7 @@ import { PDFDocumentProxy, PDFPageProxy, PDFWorker, + RenderTask, } from "../../src/display/api.js"; import { RenderingCancelledException, @@ -1874,11 +1875,14 @@ describe("api", function () { viewport.width, viewport.height ); + const renderTask = pdfPage.render({ canvasContext: canvasAndCtx.context, canvasFactory: CanvasFactory, viewport, }); + expect(renderTask instanceof RenderTask).toEqual(true); + await renderTask.promise; const stats = pdfPage.stats; @@ -1911,6 +1915,8 @@ describe("api", function () { canvasFactory: CanvasFactory, viewport, }); + expect(renderTask instanceof RenderTask).toEqual(true); + renderTask.cancel(); try { @@ -1939,6 +1945,8 @@ describe("api", function () { canvasFactory: CanvasFactory, viewport, }); + expect(renderTask instanceof RenderTask).toEqual(true); + renderTask.cancel(); try { @@ -1955,6 +1963,8 @@ describe("api", function () { canvasFactory: CanvasFactory, viewport, }); + expect(reRenderTask instanceof RenderTask).toEqual(true); + await reRenderTask.promise; CanvasFactory.destroy(canvasAndCtx); @@ -1976,12 +1986,15 @@ describe("api", function () { viewport, optionalContentConfigPromise, }); + expect(renderTask1 instanceof RenderTask).toEqual(true); + const renderTask2 = page.render({ canvasContext: canvasAndCtx.context, canvasFactory: CanvasFactory, viewport, optionalContentConfigPromise, }); + expect(renderTask2 instanceof RenderTask).toEqual(true); await Promise.all([ renderTask1.promise, @@ -2014,8 +2027,9 @@ describe("api", function () { canvasFactory: CanvasFactory, viewport, }); - await renderTask.promise; + expect(renderTask instanceof RenderTask).toEqual(true); + await renderTask.promise; await pdfDoc.cleanup(); expect(true).toEqual(true); @@ -2042,6 +2056,8 @@ describe("api", function () { canvasFactory: CanvasFactory, viewport, }); + expect(renderTask instanceof RenderTask).toEqual(true); + // Ensure that clean-up runs during rendering. renderTask.onContinue = function (cont) { waitSome(cont); From 95057a4e56cd04ecf4248afcc7e82a303d36c5bb Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Mon, 13 Sep 2021 13:43:16 +0200 Subject: [PATCH 3/3] Try to expose more API-functionality in the TypeScript definitions While these types apparently makes sense in TypeScript environments, we really don't want to extend the *public* API by simply exporting the relevant classes directly in `src/pdf.js` (since they should never be called/initialized manually). Please see e.g. issue 12384 where this was first requested, and note that a possible work-around was also provided there. This patch simply implements that work-around[1], which will hopefully be helpful to TypeScript users. --- [1] Based on the discussion in PR 13957, the two previous patches appear to be necessary for this to actually work. --- src/pdf.js | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/pdf.js b/src/pdf.js index 364114c66675f..3a4b38e0f3ca6 100644 --- a/src/pdf.js +++ b/src/pdf.js @@ -14,6 +14,12 @@ */ /* eslint-disable sort-exports/sort-exports */ +// eslint-disable-next-line max-len +/** @typedef {import("./display/api").PDFDocumentLoadingTask} PDFDocumentLoadingTask */ +/** @typedef {import("./display/api").PDFDocumentProxy} PDFDocumentProxy */ +/** @typedef {import("./display/api").PDFPageProxy} PDFPageProxy */ +/** @typedef {import("./display/api").RenderTask} RenderTask */ + import { addLinkAttributes, getFilenameFromUrl,