Merge pull request #2916 from patrick-rodgers/version-3

updates to graph batch parsing

packages/graph/batching.ts

@@ -1,4 +1,4 @@
-import { isUrlAbsolute, hOP, TimelinePipe, getGUID, CopyFrom, objectDefinedNotNull, isFunc, combine } from "@pnp/core";
+import { isUrlAbsolute, hOP, TimelinePipe, getGUID, CopyFrom, objectDefinedNotNull, isFunc, combine, jsS } from "@pnp/core";
 import { parseBinderWithErrorCheck, Queryable, body, InjectHeaders } from "@pnp/queryable";
 import { IGraphQueryable, _GraphQueryable } from "./graphqueryable.js";
 import { graphPost } from "./operations.js";
@@ -48,9 +48,7 @@ interface IGraphBatchResponseFragment {
     statusText?: string;
     method: string;
     url: string;
-    headers?: string[][] | {
-        [key: string]: string;
-    };
+    headers?: [string, string][] | Record<string, string>;
     body?: any;
 }
 
@@ -373,36 +371,60 @@ function formatRequests(requests: RequestRecord[], batchId: string): IGraphBatch
     });
 }
 
-function parseResponse(graphResponse: IGraphBatchResponse): Promise<ParsedGraphResponse> {
+function parseResponse(graphResponse: IGraphBatchResponse): ParsedGraphResponse {
 
-    return new Promise((resolve, reject) => {
+    // we need to see if we have an error and report that
+    if (hOP(graphResponse, "error")) {
+        throw Error(`Error Porcessing Batch: (${graphResponse.error.code}) ${graphResponse.error.message}`);
+    }
 
-        // we need to see if we have an error and report that
-        if (hOP(graphResponse, "error")) {
-            return reject(Error(`Error Porcessing Batch: (${graphResponse.error.code}) ${graphResponse.error.message}`));
-        }
+    const parsedResponses: Response[] = new Array(graphResponse.responses.length).fill(null);
 
-        const parsedResponses: Response[] = new Array(graphResponse.responses.length).fill(null);
+    for (let i = 0; i < graphResponse.responses.length; ++i) {
 
-        for (let i = 0; i < graphResponse.responses.length; ++i) {
+        const response = graphResponse.responses[i];
+        // we create the request id by adding 1 to the index, so we place the response by subtracting one to match
+        // the array of requests and make it easier to map them by index
+        const responseId = parseInt(response.id, 10) - 1;
+        const contentType = response.headers["Content-Type"];
+        const { status, statusText, headers, body } = response;
+        const init = { status, statusText, headers };
 
-            const response = graphResponse.responses[i];
-            // we create the request id by adding 1 to the index, so we place the response by subtracting one to match
-            // the array of requests and make it easier to map them by index
-            const responseId = parseInt(response.id, 10) - 1;
 
-            if (response.status === 204) {
+        // this is to handle special cases before we pass to the default parsing logic
+        if (status === 204) {
 
-                parsedResponses[responseId] = new Response();
-            } else {
+            // this handles cases where the response body is empty and has a 204 response status (No Content)
+            parsedResponses[responseId] = new Response(null, init);
 
-                parsedResponses[responseId] = new Response(JSON.stringify(response.body), response);
-            }
+        } else if (status === 302) {
+
+            // this is the case where (probably) a file download was included in the batch and the service has returned a 302 redirect to that file
+            // the url should be in the response's location header, so we transform the response to a 200 with the location in the body as 302 will be an
+            // error in the default parser used on the individual request
+
+            init.status = 200;
+            // eslint-disable-next-line @typescript-eslint/dot-notation
+            parsedResponses[responseId] = new Response(jsS({ location: headers["Location"] || "" }), init);
+
+        } else if (status === 200 && /^image[\\|/]/i.test(contentType)) {
+
+            // this handles the case where image content is returned as base 64 data in the batch body, such as /me/photos/$value (https://github.com/pnp/pnpjs/issues/2825)
+
+            const encoder = new TextEncoder();
+            parsedResponses[responseId] = new Response(encoder.encode(body), init);
+
+        } else {
+
+            // this is the default case where we have a json body which we remake into a string for the downstream parser to parse again
+            // a bit circular, but this provides consistent behavior for downstream parsers
+
+            parsedResponses[responseId] = new Response(jsS(body), init);
         }
+    }
 
-        resolve({
-            nextLink: graphResponse.nextLink,
-            responses: parsedResponses,
-        });
-    });
+    return {
+        nextLink: graphResponse.nextLink,
+        responses: parsedResponses,
+    };
 }

packages/queryable/behaviors/parsers.ts

@@ -1,5 +1,5 @@
 import { Queryable } from "../queryable.js";
-import { hOP, TimelinePipe, parseToAtob } from "@pnp/core";
+import { hOP, TimelinePipe } from "@pnp/core";
 import { isFunc } from "@pnp/core";
 
 export function DefaultParse(): TimelinePipe {
@@ -25,21 +25,7 @@ export function TextParse(): TimelinePipe {
 
 export function BlobParse(): TimelinePipe {
 
-    return parseBinderWithErrorCheck( async (response) => {
-        const binaryResponseBody = parseToAtob(await response.text());
-        // handle batch responses for things that are base64, like photos https://github.com/pnp/pnpjs/issues/2825
-        if(binaryResponseBody){
-            // Create an array buffer from the binary string
-            const arrayBuffer = new ArrayBuffer(binaryResponseBody.length);
-            const uint8Array = new Uint8Array(arrayBuffer);
-            for (let i = 0; i < binaryResponseBody.length; i++) {
-                uint8Array[i] = binaryResponseBody.charCodeAt(i);
-            }
-            // Create a Blob from the array buffer
-            return new Blob([arrayBuffer], {type:response.headers.get("Content-Type")});
-        }
-        return response.blob();
-    });
+    return parseBinderWithErrorCheck(r => r.blob());
 }
 
 export function JSONParse(): TimelinePipe {
@@ -69,8 +55,7 @@ export function JSONHeaderParse(): TimelinePipe {
         // patch to handle cases of 200 response with no or whitespace only bodies (#487 & #545)
         const txt = await response.text();
         const json = txt.replace(/\s/ig, "").length > 0 ? JSON.parse(txt) : {};
-        const all = { data: { ...parseODataJSON(json) }, headers: { ...response.headers } };
-        return all;
+        return { data: { ...parseODataJSON(json) }, headers: { ...response.headers } };
     });
 }