Merge pull request #105 from tangkong/bug_tree_fill

BUG: fix tree fill behavior

docs/source/upcoming_release_notes/105-bug_tree_fill.rst

@@ -0,0 +1,24 @@
+105 bug_tree_fill
+#################
+
+API Breaks
+----------
+- N/A
+
+Features
+--------
+- N/A
+
+Bugfixes
+--------
+- Properly fill items in the the shared tree view (`RootTree`)
+
+Maintenance
+-----------
+- Adjust `Client.fill` to allow us to specify fill depth
+- Move `FilestoreBackend.compare` upstream into `_Backend`
+- Implement `.search` and `.root` in `TestBackend` for completeness
+
+Contributors
+------------
+- tangkong

superscore/backends/core.py

@@ -1,6 +1,7 @@
 """
 Base superscore data storage backend interface
 """
+import re
 from collections.abc import Container, Generator
 from typing import NamedTuple, Union
 from uuid import UUID
@@ -66,6 +67,41 @@ def search(self, *search_terms: SearchTermType) -> Generator[Entry, None, None]:
         """
         raise NotImplementedError
 
+    @staticmethod
+    def compare(op: str, data: AnyEpicsType, target: SearchTermValue) -> bool:
+        """
+        Return whether data and target satisfy the op comparator, typically during application
+        of a search filter. Possible values of op are detailed in _Backend.search
+
+        Parameters
+        ----------
+        op: str
+            one of the comparators that all backends must support, detailed in _Backend.search
+        data: AnyEpicsType | Tuple[AnyEpicsType]
+            data from an Entry that is being used to decide whether the Entry passes a filter
+        target: AnyEpicsType | Tuple[AnyEpicsType]
+            the filter value
+
+        Returns
+        -------
+        bool
+            whether data and target satisfy the op condition
+        """
+        if op == "eq":
+            return data == target
+        elif op == "lt":
+            return data <= target
+        elif op == "gt":
+            return data >= target
+        elif op == "in":
+            return data in target
+        elif op == "like":
+            if isinstance(data, UUID):
+                data = str(data)
+            return re.search(target, data)
+        else:
+            raise ValueError(f"SearchTerm does not support operator \"{op}\"")
+
     @property
     def root(self) -> Root:
         """Return the Root Entry in this backend"""

superscore/backends/filestore.py

@@ -6,18 +6,16 @@
 import json
 import logging
 import os
-import re
 import shutil
 from dataclasses import fields, replace
 from typing import Any, Dict, Generator, Optional, Union
 from uuid import UUID, uuid4
 
 from apischema import deserialize, serialize
 
-from superscore.backends.core import SearchTermType, SearchTermValue, _Backend
+from superscore.backends.core import SearchTermType, _Backend
 from superscore.errors import BackendError
 from superscore.model import Entry, Root
-from superscore.type_hints import AnyEpicsType
 from superscore.utils import build_abs_path
 
 logger = logging.getLogger(__name__)
@@ -304,41 +302,6 @@ def search(self, *search_terms: SearchTermType) -> Generator[Entry, None, None]:
                 if all(conditions):
                     yield entry
 
-    @staticmethod
-    def compare(op: str, data: AnyEpicsType, target: SearchTermValue) -> bool:
-        """
-        Return whether data and target satisfy the op comparator, typically durihg application
-        of a search filter. Possible values of op are detailed in _Backend.search
-
-        Parameters
-        ----------
-        op: str
-            one of the comparators that all backends must support, detailed in _Backend.search
-        data: AnyEpicsType | Tuple[AnyEpicsType]
-            data from an Entry that is being used to decide whether the Entry passes a filter
-        target: AnyEpicsType | Tuple[AnyEpicsType]
-            the filter value
-
-        Returns
-        -------
-        bool
-            whether data and target satisfy the op condition
-        """
-        if op == "eq":
-            return data == target
-        elif op == "lt":
-            return data <= target
-        elif op == "gt":
-            return data >= target
-        elif op == "in":
-            return data in target
-        elif op == "like":
-            if isinstance(data, UUID):
-                data = str(data)
-            return re.search(target, data)
-        else:
-            raise ValueError(f"SearchTerm does not support operator \"{op}\"")
-
     @contextlib.contextmanager
     def _load_and_store_context(self) -> Generator[Dict[UUID, Any], None, None]:
         """

