plone.restapi icon indicating copy to clipboard operation
plone.restapi copied to clipboard

Querystring vocabulary ordering is not retained by REST API response

Open JeffersonBledsoe opened this issue 11 months ago • 5 comments

The sort_keys=True argument used for the response of all REST API calls means that any ordering which was present in the vocabulary is lost.

https://github.com/plone/plone.restapi/blob/7214df34f0c04b621004b79d12691c94761f0160/src/plone/restapi/services/init.py#L17-L24

JeffersonBledsoe avatar Jan 10 '25 00:01 JeffersonBledsoe

so we only have to remove sort_keys argument? @JeffersonBledsoe

ShailRT avatar Jan 10 '25 07:01 ShailRT

@JeffersonBledsoe I'm not sure I understand the specific context where you're having trouble with the ordering -- are you talking about this endpoint? https://6.docs.plone.org/plone.restapi/docs/source/endpoints/vocabularies.html#get-a-vocabulary

If so then I doubt this is related to sort_keys=True, because that's for sorting object keys, but the vocabulary terms are in an items array, not an object.

In general there's no guarantee that the order of object properties will be preserved through JSON serialization[1], so I would almost consider this a feature rather than a bug, to make sure people don't accidentally try to depend on it being preserved. Where the order is important, we need to serialize that as an array rather than an object.

[1] See https://www.rfc-editor.org/rfc/rfc7159.html#page-6: "JSON parsing libraries have been observed to differ as to whether or not they make the ordering of object members visible to calling software. Implementations whose behavior does not depend on member ordering will be interoperable in the sense that they will not be affected by these differences."

davisagli avatar Jan 10 '25 17:01 davisagli

@davisagli Thanks for the quick response! If you take a look at the querystring endpoint docs and look at the responses for something that has values in the value key (e.g. review state), you'll see the values are a dictionary with a key of 'title':

"values": {
                "external": {
                    "title": "Externally visible [external]"
                },
                "internal": {
                    "title": "Internal draft [internal]"
                },
                "internally_published": {
                    "title": "Internally published [internally_published]"
                },
                "pending": {
                    "title": "Pending [pending]"
                },
                "private": {
                    "title": "Private [private]"
                },
                "published": {
                    "title": "Published with accent \u00e9 [published]"
                },
                "rejected": {
                    "title": "Rejected [rejected]"
                },
                "spam": {
                    "title": "Spam [spam]"
                },
                "visible": {
                    "title": "Public draft [visible]"
                }
            }

I'm in the middle of a PR to plone.app.querystring (where the REST API fetches it's values from) to include an order key in the response so that even after serialization to JSON, the correct ordering can be re-achieved. Think of it the same as the 'blocks' and 'blocks_layout' keys.

In general there's no guarantee that the order of object properties will be preserved through JSON serialization, so I would almost consider this a feature rather than a bug, to make sure people don't accidentally try to depend on it being preserved. Where the order is important, we need to serialize that as an array rather than an object.

Yeah, I found this not long after posting this issue, I assumed JSON keys were ordered, but I stand corrected.

For a little context on how I came across this: We're trying to use collective.taxonomy in the Volto search block where the ordering is non-alphabetical

JeffersonBledsoe avatar Jan 10 '25 17:01 JeffersonBledsoe

@JeffersonBledsoe Thanks. I was searching the querystring endpoint docs for "vocabulary" and forgot to look for "values". This is an example of why it's always helpful to show specific examples / steps to reproduce when you report an issue, and not just describe the problem. Anyway, I digress...

Yeah, this seems like a bug to me that the current serialization can't represent the order.

I'm in the middle of a PR to plone.app.querystring (where the REST API fetches it's values from) to include an order key in the response so that even after serialization to JSON, the correct ordering can be re-achieved. Think of it the same as the 'blocks' and 'blocks_layout' keys.

Okay. Other approaches we could consider:

  1. Change values to an array instead of an object.
  2. Add a new key (values_list?) that is an array, without changing the existing one. With either of these approaches, Volto would need to support both the old and new format to stay flexible with what version of plone.restapi is used. With the first option, we would need a new major version of plone.restapi since it's not backwards-compatible.

davisagli avatar Jan 10 '25 17:01 davisagli

@JeffersonBledsoe Thanks. I was searching the querystring endpoint docs for "vocabulary" and forgot to look for "values". This is an example of why it's always helpful to show specific examples / steps to reproduce when you report an issue, and not just describe the problem. Anyway, I digress...

I completely agree, it was late when I filed this and I should have provided a better example, sorry for the drawn out discussion because of this!

Add a new key (values_list?)

Agreed on option 1 being breaking, which is why I'd like to avoid it. I did think about option 2, but the duplication of values felt like a waste of bytes and a potential tripping point for users of the endpoint (albeit a small one).p.a.querystring does currently only set title for the values, but I suppose this could have been extended by somebody at some point and so by having the order key, you still have access to these other attributes

Issue in plone.app.querystring: https://github.com/plone/plone.app.querystring/issues/161

JeffersonBledsoe avatar Jan 10 '25 17:01 JeffersonBledsoe