Skip to content

Commit

Permalink
fix: ensure cachehandler responses are cast to documents (#9459)
Browse files Browse the repository at this point in the history
  • Loading branch information
runspired authored May 28, 2024
1 parent 1f965a9 commit f178782
Show file tree
Hide file tree
Showing 13 changed files with 216 additions and 50 deletions.
37 changes: 30 additions & 7 deletions packages/request/src/-private/utils.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
import { DEBUG } from '@warp-drive/build-config/env';
import { getOrSetGlobal } from '@warp-drive/core-types/-private';
import {
type RequestInfo,
STRUCTURED,
type StructuredDataDocument,
type StructuredErrorDocument,
import type {
RequestInfo,
StructuredDataDocument,
StructuredDocument,
StructuredErrorDocument,
} from '@warp-drive/core-types/request';
import { STRUCTURED } from '@warp-drive/core-types/request';

import { Context, ContextOwner } from './context';
import { assertValidRequest } from './debug';
Expand Down Expand Up @@ -59,6 +60,28 @@ function isDoc<T>(doc: T | StructuredDataDocument<T>): doc is StructuredDataDocu
return doc && (doc as StructuredDataDocument<T>)[STRUCTURED] === true;
}

function ensureDoc<T>(owner: ContextOwner, content: T | Error, isError: boolean): StructuredDocument<T> {
if (isDoc(content)) {
return content as StructuredDocument<T>;
}

if (isError) {
return {
[STRUCTURED]: true,
request: owner.request,
response: owner.getResponse(),
error: content as Error,
} as StructuredErrorDocument<T>;
}

return {
[STRUCTURED]: true,
request: owner.request,
response: owner.getResponse(),
content: content as T,
};
}

export type HttpErrorProps = {
code: number;
name: string;
Expand Down Expand Up @@ -151,7 +174,7 @@ export function executeNextHandler<T>(
outcome = wares[i].request<T>(context, next);
if (!!outcome && isCacheHandler(wares[i], i)) {
if (!(outcome instanceof Promise)) {
setRequestResult(owner.requestId, { isError: false, result: outcome });
setRequestResult(owner.requestId, { isError: false, result: ensureDoc(owner, outcome, false) });
outcome = Promise.resolve(outcome);
}
} else if (DEBUG) {
Expand All @@ -166,7 +189,7 @@ export function executeNextHandler<T>(
}
} catch (e) {
if (isCacheHandler(wares[i], i)) {
setRequestResult(owner.requestId, { isError: true, result: e });
setRequestResult(owner.requestId, { isError: true, result: ensureDoc(owner, e, true) });
}
outcome = Promise.reject<StructuredDataDocument<T>>(e);
}
Expand Down
2 changes: 1 addition & 1 deletion packages/schema-record/eslint.config.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ export default [
// browser (js/ts) ================
typescript.browser({
srcDirs: ['src'],
allowedImports: ['@ember/debug'],
allowedImports: ['@ember/debug', '@ember/-internals/metal'],
}),

// node (module) ================
Expand Down
4 changes: 4 additions & 0 deletions packages/schema-record/src/record.ts
Original file line number Diff line number Diff line change
Expand Up @@ -463,6 +463,10 @@ export class SchemaRecord {
if (prop === 'constructor') {
return SchemaRecord;
}
// too many things check for random symbols
if (typeof prop === 'symbol') {
return undefined;
}
throw new Error(`No field named ${String(prop)} on ${identifier.type}`);
}

Expand Down
9 changes: 6 additions & 3 deletions packages/store/src/-private/cache-handler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -507,9 +507,12 @@ export const CacheHandler: CacheHandlerType = {
store.requestManager._pending.set(context.id, promise);
}

assert(`Expected a peeked request to be present`, peeked);

const shouldHydrate: boolean = context.request[EnableHydration] || false;
context.setResponse(peeked.response);

if ('error' in peeked!) {
if ('error' in peeked) {
const content = shouldHydrate
? maybeUpdateUiObjects<T>(
store,
Expand All @@ -529,10 +532,10 @@ export const CacheHandler: CacheHandlerType = {
store,
context.request,
{ shouldHydrate, identifier },
peeked!.content as ResourceDataDocument,
peeked.content as ResourceDataDocument,
true
)
: (peeked!.content as T);
: (peeked.content as T);

return result;
},
Expand Down
Binary file not shown.
Binary file not shown.
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
{
"url": "users/1",
"url": "users/2",
"status": 200,
"statusText": "OK",
"headers": {
Expand Down
Binary file not shown.
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
{
"url": "users/1",
"url": "users/2",
"status": 200,
"statusText": "OK",
"headers": {
Expand Down
Binary file not shown.
11 changes: 8 additions & 3 deletions tests/warp-drive__ember/app/services/store.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import type { StableRecordIdentifier } from '@warp-drive/core-types';
import type { Cache } from '@warp-drive/core-types/cache';
import { instantiateRecord, teardownRecord } from '@warp-drive/schema-record/hooks';
import type { SchemaRecord } from '@warp-drive/schema-record/record';
import { SchemaService } from '@warp-drive/schema-record/schema';

export default class Store extends DataStore {
constructor(args: unknown) {
Expand All @@ -17,15 +18,19 @@ export default class Store extends DataStore {
manager.useCache(CacheHandler);
}

override createCache(capabilities: CacheCapabilitiesManager): Cache {
createSchemaService() {
return new SchemaService();
}

createCache(capabilities: CacheCapabilitiesManager): Cache {
return new JSONAPICache(capabilities);
}

override instantiateRecord(identifier: StableRecordIdentifier, createArgs?: Record<string, unknown>): SchemaRecord {
instantiateRecord(identifier: StableRecordIdentifier, createArgs?: Record<string, unknown>): SchemaRecord {
return instantiateRecord(this, identifier, createArgs);
}

override teardownRecord(record: SchemaRecord): void {
teardownRecord(record: SchemaRecord): void {
return teardownRecord(record);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { rerender, settled } from '@ember/test-helpers';
import type { CacheHandler, Future, NextFn, RequestContext, StructuredDataDocument } from '@ember-data/request';
import RequestManager from '@ember-data/request';
import Fetch from '@ember-data/request/fetch';
import { buildBaseURL } from '@ember-data/request-utils';
import type { RenderingTestContext } from '@warp-drive/diagnostic/ember';
import { module, setupRenderingTest, test as _test } from '@warp-drive/diagnostic/ember';
import { getRequestState } from '@warp-drive/ember';
Expand Down Expand Up @@ -77,7 +78,7 @@ async function mockGETSuccess(context: LocalTestContext): Promise<string> {
}),
{ RECORD: RECORD }
);
return 'https://localhost:1135/users/1';
return buildBaseURL({ resourcePath: 'users/1' });
}
async function mockGETFailure(context: LocalTestContext): Promise<string> {
await mock(
Expand All @@ -102,7 +103,7 @@ async function mockGETFailure(context: LocalTestContext): Promise<string> {
RECORD
);

return 'https://localhost:1135/users/2';
return buildBaseURL({ resourcePath: 'users/2' });
}

module<LocalTestContext>('Integration | get-request-state', function (hooks) {
Expand Down Expand Up @@ -169,9 +170,11 @@ module<LocalTestContext>('Integration | get-request-state', function (hooks) {
status: 404,
name: 'NotFoundError',
isRequestError: true,
error: '[404 Not Found] GET (cors) - https://localhost:1135/users/2?__xTestId=b830e11d&__xTestRequestNumber=0',
error: `[404 Not Found] GET (cors) - ${buildBaseURL({
resourcePath: 'users/2',
})}?__xTestId=b830e11d&__xTestRequestNumber=0`,
statusText: 'Not Found',
message: '[404 Not Found] GET (cors) - https://localhost:1135/users/2',
message: `[404 Not Found] GET (cors) - ${buildBaseURL({ resourcePath: 'users/2' })}`,
errors: [{ status: '404', title: 'Not Found', detail: 'The resource does not exist.' }],
content: {
errors: [{ status: '404', title: 'Not Found', detail: 'The resource does not exist.' }],
Expand Down Expand Up @@ -414,13 +417,13 @@ module<LocalTestContext>('Integration | get-request-state', function (hooks) {
assert.true(state1!.error instanceof Error, 'error is an instance of Error');
assert.equal(
(state1!.error as Error | undefined)?.message,
'[404 Not Found] GET (cors) - https://localhost:1135/users/2',
`[404 Not Found] GET (cors) - ${buildBaseURL({ resourcePath: 'users/2' })}`,
'error message is correct'
);
assert.equal(counter, 2, 'counter is 2');
assert.equal(
this.element.textContent?.trim(),
'[404 Not Found] GET (cors) - https://localhost:1135/users/2\n Count:\n 2'
`[404 Not Found] GET (cors) - ${buildBaseURL({ resourcePath: 'users/2' })}\n Count:\n 2`
);
});

Expand Down Expand Up @@ -463,26 +466,26 @@ module<LocalTestContext>('Integration | get-request-state', function (hooks) {
assert.true(state1!.error instanceof Error, 'error is an instance of Error');
assert.equal(
(state1!.error as Error | undefined)?.message,
'[404 Not Found] GET (cors) - https://localhost:1135/users/2',
`[404 Not Found] GET (cors) - ${buildBaseURL({ resourcePath: 'users/2' })}`,
'error message is correct'
);
assert.equal(counter, 1, 'counter is 1');
assert.equal(
this.element.textContent?.trim(),
'[404 Not Found] GET (cors) - https://localhost:1135/users/2\n Count:\n 1'
`[404 Not Found] GET (cors) - ${buildBaseURL({ resourcePath: 'users/2' })}\n Count:\n 1`
);
await rerender();
assert.equal(state1!.result, null, 'after rerender result is still null');
assert.true(state1!.error instanceof Error, 'error is an instance of Error');
assert.equal(
(state1!.error as Error | undefined)?.message,
'[404 Not Found] GET (cors) - https://localhost:1135/users/2',
`[404 Not Found] GET (cors) - ${buildBaseURL({ resourcePath: 'users/2' })}`,
'error message is correct'
);
assert.equal(counter, 1, 'counter is 1');
assert.equal(
this.element.textContent?.trim(),
'[404 Not Found] GET (cors) - https://localhost:1135/users/2\n Count:\n 1'
`[404 Not Found] GET (cors) - ${buildBaseURL({ resourcePath: 'users/2' })}\n Count:\n 1`
);
});
});
Loading

0 comments on commit f178782

Please sign in to comment.