Skip to content

Commit

Permalink
Plugins: no server; interface contravariant; no factory functions
Browse files Browse the repository at this point in the history
Previously on `version-4`, we had made `ApolloServerPlugin<TContext>`
"invariant" in `TContext` via the new `in out` annotation. That's
because we had added a `server` field to `GraphQLRequestContext` and
`GraphQLServerContext`.

The issue here is that because you can invoke
`server.executeOperation(op, contextValue)`, then a
`ApolloServerPlugin<BaseContext>` was not assignable to
`ApolloServerPlugin<SomeSpecificContext>`, because the former was able
to call `requestContext.executeOperation(op, {})`, and a server whose
context has additional required fields could not load this plugin. This
essentially meant that you couldn't have a single object (not created by
a generic function or generic class) that was a plugin that works with
any server, even if it doesn't actually care about context values at
all.

When trying to dogfood AS4 it became clear that this was annoying. So
let's fix it by removing `server` from those objects and making
`ApolloServerPlugin` "contravariant" instead. This means that plugins
can only *receive* `contextValue`, not *send* it. And so
`ApolloServerPlugin<BaseContext>` (a plugin that doesn't bother to read
any fields from `contextValue`) can be assigned to
`ApolloServerPlugin<SpecificContext>` (a plugin that is permitted to
read specific fields on `contextValue`, though it may choose not to).

After removing `server`, restore `logger` (now `readonly`) and `cache`
to `GraphQLRequestContext` and `GraphQLServerContext` (`cache` is
actually new to `GraphQLServerContext` in AS4). They had previously been
removed for redundancy.

This un-does the fix to #6264. If your plugin needs access to the
`server` object, you can just pass it to your plugin's
constructor/factory yourself. This wasn't an option in AS3 because there
was no `server.addPlugin` method... or actually, you could do it with
the plugin factory feature.

Note that we do leave `ApolloServer<TContext>` as invariant in
`TContext`, for the reasons described in a comment above the class.

While we're at it, remove the unnecessary ability to specify plugins as
factory functions. As far as we know, the main use case for this feature
is to refer to the `ApolloServer` itself in the expression creating the
plugin, but the new `addPlugin` method lets you do the same thing.
Reducing the number of ways to specify constructor options helps keep
type errors simpler.
  • Loading branch information
glasser committed Aug 15, 2022
1 parent afb0218 commit 5015fe0
Show file tree
Hide file tree
Showing 17 changed files with 74 additions and 92 deletions.
6 changes: 3 additions & 3 deletions packages/integration-testsuite/src/apolloServerTests.ts
Expand Up @@ -33,7 +33,7 @@ import type {
ApolloServerOptions,
ApolloServer,
BaseContext,
PluginDefinition,
ApolloServerPlugin,
} from '@apollo/server';
import fetch from 'node-fetch';

