Address review: topology JSON on domain classes, cache read tests, fix ts_names_for

- Move topology serialization onto the domain classes: add Topology.to_json/from_json
  and ServerNode.to_json/from_json; the topology cache now uses them instead of local helpers.
- Add end-to-end read-path coverage for the topology cache (previously only creation/write
  was tested): a serialization round-trip test and a seed-from-cache test that boots a store
  against an unreachable url and asserts the topology is loaded from disk. The seed path now
  also restores the cached topology etag.
- test_time_series_names_for: the server is fine - an empty timeSeriesNamesFor array indexes
  as a blank term on Corax but not on Lucene. Unskip and force the static index engine to
  Lucene (matching the other Corax-limited tests) instead of skipping.

Author: poissoncorp

ravendb/http/request_executor.py

@@ -470,6 +470,7 @@ def __run(errors: list):
                 cached_topology = self._try_load_topology_from_cache(url)
                 if cached_topology is not None:
                     self._node_selector = NodeSelector(cached_topology, self._thread_pool_executor)
+                    self._topology_etag = cached_topology.etag
                     self.__initialize_update_topology_timer()
                     self.__topology_taken_from_node = ServerNode(url, self._database_name)
                     return

ravendb/http/server_node.py

@@ -47,6 +47,24 @@ def __hash__(self) -> int:
     def last_server_version(self) -> str:
         return self.__last_server_version
 
+    def to_json(self) -> dict:
+        return {
+            "Url": self.url,
+            "Database": self.database,
+            "ClusterTag": self.cluster_tag,
+            "ServerRole": self.server_role.value if self.server_role is not None else None,
+        }
+
+    @classmethod
+    def from_json(cls, json_dict: dict) -> "ServerNode":
+        role = json_dict.get("ServerRole")
+        return cls(
+            json_dict.get("Url"),
+            json_dict.get("Database"),
+            json_dict.get("ClusterTag"),
+            cls.Role(role) if role else None,
+        )
+
     @classmethod
     def create_from(cls, topology: "ClusterTopology"):
         nodes = []

ravendb/http/topology.py

@@ -25,6 +25,16 @@ def __init__(self, etag: int, nodes: List[ServerNode]):
         self.etag = etag
         self.nodes = nodes
 
+    def to_json(self) -> Dict:
+        return {"Etag": self.etag, "Nodes": [node.to_json() for node in (self.nodes or [])]}
+
+    @classmethod
+    def from_json(cls, json_dict: Dict) -> "Topology":
+        return cls(
+            json_dict.get("Etag"),
+            [ServerNode.from_json(node_json) for node_json in (json_dict.get("Nodes") or [])],
+        )
+
 
 class ClusterTopology:
     def __init__(self):

ravendb/http/topology_local_cache.py

@@ -5,7 +5,6 @@
 import os
 from typing import Optional
 
-from ravendb.http.server_node import ServerNode
 from ravendb.http.topology import Topology
 
 # On-disk topology cache (mirrors the .NET client's TopologyLocalCache). The feature is
@@ -24,44 +23,14 @@ def _path(location: str, topology_hash: str, extension: str) -> str:
     return os.path.join(location, topology_hash + extension)
 
 
-def _topology_to_json(topology: Topology) -> dict:
-    return {
-        "Etag": topology.etag,
-        "Nodes": [
-            {
-                "Url": node.url,
-                "Database": node.database,
-                "ClusterTag": node.cluster_tag,
-                "ServerRole": node.server_role.value if node.server_role is not None else None,
-            }
-            for node in (topology.nodes or [])
-        ],
-    }
-
-
-def _topology_from_json(data: dict) -> Topology:
-    nodes = []
-    for node_json in data.get("Nodes") or []:
-        role = node_json.get("ServerRole")
-        nodes.append(
-            ServerNode(
-                node_json.get("Url"),
-                node_json.get("Database"),
-                node_json.get("ClusterTag"),
-                ServerNode.Role(role) if role else None,
-            )
-        )
-    return Topology(data.get("Etag"), nodes)
-
-
 def try_save(location: Optional[str], topology_hash: str, topology: Topology, extension: str) -> None:
     # Best-effort: caching failures must never break a topology update.
     if not location or topology is None:
         return
     try:
         os.makedirs(location, exist_ok=True)
         with open(_path(location, topology_hash, extension), "w", encoding="utf-8") as stream:
-            json.dump(_topology_to_json(topology), stream)
+            json.dump(topology.to_json(), stream)
     except Exception:
         pass
 
@@ -74,6 +43,6 @@ def try_load(location: Optional[str], topology_hash: str, extension: str) -> Opt
         if not os.path.isfile(path):
             return None
         with open(path, "r", encoding="utf-8") as stream:
-            return _topology_from_json(json.load(stream))
+            return Topology.from_json(json.load(stream))
     except Exception:
         return None

ravendb/tests/jvm_migrated_tests/client_tests/indexing_tests/time_series_tests/test_basic_time_series_indexes_java_script.py

@@ -24,10 +24,11 @@ class TestBasicTimeSeriesIndexesJavaScript(TestBase):
     def setUp(self):
         super(TestBasicTimeSeriesIndexesJavaScript, self).setUp()
 
-    @unittest.skip(
-        "JS time-series index: timeSeriesNamesFor emits a 'names' term before any time series "
-        "exists on 7.2.x (Corax term generation differs from the original Lucene expectation)"
-    )
+    def _customize_db_record(self, db_record):
+        # An empty timeSeriesNamesFor array indexes as a (blank) term on Corax but produces no term on
+        # Lucene; force the static index engine to Lucene so the "no time series -> no names" expectation holds.
+        db_record.settings["Indexing.Static.SearchEngineType"] = "Lucene"
+
     def test_time_series_names_for(self):
         now = RavenTestHelper.utc_today()
         index = Companies_ByTimeSeriesNames()

ravendb/tests/system_tests/test_system_create_topology_files.py

@@ -4,6 +4,9 @@
 from shutil import rmtree
 
 from ravendb.documents.store.definition import DocumentStore
+from ravendb.http import topology_local_cache
+from ravendb.http.server_node import ServerNode
+from ravendb.http.topology import Topology
 from ravendb.serverwide.database_record import DatabaseRecord
 from ravendb.serverwide.operations.common import DeleteDatabaseOperation, CreateDatabaseOperation
 from ravendb.tests.test_base import TestBase
@@ -60,6 +63,52 @@ def test_topology_creation(self):
             os.path.exists(os.path.join(TOPOLOGY_FILES_DIR, cluster_topology_hash + ".raven-cluster-topology"))
         )
 
+    def test_topology_cache_round_trip(self):
+        os.makedirs(TOPOLOGY_FILES_DIR, exist_ok=True)
+        topology = Topology(3, [ServerNode("http://localhost:9999", "db1", "B", ServerNode.Role.MEMBER)])
+        topology_hash = topology_local_cache.server_hash("http://localhost:9999", "db1")
+        topology_local_cache.try_save(
+            TOPOLOGY_FILES_DIR, topology_hash, topology, topology_local_cache.DATABASE_TOPOLOGY_EXTENSION
+        )
+
+        loaded = topology_local_cache.try_load(
+            TOPOLOGY_FILES_DIR, topology_hash, topology_local_cache.DATABASE_TOPOLOGY_EXTENSION
+        )
+        self.assertIsNotNone(loaded)
+        self.assertEqual(3, loaded.etag)
+        self.assertEqual(1, len(loaded.nodes))
+        node = loaded.nodes[0]
+        self.assertEqual("http://localhost:9999", node.url)
+        self.assertEqual("db1", node.database)
+        self.assertEqual("B", node.cluster_tag)
+        self.assertEqual(ServerNode.Role.MEMBER, node.server_role)
+
+    def test_topology_is_loaded_from_cache_when_urls_unreachable(self):
+        bad_url = "http://127.0.0.1:1"
+        database = "CacheSeedTest"
+        os.makedirs(TOPOLOGY_FILES_DIR, exist_ok=True)
+        cached = Topology(9, [ServerNode(bad_url, database, "A", ServerNode.Role.MEMBER)])
+        topology_local_cache.try_save(
+            TOPOLOGY_FILES_DIR,
+            topology_local_cache.server_hash(bad_url, database),
+            cached,
+            topology_local_cache.DATABASE_TOPOLOGY_EXTENSION,
+        )
+
+        # the server is unreachable, so the first topology update must fall back to the on-disk cache
+        with DocumentStore(urls=[bad_url], database=database) as store:
+            store.conventions.topology_cache_location = TOPOLOGY_FILES_DIR
+            store.initialize()
+            request_executor = store.get_request_executor()
+            request_executor._first_topology_update_task.result(30)
+
+            nodes = request_executor.topology_nodes
+            self.assertEqual(1, len(nodes))
+            self.assertEqual("A", nodes[0].cluster_tag)
+            self.assertEqual(bad_url, nodes[0].url)
+            self.assertEqual(ServerNode.Role.MEMBER, nodes[0].server_role)
+            self.assertEqual(9, request_executor.topology_etag)
+
 
 if __name__ == "__main__":
     unittest.main()