-
Notifications
You must be signed in to change notification settings - Fork 8
Stricter validation rules for auth directives, @cost, and @listSize
#215
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
Conversation
The `@cost` directive should not be applied to interface fields, field arguments, or the interface type itself. This commit adds tests and validation to enforce this rule. Additionally, it ensures that cost calculations are correctly applied when an object type implements an interface with `@cost` and `@key` directives. The behavior of `@listSize` is also fixed as previously `sizedFields` expected Integers, but now it expects a List
Bumped the Apollo composition package to the latest version.
Summary of ChangesHello @kamilkisiela, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces significant improvements to the composition and validation logic within the federation-composition library. It tightens the rules around how authentication and demand-related directives are applied and inherited, leading to a more robust and predictable supergraph schema. Additionally, it addresses a specific bug related to external fields and interface objects, enhancing overall schema integrity. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
@cost, and @listSize
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request introduces significant enhancements to composition and validation, particularly around authentication and demand-shaping directives. Key changes include stricter placement rules for auth directives, transitive authentication checks for @requires, and propagation of auth requirements through interface hierarchies. Additionally, it disallows @cost on interfaces and improves validation for @listSize. The implementation is comprehensive and includes extensive tests. I've identified a critical issue in the policy merging logic and a minor recurring typo in the test files.
n1ru4l
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
legito
gracito |
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 main, this PR will be updated. # Releases ## @theguild/[email protected] ### Minor Changes - [#215](#215) [`5edf421`](5edf421) Thanks [@kamilkisiela](https://github.com/kamilkisiela)! - **Enforce correct placement of auth directives.** A new validation rule (`AUTH_REQUIREMENTS_APPLIED_ON_INTERFACE`) rejects any attempt to put these directives on interfaces, interface fields or interface objects. **Add transitive-auth requirements checking.** A new rule verifies that any field using `@requires` specifies at least the auth requirements of the fields it selects. If a field doesn't carry forward the `@authenticated`, `@requiresScopes` or `@policy` requirements of its dependencies, composition fails with a `MISSING_TRANSITIVE_AUTH_REQUIREMENTS` error. **Propagate auth requirements through interface hierarchies.** Interface types and fields now inherit `@authenticated`, `@requiresScopes` and `@policy` from the object types that implement them. - [#215](#215) [`5edf421`](5edf421) Thanks [@kamilkisiela](https://github.com/kamilkisiela)! - Disallowed using `@cost` on interfaces - you can no longer place `@cost` on an interface type, its fields, or field arguments. Composition now fails with a clear error instead of accepting it silently. The `@listSize` directive now validates that `sizedFields` point to list fields, not integer counters (e.g., use `edges` instead of `count`). Added validation for `slicingArguments` in `@listSize`. Only arguments that exist in all subgraphs are kept, invalid ones trigger an error. ### Patch Changes - [#215](#215) [`5edf421`](5edf421) Thanks [@kamilkisiela](https://github.com/kamilkisiela)! - The `EXTERNAL_MISSING_ON_BASE` rule has been updated to handle `@interfaceObject` corner‑cases, like `@external` fields on object types, but provided by interface objects, were triggering false positives. Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Auth Directives
@authenticated,@requiresScopes,@policy) by rejecting any attempt to place them on interfaces, interface fields, or interface objects.@requiresspecify at least the auth requirements of the fields they selectDemand Directives
@coston interfaces - composition now fails with a clear error instead of accepting it silently@listSizesizedFieldsto ensure they point to list fields, not integer counters@listSizeslicingArgumentsto ensure only arguments that exist in all subgraphs are keptBug Fixes
EXTERNAL_MISSING_ON_BASErule has been updated to handle@interfaceObjectcorner‑cases, like@externalfields on object types, but provided by interface objects, were triggering false positives.