Repository: plone/plone.restapi (language: python)

Issue:
SearchableText indexer should preserve order of blocks



Discussion / hints:

Fix patch:
diff --git a/docs/source/blocks.rst b/docs/source/blocks.rst
index 2e4a9c117a..e957bf3397 100644
--- a/docs/source/blocks.rst
+++ b/docs/source/blocks.rst
@@ -169,7 +169,29 @@ SearchableText indexing for blocks
 
 As the main consumer of plone.restapi's blocks, this functionality is specific to Volto blocks. By default searchable text (for Plone's SearchableText index) is extracted from `text` blocks.
 
-To extract searchable text for other types of blocks, you need to write an adapter that can process that type of block.::
+To extract searchable text for other types of blocks, there are two approaches:
+
+Client side solution
+++++++++++++++++++++
+
+The block provides the data to be indexed in its **searchableText** attribute::
+
+  {
+    "@type": "image",
+    "align": "center",
+    "alt": "Plone Conference 2021 logo",
+    "searchableText": "Plone Conference 2021 logo",
+    "size": "l",
+    "url": "https://2021.ploneconf.org/images/logoandfamiliesalt.svg"
+  }
+
+
+This is the preferred solution.
+
+Server side solution
+++++++++++++++++++++
+
+ For each new block, you need to write an adapter that will extract the searchable text from the block information::
 
   @implementer(IBlockSearchableText)
   @adapter(IBlocks, IBrowserRequest)
@@ -186,4 +208,3 @@ See ``plone.restapi.interfaces.IBlockSearchableText`` for details. The ``__call_
 This adapter needs to be registered as a named adapter, where the name is the same as the block type (its `@type` property from the block value).::
 
     <adapter name="image" factory=".indexers.ImageBlockSearchableText" />
-
diff --git a/news/1292.bugfix b/news/1292.bugfix
new file mode 100644
index 0000000000..33c15b53c8
--- /dev/null
+++ b/news/1292.bugfix
@@ -0,0 +1,2 @@
+SearchableText indexer should maintain the order of the blocks
+[ericof]
\ No newline at end of file
diff --git a/src/plone/restapi/indexers.py b/src/plone/restapi/indexers.py
index c3663a91d8..b7dd49c8d9 100644
--- a/src/plone/restapi/indexers.py
+++ b/src/plone/restapi/indexers.py
@@ -2,7 +2,7 @@
 # This is an experimental feature meant for use in Volto only!
 # This code is likely to change in the future, even within minor releases.
 # We will make sure plone.restapi latest always works with the latest Volto release.
-# This code is planned to being refactored into CMFPlone 6.0 as soon as Volto 4 final is out.
+# This code is planned to being refactored into plone.volto before CMFPlone 6.0 is out.
 # <tisto@plone.org>
 
 from plone.app.contenttypes.indexers import SearchableText
@@ -16,28 +16,28 @@
 from zope.publisher.interfaces.browser import IBrowserRequest
 
 
-def _extract_text(block):
-    result = ""
-    for paragraph in block.get("text", {}).get("blocks", {}):
-        text = paragraph["text"]
-        result = " ".join((result, text))
-    return result
-
-
 @implementer(IBlockSearchableText)
 @adapter(IBlocks, IBrowserRequest)
 class TextBlockSearchableText:
+    """Searchable Text indexer for Text (DraftJS) blocks."""
+
     def __init__(self, context, request):
         self.context = context
         self.request = request
 
     def __call__(self, value):
-        return _extract_text(value)
+        result = ""
+        for paragraph in value.get("text", {}).get("blocks", {}):
+            text = paragraph["text"]
+            result = " ".join((result, text))
+        return result
 
 
 @implementer(IBlockSearchableText)
 @adapter(IBlocks, IBrowserRequest)
 class TableBlockSearchableText:
+    """Searchable Text indexer for Table blocks."""
+
     def __init__(self, context, request):
         self.context = context
         self.request = request
@@ -55,59 +55,85 @@ def __call__(self, value):
         return result
 
 
-@indexer(IBlocks)
-def SearchableText_blocks(obj):
-    request = getRequest()
-    blocks = obj.blocks
-    blocks_text = []
-
-    for block in blocks.values():
-
-        searchableText = block.get("searchableText", "")
-        if searchableText:
-            # TODO: should we evaluate in some way this value? maybe passing
-            # it into html/plain text transformer?
-            blocks_text.append(searchableText)
+@implementer(IBlockSearchableText)
+@adapter(IBlocks, IBrowserRequest)
+class SlateTextIndexer:
+    """Searchable Text indexer for Slate blocks."""
 
