Skip to content

Commit

Permalink
fix(http-negotiator): #2381 add additional warn log message
Browse files Browse the repository at this point in the history
  • Loading branch information
@lukaszzazulak
lukaszzazulak committed Jul 2, 2024
1 parent 8733b99 commit d0dbb7f
Show file tree
Hide file tree
Showing 2 changed files with 37 additions and 27 deletions.
32 changes: 20 additions & 12 deletions packages/http/src/mocker/negotiator/NegotiatorHelpers.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,18 +47,20 @@ type BodyNegotiationResult = Omit<IHttpNegotiationResult, 'headers'>;
const helpers = {
negotiateByPartialOptionsAndHttpContent(
{ code, exampleKey, dynamic }: NegotiatePartialOptions,
httpContent: IMediaTypeContent
httpContent: IMediaTypeContent,
logger: Logger
): E.Either<Error, BodyNegotiationResult> {
const { mediaType, schema } = httpContent;
if (exampleKey) {
return pipe(
findExampleByKey(httpContent, exampleKey),
E.fromOption(() =>
ProblemJsonError.fromTemplate(
E.fromOption(() => {
logger.warn(`Response for contentType: ${mediaType} and exampleKey: ${exampleKey} does not exist.`);
return ProblemJsonError.fromTemplate(
NOT_FOUND,
`Response for contentType: ${mediaType} and exampleKey: ${exampleKey} does not exist.`
)
),
);
}),
E.map(bodyExample => ({ code, mediaType, bodyExample, schema }))
);
} else if (dynamic) {
Expand Down Expand Up @@ -86,7 +88,8 @@ const helpers = {

negotiateDefaultMediaType(
partialOptions: NegotiatePartialOptions,
response: IHttpOperationResponse
response: IHttpOperationResponse,
logger: Logger
): E.Either<Error, IHttpNegotiationResult> {
const { code, dynamic, exampleKey } = partialOptions;
const { headers = [] } = response;
Expand Down Expand Up @@ -117,7 +120,7 @@ const helpers = {
}),
content =>
pipe(
helpers.negotiateByPartialOptionsAndHttpContent({ code, dynamic, exampleKey }, content),
helpers.negotiateByPartialOptionsAndHttpContent({ code, dynamic, exampleKey }, content, logger),
E.map(contentNegotiationResult => ({ headers, ...contentNegotiationResult }))
)
)
Expand Down Expand Up @@ -150,7 +153,8 @@ const helpers = {
dynamic,
exampleKey,
},
response
response,
logger
);
},
mediaTypes =>
Expand All @@ -165,9 +169,12 @@ const helpers = {
if (response.contents?.length && response.contents?.length > 0) {
return pipe(
createEmptyResponse(response.code, headers, mediaTypes),
E.fromOption<Error>(() => {
return ProblemJsonError.fromTemplate(NOT_ACCEPTABLE, `Unable to find content for ${mediaTypes}`);
})
E.fromOption<Error>(() => {
return ProblemJsonError.fromTemplate(
NOT_ACCEPTABLE,
`Unable to find content for ${mediaTypes}`
);
})
)}

// though accept header may have a request media type, the spec does not define a response body for the endpoint, so we essentially ignore the accept header (no error)
Expand All @@ -186,7 +193,8 @@ const helpers = {
dynamic,
exampleKey,
},
content
content,
logger
),
E.map(contentNegotiationResult => ({
headers,
Expand Down
32 changes: 17 additions & 15 deletions packages/http/src/mocker/negotiator/__tests__/NegotiatorHelpers.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ import { assertRight, assertLeft } from '@stoplight/prism-core/src/__tests__/uti
import helpers from '../NegotiatorHelpers';
import { IHttpNegotiationResult, NegotiationOptions } from '../types';
import { NonEmptyArray } from 'fp-ts/NonEmptyArray';
import * as pino from 'pino';
const logger = createLogger('TEST', { enabled: false });

const assertPayloadlessResponse = (actualResponse: E.Either<Error, IHttpNegotiationResult>) => {
Expand Down Expand Up @@ -545,7 +544,8 @@ describe('NegotiatorHelpers', () => {
dynamic: desiredOptions.dynamic,
exampleKey: desiredOptions.exampleKey,
},
contents
contents,
logger
);
expect(helpers.negotiateDefaultMediaType).not.toHaveBeenCalled();
assertRight(actualOperationConfig, operationConfig => {
Expand Down Expand Up @@ -742,7 +742,8 @@ describe('NegotiatorHelpers', () => {
dynamic: desiredOptions.dynamic,
exampleKey: desiredOptions.exampleKey,
},
httpResponseSchema
httpResponseSchema,
logger
);

assertRight(actualOperationConfig, operationConfig => {
Expand Down Expand Up @@ -788,7 +789,7 @@ describe('NegotiatorHelpers', () => {

jest.spyOn(helpers, 'negotiateByPartialOptionsAndHttpContent').mockReturnValue(E.right(fakeOperationConfig));

const actualOperationConfig = helpers.negotiateDefaultMediaType(partialOptions, response);
const actualOperationConfig = helpers.negotiateDefaultMediaType(partialOptions, response, logger);

expect(helpers.negotiateByPartialOptionsAndHttpContent).toHaveBeenCalledTimes(1);
expect(helpers.negotiateByPartialOptionsAndHttpContent).toHaveBeenCalledWith(
Expand All @@ -797,7 +798,8 @@ describe('NegotiatorHelpers', () => {
dynamic: partialOptions.dynamic,
exampleKey: partialOptions.exampleKey,
},
contents[1] // Check that the */* has been requested
contents[1], // Check that the */* has been requested
logger
);

assertRight(actualOperationConfig, operationConfig => {
Expand Down Expand Up @@ -828,7 +830,7 @@ describe('NegotiatorHelpers', () => {
};

it('returns text/plain with empty body', () => {
const negotiationResult = helpers.negotiateDefaultMediaType(partialOptions, response);
const negotiationResult = helpers.negotiateDefaultMediaType(partialOptions, response, logger);

assertRight(negotiationResult, result => {
expect(result).toEqual(expectedResponse);
Expand Down Expand Up @@ -858,7 +860,7 @@ describe('NegotiatorHelpers', () => {
examples: [{ id: faker.random.word(), key: 'hey', value: {} }],
},
],
});
}, logger);

it('should give json precedence', () => {
assertRight(negotiationResult, result => {
Expand All @@ -884,7 +886,7 @@ describe('NegotiatorHelpers', () => {
examples: [{ id: faker.random.word(), key: 'hey', value: {} }],
},
],
});
}, logger);

it('should take the first content type', () => {
assertRight(negotiationResult, result => {
Expand Down Expand Up @@ -917,7 +919,7 @@ describe('negotiateByPartialOptionsAndHttpContent()', () => {
encodings: [],
};

const actualOperationConfig = helpers.negotiateByPartialOptionsAndHttpContent(partialOptions, httpContent);
const actualOperationConfig = helpers.negotiateByPartialOptionsAndHttpContent(partialOptions, httpContent, logger);

const expectedConfig: Omit<IHttpNegotiationResult, 'headers'> = {
code: partialOptions.code,
Expand All @@ -944,7 +946,7 @@ describe('negotiateByPartialOptionsAndHttpContent()', () => {
encodings: [],
};

const negotiationResult = helpers.negotiateByPartialOptionsAndHttpContent(partialOptions, httpContent);
const negotiationResult = helpers.negotiateByPartialOptionsAndHttpContent(partialOptions, httpContent, logger);
assertLeft(negotiationResult, e => {
expect(e.message).toBe('The server cannot find the requested content');
});
Expand All @@ -965,7 +967,7 @@ describe('negotiateByPartialOptionsAndHttpContent()', () => {
encodings: [],
};

const actualOperationConfig = helpers.negotiateByPartialOptionsAndHttpContent(partialOptions, httpContent);
const actualOperationConfig = helpers.negotiateByPartialOptionsAndHttpContent(partialOptions, httpContent, logger);

assertRight(actualOperationConfig, operationConfig => {
expect(operationConfig).toEqual({
Expand All @@ -988,7 +990,7 @@ describe('negotiateByPartialOptionsAndHttpContent()', () => {
encodings: [],
};

const negotiationResult = helpers.negotiateByPartialOptionsAndHttpContent(partialOptions, httpContent);
const negotiationResult = helpers.negotiateByPartialOptionsAndHttpContent(partialOptions, httpContent, logger);

assertLeft(negotiationResult, e =>
expect(e.message).toBe(
Expand Down Expand Up @@ -1025,7 +1027,7 @@ describe('negotiateByPartialOptionsAndHttpContent()', () => {
encodings: [],
};

const actualOperationConfig = helpers.negotiateByPartialOptionsAndHttpContent(partialOptions, httpContent);
const actualOperationConfig = helpers.negotiateByPartialOptionsAndHttpContent(partialOptions, httpContent, logger);
const expectedConfig: Omit<IHttpNegotiationResult, 'headers'> = {
code: partialOptions.code,
mediaType: httpContent.mediaType,
Expand All @@ -1050,7 +1052,7 @@ describe('negotiateByPartialOptionsAndHttpContent()', () => {
encodings: [],
};

const actualOperationConfig = helpers.negotiateByPartialOptionsAndHttpContent(partialOptions, httpContent);
const actualOperationConfig = helpers.negotiateByPartialOptionsAndHttpContent(partialOptions, httpContent, logger);

assertRight(actualOperationConfig, operationConfig => {
expect(operationConfig).toEqual({
Expand All @@ -1074,7 +1076,7 @@ describe('negotiateByPartialOptionsAndHttpContent()', () => {
encodings: [],
};

const proposedResponse = helpers.negotiateByPartialOptionsAndHttpContent(partialOptions, httpContent);
const proposedResponse = helpers.negotiateByPartialOptionsAndHttpContent(partialOptions, httpContent, logger);

assertRight(proposedResponse, response => {
expect(response).toHaveProperty('code');
Expand Down

0 comments on commit d0dbb7f

Please sign in to comment.