Skip to content

Commit

Permalink
fix(i): Panic on aggregate alias (sourcenetwork#3174)
Browse files Browse the repository at this point in the history
## Relevant issue(s)

Resolves sourcenetwork#3160

## Description

This PR fixes an issue where aliases on top level aggregates would cause
a panic.

## Tasks

- [x] I made sure the code is well commented, particularly
hard-to-understand areas.
- [x] I made sure the repository-held documentation is changed
accordingly.
- [x] I made sure the pull request title adheres to the conventional
commit style (the subset used in the project can be found in
[tools/configs/chglog/config.yml](tools/configs/chglog/config.yml)).
- [x] I made sure to discuss its limitations such as threats to
validity, vulnerability to mistake and misuse, robustness to
invalidation of assumptions, resource requirements, ...

## How has this been tested?

Added integration tests.

Specify the platform(s) on which this was tested:
- MacOS
  • Loading branch information
nasdf authored Oct 24, 2024
1 parent c1fcde0 commit bb0917e
Show file tree
Hide file tree
Showing 7 changed files with 161 additions and 7 deletions.
14 changes: 7 additions & 7 deletions internal/request/graphql/parser/query.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,18 +25,18 @@ func parseQueryOperationDefinition(
collectedFields map[string][]*ast.Field,
) (*request.OperationDefinition, []error) {
var selections []request.Selection
for name, fields := range collectedFields {
for _, node := range fields {
for _, fields := range collectedFields {
for _, field := range fields {
var parsedSelection request.Selection
if _, isCommitQuery := request.CommitQueries[name]; isCommitQuery {
parsed, err := parseCommitSelect(exe, exe.Schema.QueryType(), node)
if _, isCommitQuery := request.CommitQueries[field.Name.Value]; isCommitQuery {
parsed, err := parseCommitSelect(exe, exe.Schema.QueryType(), field)
if err != nil {
return nil, []error{err}
}

parsedSelection = parsed
} else if _, isAggregate := request.Aggregates[name]; isAggregate {
parsed, err := parseAggregate(exe, exe.Schema.QueryType(), node)
} else if _, isAggregate := request.Aggregates[field.Name.Value]; isAggregate {
parsed, err := parseAggregate(exe, exe.Schema.QueryType(), field)
if err != nil {
return nil, []error{err}
}
Expand All @@ -56,7 +56,7 @@ func parseQueryOperationDefinition(
} else {
// the query doesn't match a reserve name
// so its probably a generated query
parsed, err := parseSelect(exe, exe.Schema.QueryType(), node)
parsed, err := parseSelect(exe, exe.Schema.QueryType(), field)
if err != nil {
return nil, []error{err}
}
Expand Down
38 changes: 38 additions & 0 deletions tests/integration/query/commits/simple_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -444,3 +444,41 @@ func TestQuery_CommitsWithAllFieldsWithUpdate_NoError(t *testing.T) {

testUtils.ExecuteTestCase(t, test)
}

func TestQueryCommits_WithAlias_Succeeds(t *testing.T) {
test := testUtils.TestCase{
Description: "Simple all commits with alias query",
Actions: []any{
updateUserCollectionSchema(),
testUtils.CreateDoc{
CollectionID: 0,
Doc: `{
"name": "John",
"age": 21
}`,
},
testUtils.Request{
Request: `query {
history: commits {
cid
}
}`,
Results: map[string]any{
"history": []map[string]any{
{
"cid": "bafyreif6dqbkr7t37jcjfxxrjnxt7cspxzvs7qwlbtjca57cc663he4s7e",
},
{
"cid": "bafyreigtnj6ntulcilkmin4pgukjwv3nwglqpiiyddz3dyfexdbltze7sy",
},
{
"cid": "bafyreia2vlbfkcbyogdjzmbqcjneabwwwtw7ti2xbd7yor5mbu2sk4pcoy",
},
},
},
},
},
}

testUtils.ExecuteTestCase(t, test)
}
44 changes: 44 additions & 0 deletions tests/integration/query/latest_commits/with_doc_id_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -91,3 +91,47 @@ func TestQueryLatestCommitsWithDocIDWithSchemaVersionIDField(t *testing.T) {

executeTestCase(t, test)
}

func TestQueryLatestCommits_WithDocIDAndAliased_Succeeds(t *testing.T) {
test := testUtils.TestCase{
Description: "Simple latest commits query with docID and aliased",
Actions: []any{
testUtils.CreateDoc{
Doc: `{
"name": "John",
"age": 21
}`,
},
testUtils.Request{
Request: `query {
history: latestCommits(docID: "bae-c9fb0fa4-1195-589c-aa54-e68333fb90b3") {
cid
links {
cid
name
}
}
}`,
Results: map[string]any{
"history": []map[string]any{
{
"cid": "bafyreia2vlbfkcbyogdjzmbqcjneabwwwtw7ti2xbd7yor5mbu2sk4pcoy",
"links": []map[string]any{
{
"cid": "bafyreif6dqbkr7t37jcjfxxrjnxt7cspxzvs7qwlbtjca57cc663he4s7e",
"name": "age",
},
{
"cid": "bafyreigtnj6ntulcilkmin4pgukjwv3nwglqpiiyddz3dyfexdbltze7sy",
"name": "name",
},
},
},
},
},
},
},
}

executeTestCase(t, test)
}
18 changes: 18 additions & 0 deletions tests/integration/query/simple/with_average_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -95,3 +95,21 @@ func TestQuerySimpleWithAverage(t *testing.T) {

executeTestCase(t, test)
}

func TestQuerySimple_WithAliasedAverage_OnEmptyCollection_Succeeds(t *testing.T) {
test := testUtils.TestCase{
Description: "Simple query, aliased average on empty",
Actions: []any{
testUtils.Request{
Request: `query {
average: _avg(Users: {field: Age})
}`,
Results: map[string]any{
"average": float64(0),
},
},
},
}

executeTestCase(t, test)
}
18 changes: 18 additions & 0 deletions tests/integration/query/simple/with_count_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -79,3 +79,21 @@ func TestQuerySimpleWithCount(t *testing.T) {

executeTestCase(t, test)
}

func TestQuerySimple_WithAliasedCount_OnEmptyCollection_Succeeds(t *testing.T) {
test := testUtils.TestCase{
Description: "Simple query, aliased count on empty",
Actions: []any{
testUtils.Request{
Request: `query {
number: _count(Users: {})
}`,
Results: map[string]any{
"number": 0,
},
},
},
}

executeTestCase(t, test)
}
18 changes: 18 additions & 0 deletions tests/integration/query/simple/with_max_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -127,3 +127,21 @@ func TestQuerySimple_WithMaxAndMaxValueInt_Succeeds(t *testing.T) {

executeTestCase(t, test)
}

func TestQuerySimple_WithAliasedMaxOnEmptyCollection_Succeeds(t *testing.T) {
test := testUtils.TestCase{
Description: "Simple query aliased max on empty",
Actions: []any{
testUtils.Request{
Request: `query {
maximum: _max(Users: {field: Age})
}`,
Results: map[string]any{
"maximum": nil,
},
},
},
}

executeTestCase(t, test)
}
18 changes: 18 additions & 0 deletions tests/integration/query/simple/with_min_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -127,3 +127,21 @@ func TestQuerySimple_WithMinAndMaxValueInt_Succeeds(t *testing.T) {

executeTestCase(t, test)
}

func TestQuerySimple_WithAliasedMinOnEmptyCollection_Succeeds(t *testing.T) {
test := testUtils.TestCase{
Description: "Simple query aliased min on empty",
Actions: []any{
testUtils.Request{
Request: `query {
minimum: _min(Users: {field: Age})
}`,
Results: map[string]any{
"minimum": nil,
},
},
},
}

executeTestCase(t, test)
}

0 comments on commit bb0917e

Please sign in to comment.