superscore/backends/test.py

@@ -1,41 +1,57 @@
 """
 Backend that manipulates Entries in-memory for testing purposes.
 """
-from typing import List, Optional, Union
+from copy import deepcopy
+from typing import Dict, List, Optional, Union
 from uuid import UUID
 
-from superscore.backends.core import _Backend
+from superscore.backends.core import SearchTermType, _Backend
 from superscore.errors import (BackendError, EntryExistsError,
                                EntryNotFoundError)
-from superscore.model import Entry, Nestable
+from superscore.model import Entry, Nestable, Root
 
 
 class TestBackend(_Backend):
     """Backend that manipulates Entries in-memory, for testing purposes."""
+    _entry_cache: Dict[UUID, Entry] = {}
+
     def __init__(self, data: Optional[List[Entry]] = None):
         if data is None:
             self.data = []
         else:
             self.data = data
 
+        self._root = Root(entries=self.data)
+        self._fill_entry_cache()
+
+    def _fill_entry_cache(self) -> None:
+        self._entry_cache = {}
+        stack = deepcopy(self.data)
+        while len(stack) > 0:
+            entry = stack.pop()
+            uuid = entry.uuid
+            if isinstance(uuid, str):
+                uuid = UUID(uuid)
+            self._entry_cache[uuid] = entry
+            if isinstance(entry, Nestable):
+                stack.extend(entry.children)
+
     def save_entry(self, entry: Entry) -> None:
         try:
             self.get_entry(entry.uuid)
             raise EntryExistsError(f"Entry {entry.uuid} already exists")
         except EntryNotFoundError:
             self.data.append(entry)
+            self._fill_entry_cache()
 
     def get_entry(self, uuid: Union[UUID, str]) -> Entry:
         if isinstance(uuid, str):
             uuid = UUID(uuid)
-        stack = self.data.copy()
-        while len(stack) > 0:
-            entry = stack.pop()
-            if entry.uuid == uuid:
-                return entry
-            if isinstance(entry, Nestable):
-                stack.extend(entry.children)
-        raise EntryNotFoundError(f"Entry {entry.uuid} could not be found")
+
+        try:
+            return self._entry_cache[uuid]
+        except KeyError:
+            raise EntryNotFoundError(f"Entry {uuid} could not be found")
 
     def update_entry(self, entry: Entry) -> None:
         original = self.get_entry(entry.uuid)
@@ -51,3 +67,26 @@ def delete_entry(self, to_delete: Entry) -> None:
                 elif entry.uuid == to_delete.uuid:
                     raise BackendError(f"Can't delete: entry {to_delete.uuid} is out of sync with the version in the backend")
             stack.extend([entry.children for entry in children if isinstance(entry, Nestable)])
+
+        self._fill_entry_cache()
+
+    @property
+    def root(self) -> Root:
+        return self._root
+
+    def search(self, *search_terms: SearchTermType):
+        for entry in self._entry_cache.values():
+            conditions = []
+            for attr, op, target in search_terms:
+                # TODO: search for child pvs?
+                if attr == "entry_type":
+                    conditions.append(isinstance(entry, target))
+                else:
+                    try:
+                        # check entry attribute by name
+                        value = getattr(entry, attr)
+                        conditions.append(self.compare(op, value, target))
+                    except AttributeError:
+                        conditions.append(False)
+            if all(conditions):
+                yield entry

superscore/client.py

@@ -216,23 +216,32 @@ def compare(self, entry_l: Entry, entry_r: Entry) -> EntryDiff:
 
         return EntryDiff(original_entry=entry_l, new_entry=entry_r, diffs=list(diffs))
 
-    def fill(self, entry: Entry) -> None:
+    def fill(self, entry: Union[Entry, UUID], fill_depth: Optional[int] = None) -> None:
         """
         Walk through ``entry`` and replace UUIDs with corresponding Entry's.
-        Currently only has meaning for Nestables.
+        Does nothing if ``entry`` is a non-Nestable or UUID.
+        Filling happens "in-place", modifying ``entry``.
 
         Parameters
         ----------
-        entry : Entry
+        entry : Union[Entry, UUID]
             Entry that may contain UUIDs to be filled with full Entry's
+        fill_depth : Optional[int], by default None
+            The depth to fill.  (value of 1 will fill just ``entry``'s children)
+            If None, fill until there is no filling left
         """
