diff --git a/openc3/lib/openc3/api/interface_api.rb b/openc3/lib/openc3/api/interface_api.rb index 5e0be15d77..2bda484890 100644 --- a/openc3/lib/openc3/api/interface_api.rb +++ b/openc3/lib/openc3/api/interface_api.rb @@ -48,8 +48,7 @@ def get_interface(interface_name, manual: false, scope: $openc3_scope, token: $o authorize(permission: 'system', interface_name: interface_name, manual: manual, scope: scope, token: token) interface = InterfaceModel.get(name: interface_name, scope: scope) raise "Interface '#{interface_name}' does not exist (get_interface)" unless interface - - interface.merge(InterfaceStatusModel.get(name: interface_name, scope: scope)) + interface.merge(InterfaceStatusModel.get_or_default(name: interface_name, scope: scope)) end # @return [Array] All the interface names diff --git a/openc3/lib/openc3/api/router_api.rb b/openc3/lib/openc3/api/router_api.rb index 042d6c286b..27eb50a00a 100644 --- a/openc3/lib/openc3/api/router_api.rb +++ b/openc3/lib/openc3/api/router_api.rb @@ -47,8 +47,7 @@ def get_router(router_name, manual: false, scope: $openc3_scope, token: $openc3_ authorize(permission: 'system', router_name: router_name, manual: manual, scope: scope, token: token) router = RouterModel.get(name: router_name, scope: scope) raise "Router '#{router_name}' does not exist (get_router)" unless router - - router.merge(RouterStatusModel.get(name: router_name, scope: scope)) + router.merge(RouterStatusModel.get_or_default(name: router_name, scope: scope)) end # @return [Array] All the router names diff --git a/openc3/lib/openc3/models/interface_status_model.rb b/openc3/lib/openc3/models/interface_status_model.rb index 82c66604ad..fbaaf6f995 100644 --- a/openc3/lib/openc3/models/interface_status_model.rb +++ b/openc3/lib/openc3/models/interface_status_model.rb @@ -62,6 +62,12 @@ def self.get(name:, scope:) _db_sharded_get("#{scope}__#{_get_key}", name: name, scope: scope) end + # Like get but returns default instead of nil when no status exists + def self.get_or_default(name:, scope:, default: { 'state' => 'UNKNOWN' }) + status = get(name: name, scope: scope) + status.nil? ? default : status + end + def self.names(scope:) _db_sharded_names("#{scope}__#{_get_key}", scope: scope) end diff --git a/openc3/python/openc3/api/interface_api.py b/openc3/python/openc3/api/interface_api.py index c1f2e3731c..54e0d3dd50 100644 --- a/openc3/python/openc3/api/interface_api.py +++ b/openc3/python/openc3/api/interface_api.py @@ -47,8 +47,8 @@ def get_interface(interface_name, scope=OPENC3_SCOPE): authorize(permission="system", interface_name=interface_name, scope=scope) interface = InterfaceModel.get(name=interface_name, scope=scope) if not interface: - raise RuntimeError(f"Interface '{interface_name}' does not exist") - return interface | InterfaceStatusModel.get(name=interface_name, scope=scope) + raise RuntimeError(f"Interface '{interface_name}' does not exist (get_interface)") + return interface | InterfaceStatusModel.get_or_default(name=interface_name, scope=scope) # @return [Array] All the interface names diff --git a/openc3/python/openc3/api/router_api.py b/openc3/python/openc3/api/router_api.py index d92bf2bd40..cfca498c00 100644 --- a/openc3/python/openc3/api/router_api.py +++ b/openc3/python/openc3/api/router_api.py @@ -47,8 +47,8 @@ def get_router(router_name, scope=OPENC3_SCOPE): authorize(permission="system", router_name=router_name, scope=scope) router = RouterModel.get(name=router_name, scope=scope) if not router: - raise RuntimeError(f"Router '{router_name}' does not exist") - return router | RouterStatusModel.get(name=router_name, scope=scope) + raise RuntimeError(f"Router '{router_name}' does not exist (get_router)") + return router | RouterStatusModel.get_or_default(name=router_name, scope=scope) # @return [Array] All the router names diff --git a/openc3/python/openc3/models/interface_status_model.py b/openc3/python/openc3/models/interface_status_model.py index f888d971ff..e368b406b9 100644 --- a/openc3/python/openc3/models/interface_status_model.py +++ b/openc3/python/openc3/models/interface_status_model.py @@ -49,6 +49,14 @@ def _collect_db_shards(cls, scope): def get(cls, name: str, scope: str): return cls._db_sharded_get(f"{scope}__{cls._get_key()}", name, scope) + # Like get but returns default instead of None when no status exists + @classmethod + def get_or_default(cls, name: str, scope: str, default: dict | None = None): + status = cls.get(name=name, scope=scope) + if status is not None: + return status + return default if default is not None else {"state": "UNKNOWN"} + @classmethod def names(cls, scope: str): return cls._db_sharded_names(f"{scope}__{cls._get_key()}", scope) diff --git a/openc3/python/test/api/test_interface_api.py b/openc3/python/test/api/test_interface_api.py index 508fd94d68..f3a01ada8d 100644 --- a/openc3/python/test/api/test_interface_api.py +++ b/openc3/python/test/api/test_interface_api.py @@ -104,9 +104,20 @@ def test_returns_interface_hash(self): self.assertEqual(type(interface), dict) self.assertEqual(interface["name"], "INST_INT") # Verify it also includes the status - self.assertIn(get_interface("INST_INT")["state"], ["ATTEMPTING", "CONNECTED"]) + self.assertIn(interface["state"], ["ATTEMPTING", "CONNECTED"]) self.assertEqual(interface["clients"], 0) + def test_get_interface_raises_if_interface_does_not_exist(self): + with self.assertRaisesRegex(RuntimeError, r"Interface 'NONEXISTENT_INT' does not exist \(get_interface\)"): + get_interface("NONEXISTENT_INT") + + def test_get_interface_defaults_state_if_interface_status_does_not_exist(self): + model = InterfaceModel(name="NO_STATUS_INT", scope="DEFAULT") + model.create() + interface = get_interface("NO_STATUS_INT") + self.assertEqual(interface["name"], "NO_STATUS_INT") + self.assertEqual(interface["state"], "UNKNOWN") + def test_returns_all_interface_names(self): model = InterfaceModel(name="INT1", scope="DEFAULT") model.create() diff --git a/openc3/python/test/api/test_router_api.py b/openc3/python/test/api/test_router_api.py index daa279159f..9492aec912 100644 --- a/openc3/python/test/api/test_router_api.py +++ b/openc3/python/test/api/test_router_api.py @@ -15,6 +15,7 @@ from openc3.api.router_api import * from openc3.interfaces.interface import Interface +from openc3.microservices.router_microservice import RouterMicroservice from openc3.models.router_model import RouterModel from test.test_helper import * @@ -68,22 +69,50 @@ def build(): config_params=["openc3/interfaces/interface.py"], ) model.create() - # self.im = RouterMicroservice("DEFAULT__INTERFACE__ROUTE_INT") - # self.im_thread = threading.Thread(target=self.im.run) - # self.im_thread.start() + self.im = RouterMicroservice("DEFAULT__ROUTER__ROUTE_INT") + self.im_thread = threading.Thread(target=self.im.run) + self.im_thread.start() + # Wait for the command handler thread to subscribe to the interface + # command topic before returning. The handler captures its stream read + # offset lazily on its first read, so a command written by a test before + # that happens would be skipped (xread only returns newer messages), + # causing flaky failures under load. + self._wait_for_handler_ready() # time.sleep(0.001) # Allow the thread to run - # def tearDown(self): - # self.im.shutdown() - # time.sleep(0.001) - - # def test_returns_router_hash(self): - # interface = get_router("ROUTE_INT") - # self.assertEqual(type(interface), dict) - # self.assertEqual(interface["name"], "ROUTE_INT") - # # Verify it also includes the status - # self.assertEqual(interface["state"], "CONNECTED") - # self.assertEqual(interface["clients"], 0) + def tearDown(self): + self.im.shutdown() + self.im.graceful_kill() + self.im_thread.join() + time.sleep(0.001) + + def _wait_for_handler_ready(self, timeout=2.0): + topic = "{DEFAULT__CMD}ROUTER__ROUTE_INT" + store = EphemeralStore.instance() + end_time = time.time() + timeout + while time.time() < end_time: + if any(topic in offsets for offsets in store.topic_offsets.values()): + return + time.sleep(0.001) + + def test_returns_router_hash(self): + interface = get_router("ROUTE_INT") + self.assertEqual(type(interface), dict) + self.assertEqual(interface["name"], "ROUTE_INT") + # Verify it also includes the status + self.assertIn(interface["state"], ["ATTEMPTING", "CONNECTED"]) + self.assertEqual(interface["clients"], 0) + + def test_get_router_raises_if_router_does_not_exist(self): + with self.assertRaisesRegex(RuntimeError, "Router 'NONEXISTENT_ROUTER' does not exist"): + get_router("NONEXISTENT_ROUTER") + + def test_get_router_defaults_state_if_router_status_does_not_exist(self): + model = RouterModel(name="NO_STATUS_ROUTER", scope="DEFAULT") + model.create() + router = get_router("NO_STATUS_ROUTER") + self.assertEqual(router["name"], "NO_STATUS_ROUTER") + self.assertEqual(router["state"], "UNKNOWN") def test_returns_all_router_names(self): model = RouterModel(name="INT1", scope="DEFAULT") diff --git a/openc3/python/test/models/test_interface_status_model.py b/openc3/python/test/models/test_interface_status_model.py index f070d3c563..f5df928ae5 100644 --- a/openc3/python/test/models/test_interface_status_model.py +++ b/openc3/python/test/models/test_interface_status_model.py @@ -37,6 +37,19 @@ def test_set_and_get(self): status = InterfaceStatusModel.get("MyInterface", "DEFAULT") self.assertEqual(my.name, status["name"]) + def test_get_or_default_returns_unknown_for_nonexistent(self): + self.assertEqual(InterfaceStatusModel.get_or_default(name="IS", scope="DEFAULT"), {"state": "UNKNOWN"}) + + def test_get_or_default_returns_custom_default_for_nonexistent(self): + result = InterfaceStatusModel.get_or_default(name="IS", scope="DEFAULT", default={"state": "OTHER"}) + self.assertEqual(result, {"state": "OTHER"}) + + def test_get_or_default_returns_status_when_exists(self): + InterfaceStatusModel.set({"name": "IS", "state": "CONNECTED"}, scope="DEFAULT") + result = InterfaceStatusModel.get_or_default(name="IS", scope="DEFAULT") + self.assertEqual(result["name"], "IS") + self.assertEqual(result["state"], "CONNECTED") + def test_names_and_all(self): my = MyInterface() InterfaceStatusModel.set(my.as_json(), "DEFAULT") diff --git a/openc3/spec/api/interface_api_spec.rb b/openc3/spec/api/interface_api_spec.rb index 09cd1b12b7..6b0217185c 100644 --- a/openc3/spec/api/interface_api_spec.rb +++ b/openc3/spec/api/interface_api_spec.rb @@ -69,6 +69,18 @@ class ApiTest expect(interface['state']).to eql "CONNECTED" expect(interface['clients']).to eql 0 end + + it "raises if the interface does not exist" do + expect { @api.get_interface("NONEXISTENT_INT") }.to raise_error("Interface 'NONEXISTENT_INT' does not exist (get_interface)") + end + + it "defaults the state to UNKNOWN if the interface status does not exist" do + model = InterfaceModel.new(name: "NO_STATUS_INT", scope: "DEFAULT") + model.create + interface = @api.get_interface("NO_STATUS_INT") + expect(interface['name']).to eql "NO_STATUS_INT" + expect(interface['state']).to eql "UNKNOWN" + end end describe "get_interface_names" do diff --git a/openc3/spec/api/router_api_spec.rb b/openc3/spec/api/router_api_spec.rb index 151af56094..e567ffecba 100644 --- a/openc3/spec/api/router_api_spec.rb +++ b/openc3/spec/api/router_api_spec.rb @@ -45,9 +45,9 @@ class ApiTest @im_shutdown = false allow_any_instance_of(OpenC3::Interface).to receive(:read_interface) { sleep(0.01) until @im_shutdown } - model = MicroserviceModel.new(name: "DEFAULT__INTERFACE__ROUTE_INT", scope: "DEFAULT", target_names: ["INST"]) + model = MicroserviceModel.new(name: "DEFAULT__ROUTER__ROUTE_INT", scope: "DEFAULT", target_names: ["INST"]) model.create - @im = RouterMicroservice.new("DEFAULT__INTERFACE__ROUTE_INT") + @im = RouterMicroservice.new("DEFAULT__ROUTER__ROUTE_INT") @im_thread = Thread.new { @im.run } sleep(0.01) # Allow the thread to run @@ -69,6 +69,18 @@ class ApiTest expect(router['state']).to match(/CONNECTED|ATTEMPTING/) expect(router['clients']).to eql 0 end + + it "raises if the router does not exist" do + expect { @api.get_router("NONEXISTENT_ROUTER") }.to raise_error("Router 'NONEXISTENT_ROUTER' does not exist (get_router)") + end + + it "defaults the state to UNKNOWN if the router status does not exist" do + model = RouterModel.new(name: "NO_STATUS_ROUTER", scope: "DEFAULT") + model.create + router = @api.get_router("NO_STATUS_ROUTER") + expect(router['name']).to eql "NO_STATUS_ROUTER" + expect(router['state']).to eql "UNKNOWN" + end end describe "get_router_names" do diff --git a/openc3/spec/models/interface_status_model_spec.rb b/openc3/spec/models/interface_status_model_spec.rb index 6fdd44d9fd..cc133c78c5 100644 --- a/openc3/spec/models/interface_status_model_spec.rb +++ b/openc3/spec/models/interface_status_model_spec.rb @@ -65,6 +65,26 @@ module OpenC3 end end + describe "self.get_or_default" do + it "returns UNKNOWN state for nonexistent" do + result = InterfaceStatusModel.get_or_default(name: 'IS', scope: 'DEFAULT') + expect(result).to eq({ 'state' => 'UNKNOWN' }) + end + + it "returns a custom default for nonexistent" do + result = InterfaceStatusModel.get_or_default(name: 'IS', scope: 'DEFAULT', default: { 'state' => 'OTHER' }) + expect(result).to eq({ 'state' => 'OTHER' }) + end + + it "returns the status when it exists" do + InterfaceModel.new(name: "TEST_INT", scope: "DEFAULT").create + InterfaceStatusModel.set({ name: 'TEST_INT', state: 'CONNECTED' }, scope: 'DEFAULT') + result = InterfaceStatusModel.get_or_default(name: 'TEST_INT', scope: 'DEFAULT') + expect(result['name']).to eq('TEST_INT') + expect(result['state']).to eq('CONNECTED') + end + end + describe "self.names" do it "returns empty for no statuses" do names = InterfaceStatusModel.names(scope: 'DEFAULT')