[Fix] Querying same field multiple times causes an error

[edgarsn]

Summary

Querying same field (e.g. id) multiple times causes an error because of array_merge_recursive merging values into array.

Failing GraphQL example:

query MyQuery {
  show(slug: "news") {
    id
    title
    ...Base
    ...Base2
  }
}

fragment Base on Show {
  id
}

fragment Base2 on Show {
  id
}

as you can see - id is queried 3 times and that's causing an error shown below.

Laravel Error response

{
  "errors": [
    {
      "debugMessage": "Call to undefined method App\\Orion\\Modules\\Shows\\Models\\Show::id()",
      "message": "Internal server error",
      "extensions": {
        "category": "internal"
      },
      "locations": [
        {
          "line": 2,
          "column": 3
        }
      ],
      "path": [
        "show"
      ],
      "trace": [
        {
          "file": "/home/vagrant/code/vendor/laravel/framework/src/Illuminate/Support/Traits/ForwardsCalls.php",
          "line": 36,
          "call": "Illuminate\\Database\\Eloquent\\Model::throwBadMethodCallException('id')"
        },
        {
          "file": "/home/vagrant/code/vendor/laravel/framework/src/Illuminate/Database/Eloquent/Model.php",
          "line": 1618,
          "call": "Illuminate\\Database\\Eloquent\\Model::forwardCallTo(instance of Illuminate\\Database\\Eloquent\\Builder, 'id', array(0))"
        },
        {
          "call": "Illuminate\\Database\\Eloquent\\Model::__call('id', array(0))"
        },
        {
          "file": "/home/vagrant/code/vendor/rebing/graphql-laravel/src/Support/SelectFields.php",
          "line": 181,
          "function": "call_user_func(array(2))"
        },
        {
          "file": "/home/vagrant/code/vendor/rebing/graphql-laravel/src/Support/SelectFields.php",
          "line": 94,
          "call": "Rebing\\GraphQL\\Support\\SelectFields::handleFields(array(1), array(2), GraphQLType: Show, array(0), array(0), null)"
        },
        {
          "file": "/home/vagrant/code/vendor/rebing/graphql-laravel/src/Support/SelectFields.php",
          "line": 48,
          "call": "Rebing\\GraphQL\\Support\\SelectFields::getSelectableFieldsAndRelations(array(1), array(2), GraphQLType: Show, null, true, null)"
        },
        {
          "file": "/home/vagrant/code/vendor/rebing/graphql-laravel/src/Support/Field.php",
          "line": 198,
          "call": "Rebing\\GraphQL\\Support\\SelectFields::__construct(instance of GraphQL\\Type\\Definition\\ResolveInfo, GraphQLType: Show, array(1), 5, null)"
        },
        {
          "file": "/home/vagrant/code/app/GraphQL/Query/Shows/ShowQuery.php",
          "line": 59,
          "call": "Rebing\\GraphQL\\Support\\Field::Rebing\\GraphQL\\Support\\{closure}()"
        },
        {
          "call": "App\\GraphQL\\Query\\Shows\\ShowQuery::resolve(null, array(1), null, instance of GraphQL\\Type\\Definition\\ResolveInfo, instance of Closure)"
        },
        {
          "file": "/home/vagrant/code/vendor/rebing/graphql-laravel/src/Support/Field.php",
          "line": 207,
          "function": "call_user_func_array(array(2), array(5))"
        },
        {
          "file": "/home/vagrant/code/vendor/webonyx/graphql-php/src/Executor/ReferenceExecutor.php",
          "line": 632,
          "call": "Rebing\\GraphQL\\Support\\Field::Rebing\\GraphQL\\Support\\{closure}(null, array(1), null, instance of GraphQL\\Type\\Definition\\ResolveInfo)"
        },
        {
          "file": "/home/vagrant/code/vendor/webonyx/graphql-php/src/Executor/ReferenceExecutor.php",
          "line": 560,
          "call": "GraphQL\\Executor\\ReferenceExecutor::resolveOrError(instance of GraphQL\\Type\\Definition\\FieldDefinition, instance of GraphQL\\Language\\AST\\FieldNode, instance of Closure, null, null, instance of GraphQL\\Type\\Definition\\ResolveInfo)"
        },
        {
          "file": "/home/vagrant/code/vendor/webonyx/graphql-php/src/Executor/ReferenceExecutor.php",
          "line": 1247,
          "call": "GraphQL\\Executor\\ReferenceExecutor::resolveField(GraphQLType: Query, null, instance of ArrayObject(1), array(1))"
        },
        {
          "file": "/home/vagrant/code/vendor/webonyx/graphql-php/src/Executor/ReferenceExecutor.php",
          "line": 257,
          "call": "GraphQL\\Executor\\ReferenceExecutor::executeFields(GraphQLType: Query, null, array(0), instance of ArrayObject(1))"
        },
        {
          "file": "/home/vagrant/code/vendor/webonyx/graphql-php/src/Executor/ReferenceExecutor.php",
          "line": 208,
          "call": "GraphQL\\Executor\\ReferenceExecutor::executeOperation(instance of GraphQL\\Language\\AST\\OperationDefinitionNode, null)"
        },
        {
          "file": "/home/vagrant/code/vendor/webonyx/graphql-php/src/Executor/Executor.php",
          "line": 155,
          "call": "GraphQL\\Executor\\ReferenceExecutor::doExecute()"
        },
        {
          "file": "/home/vagrant/code/vendor/webonyx/graphql-php/src/GraphQL.php",
          "line": 165,
          "call": "GraphQL\\Executor\\Executor::promiseToExecute(instance of GraphQL\\Executor\\Promise\\Adapter\\SyncPromiseAdapter, instance of GraphQL\\Type\\Schema, instance of GraphQL\\Language\\AST\\DocumentNode, null, null, array(0), 'MyQuery', null)"
        },
        {
          "file": "/home/vagrant/code/vendor/webonyx/graphql-php/src/GraphQL.php",
          "line": 98,
          "call": "GraphQL\\GraphQL::promiseToExecute(instance of GraphQL\\Executor\\Promise\\Adapter\\SyncPromiseAdapter, instance of GraphQL\\Type\\Schema, 'query MyQuery {\n  show(slug: \"aizliegtais-panemiens\") {\n    id\n    title\n    ...Base\n    ...Base2\n  }\n}\n\nfragment Base on Show {\n  id\n}\n\nfragment Base2 on Show {\n  id\n}', null, null, array(0), 'MyQuery', null, null)"
        },
        {
          "file": "/home/vagrant/code/vendor/rebing/graphql-laravel/src/GraphQL.php",
          "line": 145,
          "call": "GraphQL\\GraphQL::executeQuery(instance of GraphQL\\Type\\Schema, 'query MyQuery {\n  show(slug: \"aizliegtais-panemiens\") {\n    id\n    title\n    ...Base\n    ...Base2\n  }\n}\n\nfragment Base on Show {\n  id\n}\n\nfragment Base2 on Show {\n  id\n}', null, null, array(0), 'MyQuery', null)"
        },
        {
          "file": "/home/vagrant/code/vendor/rebing/graphql-laravel/src/GraphQL.php",
          "line": 123,
          "call": "Rebing\\GraphQL\\GraphQL::queryAndReturnResult('query MyQuery {\n  show(slug: \"aizliegtais-panemiens\") {\n    id\n    title\n    ...Base\n    ...Base2\n  }\n}\n\nfragment Base on Show {\n  id\n}\n\nfragment Base2 on Show {\n  id\n}', array(0), array(3))"
        },
        {
          "file": "/home/vagrant/code/vendor/rebing/graphql-laravel/src/GraphQLController.php",
          "line": 76,
          "call": "Rebing\\GraphQL\\GraphQL::query('query MyQuery {\n  show(slug: \"aizliegtais-panemiens\") {\n    id\n    title\n    ...Base\n    ...Base2\n  }\n}\n\nfragment Base on Show {\n  id\n}\n\nfragment Base2 on Show {\n  id\n}', array(0), array(3))"
        },
        {
          "file": "/home/vagrant/code/vendor/rebing/graphql-laravel/src/GraphQLController.php",
          "line": 49,
          "call": "Rebing\\GraphQL\\GraphQLController::executeQuery('default', array(3))"
        },
        {
          "call": "Rebing\\GraphQL\\GraphQLController::query(instance of Illuminate\\Http\\Request, 'default')"
        },
        {
          "file": "/home/vagrant/code/vendor/laravel/framework/src/Illuminate/Routing/Controller.php",
          "line": 54,
          "function": "call_user_func_array(array(2), array(2))"
        },
        {
          "file": "/home/vagrant/code/vendor/laravel/framework/src/Illuminate/Routing/ControllerDispatcher.php",
          "line": 45,
          "call": "Illuminate\\Routing\\Controller::callAction('query', array(2))"
        },
        {
          "file": "/home/vagrant/code/vendor/laravel/framework/src/Illuminate/Routing/Route.php",
          "line": 219,
          "call": "Illuminate\\Routing\\ControllerDispatcher::dispatch(instance of Illuminate\\Routing\\Route, instance of Rebing\\GraphQL\\GraphQLController, 'query')"
        },
        {
          "file": "/home/vagrant/code/vendor/laravel/framework/src/Illuminate/Routing/Route.php",
          "line": 176,
          "call": "Illuminate\\Routing\\Route::runController()"
        },
        {
          "file": "/home/vagrant/code/vendor/laravel/framework/src/Illuminate/Routing/Router.php",
          "line": 680,
          "call": "Illuminate\\Routing\\Route::run()"
        },
        {
          "file": "/home/vagrant/code/vendor/laravel/framework/src/Illuminate/Pipeline/Pipeline.php",
          "line": 130,
          "call": "Illuminate\\Routing\\Router::Illuminate\\Routing\\{closure}(instance of Illuminate\\Http\\Request)"
        },
        {
          "file": "/home/vagrant/code/vendor/barryvdh/laravel-cors/src/HandleCors.php",
          "line": 58,
          "call": "Illuminate\\Pipeline\\Pipeline::Illuminate\\Pipeline\\{closure}(instance of Illuminate\\Http\\Request)"
        },
        {
          "file": "/home/vagrant/code/vendor/laravel/framework/src/Illuminate/Pipeline/Pipeline.php",
          "line": 171,
          "call": "Barryvdh\\Cors\\HandleCors::handle(instance of Illuminate\\Http\\Request, instance of Closure)"
        },
        {
          "file": "/home/vagrant/code/vendor/laravel/framework/src/Illuminate/Pipeline/Pipeline.php",
          "line": 105,
          "call": "Illuminate\\Pipeline\\Pipeline::Illuminate\\Pipeline\\{closure}(instance of Illuminate\\Http\\Request)"
        },
        {
          "file": "/home/vagrant/code/vendor/laravel/framework/src/Illuminate/Routing/Router.php",
          "line": 682,
          "call": "Illuminate\\Pipeline\\Pipeline::then(instance of Closure)"
        },
        {
          "file": "/home/vagrant/code/vendor/laravel/framework/src/Illuminate/Routing/Router.php",
          "line": 657,
          "call": "Illuminate\\Routing\\Router::runRouteWithinStack(instance of Illuminate\\Routing\\Route, instance of Illuminate\\Http\\Request)"
        },
        {
          "file": "/home/vagrant/code/vendor/laravel/framework/src/Illuminate/Routing/Router.php",
          "line": 623,
          "call": "Illuminate\\Routing\\Router::runRoute(instance of Illuminate\\Http\\Request, instance of Illuminate\\Routing\\Route)"
        },
        {
          "file": "/home/vagrant/code/vendor/laravel/framework/src/Illuminate/Routing/Router.php",
          "line": 612,
          "call": "Illuminate\\Routing\\Router::dispatchToRoute(instance of Illuminate\\Http\\Request)"
        },
        {
          "file": "/home/vagrant/code/vendor/laravel/framework/src/Illuminate/Foundation/Http/Kernel.php",
          "line": 176,
          "call": "Illuminate\\Routing\\Router::dispatch(instance of Illuminate\\Http\\Request)"
        },
        {
          "file": "/home/vagrant/code/vendor/laravel/framework/src/Illuminate/Pipeline/Pipeline.php",
          "line": 130,
          "call": "Illuminate\\Foundation\\Http\\Kernel::Illuminate\\Foundation\\Http\\{closure}(instance of Illuminate\\Http\\Request)"
        },
        {
          "file": "/home/vagrant/code/vendor/fideloper/proxy/src/TrustProxies.php",
          "line": 57,
          "call": "Illuminate\\Pipeline\\Pipeline::Illuminate\\Pipeline\\{closure}(instance of Illuminate\\Http\\Request)"
        },
        {
          "file": "/home/vagrant/code/vendor/laravel/framework/src/Illuminate/Pipeline/Pipeline.php",
          "line": 171,
          "call": "Fideloper\\Proxy\\TrustProxies::handle(instance of Illuminate\\Http\\Request, instance of Closure)"
        },
        {
          "file": "/home/vagrant/code/vendor/laravel/framework/src/Illuminate/Foundation/Http/Middleware/TransformsRequest.php",
          "line": 21,
          "call": "Illuminate\\Pipeline\\Pipeline::Illuminate\\Pipeline\\{closure}(instance of Illuminate\\Http\\Request)"
        },
        {
          "file": "/home/vagrant/code/vendor/laravel/framework/src/Illuminate/Pipeline/Pipeline.php",
          "line": 171,
          "call": "Illuminate\\Foundation\\Http\\Middleware\\TransformsRequest::handle(instance of Illuminate\\Http\\Request, instance of Closure)"
        },
        {
          "file": "/home/vagrant/code/vendor/laravel/framework/src/Illuminate/Foundation/Http/Middleware/TransformsRequest.php",
          "line": 21,
          "call": "Illuminate\\Pipeline\\Pipeline::Illuminate\\Pipeline\\{closure}(instance of Illuminate\\Http\\Request)"
        },
        {
          "file": "/home/vagrant/code/vendor/laravel/framework/src/Illuminate/Pipeline/Pipeline.php",
          "line": 171,
          "call": "Illuminate\\Foundation\\Http\\Middleware\\TransformsRequest::handle(instance of Illuminate\\Http\\Request, instance of Closure)"
        },
        {
          "file": "/home/vagrant/code/vendor/laravel/framework/src/Illuminate/Foundation/Http/Middleware/ValidatePostSize.php",
          "line": 27,
          "call": "Illuminate\\Pipeline\\Pipeline::Illuminate\\Pipeline\\{closure}(instance of Illuminate\\Http\\Request)"
        },
        {
          "file": "/home/vagrant/code/vendor/laravel/framework/src/Illuminate/Pipeline/Pipeline.php",
          "line": 171,
          "call": "Illuminate\\Foundation\\Http\\Middleware\\ValidatePostSize::handle(instance of Illuminate\\Http\\Request, instance of Closure)"
        },
        {
          "file": "/home/vagrant/code/vendor/laravel/framework/src/Illuminate/Foundation/Http/Middleware/CheckForMaintenanceMode.php",
          "line": 62,
          "call": "Illuminate\\Pipeline\\Pipeline::Illuminate\\Pipeline\\{closure}(instance of Illuminate\\Http\\Request)"
        },
        {
          "file": "/home/vagrant/code/vendor/laravel/framework/src/Illuminate/Pipeline/Pipeline.php",
          "line": 171,
          "call": "Illuminate\\Foundation\\Http\\Middleware\\CheckForMaintenanceMode::handle(instance of Illuminate\\Http\\Request, instance of Closure)"
        },
        {
          "file": "/home/vagrant/code/vendor/barryvdh/laravel-cors/src/HandlePreflight.php",
          "line": 29,
          "call": "Illuminate\\Pipeline\\Pipeline::Illuminate\\Pipeline\\{closure}(instance of Illuminate\\Http\\Request)"
        },
        {
          "file": "/home/vagrant/code/vendor/laravel/framework/src/Illuminate/Pipeline/Pipeline.php",
          "line": 171,
          "call": "Barryvdh\\Cors\\HandlePreflight::handle(instance of Illuminate\\Http\\Request, instance of Closure)"
        },
        {
          "file": "/home/vagrant/code/orion/src/Foundation/Http/Middleware/HandlePreflight.php",
          "line": 32,
          "call": "Illuminate\\Pipeline\\Pipeline::Illuminate\\Pipeline\\{closure}(instance of Illuminate\\Http\\Request)"
        },
        {
          "file": "/home/vagrant/code/vendor/laravel/framework/src/Illuminate/Pipeline/Pipeline.php",
          "line": 171,
          "call": "Orion\\Foundation\\Http\\Middleware\\HandlePreflight::handle(instance of Illuminate\\Http\\Request, instance of Closure)"
        },
        {
          "file": "/home/vagrant/code/vendor/laravel/framework/src/Illuminate/Pipeline/Pipeline.php",
          "line": 105,
          "call": "Illuminate\\Pipeline\\Pipeline::Illuminate\\Pipeline\\{closure}(instance of Illuminate\\Http\\Request)"
        },
        {
          "file": "/home/vagrant/code/vendor/laravel/framework/src/Illuminate/Foundation/Http/Kernel.php",
          "line": 151,
          "call": "Illuminate\\Pipeline\\Pipeline::then(instance of Closure)"
        },
        {
          "file": "/home/vagrant/code/vendor/laravel/framework/src/Illuminate/Foundation/Http/Kernel.php",
          "line": 116,
          "call": "Illuminate\\Foundation\\Http\\Kernel::sendRequestThroughRouter(instance of Illuminate\\Http\\Request)"
        },
        {
          "file": "/home/vagrant/code/public/index.php",
          "line": 55,
          "call": "Illuminate\\Foundation\\Http\\Kernel::handle(instance of Illuminate\\Http\\Request)"
        }
      ]
    }
  ],
  "data": {
    "show": null
  }
}

