refactor: GraphQL order input

internal/request/graphql/parser/commit.go

@@ -55,11 +55,11 @@ func parseCommitSelect(
 			}
 
 		case request.OrderClause:
-			v, ok := value.(map[string]any)
+			v, ok := value.([]any)
 			if !ok {
 				continue // value is nil
 			}
-			conditions, err := ParseConditionsInOrder(argument.Value.(*ast.ObjectValue), v)
+			conditions, err := parseOrderConditionList(v)
 			if err != nil {
 				return nil, err
 			}

internal/request/graphql/parser/errors.go

@@ -24,4 +24,5 @@ var (
 	ErrUnknownExplainType             = errors.New("invalid / unknown explain type")
 	ErrUnknownGQLOperation            = errors.New("unknown GraphQL operation type")
 	ErrInvalidFilterConditions        = errors.New("invalid filter condition type, expected map")
+	ErrMultipleOrderFieldsDefined     = errors.New("order arguments can only define one field at a time")
 )

internal/request/graphql/parser/filter.go

@@ -64,77 +64,19 @@
 	return NewFilter(obj, filterType)
 }
 
-// ParseConditionsInOrder is similar to ParseConditions, except instead
-// of returning a map[string]any, we return a []any. This
-// is to maintain the ordering info of the statements within the ObjectValue.
-// This function is mostly used by the Order parser, which needs to parse
-// conditions in the same way as the Filter object, however the order
-// of the arguments is important.
-func ParseConditionsInOrder(stmt *ast.ObjectValue, args map[string]any) ([]request.OrderCondition, error) {
-	conditions := make([]request.OrderCondition, 0)
-	if stmt == nil {
-		return conditions, nil
-	}
-	for _, field := range stmt.Fields {
-		switch v := args[field.Name.Value].(type) {
-		case int: // base direction parsed (hopefully, check NameToOrderDirection)
-			dir, err := parseOrderDirection(v)
-			if err != nil {
-				return nil, err
-			}
-			conditions = append(conditions, request.OrderCondition{
-				Fields:    []string{field.Name.Value},
-				Direction: dir,
-			})
-
-		case map[string]any: // flatten and incorporate the parsed slice into our current one
-			sub, err := ParseConditionsInOrder(field.Value.(*ast.ObjectValue), v)
-			if err != nil {
-				return nil, err
-			}
-			for _, cond := range sub {
-				// prepend the current field name, to the parsed condition from the slice
-				// Eg. order: {author: {name: ASC, birthday: DESC}}
-				// This results in an array of [name, birthday] converted to
-				// [author.name, author.birthday].
-				// etc.
-				cond.Fields = append([]string{field.Name.Value}, cond.Fields...)
-				conditions = append(conditions, cond)
-			}
-
-		case nil:
-			continue // ignore nil filter input
-
-		default:
-			return nil, client.NewErrUnhandledType("parseConditionInOrder", v)
-		}
-	}
-
-	return conditions, nil
-}
-
 // parseConditions loops over the stmt ObjectValue fields, and extracts
 // all the relevant name/value pairs.
 func ParseConditions(stmt *ast.ObjectValue, inputType gql.Input) (map[string]any, error) {
-	cond, err := parseConditions(stmt, inputType)
-	if err != nil {
-		return nil, err
+	cond := gql.ValueFromAST(stmt, inputType, nil)
+	if cond == nil {
+		return nil, ErrFailedToParseConditionsFromAST
 	}
-
 	if v, ok := cond.(map[string]any); ok {
 		return v, nil
 	}
 	return nil, client.NewErrUnexpectedType[map[string]any]("condition", cond)
 }
 
-func parseConditions(stmt *ast.ObjectValue, inputArg gql.Input) (any, error) {
-	val := gql.ValueFromAST(stmt, inputArg, nil)
-	if val == nil {
-		return nil, ErrFailedToParseConditionsFromAST
-	}
-	return val, nil
-}
-
 // ParseFilterFieldsForDescription parses the fields that are defined in the SchemaDescription
 // from the filter conditions“
 func ParseFilterFieldsForDescription(
@@ -195,16 +137,3 @@
 	}
 	return fields, nil
 }
-
-func parseOrderDirection(v int) (request.OrderDirection, error) {
-	switch v {
-	case 0:
-		return request.ASC, nil
-
-	case 1:
-		return request.DESC, nil
-
-	default:
-		return request.ASC, ErrInvalidOrderDirection
-	}
-}

internal/request/graphql/parser/order.go

