Skip to content
Closed
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
2 changes: 2 additions & 0 deletions goldens/public-api/common/http/testing/index.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,8 @@ export class TestRequest {
};
status?: number;
statusText?: string;
redirected?: boolean;
responseType?: ResponseType;
}): void;
// (undocumented)
request: HttpRequest<any>;
Expand Down
12 changes: 6 additions & 6 deletions packages/common/http/src/fetch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -143,14 +143,16 @@ export class FetchBackend implements HttpBackend {
statusText: error.statusText,
url: request.urlWithParams,
headers: error.headers,
redirected: response?.redirected,
responseType: response?.type,
}),
);
return;
}

const headers = new HttpHeaders(response.headers);
const statusText = response.statusText;
const url = response.url || request.urlWithParams;
const {statusText, url: responseUrl, redirected, type: responseType} = response;
const url = responseUrl || request.urlWithParams;

let status = response.status;
let body: string | ArrayBuffer | Blob | object | null = null;
Expand Down Expand Up @@ -261,6 +263,8 @@ export class FetchBackend implements HttpBackend {
status: response.status,
statusText: response.statusText,
url: response.url || request.urlWithParams,
redirected,
responseType,
}),
);
return;
Expand All @@ -278,10 +282,6 @@ export class FetchBackend implements HttpBackend {
// asked for JSON data and the body cannot be parsed as such.
const ok = status >= 200 && status < 300;

const redirected = response.redirected;

const responseType = response.type;

if (ok) {
observer.next(
new HttpResponse({
Expand Down
12 changes: 12 additions & 0 deletions packages/common/http/src/transfer_cache.ts
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,8 @@ export const STATUS = 's';
export const STATUS_TEXT = 'st';
export const REQ_URL = 'u';
export const RESPONSE_TYPE = 'rt';
export const REDIRECTED = 'r';
export const EVENT_RESPONSE_TYPE = 'ert';

interface TransferHttpResponse {
/** body */
Expand All @@ -110,6 +112,10 @@ interface TransferHttpResponse {
[REQ_URL]: string;
/** responseType */
[RESPONSE_TYPE]: HttpRequest<unknown>['responseType'];
/** `Response.redirected` from the Fetch API */
[REDIRECTED]?: boolean;
/** `Response.type` value from the Fetch API */
[EVENT_RESPONSE_TYPE]?: ResponseType;
}

interface CacheOptions extends HttpTransferCacheOptions {
Expand Down Expand Up @@ -214,6 +220,8 @@ export function retrieveStateFromCache(
[STATUS]: status,
[STATUS_TEXT]: statusText,
[REQ_URL]: url,
[REDIRECTED]: redirected,
[EVENT_RESPONSE_TYPE]: eventResponseType,
} = response;
// Request found in cache. Respond using it.
let body: ArrayBuffer | Blob | string | undefined = undecodedBody;
Expand Down Expand Up @@ -246,6 +254,8 @@ export function retrieveStateFromCache(
status,
statusText,
url,
redirected,
responseType: eventResponseType,
});
}

Expand Down Expand Up @@ -306,6 +316,8 @@ export function transferCacheInterceptorFn(
[STATUS_TEXT]: statusText,
[REQ_URL]: requestUrl,
[RESPONSE_TYPE]: responseType,
...(event.redirected !== undefined && {[REDIRECTED]: event.redirected}),
...(event.responseType !== undefined && {[EVENT_RESPONSE_TYPE]: event.responseType}),
});
}
}),
Expand Down
80 changes: 73 additions & 7 deletions packages/common/http/test/transfer_cache_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,12 +30,14 @@ import {
import {
BODY,
CACHE_OPTIONS,
EVENT_RESPONSE_TYPE,
HEADERS,
HTTP_TRANSFER_CACHE_ORIGIN_MAP,
REDIRECTED,
REQ_URL,
RESPONSE_TYPE,
STATUS,
STATUS_TEXT,
REQ_URL,
transferCacheInterceptorFn,
withHttpTransferCache,
generateHash,
Expand All @@ -56,6 +58,11 @@ interface RequestParams {
body?: RequestBody;
}

interface ResponseOptions {
redirected?: boolean;
responseType?: ResponseType;
}

type RequestBody =
| ArrayBuffer
| Blob
Expand Down Expand Up @@ -302,20 +309,27 @@ describe('TransferCache', () => {
url: string,
body: RequestBody,
params?: RequestParams,
responseOptions?: ResponseOptions,
): string;
function makeRequestAndExpectOne(
url: string,
body: RequestBody,
params?: RequestParams & {observe: 'response'},
responseOptions?: ResponseOptions,
): HttpResponse<string>;
function makeRequestAndExpectOne(url: string, body: RequestBody, params?: RequestParams): any {
function makeRequestAndExpectOne(
url: string,
body: RequestBody,
params?: RequestParams,
responseOptions?: ResponseOptions,
): any {
let response!: any;
TestBed.inject(HttpClient)
.request(params?.method ?? 'GET', url, params)
.subscribe((r) => (response = r));
TestBed.inject(HttpTestingController)
.expectOne(url)
.flush(body, {headers: params?.responseHeaders ?? params?.headers});
.flush(body, {headers: params?.responseHeaders ?? params?.headers, ...responseOptions});
return response;
}

Expand Down Expand Up @@ -408,8 +422,24 @@ describe('TransferCache', () => {
});

it('should stop storing HTTP calls in `TransferState` after application becomes stable', async () => {
makeRequestAndExpectOne('/test-1', 'foo');
makeRequestAndExpectOne('/test-2', 'buzz');
makeRequestAndExpectOne(
'/test-1',
'foo',
{},
{
responseType: 'default',
redirected: true,
},
);
makeRequestAndExpectOne(
'/test-2',
'buzz',
{},
{
responseType: 'cors',
redirected: false,
},
);

isStable.next(true);

Expand All @@ -426,6 +456,8 @@ describe('TransferCache', () => {
[STATUS_TEXT]: 'OK',
[REQ_URL]: '/test-1',
[RESPONSE_TYPE]: 'json',
[REDIRECTED]: true,
[EVENT_RESPONSE_TYPE]: 'default',
},
'869485290d9385f3c0a9ba571918c335bbca9e03373bf8260d02f2b7dd335849': {
[BODY]: 'buzz',
Expand All @@ -434,14 +466,48 @@ describe('TransferCache', () => {
[STATUS_TEXT]: 'OK',
[REQ_URL]: '/test-2',
[RESPONSE_TYPE]: 'json',
[REDIRECTED]: false,
[EVENT_RESPONSE_TYPE]: 'cors',
},
});
});

it('should NOT store redirected and responseType in transfer state when using XHR', async () => {
// XHR-based requests do not populate `redirected` or `responseType` on the HttpResponse,
// so those fields should be omitted from the cached entry to avoid bloating TransferState.
makeRequestAndExpectOne('/test-xhr', 'foo');

isStable.next(true);
await timeout();

const transferState = TestBed.inject(TransferState);
const stateJson = JSON.parse(transferState.toJson()) as Record<string, unknown>;

expect(stateJson).toEqual({
'f5ec817a9aa3d4b788d6624a845ffbfb247d82c0e48beba163d0a7947bdcb0c2': {
[BODY]: 'foo',
[HEADERS]: {},
[STATUS]: 200,
[STATUS_TEXT]: 'OK',
[REQ_URL]: '/test-xhr',
[RESPONSE_TYPE]: 'json',
},
});
});

it(`should use calls from cache when present and application is not stable`, () => {
makeRequestAndExpectOne('/test-1', 'foo');
makeRequestAndExpectOne(
'/test-1',
'foo',
{observe: 'response'},
{redirected: true, responseType: 'cors'},
);
// Do the same call, this time it should served from cache.
makeRequestAndExpectNone('/test-1');
const cachedResponse = makeRequestAndExpectNone('/test-1');

expect(cachedResponse.body).toBe('foo');
expect(cachedResponse.redirected).toBeTrue();
expect(cachedResponse.responseType).toBe('cors');
});

it(`should not use calls from cache when present and application is stable`, async () => {
Expand Down
32 changes: 30 additions & 2 deletions packages/common/http/testing/src/request.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ type TestRequestErrorOptions = {
headers?: HttpHeaders | {[name: string]: string | string[]};
status?: number;
statusText?: string;
redirected?: boolean;
responseType?: ResponseType;
};

/**
Expand Down Expand Up @@ -74,6 +76,8 @@ export class TestRequest {
headers?: HttpHeaders | {[name: string]: string | string[]};
status?: number;
statusText?: string;
redirected?: boolean;
responseType?: ResponseType;
} = {},
): void {
if (this.cancelled) {
Expand All @@ -85,6 +89,8 @@ export class TestRequest {
body = _maybeConvertBody(this.request.responseType, body);
let statusText: string | undefined = opts.statusText;
let status: number = opts.status !== undefined ? opts.status : HttpStatusCode.Ok;
const responseType = opts.responseType;
const redirected = opts.redirected;
if (opts.status === undefined) {
if (body === null) {
status = HttpStatusCode.NoContent;
Expand All @@ -97,10 +103,30 @@ export class TestRequest {
throw new Error('statusText is required when setting a custom status.');
}
if (status >= 200 && status < 300) {
this.observer.next(new HttpResponse<any>({body, headers, status, statusText, url}));
this.observer.next(
new HttpResponse<any>({
body,
headers,
status,
statusText,
url,
redirected,
responseType,
}),
);
this.observer.complete();
} else {
this.observer.error(new HttpErrorResponse({error: body, headers, status, statusText, url}));
this.observer.error(
new HttpErrorResponse({
error: body,
headers,
status,
statusText,
url,
redirected,
responseType,
}),
);
}
}

Expand All @@ -126,6 +152,8 @@ export class TestRequest {
status: opts.status || 0,
statusText: opts.statusText || '',
url: this.request.urlWithParams,
redirected: opts.redirected,
responseType: opts.responseType,
}),
);
}
Expand Down
9 changes: 8 additions & 1 deletion packages/common/http/testing/test/request_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -153,12 +153,19 @@ describe('HttpClient TestRequest', () => {

it('should allow creating HttpErrorResponse with any status code', () => {
const error = new ProgressEvent('error');
request.error(error, {status: 404, statusText: 'Not Found'});
request.error(error, {
status: 404,
statusText: 'Not Found',
responseType: 'cors',
redirected: true,
});

expect(lastError).toBeDefined();
expect(lastError).toBeInstanceOf(HttpErrorResponse);
expect(lastError.status).toBe(404);
expect(lastError.statusText).toBe('Not Found');
expect(lastError.responseType).toBe('cors');
expect(lastError.redirected).toBeTrue();
});
} else {
it('dummy test for node tests', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,7 @@
"EVENT_MANAGER_PLUGINS",
"EVENT_REPLAY_ENABLED_DEFAULT",
"EVENT_REPLAY_QUEUE",
"EVENT_RESPONSE_TYPE",
"EffectRefImpl",
"EffectScheduler",
"ElementRef",
Expand Down Expand Up @@ -230,6 +231,7 @@
"REACTIVE_LVIEW_CONSUMER_NODE",
"REACTIVE_NODE",
"REACTIVE_TEMPLATE_CONSUMER",
"REDIRECTED",
"REFERENCE_NODE_BODY",
"REFERENCE_NODE_HOST",
"REF_EXTRACTOR_REGEXP",
Expand Down
Loading