-        block_type = block.get("@type", "")
-        adapter = queryMultiAdapter(
-            (obj, request), IBlockSearchableText, name=block_type
-        )
+    def __init__(self, context, request):
+        self.context = context
+        self.request = request
 
-        if adapter is not None:
-            text = adapter(block)
+    def __call__(self, block):
+        block = block or {}
+        # BBB compatibility with slate blocks that used the "plaintext" field
+        return block.get("plaintext", "")
 
-            if text:
-                blocks_text.append(text)
 
-    std_text = SearchableText(obj)
-    blocks_text.append(std_text)
+def extract_subblocks(block):
+    """Extract subblocks from a block.
 
-    return " ".join(blocks_text)
+    :param block: Dictionary with block information.
+    :returns: A list with subblocks, if present, or an empty list.
+    """
+    if "data" in block and "blocks" in block["data"]:
+        raw_blocks = block["data"]["blocks"]
+    elif "blocks" in block:
+        raw_blocks = block["blocks"]
+    else:
+        raw_blocks = None
+    return list(raw_blocks.values()) if isinstance(raw_blocks, dict) else []
 
 
-class SlateTextIndexer:
-    """SlateTextIndexer."""
+def extract_text(block, obj, request):
+    """Extract text information from a block.
 
-    def __init__(self, context, request):
-        """__init__.
+    This function tries the following methods, until it finds a result:
+        1. searchableText attribute
+        2. Server side adapter
+        3. Subblocks
 
-        :param context:
-        :param request:
-        """
-        self.context = context
-        self.request = request
+    The decision to use the server side adapter before the subblocks traversal
+    allows addon developers to choose this implementation when they want a
+    more granular control of the indexing.
 
-    def __call__(self, block):
-        """__call__.
+    :param block: Dictionary with block information.
+    :param obj: Context to be used to get a IBlockSearchableText.
+    :param request: Current request.
+    :returns: A string with text found in the block.
+    """
+    result = ""
+    block_type = block.get("@type", "")
+    # searchableText is the conventional way of storing
+    # searchable info in a block
+    searchableText = block.get("searchableText", "")
+    if searchableText:
+        # TODO: should we evaluate in some way this value? maybe passing
+        # it into html/plain text transformer?
+        return searchableText
+    # Use server side adapters to extract the text data
+    adapter = queryMultiAdapter((obj, request), IBlockSearchableText, name=block_type)
+    result = adapter(block) if adapter is not None else ""
+    if not result:
+        subblocks = extract_subblocks(block)
+        for subblock in subblocks:
+            tmp_result = extract_text(subblock, obj, request)
+            result = f"{result}\n{tmp_result}"
+    return result
 
-        :param block:
-        """
-        # text indexer for slate blocks. Relies on the slate field
-        block = block or {}
 
-        if block.get("searchableText"):
-            return
+@indexer(IBlocks)
+def SearchableText_blocks(obj):
+    """Extract text to be used by the SearchableText index in the Catalog."""
+    request = getRequest()
+    blocks = obj.blocks
+    blocks_layout = obj.blocks_layout
+    blocks_text = []
+    for block_id in blocks_layout.get("items", []):
+        block = blocks.get(block_id, {})
+        blocks_text.append(extract_text(block, obj, request))
 
-        # BBB compatibility with slate blocks that used the "plaintext" field
-        return (block or {}).get("plaintext", "")
+    # Extract text using the base plone.app.contenttypes indexer
+    std_text = SearchableText(obj)
+    blocks_text.append(std_text)
+    return " ".join([text.strip() for text in blocks_text if text.strip()])
diff --git a/src/plone/restapi/indexers.zcml b/src/plone/restapi/indexers.zcml
index 39ec49297c..345a065ecb 100644
--- a/src/plone/restapi/indexers.zcml
+++ b/src/plone/restapi/indexers.zcml
@@ -15,9 +15,6 @@
         />
     <adapter
         factory=".indexers.SlateTextIndexer"
-        provides="plone.restapi.interfaces.IBlockSearchableText"
-        for="*
-             *"
         name="slate"
         />
     <adapter


