Query sorting approach may be incorrect

[kinglozzer]

Module version(s) affected

v4, v5

Description

Currently sort arguments are passed as an object:

{
    "created": "asc",
    "name": "asc"
}

According to the GraphQL spec, there’s no guarantee about the order that these will be interpreted, so the sort order might be interpreted as either Created ASC, Name ASC or Name ASC, Created ASC. It appears that webonyx/graphql-php will interpret them in the order they’re defined in the schema, not in the query or inputs.

We’ve managed to work around this by manually parsing the query in https://github.com/silverstripe/silverstripe-graphql/pull/563, but that only works if the sort order is baked into the query itself. When the sort order is passed as an argument to the query, the original order is lost as soon as the query is parsed (more info: https://github.com/silverstripe/silverstripe-graphql/issues/571#issuecomment-1933825823). That aside, manually inspecting the query AST to try to “fix” the sort order is probably an indication we’re approaching this the wrong way.

One alternative approach suggested is to use an array instead (a “list” in GraphQL speak). That might look something like:

[
    {"created": "asc"},
    {"name": "asc"}
]

If we could use that approach, I think it’d be important to preserve backward compatibility if at all possible. I haven’t looked at how that might be possible yet - perhaps interfaces or union types could help.

How to reproduce

-

Possible Solution

No response

Additional Context

No response

Validations

• [X] Check that there isn't already an issue that reports the same bug
• [X] Double check that your reproduction steps work in a fresh installation of silverstripe/installer (with any code examples you've provided)

[kinglozzer]

I’ve been trying to think how we might be able to set up types for this while preserving some kind of backward compatibility with the existing sort fields. The only way I can come up with is this:

type Query {
  readMembers(
    filter: MemberFilterFields
    sort: MemberSortOptions
  ): [Member!]!
}

# New union type to support both old input (for bc) and new input
union MemberSortOptions = MemberSortFields | MemberSortExpressions

# Already exists
enum SortDirection {
  ASC
  DESC
}

# Already exists
input MemberSortFields {
  created: SortDirection
  name: SortDirection
}

# New enum of available fields to sort by
enum MemberSortField {
  name
  created
}

# New type
# Necessary because it's not possible to do a union type like: MemberSortFields | [MemberSortExpression]
type MemberSortExpressions {
  expressions: [MemberSortExpression!]!
}

# New type
type MemberSortExpression {
  field: MemberSortField,
  direction: SortDirection
}

So args for a sorted query with the new sorting approach would look like:

{
    "filter": {},
    "sort": {
        "expressions": [
            {"field": "created", "direction": "asc"},
            {"field": "name", "direction": "asc"}
        ]
    }
}

What’s not clear to me, however, is if backward compatibility is really feasible. If people are doing introspection on the schema, and expecting e.g. the existing MemberSortFields input type to be returned, would switching that to a union type break things for them? How would we “deprecate” the old approach?