Skip to content

Commit 454c841

Browse files
williamjallenwilliamjallen
committed
Improve query plans for test measurement GraphQL field
1 parent b1bc0f0 commit 454c841

File tree

5 files changed

+161
-54
lines changed

5 files changed

+161
-54
lines changed

‎app/GraphQL/Directives/FilterDirective.php‎

Lines changed: 12 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
use GraphQL\Language\AST\InputObjectTypeDefinitionNode;
1010
use GraphQL\Language\AST\InputValueDefinitionNode;
1111
use GraphQL\Language\AST\InterfaceTypeDefinitionNode;
12+
use GraphQL\Language\AST\ListTypeNode;
1213
use GraphQL\Language\AST\ObjectTypeDefinitionNode;
1314
use GraphQL\Language\Parser;
1415
use Illuminate\Database\Eloquent\Builder as EloquentBuilder;
@@ -47,8 +48,7 @@ public function manipulateArgDefinition(
4748
FieldDefinitionNode &$parentField,
4849
ObjectTypeDefinitionNode|InterfaceTypeDefinitionNode &$parentType,
4950
): void {
50-
// A hack to get the return type, assuming all lists are paginated...
51-
$returnType = Str::replaceEnd('Connection', '', $parentField->type->type->name->value);
51+
$returnType = Str::replaceEnd('Connection', '', ASTHelper::getUnderlyingTypeName($parentField));
5252

5353
$multiFilterName = ASTHelper::qualifiedArgType($argDefinition, $parentField, $parentType) . 'MultiFilterInput';
5454
$argDefinition->type = Parser::namedType($multiFilterName);
@@ -58,7 +58,10 @@ public function manipulateArgDefinition(
5858

5959
if (
6060
$this->getSubFilterableFieldsForType($argDefinition, $parentType) === []
61-
|| !str_ends_with($parentField->type->type->name->value, 'Connection')
61+
|| (
62+
!($parentField->type instanceof ListTypeNode)
63+
&& Str::doesntEndWith(ASTHelper::getUnderlyingTypeName($parentField), 'Connection')
64+
)
6265
|| $this->getSubFilterableFieldsForType($argDefinition, $documentAST->types[$returnType]) === []
6366
) {
6467
// Don't create a relationship filter input type because this type has no relationships
@@ -245,13 +248,12 @@ private function getSubFilterableFieldsForType(InputValueDefinitionNode $argDefi
245248
$subFilterableFieldNames = [];
246249
foreach ($type->fields as $field) {
247250
foreach ($field->arguments as $argument) {
248-
foreach ($argument->directives as $directive) {
249-
// We abuse the fact that all list return values are paginated to exclude filterable fields which
250-
// cannot have subqueries.
251-
if ($directive->name->value === 'filter' && str_ends_with($field->type->type->name->value, 'Connection')) {
252-
$subFilterableFieldNames[(string) $field->name->value] = ASTHelper::qualifiedArgType($argDefinition, $field, $type) . 'MultiFilterInput';
253-
break 2;
254-
}
251+
if (
252+
ASTHelper::hasDirective($argument, 'filter')
253+
&& Str::endsWith(ASTHelper::getUnderlyingTypeName($field), 'Connection')
254+
) {
255+
$subFilterableFieldNames[(string)$field->name->value] = ASTHelper::qualifiedArgType($argDefinition, $field, $type) . 'MultiFilterInput';
256+
break;
255257
}
256258
}
257259
}

‎graphql/schema.graphql‎

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -553,13 +553,9 @@ type Test {
553553

554554
details: String! @filterable
555555

556-
"""
557-
Test measurements for this test, sorted in descending order so newer results
558-
appear first when using paginated queries.
559-
"""
560556
testMeasurements(
561557
filters: _ @filter
562-
): [TestMeasurement!]! @hasMany(type: CONNECTION) @orderBy(column: "id", direction: DESC)
558+
): [TestMeasurement!]! @hasMany @orderBy(column: "id", direction: DESC)
563559

564560
labels(
565561
filters: _ @filter

‎resources/js/vue/components/BuildTestsPage.vue‎

Lines changed: 9 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -123,14 +123,10 @@ export default {
123123
details
124124
runningTime
125125
timeStatusCategory
126-
testMeasurements(filters: $measurementFilters, first: 100) {
127-
edges {
128-
node {
129-
id
130-
name
131-
value
132-
}
133-
}
126+
testMeasurements(filters: $measurementFilters) {
127+
id
128+
name
129+
value
134130
}
135131
}
136132
}
@@ -148,14 +144,10 @@ export default {
148144
details
149145
runningTime
150146
timeStatusCategory
151-
testMeasurements(filters: $measurementFilters, first: 100) {
152-
edges {
153-
node {
154-
id
155-
name
156-
value
157-
}
158-
}
147+
testMeasurements(filters: $measurementFilters) {
148+
id
149+
name
150+
value
159151
}
160152
}
161153
}
@@ -254,7 +246,7 @@ export default {
254246
},
255247
...this.pinnedMeasurements.reduce((acc, name) => ({
256248
...acc,
257-
[name]: edge.node.testMeasurements.edges.find((measurementEdge) => measurementEdge.node.name === name)?.node.value ?? '',
249+
[name]: edge.node.testMeasurements.find((measurementEdge) => measurementEdge.name === name)?.value ?? '',
258250
}), {}),
259251
};
260252
});

‎tests/Feature/GraphQL/FilterTest.php‎

Lines changed: 127 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,11 @@
33
namespace Tests\Feature\GraphQL;
44

55
use App\Enums\TargetType;
6+
use App\Models\Build;
67
use App\Models\Project;
78
use App\Models\Site;
9+
use App\Models\Test;
10+
use App\Models\TestOutput;
811
use App\Models\User;
912
use Illuminate\Foundation\Testing\DatabaseTruncation;
1013
use Illuminate\Support\Facades\DB;
@@ -36,6 +39,8 @@ class FilterTest extends TestCase
3639
*/
3740
private array $sites = [];
3841

42+
private TestOutput $testOutput;
43+
3944
protected function setUp(): void
4045
{
4146
parent::setUp();
@@ -54,6 +59,12 @@ protected function setUp(): void
5459
'normal' => $this->makeNormalUser(),
5560
'admin' => $this->makeAdminUser(),
5661
];
62+
63+
$this->testOutput = TestOutput::create([
64+
'path' => 'a',
65+
'command' => 'b',
66+
'output' => 'c',
67+
]);
5768
}
5869

5970
protected function tearDown(): void
@@ -73,6 +84,8 @@ protected function tearDown(): void
7384
}
7485
$this->sites = [];
7586

87+
$this->testOutput->delete();
88+
7689
parent::tearDown();
7790
}
7891

