Merge pull request #781 from imobachgs/reconnect

Reconnect to the D-Bus client

web/package/cockpit-agama.changes

@@ -1,3 +1,9 @@
+-------------------------------------------------------------------
+Thu Oct  5 05:50:57 UTC 2023 - Imobach Gonzalez Sosa <[email protected]>
+
+- Implement a mechanism to reconnect to the D-Bus service
+  (gh#openSUSE/agama#781).
+
 -------------------------------------------------------------------
 Mon Oct  2 08:02:23 UTC 2023 - David Diaz <[email protected]>
 

web/src/App.jsx

@@ -39,13 +39,12 @@ import {
   Sidebar
 } from "~/components/core";
 import { ChangeProductLink } from "~/components/software";
-import { Layout, Title, DBusError } from "~/components/layout";
+import { SidebarArea } from "~/components/layout";
 
 function App() {
   const client = useInstallerClient();
   const [status, setStatus] = useState(undefined);
   const [phase, setPhase] = useState(undefined);
-  const [error, setError] = useState(undefined);
 
   useEffect(() => {
     const loadPhase = async () => {
@@ -55,22 +54,14 @@ function App() {
       setStatus(status);
     };
 
-    loadPhase().catch(setError);
-  }, [client.manager, setPhase, setStatus, setError]);
+    loadPhase().catch(console.error);
+  }, [client.manager, setPhase, setStatus]);
 
   useEffect(() => {
     return client.manager.onPhaseChange(setPhase);
   }, [client.manager, setPhase]);
 
-  useEffect(() => {
-    return client.monitor.onConnectionChange(connected => {
-      connected ? location.reload() : setError(true);
-    });
-  }, [client.monitor, setError]);
-
   const Content = () => {
-    if (error) return <DBusError />;
-
     if ((phase === STARTUP && status === BUSY) || phase === undefined || status === undefined) {
       return <LoadingEnvironment onStatusChange={setStatus} />;
     }
@@ -84,23 +75,20 @@ function App() {
 
   return (
     <>
-      <Sidebar>
-        <ChangeProductLink />
-        <IssuesLink />
-        <Disclosure label={_("Diagnostic tools")} data-keep-sidebar-open>
-          <ShowLogButton />
-          <LogsButton data-keep-sidebar-open="true" />
-          <ShowTerminalButton />
-        </Disclosure>
-        <About />
-      </Sidebar>
+      <SidebarArea>
+        <Sidebar>
+          <ChangeProductLink />
+          <IssuesLink />
+          <Disclosure label={_("Diagnostic tools")} data-keep-sidebar-open>
+            <ShowLogButton />
+            <LogsButton data-keep-sidebar-open="true" />
+            <ShowTerminalButton />
+          </Disclosure>
+          <About />
+        </Sidebar>
+      </SidebarArea>
 
-      <Layout>
-        {/* this is the name of the tool, do not translate it */}
-        {/* eslint-disable-next-line i18next/no-literal-string */}
-        <Title>Agama</Title>
-        <Content />
-      </Layout>
+      <Content />
     </>
   );
 }

web/src/App.test.jsx

@@ -21,22 +21,21 @@
 
 import React from "react";
 import { act, screen } from "@testing-library/react";
-import { installerRender, mockComponent, mockLayout } from "~/test-utils";
+import { installerRender } from "~/test-utils";
 import App from "./App";
 import { createClient } from "~/client";
 import { STARTUP, CONFIG, INSTALL } from "~/client/phase";
 import { IDLE, BUSY } from "~/client/status";
 
 jest.mock("~/client");
-jest.mock("~/components/layout/Layout", () => mockLayout());
 
 // Mock some components,
 // See https://www.chakshunyu.com/blog/how-to-mock-a-react-component-in-jest/#default-export
-jest.mock("~/components/layout/DBusError", () => mockComponent("D-BusError Mock"));
-jest.mock("~/components/core/LoadingEnvironment", () => mockComponent("LoadingEnvironment Mock"));
-jest.mock("~/components/questions/Questions", () => mockComponent("Questions Mock"));
-jest.mock("~/components/core/Installation", () => mockComponent("Installation Mock"));
-jest.mock("~/components/core/Sidebar", () => mockComponent("Sidebar Mock"));
+jest.mock("~/components/core/DBusError", () => <div>D-BusError Mock</div>);
+jest.mock("~/components/core/LoadingEnvironment", () => () => <div>LoadingEnvironment Mock</div>);
+jest.mock("~/components/questions/Questions", () => () => <div>Questions Mock</div>);
+jest.mock("~/components/core/Installation", () => () => <div>Installation Mock</div>);
+jest.mock("~/components/core/Sidebar", () => () => <div>Sidebar Mock</div>);
 
 // this object holds the mocked callbacks
 const callbacks = {};
@@ -45,9 +44,7 @@ const getPhaseFn = jest.fn();
 
 // capture the latest subscription to the manager#onPhaseChange for triggering it manually
 const onPhaseChangeFn = cb => { callbacks.onPhaseChange = cb };
-const onConnectionChangeFn = cb => { callbacks.onConnectionChange = cb };
 const changePhaseTo = phase => act(() => callbacks.onPhaseChange(phase));
-const changeConnectionTo = connected => act(() => callbacks.onConnectionChange(connected));
 
 describe("App", () => {
   beforeEach(() => {
@@ -58,59 +55,11 @@ describe("App", () => {
           getPhase: getPhaseFn,
           onPhaseChange: onPhaseChangeFn,
         },
-        monitor: {
-          onConnectionChange: onConnectionChangeFn
-        }
+        isConnected: async () => true,
       };
     });
   });
 
-  describe("when there are problems connecting with D-Bus service", () => {
-    beforeEach(() => {
-      getStatusFn.mockRejectedValue(new Error("Couldn't connect to D-Bus service"));
-    });
-
-    it("renders the DBusError component", async () => {
-      installerRender(<App />);
-      await screen.findByText("D-BusError Mock");
-    });
-  });
-
-  describe("when the D-Bus service is disconnected", () => {
-    beforeEach(() => {
-      Object.defineProperty(window, 'location', {
-        configurable: true,
-        value: { reload: jest.fn() },
-      });
-    });
-
-    it("renders the DBusError component", async () => {
-      installerRender(<App />);
-      changeConnectionTo(false);
-      installerRender(<App />);
-      await screen.findByText("D-BusError Mock");
-    });
-  });
-
-  describe("when the D-Bus service is re-connected", () => {
-    beforeEach(() => {
-      getStatusFn.mockRejectedValue(new Error("Couldn't connect to D-Bus service"));
-
-      Object.defineProperty(window, 'location', {
-        configurable: true,
-        value: { reload: jest.fn() },
-      });
-    });
-
-    it("reloads the page", async () => {
-      installerRender(<App />);
-      await screen.findByText("D-BusError Mock");
-
-      changeConnectionTo(true);
-      expect(window.location.reload).toHaveBeenCalled();
-    });
-  });
-
   describe("on the startup phase", () => {
     beforeEach(() => {
       getPhaseFn.mockResolvedValue(STARTUP);

web/src/DevServerWrapper.test.jsx

@@ -23,11 +23,8 @@ import React from "react";
 import { render, screen, waitFor } from "@testing-library/react";
 import { act } from "react-dom/test-utils";
 
-import { mockComponent } from "~/test-utils";
 import DevServerWrapper from "~/DevServerWrapper";
 
-jest.mock("~/components/layout/Loading", () => mockComponent("Loading Component Mock"));
-
 // mock XMLHttpRequest object
 const xhrMock = {
   open: jest.fn(),
@@ -39,7 +36,7 @@ describe("DevServerWrapper", () => {
     jest.spyOn(window, 'XMLHttpRequest').mockImplementation(() => xhrMock);
 
     render(<DevServerWrapper />);
-    screen.getByText(/Loading Component Mock/);
+    screen.getByText(/Loading installation environment/);
 
     expect(xhrMock.open).toBeCalledWith("GET", "/cockpit/login");
     expect(xhrMock.send).toHaveBeenCalled();

web/src/L10nBackendWrapper.test.jsx

@@ -27,6 +27,7 @@ import cockpit from "~/lib/cockpit";
 import { createClient } from "~/client";
 import { InstallerClientProvider } from "~/context/installer";
 import L10nBackendWrapper from "~/L10nBackendWrapper";
+import { noop } from "./utils";
 
 jest.mock("~/client");
 
@@ -40,7 +41,8 @@ beforeEach(() => {
       language: {
         getUILanguage: () => Promise.resolve(backendLang),
         setUILanguage: (lang) => new Promise((resolve) => resolve(setLanguageFn(lang)))
-      }
+      },
+      onDisconnect: () => noop
     };
   });
 });

web/src/Main.test.jsx

@@ -21,11 +21,11 @@
 
 import React from "react";
 import { screen } from "@testing-library/react";
-import { plainRender, mockComponent } from "~/test-utils";
+import { plainRender } from "~/test-utils";
 
 import Main from "~/Main";
 
-jest.mock("~/components/questions/Questions", () => mockComponent("Questions Mock"));
+jest.mock("~/components/questions/Questions", () => () => <div>Questions Mock</div>);
 
 it("renders the Questions component and the content", async () => {
   plainRender(<Main />);

web/src/client/index.js

@@ -31,8 +31,10 @@ import phase from "./phase";
 import { QuestionsClient } from "./questions";
 import { NetworkClient } from "./network";
 import { IssuesClient } from "./issues";
+import cockpit from "../lib/cockpit";
 
-const SERVICE_NAME = "org.opensuse.Agama";
+const BUS_ADDRESS_FILE = "/run/agama/bus.address";
+const MANAGER_SERVICE = "org.opensuse.Agama.Manager1";
 
 /**
  * @typedef {object} InstallerClient
@@ -45,6 +47,10 @@ const SERVICE_NAME = "org.opensuse.Agama";
  * @property {UsersClient} users - users client
  * @property {QuestionsClient} questions - questions client
  * @property {IssuesClient} issues - issues client
+ * @property {() => Promise<boolean>} isConnected - determines whether the client is connected
+ * @property {(handler: () => void) => (() => void)} onDisconnect - registers a handler to run
+ *   when the connection is lost. It returns a function to deregister the
+ *   handler.
  */
 
 /**
@@ -55,14 +61,23 @@ const SERVICE_NAME = "org.opensuse.Agama";
 const createClient = (address = "unix:path=/run/agama/bus") => {
   const language = new LanguageClient(address);
   const manager = new ManagerClient(address);
-  const monitor = new Monitor(address, SERVICE_NAME);
+  const monitor = new Monitor(address, MANAGER_SERVICE);
   const network = new NetworkClient();
   const software = new SoftwareClient(address);
   const storage = new StorageClient(address);
   const users = new UsersClient(address);
   const questions = new QuestionsClient(address);
   const issues = new IssuesClient({ storage });
 
+  const isConnected = async () => {
+    try {
+      await manager.getStatus();
+      return true;
+    } catch (e) {
+      return false;
+    }
+  };
+
   return {
     language,
     manager,
@@ -72,8 +87,16 @@ const createClient = (address = "unix:path=/run/agama/bus") => {
     storage,
     users,
     questions,
-    issues
+    issues,
+    isConnected,
+    onDisconnect: (handler) => monitor.onDisconnect(handler)
   };
 };
 
-export { createClient, phase };
+const createDefaultClient = async () => {
+  const file = cockpit.file(BUS_ADDRESS_FILE);
+  const address = await file.read();
+  return (address) ? createClient(address) : createClient();
+};
+
+export { createClient, createDefaultClient, phase };

web/src/client/monitor.js

@@ -23,8 +23,9 @@
 
 import DBusClient from "./dbus";
 
-const DBUS_SERVICE = "org.freedesktop.DBus";
-const MATCHER = { interface: DBUS_SERVICE, member: "NameOwnerChanged" };
+const NAME_OWNER_CHANGED = {
+  interface: "org.freedesktop.DBus", member: "NameOwnerChanged"
+};
 
 /**
  * Monitor a D-Bus service
@@ -42,15 +43,15 @@ class Monitor {
   /**
    * Registers a callback to be executed when the D-Bus service connection changes
    *
-   * @param {(connected: boolean) => void} handler - function to execute. It receives true if the
-   *   service was connected and false if the service was disconnected.
+   * @param {() => void} handler - function to execute when the client gets
+   *   disconnected.
+   * @return {() => void} function to deregister the callbacks.
    */
-  onConnectionChange(handler) {
-    return this.client.onSignal(MATCHER, (_path, _interface, _signal, args) => {
+  onDisconnect(handler) {
+    return this.client.onSignal(NAME_OWNER_CHANGED, (_path, _interface, _signal, args) => {
       const [service, , newOwner] = args;
-      if (service === this.serviceName) {
-        const connected = newOwner.length !== 0;
-        handler(connected);
+      if (service === this.serviceName && newOwner === "") {
+        handler();
       }
     });
   }

web/src/components/layout/DBusError.test.jsx → web/src/components/core/DBusError.test.jsx

@@ -22,11 +22,9 @@
 import React from "react";
 
 import { screen } from "@testing-library/react";
-import { plainRender, mockLayout } from "~/test-utils";
+import { plainRender } from "~/test-utils";
 
-import { DBusError } from "~/components/layout";
-
-jest.mock("~/components/layout/Layout", () => mockLayout());
+import { DBusError } from "~/components/core";
 
 describe("DBusError", () => {
   it("includes a generic D-Bus connection problem message", () => {
@@ -37,7 +35,7 @@ describe("DBusError", () => {
   });
 
   it("calls location.reload when user clicks on 'Reload'", async () => {
-    const { user } = plainRender(<DBusError />);
+    const { user } = plainRender(<DBusError />, { layout: true });
 
     const reloadButton = await screen.findByRole("button", { name: /Reload/i });
 

web/src/components/core/InstallationFinished.test.jsx

@@ -22,13 +22,12 @@
 import React from "react";
 
 import { screen } from "@testing-library/react";
-import { installerRender, mockLayout } from "~/test-utils";
+import { installerRender } from "~/test-utils";
 import { createClient } from "~/client";
 
 import InstallationFinished from "./InstallationFinished";
 
 jest.mock("~/client");
-jest.mock("~/components/layout/Layout", () => mockLayout());
 
 const finishInstallationFn = jest.fn();
 

web/src/components/core/InstallationProgress.test.jsx

@@ -22,12 +22,11 @@
 import React from "react";
 
 import { screen } from "@testing-library/react";
-import { installerRender, mockComponent, mockLayout } from "~/test-utils";
+import { installerRender } from "~/test-utils";
 
 import InstallationProgress from "./InstallationProgress";
 
-jest.mock("~/components/layout/Layout", () => mockLayout());
-jest.mock("~/components/core/ProgressReport", () => mockComponent("ProgressReport Mock"));
+jest.mock("~/components/core/ProgressReport", () => () => <div>ProgressReport Mock</div>);
 
 describe("InstallationProgress", () => {
   it("uses 'Installing' as title", async () => {