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.

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

Fix(module-http): prevent modification to config #2043

Merged
merged 2 commits into from
Apr 8, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/breezy-scissors-speak.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@equinor/fusion-framework-module-http": patch
---

Added test for http client, to check if configured operators are not altered
48 changes: 48 additions & 0 deletions .changeset/loud-waves-laugh.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
---
"@equinor/fusion-framework-module-http": patch
---

When adding operators to request and response handler to an http client instanstance, the values where added to the configured handlers permanently

```ts
// create a new client from configuration
const fooClient = provider.createClient("foo");
fooClient.requestHandler.setHeader("x-foo", "bar");

// generate a RequestInit object
const fooRequest = await lastValueFrom(
fooClient.requestHandler.process({ path: "/api", uri: fooClient.uri }),
);

expect((fooRequest.headers as Headers)?.get("x-foo")).toBe("bar");

// create a new client from the same configuration
const barClient = provider.createClient("foo");

// generate a RequestInit object
const barRequest = await lastValueFrom(
barClient.requestHandler.process({ path: "/api", uri: barClient.uri }),
);

// expect the request header to not been modified
// FAILED
expect((barRequest.headers as Headers)?.get("x-foo")).toBeUndefined();
```

modified the `ProcessOperators` to accept operators on creation, which are clone to the instance.

```diff
--- a/packages/modules/http/src/lib/client/client.ts
+++ a/packages/modules/http/src/lib/client/client.ts
constructor(
public uri: string,
options?: Partial<HttpClientCreateOptions<TRequest, TResponse>>,
) {
- this.requestHandler = options?.requestHandler ?? new HttpRequestHandler<TRequest>();
+ this.requestHandler = new HttpRequestHandler<TRequest>(options?.requestHandler);
- this.responseHandler = options?.responseHandler ?? new HttpResponseHandler<TResponse>();
+ this.responseHandler = new HttpResponseHandler<TResponse>(options?.responseHandler);
this._init();
}

```
7 changes: 5 additions & 2 deletions packages/modules/http/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
"description": "",
"main": "dist/esm/index.js",
"types": "index.d.ts",
"type": "module",
"exports": {
".": {
"import": "./dist/esm/index.js",
Expand Down Expand Up @@ -44,7 +45,8 @@
},
"scripts": {
"build": "tsc -b",
"prepack": "pnpm build"
"prepack": "pnpm build",
"test": "vitest"
},
"keywords": [],
"author": "",
Expand All @@ -63,6 +65,7 @@
"rxjs": "^7.8.1"
},
"devDependencies": {
"typescript": "^5.4.2"
"typescript": "^5.4.2",
"vitest": "^1.2.2"
}
}
4 changes: 2 additions & 2 deletions packages/modules/http/src/lib/client/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,8 @@ export class HttpClient<
public uri: string,
options?: Partial<HttpClientCreateOptions<TRequest, TResponse>>,
) {
this.requestHandler = options?.requestHandler ?? new HttpRequestHandler<TRequest>();
this.responseHandler = options?.responseHandler ?? new HttpResponseHandler<TResponse>();
this.requestHandler = new HttpRequestHandler<TRequest>(options?.requestHandler);
this.responseHandler = new HttpResponseHandler<TResponse>(options?.responseHandler);
this._init();
}

Expand Down
53 changes: 52 additions & 1 deletion packages/modules/http/src/lib/operators/process-operators.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,24 +3,75 @@ import type { Observable } from 'rxjs';
import { last, mergeScan } from 'rxjs/operators';
import { IProcessOperators, ProcessOperator } from './types';

