Merge pull request #166 from poissoncorp/RDBC-614

RDBC-614 Refactor GetCompareExchangeValuesOperation

Author: ml054

ravendb/documents/operations/compare_exchange/compare_exchange.py

@@ -89,8 +89,10 @@ def get_value(
                         raise RavenException(
                             f"Unable to read compare exchange value: {self.__original_value.value}", ex
                         )
-                else:
+                elif isinstance(self.__original_value.value, dict):
                     entity = Utils.convert_json_dict_to_object(self.__original_value.value, object_type)
+                else:
+                    entity = self.__original_value.value
 
                 value = CompareExchangeValue(self._key, self._index, entity)
                 self.__value = value

ravendb/documents/operations/compare_exchange/operations.py

@@ -190,31 +190,30 @@ def get_raft_unique_request_id(self) -> str:
 class GetCompareExchangeValuesOperation(IOperation[Dict[str, CompareExchangeValue[_T]]], Generic[_T]):
     def __init__(
         self,
-        keys_or_start_with: Union[Collection[str], StartingWithOptions],
-        object_type: Optional[Type[_T]],
+        keys: Optional[Collection[str]] = None,
+        object_type: Optional[Type[_T]] = None,
         materialize_metadata: Optional[bool] = True,
-    ):  # todo: starting with
-        start_with = isinstance(keys_or_start_with, StartingWithOptions)
-        if not materialize_metadata and start_with:
-            raise ValueError(
-                f"Cannot set materialize_metadata to False while "
-                f"collecting cmpxchg values starting with '{keys_or_start_with.starts_with}'"
-            )
-
+    ):
+        self._materialize_metadata = materialize_metadata
+        self._keys = keys
         self._object_type = object_type
+        self._start = None
+        self._page_size = None
+        self._start_with = None
 
-        if start_with:
-            self._keys = None
-            self._materialize_metadata = True  # todo: documentation - tell the user that it'll be set to True anyway
-            self._start = keys_or_start_with.start
-            self._page_size = keys_or_start_with.page_size
-            self._start_with = keys_or_start_with.starts_with
-        else:
-            self._materialize_metadata = materialize_metadata
-            self._keys = keys_or_start_with
-            self._start = None
-            self._page_size = None
-            self._start_with = None
+    @classmethod
+    def create_for_start_with(
+        cls,
+        start_with: str,
+        start: Optional[int] = None,
+        page_size: Optional[int] = None,
+        object_type: Optional[Type[_T]] = None,
+    ) -> GetCompareExchangeValuesOperation:
+        operation = cls(None, object_type, True)
+        operation._starts_with = start_with
+        operation._start = start
+        operation._page_size = page_size
+        return operation
 
     def get_command(self, store: DocumentStore, conventions: DocumentConventions, cache: HttpCache) -> RavenCommand[_T]:
         return self.GetCompareExchangeValuesCommand(self, self._materialize_metadata, conventions)

ravendb/documents/session/cluster_transaction_operation.py

@@ -144,31 +144,9 @@ def _get_compare_exchange_value_internal(
         return None
 
     def _get_compare_exchange_values_internal(
-        self, keys_or_start_with_options: Union[List[str], StartingWithOptions], object_type: Optional[Type[_T]]
+        self, keys: List[str], object_type: Optional[Type[_T]]
     ) -> Dict[str, Optional[CompareExchangeValue[_T]]]:
-        start_with = isinstance(keys_or_start_with_options, StartingWithOptions)
-        if start_with:
-            self._session.increment_requests_count()
-            values = self._session.operations.send(
-                GetCompareExchangeValuesOperation(keys_or_start_with_options, dict), self.session.session_info
-            )
-
-            results = {}
-
-            for key, value in values.items():
-                if value is None:
-                    self.register_missing_compare_exchange_value(key)
-                    results[key] = None
-                    continue
-
-                session_value = self.register_compare_exchange_value(value)
-                results[key] = session_value.get_value(object_type, self.session.conventions)
-
-            return results
-
-        results, not_tracked_keys = self.get_compare_exchange_values_from_session_internal(
-            keys_or_start_with_options, object_type
-        )
+        results, not_tracked_keys = self.get_compare_exchange_values_from_session_internal(keys, object_type)
 
         if not not_tracked_keys:
             return results
@@ -191,6 +169,32 @@ def _get_compare_exchange_values_internal(
 
         return results
 
+    def _get_compare_exchange_values_starting_with_internal(
+        self,
+        start_with: str,
+        start: Optional[int] = None,
+        page_size: Optional[int] = None,
+        object_type: Optional[Type[_T]] = None,
+    ) -> Dict[str, Optional[CompareExchangeValue[_T]]]:
+        self._session.increment_requests_count()
+        values = self._session.operations.send(
+            GetCompareExchangeValuesOperation.create_for_start_with(start_with, start, page_size, object_type),
+            self.session.session_info,
+        )
+
+        results = {}
+
+        for key, value in values.items():
+            if value is None:
+                self.register_missing_compare_exchange_value(key)
+                results[key] = None
+                continue
+
+            session_value = self.register_compare_exchange_value(value)
+            results[key] = session_value.get_value(object_type, self.session.conventions)
+
+        return results
+
     def get_compare_exchange_value_from_session_internal(
         self, key: str, object_type: Optional[Type[_T]] = None
     ) -> Tuple[Union[None, CompareExchangeValue[_T]], bool]:
@@ -299,9 +303,7 @@ def get_compare_exchange_values_starting_with(
         page_size: Optional[int] = None,
         object_type: Optional[Type[_T]] = None,
     ):
-        return self._get_compare_exchange_values_internal(
-            StartingWithOptions(starts_with, start, page_size), object_type
-        )
+        return self._get_compare_exchange_values_starting_with_internal(starts_with, start, page_size, object_type)
 
 
 # this class helps to expose better typehints without tons of methods and fields from ClusterTransactionOperationsBase

ravendb/tests/jvm_migrated_tests/issues_tests/test_ravenDB_14006.py

@@ -32,8 +32,8 @@ def test_compare_exchange_value_tracking_in_session_starts_with(self):
             session.save_changes()
 
         with self.store.open_session(session_options=session_options) as session:
-            results = session.advanced.cluster_transaction.get_compare_exchange_values(
-                StartingWithOptions("comp"), Company
+            results = session.advanced.cluster_transaction.get_compare_exchange_values_starting_with(
+                "comp", object_type=Company
             )
             self.assertEqual(10, len(results))
             self.assertTrue(all(map(lambda x: x is not None, results)))

ravendb/tests/jvm_migrated_tests/test_unique_values.py

@@ -12,7 +12,6 @@
 from ravendb.documents.operations.statistics import GetDetailedStatisticsOperation, DetailedDatabaseStatistics
 from ravendb.documents.session.misc import TransactionMode
 from ravendb.tests.test_base import TestBase, User
-from ravendb.util.util import StartingWithOptions
 
 
 class TestUniqueValues(TestBase):
@@ -101,7 +100,7 @@ def test_can_list_compare_exchange(self):
         self.assertTrue(res2.successful)
 
         values: Dict[str, CompareExchangeValue[User]] = self.store.operations.send(
-            GetCompareExchangeValuesOperation(StartingWithOptions("test"), User)
+            GetCompareExchangeValuesOperation.create_for_start_with("test", object_type=User)
         )
         self.assertEqual(2, len(values))