From 9cca190d626d2d7e9cb4f0a5292d035e4e6bf043 Mon Sep 17 00:00:00 2001 From: William Allen <16820599+williamjallen@users.noreply.github.com> Date: Sat, 27 Dec 2025 17:50:04 -0500 Subject: [PATCH] Improve query plans for test measurement GraphQL field --- app/GraphQL/Directives/FilterDirective.php | 22 +-- graphql/schema.graphql | 6 +- phpstan-baseline.neon | 4 +- .../js/vue/components/BuildTestsPage.vue | 26 ++-- tests/Feature/GraphQL/FilterTest.php | 127 ++++++++++++++++++ .../GraphQL/TestMeasurementTypeTest.php | 34 ++--- 6 files changed, 163 insertions(+), 56 deletions(-) diff --git a/app/GraphQL/Directives/FilterDirective.php b/app/GraphQL/Directives/FilterDirective.php index 5ee98926c1..351a4c895c 100644 --- a/app/GraphQL/Directives/FilterDirective.php +++ b/app/GraphQL/Directives/FilterDirective.php @@ -9,6 +9,7 @@ use GraphQL\Language\AST\InputObjectTypeDefinitionNode; use GraphQL\Language\AST\InputValueDefinitionNode; use GraphQL\Language\AST\InterfaceTypeDefinitionNode; +use GraphQL\Language\AST\ListTypeNode; use GraphQL\Language\AST\ObjectTypeDefinitionNode; use GraphQL\Language\Parser; use Illuminate\Database\Eloquent\Builder as EloquentBuilder; @@ -47,8 +48,7 @@ public function manipulateArgDefinition( FieldDefinitionNode &$parentField, ObjectTypeDefinitionNode|InterfaceTypeDefinitionNode &$parentType, ): void { - // A hack to get the return type, assuming all lists are paginated... - $returnType = Str::replaceEnd('Connection', '', $parentField->type->type->name->value); + $returnType = Str::replaceEnd('Connection', '', ASTHelper::getUnderlyingTypeName($parentField)); $multiFilterName = ASTHelper::qualifiedArgType($argDefinition, $parentField, $parentType) . 'MultiFilterInput'; $argDefinition->type = Parser::namedType($multiFilterName); @@ -58,7 +58,10 @@ public function manipulateArgDefinition( if ( $this->getSubFilterableFieldsForType($argDefinition, $parentType) === [] - || !str_ends_with($parentField->type->type->name->value, 'Connection') + || ( + !($parentField->type instanceof ListTypeNode) + && Str::doesntEndWith(ASTHelper::getUnderlyingTypeName($parentField), 'Connection') + ) || $this->getSubFilterableFieldsForType($argDefinition, $documentAST->types[$returnType]) === [] ) { // Don't create a relationship filter input type because this type has no relationships @@ -245,13 +248,12 @@ private function getSubFilterableFieldsForType(InputValueDefinitionNode $argDefi $subFilterableFieldNames = []; foreach ($type->fields as $field) { foreach ($field->arguments as $argument) { - foreach ($argument->directives as $directive) { - // We abuse the fact that all list return values are paginated to exclude filterable fields which - // cannot have subqueries. - if ($directive->name->value === 'filter' && str_ends_with($field->type->type->name->value, 'Connection')) { - $subFilterableFieldNames[(string) $field->name->value] = ASTHelper::qualifiedArgType($argDefinition, $field, $type) . 'MultiFilterInput'; - break 2; - } + if ( + ASTHelper::hasDirective($argument, 'filter') + && Str::endsWith(ASTHelper::getUnderlyingTypeName($field), 'Connection') + ) { + $subFilterableFieldNames[(string) $field->name->value] = ASTHelper::qualifiedArgType($argDefinition, $field, $type) . 'MultiFilterInput'; + break; } } } diff --git a/graphql/schema.graphql b/graphql/schema.graphql index 536caca038..4880d8fc83 100644 --- a/graphql/schema.graphql +++ b/graphql/schema.graphql @@ -553,13 +553,9 @@ type Test { details: String! @filterable - """ - Test measurements for this test, sorted in descending order so newer results - appear first when using paginated queries. - """ testMeasurements( filters: _ @filter - ): [TestMeasurement!]! @hasMany(type: CONNECTION) @orderBy(column: "id", direction: DESC) + ): [TestMeasurement!]! @hasMany @orderBy(column: "id", direction: DESC) labels( filters: _ @filter diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index edf8f94d0e..624c22c848 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -159,13 +159,13 @@ parameters: - rawMessage: Access to an undefined property GraphQL\Language\AST\ListTypeNode|GraphQL\Language\AST\NamedTypeNode|GraphQL\Language\AST\NonNullTypeNode::$name. identifier: property.notFound - count: 4 + count: 1 path: app/GraphQL/Directives/FilterDirective.php - rawMessage: Access to an undefined property GraphQL\Language\AST\ListTypeNode|GraphQL\Language\AST\NamedTypeNode|GraphQL\Language\AST\NonNullTypeNode::$type. identifier: property.notFound - count: 4 + count: 1 path: app/GraphQL/Directives/FilterDirective.php - diff --git a/resources/js/vue/components/BuildTestsPage.vue b/resources/js/vue/components/BuildTestsPage.vue index 4123e1e4fd..0a597e9144 100644 --- a/resources/js/vue/components/BuildTestsPage.vue +++ b/resources/js/vue/components/BuildTestsPage.vue @@ -123,14 +123,10 @@ export default { details runningTime timeStatusCategory - testMeasurements(filters: $measurementFilters, first: 100) { - edges { - node { - id - name - value - } - } + testMeasurements(filters: $measurementFilters) { + id + name + value } } } @@ -148,14 +144,10 @@ export default { details runningTime timeStatusCategory - testMeasurements(filters: $measurementFilters, first: 100) { - edges { - node { - id - name - value - } - } + testMeasurements(filters: $measurementFilters) { + id + name + value } } } @@ -254,7 +246,7 @@ export default { }, ...this.pinnedMeasurements.reduce((acc, name) => ({ ...acc, - [name]: edge.node.testMeasurements.edges.find((measurementEdge) => measurementEdge.node.name === name)?.node.value ?? '', + [name]: edge.node.testMeasurements.find((measurementEdge) => measurementEdge.name === name)?.value ?? '', }), {}), }; }); diff --git a/tests/Feature/GraphQL/FilterTest.php b/tests/Feature/GraphQL/FilterTest.php index 8ead0eec3f..dcb121798f 100644 --- a/tests/Feature/GraphQL/FilterTest.php +++ b/tests/Feature/GraphQL/FilterTest.php @@ -3,8 +3,11 @@ namespace Tests\Feature\GraphQL; use App\Enums\TargetType; +use App\Models\Build; use App\Models\Project; use App\Models\Site; +use App\Models\Test; +use App\Models\TestOutput; use App\Models\User; use Illuminate\Foundation\Testing\DatabaseTruncation; use Illuminate\Support\Facades\DB; @@ -36,6 +39,8 @@ class FilterTest extends TestCase */ private array $sites = []; + private TestOutput $testOutput; + protected function setUp(): void { parent::setUp(); @@ -54,6 +59,12 @@ protected function setUp(): void 'normal' => $this->makeNormalUser(), 'admin' => $this->makeAdminUser(), ]; + + $this->testOutput = TestOutput::create([ + 'path' => 'a', + 'command' => 'b', + 'output' => 'c', + ]); } protected function tearDown(): void @@ -73,6 +84,8 @@ protected function tearDown(): void } $this->sites = []; + $this->testOutput->delete(); + parent::tearDown(); } @@ -1000,4 +1013,118 @@ public function testFilterByRelationshipsOfRelationships(): void ], ]); } + + public function testFilterAggregateField(): void + { + $this->projects['public1']->builds()->create([ + 'name' => 'build1', + 'uuid' => Str::uuid()->toString(), + ]); + + $this->projects['public1']->builds()->create([ + 'name' => 'build2', + 'uuid' => Str::uuid()->toString(), + ]); + + $this->projects['public2']->builds()->create([ + 'name' => 'build1', + 'uuid' => Str::uuid()->toString(), + ]); + + $this->actingAs($this->users['admin'])->graphQL(' + query($projectid: ID!, $buildname: String!) { + project(id: $projectid) { + countWithoutFilters: buildCount + countWithFilters: buildCount(filters: { + eq: { + name: $buildname + } + }) + } + } + ', [ + 'projectid' => $this->projects['public1']->id, + 'buildname' => 'build1', + ])->assertExactJson([ + 'data' => [ + 'project' => [ + 'countWithoutFilters' => 2, + 'countWithFilters' => 1, + ], + ], + ]); + } + + public function testFilterNonPaginatedList(): void + { + /** @var Build $build */ + $build = $this->projects['public1']->builds()->create([ + 'name' => 'build1', + 'uuid' => Str::uuid()->toString(), + ]); + + /** @var Test $test */ + $test = $build->tests()->create([ + 'testname' => Str::uuid()->toString(), + 'status' => 'passed', + 'outputid' => $this->testOutput->id, + ]); + + $measurement1 = $test->testMeasurements()->create([ + 'name' => Str::uuid()->toString(), + 'type' => 'text/string', + 'value' => Str::uuid()->toString(), + ]); + + $measurement2 = $test->testMeasurements()->create([ + 'name' => Str::uuid()->toString(), + 'type' => 'text/string', + 'value' => Str::uuid()->toString(), + ]); + + $this->actingAs($this->users['admin'])->graphQL(' + query($buildid: ID!, $measurementname: String!) { + build(id: $buildid) { + tests { + edges { + node { + testMeasurements(filters: { + eq: { + name: $measurementname + } + }) { + name + type + value + } + } + } + } + } + } + ', [ + 'buildid' => $build->id, + 'measurementname' => $measurement1->name, + ])->assertExactJson([ + 'data' => [ + 'build' => [ + 'tests' => [ + 'edges' => [ + [ + 'node' => [ + 'testMeasurements' => [ + [ + 'name' => $measurement1->name, + 'type' => $measurement1->type, + 'value' => $measurement1->value, + ], + ], + ], + ], + ], + ], + ], + ], + ]); + } } diff --git a/tests/Feature/GraphQL/TestMeasurementTypeTest.php b/tests/Feature/GraphQL/TestMeasurementTypeTest.php index 7bf1e59907..7eecfcb4dc 100644 --- a/tests/Feature/GraphQL/TestMeasurementTypeTest.php +++ b/tests/Feature/GraphQL/TestMeasurementTypeTest.php @@ -82,13 +82,9 @@ public function testBasicFieldAccess(): void node { name testMeasurements { - edges { - node { - name - type - value - } - } + name + type + value } } } @@ -114,21 +110,15 @@ public function testBasicFieldAccess(): void 'node' => [ 'name' => 'test1', 'testMeasurements' => [ - 'edges' => [ - [ - 'node' => [ - 'name' => 'measurement 2', - 'type' => 'numeric/double', - 'value' => '6', - ], - ], - [ - 'node' => [ - 'name' => 'measurement 1', - 'type' => 'text/string', - 'value' => 'test', - ], - ], + [ + 'name' => 'measurement 2', + 'type' => 'numeric/double', + 'value' => '6', + ], + [ + 'name' => 'measurement 1', + 'type' => 'text/string', + 'value' => 'test', ], ], ],