/**
* ProcessOperators class manages a collection of process operators
* and provides methods to add, set, get, and process these operators.
*/
export class ProcessOperators<T> implements IProcessOperators<T> {
protected _operators: Record<string, ProcessOperator<T>> = {};
/**
* A record of process operators keyed by a string.
*/
protected _operators: Record<string, ProcessOperator<T>>;

/**
* Accessor for the operators.
* @returns The record of process operators.
*/
get operators(): Record<string, ProcessOperator<T>> {
return this._operators;
}

/**
* Constructs a new instance of the ProcessOperators class.
* @param operators - An optional object containing process operators.
* It can be either an instance of IProcessOperators<T> or a record of string keys and ProcessOperator<T> values.
*/
constructor(operators?: IProcessOperators<T> | Record<string, ProcessOperator<T>>) {
if (operators && 'operators' in operators) {
this._operators = { ...operators.operators };
} else {
this._operators = operators ?? {};
}
}

/**
* Adds a new operator to the collection.
* @param key The key under which the operator is stored.
* @param operator The operator to be added.
* @returns The instance of ProcessOperators for chaining.
* @throws Error if an operator with the same key already exists.
*/
add(key: string, operator: ProcessOperator<T>): ProcessOperators<T> {
if (Object.keys(this._operators).includes(key))
throw Error(`Operator [${key}] already defined`);
return this.set(key, operator);
}

/**
* Sets or updates an operator in the collection.
* @param key The key under which the operator is stored.
* @param operator The operator to be set.
* @returns The instance of ProcessOperators for chaining.
*/
set(key: string, operator: ProcessOperator<T>): ProcessOperators<T> {
this._operators[key] = operator;
return this;
}

/**
* Retrieves an operator from the collection by its key.
* @param key The key of the operator to retrieve.
* @returns The retrieved operator.
*/
get(key: string): ProcessOperator<T> {
return this._operators[key];
}

/**
* Processes an input request through the chain of operators.
* @param request The request to be processed.
* @returns An Observable of the processed request.
*/
process(request: T): Observable<T> {
const operators = Object.values(this._operators);
/** if no operators registered, just return the observable value */
Expand Down
28 changes: 24 additions & 4 deletions packages/modules/http/src/lib/operators/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,24 +7,44 @@ export type ProcessOperator<T, R = T> = (request: T) => R | void | Promise<R | v
* Container for sync/async operators.
* Pipes each operator sequential
*/
/**
* Represents a collection of process operators.
* @template T The type of the request being processed.
*/
export interface IProcessOperators<T> {
/**
* Add a new operator (throw error if already defined)
* Gets the operators registered in the collection.
*/
get operators(): Record<string, ProcessOperator<T>>;

/**
* Adds a new operator to the collection.
* @param key The key to identify the operator.
* @param operator The process operator to add.
* @returns The updated collection of process operators.
* @throws An error if the operator is already defined.
*/
add(key: string, operator: ProcessOperator<T>): IProcessOperators<T>;

/**
* Add or sets a operator
* Adds or sets a process operator in the collection.
* @param key The key to identify the operator.
* @param operator The process operator to add or set.
* @returns The updated collection of process operators.
*/
set(key: string, operator: ProcessOperator<T>): IProcessOperators<T>;

/**
* Get a operator, will return undefined on invalid key.
* Gets a process operator from the collection.
* @param key The key of the operator to retrieve.
* @returns The process operator associated with the key, or undefined if the key is invalid.
*/
get(key: string): ProcessOperator<T>;

/**
* Process registered processors.
* Processes the registered process operators.
* @param request The request to process.
* @returns An observable that emits the processed request.
*/
process(request: T): Observable<T>;
}
Expand Down
48 changes: 48 additions & 0 deletions packages/modules/http/tests/HttpClient.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
import { describe, it, expect, beforeEach } from 'vitest';

import { HttpClientConfigurator } from '../src/configurator';
import { HttpClient } from '../src/lib';
import { HttpClientProvider } from '../src';
import { lastValueFrom } from 'rxjs';

let provider: HttpClientProvider;

describe('HttpClient', () => {
beforeEach(() => {
const config = new HttpClientConfigurator(HttpClient);
config.configureClient('foo', 'http://localhost:3000');
provider = new HttpClientProvider(config);
});

it('should create instance', () => {
const client = provider.createClient('foo');
expect(client).toBeDefined();
expect(client.uri).toBe('http://localhost:3000');
});

it('should allow providing headers in request', async () => {});

it('should not modify configured headers', async () => {
// create a new client from configuration
const fooClient = provider.createClient('foo');
fooClient.requestHandler.setHeader('x-foo', 'bar');

// generate a RequestInit object
const fooRequest = await lastValueFrom(
fooClient.requestHandler.process({ path: '/api', uri: fooClient.uri }),
);

expect((fooRequest.headers as Headers)?.get('x-foo')).toBe('bar');

// create a new client from the same configuration
const barClient = provider.createClient('foo');

// generate a RequestInit object
const barRequest = await lastValueFrom(
barClient.requestHandler.process({ path: '/api', uri: barClient.uri }),
);

// expect the request header to not been modified
expect((barRequest.headers as Headers)?.get('x-foo')).toBeUndefined();
});
});
2 changes: 1 addition & 1 deletion packages/modules/http/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
}
],
"include": [
"src/**/*",
"src/**/*"
],
"exclude": [
"node_modules",
Expand Down
12 changes: 12 additions & 0 deletions packages/modules/http/vitest.config.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
import { defineProject } from 'vitest/config';

import { name, version } from './package.json';

export default defineProject({
test: {
// TODO remove after __tests__ are deleted!
include: ['tests/**'],
name: `${name}@${version}`,
environment: 'happy-dom',
},
});
41 changes: 40 additions & 1 deletion pnpm-lock.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading