From c393d4a2c6a5c9fd916cab3f281e32cd844086c6 Mon Sep 17 00:00:00 2001 From: Rodolfo Olivieri Date: Tue, 17 Dec 2024 10:58:23 -0300 Subject: [PATCH] Switch to system bus instead of session bus (#69) --- Makefile | 1 + command_line_assistant/dbus/constants.py | 6 ++--- command_line_assistant/dbus/server.py | 14 +++++----- data/development/systemd/clad-devel.service | 2 ++ data/release/com.redhat.lightspeed.conf | 12 ++++++--- tests/dbus/test_server.py | 30 +++++++++------------ 6 files changed, 34 insertions(+), 31 deletions(-) diff --git a/Makefile b/Makefile index eba937b..61464d6 100644 --- a/Makefile +++ b/Makefile @@ -86,6 +86,7 @@ link-systemd-units: ## Link the systemd units to /etc/systemd/user @echo "Linking the systemd units from $(CLAD_SYSTEMD_DEVEL_PATH) to $(SYSTEMD_USER_UNITS)/clad.service" @sed -e 's/{{ EXEC_START }}/$(CLAD_VENV_BIN)/' \ -e 's/{{ CONFIG_FILE_PATH }}/$(XDG_CONFIG_DIRS)/' \ + -e 's/{{ DBUS_SESSION_ADDRESS }}/$(subst /,\/,$(DBUS_SESSION_BUS_ADDRESS))/' \ $(CLAD_SYSTEMD_DEVEL_PATH) > $(CLAD_SYSTEMD_USER_PATH) @ln -s $(CLAD_SYSTEMD_USER_PATH) $(SYSTEMD_USER_UNITS)/clad.service diff --git a/command_line_assistant/dbus/constants.py b/command_line_assistant/dbus/constants.py index b1fe6ee..757b00d 100644 --- a/command_line_assistant/dbus/constants.py +++ b/command_line_assistant/dbus/constants.py @@ -4,7 +4,7 @@ ERROR_MAPPER = ErrorMapper() -SESSION_BUS = SystemMessageBus(error_mapper=ERROR_MAPPER) +SYSTEM_BUS = SystemMessageBus(error_mapper=ERROR_MAPPER) SERVICE_NAMESPACE = ("com", "redhat", "lightspeed") @@ -13,9 +13,9 @@ # Define the service identifier for a query QUERY_IDENTIFIER = DBusServiceIdentifier( - namespace=QUERY_NAMESAPCE, message_bus=SESSION_BUS + namespace=QUERY_NAMESAPCE, message_bus=SYSTEM_BUS ) # Define the service identifier for a history HISTORY_IDENTIFIER = DBusServiceIdentifier( - namespace=HISTORY_NAMESPACE, message_bus=SESSION_BUS + namespace=HISTORY_NAMESPACE, message_bus=SYSTEM_BUS ) diff --git a/command_line_assistant/dbus/server.py b/command_line_assistant/dbus/server.py index 7a6ead8..8dc0926 100644 --- a/command_line_assistant/dbus/server.py +++ b/command_line_assistant/dbus/server.py @@ -7,7 +7,7 @@ from command_line_assistant.dbus.constants import ( HISTORY_IDENTIFIER, QUERY_IDENTIFIER, - SESSION_BUS, + SYSTEM_BUS, ) from command_line_assistant.dbus.context import HistoryContext, QueryContext from command_line_assistant.dbus.interfaces import HistoryInterface, QueryInterface @@ -19,24 +19,22 @@ def serve(config: Config): """Start the daemon.""" logger.info("Starting clad!") try: - SESSION_BUS.publish_object( + SYSTEM_BUS.publish_object( QUERY_IDENTIFIER.object_path, QueryInterface(QueryContext(config)) ) - SESSION_BUS.publish_object( + SYSTEM_BUS.publish_object( HISTORY_IDENTIFIER.object_path, HistoryInterface(HistoryContext(config)) ) # The flag DBUS_NAME_FLAG_REPLACE_EXISTING is needed during development # so ew can replace the existing bus. # TODO(r0x0d): See what to do with it later. - SESSION_BUS.register_service( - QUERY_IDENTIFIER.service_name, flags=DBUS_NAME_FLAG_REPLACE_EXISTING - ) - SESSION_BUS.register_service( + SYSTEM_BUS.register_service(QUERY_IDENTIFIER.service_name) + SYSTEM_BUS.register_service( HISTORY_IDENTIFIER.service_name, flags=DBUS_NAME_FLAG_REPLACE_EXISTING ) loop = EventLoop() loop.run() finally: # Unregister the DBus service and objects. - SESSION_BUS.disconnect() + SYSTEM_BUS.disconnect() diff --git a/data/development/systemd/clad-devel.service b/data/development/systemd/clad-devel.service index 0c9d153..dae358b 100644 --- a/data/development/systemd/clad-devel.service +++ b/data/development/systemd/clad-devel.service @@ -8,6 +8,8 @@ PrivateTmp=yes RemainAfterExit=no ExecStart={{ EXEC_START }} Environment="XDG_CONFIG_DIRS={{ CONFIG_FILE_PATH }}" +# This is a workaround for intended only for local development. +Environment="DBUS_SYSTEM_BUS_ADDRESS={{ DBUS_SESSION_ADDRESS }}" [Install] WantedBy=multi-user.target diff --git a/data/release/com.redhat.lightspeed.conf b/data/release/com.redhat.lightspeed.conf index 9de351f..f2bac57 100644 --- a/data/release/com.redhat.lightspeed.conf +++ b/data/release/com.redhat.lightspeed.conf @@ -4,12 +4,18 @@ - + + - - + + + + + + + diff --git a/tests/dbus/test_server.py b/tests/dbus/test_server.py index 5b5e96c..fab76dc 100644 --- a/tests/dbus/test_server.py +++ b/tests/dbus/test_server.py @@ -8,9 +8,9 @@ def test_serve(monkeypatch): event_loop_mock = mock.Mock() - session_bus_mock = mock.Mock() + system_bus_mock = mock.Mock() monkeypatch.setattr(server, "EventLoop", event_loop_mock) - monkeypatch.setattr(server, "SESSION_BUS", session_bus_mock) + monkeypatch.setattr(server, "SYSTEM_BUS", system_bus_mock) config = Config() server.serve(config) @@ -20,21 +20,17 @@ def test_serve(monkeypatch): def test_serve_registers_services(monkeypatch): event_loop_mock = mock.Mock() - session_bus_mock = mock.Mock() + system_bus_mock = mock.Mock() monkeypatch.setattr(server, "EventLoop", event_loop_mock) - monkeypatch.setattr(server, "SESSION_BUS", session_bus_mock) + monkeypatch.setattr(server, "SYSTEM_BUS", system_bus_mock) config = Config() server.serve(config) - assert session_bus_mock.publish_object.call_count == 2 - assert session_bus_mock.register_service.call_count == 2 + assert system_bus_mock.publish_object.call_count == 2 + assert system_bus_mock.register_service.call_count == 2 assert ( - session_bus_mock.register_service.call_args_list[0][1]["flags"] - == DBUS_NAME_FLAG_REPLACE_EXISTING - ) - assert ( - session_bus_mock.register_service.call_args_list[1][1]["flags"] + system_bus_mock.register_service.call_args_list[1][1]["flags"] == DBUS_NAME_FLAG_REPLACE_EXISTING ) @@ -42,9 +38,9 @@ def test_serve_registers_services(monkeypatch): def test_serve_cleanup_on_exception(monkeypatch): event_loop_mock = mock.Mock() event_loop_mock.return_value.run.side_effect = Exception("Test error") - session_bus_mock = mock.Mock() + system_bus_mock = mock.Mock() monkeypatch.setattr(server, "EventLoop", event_loop_mock) - monkeypatch.setattr(server, "SESSION_BUS", session_bus_mock) + monkeypatch.setattr(server, "SYSTEM_BUS", system_bus_mock) config = Config() try: @@ -52,19 +48,19 @@ def test_serve_cleanup_on_exception(monkeypatch): except Exception: pass - session_bus_mock.disconnect.assert_called_once() + system_bus_mock.disconnect.assert_called_once() def test_serve_creates_interfaces(monkeypatch): event_loop_mock = mock.Mock() - session_bus_mock = mock.Mock() + system_bus_mock = mock.Mock() monkeypatch.setattr(server, "EventLoop", event_loop_mock) - monkeypatch.setattr(server, "SESSION_BUS", session_bus_mock) + monkeypatch.setattr(server, "SYSTEM_BUS", system_bus_mock) config = Config() server.serve(config) - publish_calls = session_bus_mock.publish_object.call_args_list + publish_calls = system_bus_mock.publish_object.call_args_list assert len(publish_calls) == 2 assert isinstance(publish_calls[0][0][1], server.QueryInterface) assert isinstance(publish_calls[1][0][1], server.HistoryInterface)