Repository: jina-ai/serve
Pull request: feat: add listview

Problem statement:
need to create a `ListValueView` similar to `StructView` for nested array objects
**Describe the bug**

Nested `array` objects in `tags` or `struct-like` protobuf fields cannot be updated:

```python

>>> from jina import Document
>>> d = Document()
>>> d.tags = {'hey': {'nested': True}, 'hoy': [0, 1]}
>>> d.tags
<jina.types.struct.StructView hoy=[0.0, 1.0] hey={'nested': True} at 140610499183952>
>>> d.tags['hey']['nested'] = False
>>> d.tags
<jina.types.struct.StructView hoy=[0.0, 1.0] hey={'nested': False} at 140610499183824>
>>> d.tags['hey']['hoy'][0] = 1
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/home/joan/jina/jina/jina/types/struct.py", line 36, in __getitem__
    raise KeyError()
```

**Proposal**
This can be fixed adding a new `View` object as for `ListValue`

Hints from issue discussion:

Commit message: Merge branch 'master' of https://github.com/jina-ai/jina into feat-listview

Patch to review:
diff --git a/jina/helper.py b/jina/helper.py
index 59e9cefd1209a..59df2f8d6cb46 100644
--- a/jina/helper.py
+++ b/jina/helper.py
@@ -1158,11 +1158,14 @@ def dunder_get(_dict: Any, key: str) -> Any:
     except ValueError:
         pass
 
+    from google.protobuf.struct_pb2 import ListValue
     from google.protobuf.struct_pb2 import Struct
     from google.protobuf.pyext._message import MessageMapContainer
 
     if isinstance(part1, int):
         result = _dict[part1]
+    elif isinstance(_dict, (Iterable, ListValue)):
+        result = _dict[part1]
     elif isinstance(_dict, (dict, Struct, MessageMapContainer)):
         if part1 in _dict:
             result = _dict[part1]
diff --git a/jina/types/list.py b/jina/types/list.py
new file mode 100644
index 0000000000000..b44721d85e8b1
--- /dev/null
+++ b/jina/types/list.py
@@ -0,0 +1,64 @@
+from collections.abc import MutableSequence
+from typing import Union
+
+from google.protobuf import struct_pb2
+
+from .mixin import ProtoTypeMixin
+
+
+class ListView(ProtoTypeMixin, MutableSequence):
+    """Create a Python mutable sequence view of Protobuf ListValue struct.
+
+    This can be used in all Jina types where a protobuf.ListValue is returned.
+
+    Used inside `StructView` when the inner element is a `ListValue`
+    """
+
+    def __init__(self, list: struct_pb2.ListValue):
+        """Create a Python list view of Protobuf ListValue.
+
+        :param list: the protobuf ListValue object
+        """
+        self._pb_body = list
+
+    def insert(self, index: int, object) -> None:
+        """
+        Insert any proto object into the list
+
+        :param index: Position of the insertion.
+        :param object: The object that needs to be inserted.
+        """
+        self._pb_body.insert(index, object)
+
+    def __getitem__(self, i: Union[int, slice]):
+        if i < len(self):
+            value = self._pb_body[i]
+            if isinstance(value, struct_pb2.Struct):
+                from .struct import StructView
+
+                return StructView(value)
+            elif isinstance(value, struct_pb2.ListValue):
+                return ListView(value)
+            else:
+                return value
+
+    def __setitem__(self, i, value):
+        self._pb_body[i] = value
+
+    def __delitem__(self, i: Union[int, slice]) -> None:
+        del self._pb_body[i]
+
+    def __len__(self) -> int:
+        return len(self._pb_body)
+
+    def __iter__(self):
+        for i in range(len(self)):
+            yield self[i]
+
+    def clear(self):
+        """
+        # noqa: DAR101
+        # noqa: DAR102
+        # noqa: DAR103
+        """
+        self._pb_body.Clear()
diff --git a/jina/types/struct.py b/jina/types/struct.py
index 218fb489f6826..8f8973d3ab5b9 100644
--- a/jina/types/struct.py
+++ b/jina/types/struct.py
@@ -27,9 +27,12 @@ def __delitem__(self, key):
     def __getitem__(self, key):
         if key in self._pb_body.keys():
             value = self._pb_body[key]