+        if fill_depth is not None:
+            fill_depth -= 1
+            if fill_depth <= 0:
+                return
+
         if isinstance(entry, Nestable):
             new_children = []
             for child in entry.children:
                 if isinstance(child, UUID):
                     search_condition = SearchTerm('uuid', 'eq', child)
                     filled_child = list(self.search(search_condition))[0]
-                    self.fill(filled_child)
+                    self.fill(filled_child, fill_depth)
                     new_children.append(filled_child)
                 else:
                     new_children.append(child)

superscore/tests/conftest.py

@@ -1,6 +1,6 @@
 import shutil
 from pathlib import Path
-from typing import List
+from typing import List, Union
 from unittest.mock import MagicMock
 from uuid import UUID
 
@@ -12,9 +12,10 @@
 from superscore.client import Client
 from superscore.control_layers._base_shim import _BaseShim
 from superscore.control_layers.core import ControlLayer
-from superscore.model import (Collection, Parameter, Readback, Root, Setpoint,
-                              Severity, Snapshot, Status)
+from superscore.model import (Collection, Nestable, Parameter, Readback, Root,
+                              Setpoint, Severity, Snapshot, Status)
 from superscore.tests.ioc import IOCFactory
+from superscore.widgets.views import EntryItem
 
 
 def linac_data():
@@ -911,3 +912,31 @@ def linac_ioc(linac_backend):
     client = Client(backend=linac_backend)
     with IOCFactory.from_entries(snapshot.children, client)(prefix="SCORETEST:") as ioc:
         yield ioc
+
+
+def nest_depth(entry: Union[Nestable, EntryItem]) -> int:
+    """
+    Return the depth of nesting in ``entry``.
+    Works for Entries or EntryItem's (tree items)
+    """
+    depths = []
+    q = []
+    q.append((entry, 0))  # entry and depth
+    while q:
+        e, depth = q.pop()
+        if isinstance(e, Nestable):
+            attr = 'children'
+        elif isinstance(e, EntryItem):
+            attr = '_children'
+        else:
+            depths.append(depth)
+            continue
+
+        children = getattr(e, attr)
+        if not children:
+            depths.append(depth)
+        else:
+            for child in children:
+                q.append((child, depth+1))
+
+    return max(depths)

superscore/tests/test_client.py

@@ -7,13 +7,13 @@
 
 from superscore.backends.core import SearchTerm
 from superscore.backends.filestore import FilestoreBackend
+from superscore.backends.test import TestBackend
 from superscore.client import Client
 from superscore.control_layers import EpicsData
 from superscore.errors import CommunicationError
 from superscore.model import (Collection, Entry, Nestable, Parameter, Readback,
                               Root, Setpoint)
-
-from .conftest import MockTaskStatus
+from superscore.tests.conftest import MockTaskStatus, nest_depth
 
 SAMPLE_CFG = Path(__file__).parent / 'config.cfg'
 
@@ -181,3 +181,28 @@ def test_fill(sample_client: Client, entry_uuid: str):
 
     sample_client.fill(entry)
     assert not uuids_in_entry(entry)
+
+
+@pytest.mark.parametrize("fill_depth,", list(range(1, 11)))
+def test_fill_depth(fill_depth: int):
+    deep_coll = Collection()
+    prev_coll = deep_coll
+    # Be sure more depth exists than the requested depth
+    for i in range(20):
+        child_coll = Collection(title=f"collection {i}")
+        prev_coll.children.append(child_coll)
+        prev_coll = child_coll
+    bknd = TestBackend([deep_coll])
+    client = Client(backend=bknd)
+
+    assert nest_depth(deep_coll) == 20
+    deep_coll.swap_to_uuids()
+    # for this test we want everything to be UUIDS
+    for entry in bknd._entry_cache.values():
+        entry.swap_to_uuids()
+
+    assert nest_depth(deep_coll) == 1
+
+    client.fill(deep_coll, fill_depth)
+
+    assert nest_depth(deep_coll) == fill_depth