plone / plone.restapi

RESTful API for Plone.
http://plonerestapi.readthedocs.org/
84 stars 73 forks source link

Collection @id field is not unique. #853

Open pnicolli opened 4 years ago

pnicolli commented 4 years ago

While querying for items in the site, I ended up with duplicate @ids. I found out that apparently the global querystring endpoint is returned as the @id for Collections, instead of the url. Here is the api query i tried. I sent a POST to http://localhost:8080/Plone2/@querystring-search with the following body:

{
    "query": [
        {
            "i": "path",
            "o": "plone.app.querystring.operation.string.relativePath",
            "v": "."
        },
        {
            "i": "portal_type",
            "o": "plone.app.querystring.operation.selection.any",
            "v": "Collection"
        }
    ],
    "fullobjects": 1
}

And this is the response I got:

{
  "@id": "http://localhost:8080/Plone2/@querystring-search",
  "items": [
    {
      "@components": {
        "actions": {
          "@id": "http://localhost:8080/Plone2/news/aggregator/@actions"
        },
        "breadcrumbs": {
          "@id": "http://localhost:8080/Plone2/news/aggregator/@breadcrumbs"
        },
        "navigation": {
          "@id": "http://localhost:8080/Plone2/news/aggregator/@navigation"
        },
        "types": {
          "@id": "http://localhost:8080/Plone2/news/aggregator/@types"
        },
        "workflow": {
          "@id": "http://localhost:8080/Plone2/news/aggregator/@workflow"
        }
      },
      "@id": "http://localhost:8080/Plone2/@querystring-search",
      "@type": "Collection",
      "UID": "5652964873654ed58c6989036c1d096c",
      "allow_discussion": false,
      "contributors": [],
      "created": "2020-01-10T11:46:26+00:00",
      "creators": [
        "admin"
      ],
      "customViewFields": [
        {
          "title": "Title",
          "token": "Title"
        },
        {
          "title": "Creator",
          "token": "Creator"
        },
        {
          "title": "Type",
          "token": "Type"
        },
        {
          "title": "ModificationDate",
          "token": "ModificationDate"
        }
      ],
      "description": "Notizie del sito",
      "effective": null,
      "exclude_from_nav": false,
      "expires": null,
      "id": "aggregator",
      "is_folderish": false,
      "item_count": 30,
      "items": [],
      "items_total": 0,
      "language": "",
      "layout": "summary_view",
      "limit": 1000,
      "modified": "2020-01-10T11:46:26+00:00",
      "parent": {
        "@id": "http://localhost:8080/Plone2/news",
        "@type": "Folder",
        "description": "Notizie del sito",
        "review_state": "published",
        "title": "Notizie"
      },
      "query": [
        {
          "i": "portal_type",
          "o": "plone.app.querystring.operation.selection.any",
          "v": [
            "News Item"
          ]
        },
        {
          "i": "review_state",
          "o": "plone.app.querystring.operation.selection.any",
          "v": [
            "published"
          ]
        }
      ],
      "relatedItems": [],
      "review_state": "published",
      "rights": "",
      "sort_on": "effective",
      "sort_reversed": true,
      "subjects": [],
      "text": null,
      "title": "Notizie",
      "version": "current"
    },
    {
      "@components": {
        "actions": {
          "@id": "http://localhost:8080/Plone2/events/aggregator/@actions"
        },
        "breadcrumbs": {
          "@id": "http://localhost:8080/Plone2/events/aggregator/@breadcrumbs"
        },
        "navigation": {
          "@id": "http://localhost:8080/Plone2/events/aggregator/@navigation"
        },
        "types": {
          "@id": "http://localhost:8080/Plone2/events/aggregator/@types"
        },
        "workflow": {
          "@id": "http://localhost:8080/Plone2/events/aggregator/@workflow"
        }
      },
      "@id": "http://localhost:8080/Plone2/@querystring-search",
      "@type": "Collection",
      "UID": "743e2066df7044b88bee59c491d796cf",
      "allow_discussion": false,
      "contributors": [],
      "created": "2020-01-10T11:46:27+00:00",
      "creators": [
        "admin"
      ],
      "customViewFields": [
        {
          "title": "Title",
          "token": "Title"
        },
        {
          "title": "Creator",
          "token": "Creator"
        },
        {
          "title": "Type",
          "token": "Type"
        },
        {
          "title": "ModificationDate",
          "token": "ModificationDate"
        }
      ],
      "description": "Eventi del sito",
      "effective": null,
      "exclude_from_nav": false,
      "expires": null,
      "id": "aggregator",
      "is_folderish": false,
      "item_count": 30,
      "items": [],
      "items_total": 0,
      "language": "",
      "layout": "event_listing",
      "limit": 1000,
      "modified": "2020-01-10T11:46:27+00:00",
      "parent": {
        "@id": "http://localhost:8080/Plone2/events",
        "@type": "Folder",
        "description": "Eventi del sito",
        "review_state": "published",
        "title": "Eventi"
      },
      "query": [
        {
          "i": "portal_type",
          "o": "plone.app.querystring.operation.selection.any",
          "v": [
            "Event"
          ]
        },
        {
          "i": "review_state",
          "o": "plone.app.querystring.operation.selection.any",
          "v": [
            "published"
          ]
        }
      ],
      "relatedItems": [],
      "review_state": "published",
      "rights": "",
      "sort_on": "start",
      "sort_reversed": true,
      "subjects": [],
      "text": null,
      "title": "Eventi",
      "version": "current"
    }
  ],
  "items_total": 2
}