-            # TODO: Maybe do the same with ListValue and build a ListValueView
             if isinstance(value, struct_pb2.Struct):
                 return StructView(value)
+            elif isinstance(value, struct_pb2.ListValue):
+                from .list import ListView
+
+                return ListView(value)
             else:
                 return value
         else:
diff --git a/tests/unit/test_helper.py b/tests/unit/test_helper.py
index cb1b684b54784..5f201b788d675 100644
--- a/tests/unit/test_helper.py
+++ b/tests/unit/test_helper.py
@@ -73,8 +73,11 @@ def test_time_context():
 
 def test_dunder_get():
     a = SimpleNamespace()
-    a.b = {'c': 1}
+    a.b = {'c': 1, 'd': {'e': 'f', 'g': [0, 1, {'h': 'i'}]}}
     assert dunder_get(a, 'b__c') == 1
+    assert dunder_get(a, 'b__d__e') == 'f'
+    assert dunder_get(a, 'b__d__g__0') == 0
+    assert dunder_get(a, 'b__d__g__2__h') == 'i'
 
 
 def test_check_update():
diff --git a/tests/unit/types/document/test_document.py b/tests/unit/types/document/test_document.py
index 0be2a11b0cb2a..ea3348e4210c6 100644
--- a/tests/unit/types/document/test_document.py
+++ b/tests/unit/types/document/test_document.py
@@ -542,7 +542,7 @@ def test_get_attr_values():
             'text': 'document',
             'feature1': 121,
             'name': 'name',
-            'tags': {'id': 'identity', 'a': 'b', 'c': 'd'},
+            'tags': {'id': 'identity', 'a': 'b', 'c': 'd', 'e': [0, 1, {'f': 'g'}]},
         }
     )
     d.scores['metric'] = NamedScore(value=42)
@@ -556,6 +556,7 @@ def test_get_attr_values():
         'tags__c',
         'tags__id',
         'tags__inexistant',
+        'tags__e__2__f',
         'inexistant',
     ]
     res = d.get_attributes(*required_keys)
@@ -569,12 +570,13 @@ def test_get_attr_values():
     assert res[required_keys.index('scores__values__metric__value')] == 42
     assert res[required_keys.index('tags__inexistant')] is None
     assert res[required_keys.index('inexistant')] is None
+    assert res[required_keys.index('tags__e__2__f')] == 'g'
 
     required_keys_2 = ['tags', 'text']
     res2 = d.get_attributes(*required_keys_2)
     assert len(res2) == 2
     assert res2[required_keys_2.index('text')] == 'document'
-    assert res2[required_keys_2.index('tags')] == d.tags
+    assert res2[required_keys_2.index('tags')].dict() == d.tags.dict()
 
     d = Document({'id': '123', 'tags': {'outterkey': {'innerkey': 'real_value'}}})
     required_keys_3 = ['tags__outterkey__innerkey']
@@ -1043,6 +1045,33 @@ def test_tag_compare_dict():
     assert d.tags.dict() == {'hey': {'bye': 4}}
 
     d.tags = {'hey': [1, 2]}
-    # TODO: Issue about having proper ListValueView
     assert d.tags != {'hey': [1, 2]}
     assert d.tags.dict() == {'hey': [1, 2]}
+
+
+def test_tags_update_nested_lists():
+    from jina import Document
+    from jina.types.list import ListView
+    from jina.types.struct import StructView
+
+    d = Document()
+    d.tags = {
+        'hey': {'nested': True, 'list': ['elem1', 'elem2', {'inlist': 'here'}]},
+        'hoy': [0, 1],
+    }
+    assert d.tags.dict() == {
+        'hey': {'nested': True, 'list': ['elem1', 'elem2', {'inlist': 'here'}]},
+        'hoy': [0, 1],
+    }
+    assert isinstance(d.tags['hoy'], ListView)
+    assert isinstance(d.tags['hey']['list'], ListView)
+    assert isinstance(d.tags['hey']['list'][2], StructView)
+    d.tags['hey']['nested'] = False
+    d.tags['hey']['list'][1] = True
+    d.tags['hey']['list'][2]['inlist'] = 'not here'
+    d.tags['hoy'][0] = 1
+
+    assert d.tags['hey']['nested'] is False
+    assert d.tags['hey']['list'][1] is True
+    assert d.tags['hey']['list'][2]['inlist'] == 'not here'
+    assert d.tags['hoy'][0] == 1
diff --git a/tests/unit/types/document/test_graph_document.py b/tests/unit/types/document/test_graph_document.py
index 3c538ac2a6ac9..92b580bfba33f 100644
--- a/tests/unit/types/document/test_graph_document.py
+++ b/tests/unit/types/document/test_graph_document.py
@@ -315,3 +315,25 @@ def test_manual_update_edges_features(graph, expected_output):
     graph._pb_body.graph.edge_features[edge_key] = {'number_value': 1234}
 
     assert graph._pb_body.graph.edge_features[edge_key]['number_value'] == 1234
