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

[chore][GraphQL/Limits] Separate QueryLimitChecker extension/factory #18660

Merged
merged 6 commits into from
Jul 16, 2024

Commits on Jul 16, 2024

  1. [chore][GraphQL/Limits] Separate QueryLimitChecker extension/factory (#…

    …18660)
    
    ## Description
    
    Split up the extension factory from the extension itself, similar to
    what we did for `Timeout` before. This avoids the confusion of the
    single type being created with defaulted fields to act as the factory
    and then creating new versions of itself to act as the extension.
    
    ## Test plan
    
    CI
    
    ---
    
    ## Release notes
    
    Check each box that your changes affect. If none of the boxes relate to
    your changes, release notes aren't required.
    
    For each box you select, include information after the relevant heading
    that describes the impact of your changes that a user might notice and
    any actions they must take to implement updates.
    
    - [ ] Protocol:
    - [ ] Nodes (Validators and Full nodes):
    - [ ] Indexer:
    - [ ] JSON-RPC:
    - [ ] GraphQL:
    - [ ] CLI:
    - [ ] Rust SDK:
    amnn committed Jul 16, 2024
    Configuration menu
    Copy the full SHA
    a67e7c2 View commit details
    Browse the repository at this point in the history
  2. [GraphQL/Limits] BUGFIX: directives test error type (#18661)

    ## Description
    
    Passing an unsupported directive should be a user input error, not an
    internal error.
    
    ## Test plan
    
    ```
    sui-graphql-e2e-tests$ cargo nextest run --features pg_integration -- limits/directives
    ```
    
    ## Stack
    - #18660
    
    ---
    
    ## Release notes
    
    Check each box that your changes affect. If none of the boxes relate to
    your changes, release notes aren't required.
    
    For each box you select, include information after the relevant heading
    that describes the impact of your changes that a user might notice and
    any actions they must take to implement updates.
    
    - [ ] Protocol: 
    - [ ] Nodes (Validators and Full nodes): 
    - [ ] Indexer: 
    - [ ] JSON-RPC: 
    - [x] GraphQL: Passing an unsupported directive to the service will be
    treated as a `BAD_USER_INPUT` rather than an `INTERNAL_SERVER_ERROR`.
    - [ ] CLI: 
    - [ ] Rust SDK:
    amnn authored Jul 16, 2024
    Configuration menu
    Copy the full SHA
    887812f View commit details
    Browse the repository at this point in the history
  3. [chore][GraphQL/Limits] Clean up headers (#18662)

    ## Description
    
    Two header related clean-ups:
    
    - The version header should no longer be part of the CORS configuration,
    because we don't expect versions to be configured by (request) header.
    - The `ShowUsage` type doesn't need to implement `Header` because we
    only care about comparing its name.
    
    ## Test plan
    
    CI
    
    ## Stack
    - #18660
    - #18661 
    
    ---
    
    ## Release notes
    
    Check each box that your changes affect. If none of the boxes relate to
    your changes, release notes aren't required.
    
    For each box you select, include information after the relevant heading
    that describes the impact of your changes that a user might notice and
    any actions they must take to implement updates.
    
    - [ ] Protocol: 
    - [ ] Nodes (Validators and Full nodes): 
    - [ ] Indexer: 
    - [ ] JSON-RPC: 
    - [x] GraphQL: `x-sui-rpc-version` is no longer an accepted request
    header, as versions are now selected by modifying the path.
    - [ ] CLI: 
    - [ ] Rust SDK:
    amnn authored Jul 16, 2024
    Configuration menu
    Copy the full SHA
    b124631 View commit details
    Browse the repository at this point in the history
  4. [chore][GraphQL/Limits] Standardise query_limits_checker file order (#…

    …18663)
    
    ## Description
    
    This is a file order change only, with no other meaningful changes. It
    standardises the order to:
    
    - Constants
    - Types
    - Impls
    - Trait Impls
    - Free functions
    
    ## Test plan
    
    CI
    
    ## Stack
    - #18660
    - #18661 
    - #18662 
    
    ---
    
    ## Release notes
    
    Check each box that your changes affect. If none of the boxes relate to
    your changes, release notes aren't required.
    
    For each box you select, include information after the relevant heading
    that describes the impact of your changes that a user might notice and
    any actions they must take to implement updates.
    
    - [ ] Protocol: 
    - [ ] Nodes (Validators and Full nodes): 
    - [ ] Indexer: 
    - [ ] JSON-RPC: 
    - [ ] GraphQL: 
    - [ ] CLI: 
    - [ ] Rust SDK:
    amnn authored Jul 16, 2024
    Configuration menu
    Copy the full SHA
    08c59c9 View commit details
    Browse the repository at this point in the history
  5. [GraphQL/Limits] Separate out directive checks (#18664)

    ## Description
    
    Trying to do the directive checks at the same time as the query checks
    complicates both implementations. Split out the directive check into its
    own extension.
    
    Also fix the directive checks not looking at directives on variable
    definitions.
    
    ## Test plan
    
    ```
    sui-graphql-e2e-tests$ cargo nextest run \
      --features pg_integration              \
      -- limits/directives.move
    ```
    
    ## Stack
    - #18660
    - #18661 
    - #18662 
    - #18663 
    
    ---
    
    ## Release notes
    
    Check each box that your changes affect. If none of the boxes relate to
    your changes, release notes aren't required.
    
    For each box you select, include information after the relevant heading
    that describes the impact of your changes that a user might notice and
    any actions they must take to implement updates.
    
    - [ ] Protocol: 
    - [ ] Nodes (Validators and Full nodes): 
    - [ ] Indexer: 
    - [ ] JSON-RPC: 
    - [x] GraphQL: The service now detects unsupported directives on query
    variable definitions.
    - [ ] CLI: 
    - [ ] Rust SDK:
    amnn authored Jul 16, 2024
    Configuration menu
    Copy the full SHA
    f13bc4d View commit details
    Browse the repository at this point in the history
  6. [GraphQL/Limits] Reimplement QueryLimitsChecker (#18666)

    ## Description
    
    Rewriting query limits checker to land a number of improvements and
    fixes:
    
    - Avoid issues with overflows by counting down from a predefined budget,
    rather than counting up to the limit and protecting multiplications
    using `checked_mul`.
    
    - Improve detection of paginated fields: 
    - Previously we treated all connections-related fields as appearing as
    many times as the page size (including the field that introduced the
    connection, and the `pageInfo` field). This was over-approximated the
    output size by a large margin. The new approach counts exactly the
    number of nodes in the output: The connection's root field, and any
    non-`edges` or `nodes` field will not get multiplied by the page size.
    - The checker now also detects connections-related fields even if they
    are obscured by fragment or inline fragment spreads.
    
    - Tighter `__schema` query detection: Previously we would skip requests
    that started with a `__schema` introspection query. Now it's required to
    be the only operation in the request (not just the first).
    
    - Fix metrics collection after limits are hit: Previously, if a limit
    was hit, we would not observe validation-related metrics in prometheus.
    Now we will always record such metrics, and if a limit has been hit, it
    will register as being "at" the limit.
    
    ## Test plan
    
    ```
    sui-graphql-e2e-tests$ cargo nextest run --features pg_integration -- limits/
    ```
    
    ## Stack
    
    - #18660 
    - #18661 
    - #18662
    - #18663 
    - #18664 
    
    ---
    
    ## Release notes
    
    Check each box that your changes affect. If none of the boxes relate to
    your changes, release notes aren't required.
    
    For each box you select, include information after the relevant heading
    that describes the impact of your changes that a user might notice and
    any actions they must take to implement updates.
    
    - [ ] Protocol: 
    - [ ] Nodes (Validators and Full nodes): 
    - [ ] Indexer: 
    - [ ] JSON-RPC: 
    - [x] GraphQL: Output node estimation has been made more accurate -- the
    estimate should now track the theoretical max number of nodes on the
    JSON `data` output.
    - [ ] CLI: 
    - [ ] Rust SDK:
    amnn authored Jul 16, 2024
    Configuration menu
    Copy the full SHA
    14790e0 View commit details
    Browse the repository at this point in the history