Test patch:
diff --git a/src/plone/restapi/tests/test_blocks_searchable_text.py b/src/plone/restapi/tests/test_blocks_searchable_text.py
index 414679d5e3..9867a371c5 100644
--- a/src/plone/restapi/tests/test_blocks_searchable_text.py
+++ b/src/plone/restapi/tests/test_blocks_searchable_text.py
@@ -50,6 +50,7 @@ def test_search_text(self):
         response = self.api_session.patch(
             "/doc",
             json={
+                "blocks_layout": {"items": ["uuid1", "uuid2"]},
                 "blocks": {
                     "uuid1": {
                         "@type": "text",
@@ -85,7 +86,7 @@ def test_search_text(self):
                             "entityMap": {},
                         },
                     },
-                }
+                },
             },
         )
 
@@ -147,6 +148,7 @@ def __call__(self, value):
         }
 
         self.doc.blocks = blocks
+        self.doc.blocks_layout = {"items": ["uuid1", "uuid3"]}
         from plone.indexer.interfaces import IIndexableObject
         from zope.component import queryMultiAdapter
 
@@ -160,6 +162,7 @@ def test_index_searchableText_value(self):
         response = self.api_session.patch(
             "/doc",
             json={
+                "blocks_layout": {"items": ["uuid1", "uuid2"]},
                 "blocks": {
                     "uuid1": {
                         "@type": "text",
@@ -182,7 +185,7 @@ def test_index_searchableText_value(self):
                         "@type": "custom_type",
                         "searchableText": "custom text foo",
                     },
-                }
+                },
             },
         )
 
@@ -231,10 +234,12 @@ def test_search_slate_text(self):
                 "value": [],
             },
         }
-        self.doc.blocks_layout = [
-            "38541872-06c2-41c9-8709-37107e597b18",
-            "4fcfeb9b-f73e-427c-9e06-2e4d53b06865",
-        ]
+        self.doc.blocks_layout = {
+            "items": [
+                "38541872-06c2-41c9-8709-37107e597b18",
+                "4fcfeb9b-f73e-427c-9e06-2e4d53b06865",
+            ]
+        }
         self.portal.portal_catalog.indexObject(self.doc)
 
         query = {"SearchableText": "climatic"}
@@ -342,9 +347,7 @@ def test_search_table_text(self):
                 },
             }
         }
-        self.doc.blocks_layout = [
-            "uuid1",
-        ]
+        self.doc.blocks_layout = {"items": ["uuid1"]}
         self.portal.portal_catalog.indexObject(self.doc)
 
         query = {"SearchableText": "foo"}
