+ Fixed _result_cache in QuerySet

+ Fixed __getitem()__ to have coherent response on slice and to use _result_cache if possible
+ Reimplemented CacheIterator __iter__() to support multiple owners in single thread

Author: desiena

fmdata/cache_iterator.py

@@ -1,4 +1,3 @@
-import itertools
 from typing import Iterator, List, TypeVar, Optional
 
 from typing_extensions import Generic
@@ -15,14 +14,25 @@ def __init__(self, iterator: Iterator[T]) -> None:
         self.cache_complete: bool = False
 
     def __iter__(self) -> Iterator[T]:
-        if self.cache_complete:
-            # all values have been cached
-            return iter(self.cached_values)
-
-        return itertools.chain(self.cached_values, self._iter)
+        idx = 0
+        # keep pulling from cache first, then from the generator
+        while True:
+            if idx < len(self.cached_values):
+                yield self.cached_values[idx]
+                idx += 1
+            elif not self.cache_complete:
+                val = next(self._iter, None)
+
+                if val is None:
+                    return
+
+                yield val
+                idx += 1
+            else:
+                return
 
     def __len__(self):
-        return len(self.list)
+        return sum(1 for _ in self.__iter__())
 
     def __getitem__(self, k) -> T:
         def read_until(index: Optional[int]):
@@ -52,7 +62,6 @@ def __repr__(self) -> str:
             len(self.cached_values), self.cache_complete
         )
 
-    @property
     def empty(self):
         # If cache is not empty there is for sure at least one element
         if not len(self.cached_values) == 0:
@@ -68,13 +77,6 @@ def empty(self):
             # If cached values changes, there is at least one element so is not empty
             return len(self.cached_values) == 0
 
-    @property
-    def list(self):
-        while not self.cache_complete:
-            next(self._iter, None)
-
-        return self.cached_values
-
     def _cache_generator(self, iterator: Iterator) -> Iterator:
         for val in iterator:
             self.cached_values.append(val)

fmdata/orm.py

@@ -1,6 +1,7 @@
 from __future__ import annotations
 
 import dataclasses
+import itertools
 from datetime import date, datetime
 from functools import cached_property
 from typing import Type, Optional, List, Any, Iterator, Iterable, Set, Dict, Union, Tuple
@@ -145,9 +146,9 @@ def __init__(self):
         self._chunk_size = None
         self._slice_start: int = 0
         self._slice_stop: Optional[int] = None
-        self._result_cache: Optional[CacheIterator[PortalModel]] = None
         self._avoid_prefetch_cache = False
         self._only_prefetched = False
+        self._result_cache: Optional[CacheIterator[PortalModel]] = None
 
     def _set_model(self, model: Model, meta_portal: ModelMetaPortalField):
         self._model = model
@@ -160,7 +161,6 @@ def _clone(self):
         qs._chunk_size = self._chunk_size
         qs._slice_start = self._slice_start
         qs._slice_stop = self._slice_stop
-        qs._result_cache = self._result_cache
         qs._avoid_prefetch_cache = self._avoid_prefetch_cache
         qs._only_prefetched = self._only_prefetched
 
@@ -180,14 +180,14 @@ def _fetch_all(self):
             return
 
         # Try to use cache if the request is inside the prefetch data slice
-        if prefetch_data is not None:
+        if not self._avoid_prefetch_cache and prefetch_data is not None:
             prefetch_data_slice_start = prefetch_data.offset - 1
             prefetch_data_slice_stop = prefetch_data_slice_start + prefetch_data.limit
 
             search_slice_is_inside_prefetch_slice = self._slice_stop is not None and (
                     self._slice_start >= prefetch_data_slice_start and self._slice_stop <= prefetch_data_slice_stop)
 
-            if not self._avoid_prefetch_cache and search_slice_is_inside_prefetch_slice:
+            if search_slice_is_inside_prefetch_slice:
                 slice_relative_start = self._slice_start - prefetch_data_slice_start
                 slice_relative_stop = self._slice_stop - prefetch_data_slice_start
                 self._result_cache = prefetch_data.cache[slice_relative_start:slice_relative_stop]
