fix(http): manage different body types for caching POST requests (ang…

…ular#54980)

This update enhances the encoding handling of request bodies to generate the necessary cache key for transfer cache functionality.

Closes angular#54956

PR Close angular#54980

packages/common/http/src/request.ts

@@ -362,11 +362,11 @@ export class HttpRequest<T> {
     // Check whether the body is already in a serialized form. If so,
     // it can just be returned directly.
     if (
+      typeof this.body === 'string' ||
       isArrayBuffer(this.body) ||
       isBlob(this.body) ||
       isFormData(this.body) ||
-      isUrlSearchParams(this.body) ||
-      typeof this.body === 'string'
+      isUrlSearchParams(this.body)
     ) {
       return this.body;
     }

packages/common/http/src/transfer_cache.ts

@@ -28,6 +28,7 @@ import {HttpHeaders} from './headers';
 import {HTTP_ROOT_INTERCEPTOR_FNS, HttpHandlerFn} from './interceptor';
 import {HttpRequest} from './request';
 import {HttpEvent, HttpResponse} from './response';
+import {HttpParams} from './params';
 
 /**
  * Options to configure how TransferCache should be used to cache requests made via HttpClient.
@@ -198,17 +199,26 @@ function getFilteredHeaders(
   return headersMap;
 }
 
-function makeCacheKey(request: HttpRequest<any>): StateKey<TransferHttpResponse> {
-  // make the params encoded same as a url so it's easy to identify
-  const {params, method, responseType, url, body} = request;
-  const encodedParams = params
-    .keys()
+function sortAndConcatParams(params: HttpParams | URLSearchParams): string {
+  return [...params.keys()]
     .sort()
     .map((k) => `${k}=${params.getAll(k)}`)
     .join('&');
-  const strBody = typeof body === 'string' ? body : '';
-  const key = [method, responseType, url, strBody, encodedParams].join('|');
+}
+
+function makeCacheKey(request: HttpRequest<any>): StateKey<TransferHttpResponse> {
+  // make the params encoded same as a url so it's easy to identify
+  const {params, method, responseType, url} = request;
+  const encodedParams = sortAndConcatParams(params);
+
+  let serializedBody = request.serializeBody();
+  if (serializedBody instanceof URLSearchParams) {
+    serializedBody = sortAndConcatParams(serializedBody);
+  } else if (typeof serializedBody !== 'string') {
+    serializedBody = '';
+  }
 
+  const key = [method, responseType, url, serializedBody, encodedParams].join('|');
   const hash = generateHash(key);
 
   return makeStateKey(hash);

packages/common/http/test/transfer_cache_spec.ts

@@ -30,29 +30,42 @@ interface RequestParams {
   observe?: 'body' | 'response';
   transferCache?: {includeHeaders: string[]} | boolean;
   headers?: {[key: string]: string};
+  body?: RequestBody;
 }
 
+type RequestBody =
+  | ArrayBuffer
+  | Blob
+  | boolean
+  | string
+  | number
+  | Object
+  | (boolean | string | number | Object | null)[]
+  | null;
+
 describe('TransferCache', () => {
   @Component({selector: 'test-app-http', template: 'hello'})
   class SomeComponent {}
 
   describe('withHttpTransferCache', () => {
     let isStable: BehaviorSubject<boolean>;
 
-    function makeRequestAndExpectOne(url: string, body: string, params?: RequestParams): string;
     function makeRequestAndExpectOne(
       url: string,
-      body: string,
+      body: RequestBody,
+      params?: RequestParams,
+    ): string;
+    function makeRequestAndExpectOne(
+      url: string,
+      body: RequestBody,
       params?: RequestParams & {observe: 'response'},
     ): HttpResponse<string>;
-    function makeRequestAndExpectOne(url: string, body: string, params?: RequestParams): any {
+    function makeRequestAndExpectOne(url: string, body: RequestBody, params?: RequestParams): any {
       let response!: any;
       TestBed.inject(HttpClient)
         .request(params?.method ?? 'GET', url, params)
         .subscribe((r) => (response = r));
-      TestBed.inject(HttpTestingController)
-        .expectOne(url)
-        .flush(body, {headers: params?.headers});
+      TestBed.inject(HttpTestingController).expectOne(url).flush(body, {headers: params?.headers});
       return response;
     }
 
@@ -264,6 +277,43 @@ describe('TransferCache', () => {
       makeRequestAndExpectOne('/test-auth', 'foo');
     });
 
+    it('should cache POST with the differing body in string form', () => {
+      makeRequestAndExpectOne('/test-1', null, {method: 'POST', transferCache: true, body: 'foo'});
+      makeRequestAndExpectNone('/test-1', 'POST', {transferCache: true, body: 'foo'});
+      makeRequestAndExpectOne('/test-1', null, {method: 'POST', transferCache: true, body: 'bar'});
+    });
+
+    it('should cache POST with the differing body in object form', () => {
+      makeRequestAndExpectOne('/test-1', null, {
+        method: 'POST',
+        transferCache: true,
+        body: {foo: true},
+      });
+      makeRequestAndExpectNone('/test-1', 'POST', {transferCache: true, body: {foo: true}});
+      makeRequestAndExpectOne('/test-1', null, {
+        method: 'POST',
+        transferCache: true,
+        body: {foo: false},
+      });
+    });
+
+    it('should cache POST with the differing body in URLSearchParams form', () => {
+      makeRequestAndExpectOne('/test-1', null, {
+        method: 'POST',
+        transferCache: true,
+        body: new URLSearchParams('foo=1'),
+      });
+      makeRequestAndExpectNone('/test-1', 'POST', {
+        transferCache: true,
+        body: new URLSearchParams('foo=1'),
+      });
+      makeRequestAndExpectOne('/test-1', null, {
+        method: 'POST',
+        transferCache: true,
+        body: new URLSearchParams('foo=2'),
+      });
+    });
+
     describe('caching with global setting', () => {
       beforeEach(
         withBody('<test-app-http></test-app-http>', () => {

packages/core/test/bundling/hydration/bundle.golden_symbols.json

@@ -1295,6 +1295,9 @@
   {
     "name": "siblingAfter"
   },
+  {
+    "name": "sortAndConcatParams"
+  },
   {
     "name": "storeLViewOnDestroy"
   },