Query Sorting

[fonkadelic]

This addresses an issue i've came across by using a list of query items with the following format:

?key[0]=a&key[1]=b&key[2]=c

As soon as the list grows to 10+ elements the result looks like this:

?key[0]=a&key[10]=z&key[1]=b

Since certain backends require a specific order of the query items especially when it comes to list indices it's probably a good idea to use a different comparison function for the sorting.

I've introduced a comparator on URLQueryItem which is basically just an alias for a string comparison function that respects number values. It can be reused across the library to get a consistent sorting behavior.

[stephencelis]

@fonkadelic Backend requirements never cease to surprise 😆

Instead of sorting, maybe we should just respect the ordering of both the parsed/printed query string. This would require changing the Fields' type to use an array instead of a dictionary. Wanna give that a try instead? If not, we could open a dedicated issue to track the problem and will try to tackle it in the future.

[fonkadelic]

I already gave it a try to use an array instead of of a dictionary. But i immediately got stuck since a lot of parts rely heavily on dictionary features.

I'm really struggling when it comes to things like the subscript in the Fields type or merging fields in the printer. I don't see an efficient way to use an array here.

The only thing that comes to my mind would be an OrderedDictionary but i'm not sure if the sorting issue justifies the introduction of a dependency.

[fonkadelic]

@stephencelis @mbrandonw Additional feedback from your side would be highly appreciated in order to move forward here.

[stephencelis]

@fonkadelic Sorry about that! We totally lost track of this issue. We are both OK with introducing swift-collections to the library to address things. Were you able to make good progress there?

[fonkadelic]

@stephencelis No worries!

I've swapped out the Fields' dictionary for an ordered dictionary and the result looks quite promising. I think the behavior is now really similar to URLComponents which also has it's query items ordered ([URLQueryItem]).

One thing i would like to discuss though is the fact that printing query items will happen in reverse order (due to the builder syntax):

let p = Query {
  Field("a")
  Field("b")
  Field("c")
}

let data = try p.print(("1", "2", "3"))
print(data.query) // ["c": ["3"], "b": ["2"], "a": ["1"]]

I've noticed this is essentially the same problem that the Path printer already solved by introducing a dedicated PathBuilder. Do you think we should go a similar route here?

[stephencelis]

@fonkadelic I think this is more due to the fact that parser printers should print via prepending when doing so into collections. Now that we don't sort automatically, we should be more careful.

In particular, I believe this line should be updated:

swift-url-routing/Sources/URLRouting/Field.swift

Line 106 in 6baa017

input[self.name, default: []].prepend(try valueParser.print(output))

To this:

    try input.fields.updateValue(
      forKey: input.isNameCaseSensitive ? self.name : self.name.lowercased(),
      insertingDefault: [],
      at: 0,
      with: { $0.prepend(try self.valueParser.print(output)) }
    )

Which will preserve the expected order.

The name case sensitivity behavior of Fields does bother me a bit, and the fact we need to be mindful about it in yet another place here. Case-insensitivity is only enabled for headers, and as a result we have an existing bug to solve: #12

I think this issue already exists, though, and doesn't need to block this PR. Wanna try the above code snippet and see if that fixes things?

[fonkadelic]

@stephencelis Looking good. Thanks for the push in this direction.

[stephencelis]

Thanks!