Skip to content

Commit

Permalink
Merge pull request #22 from ty-ras/issue/21-update-to-2-0
Browse files Browse the repository at this point in the history
#21 Updating the client-fetch library to 2.0.0.
  • Loading branch information
stazz committed Aug 2, 2023
2 parents ccace0c + 1430a04 commit ec8c0c7
Show file tree
Hide file tree
Showing 4 changed files with 847 additions and 56 deletions.
13 changes: 10 additions & 3 deletions client/package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "@ty-ras/client-fetch",
"version": "1.0.1",
"version": "2.0.0",
"author": {
"name": "Stanislav Muhametsin",
"email": "346799+stazz@users.noreply.github.com",
Expand Down Expand Up @@ -32,7 +32,7 @@
}
},
"dependencies": {
"@ty-ras/data-frontend": "^1.0.1"
"@ty-ras/data-frontend": "^2.0.1"
},
"devDependencies": {
"@babel/core": "7.21.5",
Expand All @@ -50,6 +50,7 @@
"eslint-plugin-prettier": "4.2.1",
"eslint-plugin-sonarjs": "0.19.0",
"fetch-mock": "9.11.0",
"madge": "6.1.0",
"node-fetch": "2.6.7",
"prettier": "2.8.8",
"ts-node": "10.9.1",
Expand All @@ -65,11 +66,17 @@
"format-output-files-ts": "eslint --no-eslintrc --config '.eslintrc.out-ts.cjs' --fix --fix-type layout './dist-ts/**/*.ts'",
"format-output-files-js": "eslint --no-eslintrc --config '.eslintrc.out.cjs' --fix 'dist-cjs/**/*js' 'dist-esm/**/*js'",
"generate-stub-package-json-for-cjs": "../scripts/generate-stub-package-json.cjs",
"lint": "eslint ./src --ext .ts,.tsx",
"lint": "yarn run lint:eslint && yarn run lint:circular",
"lint:circular": "madge --circular --no-color --no-spinner --extensions ts --warning ./src",
"lint:eslint": "eslint ./src --ext .ts,.tsx",
"remove-empty-js-files": "../scripts/remove-empty-js-files.cjs",
"tsc": "tsc --project tsconfig.build.json",
"tsc:plain": "tsc",
"test:coverage": "c8 --temp-directory /tmp ava",
"test:run": "c8 --temp-directory /tmp --reporter text ava"
},
"resolutions": {
"dependency-tree": "10.0.9",
"precinct": "11.0.5"
}
}
26 changes: 25 additions & 1 deletion client/src/__test__/client.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,11 +46,12 @@ const validateSuccessfulInvocation = async (
expectedResult: data.HTTPInvocationResult,
mockedFetchResponse: MockedFetchResponses[number],
expectedFetchInput: ExpectedFetchInputs[number],
callHttpInstance?: typeof callHttp,
) => {
c.plan(2);
resetState();
returnedResponses.push(mockedFetchResponse);
const result = await callHttp(input);
const result = await (callHttpInstance ?? callHttp)(input);
c.deepEqual(result, expectedResult);
c.deepEqual(recordedCalls, [expectedFetchInput]);
};
Expand Down Expand Up @@ -203,5 +204,28 @@ test.serial(
},
);

test.serial(
"Validate that callHttp by default doesn't deserialize __proto__ properties",
validateSuccessfulInvocation,
{ method: "GET", url: "/" },
{ body: { testProperty: "yes" } },
JSON.stringify({ __proto__: "Injected", testProperty: "yes" }),
{ url: fetchInputURL, opts: { method: "GET", headers: {} } },
);

test.serial(
"Validate that callHttp deserializes __proto__ property when instructed",
validateSuccessfulInvocation,
{ method: "GET", url: "/" },
{ body: { __proto__: "Injected", testProperty: "yes" } },
JSON.stringify({ __proto__: "Injected", testProperty: "yes" }),
{ url: "http://example.com/", opts: { method: "GET", headers: {} } },
spec.createCallHTTPEndpoint({
scheme: "http",
host: "example.com",
allowProtoProperty: true,
}),
);

type MockedFetchResponses = Array<string | Response>;
type ExpectedFetchInputs = Array<{ url: string; opts: MockRequest }>;
79 changes: 51 additions & 28 deletions client/src/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
* @file This file contains function to create {@link feCommon.CallHTTPEndpoint} which will use `fetch` API to do the requests.
*/

