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: ensure cachehandler responses are cast to documents #9459

Merged
merged 1 commit into from
May 28, 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
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
Loading