fix: stop leaking optional query params and headers across subsequent…

… requests
nullcc · Feb 21, 2024 · 5214590 · 5214590
1 parent 9383ccb
commit 5214590
Show file tree

Hide file tree

Showing 4 changed files with 129 additions and 7 deletions.
diff --git a/README.md b/README.md
@@ -5,7 +5,7 @@
 
 | Statements                               | Branches                                 | Functions                                | Lines                               |
 | ---------------------------------------- | ---------------------------------------- | ---------------------------------------- | ----------------------------------- |
-| ![Statements](https://img.shields.io/badge/Coverage-93.98%25-brightgreen.svg "Make me better!") | ![Branches](https://img.shields.io/badge/Coverage-79.42%25-red.svg "Make me better!") | ![Functions](https://img.shields.io/badge/Coverage-87.58%25-yellow.svg "Make me better!") | ![Lines](https://img.shields.io/badge/Coverage-95.32%25-brightgreen.svg "Make me better!") |
+| ![Statements](https://img.shields.io/badge/Coverage-93.98%25-brightgreen.svg "Make me better!") | ![Branches](https://img.shields.io/badge/Coverage-80.14%25-yellow.svg "Make me better!") | ![Functions](https://img.shields.io/badge/Coverage-87.58%25-yellow.svg "Make me better!") | ![Lines](https://img.shields.io/badge/Coverage-95.32%25-brightgreen.svg "Make me better!") |
 
 > A declarative and [axios](https://github.com/axios/axios) based retrofit implementation for JavaScript and TypeScript.
 

diff --git a/src/baseService.ts b/src/baseService.ts
@@ -243,17 +243,17 @@ export class BaseService {
   @nonHTTPRequestMethod
   private _resolveHeaders(methodName: string, args: any[]): any {
     const meta = this.__meta__;
-    const headers = meta[methodName].headers || {};
+    const headers = { ...meta[methodName].headers };
     const headerParams = meta[methodName].headerParams;
     for (const pos in headerParams) {
-      if (headerParams[pos]) {
+      if (headerParams[pos] && args[pos] !== undefined) {
         headers[headerParams[pos]] = args[pos];
       }
     }
     const headerMapIndex = meta[methodName].headerMapIndex;
     if (headerMapIndex >= 0) {
       for (const key in args[headerMapIndex]) {
-        if (args[headerMapIndex][key]) {
+        if (args[headerMapIndex][key] !== undefined) {
           headers[key] = args[headerMapIndex][key];
         }
       }
@@ -264,17 +264,17 @@ export class BaseService {
   @nonHTTPRequestMethod
   private _resolveQuery(methodName: string, args: any[]): any {
     const meta = this.__meta__;
-    const query = meta[methodName].query || {};
+    const query = { ...meta[methodName].query };
     const queryParams = meta[methodName].queryParams;
     for (const pos in queryParams) {
-      if (queryParams[pos]) {
+      if (queryParams[pos] && args[pos] !== undefined) {
         query[queryParams[pos]] = args[pos];
       }
     }
     const queryMapIndex = meta[methodName].queryMapIndex;
     if (queryMapIndex >= 0) {
       for (const key in args[queryMapIndex]) {
-        if (args[queryMapIndex][key]) {
+        if (args[queryMapIndex][key] !== undefined) {
           query[key] = args[queryMapIndex][key];
         }
       }

diff --git a/test/fixture/fixtures.ts b/test/fixture/fixtures.ts
@@ -132,6 +132,34 @@ export class PostService extends BaseService {
   @QueryArrayFormat('comma')
   async getPostsWithQueryArrayFormatComma(@Query('groups') groups: string[]): Promise<Response> { return <Response>{} };
 
+  @GET("/posts")
+  @Queries({
+    page: 1,
+    size: 20,
+    sort: "createdAt:desc",
+  })
+  async getPostsWithOptionalQuery(@Query('since') since?: string): Promise<Response> { return <Response>{} };
+
+  @GET("/posts")
+  @Queries({
+    page: 1,
+    size: 20,
+    sort: "createdAt:desc",
+  })
+  async getPostsWithOptionalQueryMap(@QueryMap filters?: SearchQuery): Promise<Response> { return <Response>{} };
+
+  @GET("/posts")
+  @Headers({
+    'Cache-Control': 'no-cache'
+  })
+  async getPostsWithOptionalHeader(@Header('X-Correlation-Id') correlationId?: string): Promise<Response> { return <Response>{} };
+
+  @GET("/posts")
+  @Headers({
+    'Cache-Control': 'no-cache'
+  })
+  async getPostsWithOptionalHeaderMap(@HeaderMap headers?: any): Promise<Response> { return <Response>{} };
+
   @POST("/posts")
   @FormUrlEncoded
   async createPost(@Field("title") title: string, @Field("content") content: string): Promise<Response> { return <Response>{} };

diff --git a/test/ts-retrofit.test.ts b/test/ts-retrofit.test.ts
@@ -248,6 +248,100 @@ describe("Test ts-retrofit.", () => {
     expect(response.config.params).toMatchObject(query);
   });
 
+  test("Test `@Query` decorator should not persist optional param across requests.", async () => {
+    const postsService = new ServiceBuilder()
+      .setEndpoint(TEST_SERVER_ENDPOINT)
+      .build(PostService);
+    const since1 = "2022-12-01"
+    const since3 = "2020-10-31"
+    const response1 = await postsService.getPostsWithOptionalQuery(since1);
+    const response2 = await postsService.getPostsWithOptionalQuery();
+    const response3 = await postsService.getPostsWithOptionalQuery(since3);
+    console.log(response1.config.params)
+    console.log(response2.config.params)
+    console.log(response3.config.params)
+    expect(response1.config.params.since).toEqual(since1);
+
+    expect(response2.config.params.since).toBeUndefined();
+    expect(Object.keys(response2.config.params)).not.toContain('since');
+
+    expect(response3.config.params.since).toEqual(since3);
+  });
+
+  test("Test `@QueryMap` decorator should not persist optional params across requests.", async () => {
+    const postsService = new ServiceBuilder()
+      .setEndpoint(TEST_SERVER_ENDPOINT)
+      .build(PostService);
+    const query1: SearchQuery = {
+      title: "TypeScript",
+    };
+    const query3: SearchQuery = {
+      author: "John Doe",
+    };
+    const response1 = await postsService.getPostsWithOptionalQueryMap(query1);
+    const response2 = await postsService.getPostsWithOptionalQueryMap();
+    const response3 = await postsService.getPostsWithOptionalQueryMap(query3);
+
+    expect(response1.config.params.title).toEqual(query1.title);
+    expect(response1.config.params.author).toBeUndefined();
+    expect(Object.keys(response1.config.params)).not.toContain('author');
+
+    expect(response2.config.params.title).toBeUndefined();
+    expect(Object.keys(response2.config.params)).not.toContain('title');
+    expect(response2.config.params.author).toBeUndefined();
+    expect(Object.keys(response2.config.params)).not.toContain('author');
+
+    expect(response3.config.params.title).toBeUndefined();
+    expect(Object.keys(response3.config.params)).not.toContain('title');
+    expect(response3.config.params.author).toEqual(query3.author);
+  });
+
+  test("Test `@Header` decorator should not persist optional header across requests.", async () => {
+    const postsService = new ServiceBuilder()
+      .setEndpoint(TEST_SERVER_ENDPOINT)
+      .build(PostService);
+    const correlationId1 = "1234abcd"
+    const correlationId3 = "9876zyxw"
+    const response1 = await postsService.getPostsWithOptionalHeader(correlationId1);
+    const response2 = await postsService.getPostsWithOptionalHeader();
+    const response3 = await postsService.getPostsWithOptionalHeader(correlationId3);
+
+    expect(response1.config.headers['X-Correlation-Id']).toEqual(correlationId1);
+
+    expect(response2.config.headers['X-Correlation-Id']).toBeUndefined();
+    expect(Object.keys(response2.config.headers)).not.toContain('X-Correlation-Id');
+
+    expect(response3.config.headers['X-Correlation-Id']).toEqual(correlationId3);
+  });
+
+  test("Test `@HeaderMap` decorator should not persist optional headers across requests.", async () => {
+    const postsService = new ServiceBuilder()
+      .setEndpoint(TEST_SERVER_ENDPOINT)
+      .build(PostService);
+    const headers1 = {
+      'X-Session-Id': "1234abcd"
+    };
+    const headers3 = {
+      'X-Correlation-Id': "9876zyxw",
+    };
+    const response1 = await postsService.getPostsWithOptionalHeaderMap(headers1);
+    const response2 = await postsService.getPostsWithOptionalHeaderMap();
+    const response3 = await postsService.getPostsWithOptionalHeaderMap(headers3);
+
+    expect(response1.config.headers['X-Session-Id']).toEqual(headers1['X-Session-Id']);
+    expect(response1.config.headers['X-Correlation-Id']).toBeUndefined();
+    expect(Object.keys(response1.config.headers)).not.toContain('X-Correlation-Id');
+
+    expect(response2.config.headers['X-Session-Id']).toBeUndefined();
+    expect(Object.keys(response2.config.headers)).not.toContain('X-Session-Id');
+    expect(response2.config.headers['X-Correlation-Id']).toBeUndefined();
+    expect(Object.keys(response2.config.headers)).not.toContain('X-Correlation-Id');
+
+    expect(response3.config.headers['X-Session-Id']).toBeUndefined();
+    expect(Object.keys(response3.config.headers)).not.toContain('X-Session-Id');
+    expect(response3.config.headers['X-Correlation-Id']).toEqual(headers3['X-Correlation-Id']);
+  });
+
   test("Test `@FormUrlEncoded` decorator.", async () => {
     const postService = new ServiceBuilder()
       .setEndpoint(TEST_SERVER_ENDPOINT)