graphql-ruby icon indicating copy to clipboard operation
graphql-ruby copied to clipboard

Type comparison missing from `FieldsWillMerge` validation

Open gmac opened this issue 2 years ago • 0 comments

Describe the bug

FieldsWillMerge.find_conflict omits the type parity check implemented by graphql-js. This deviates the GraphQL Ruby implementation from spec compliance. Unfortunately, fixing this issue is a breaking change because existing queries will start getting rejected with a validation error.

This issue has been discussed and some work has been done. Steps to resolution:

  • Partial fix by @rmosolgo done in https://github.com/rmosolgo/graphql-ruby/pull/4347.
  • Some adjustments needed; fields? checks should use leaf? instead.

GraphQL schema

class TestSchema < GraphQL::Schema
  class Decimal < GraphQL::Schema::Scalar
    graphql_name "Decimal"
  end

  class MoneyV2 < GraphQL::Schema::Object
    graphql_name "MoneyV2"
    field :amount, Decimal, null: false
  end

  class PricingPercentageValue < GraphQL::Schema::Object
    graphql_name "PricingPercentageValue"
    field :percentage, Float, null: false
  end

  class PricingValue < GraphQL::Schema::Union
    graphql_name "PricingValue"
    possible_types MoneyV2, PricingPercentageValue

    def self.resolve_type(obj, ctx)
      {
        "MoneyV2" => MoneyV2,
        "PricingPercentageValue" => PricingPercentageValue,
      }.fetch(obj[:__typename])
    end
  end

  class Query < GraphQL::Schema::Object
    graphql_name "Query"
    field :pricing_values, [PricingValue], null: false

    def pricing_values
      [
        { amount: "1.0", __typename: "MoneyV2" },
        { percentage: 1.0, __typename: "PricingPercentageValue" }
      ]
    end
  end

  query Query
end

GraphQL query

query = <<~GRAPHQL
  {
    pricingValues {
      __typename
      ...on MoneyV2 {
        discountValue: amount
      }
      ...on PricingPercentageValue {
        discountValue: percentage
      }
    }
  }
GRAPHQL

result = TestSchema.execute(query, validate: true)
pp result.to_h

Expected behavior

The following error is logged for this same composition from graphql-js. It's triggered by the types check in OverlappingFieldsCanBeMergedRule, which is not implemented in GraphQL Ruby.

{
  "errors": [
    {
      "message": "Fields \"discountValue\" conflict because they return conflicting types \"Decimal!\" and \"Float!\". Use different aliases on the fields to fetch both if this was intentional.",
      "locations": [
        {
          "line": 5,
          "column": 7
        },
        {
          "line": 8,
          "column": 7
        }
      ]
    }
  ]
}

Actual behavior

This test succeeds without validation error, because the relevant rules are missing from GraphQL Ruby:

{"data"=>
  {"pricingValues"=>
    [{"__typename"=>"MoneyV2", "discountValue"=>"1.0"},
     {"__typename"=>"PricingPercentageValue", "discountValue"=>1.0}]}}

gmac avatar Mar 24 '23 16:03 gmac