From 2ac7b98ed2e2308939622ffe43c97c27c9dd3dea Mon Sep 17 00:00:00 2001 From: Asad Ali Date: Mon, 8 Jul 2024 12:00:24 +0500 Subject: [PATCH 1/6] feat: added support for handling empty responses with nullable types --- packages/core/src/http/requestBuilder.ts | 11 ++- .../core/test/http/requestBuilder.test.ts | 90 ++++++++++++++++++- 2 files changed, 94 insertions(+), 7 deletions(-) diff --git a/packages/core/src/http/requestBuilder.ts b/packages/core/src/http/requestBuilder.ts index 4cfa4ad7..b5d93570 100644 --- a/packages/core/src/http/requestBuilder.ts +++ b/packages/core/src/http/requestBuilder.ts @@ -500,9 +500,14 @@ export class DefaultRequestBuilder }); const result = await this.call(requestOptions); if (result.body === '') { - throw new Error( - 'Could not parse body as JSON. The response body is empty.' - ); + // Try mapping the missing body as null + const nullMappingResult = validateAndMap(null, schema); + if (nullMappingResult.errors) { + throw new Error( + 'Could not parse body as JSON. The response body is empty.' + ); + } + return { ...result, result: nullMappingResult.result }; } if (typeof result.body !== 'string') { throw new Error( diff --git a/packages/core/test/http/requestBuilder.test.ts b/packages/core/test/http/requestBuilder.test.ts index 3a8c71c1..112bc30e 100644 --- a/packages/core/test/http/requestBuilder.test.ts +++ b/packages/core/test/http/requestBuilder.test.ts @@ -1,5 +1,6 @@ import { HttpClientInterface, + RequestBuilder, createRequestBuilderFactory, skipEncode, } from '../../src/http/requestBuilder'; @@ -24,6 +25,7 @@ import { FileWrapper } from '../../src/fileWrapper'; import fs from 'fs'; import path from 'path'; import { bossSchema } from '../../../schema/test/bossSchema'; +import { boolean, nullable, optional } from '@apimatic/schema/src'; describe('test default request builder behavior with succesful responses', () => { const authParams = { @@ -420,7 +422,7 @@ describe('test default request builder behavior with succesful responses', () => await reqBuilder.callAsJson(employeeSchema); } catch (error) { const expectedResult = - "Could not parse body as JSON.\n\nExpected 'r' instead of 'e'"; + 'Could not parse body as JSON.\n\nExpected \'r\' instead of \'e\''; expect(error.message).toEqual(expectedResult); } }); @@ -473,6 +475,81 @@ describe('test default request builder behavior with succesful responses', () => expect(error.message).toEqual(`Response status code was not ok: 400.`); } }); + it('should test request builder with 400 response code', async () => { + try { + const reqBuilder = defaultRequestBuilder( + 'GET', + '/test/requestBuilder/errorResponse' + ); + reqBuilder.baseUrl('default'); + await reqBuilder.callAsText(); + } catch (error) { + expect(error.message).toEqual(`Response status code was not ok: 400.`); + } + }); + it('should test response with no content textual types', async () => { + const reqBuilder = customRequestBuilder({ + statusCode: 204, + body: '', + headers: {}, + }); + const { result } = await reqBuilder.callAsText(); + expect(result).toEqual(''); + }); + it('should test response with no content string cases', async () => { + const reqBuilder = customRequestBuilder({ + statusCode: 204, + body: '', + headers: {}, + }); + const nullableString = await reqBuilder.callAsJson(nullable(string())); + expect(nullableString.result).toEqual(null); + + const optionalString = await reqBuilder.callAsJson(optional(string())); + expect(optionalString.result).toEqual(undefined); + }); + it('should test response with no content boolean cases', async () => { + const reqBuilder = customRequestBuilder({ + statusCode: 204, + body: '', + headers: {}, + }); + const nullableString = await reqBuilder.callAsJson(nullable(boolean())); + expect(nullableString.result).toEqual(null); + + const optionalString = await reqBuilder.callAsJson(optional(boolean())); + expect(optionalString.result).toEqual(undefined); + }); + it('should test response with no content object cases', async () => { + const reqBuilder = customRequestBuilder({ + statusCode: 204, + body: '', + headers: {}, + }); + const nullableString = await reqBuilder.callAsJson( + nullable(employeeSchema) + ); + expect(nullableString.result).toEqual(null); + + const optionalString = await reqBuilder.callAsJson( + optional(employeeSchema) + ); + expect(optionalString.result).toEqual(undefined); + }); + + function customRequestBuilder( + response: HttpResponse + ): RequestBuilder { + const reqBuilder = createRequestBuilderFactory( + mockHttpClientAdapter(response), + (server) => mockBaseURIProvider(server), + ApiError, + basicAuth, + retryConfig + )('GET', '/test/requestBuilder'); + reqBuilder.baseUrl('default'); + return reqBuilder; + } function mockBasicAuthenticationInterface({ username, @@ -497,16 +574,21 @@ describe('test default request builder behavior with succesful responses', () => }; } - function mockHttpClientAdapter(): HttpClientInterface { + function mockHttpClientAdapter( + customResponse?: HttpResponse + ): HttpClientInterface { return async (request, requestOptions) => { + if (typeof customResponse !== 'undefined') { + return customResponse; + } const iserrorResponse = request.url.startsWith( 'http://apimatic.hopto.org:3000/test/requestBuilder/errorResponse' ); if (iserrorResponse) { - return await mockErrorResponse(request, requestOptions); + return mockErrorResponse(request, requestOptions); } - return await mockResponse(request, requestOptions); + return mockResponse(request, requestOptions); }; } From 28867f14447a5e8ffe68510b31d9a44b348801d0 Mon Sep 17 00:00:00 2001 From: Asad Ali Date: Mon, 8 Jul 2024 12:13:49 +0500 Subject: [PATCH 2/6] fix: adds whitespace check along with no content check --- packages/core/src/http/requestBuilder.ts | 12 ++--- .../core/test/http/requestBuilder.test.ts | 49 +++++++++++++++++++ 2 files changed, 55 insertions(+), 6 deletions(-) diff --git a/packages/core/src/http/requestBuilder.ts b/packages/core/src/http/requestBuilder.ts index b5d93570..9d489085 100644 --- a/packages/core/src/http/requestBuilder.ts +++ b/packages/core/src/http/requestBuilder.ts @@ -499,7 +499,12 @@ export class DefaultRequestBuilder return { ...request, headers }; }); const result = await this.call(requestOptions); - if (result.body === '') { + if (typeof result.body !== 'string') { + throw new Error( + 'Could not parse body as JSON. The response body is not a string.' + ); + } + if (result.body.toString().trim() === '') { // Try mapping the missing body as null const nullMappingResult = validateAndMap(null, schema); if (nullMappingResult.errors) { @@ -509,11 +514,6 @@ export class DefaultRequestBuilder } return { ...result, result: nullMappingResult.result }; } - if (typeof result.body !== 'string') { - throw new Error( - 'Could not parse body as JSON. The response body is not a string.' - ); - } let parsed: unknown; try { parsed = JSON.parse(result.body); diff --git a/packages/core/test/http/requestBuilder.test.ts b/packages/core/test/http/requestBuilder.test.ts index 112bc30e..9f92cfae 100644 --- a/packages/core/test/http/requestBuilder.test.ts +++ b/packages/core/test/http/requestBuilder.test.ts @@ -496,6 +496,15 @@ describe('test default request builder behavior with succesful responses', () => const { result } = await reqBuilder.callAsText(); expect(result).toEqual(''); }); + it('should test response with whitespace content textual types', async () => { + const reqBuilder = customRequestBuilder({ + statusCode: 204, + body: ' ', + headers: {}, + }); + const { result } = await reqBuilder.callAsText(); + expect(result).toEqual(' '); + }); it('should test response with no content string cases', async () => { const reqBuilder = customRequestBuilder({ statusCode: 204, @@ -508,6 +517,18 @@ describe('test default request builder behavior with succesful responses', () => const optionalString = await reqBuilder.callAsJson(optional(string())); expect(optionalString.result).toEqual(undefined); }); + it('should test response with whitespace content string cases', async () => { + const reqBuilder = customRequestBuilder({ + statusCode: 204, + body: ' ', + headers: {}, + }); + const nullableString = await reqBuilder.callAsJson(nullable(string())); + expect(nullableString.result).toEqual(null); + + const optionalString = await reqBuilder.callAsJson(optional(string())); + expect(optionalString.result).toEqual(undefined); + }); it('should test response with no content boolean cases', async () => { const reqBuilder = customRequestBuilder({ statusCode: 204, @@ -520,6 +541,18 @@ describe('test default request builder behavior with succesful responses', () => const optionalString = await reqBuilder.callAsJson(optional(boolean())); expect(optionalString.result).toEqual(undefined); }); + it('should test response with whitespace content boolean cases', async () => { + const reqBuilder = customRequestBuilder({ + statusCode: 204, + body: ' ', + headers: {}, + }); + const nullableString = await reqBuilder.callAsJson(nullable(boolean())); + expect(nullableString.result).toEqual(null); + + const optionalString = await reqBuilder.callAsJson(optional(boolean())); + expect(optionalString.result).toEqual(undefined); + }); it('should test response with no content object cases', async () => { const reqBuilder = customRequestBuilder({ statusCode: 204, @@ -536,6 +569,22 @@ describe('test default request builder behavior with succesful responses', () => ); expect(optionalString.result).toEqual(undefined); }); + it('should test response with whitespace content object cases', async () => { + const reqBuilder = customRequestBuilder({ + statusCode: 204, + body: ' ', + headers: {}, + }); + const nullableString = await reqBuilder.callAsJson( + nullable(employeeSchema) + ); + expect(nullableString.result).toEqual(null); + + const optionalString = await reqBuilder.callAsJson( + optional(employeeSchema) + ); + expect(optionalString.result).toEqual(undefined); + }); function customRequestBuilder( response: HttpResponse From 3a035636601a5434f5eadb841e7546844dd2418a Mon Sep 17 00:00:00 2001 From: Asad Ali Date: Mon, 8 Jul 2024 12:14:49 +0500 Subject: [PATCH 3/6] refactor: remove unnecessary toString conversion --- packages/core/src/http/requestBuilder.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/core/src/http/requestBuilder.ts b/packages/core/src/http/requestBuilder.ts index 9d489085..becc0c97 100644 --- a/packages/core/src/http/requestBuilder.ts +++ b/packages/core/src/http/requestBuilder.ts @@ -504,7 +504,7 @@ export class DefaultRequestBuilder 'Could not parse body as JSON. The response body is not a string.' ); } - if (result.body.toString().trim() === '') { + if (result.body.trim() === '') { // Try mapping the missing body as null const nullMappingResult = validateAndMap(null, schema); if (nullMappingResult.errors) { From a00e00d1bfe7c741c3ea88fb64417eb390d0449e Mon Sep 17 00:00:00 2001 From: Asad Ali Date: Mon, 8 Jul 2024 12:28:54 +0500 Subject: [PATCH 4/6] refactor: extracted a function --- packages/core/src/http/requestBuilder.ts | 22 +++++++++++++++------- 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/packages/core/src/http/requestBuilder.ts b/packages/core/src/http/requestBuilder.ts index becc0c97..d22421a4 100644 --- a/packages/core/src/http/requestBuilder.ts +++ b/packages/core/src/http/requestBuilder.ts @@ -506,13 +506,7 @@ export class DefaultRequestBuilder } if (result.body.trim() === '') { // Try mapping the missing body as null - const nullMappingResult = validateAndMap(null, schema); - if (nullMappingResult.errors) { - throw new Error( - 'Could not parse body as JSON. The response body is empty.' - ); - } - return { ...result, result: nullMappingResult.result }; + return this.tryMappingAsNull(schema, result); } let parsed: unknown; try { @@ -526,6 +520,20 @@ export class DefaultRequestBuilder } return { ...result, result: mappingResult.result }; } + + private tryMappingAsNull( + schema: Schema, + result: ApiResponse + ) { + const nullMappingResult = validateAndMap(null, schema); + if (nullMappingResult.errors) { + throw new Error( + 'Could not parse body as JSON. The response body is empty.' + ); + } + return { ...result, result: nullMappingResult.result }; + } + public async callAsXml( rootName: string, schema: Schema, From d0289f56ffa6fc9d50f41e662941eee0c296df3f Mon Sep 17 00:00:00 2001 From: Asad Ali Date: Mon, 8 Jul 2024 12:34:01 +0500 Subject: [PATCH 5/6] refactor: remove code duplication issues --- .../core/test/http/requestBuilder.test.ts | 58 ++++++------------- 1 file changed, 18 insertions(+), 40 deletions(-) diff --git a/packages/core/test/http/requestBuilder.test.ts b/packages/core/test/http/requestBuilder.test.ts index 9f92cfae..1a5a7f9b 100644 --- a/packages/core/test/http/requestBuilder.test.ts +++ b/packages/core/test/http/requestBuilder.test.ts @@ -41,6 +41,16 @@ describe('test default request builder behavior with succesful responses', () => httpStatusCodesToRetry: [408, 413, 429, 500, 502, 503, 504, 521, 522, 524], httpMethodsToRetry: ['GET', 'PUT'] as HttpMethod[], }; + const noContentResponse: HttpResponse = { + statusCode: 204, + body: '', + headers: {}, + }; + const whitespacedResponse: HttpResponse = { + statusCode: 204, + body: ' ', + headers: {}, + }; const basicAuth = mockBasicAuthenticationInterface(authParams); const defaultRequestBuilder = createRequestBuilderFactory( mockHttpClientAdapter(), @@ -488,29 +498,17 @@ describe('test default request builder behavior with succesful responses', () => } }); it('should test response with no content textual types', async () => { - const reqBuilder = customRequestBuilder({ - statusCode: 204, - body: '', - headers: {}, - }); + const reqBuilder = customRequestBuilder(noContentResponse); const { result } = await reqBuilder.callAsText(); expect(result).toEqual(''); }); it('should test response with whitespace content textual types', async () => { - const reqBuilder = customRequestBuilder({ - statusCode: 204, - body: ' ', - headers: {}, - }); + const reqBuilder = customRequestBuilder(whitespacedResponse); const { result } = await reqBuilder.callAsText(); expect(result).toEqual(' '); }); it('should test response with no content string cases', async () => { - const reqBuilder = customRequestBuilder({ - statusCode: 204, - body: '', - headers: {}, - }); + const reqBuilder = customRequestBuilder(noContentResponse); const nullableString = await reqBuilder.callAsJson(nullable(string())); expect(nullableString.result).toEqual(null); @@ -518,11 +516,7 @@ describe('test default request builder behavior with succesful responses', () => expect(optionalString.result).toEqual(undefined); }); it('should test response with whitespace content string cases', async () => { - const reqBuilder = customRequestBuilder({ - statusCode: 204, - body: ' ', - headers: {}, - }); + const reqBuilder = customRequestBuilder(whitespacedResponse); const nullableString = await reqBuilder.callAsJson(nullable(string())); expect(nullableString.result).toEqual(null); @@ -530,11 +524,7 @@ describe('test default request builder behavior with succesful responses', () => expect(optionalString.result).toEqual(undefined); }); it('should test response with no content boolean cases', async () => { - const reqBuilder = customRequestBuilder({ - statusCode: 204, - body: '', - headers: {}, - }); + const reqBuilder = customRequestBuilder(noContentResponse); const nullableString = await reqBuilder.callAsJson(nullable(boolean())); expect(nullableString.result).toEqual(null); @@ -542,11 +532,7 @@ describe('test default request builder behavior with succesful responses', () => expect(optionalString.result).toEqual(undefined); }); it('should test response with whitespace content boolean cases', async () => { - const reqBuilder = customRequestBuilder({ - statusCode: 204, - body: ' ', - headers: {}, - }); + const reqBuilder = customRequestBuilder(whitespacedResponse); const nullableString = await reqBuilder.callAsJson(nullable(boolean())); expect(nullableString.result).toEqual(null); @@ -554,11 +540,7 @@ describe('test default request builder behavior with succesful responses', () => expect(optionalString.result).toEqual(undefined); }); it('should test response with no content object cases', async () => { - const reqBuilder = customRequestBuilder({ - statusCode: 204, - body: '', - headers: {}, - }); + const reqBuilder = customRequestBuilder(noContentResponse); const nullableString = await reqBuilder.callAsJson( nullable(employeeSchema) ); @@ -570,11 +552,7 @@ describe('test default request builder behavior with succesful responses', () => expect(optionalString.result).toEqual(undefined); }); it('should test response with whitespace content object cases', async () => { - const reqBuilder = customRequestBuilder({ - statusCode: 204, - body: ' ', - headers: {}, - }); + const reqBuilder = customRequestBuilder(whitespacedResponse); const nullableString = await reqBuilder.callAsJson( nullable(employeeSchema) ); From 8a024f0a5f929c3dcf64dced852a6ef3d7ab59be Mon Sep 17 00:00:00 2001 From: Asad Ali Date: Mon, 8 Jul 2024 12:41:45 +0500 Subject: [PATCH 6/6] refactor: fix formatting issues --- packages/core/test/http/requestBuilder.test.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/core/test/http/requestBuilder.test.ts b/packages/core/test/http/requestBuilder.test.ts index 1a5a7f9b..2d3b0c4a 100644 --- a/packages/core/test/http/requestBuilder.test.ts +++ b/packages/core/test/http/requestBuilder.test.ts @@ -431,8 +431,7 @@ describe('test default request builder behavior with succesful responses', () => reqBuilder.validateResponse(false); await reqBuilder.callAsJson(employeeSchema); } catch (error) { - const expectedResult = - 'Could not parse body as JSON.\n\nExpected \'r\' instead of \'e\''; + const expectedResult = `Could not parse body as JSON.\n\nExpected 'r' instead of 'e'`; expect(error.message).toEqual(expectedResult); } });