import * as data from "@ty-ras/data";
import type * as feCommon from "@ty-ras/data-frontend";
import * as errors from "./errors";

Expand All @@ -17,6 +18,11 @@ export const createCallHTTPEndpoint = (
): feCommon.CallHTTPEndpoint => {
// If some garbage provided as args, then this will throw
const baseURLString = validateBaseURL(callerArgs);
const allowProtoProperty =
typeof callerArgs == "string"
? true
: callerArgs.allowProtoProperty === true;
const reviver = data.getJSONParseReviver(allowProtoProperty);
return async ({ headers, url, method, query, ...args }) => {
const body = "body" in args ? JSON.stringify(args.body) : undefined;

Expand All @@ -25,35 +31,13 @@ export const createCallHTTPEndpoint = (
throw new errors.InvalidPathnameError(url);
}
if (query) {
urlObject.search = new URLSearchParams(
Object.entries(query)
.filter(([, value]) => value !== undefined)
.flatMap<[string, string]>(([qKey, qValue]) =>
Array.isArray(qValue)
? qValue.map<[string, string]>((value) => [qKey, `${value}`])
: [[qKey, `${qValue}`]],
),
).toString();
urlObject.search = getURLSearchParams(query);
}

const response = await fetch(urlObject, {
method,
...(body === undefined ? {} : { body }),
headers: {
// Notice that we allow overriding these specific headers with values in 'headers' below.
// This is only because this callback is used in tests, and they require such functionality.
// In reality, the spread of 'headers' should come first, and only then the headers related to body.
// Even better, we should delete the reserved header names if body is not specified.
...(body === undefined
? {}
: {
["Content-Type"]: "application/json", // TODO ;charset=utf8 ?
// ["Content-Length"]: `${body.byteLength}`,
// ["Content-Encoding"]: encoding,
}),
...headers,
},
});
const response = await fetch(
urlObject,
getFetchArgs(method, body, headers),
);

// Will throw (TODO verify this) on any response which code is not >= 200 and < 300.
// So just verify that it is one of the OK or No Content.
Expand All @@ -65,7 +49,7 @@ export const createCallHTTPEndpoint = (
const bodyString = await response.text();
const headersObject = Object.fromEntries(responseHeaders.entries());
return {
body: bodyString.length > 0 ? JSON.parse(bodyString) : undefined,
body: bodyString.length > 0 ? JSON.parse(bodyString, reviver) : undefined,
// TODO multiple entries with same header name!
...(Object.keys(headersObject).length > 0
? { headers: headersObject }
Expand Down Expand Up @@ -104,6 +88,11 @@ export interface HTTPEndpointCallerOptions {
* If provided, typically should include the last `/` character - the given URL paths will be concatenated directly after this without putting any logic in concatenation.
*/
path?: string;

/**
* If set to `true`, will NOT strip the `__proto__` properties of the result.
*/
allowProtoProperty?: boolean;
}

/**
Expand All @@ -123,3 +112,37 @@ export const validateBaseURL = (args: HTTPEndpointCallerArgs) => {
new URL(baseURLString);
return baseURLString;
};

const getURLSearchParams = (query: Record<string, unknown>) =>
new URLSearchParams(
Object.entries(query)
.filter(([, value]) => value !== undefined)
.flatMap<[string, string]>(([qKey, qValue]) =>
Array.isArray(qValue)
? qValue.map<[string, string]>((value) => [qKey, `${value}`])
: [[qKey, `${qValue}`]],
),
).toString();

const getFetchArgs = (
method: string,
body: string | undefined,
headers: Record<string, unknown> | undefined,
): RequestInit => ({
method,
...(body === undefined ? {} : { body }),
headers: {
// Notice that we allow overriding these specific headers with values in 'headers' below.
// This is only because this callback is used in tests, and they require such functionality.
// In reality, the spread of 'headers' should come first, and only then the headers related to body.
// Even better, we should delete the reserved header names if body is not specified.
...(body === undefined
? {}
: {
["Content-Type"]: "application/json", // TODO ;charset=utf8 ?
// ["Content-Length"]: `${body.byteLength}`,
// ["Content-Encoding"]: encoding,
}),
...headers,
},
});
Loading

0 comments on commit ec8c0c7

Please sign in to comment.