raku-community-modules / URI

Raku realization of URI — Uniform Resource Identifiers handler
Artistic License 2.0
3 stars 14 forks source link

The query-form incorrectly treats the query as a Hash #33

Closed zostay closed 3 years ago

zostay commented 7 years ago

While working on my PR for #27, I discovered a problem with the URI interface provided. I'm not sure it can be fixed without breaking existing code, but I believe it needs to be broken. The problem is the code current provides query-form as a Hash. This has a few problems:

  1. Hashes are mutable, so you can modify the query-form, but it has no impact on the object. So, a Map would be more appropriate until my PR for #27 is implemented or some other mutator implementation is built. (Pretty sure that Map did not exist when this interface was first created as the GLR hadn't happened yet.) This wouldn't be a problem if $!query-form were not an accessor and query-form just re-ran split-query on each call.
  2. A Perl 6 Hash (and Map) are unordered, but query forms are ordered.
  3. A Perl 6 Hash requires unique keys, but query forms allow duplicate keys.

To address the last issue, the interface allows for a mixed-value approach where a query form containing duplicate keys causes that key in the Hash to be changed from a Str to an Array. So some of the values are strings and some are arrays of strings. This allows the order of values to be preserved, but not the order of keys. It also means that the caller has to be carefully aware of what it is doing to avoid problems.

The most obvious solution is to use one similar to that of the Perl 5 URI module, which is to just returns a list of Pairs. This would be simplest to implement. However, it will break code that depends on it:

my $u = URI.new("http://example.com/?foo=bar&foo=baz&bar=qux");
my %q = $u.query-form;
say "bar = %q<bar>"; #works!
say "foo = %q<foo>[0]"; #FAIL
say "foo = $u.query-form<foo>[1]"; #FAIL

Another solution might be to just document this deficiency and add a different method that does this better.

jonathanstowe commented 5 years ago

I think this may be addressed by the #44 to some extent.

jonathanstowe commented 3 years ago

The API has changed somewhat but this actually does work as expected:

use URI;

my $u = URI.new("http://example.com/?foo=bar&foo=baz&bar=qux");
my $q = $u.query;
say "bar = $q<bar>";
say "foo = $q<foo>[0]";
say "foo = ", $u.query<foo>[1];

Imma close this.