@@ -0,0 +1,90 @@
+// Copyright 2024 Democratized Data Foundation
+//
+// Use of this software is governed by the Business Source License
+// included in the file licenses/BSL.txt.
+//
+// As of the Change Date specified in that file, in accordance with
+// the Business Source License, use of this software will be governed
+// by the Apache License, Version 2.0, included in the file
+// licenses/APL.txt.
+
+package parser
+
+import (
+	"github.com/sourcenetwork/defradb/client/request"
+)
+
+func parseOrderConditionList(args []any) ([]request.OrderCondition, error) {
+	var conditions []request.OrderCondition
+	for _, a := range args {
+		v, ok := a.(map[string]any)
+		if !ok {
+			continue // order value is nil
+		}
+		condition, err := parseOrderCondition(v)
+		if err != nil {
+			return nil, err
+		}
+		if condition != nil {
+			conditions = append(conditions, *condition)
+		}
+	}
+	return conditions, nil
+}
+
+func parseOrderCondition(arg map[string]any) (*request.OrderCondition, error) {
+	if len(arg) == 0 {
+		return nil, nil
+	}
+	if len(arg) != 1 {
+		return nil, ErrMultipleOrderFieldsDefined
+	}
+	var fieldName string
+	for name := range arg {
+		fieldName = name
+	}
+	switch t := arg[fieldName].(type) {
+	case int:
+		dir, err := parseOrderDirection(t)
+		if err != nil {
+			return nil, err
+		}
+		return &request.OrderCondition{
+			Fields:    []string{fieldName},
+			Direction: dir,
+		}, nil
+
+	case map[string]any:
+		cond, err := parseOrderCondition(t)
+		if err != nil {
+			return nil, err
+		}
+		if cond == nil {
+			return nil, nil
+		}
+		// prepend the current field name, to the parsed condition from the slice
+		// Eg. order: {author: {name: ASC, birthday: DESC}}
+		// This results in an array of [name, birthday] converted to
+		// [author.name, author.birthday].
+		// etc.
+		cond.Fields = append([]string{fieldName}, cond.Fields...)
+		return cond, nil
+
+	default:
+		// field value is null so don't include the condition
+		return nil, nil
+	}
+}
+
+func parseOrderDirection(v int) (request.OrderDirection, error) {
+	switch v {
+	case 0:
+		return request.ASC, nil
+
+	case 1:
+		return request.DESC, nil
+
+	default:
+		return request.ASC, ErrInvalidOrderDirection
+	}
+}

internal/request/graphql/parser/query.go

@@ -143,11 +143,11 @@ func parseSelect(
 			}
 
 		case request.OrderClause: // parse order by
-			v, ok := value.(map[string]any)
+			v, ok := value.([]any)
 			if !ok {
 				continue // value is nil
 			}
-			conditions, err := ParseConditionsInOrder(argument.Value.(*ast.ObjectValue), v)
+			conditions, err := parseOrderConditionList(v)
 			if err != nil {
 				return nil, err
 			}
@@ -213,11 +213,7 @@ func parseAggregate(
 			})
 
 		case map[string]any:
-			value, ok := argument.Value.(*ast.ObjectValue)
-			if !ok {
-				continue // value is nil
-			}
-			target, err := parseAggregateTarget(name, value, v)
+			target, err := parseAggregateTarget(name, v)
 			if err != nil {
 				return nil, err
 			}
