Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add facet count endpoint #1637

Draft
wants to merge 19 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/plone/restapi/services/configure.zcml
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
<include package=".copymove" />
<include package=".database" />
<include package=".discussion" />
<include package=".facet_count" />
<include package=".groups" />
<include package=".navigation" />
<include package=".contextnavigation" />
Expand Down
Empty file.
16 changes: 16 additions & 0 deletions src/plone/restapi/services/facet_count/configure.zcml
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
<configure
xmlns="http://namespaces.zope.org/zope"
xmlns:cache="http://namespaces.zope.org/cache"
xmlns:plone="http://namespaces.plone.org/plone"
xmlns:zcml="http://namespaces.zope.org/zcml"
>

<plone:service
method="POST"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be nice to support calling this as a GET with the query encoded in the querystring like we recently added for the @querystringsearch endpoint. It makes it possible to cache the result in varnish when varnish is configured to not cache any POST requests.

factory=".get.FacetCountGet"
for="Products.CMFPlone.interfaces.IPloneSiteRoot"
permission="zope2.View"
name="@facet-count"
/>

</configure>
54 changes: 54 additions & 0 deletions src/plone/restapi/services/facet_count/get.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
from plone.restapi.deserializer import json_body
from plone.restapi.services import Service
from plone.restapi.services.querystringsearch.get import QuerystringSearch
from Products.CMFCore.utils import getToolByName
import json


class FacetCountGet(Service):
"""Returns facet count."""

def reply(self):
facet_count = {}
ctool = getToolByName(self.context, "portal_catalog")
body = json_body(self.request)

facet = body.get("facet", None)
query = body.get("query", None)

try:
index = ctool._catalog.getIndex(facet)
except:
index = None

if query:
body["query"] = [qs for qs in query if qs["i"] != facet]
self.request.set("BODY", json.dumps(body))

brains = QuerystringSearch(self.context, self.request).getResults()

brains_rids = set(brain.getRID() for brain in brains)
index_rids = [rid for rid in index.documentToKeyMap()] if index else []
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should probably consider adding an official API in ZCatalog to do a query and get the results as document ids rather than brains. There is some overhead to loading the brains for each result item (and it leads to more churn in the ZODB cache) so it could help performance.

Actually now that I see how it works, I wonder if it would perform better to get the counts for multiple facets in a single backend request, rather than a separate request for each facet. That way the main query would only need to be done once.

Copy link
Contributor Author

@razvanMiu razvanMiu May 17, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, we can make it return the count for multiple facets but we would still have to make a querystringsearch for each facet because we have to exclude the facet itself from the query.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@razvanMiu oh right, I skipped over that part. Makes sense. Then there's probably less benefit from combining into a single request for multiple facets.


rids = brains_rids.intersection(index_rids)

for rid in rids:
for key in index.keyForDocument(rid):
if key not in facet_count:
facet_count[key] = 0
facet_count[key] += 1

return {
"@id": "%s/@facet-count" % self.context.absolute_url(),
"facets": {
facet: {
"count": len(rids),
"data": [
{"value": key, "count": value}
for key, value in facet_count.items()
],
}
}
if facet
else {},
}