+
+
+def test_edge_update_nested_lists():
+    graph = GraphDocument(force_undirected=True)
+    doc0 = Document(text='Document0')
+    doc1 = Document(text='Document1')
+
+    graph.add_edge(doc0, doc1)
+    edge_key = graph._get_edge_key(doc0, doc1)
+    graph.edge_features[edge_key] = {
+        'hey': {'nested': True, 'list': ['elem1', 'elem2', {'inlist': 'here'}]},
+        'hoy': [0, 1],
+    }
+    graph.edge_features[edge_key]['hey']['nested'] = False
+    graph.edge_features[edge_key]['hey']['list'][1] = True
+    graph.edge_features[edge_key]['hey']['list'][2]['inlist'] = 'not here'
+    graph.edge_features[edge_key]['hoy'][0] = 1
+
+    assert graph.edge_features[edge_key]['hey']['nested'] is False
+    assert graph.edge_features[edge_key]['hey']['list'][1] is True
+    assert graph.edge_features[edge_key]['hey']['list'][2]['inlist'] == 'not here'
+    assert graph.edge_features[edge_key]['hoy'][0] == 1
diff --git a/tests/unit/types/test_list_view.py b/tests/unit/types/test_list_view.py
new file mode 100644
index 0000000000000..6ac7619efad5f
--- /dev/null
+++ b/tests/unit/types/test_list_view.py
@@ -0,0 +1,130 @@
+import pytest
+
+from google.protobuf.struct_pb2 import ListValue
+
+from jina.types.list import ListView
+
+
+def test_empty_struct_view():
+    list = ListValue()
+    view = ListView(list)
+    assert len(view) == 0
+
+
+@pytest.fixture()
+def list_proto():
+    list = ListValue()
+    l = [0, 1, 'hey', {'key': 'value'}, [0, 1, 2]]
+    list.extend(l)
+    return list
+
+
+def test_list_view(list_proto):
+    view = ListView(list_proto)
+    assert len(view) == 5
+    assert view[-4] == 1
+    assert view[0] == 0
+    assert view[1] == 1
+    assert view[2] == 'hey'
+    assert view[3] == {'key': 'value'}
+    assert len(view[4]) == 3
+    assert view[4][0] == 0
+    assert view[4][1] == 1
+    assert view[4][2] == 2
+
+
+def test_list_view_set_items(list_proto):
+    view = ListView(list_proto)
+    view[0] = 20
+    view[1] = 'now string'
+    view[2] = {'key': 'value'}
+    view[3]['key'] = 50
+    view[4][0] = {'new_key': 'new_value'}
+    assert view[0] == 20
+    assert view[1] == 'now string'
+    assert view[2] == {'key': 'value'}
+    assert view[3] == {'key': 50}
+    assert view[4][0] == {'new_key': 'new_value'}
+    assert view[4][1] == 1
+    assert view[4][2] == 2
+    view[4][0]['new_key'] = None
+    assert view[4][0] == {'new_key': None}
+
+
+def test_list_view_delete(list_proto):
+    view = ListView(list_proto)
+    del view[1]
+    assert len(view) == 4
+    assert view[0] == 0
+    assert view[1] == 'hey'
+    assert view[2] == {'key': 'value'}
+    assert len(view[3]) == 3
+    assert view[3][0] == 0
+    assert view[3][1] == 1
+    assert view[3][2] == 2
+
+
+def test_list_view_clear(list_proto):
+    view = ListView(list_proto)
+    assert len(view) == 5
+    view.clear()
+    assert len(view) == 0
+
+
+def test_list_view_iterate(list_proto):
+    view = ListView(list_proto)
+    for i, element in enumerate(view):
+        assert view[0] == 0
+        assert view[1] == 1
+        assert view[2] == 'hey'
+        assert view[3] == {'key': 'value'}
+        assert len(view[4]) == 3
+        assert view[4][0] == 0
+        assert view[4][1] == 1
+        assert view[4][2] == 2
+        if i == 0:
+            assert element == 0
+        elif i == 1:
+            assert element == 1
+        elif i == 2:
+            assert element == 'hey'
+        elif i == 3:
+            assert element == {'key': 'value'}
+        elif i == 4:
+            assert len(element) == 3
+            for j, e in enumerate(element):
+                if j == 0:
+                    assert e == 0
+                elif j == 1:
+                    assert e == 1
+                elif j == 2:
+                    assert e == 2
+
+
+def test_list_view_dict():
+    list = ListValue()
+    l = [0, 1, 'hey', {'key': 'value'}, [0, 1, 2]]
+    list.extend(l)
+    view = ListView(list)
+    assert view.dict() == l
+
+
+def test_list_contains(list_proto):
+    view = ListView(list_proto)
+    assert 'hey' in view
+    assert 'heya' not in view
+    assert {'key': 'value'} in view
+    assert {'key': 'value-aaa'} not in view
+    # not hashable
+    assert [0, 1, 2] not in view
+
+
+def test_list_reverse():
+    list = ListValue()
+    l = [0, 2, 1]
+    list.extend(l)
+    view = ListView(list)
+    view.reverse()
+    assert view[0] == 1
+    assert view[1] == 2
+    assert view[2] == 0
diff --git a/tests/unit/types/test_struct_view.py b/tests/unit/types/test_struct_view.py
index ac807f7975bd2..ca1b4e498c15c 100644
--- a/tests/unit/types/test_struct_view.py
+++ b/tests/unit/types/test_struct_view.py
@@ -59,6 +59,12 @@ def test_struct_view(struct_proto):
 
 
 def test_struct_view_set_items(struct_proto):