@@ -236,7 +232,6 @@ func parseAggregate(
 
 func parseAggregateTarget(
 	hostName string,
-	value *ast.ObjectValue,
 	arguments map[string]any,
 ) (*request.AggregateTarget, error) {
 	var childName string
@@ -245,10 +240,7 @@ func parseAggregateTarget(
 	var offset immutable.Option[uint64]
 	var order immutable.Option[request.OrderBy]
 
-	for _, f := range value.Fields {
-		name := f.Name.Value
-		value := arguments[name]
-
+	for name, value := range arguments {
 		switch name {
 		case request.FieldName:
 			if v, ok := value.(string); ok {
@@ -271,29 +263,21 @@ func parseAggregateTarget(
 			}
 
 		case request.OrderClause:
-			switch conditionsAST := f.Value.(type) {
-			case *ast.EnumValue:
+			switch t := value.(type) {
+			case int:
 				// For inline arrays the order arg will be a simple enum declaring the order direction
-				v, ok := value.(int)
-				if !ok {
-					continue // value is nil
-				}
-				dir, err := parseOrderDirection(v)
+				dir, err := parseOrderDirection(t)
 				if err != nil {
 					return nil, err
 				}
 				order = immutable.Some(request.OrderBy{
 					Conditions: []request.OrderCondition{{Direction: dir}},
 				})
 
-			case *ast.ObjectValue:
+			case []any:
 				// For relations the order arg will be the complex order object as used by the host object
 				// for non-aggregate ordering
-				v, ok := value.(map[string]any)
-				if !ok {
-					continue // value is nil
-				}
-				conditions, err := ParseConditionsInOrder(conditionsAST, v)
+				conditions, err := parseOrderConditionList(t)
 				if err != nil {
 					return nil, err
 				}

internal/request/graphql/schema/generate.go

@@ -396,7 +396,7 @@ func (g *Generator) createExpandedFieldList(
 				schemaTypes.GroupByArgDescription,
 			),
 			"order": schemaTypes.NewArgConfig(
-				g.manager.schema.TypeMap()[typeName+"OrderArg"],
+				gql.NewList(g.manager.schema.TypeMap()[typeName+"OrderArg"]),
 				schemaTypes.OrderArgDescription,
 			),
 			request.LimitClause:  schemaTypes.NewArgConfig(gql.Int, schemaTypes.LimitArgDescription),
@@ -980,7 +980,7 @@ func (g *Generator) genNumericAggregateBaseArgInputs(obj *gql.Object) *gql.Input
 				Description: schemaTypes.OffsetArgDescription,
 			},
 			request.OrderClause: &gql.InputObjectFieldConfig{
-				Type:        g.manager.schema.TypeMap()[genTypeName(obj, "OrderArg")],
+				Type:        gql.NewList(g.manager.schema.TypeMap()[genTypeName(obj, "OrderArg")]),
 				Description: schemaTypes.OrderArgDescription,
 			},
 		}, nil
@@ -1303,7 +1303,7 @@ func (g *Generator) genTypeQueryableFieldList(
 				gql.NewList(gql.NewNonNull(config.groupBy)),
 				schemaTypes.GroupByArgDescription,
 			),
-			"order":              schemaTypes.NewArgConfig(config.order, schemaTypes.OrderArgDescription),
+			"order":              schemaTypes.NewArgConfig(gql.NewList(config.order), schemaTypes.OrderArgDescription),
 			request.ShowDeleted:  schemaTypes.NewArgConfig(gql.Boolean, showDeletedArgDescription),
 			request.LimitClause:  schemaTypes.NewArgConfig(gql.Int, schemaTypes.LimitArgDescription),
 			request.OffsetClause: schemaTypes.NewArgConfig(gql.Int, schemaTypes.OffsetArgDescription),

internal/request/graphql/schema/types/commits.go

@@ -148,7 +148,7 @@ func QueryCommits(commitObject *gql.Object, commitsOrderArg *gql.InputObject) *g
 		Args: gql.FieldConfigArgument{
 			request.DocIDArgName: NewArgConfig(gql.ID, commitDocIDArgDescription),
 			request.FieldIDName:  NewArgConfig(gql.String, commitFieldIDArgDescription),
-			"order":              NewArgConfig(commitsOrderArg, OrderArgDescription),
+			"order":              NewArgConfig(gql.NewList(commitsOrderArg), OrderArgDescription),
 			"cid":                NewArgConfig(gql.ID, commitCIDArgDescription),
 			"groupBy": NewArgConfig(
 				gql.NewList(

tests/integration/explain/debug/with_order_test.go

@@ -69,7 +69,7 @@ func TestDebugExplainRequestWithMultiOrderFieldsOnParent(t *testing.T) {
 			testUtils.ExplainRequest{
 
 				Request: `query @explain(type: debug) {
-					Author(order: {name: ASC, age: DESC}) {
+					Author(order: [{name: ASC}, {age: DESC}]) {
 						name
 						age
 					}

tests/integration/explain/default/with_order_test.go

@@ -86,7 +86,7 @@ func TestDefaultExplainRequestWithMultiOrderFieldsOnParent(t *testing.T) {
 			testUtils.ExplainRequest{
 
 				Request: `query @explain {
-					Author(order: {name: ASC, age: DESC}) {
+					Author(order: [{name: ASC}, {age: DESC}]) {
 						name
 						age
 					}

tests/integration/explain/execute/with_order_test.go

@@ -117,7 +117,7 @@ func TestExecuteExplainRequestWithMultiOrderFieldsOnParent(t *testing.T) {
 
 			testUtils.ExplainRequest{
 				Request: `query @explain(type: execute) {
-					Author(order: {age: ASC, name: DESC}) {
+					Author(order: [{age: ASC}, {name: DESC}]) {
 						name
 						age
 					}

tests/integration/query/one_to_many_to_one/with_order_test.go

@@ -24,7 +24,7 @@ func TestMultipleOrderByWithDepthGreaterThanOne(t *testing.T) {
 			createDocsWith6BooksAnd5Publishers(),
 			testUtils.Request{
 				Request: `query {
-			Book (order: {rating: ASC, publisher: {yearOpened: DESC}}) {
+			Book (order: [{rating: ASC}, {publisher: {yearOpened: DESC}}]) {
 				name
 				rating
 				publisher{
@@ -97,15 +97,15 @@ func TestMultipleOrderByWithDepthGreaterThanOneOrderSwitched(t *testing.T) {
 			createDocsWith6BooksAnd5Publishers(),
 			testUtils.Request{
 				Request: `query {
-			Book (order: {publisher: {yearOpened: DESC}, rating: ASC}) {
-				name
-				rating
-				publisher{
-					name
-					yearOpened
-				}
-			}
-		}`,
+					Book (order: [{publisher: {yearOpened: DESC}}, {rating: ASC}]) {
+						name
+						rating
+						publisher{
+							name
+							yearOpened
+						}
+					}
+				}`,
 				Results: map[string]any{
 					"Book": []map[string]any{
 						{

tests/integration/query/simple/with_order_test.go

@@ -358,7 +358,7 @@ func TestQuerySimpleWithNumericOrderDescendingAndBooleanOrderAscending(t *testin
 			},
 			testUtils.Request{
 				Request: `query {
-					Users(order: {Age: DESC, Verified: ASC}) {
+					Users(order: [{Age: DESC}, {Verified: ASC}]) {
 						Name
 						Age
 						Verified