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

Allow known FeatureDefinition subclasses to define custom subgraph schema validation rules #2910

Merged
merged 17 commits into from
Jan 19, 2024

Conversation

benjamn
Copy link
Member

@benjamn benjamn commented Jan 16, 2024

There are a number of // TODO comments left in this implementation, but this PR demonstrates a basic framework for allowing known FeatureDefinition subclasses, like SourceSpecDefinition, to define their own subgraph validation rules, by overriding the validateSubgraphSchema(schema: Schema): GraphQLError[] method.

It's the responsibility of validateSubgraphSchema to determine if its validation rules are applicable to a given schema, ideally returning an empty array quickly if the feature in question is not used in the schema.

In order for validateKnownFeatures to work, a given FeatureDefinition subclass needs to have been registered with registerKnownFeature.

I'll keep working on the TODOs and the tests.

@benjamn benjamn self-assigned this Jan 16, 2024

This comment was marked as resolved.

Copy link

codesandbox-ci bot commented Jan 16, 2024

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

package.json Outdated Show resolved Hide resolved
internals-js/src/specs/sourceSpec.ts Outdated Show resolved Hide resolved
internals-js/src/specs/sourceSpec.ts Show resolved Hide resolved
internals-js/src/specs/sourceSpec.ts Outdated Show resolved Hide resolved
internals-js/src/specs/sourceSpec.ts Outdated Show resolved Hide resolved
internals-js/src/specs/sourceSpec.ts Outdated Show resolved Hide resolved