diff --git a/src/plone/restapi/tests/test_indexers.py b/src/plone/restapi/tests/test_indexers.py
new file mode 100644
index 0000000000..0e227a4c9f
--- /dev/null
+++ b/src/plone/restapi/tests/test_indexers.py
@@ -0,0 +1,215 @@
+from plone.dexterity.interfaces import IDexterityFTI
+from plone.restapi.indexers import SearchableText_blocks
+from plone.restapi.testing import PLONE_RESTAPI_DX_INTEGRATION_TESTING
+from uuid import uuid4
+from zope.component import queryUtility
+
+import unittest
+
+
+DRAFTJS_BLOCK = {
+    "@type": "text",
+    "text": {
+        "blocks": [
+            {
+                "data": {},
+                "depth": 0,
+                "entityRanges": [],
+                "inlineStyleRanges": [],
+                "key": "behki",
+                "text": "Plone is a powerful content management system",
+                "type": "unstyled",
+            }
+        ],
+        "entityMap": {},
+    },
+}
+
+SLATE_BLOCK = {
+    "@type": "slate",
+    "plaintext": "Follow Plone Conference",
+    "value": [{"children": [{"text": "Follow Plone Conference"}]}],
+}
+
+
+TABLE_BLOCK = {
+    "@type": "table",
+    "table": {
+        "rows": [
+            {
+                "key": "a",
+                "cells": [
+                    {
+                        "type": "data",
+                        "key": "b",
+                        "value": {
+                            "blocks": [
+                                {
+                                    "data": {},
+                                    "depth": 0,
+                                    "entityRanges": [],
+                                    "inlineStyleRanges": [],
+                                    "key": "fgm98",
+                                    "text": "My header",
+                                    "type": "header-two",
+                                },
+                            ],
+                            "entityMap": {},
+                        },
+                    },
+                ],
+            },
+            {
+                "key": "c",
+                "cells": [
+                    {
+                        "type": "data",
+                        "key": "d",
+                        "value": {
+                            "blocks": [
+                                {
+                                    "data": {},
+                                    "depth": 0,
+                                    "entityRanges": [],
+                                    "inlineStyleRanges": [],
+                                    "key": "mmm99",
+                                    "text": "My data",
+                                    "type": "text",
+                                },
+                            ],
+                            "entityMap": {},
+                        },
+                    },
+                ],
+            },
+        ]
+    },
+}
+
+
+class TestSearchableTextIndexer(unittest.TestCase):
+
+    layer = PLONE_RESTAPI_DX_INTEGRATION_TESTING
+
+    def setUp(self):
+        self.portal = self.layer["portal"]
+        self.request = self.layer["request"]
+        fti = queryUtility(IDexterityFTI, name="Document")
+        behavior_list = [a for a in fti.behaviors]
+        behavior_list.append("volto.blocks")
+        fti.behaviors = tuple(behavior_list)
+        self.portal.invokeFactory(
+            "Document",
+            id="doc1",
+            title="Title is here",
+            description="Description is there",
+            blocks={},
+            blocks_layout={"items": []},
+        )
+        self.document = self.portal["doc1"]
+
+    @staticmethod
+    def _extract_searchable_text(obj):
+        indexer = SearchableText_blocks(obj)
+        return indexer()
+
+    @staticmethod
+    def _add_blocks(obj, raw_blocks):
+        blocks = {str(uuid4()): block for block in raw_blocks}
+        layout = list(blocks.keys())
+        obj.blocks = blocks
+        obj.blocks_layout["items"] = layout
+
+    def test_indexer_no_blocks(self):
+        result = self._extract_searchable_text(self.document)
+        self.assertIn("Title is here", result)
+        self.assertIn("Description is there", result)
+
+    def test_indexer_block_has_searchableText(self):
+        document = self.document
+        self._add_blocks(
+            document,
+            [{"@type": "new-block", "attribute": "bar", "searchableText": "Foo Bar"}],
+        )
+        result = self._extract_searchable_text(document)
+        self.assertIn("Title is here Description is there", result)
+        self.assertIn("Foo Bar", result)
+
+    def test_indexer_multiple_blocks(self):
+        document = self.document
+        self._add_blocks(
+            document,
+            [
+                {"@type": "a-block", "key": "value", "searchableText": "Plone is a"},
+                {"@type": "a-block", "attribute": "bar", "searchableText": "CMS"},
+            ],
+        )
+        result = self._extract_searchable_text(document)
+        self.assertIn("Plone is a CMS ", result)
+
+    def test_indexer_block_slate_bbb(self):
+        document = self.document
+        self._add_blocks(
+            document,
+            [
+                SLATE_BLOCK,
+            ],
+        )
+        result = self._extract_searchable_text(document)
+        self.assertIn("Title is here Description is there", result)
+        self.assertIn("Follow Plone Conference", result)
+
+    def test_indexer_block_text(self):
+        document = self.document
+        self._add_blocks(
+            document,
+            [
+                DRAFTJS_BLOCK,
+            ],
+        )
+        result = self._extract_searchable_text(document)
+        self.assertIn("Title is here Description is there", result)
+        self.assertIn("Plone is a powerful content management system", result)
+
+    def test_indexer_block_table(self):
+        document = self.document
+        self._add_blocks(
+            document,
+            [
+                TABLE_BLOCK,
+            ],
+        )
+        result = self._extract_searchable_text(document)
+        self.assertIn("Title is here Description is there", result)
+        self.assertIn("My data", result)
+
+    def test_indexer_block_with_subblocks(self):
+        document = self.document
+        block = {
+            "@type": "complex-block",
+            "blocks": {
+                "ad625c27-670a-412e-821d-57870b6e82f1": SLATE_BLOCK,
+                "80c5d58d-4baf-4adc-8ef4-36810df5628d": DRAFTJS_BLOCK,
+                "4061b21d-c971-4ccc-a719-d8b24214fd8b": {
+                    "@type": "old-complex-block",
+                    "data": {
+                        "blocks": {
+                            "116feae0-7aa3-40f4-a776-fc893d78b353": TABLE_BLOCK,
+                        }
+                    },
+                },
+            },
+        }
+        self._add_blocks(
+            document,
+            [
+                block,
+            ],
+        )
+        result = self._extract_searchable_text(document)
+        # From Slate sub block
+        self.assertIn("Follow Plone Conference", result)
+        # From DraftJS
+        self.assertIn("Plone is a powerful content management system", result)
+        # From Table block
+        self.assertIn("My data", result)