@@ -1000,4 +1013,118 @@ public function testFilterByRelationshipsOfRelationships(): void
10001013
],
10011014
]);
10021015
}
1016+
1017+
public function testFilterAggregateField(): void
1018+
{
1019+
$this->projects['public1']->builds()->create([
1020+
'name' => 'build1',
1021+
'uuid' => Str::uuid()->toString(),
1022+
]);
1023+
1024+
$this->projects['public1']->builds()->create([
1025+
'name' => 'build2',
1026+
'uuid' => Str::uuid()->toString(),
1027+
]);
1028+
1029+
$this->projects['public2']->builds()->create([
1030+
'name' => 'build1',
1031+
'uuid' => Str::uuid()->toString(),
1032+
]);
1033+
1034+
$this->actingAs($this->users['admin'])->graphQL('
1035+
query($projectid: ID!, $buildname: String!) {
1036+
project(id: $projectid) {
1037+
countWithoutFilters: buildCount
1038+
countWithFilters: buildCount(filters: {
1039+
eq: {
1040+
name: $buildname
1041+
}
1042+
})
1043+
}
1044+
}
1045+
', [
1046+
'projectid' => $this->projects['public1']->id,
1047+
'buildname' => 'build1',
1048+
])->assertExactJson([
1049+
'data' => [
1050+
'project' => [
1051+
'countWithoutFilters' => 2,
1052+
'countWithFilters' => 1,
1053+
],
1054+
],
1055+
]);
1056+
}
1057+
1058+
public function testFilterNonPaginatedList(): void
1059+
{
1060+
/** @var Build $build */
1061+
$build = $this->projects['public1']->builds()->create([
1062+
'name' => 'build1',
1063+
'uuid' => Str::uuid()->toString(),
1064+
]);
1065+
1066+
/** @var Test $test */
1067+
$test = $build->tests()->create([
1068+
'testname' => Str::uuid()->toString(),
1069+
'status' => 'passed',
1070+
'outputid' => $this->testOutput->id,
1071+
]);
1072+
1073+
$measurement1 = $test->testMeasurements()->create([
1074+
'name' => Str::uuid()->toString(),
1075+
'type' => 'text/string',
1076+
'value' => Str::uuid()->toString(),
1077+
]);
1078+
1079+
$measurement2 = $test->testMeasurements()->create([
1080+
'name' => Str::uuid()->toString(),
1081+
'type' => 'text/string',
1082+
'value' => Str::uuid()->toString(),
1083+
]);
1084+
1085+
$this->actingAs($this->users['admin'])->graphQL('
1086+
query($buildid: ID!, $measurementname: String!) {
1087+
build(id: $buildid) {
1088+
tests {
1089+
edges {
1090+
node {
1091+
testMeasurements(filters: {
1092+
eq: {
1093+
name: $measurementname
1094+
}
1095+
}) {
1096+
name
1097+
type
1098+
value
1099+
}
1100+
}
1101+
}
1102+
}
1103+
}
1104+
}
1105+
', [
1106+
'buildid' => $build->id,
1107+
'measurementname' => $measurement1->name,
1108+
])->assertExactJson([
1109+
'data' => [
1110+
'build' => [
1111+
'tests' => [
1112+
'edges' => [
1113+
[
1114+
'node' => [
1115+
'testMeasurements' => [
1116+
[
1117+
'name' => $measurement1->name,
1118+
'type' => $measurement1->type,
1119+
'value' => $measurement1->value,
1120+
],
1121+
],
1122+
],
1123+
],
1124+
],
1125+
],
1126+
],
1127+
],
1128+
]);
1129+
}
10031130
}

‎tests/Feature/GraphQL/TestMeasurementTypeTest.php‎

Lines changed: 12 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -82,13 +82,9 @@ public function testBasicFieldAccess(): void
8282
node {
8383
name
8484
testMeasurements {
85-
edges {
86-
node {
87-
name
88-
type
89-
value
90-
}
91-
}
85+
name
86+
type
87+
value
9288
}
9389
}
9490
}
@@ -114,21 +110,15 @@ public function testBasicFieldAccess(): void
114110
'node' => [
115111
'name' => 'test1',
116112
'testMeasurements' => [
117-
'edges' => [
118-
[
119-
'node' => [
120-
'name' => 'measurement 2',
121-
'type' => 'numeric/double',
122-
'value' => '6',
123-
],
124-
],
125-
[
126-
'node' => [
127-
'name' => 'measurement 1',
128-
'type' => 'text/string',
129-
'value' => 'test',
130-
],
131-
],
113+
[
114+
'name' => 'measurement 2',
115+
'type' => 'numeric/double',
116+
'value' => '6',
117+
],
118+
[
119+
'name' => 'measurement 1',
120+
'type' => 'text/string',
121+
'value' => 'test',
132122
],
133123
],
134124
],

0 commit comments

Comments
 (0)