Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix: remove deprecated usage of parse #1959

Merged
merged 3 commits into from
Mar 17, 2022
Merged

fix: remove deprecated usage of parse #1959

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
af6bfd5
fix: remove deprecated usage of parse
jbl428 Mar 3, 2022
4704de0
fix: add nullable checking to `search` field
jbl428 Mar 3, 2022
3fbc964
fix: change misleading code
jbl428 Mar 3, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
8 changes: 6 additions & 2 deletions packages/http/src/__tests__/client.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,11 +43,11 @@ describe('User Http Client', () => {
jest.spyOn(client, 'request');
});

afterAll(() => jest.clearAllMocks());
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why was this moved into the inner scopes?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought that previous mock's call history should be cleared before each describe started .

If second describe pass, the third one always pass because both have same test body.

      describe('when calling a method with overridden url', () => {
        beforeAll(() => client.get('/pet', { baseUrl: 'https://www.google.it' }));

        afterAll(() => jest.clearAllMocks());

        test('should call the mocker with the replaced full url', () => {
          expect(mock.default).toBeCalledWith({ // <-- same body
            resource: expect.anything(),
            input: expect.objectContaining({
              data: expect.objectContaining({
                url: expect.objectContaining({
                  baseUrl: 'https://www.google.it',
                  path: '/pet',
                }),
              }),
            }),
            config: expect.anything(),
          });
        });
      });

      describe('when calling a method with a full url', () => {
        beforeAll(() => client.get('https://www.google.it/pet'));

        afterAll(() => jest.clearAllMocks());

        test('should call the mocker with the replaced full url', () => {
          expect(mock.default).toBeCalledWith({  // <-- same body
            resource: expect.anything(),
            input: expect.objectContaining({
              data: expect.objectContaining({
                url: expect.objectContaining({
                  baseUrl: 'https://www.google.it',
                  path: '/pet',
                }),
              }),
            }),
            config: expect.anything(),
          });
        });
      });
    })


