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

Custom directives are ignored when using the "relationship" directive. #1894

Closed
AccsoSG opened this issue Aug 12, 2022 · 5 comments
Closed
Labels
bug Something isn't working confirmed Confirmed bug

Comments

@AccsoSG
Copy link

AccsoSG commented Aug 12, 2022

Describe the bug
We use the graphql-query-complexity package for calculating and limiting the complexity of GraphQL queries in our API. There are various "estimators" that can be used to calculate the complexity. One of them is the directiveEstimator. With this we can use a directive "complexity" to assign a complexity to fields. This works perfectly with normal (scalar) fields. However, as soon as a "relationship" directive is used, the "complexity" directive is lost. I suspect that when the schema is generated, this information is lost.

Type definitions

type Person {
  id: ID!
  givenName: String!
  familyName: String @complexity(value: 2) #works as expected
  actorOf: [RolePlay!]! @relationship(type: "ROLE_PLAYED_BY", direction: IN) @complexity(value: 100)
}

type RolePlay {
  id: ID!
}

directive @complexity(
  # The complexity value for the field
  value: Int!

  # Optional multipliers
  multipliers: [String!]
) on FIELD_DEFINITION

To Reproduce
Steps to reproduce the behavior:

  1. Run a server with the following code...
const query = parse(`
  query {
    people {
      id
      givenName
      familyName
      actorOf {
        id
      }
    }
  }
`);

try {
  const complexity = getComplexity({
    estimators: [
      directiveEstimator(),
      simpleEstimator({ defaultComplexity: 1 })
    ],
    schema,
    query
  });

  console.log(complexity); // Output: 7
} catch (e) {
  console.error('Could not calculate complexity', e.message);
}
  1. See error
    The complexity was incorrectly calculated. It is 7 but should be 106.

Expected behavior
Custom directives should not be lost with a relationship directive.

Additional context
We don't want to use the directive to attach complexity to every field in our schema. Instead, we use a custom estimator that automatically sets the complexity for complex types (like RolePlay) higher than for scalar types. In exceptional cases, however, we want to specify the complexity for certain fields using the complexity directive.

@AccsoSG AccsoSG added the bug Something isn't working label Aug 12, 2022
@neo4j-team-graphql
Copy link
Collaborator

Many thanks for raising this bug report @AccsoSG. 🐛 We will now attempt to reproduce the bug based on the steps you have provided.

Please ensure that you've provided the necessary information for a minimal reproduction, including but not limited to:

  • Type definitions
  • Resolvers
  • Query and/or Mutation (or multiple) needed to reproduce

If you have a support agreement with Neo4j, please link this GitHub issue to a new or existing Zendesk ticket.

Thanks again! 🙏

@tbwiss tbwiss added the confirmed Confirmed bug label Aug 16, 2022
@neo4j-team-graphql
Copy link
Collaborator

We've been able to confirm this bug using the steps to reproduce that you provided - many thanks @AccsoSG! 🙏 We will now prioritise the bug and address it appropriately.

@tbwiss
Copy link
Contributor

tbwiss commented Aug 16, 2022

Thank you @AccsoSG for raising this bug!
Just want to drop the code here for someone else to reproduce this (in a single file):

const { Neo4jGraphQL } = require("@neo4j/graphql");
const { getComplexity, directiveEstimator, simpleEstimator } = require("graphql-query-complexity");
const { parse } = require('graphql');
const neo4j = require("neo4j-driver");

const driver = neo4j.driver("neo4j://localhost:7687", neo4j.auth.basic("neo4j", "xxxxxxx"));

async function main() {
    const typeDefs = `
        type Person {
            id: ID!
            givenName: String!
            familyName: String @complexity(value: 2) #works as expected
            actorOf: [RolePlay!]! @relationship(type: "ROLE_PLAYED_BY", direction: IN) @complexity(value: 100)
        }
        
        type RolePlay {
            id: ID!
        }

        directive @complexity(
            # The complexity value for the field
            value: Int!
        
            # Optional multipliers
            multipliers: [String!]
        ) on FIELD_DEFINITION
    `
    const neoSchema = new Neo4jGraphQL({ typeDefs, driver });

    const query = parse(`
        query {
            people {
                id
                givenName
                familyName
                actorOf {
                    id
                }
            }
        }
    `);

    try {
        const complexity = getComplexity({
            estimators: [
                directiveEstimator(),
                simpleEstimator({ defaultComplexity: 1 })
            ],
            schema: await neoSchema.schema,
            query
        });

        console.log(complexity); // Output: 7
    } catch (e) {
        console.error('Could not calculate complexity', e.message);
    }
}

main();

Run npm i @neo4j/graphql graphql-query-complexity neo4j-driver beforehand.

@a-alle
Copy link
Contributor

a-alle commented Feb 29, 2024

Just wanted to confirm the @complexity directive is being propagated in the resulted schema.
Generated type:

type Person {
  actorOf(directed: Boolean = true, options: RolePlayOptions, where: RolePlayWhere): [RolePlay!]! @complexity(value: 100)
  actorOfAggregate(directed: Boolean = true, where: RolePlayWhere): PersonRolePlayActorOfAggregationSelection
  actorOfConnection(after: String, directed: Boolean = true, first: Int, sort: [PersonActorOfConnectionSort!], where: PersonActorOfConnectionWhere): PersonActorOfConnection!
  familyName: String @complexity(value: 2)
  givenName: String!
  id: ID!
}

@darrellwarde
Copy link
Contributor

Closing this because it seems as though the directive is there in the output and should work as advertised - perhaps an issue with the cost calculation library itself?

@darrellwarde darrellwarde closed this as not planned Won't fix, can't repro, duplicate, stale Mar 5, 2024
@github-project-automation github-project-automation bot moved this to Low priority in Bug Triage Aug 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working confirmed Confirmed bug
Projects
Archived in project
Development

No branches or pull requests

5 participants