+    view = StructView(struct_proto)
+    view['key_array'][0] = 2
+    assert view['key_array'][0] == 2
+
+
+def test_struct_view_set_listitems(struct_proto):
     view = StructView(struct_proto)
     view['key_int'] = 20
     view['key_new'] = 30
@@ -136,7 +142,7 @@ def test_struct_view_update(struct_proto):
 
 def test_struct_view_dict():
     struct = Struct()
-    d = {'a': 1, 'b': 2}
+    d = {'a': 1, 'b': 2, 'c': {'e': None, 'l': [40, 30]}, 'f': [5, 1]}
     struct.update(d)
     view = StructView(struct)
     assert view.dict() == d
@@ -145,4 +151,4 @@ def test_struct_view_dict():
 def test_struct_contains(struct_proto):
     view = StructView(struct_proto)
     assert 'key_nested' in view
-    assert not 'a' in view
+    assert 'a' not in view


Code review comments:
diff_hunk: @@ -0,0 +1,64 @@
+from collections.abc import MutableSequence
+from typing import Union
+
+from google.protobuf import struct_pb2
+
+from .mixin import ProtoTypeMixin
+
+
+class ListView(ProtoTypeMixin, MutableSequence):
+    """Create a Python mutable sequence view of Protobuf ListValue struct.
+
+    This can be used in all Jina types where a protobuf.ListValue is returned.
+
+    Used inside `StructView` when the inner element is a `ListValue`
+    """
+
+    def __init__(self, list: struct_pb2.ListValue):
+        """Create a Python list view of Protobuf ListValue.
+
+        :param list: the protobuf ListValue object
+        """
+        self._pb_body = list
+
+    def insert(self, index: int, object) -> None:
+        """
+        Insert any proto object into the list
+
+        :param index: Position of the insertion.
+        :param object: The object that needs to be inserted.
+        """
+        self._pb_body.insert(index, object)
+
+    def __getitem__(self, i: Union[int, slice]):
+        if i < len(self):
path: jina/types/list.py
text: @user1:
Raise an error when the index is out of range.
```suggestion
        if i >= len(self):
            raise IndexError('list index out of range')
        ...
```

@author:
Done!