Expand Down Expand Up @@ -541,7 +541,7 @@ export function defineIntegrationTestSuiteApolloServerTests(
let serverInstance: ApolloServer<BaseContext>;

const setupApolloServerAndFetchPairForPlugins = async (
plugins: PluginDefinition<BaseContext>[] = [],
plugins: ApolloServerPlugin<BaseContext>[] = [],
) => {
const { server, url } = await createServer(
{
Expand Down Expand Up @@ -878,7 +878,7 @@ export function defineIntegrationTestSuiteApolloServerTests(
ApolloServerPluginUsageReportingOptions<any>
> = {},
constructorOptions: Partial<CreateServerForIntegrationTests> = {},
plugins: PluginDefinition<BaseContext>[] = [],
plugins: ApolloServerPlugin<BaseContext>[] = [],
) => {
const uri = await createServerAndGetUrl({
typeDefs: gql`
Expand Down
Expand Up @@ -174,7 +174,7 @@ export default function plugin<TContext extends BaseContext>(
outerRequestContext: GraphQLRequestContext<any>,
): Promise<GraphQLRequestListener<any>> {
const cache = new PrefixingKeyValueCache(
options.cache ?? outerRequestContext.server.cache,
options.cache ?? outerRequestContext.cache,
'fqc:',
);

Expand Down Expand Up @@ -267,7 +267,7 @@ export default function plugin<TContext extends BaseContext>(
},

async willSendResponse(requestContext) {
const logger = requestContext.server.logger || console;
const logger = requestContext.logger || console;

if (!isGraphQLQuery(requestContext)) {
return;
Expand Down
31 changes: 12 additions & 19 deletions packages/server/src/ApolloServer.ts
Expand Up @@ -42,6 +42,7 @@ import type {
PersistedQueryOptions,
HTTPGraphQLHead,
ContextThunk,
GraphQLRequestContext,
} from './externalTypes';
import { runPotentiallyBatchedHttpQuery } from './httpBatching.js';
import { InternalPluginId, pluginIsInternal } from './internalPlugin.js';
Expand Down Expand Up @@ -202,20 +203,6 @@ export class ApolloServer<in out TContext extends BaseContext = BaseContext> {

const isDev = nodeEnv !== 'production';

// Plugins can be (for some reason) provided as a function, which we have to
// call first to get the actual plugin. Note that more plugins can be added
// before `start()` with `addPlugin()` (eg, plugins that want to take this
// ApolloServer as an argument), and `start()` will call
// `ensurePluginInstantiation` to add default plugins.
const plugins: ApolloServerPlugin<TContext>[] = (config.plugins ?? []).map(
(plugin) => {
if (typeof plugin === 'function') {
return plugin();
}
return plugin;
},
);

const state: ServerState = config.gateway
? // ApolloServer has been initialized but we have not yet tried to load the
// schema from the gateway. That will wait until `start()` or
Expand Down Expand Up @@ -293,7 +280,11 @@ export class ApolloServer<in out TContext extends BaseContext = BaseContext> {
nodeEnv,
allowBatchedHttpRequests: config.allowBatchedHttpRequests ?? false,
apolloConfig,
plugins,
// Note that more plugins can be added
// before `start()` with `addPlugin()` (eg, plugins that want to take this
// ApolloServer as an argument), and `start()` will call `addDefaultPlugins`
// to add default plugins.
plugins: config.plugins ?? [],
parseOptions: config.parseOptions ?? {},
state,
stopOnTerminationSignals: config.stopOnTerminationSignals,
Expand Down Expand Up @@ -377,8 +368,9 @@ export class ApolloServer<in out TContext extends BaseContext = BaseContext> {
});

const schemaDerivedData = schemaManager.getSchemaDerivedData();
const service: GraphQLServerContext<TContext> = {
server: this,
const service: GraphQLServerContext = {
logger: this.logger,
cache: this.cache,
schema: schemaDerivedData.schema,
apollo: this.internals.apolloConfig,
startedInBackground,
Expand Down Expand Up @@ -1154,8 +1146,9 @@ export async function internalExecuteOperation<TContext extends BaseContext>({
const httpGraphQLHead = newHTTPGraphQLHead();
httpGraphQLHead.headers.set('content-type', 'application/json');

const requestContext = {
server,
const requestContext: GraphQLRequestContext<TContext> = {
logger: server.logger,
cache: server.cache,
schema: schemaDerivedData.schema,
request: graphQLRequest,
response: { result: {}, http: httpGraphQLHead },
Expand Down
12 changes: 10 additions & 2 deletions packages/server/src/__tests__/ApolloServer.test.ts
Expand Up @@ -489,19 +489,27 @@ describe('ApolloServer executeOperation', () => {
},
};

// A plugin that expects specific fields to be set is not a plugin that
// doesn't promise to set any fields.
// @ts-expect-error
takesPlugin<BaseContext>(specificPlugin);
// @ts-expect-error
// This is OK: plugins only get to read context, not write it, so a plugin
// that reads no interesting fields can be used as a plugin that is
// hypothetically allowed to read some interesting fields but chooses not
// to.
takesPlugin<SpecificContext>(basePlugin);

// You can't use a plugin that expects specific fields to exist with a
// server that doesn't require them to be set when executing operations.
new ApolloServer<BaseContext>({
typeDefs: 'type Query { x: ID }',
// @ts-expect-error
plugins: [specificPlugin],
});
// A plugin that doesn't expect any fields to be set works fine with a
// server that sets some fields when executing operations.
new ApolloServer<SpecificContext>({
typeDefs: 'type Query { x: ID }',
// @ts-expect-error
plugins: [basePlugin],
});
});
Expand Down
4 changes: 2 additions & 2 deletions packages/server/src/__tests__/logger.test.ts
Expand Up @@ -15,7 +15,7 @@ describe('logger', () => {
`,
plugins: [
{
async serverWillStart({ server: { logger } }) {
async serverWillStart({ logger }) {
logger.debug(KNOWN_DEBUG_MESSAGE);
},
},
Expand All @@ -42,7 +42,7 @@ describe('logger', () => {
`,
plugins: [
{
async serverWillStart({ server: { logger } }) {
async serverWillStart({ logger }) {
logger.debug(KNOWN_DEBUG_MESSAGE);
},
},
Expand Down
4 changes: 2 additions & 2 deletions packages/server/src/externalTypes/constructor.ts
Expand Up @@ -11,7 +11,7 @@ import type {
import type { KeyValueCache } from '@apollo/utils.keyvaluecache';
import type { GatewayInterface } from '@apollo/server-gateway-interface';
import type { BaseContext } from '.';
import type { PluginDefinition } from './plugins';
import type { ApolloServerPlugin } from './plugins';

export type DocumentStore = KeyValueCache<DocumentNode>;

Expand Down Expand Up @@ -86,7 +86,7 @@ interface ApolloServerOptionsBase<TContext extends BaseContext> {
allowBatchedHttpRequests?: boolean;

introspection?: boolean;
plugins?: PluginDefinition<TContext>[];
plugins?: ApolloServerPlugin<TContext>[];
persistedQueries?: PersistedQueryOptions | false;
stopOnTerminationSignals?: boolean;
apollo?: ApolloConfigInput;
Expand Down
6 changes: 4 additions & 2 deletions packages/server/src/externalTypes/graphql.ts
Expand Up @@ -9,7 +9,8 @@ import type {
import type { CachePolicy } from '@apollo/cache-control-types';
import type { BaseContext } from './context';
import type { HTTPGraphQLHead, HTTPGraphQLRequest } from './http';
import type { ApolloServer } from '../ApolloServer';
import type { Logger } from '@apollo/utils.logger';
import type { KeyValueCache } from '@apollo/utils.keyvaluecache';

export interface GraphQLRequest {
query?: string;
Expand Down Expand Up @@ -44,7 +45,8 @@ export interface GraphQLRequestMetrics {
}

export interface GraphQLRequestContext<TContext extends BaseContext> {
readonly server: ApolloServer<TContext>;
readonly logger: Logger;
readonly cache: KeyValueCache<string>;

readonly request: GraphQLRequest;
readonly response: GraphQLResponse;
Expand Down
1 change: 0 additions & 1 deletion packages/server/src/externalTypes/index.ts
Expand Up @@ -29,7 +29,6 @@ export type {
GraphQLServerListener,
GraphQLServerContext,
LandingPage,
PluginDefinition,
} from './plugins.js';
export type {
GraphQLRequestContextDidEncounterErrors,
Expand Down
21 changes: 10 additions & 11 deletions packages/server/src/externalTypes/plugins.ts
@@ -1,5 +1,6 @@
import type { KeyValueCache } from '@apollo/utils.keyvaluecache';
import type { Logger } from '@apollo/utils.logger';
import type { GraphQLResolveInfo, GraphQLSchema } from 'graphql';
import type { ApolloServer } from '../ApolloServer';
import type { ApolloConfig } from './constructor';
import type { BaseContext } from './context';
import type { GraphQLRequestContext, GraphQLResponse } from './graphql';
Expand All @@ -14,8 +15,10 @@ import type {
GraphQLRequestContextWillSendResponse,
} from './requestPipeline';

export interface GraphQLServerContext<TContext extends BaseContext> {
server: ApolloServer<TContext>;
export interface GraphQLServerContext {
readonly logger: Logger;
readonly cache: KeyValueCache<string>;

schema: GraphQLSchema;
apollo: ApolloConfig;
// TODO(AS4): Make sure we document that we removed `persistedQueries`.
Expand All @@ -28,15 +31,11 @@ export interface GraphQLSchemaContext {
coreSupergraphSdl?: string;
}

// A plugin can return an interface that matches `ApolloServerPlugin`, or a
// factory function that returns `ApolloServerPlugin`.
export type PluginDefinition<TContext extends BaseContext> =
| ApolloServerPlugin<TContext>
| (() => ApolloServerPlugin<TContext>);

export interface ApolloServerPlugin<in out TContext extends BaseContext> {
export interface ApolloServerPlugin<
in TContext extends BaseContext = BaseContext,
> {
serverWillStart?(
service: GraphQLServerContext<TContext>,
service: GraphQLServerContext,
): Promise<GraphQLServerListener | void>;

requestDidStart?(
Expand Down
6 changes: 3 additions & 3 deletions packages/server/src/plugin/cacheControl/index.ts
@@ -1,4 +1,4 @@
import type { ApolloServerPlugin, BaseContext } from '../../externalTypes';
import type { ApolloServerPlugin } from '../../externalTypes';
import {
DirectiveNode,
getNamedType,
Expand Down Expand Up @@ -47,9 +47,9 @@ export interface ApolloServerPluginCacheControlOptions {
__testing__cacheHints?: Map<string, CacheHint>;
}

export function ApolloServerPluginCacheControl<TContext extends BaseContext>(
export function ApolloServerPluginCacheControl(
options: ApolloServerPluginCacheControlOptions = Object.create(null),
): ApolloServerPlugin<TContext> {
): ApolloServerPlugin {
let typeAnnotationCache: LRUCache<GraphQLCompositeType, CacheAnnotation>;

let fieldAnnotationCache: LRUCache<
Expand Down
22 changes: 6 additions & 16 deletions packages/server/src/plugin/disabled/index.ts
Expand Up @@ -6,37 +6,27 @@ import type {
InternalPluginId,
} from '../../internalPlugin';

function disabledPlugin<TContext extends BaseContext>(
id: InternalPluginId,
): ApolloServerPlugin<TContext> {
const plugin: InternalApolloServerPlugin<TContext> = {
function disabledPlugin(id: InternalPluginId): ApolloServerPlugin {
const plugin: InternalApolloServerPlugin<BaseContext> = {
__internal_plugin_id__() {
return id;
},
};
return plugin;
}

export function ApolloServerPluginCacheControlDisabled<
TContext extends BaseContext,
>(): ApolloServerPlugin<TContext> {
export function ApolloServerPluginCacheControlDisabled(): ApolloServerPlugin<BaseContext> {
return disabledPlugin('CacheControl');
}

export function ApolloServerPluginInlineTraceDisabled<
TContext extends BaseContext,
>(): ApolloServerPlugin<TContext> {
export function ApolloServerPluginInlineTraceDisabled(): ApolloServerPlugin<BaseContext> {
return disabledPlugin('InlineTrace');
}

export function ApolloServerPluginLandingPageDisabled<
TContext extends BaseContext,
>(): ApolloServerPlugin<TContext> {
export function ApolloServerPluginLandingPageDisabled(): ApolloServerPlugin<BaseContext> {
return disabledPlugin('LandingPageDisabled');
}

export function ApolloServerPluginUsageReportingDisabled<
TContext extends BaseContext,
>(): ApolloServerPlugin<TContext> {
export function ApolloServerPluginUsageReportingDisabled(): ApolloServerPlugin<BaseContext> {
return disabledPlugin('UsageReporting');
}
6 changes: 3 additions & 3 deletions packages/server/src/plugin/drainHttpServer/index.ts
@@ -1,5 +1,5 @@
import type http from 'http';
import type { ApolloServerPlugin, BaseContext } from '../../externalTypes';
import type { ApolloServerPlugin } from '../../externalTypes';
import { Stopper } from './stoppable.js';

/**
Expand All @@ -23,9 +23,9 @@ export interface ApolloServerPluginDrainHttpServerOptions {
* See https://www.apollographql.com/docs/apollo-server/api/plugin/drain-http-server/
* for details.
*/
export function ApolloServerPluginDrainHttpServer<TContext extends BaseContext>(
export function ApolloServerPluginDrainHttpServer(
options: ApolloServerPluginDrainHttpServerOptions,
): ApolloServerPlugin<TContext> {
): ApolloServerPlugin {
const stopper = new Stopper(options.httpServer);
return {
async serverWillStart() {
Expand Down
10 changes: 5 additions & 5 deletions packages/server/src/plugin/inlineTrace/index.ts
Expand Up @@ -3,7 +3,7 @@ import { TraceTreeBuilder } from '../traceTreeBuilder.js';
import type { SendErrorsOptions } from '../usageReporting/index.js';
import { internalPlugin } from '../../internalPlugin.js';
import { schemaIsFederated } from '../schemaIsFederated.js';
import type { ApolloServerPlugin, BaseContext } from '../../externalTypes';
import type { ApolloServerPlugin } from '../../externalTypes';

export interface ApolloServerPluginInlineTraceOptions {
/**
Expand Down Expand Up @@ -44,23 +44,23 @@ export interface ApolloServerPluginInlineTraceOptions {
// on the `extensions`.`ftv1` property of the response. The Apollo Gateway
// utilizes this data to construct the full trace and submit it to Apollo's
// usage reporting ingress.
export function ApolloServerPluginInlineTrace<TContext extends BaseContext>(
export function ApolloServerPluginInlineTrace(
options: ApolloServerPluginInlineTraceOptions = Object.create(null),
): ApolloServerPlugin<TContext> {
): ApolloServerPlugin {
let enabled: boolean | null = options.__onlyIfSchemaIsFederated ? null : true;
return internalPlugin({
__internal_plugin_id__() {
return 'InlineTrace';
},
async serverWillStart({ schema, server }) {
async serverWillStart({ schema, logger }) {
// Handle the case that the plugin was implicitly installed. We only want it
// to actually be active if the schema appears to be federated. If you don't
// like the log line, just install `ApolloServerPluginInlineTrace()` in
// `plugins` yourself.
if (enabled === null) {
enabled = schemaIsFederated(schema);
if (enabled) {
server.logger.info(
logger.info(
'Enabling inline tracing for this federated service. To disable, use ' +
'ApolloServerPluginInlineTraceDisabled.',
);
Expand Down
12 changes: 4 additions & 8 deletions packages/server/src/plugin/landingPage/default/index.ts
Expand Up @@ -15,11 +15,9 @@ export type {
ApolloServerPluginLandingPageProductionDefaultOptions,
};

export function ApolloServerPluginLandingPageLocalDefault<
TContext extends BaseContext,
>(
export function ApolloServerPluginLandingPageLocalDefault(
options: ApolloServerPluginLandingPageLocalDefaultOptions = {},
): ApolloServerPlugin<TContext> {
): ApolloServerPlugin {
const { version, __internal_apolloStudioEnv__, ...rest } = {
// we default to Sandbox unless embed is specified as false
embed: true as const,
Expand All @@ -32,11 +30,9 @@ export function ApolloServerPluginLandingPageLocalDefault<
});
}

export function ApolloServerPluginLandingPageProductionDefault<
TContext extends BaseContext,
>(
export function ApolloServerPluginLandingPageProductionDefault(
options: ApolloServerPluginLandingPageProductionDefaultOptions = {},
): ApolloServerPlugin<TContext> {
): ApolloServerPlugin {
const { version, __internal_apolloStudioEnv__, ...rest } = options;
return ApolloServerPluginLandingPageDefault(version, {
isProd: true,
Expand Down

0 comments on commit 5015fe0

Please sign in to comment.