Both results have the same @id. If I remove fullobjects the response contains the actual collection url in the @id field.

This is a fresh Plone site with just restapi installed, running on the latest Plone 5.2.1-pending with plone.restapi 6.1.0.

tisto commented 4 years ago

@pnicolli I wrote a quick test trying to reproduce the problem. Though, the test passes and looks fine:

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

Could you have a look and maybe try to amend the test setup to trigger the problem? It looks like you are using Collections (which we at kitconcept completely stopped using since we have the listing block).

pnicolli commented 4 years ago

@tisto I replicated your test but using Collections and I got it to break, I pushed it in your PR and I am now waiting for CI, but it breaks locally and I expect it to break on CI.

We are not using Collections on Volto sites, actually. I found this while using a Volto site on a fresh Plone site, where we have the two default Collections (News and Events) and I still feel like plone.restapi should support Collections, at least for now, for possible use cases that don't involve Volto. Otherwise it would be weird to call it a Plone restapi

EDIT: Since I don't know when the CI will run, this is how it fails locally:

Failure in test test_querystringsearch_fullobjects_complex_collection (plone.restapi.tests.test_services_querystringsearch.TestQuerystringSearchEndpoint)
Traceback (most recent call last):
  File "/usr/local/Cellar/python/3.7.5/Frameworks/Python.framework/Versions/3.7/lib/python3.7/unittest/case.py", line 59, in testPartExecutor
    yield
  File "/usr/local/Cellar/python/3.7.5/Frameworks/Python.framework/Versions/3.7/lib/python3.7/unittest/case.py", line 628, in run
    testMethod()
  File "/Users/pieronicolli/Lab/plone52/src/plone.restapi/src/plone/restapi/tests/test_services_querystringsearch.py", line 251, in test_querystringsearch_fullobjects_complex_collection
    response.json()["items"][4]["@id"], u"{}/testcollection9".format(self.portal_url))
  File "/usr/local/Cellar/python/3.7.5/Frameworks/Python.framework/Versions/3.7/lib/python3.7/unittest/case.py", line 1351, in deprecated_func
    return original_func(*args, **kwargs)
  File "/usr/local/Cellar/python/3.7.5/Frameworks/Python.framework/Versions/3.7/lib/python3.7/unittest/case.py", line 852, in assertEqual
    assertion_func(first, second, msg=msg)
  File "/usr/local/Cellar/python/3.7.5/Frameworks/Python.framework/Versions/3.7/lib/python3.7/unittest/case.py", line 1233, in assertMultiLineEqual
    self.fail(self._formatMessage(msg, standardMsg))
  File "/usr/local/Cellar/python/3.7.5/Frameworks/Python.framework/Versions/3.7/lib/python3.7/unittest/case.py", line 693, in fail
    raise self.failureException(msg)
AssertionError: 'http://localhost:55001/plone/@querystring-search' != 'http://localhost:55001/plone/testcollection9'
- http://localhost:55001/plone/@querystring-search
+ http://localhost:55001/plone/testcollection9
tisto commented 4 years ago

@pnicolli fair enough. I have no intention to rip out Collections from plone.restapi. It is just something that is low on my personal priorities list.