@@ -239,24 +239,27 @@ def __getitem__(self, k):
             if k.stop is not None and k.stop <= (k.start or 0):
                 raise ValueError("Stop index must be greater than start index.")
 
-            if self._result_cache is not None:
-                return self._result_cache[k]
-
             new_qs = self._clone()
             new_qs._set_new_slice(k.start, k.stop)
 
+            if self._result_cache is not None:
+                new_qs._result_cache = CacheIterator(itertools.islice(self._result_cache.__iter__(), k.start, k.stop))
+
             # In case step is present, the list() force the execution of the query then use the list step to provide the result
             return list(new_qs)[::k.step] if k.step else new_qs
 
         elif isinstance(k, int):
             if k < 0:
                 raise ValueError("Negative indexing is not supported.")
 
+            if self._result_cache is not None:
+                return self._result_cache[k]
+
             new_qs = self._clone()
             new_qs._set_new_slice(k, k + 1)
             new_qs._fetch_all()
 
-            return self._result_cache[0]
+            return new_qs._result_cache[0]
 
         else:
             raise TypeError(
@@ -344,8 +347,6 @@ def _execute_query(self):
             page_iterator=paged_result
         ))
 
-        return self._result_cache
-
     def portals_record_from_portal_page_iterator(self,
                                                  model: Model,
                                                  portal_fm_name: str,
@@ -368,7 +369,7 @@ def portals_record_from_portal_page_iterator(self,
 
 
 class PortalModel(metaclass=PortalMetaclass):
-    #Example of Meta
+    # Example of Meta
     #
     # class Meta:
     #     base_schema: FileMakerSchema = None
@@ -651,11 +652,10 @@ def _clone(self):
         qs._sort = self._sort[:]
         qs._scripts = self._scripts.copy()
         qs._chunk_size = self._chunk_size
-        qs._portals = self._portals
+        qs._portals = self._portals.copy()
         qs._slice_start = self._slice_start
         qs._slice_stop = self._slice_stop
         qs._response_layout = self._response_layout
-        qs._result_cache = self._result_cache
 
         return qs
 
@@ -831,24 +831,27 @@ def __getitem__(self, k):
             if k.stop is not None and k.stop <= (k.start or 0):
                 raise ValueError("Stop index must be greater than start index.")
 
-            if self._result_cache is not None:
-                return self._result_cache[k]
-
             new_qs = self._clone()
             new_qs._set_new_slice(k.start, k.stop)
 
+            if self._result_cache is not None:
+                new_qs._result_cache = CacheIterator(itertools.islice(self._result_cache.__iter__(), k.start, k.stop))
+
             # In case step is present, the list() force the execution of the query then use the list step to provide the result
             return list(new_qs)[::k.step] if k.step else new_qs
 
         elif isinstance(k, int):
             if k < 0:
                 raise ValueError("Negative indexing is not supported.")
 
+            if self._result_cache is not None:
+                return self._result_cache[k]
+
             new_qs = self._clone()
             new_qs._set_new_slice(k, k + 1)
             new_qs._fetch_all()
 
-            return self._result_cache[0]
+            return new_qs._result_cache[0]
 
         else:
             raise TypeError(
@@ -944,7 +947,6 @@ def _execute_query(self):
         self._result_cache = CacheIterator(
             self.records_iterator_from_page_iterator(page_iterator=paged_result.pages.__iter__(),
                                                      portals_input=self._portals))
-        return self._result_cache
 
     def records_iterator_from_page_iterator(self,
                                             page_iterator: PageIterator,
@@ -1150,7 +1152,8 @@ def __new__(mcls, name, bases, attrs):
         client: FMClient = get_meta_attribute(cls=cls, attrs_meta=attrs_meta, attribute_name="client")
         layout: str = get_meta_attribute(cls=cls, attrs_meta=attrs_meta, attribute_name="layout")
 
-        base_manager: Type[ModelManager] = get_meta_attribute(cls=cls, attrs_meta=attrs_meta, attribute_name="base_manager") or ModelManager
+        base_manager: Type[ModelManager] = get_meta_attribute(cls=cls, attrs_meta=attrs_meta,
+                                                              attribute_name="base_manager") or ModelManager
 
         cls._meta = ModelMeta(
             client=client,
@@ -1183,7 +1186,7 @@ class Model(metaclass=ModelMetaclass):
     #     base_schema: FileMakerSchema = None
     #     schema_config: dict = None
 
-    #TODO not used. Only for type hint
+    # TODO not used. Only for type hint
     objects: ModelManager = ModelManager()
 
     def __init__(self, **kwargs):

tests/integration/test_server.py

@@ -247,7 +247,7 @@ def test_create_get_delete_record(self) -> None:
 
         found_set = result.found_set
 
-        if not found_set.empty:
+        if not found_set.empty():
             found_set[0].delete_record()
 
         result = student_layout.create_record(
@@ -267,4 +267,4 @@ def test_create_get_delete_record(self) -> None:
         result.raise_exception_if_has_message(exclude_codes=[FMErrorEnum.NO_RECORDS_MATCH_REQUEST])
 
         found_set = result.found_set
-        self.assertTrue(found_set.empty)
+        self.assertTrue(found_set.empty())

tests/test_cache_iterator.py

@@ -138,14 +138,14 @@ def test_empty_property_with_data(self):
         data = [1, 2, 3]
         cache_iter = CacheIterator(iter(data))
 
-        self.assertFalse(cache_iter.empty)
+        self.assertFalse(cache_iter.empty())
         self.assertEqual(cache_iter.cached_values, [1])  # Should consume first element
 
     def test_empty_property_empty_iterator(self):
         """Test empty property with empty iterator."""
         cache_iter = CacheIterator(iter([]))
 
-        self.assertTrue(cache_iter.empty)
+        self.assertTrue(cache_iter.empty())
         self.assertEqual(cache_iter.cached_values, [])
         self.assertTrue(cache_iter.cache_complete)
 
@@ -157,25 +157,10 @@ def test_empty_property_after_partial_consumption(self):
         # Consume some elements
         next(iter(cache_iter))
 
-        self.assertFalse(cache_iter.empty)
+        self.assertFalse(cache_iter.empty())
         # After consuming one element and checking empty, only 1 element should be cached
         self.assertEqual(len(cache_iter.cached_values), 1)
 
-    def test_list_property(self):
-        """Test list property."""
-        data = [1, 2, 3, 4, 5]
-        cache_iter = CacheIterator(iter(data))
-
-        # list property should force full consumption
-        result = cache_iter.list
-        self.assertEqual(result, data)
-        self.assertEqual(cache_iter.cached_values, data)
-        self.assertTrue(cache_iter.cache_complete)
-
-        # Subsequent calls should return the same cached list
-        result2 = cache_iter.list
-        self.assertEqual(result2, data)
-        self.assertIs(result, result2)  # Should be the same object
 
     def test_repr(self):
         """Test __repr__ method."""
@@ -204,17 +189,15 @@ def test_empty_iterator(self):
         self.assertEqual(len(cache_iter), 0)
         self.assertTrue(cache_iter.empty)
         self.assertTrue(cache_iter.cache_complete)
-        self.assertEqual(cache_iter.list, [])
 
     def test_single_element_iterator(self):
         """Test CacheIterator with single element."""
         cache_iter = CacheIterator(iter([42]))
 
         self.assertEqual(list(cache_iter), [42])
         self.assertEqual(len(cache_iter), 1)
-        self.assertFalse(cache_iter.empty)
+        self.assertFalse(cache_iter.empty())
         self.assertEqual(cache_iter[0], 42)
-        self.assertEqual(cache_iter.list, [42])
 
     def test_string_iterator(self):
         """Test CacheIterator with string iterator."""
@@ -279,7 +262,7 @@ def test_mixed_operations(self):
 
         # Mix of different operations
         self.assertEqual(cache_iter[3], 3)
-        self.assertFalse(cache_iter.empty)
+        self.assertFalse(cache_iter.empty())
 
         partial_list = cache_iter[1:5]
         self.assertEqual(partial_list, [1, 2, 3, 4])