Undeprecate comma separated expansion parameters

[tisto]

Expanding multiple expansions at the same time used to be passed via a comma-separated list:

?expand=actions,breadcrumbs,navigation,workflow,types

The docs still show this actually:

https://plonerestapi.readthedocs.io/en/latest/usage/expansion.html?highlight=expand#expansion

Though, this was deprecated some time ago and instead one should use:

?expand:list=actions&expand:list=breadcrumbs&expand:list=navigation&expand:list=workflow&expand:list=types

[davisagli]

This is much less readable, and unfamiliar to anyone who doesn't know Zope. I would un-deprecate the old way and continue to support it.

[tisto]

@davisagli I agree that the new recommended way is far less readable. However, we have to make sure that the way we pass a list via URL parameters is consistent. We can use the catalog way or come up with our own way. Though, it needs to be consistent. We decided some time ago that sticking with the zcatalog way would be the most pragmatic option we have.

Here is the PR where this was introduced:

https://github.com/plone/plone.restapi/pull/1299

cc @sneridagh

FYI: I don't consider this to be a blocker for a 9.0.0 release. It was just something that I found when checking for deprecation warnings in the p.restapi source code.

[davisagli]

I'm not sure what you mean by "the zcatalog way". Processsing parameters using suffixes like :list is a feature of the ZPublisher, not ZCatalog.

What makes it more pragmatic?

[tisto]

@davisagli you are correct. This is how the zpublisher translates URL parameters to zcatalog queries.

This discussion is as old as plone.restapi. The question is: how do we encode complex data structures list nested dicts in a URL for a GET request (so that can be cached). We want a consistent way of doing so that works for simple lists (like for expand) as well as for more complex use cases (querystring-search, search endpoint).

My point is that we have to be consistent in how we pass lists or dicts or nested dicts in a URL. We need a way to do this on the backend with Python and on the frontend with JavaScript. Unfortunately, there is no recommended way how to do this. I did some research:

https://github.com/plone/plone.restapi/issues/1252

Lukas Graf did some research:

https://github.com/plone/plone.restapi/issues/45

and lately @robgietema as well if I am not mistaken. @sneridagh created the PR that deprecated the comma-separated way that we are currently discussing.

The pragmatic solution to this was, so far, that we just stick to the default way the ZPublisher handles this. This is definitely not the most elegant way, but at least we know what we are up to. The deprecation of the comma-separated way was our attempt to handle things consistently.

As said, I do not consider this to be a blocker for the 9 release. We can keep things as they are and just deprecate something for plone.restapi 10.

[davisagli]

I agree with the goal of consistency. But I don't think the ZPublisher approach handles everything we need -- particularly nested structures. So trying to use that as the consistent approach will not work, because then we'll still have to use something else when we need to serialize something it doesn't handle. For example we ended up implementing the GET version of @querystring-search using serialized JSON, not Zope parameter marshalling.

[tisto]

@davisagli right, I forgot about this. This would mean, if we would use JSON URL encoding, it would look like this:

expand=foo%2Cbar%2Cbaz

Bottom line I guess is that we are horribly inconsistent in what we use right now...

For the sake of completeness, here is where we currently use ":list" in plone.restapi:

https://github.com/search?q=repo%3Aplone%2Fplone.restapi%20%3Alist&type=code

[davisagli]

We can consider using 2 different approaches and still call it consistent as long as we have clear rules on when to use each one.

Anyway, yeah, I'd say let's continue this discussion but not try to push it for plone.restapi 9

[davisagli]

Done in #1707