if (sourceAPIInSchema) {
sourceAPIInSchema.applications().forEach(application => {
const { name, ...rest } = application.arguments();
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a reason you're doing this as opposed to const { name, http }?

Copy link
Member Author

Choose a reason for hiding this comment

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

Just trying to be agnostic to eventually having more than just http in KNOWN_SOURCE_PROTOCOLS.

Copy link
Contributor

Choose a reason for hiding this comment

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

It's just style, so I'm not going to hold up the release for it, but imo it's a little cleaner to say

const { api, http, grpc, sql } = application.arguments();
if (http) {
  assert(apiNameToProtocol.get(api) === 'http', ...);
} else if (grpc) {
  assert(apiNameToProtocol.get(api) === 'grpc', ...);
...
}

internals-js/src/specs/sourceSpec.ts Outdated Show resolved Hide resolved
internals-js/src/specs/sourceSpec.ts Show resolved Hide resolved
internals-js/src/specs/sourceSpec.ts Outdated Show resolved Hide resolved
@benjamn benjamn force-pushed the benjamn/validate-@source-directives branch from f9d1d1b to 5ffb16f Compare January 19, 2024 20:01
@benjamn benjamn marked this pull request as ready for review January 19, 2024 20:23
@benjamn benjamn requested review from a team as code owners January 19, 2024 20:23
@benjamn benjamn requested a review from clenfest January 19, 2024 20:24
docs/source/errors.md Outdated Show resolved Hide resolved
@benjamn benjamn merged commit 66833fb into next Jan 19, 2024
15 checks passed
@benjamn benjamn deleted the benjamn/validate-@source-directives branch January 19, 2024 21:20
trevor-scheer pushed a commit that referenced this pull request Jan 20, 2024
This PR was opened by the [Changesets
release](https://github.com/changesets/action) GitHub action. When
you're ready to do a release, you can merge this and the packages will
be published to npm automatically. If you're not ready to do a release
yet, that's fine, whenever you add more changesets to next, this PR will
be updated.


# Releases
## @apollo/composition@2.7.0

### Minor Changes

- Implement progressive `@override` functionality
([#2911](#2911))

The progressive `@override` feature brings a new argument to the
`@override` directive: `label: String`. When a label is added to an
`@override` application, the override becomes conditional, depending on
parameters provided to the query planner (a set of which labels should
be overridden). Note that this feature will be supported in router for
enterprise users only.

Out-of-the-box, the router will support a percentage-based use case for
progressive `@override`. For example:

    ```graphql
    type Query {
      hello: String @OverRide(from: "original", label: "percent(5)")
    }
    ```

The above example will override the root `hello` field from the
"original" subgraph 5% of the time.

More complex use cases will be supported by the router via the use of
coprocessors/rhai to resolve arbitrary labels to true/false values (i.e.
via a feature flag service).

- Support `@join__directive(graphs, name, args)` directives
([#2894](#2894))

### Patch Changes

- Allow known `FeatureDefinition` subclasses to define custom subgraph
schema validation rules
([#2910](#2910))

- Updated dependencies
\[[`6ae42942b13dccd246ccc994faa2cb36cd62cb3c`](6ae4294),
[`66833fb8d04c9376f6ed476fed6b1ca237f477b7`](66833fb),
[`931f87c6766c7439936df706727cbdc0cd6bcfd8`](931f87c)]:
    -   @apollo/query-graphs@2.7.0
    -   @apollo/federation-internals@2.7.0

## @apollo/gateway@2.7.0

### Minor Changes

- Implement progressive `@override` functionality
([#2911](#2911))

The progressive `@override` feature brings a new argument to the
`@override` directive: `label: String`. When a label is added to an
`@override` application, the override becomes conditional, depending on
parameters provided to the query planner (a set of which labels should
be overridden). Note that this feature will be supported in router for
enterprise users only.

Out-of-the-box, the router will support a percentage-based use case for
progressive `@override`. For example:

    ```graphql
    type Query {
      hello: String @OverRide(from: "original", label: "percent(5)")
    }
    ```

The above example will override the root `hello` field from the
"original" subgraph 5% of the time.

More complex use cases will be supported by the router via the use of
coprocessors/rhai to resolve arbitrary labels to true/false values (i.e.
via a feature flag service).

### Patch Changes

- Updated dependencies
\[[`6ae42942b13dccd246ccc994faa2cb36cd62cb3c`](6ae4294),
[`66833fb8d04c9376f6ed476fed6b1ca237f477b7`](66833fb),
[`931f87c6766c7439936df706727cbdc0cd6bcfd8`](931f87c)]:
    -   @apollo/query-planner@2.7.0
    -   @apollo/composition@2.7.0
    -   @apollo/federation-internals@2.7.0

## @apollo/federation-internals@2.7.0

### Minor Changes

- Implement progressive `@override` functionality
([#2911](#2911))

The progressive `@override` feature brings a new argument to the
`@override` directive: `label: String`. When a label is added to an
`@override` application, the override becomes conditional, depending on
parameters provided to the query planner (a set of which labels should
be overridden). Note that this feature will be supported in router for
enterprise users only.

Out-of-the-box, the router will support a percentage-based use case for
progressive `@override`. For example:

    ```graphql
    type Query {
      hello: String @OverRide(from: "original", label: "percent(5)")
    }
    ```

The above example will override the root `hello` field from the
"original" subgraph 5% of the time.

More complex use cases will be supported by the router via the use of
coprocessors/rhai to resolve arbitrary labels to true/false values (i.e.
via a feature flag service).

- Allow known `FeatureDefinition` subclasses to define custom subgraph
schema validation rules
([#2910](#2910))

- Support `@join__directive(graphs, name, args)` directives
([#2894](#2894))

## @apollo/query-graphs@2.7.0

### Minor Changes

- Implement progressive `@override` functionality
([#2911](#2911))

The progressive `@override` feature brings a new argument to the
`@override` directive: `label: String`. When a label is added to an
`@override` application, the override becomes conditional, depending on
parameters provided to the query planner (a set of which labels should
be overridden). Note that this feature will be supported in router for
enterprise users only.

Out-of-the-box, the router will support a percentage-based use case for
progressive `@override`. For example:

    ```graphql
    type Query {
      hello: String @OverRide(from: "original", label: "percent(5)")
    }
    ```

The above example will override the root `hello` field from the
"original" subgraph 5% of the time.

More complex use cases will be supported by the router via the use of
coprocessors/rhai to resolve arbitrary labels to true/false values (i.e.
via a feature flag service).

### Patch Changes

- Updated dependencies
\[[`6ae42942b13dccd246ccc994faa2cb36cd62cb3c`](6ae4294),
[`66833fb8d04c9376f6ed476fed6b1ca237f477b7`](66833fb),
[`931f87c6766c7439936df706727cbdc0cd6bcfd8`](931f87c)]:
    -   @apollo/federation-internals@2.7.0

## @apollo/query-planner@2.7.0

### Minor Changes

- Implement progressive `@override` functionality
([#2911](#2911))

The progressive `@override` feature brings a new argument to the
`@override` directive: `label: String`. When a label is added to an
`@override` application, the override becomes conditional, depending on
parameters provided to the query planner (a set of which labels should
be overridden). Note that this feature will be supported in router for
enterprise users only.

Out-of-the-box, the router will support a percentage-based use case for
progressive `@override`. For example:

    ```graphql
    type Query {
      hello: String @OverRide(from: "original", label: "percent(5)")
    }
    ```

The above example will override the root `hello` field from the
"original" subgraph 5% of the time.

More complex use cases will be supported by the router via the use of
coprocessors/rhai to resolve arbitrary labels to true/false values (i.e.
via a feature flag service).

### Patch Changes

- Updated dependencies
\[[`6ae42942b13dccd246ccc994faa2cb36cd62cb3c`](6ae4294),
[`66833fb8d04c9376f6ed476fed6b1ca237f477b7`](66833fb),
[`931f87c6766c7439936df706727cbdc0cd6bcfd8`](931f87c)]:
    -   @apollo/query-graphs@2.7.0
    -   @apollo/federation-internals@2.7.0

## @apollo/subgraph@2.7.0

### Minor Changes

- Implement progressive `@override` functionality
([#2911](#2911))

The progressive `@override` feature brings a new argument to the
`@override` directive: `label: String`. When a label is added to an
`@override` application, the override becomes conditional, depending on
parameters provided to the query planner (a set of which labels should
be overridden). Note that this feature will be supported in router for
enterprise users only.

Out-of-the-box, the router will support a percentage-based use case for
progressive `@override`. For example:

    ```graphql
    type Query {
      hello: String @OverRide(from: "original", label: "percent(5)")
    }
    ```

The above example will override the root `hello` field from the
"original" subgraph 5% of the time.

More complex use cases will be supported by the router via the use of
coprocessors/rhai to resolve arbitrary labels to true/false values (i.e.
via a feature flag service).

### Patch Changes

- Updated dependencies
\[[`6ae42942b13dccd246ccc994faa2cb36cd62cb3c`](6ae4294),
[`66833fb8d04c9376f6ed476fed6b1ca237f477b7`](66833fb),
[`931f87c6766c7439936df706727cbdc0cd6bcfd8`](931f87c)]:
    -   @apollo/federation-internals@2.7.0

## apollo-federation-integration-testsuite@2.7.0

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
benjamn added a commit that referenced this pull request Jan 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants