Skip to content

Commit

Permalink
Allow multiple slicing arguments when all of them are variables. (#7222)
Browse files Browse the repository at this point in the history
  • Loading branch information
michaelstaib authored Jul 4, 2024
1 parent 7b49551 commit 7605486
Show file tree
Hide file tree
Showing 7 changed files with 316 additions and 14 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -163,18 +163,31 @@ public static void ValidateRequireOneSlicingArgument(
// that it should expect that exactly one of the defined slicing arguments is present in
// a query. If that is not the case (i.e., if none or multiple slicing arguments are
// present), the static analysis may throw an error.
if (listSizeDirective is { RequireOneSlicingArgument: true })
if (listSizeDirective?.RequireOneSlicingArgument ?? false)
{
var argumentCount = 0;
var variableCount = 0;

foreach (var argumentNode in node.Arguments)
{
if (listSizeDirective.SlicingArguments.Contains(argumentNode.Name.Value))
{
argumentCount++;

if(argumentNode.Value.Kind == SyntaxKind.Variable)
{
variableCount++;
}
}
}

if(argumentCount > 0 &&
argumentCount == variableCount &&
argumentCount <= listSizeDirective.SlicingArguments.Length)
{
return;
}

if (argumentCount != 1)
{
throw new GraphQLException(ErrorHelper.ExactlyOneSlicingArgMustBeDefined(node, path));
Expand Down
168 changes: 168 additions & 0 deletions src/HotChocolate/CostAnalysis/test/CostAnalysis.Tests/PagingTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,174 @@ await snapshot
.MatchMarkdownAsync();
}

[Fact]
public async Task Require_Paging_Boundaries_Single_Boundary_With_Literal()
{
// arrange
var snapshot = new Snapshot();

var operation =
Utf8GraphQLParser.Parse(
"""
{
books(first: 1) {
nodes {
title
}
}
}
""");

var request =
OperationRequestBuilder.New()
.SetDocument(operation)
.ReportCost()
.Build();

var executor =
await new ServiceCollection()
.AddGraphQLServer()
.AddQueryType<Query>()
.AddFiltering()
.AddSorting()
.BuildRequestExecutorAsync();

// act
var response = await executor.ExecuteAsync(request);

// assert
await snapshot
.Add(operation, "Operation")
.Add(response, "Response")
.MatchMarkdownAsync();
}

[Fact]
public async Task Require_Paging_Boundaries_Single_Boundary_With_Variable()
{
// arrange
var snapshot = new Snapshot();

var operation =
Utf8GraphQLParser.Parse(
"""
query($first: Int) {
books(first: $first) {
nodes {
title
}
}
}
""");

var request =
OperationRequestBuilder.New()
.SetDocument(operation)
.ReportCost()
.Build();

var executor =
await new ServiceCollection()
.AddGraphQLServer()
.AddQueryType<Query>()
.AddFiltering()
.AddSorting()
.BuildRequestExecutorAsync();

// act
var response = await executor.ExecuteAsync(request);

// assert
await snapshot
.Add(operation, "Operation")
.Add(response, "Response")
.MatchMarkdownAsync();
}

[Fact]
public async Task Require_Paging_Boundaries_Two_Boundaries_With_Variable()
{
// arrange
var snapshot = new Snapshot();

var operation =
Utf8GraphQLParser.Parse(
"""
query($first: Int, $last: Int) {
books(first: $first, last: $last) {
nodes {
title
}
}
}
""");

var request =
OperationRequestBuilder.New()
.SetDocument(operation)
.ReportCost()
.Build();

var executor =
await new ServiceCollection()
.AddGraphQLServer()
.AddQueryType<Query>()
.AddFiltering()
.AddSorting()
.BuildRequestExecutorAsync();

// act
var response = await executor.ExecuteAsync(request);

// assert
await snapshot
.Add(operation, "Operation")
.Add(response, "Response")
.MatchMarkdownAsync();
}

[Fact]
public async Task Require_Paging_Boundaries_Two_Boundaries_Mixed()
{
// arrange
var snapshot = new Snapshot();

var operation =
Utf8GraphQLParser.Parse(
"""
query($first: Int) {
books(first: $first, last: 1) {
nodes {
title
}
}
}
""");

var request =
OperationRequestBuilder.New()
.SetDocument(operation)
.ReportCost()
.Build();

var executor =
await new ServiceCollection()
.AddGraphQLServer()
.AddQueryType<Query>()
.AddFiltering()
.AddSorting()
.BuildRequestExecutorAsync();

// act
var response = await executor.ExecuteAsync(request);

// assert
await snapshot
.Add(operation, "Operation")
.Add(response, "Response")
.MatchMarkdownAsync();
}

[Fact]
public async Task Require_Paging_Nested_Boundaries()
{
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
# Require_Paging_Boundaries_Single_Boundary_With_Literal

## Operation

```graphql
{
books(first: 1) {
nodes {
title
}
}
}
```

## Response

```json
{
"data": {
"books": {
"nodes": []
}
},
"extensions": {
"operationCost": {
"fieldCost": 11,
"typeCost": 3
}
}
}
```

Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
# Require_Paging_Boundaries_Single_Boundary_With_Variable

## Operation

```graphql
query($first: Int) {
books(first: $first) {
nodes {
title
}
}
}
```

## Response

```json
{
"data": {
"books": {
"nodes": []
}
},
"extensions": {
"operationCost": {
"fieldCost": 11,
"typeCost": 52
}
}
}
```

Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
# Require_Paging_Boundaries_Two_Boundaries_Mixed

## Operation

```graphql
query($first: Int) {
books(first: $first, last: 1) {
nodes {
title
}
}
}
```

## Response

```json
{
"errors": [
{
"message": "Exactly one slicing argument must be defined.",
"locations": [
{
"line": 2,
"column": 5
}
],
"path": [
"books"
],
"extensions": {
"code": "HC0082"
}
}
]
}
```

Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
# Require_Paging_Boundaries_Two_Boundaries_With_Variable

## Operation

```graphql
query($first: Int, $last: Int) {
books(first: $first, last: $last) {
nodes {
title
}
}
}
```

## Response

```json
{
"data": {
"books": {
"nodes": []
}
},
"extensions": {
"operationCost": {
"fieldCost": 11,
"typeCost": 52
}
}
}
```

Original file line number Diff line number Diff line change
Expand Up @@ -15,17 +15,4 @@ void Action()

Assert.Throws<ArgumentNullException>(Action);
}

[Fact]
public void AddReadOnlyFileSystemQueryStorage_Services_Is_Null()
{
// arrange
// act
void Action()
=> HotChocolateFileSystemPersistedQueriesRequestExecutorBuilderExtensions
.AddReadOnlyFileSystemQueryStorage(null!);

// assert
Assert.Throws<ArgumentNullException>(Action);
}
}

0 comments on commit 7605486

Please sign in to comment.