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.

Sign up for GitHub

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

Apollo Server 2.0 - Caching #1163

Merged
merged 15 commits into from
Jun 15, 2018
Merged

Apollo Server 2.0 - Caching #1163

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
8f74518
Enable declarationMap in tsconfig.json
martijnwalraven Jun 12, 2018
1c23535
Add apollo-server-caching package and improve typings
martijnwalraven Jun 12, 2018
f8658b9
Remove superfluous test steps
martijnwalraven Jun 12, 2018
4f25000
Add .npmignore to apollo-server-caching
martijnwalraven Jun 13, 2018
395da0f
Add apollo-server-env and apollo-datasource-rest packages
martijnwalraven Jun 14, 2018
dbd8288
Fix broken imports
martijnwalraven Jun 14, 2018
eda8ce9
Use prepublish instead of prepare
martijnwalraven Jun 14, 2018
80d074f
Merge branch 'version-2' into server-2.0/caching
evans Jun 14, 2018
e6aae1d
cache is now passed to data sources from ApolloServer constructor
evans Jun 15, 2018
29d5998
fix Object.values to use the object passed in rather than this
evans Jun 15, 2018
7b2d694
add initial datasource test
evans Jun 15, 2018
b39db20
docs: initial data source documentation
evans Jun 15, 2018
93d21f8
docs: initial data source documentation
evans Jun 15, 2018
6548196
compiles and documentation now highlights code in data-sources.md
evans Jun 15, 2018
1703e27
Some changes to the data source docs
martijnwalraven Jun 15, 2018
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
10 changes: 9 additions & 1 deletion packages/apollo-datasource-rest/src/RESTDataSource.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,15 @@ export abstract class RESTDataSource<TContext = any> {
httpCache!: HTTPCache;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I generally like variable scopes explicit, public in this case.

context!: TContext;

willSendRequest?(request: Request): void;
public willSendRequest?(request: Request): void;

public willReceiveCache(httpCache: HTTPCache) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmmm, not sure I understand the benefits of this. It will actually open up users to mistakes when they override these methods and forget to call super (or set the properties themselves).

If the idea is that we'd like to have a lifecycle method that's called when the datasource is fully initialized, we should probably define it separately and call it when both cache and context have been set.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That makes sense! We don't want users to mess with those "lifecycles", then we should probably have a ready or willReceiveRequest lifecycle for after the httpCache and context are set.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@glasser pointed out that these names aren't the best, since the data source is really just receiving the cache, so something like receiveCache would probably be better

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not convinced we need these at all, but if we do I like the consistency with React's willReceiveProps.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

want to make sure #1163 (comment) doesn't get buried

this.httpCache = httpCache;
}

public willReceiveContext(context: TContext) {
this.context = context;
}

protected async get(
path: string,
Expand Down
5 changes: 5 additions & 0 deletions packages/apollo-server-core/src/ApolloServer.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import {
} from 'graphql';
import { GraphQLExtension } from 'graphql-extensions';
import { EngineReportingAgent } from 'apollo-engine-reporting';
import { InMemoryKeyValueCache } from 'apollo-datasource-rest';

import {
SubscriptionServer,
Expand Down Expand Up @@ -121,6 +122,10 @@ export class ApolloServerBase {
delete requestOptions.persistedQueries;
}

if (!requestOptions.cache) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should stick to the philosophy that all extra features can be turned off with false.

requestOptions.cache = new InMemoryKeyValueCache();
}

this.requestOptions = requestOptions as GraphQLOptions;
this.context = context;

Expand Down
5 changes: 3 additions & 2 deletions packages/apollo-server-core/src/graphqlOptions.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import {
import { LogFunction } from './logging';
import { PersistedQueryCache } from './caching';
import { GraphQLExtension } from 'graphql-extensions';
import { RESTDataSource } from 'apollo-datasource-rest';
import { RESTDataSource, KeyValueCache } from 'apollo-datasource-rest';

/*
* GraphQLServerOptions
Expand Down Expand Up @@ -44,10 +44,11 @@ export interface GraphQLServerOptions<
cacheControl?: boolean | any;
extensions?: Array<() => GraphQLExtension>;
dataSources?: () => DataSources;
cache?: KeyValueCache;
persistedQueries?: PersistedQueryOptions;
}

type DataSources = { [name: string]: RESTDataSource };
export type DataSources = { [name: string]: RESTDataSource };

export interface PersistedQueryOptions {
cache: PersistedQueryCache;
Expand Down
9 changes: 5 additions & 4 deletions packages/apollo-server-core/src/runHttpQuery.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -286,13 +286,14 @@ export async function runHttpQuery(
}

if (optionsObject.dataSources) {
const dataSources = optionsObject.dataSources();
const dataSources = optionsObject.dataSources() || {};

const httpCache = new HTTPCache();
//we use the cache provided to the request and add the Http semantics on top
const httpCache = new HTTPCache(optionsObject.cache);

for (const dataSource of Object.values(dataSources)) {
dataSource.httpCache = httpCache;
dataSource.context = context;
dataSource.willReceiveContext(context);
dataSource.willReceiveCache(httpCache);
}

if ('dataSources' in context) {
Expand Down
3 changes: 3 additions & 0 deletions packages/apollo-server-core/src/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ import {
PersistedQueryOptions,
} from './graphqlOptions';

export { KeyValueCache } from 'apollo-datasource-rest';

export type Context<T = any> = T;
export type ContextFunction<T = any> = (
context: Context<T>,
Expand Down Expand Up @@ -41,6 +43,7 @@ export interface Config
| 'cacheControl'
| 'tracing'
| 'dataSources'
| 'cache'
> {
typeDefs?: DocumentNode | [DocumentNode];
resolvers?: IResolvers;
Expand Down