[RFC] [Discussion] Query Parameter Manipulation Support

Hey Tim,

Thank you for the help, I forgot to link the RFC indeed.

Let me answer your questions:

I don't think it is as necessary to make it readonly as it is with the
URI classes, since the query parameters class is likely to be used for
“local modification” and is less likely to be passed around as a value
object. If efficient copy-on-write is possible internally, then making
it readonly is the safer choice, of course.

Yes, as far as I know from Ilija, it's possible to optimize modification
based on
refcount: if a readonly property which is to be modified has a refcount of
1 and
there's no weakref, it can be safely updated in-place.

With regard to the RFC text, I've given it a first quick skim and have
the following notes:

1. Within the "Relation to the query component" section.

The $uri = new Uri\Rfc3986\Uri("https://example.com?foo=a b"); example
is invalid. An URI containing a space is not valid and the Uri class
will throw an exception. Can you double-check that part of the RFC?
Without looking it up, I suspect that the space should be %20 for
RFC3986?

Nice catch, I'll review and fix the examples using "a b". It is indeed
incorrect, and %20
should be used instead. as you said.

4. Within the "Supported types" section.

“The above conversion rules work for both UriQueryParams and
UrlQueryParams.”

and the examples also use Uri\Rfc3986\UriQueryParams /
Uri\WhatWg\UrlQueryParams.

This seems like an oversight from the original implementation, since
there are no longer separate classes. Can you double-check the entire
section?

I intentionally haven't updated this section yet: because there's important
info about
the difference between how WHATWG specification and uriparser handles null,
so I didn't want to "lose" this (I know it remains in the changelog).

5. Within the "Array API" section.

This also uses Uri\Rfc3986\UriQueryParams in the examples

Same situation as above, but basically the whole section is a big question
mark:
I couldn't come up with a good enough idea how arrays could be handled
efficiently and ergonomically, and in the same time, how to (mostly) conform
to WHATWG URL.

Some of my problems:

• How should $params->get("foo[bar]") behave? Should we try to parse the
  supplied key as an array and return the
  nested item if it exists? I suppose, we would internally store the query
  params as an array (just like how $_GET does),
  so then we would definitely need parsing in order to be able to return item
  "bar" stored within array "foo".
• However, what should a $params->append("foo[bar]") call do when the query
  param "foo" is already added with a string value?
  Should it modify the existing item to be an array (but then how?) or we
  should just add "foo[bar]" to the query params as-is?
• How to recompose list parameters? Ideally, we should not append [] to
  these parameters (e.g.
  "list=1&list=2" vs "list[]=1&list[]=2"), and actually, it is not needed for
  arrays in the root level, but as far as I understood,
  it's a must-have for nested lists (e.g. "map[list]=1&map[list]=2" vs.
  map[list[]]=1&map[list[]]=2), otherwise the representation
  would be ambiguous for lists with 1 item: "map[list]=1" would mean that the
  map has a list item with a value of "1". So
  we either always have to use the [] suffix for lists, or we should make a
  distinction between root-level and nested lists.

I know, most of my questions revolve around parsing the $name parameter,
but I would really want to minimize the number
of times when the $name param has to be parsed run-time so that the
performance is comparable to native array key
lookups. I don't have any idea how much the parsing would cost though...

So at this point, coming up with a sensible plan is needed, and I can fix
most inconsistencies in the RFC afterwards.

Regards,
Máté

Hi Jordi,

In the examples you mention the usage of parse and toString. I assume

there is no “default” and this is a placeholder for one of the given
implementations. But it’s perhaps better to pick one of for the examples as
it suggests it would also exist.

Ah, thanks for the info! Most of these were omission in fact, as I forgot
to update the original method names after I unified the two QueryParams
classes.

When it comes to the implementation I believe it would be nice to also
have a getKeys() method. That would only return the keys.

You do mention the focus is to move away from $_GET. I like the idea, but
implementing QueryString would still require something like the following:

Uri\QueryParams::fromArray($_GET);

If we really want to make $_GET obsolete, wouldn’t it be nice to have a
fromRequest() which would directly parse $_GET or use
$_SERVER["QUERY_STRING”].

As far as I can see, Uri\QueryParams::fromArray($_GET); wouldn't be needed.
Do you have any use-case in mind where the suggested
"Uri\QueryParams::parseRfc3986($_SERVER["QUERY_STRING"]);" call wouldn't
work instead? I'm not against adding a fromRequest()
method though if others find it useful (I'm a bit neutral about it). And
I'd probably use a fromCurrentRequest() or a similar name to
highlight the fact that it uses the current request as source.

As for whether or not this should be a readonly class. I think it should
be. The class itself is following various specs. And since all other
classes in the same namespace are also readonly, it should follow the same
principle (also for consistency) I believe.

I think the most problematic case is to return a mutable class from a
readonly class (e.g. Uri\Rfc3986\Uri::getQueryParams()). So we should either
omit this method, or make Uri\QueryParams readonly indeed.

Regards,
Máté