... and this is why:

$array1 = ["id" => ["fields" => true]];     
$array2 = ["id" => ["fields" => true]];     
array_merge_recursive($array1, $array2);

becomes

Array
(
    [id] => Array
        (
            [fields] => Array
                (
                    [0] => true
                    [1] => true
                )
        )
)

which then is "understood" as relation type of field, trying to select fields 0 and 1 from relationship id. Solution to this problem is using array_replace_recursive.

Type of change

• Bug fix (non-breaking change which fixes an issue)

Checklist

• Code style has been fixed via composer fix-tyle

P.s. I'm an amateur of pull-requests. I hope I described everything well.

[mfn]

@edgarsn thanks for the contribution and sorry for the confusion upfront, please remove all styleci fixes and just completely ignored styleci. It's a current known issue and we're just waiting for it's integration being removed 🙏

[edgarsn]

Oh ok. Styles removed 👍

[mfn]

@edgarsn please add a test for this change, this also helps me better review / understand the intention of the PR, thank you!

[edgarsn]

Sorry. Tests added 😉

[mfn]

@edgarsn when I run the test with your code changes reverted (the array casts), the test still works 🧐

My expectation here would be that the test breaks when I test it without them

[edgarsn]

@edgarsn when I run the test with your code changes reverted (the array casts), the test still works 🧐

My expectation here would be that the test breaks when I test it without them

You have to revert not only array casts, but replace array_replace_recursive with array_merge_recursive. Then the test fails.

[mfn]

🤦‍♂️

Sorry for the noise, need more ☕️

[mfn]

What do we need the casts?

Seems to me it works without them?

[edgarsn]

At some point, phpstan failed because of not having cast. array_replace_recursive can return array or null (in case of error, but I couldn't find more details about when that happens and why), but array_merge_recursive always returns array. So at the end, we need casting to array now.

[mfn]

P.s. I'm an amateur of pull-requests. I hope I described everything well.

I'd say that was a perfect first PR 🥇

Additionally I added a changelog entry for this fix.

Thank you!