Skip to content

Commit

Permalink
fix(graphql/@searchBy): Added table name prefix for Eloquent builde…
Browse files Browse the repository at this point in the history
…r to avoid "SQLSTATE[23000]: Integrity constraint violation: 1052 Column 'xxx' in where clause is ambiguous" error.
  • Loading branch information
LastDragon-ru committed Oct 29, 2022
1 parent 7e6f637 commit d2668a9
Show file tree
Hide file tree
Showing 6 changed files with 319 additions and 134 deletions.
14 changes: 14 additions & 0 deletions packages/graphql/src/SearchBy/Directives/Directive.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,9 @@
use Illuminate\Database\Eloquent\Model;
use Illuminate\Database\Query\Builder as QueryBuilder;
use LastDragon_ru\LaraASP\GraphQL\Builder\Directives\HandlerDirective;
use LastDragon_ru\LaraASP\GraphQL\Builder\Property;
use LastDragon_ru\LaraASP\GraphQL\SearchBy\Manipulator;
use Nuwave\Lighthouse\Execution\Arguments\ArgumentSet;
use Nuwave\Lighthouse\Schema\AST\DocumentAST;
use Nuwave\Lighthouse\Support\Contracts\ArgBuilderDirective;
use Nuwave\Lighthouse\Support\Contracts\ArgManipulator;
Expand Down Expand Up @@ -44,4 +46,16 @@ public function manipulateArgDefinition(
public function handleBuilder($builder, $value): EloquentBuilder|QueryBuilder {
return $this->handleAnyBuilder($builder, $value);
}

public function handle(object $builder, Property $property, ArgumentSet $conditions): object {
// Some relations (eg `HasManyThrough`) require a table name prefix to
// avoid "SQLSTATE[23000]: Integrity constraint violation: 1052 Column
// 'xxx' in where clause is ambiguous" error.
if ($builder instanceof EloquentBuilder && $property->getPath() === []) {
$property = $property->getChild($builder->getModel()->getTable());
}

// Return
return parent::handle($builder, $property, $conditions);
}
}
209 changes: 143 additions & 66 deletions packages/graphql/src/SearchBy/Directives/DirectiveTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,8 @@
use GraphQL\Type\Definition\InputObjectType;
use GraphQL\Type\Definition\Type;
use Illuminate\Contracts\Config\Repository;
use LastDragon_ru\LaraASP\Eloquent\Testing\Package\Models\TestObject;
use Illuminate\Database\Schema\Blueprint;
use Illuminate\Support\Facades\Schema;
use LastDragon_ru\LaraASP\Eloquent\Testing\Package\Models\WithTestObject;
use LastDragon_ru\LaraASP\GraphQL\Builder\Exceptions\Client\ConditionEmpty;
use LastDragon_ru\LaraASP\GraphQL\Builder\Exceptions\Client\ConditionTooManyOperators;
Expand All @@ -21,6 +22,9 @@
use LastDragon_ru\LaraASP\GraphQL\SearchBy\Operators\Property;
use LastDragon_ru\LaraASP\GraphQL\Testing\GraphQLExpectedSchema;
use LastDragon_ru\LaraASP\GraphQL\Testing\Package\BuilderDataProvider;
use LastDragon_ru\LaraASP\GraphQL\Testing\Package\EloquentBuilderDataProvider;
use LastDragon_ru\LaraASP\GraphQL\Testing\Package\Model;
use LastDragon_ru\LaraASP\GraphQL\Testing\Package\QueryBuilderDataProvider;
use LastDragon_ru\LaraASP\GraphQL\Testing\Package\TestCase;
use LastDragon_ru\LaraASP\Testing\Constraints\Json\JsonMatchesFragment;
use LastDragon_ru\LaraASP\Testing\Constraints\Response\Bodies\JsonBody;
Expand All @@ -29,6 +33,7 @@
use LastDragon_ru\LaraASP\Testing\Constraints\Response\StatusCodes\Ok;
use LastDragon_ru\LaraASP\Testing\Providers\ArrayDataProvider;
use LastDragon_ru\LaraASP\Testing\Providers\CompositeDataProvider;
use LastDragon_ru\LaraASP\Testing\Providers\MergeDataProvider;
use Nuwave\Lighthouse\Schema\DirectiveLocator;
use Nuwave\Lighthouse\Schema\TypeRegistry;
use Nuwave\Lighthouse\Testing\MakesGraphQLRequests;
Expand Down Expand Up @@ -145,9 +150,18 @@ public function testDirective(
Closure $builderFactory,
mixed $value,
): void {
$model = json_encode(TestObject::class, JSON_THROW_ON_ERROR);
$model = json_encode(Model::class, JSON_THROW_ON_ERROR);
$path = is_array($expected) ? 'data.test' : 'errors.0.message';
$body = is_array($expected) ? [] : json_encode($expected->getMessage(), JSON_THROW_ON_ERROR);
$table = (new Model())->getTable();

if (!Schema::hasTable($table)) {
Schema::create($table, static function (Blueprint $table): void {
$table->increments('id');
$table->string('a')->nullable();
$table->string('b')->nullable();
});
}

$this
->useGraphQLSchema(
Expand Down Expand Up @@ -335,68 +349,74 @@ public static function definition(): string {
* @return array<mixed>
*/
public function dataProviderHandleBuilder(): array {
return (new CompositeDataProvider(
new BuilderDataProvider(),
new ArrayDataProvider([
'empty' => [
[
'query' => <<<'SQL'
return (new MergeDataProvider([
'Both' => new CompositeDataProvider(
new BuilderDataProvider(),
new ArrayDataProvider([
'empty' => [
[
'query' => <<<'SQL'
select
*
from
"tmp"
SQL
,
'bindings' => [],
],
[
// empty
],
],
'empty operators' => [
new ConditionEmpty(),
[
'a' => [
,
'bindings' => [],
],
[
// empty
],
],
],
'too many properties' => [
new ConditionTooManyProperties(['a', 'b']),
[
'a' => [
'notEqual' => 1,
'empty operators' => [
new ConditionEmpty(),
[
'a' => [
// empty
],
],
'b' => [
'notEqual' => 'a',
],
'too many properties' => [
new ConditionTooManyProperties(['a', 'b']),
[
'a' => [
'notEqual' => 1,
],
'b' => [
'notEqual' => 'a',
],
],
],
],
'too many operators' => [
new ConditionTooManyOperators(['equal', 'notEqual']),
[
'a' => [
'equal' => 1,
'notEqual' => 1,
'too many operators' => [
new ConditionTooManyOperators(['equal', 'notEqual']),
[
'a' => [
'equal' => 1,
'notEqual' => 1,
],
],
],
],
'null' => [
[
'query' => <<<'SQL'
'null' => [
[
'query' => <<<'SQL'
select
*
from
"tmp"
SQL
,
'bindings' => [],
,
'bindings' => [],
],
null,
],
null,
],
'valid condition' => [
[
'query' => <<<'SQL'
]),
),
'Query' => new CompositeDataProvider(
new QueryBuilderDataProvider(),
new ArrayDataProvider([
'valid condition' => [
[
'query' => <<<'SQL'
select
*
from
Expand All @@ -416,37 +436,94 @@ public function dataProviderHandleBuilder(): array {
)
)
SQL
,
'bindings' => [1, 2, 'a'],
],
[
'not' => [
'allOf' => [
[
'a' => [
'notEqual' => 1,
,
'bindings' => [1, 2, 'a'],
],
[
'not' => [
'allOf' => [
[
'a' => [
'notEqual' => 1,
],
],
],
[
'anyOf' => [
[
'a' => [
'equal' => 2,
[
'anyOf' => [
[
'a' => [
'equal' => 2,
],
],
[
'b' => [
'notEqual' => 'a',
],
],
],
[
'b' => [
'notEqual' => 'a',
],
],
],
],
],
]),
),
'Eloquent' => new CompositeDataProvider(
new EloquentBuilderDataProvider(),
new ArrayDataProvider([
'valid condition' => [
[
'query' => <<<'SQL'
select
*
from
"tmp"
where
(
not (
(
("tmp"."a" != ?)
and (
(
("tmp"."a" = ?)
or ("tmp"."b" != ?)
)
)
)
)
)
SQL
,
'bindings' => [1, 2, 'a'],
],
[
'not' => [
'allOf' => [
[
'a' => [
'notEqual' => 1,
],
],
[
'anyOf' => [
[
'a' => [
'equal' => 2,
],
],
[
'b' => [
'notEqual' => 'a',
],
],
],
],
],
],
],
],
],
]),
))->getData();
]),
),
]))->getData();
}
// </editor-fold>
}
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ public function call(Handler $handler, object $builder, Property $property, Argu
$operator,
$count,
static function (EloquentBuilder $builder) use ($relation, $handler, $alias, $has): void {
if ($alias === $relation->getRelationCountHash(false)) {
if (!$alias || $alias === $relation->getRelationCountHash(false)) {
$alias = $builder->getModel()->getTable();
}

Expand Down
Loading