describe('when calling with no options', () => {
beforeAll(() => client.get('/pet'));

afterAll(() => jest.clearAllMocks());

test('shall call the mocker with the default options', () =>
expect(mock.default).toHaveBeenCalledWith({
input: expect.anything(),
Expand All @@ -62,6 +62,8 @@ describe('User Http Client', () => {
describe('when calling a method with overridden url', () => {
beforeAll(() => client.get('/pet', { baseUrl: 'https://www.google.it' }));

afterAll(() => jest.clearAllMocks());

test('should call the mocker with the replaced full url', () => {
expect(mock.default).toBeCalledWith({
resource: expect.anything(),
Expand All @@ -81,6 +83,8 @@ describe('User Http Client', () => {
describe('when calling a method with a full url', () => {
beforeAll(() => client.get('https://www.google.it/pet'));

afterAll(() => jest.clearAllMocks());

test('should call the mocker with the replaced full url', () => {
expect(mock.default).toBeCalledWith({
resource: expect.anything(),
Expand Down
21 changes: 15 additions & 6 deletions packages/http/src/client.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,11 @@ import { IPrismOutput } from '@stoplight/prism-core';
import { IHttpOperation } from '@stoplight/types';
import { defaults } from 'lodash';
import { parse as parseQueryString } from 'querystring';
import { parse as parseUrl } from 'url';
import { createInstance, IHttpNameValues } from '.';
import { IHttpConfig, IHttpRequest, IHttpResponse, IHttpUrl } from './types';
import { fold } from 'fp-ts/TaskEither';
import * as Task from 'fp-ts/Task';
import * as O from 'fp-ts/Option';
import { pipe } from 'fp-ts/function';
import * as pino from 'pino';

Expand All @@ -30,16 +30,25 @@ export function createClientFromOperations(resources: IHttpOperation[], defaultC

const client: PrismHttp = {
request(url, input, config) {
const parsedUrl = parseUrl(url);

if (!parsedUrl.pathname) throw new Error('Path name must always be specified');
if (!url) throw new Error('Path name must always be specified');

const mergedConf: IClientConfig = defaults(config, defaultConfig);
const baseUrl = pipe(
O.tryCatch(() => new URL(url)),
O.fold(
() => mergedConf.baseUrl,
url => url.origin
)
);
const parsedUrl = pipe(
O.tryCatch(() => new URL(url)),
O.getOrElseW(() => new URL(url, 'https://stoplight.io'))
);

const httpUrl: IHttpUrl = {
baseUrl: parsedUrl.host ? `${parsedUrl.protocol}//${parsedUrl.host}` : mergedConf.baseUrl,
baseUrl,
path: parsedUrl.pathname,
query: parseQueryString(parsedUrl.query || '') as IHttpNameValues,
query: parseQueryString(parsedUrl.search?.substring(1) || '') as IHttpNameValues,
};

return pipe(
Expand Down
2 changes: 1 addition & 1 deletion packages/http/src/forwarder/__tests__/index.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import { DiagnosticSeverity, Dictionary } from '@stoplight/types';
jest.mock('node-fetch');

function stubFetch({ json = {}, text = '', headers }: { headers: Dictionary<string>; text?: string; json?: unknown }) {
((fetch as unknown) as jest.Mock).mockResolvedValue({
(fetch as unknown as jest.Mock).mockResolvedValue({
headers: { get: (n: string) => headers[n], raw: () => mapValues(headers, (h: string) => h.split(' ')) },
json: jest.fn().mockResolvedValue(json),
text: jest.fn().mockResolvedValue(text),
Expand Down
19 changes: 10 additions & 9 deletions packages/http/src/forwarder/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
import { IPrismComponents, IPrismInput } from '@stoplight/prism-core';
import { IHttpOperation } from '@stoplight/types';
import fetch from 'node-fetch';
import { pipe } from 'fp-ts/function';
import { constUndefined, pipe } from 'fp-ts/function';
import * as NEA from 'fp-ts/NonEmptyArray';
import * as E from 'fp-ts/Either';
import * as TE from 'fp-ts/TaskEither';
import * as RTE from 'fp-ts/ReaderTaskEither';
import * as J from 'fp-ts/Json';
import { defaults, omit } from 'lodash';
import { format, parse } from 'url';
import { format } from 'url';
import { posix } from 'path';
import { Logger } from 'pino';
import { IHttpConfig, IHttpRequest, IHttpResponse } from '../types';
Expand All @@ -32,7 +32,7 @@ const forward: IPrismComponents<IHttpOperation, IHttpRequest, IHttpResponse, IHt
logger =>
pipe(
NEA.fromArray(validations),
TE.fromOption(() => undefined),
TE.fromOption(constUndefined),
TE.map(failedValidations => {
const securityValidation = failedValidations.find(validation => validation.code === 401);

Expand All @@ -44,12 +44,13 @@ const forward: IPrismComponents<IHttpOperation, IHttpRequest, IHttpResponse, IHt
TE.chainEitherK(() => serializeBody(input.body)),
TE.chain(body =>
TE.tryCatch(async () => {
const partialUrl = parse(baseUrl);
const url = format({
...partialUrl,
pathname: posix.join(partialUrl.pathname || '', input.url.path),
query: input.url.query,
});
const partialUrl = new URL(baseUrl);
const url = format(
Object.assign(partialUrl, {
pathname: posix.join(partialUrl.pathname || '', input.url.path),
query: input.url.query,
})
);

logger.info(`Forwarding "${input.method}" request to ${url}...`);
let proxyAgent = undefined;
Expand Down
3 changes: 2 additions & 1 deletion packages/http/src/mocker/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -349,7 +349,8 @@ function computeBody(
): E.Either<Error, unknown> {
if (isINodeExample(negotiationResult.bodyExample) && negotiationResult.bodyExample.value !== undefined) {
return E.right(negotiationResult.bodyExample.value);
} else if (negotiationResult.schema) {
}
if (negotiationResult.schema) {
return payloadGenerator(negotiationResult.schema);
}
return E.right(undefined);
Expand Down