Use cockpit.proxies and reduce the number of async methods

web/src/Network.jsx

@@ -22,50 +22,46 @@
 import React, { useEffect, useState } from "react";
 import { Button, Stack, StackItem } from "@patternfly/react-core";
 import { useInstallerClient } from "./context/installer";
-import { useCancellablePromise } from "./utils";
-import { ConnectionTypes } from "./client/network";
+import { ConnectionTypes, NetworkEventTypes } from "./client/network";
 import NetworkWiredStatus from "./NetworkWiredStatus";
 import NetworkWifiStatus from "./NetworkWifiStatus";
 import WirelessSelector from "./WirelessSelector";
 
 export default function Network() {
   const client = useInstallerClient();
   const [initialized, setInitialized] = useState(false);
-  const { cancellablePromise } = useCancellablePromise();
   const [connections, setConnections] = useState([]);
   const [accessPoints, setAccessPoints] = useState([]);
   const [openWirelessSelector, setOpenWirelessSelector] = useState(false);
 
   useEffect(() => {
-    cancellablePromise(client.network.activeConnections()).then(setConnections);
-  }, [client.network, cancellablePromise]);
-
-  useEffect(() => {
-    const onConnectionAdded = addedConnection => {
-      setConnections(conns => [...conns, addedConnection]);
-    };
+    if (!initialized) return;
 
-    return client.network.listen("connectionAdded", onConnectionAdded);
-  }, [client.network]);
+    setConnections(client.network.activeConnections());
+  }, [client.network, initialized]);
 
   useEffect(() => {
-    const onConnectionRemoved = id => {
-      setConnections(conns => conns.filter(c => c.id !== id));
-    };
+    return client.network.onNetworkEvent(({ type, payload }) => {
+      switch (type) {
+        case NetworkEventTypes.ACTIVE_CONNECTION_ADDED: {
+          setConnections(conns => [...conns, payload]);
+          break;
+        }
 
-    return client.network.listen("connectionRemoved", onConnectionRemoved);
-  }, [client.network]);
-
-  useEffect(() => {
-    const onConnectionUpdated = connection => {
-      setConnections(conns => {
-        const newConnections = conns.filter(c => c.id !== connection.id);
-        return [...newConnections, connection];
-      });
-    };
+        case NetworkEventTypes.ACTIVE_CONNECTION_UPDATED: {
+          setConnections(conns => {
+            const newConnections = conns.filter(c => c.id !== payload.id);
+            return [...newConnections, payload];
+          });
+          break;
+        }
 
-    return client.network.listen("connectionUpdated", onConnectionUpdated);
-  }, [client.network]);
+        case NetworkEventTypes.ACTIVE_CONNECTION_REMOVED: {
+          setConnections(conns => conns.filter(c => c.id !== payload.id));
+        }
+      }
+    });
+  });
 
   useEffect(() => {
     client.network.setUp().then(() => setInitialized(true));

web/src/TargetIpsPopup.jsx

@@ -30,64 +30,45 @@ import { formatIp } from "./client/network";
 export default function TargetIpsPopup() {
   const client = useInstallerClient();
   const { cancellablePromise } = useCancellablePromise();
-  const [connections, setConnections] = useState([]);
-  const [hostname, setHostname] = useState("");
+  const [addresses, setAddresses] = useState([]);
+  const [initialized, setInitialized] = useState(false);
+  const [hostname, setHostname] = useState();
   const [isOpen, setIsOpen] = useState(false);
 
   useEffect(() => {
-    cancellablePromise(client.network.config()).then(config => {
-      setConnections(config.connections);
-      setHostname(config.hostname);
-    });
+    cancellablePromise(client.network.setUp()).then(() => setInitialized(true));
   }, [client.network, cancellablePromise]);
 
   useEffect(() => {
-    const onConnectionAdded = addedConnection => {
-      setConnections(conns => [...conns, addedConnection]);
-    };
-
-    return client.network.listen("connectionAdded", onConnectionAdded);
-  }, [client.network]);
+    if (!initialized) return;
 
-  useEffect(() => {
-    const onConnectionRemoved = id => {
-      setConnections(conns => conns.filter(c => c.id !== id));
+    const refreshState = () => {
+      setAddresses(client.network.addresses());
+      setHostname(client.network.hostname());
     };
 
-    return client.network.listen("connectionRemoved", onConnectionRemoved);
-  }, [client.network]);
-
-  useEffect(() => {
-    const onConnectionUpdated = updatedConnection => {
-      setConnections(conns => {
-        const newConnections = conns.filter(c => c.id !== updatedConnection.id);
-        return [...newConnections, updatedConnection];
-      });
-    };
-
-    return client.network.listen("connectionUpdated", onConnectionUpdated);
-  }, [client.network]);
-
-  if (connections.length === 0) return null;
-
-  const ips = connections.flatMap(conn => conn.addresses.map(formatIp));
-  const [firstIp] = ips;
+    refreshState();
+    return client.network.onNetworkEvent(() => {
+      refreshState();
+    });
+  }, [client.network, initialized]);
 
-  if (ips.length === 0) return null;
+  if (addresses.length === 0) return null;
+  const [firstIp] = addresses;
 
   const open = () => setIsOpen(true);
   const close = () => setIsOpen(false);
 
   return (
     <>
-      <Button variant="link" onClick={open} isDisabled={ips.length === 1}>
-        {firstIp} {hostname && <Text component="small">({hostname})</Text>}
+      <Button variant="link" onClick={open} isDisabled={addresses.length === 1}>
+        {formatIp(firstIp)} {hostname && <Text component="small">({hostname})</Text>}
       </Button>
 
-      <Popup isOpen={isOpen} title="Ip Addresses">
+      <Popup isOpen={isOpen} title="IP Addresses">
         <List>
-          {ips.map(ip => (
-            <ListItem key={ip}>{ip}</ListItem>
+          {addresses.map((ip, index) => (
+            <ListItem key={index}>{formatIp(ip)}</ListItem>
           ))}
         </List>
         <Popup.Actions>

web/src/TargetIpsPopup.test.jsx

@@ -29,29 +29,25 @@ import TargetIpsPopup from "./TargetIpsPopup";
 
 jest.mock("./client");
 
-const conn0 = {
-  id: "7a9470b5-aa0e-4e20-b48e-3eee105543e9",
-  addresses: [
-    { address: "1.2.3.4", prefix: 24 },
-    { address: "5.6.7.8", prefix: 16 },
-  ],
-};
+const addresses = [
+  { address: "1.2.3.4", prefix: 24 },
+  { address: "5.6.7.8", prefix: 16 },
+];
+const addressFn = jest.fn().mockReturnValue(addresses);
+const hostnameFn = jest.fn().mockReturnValue("example.net");
 
 describe("TargetIpsPopup", () => {
   let callbacks;
-  const hostname = "example.net";
-
   beforeEach(() => {
-    callbacks = {};
-    const listenFn = (event, cb) => { callbacks[event] = cb };
+    callbacks = [];
+    const onNetworkEventFn = (cb) => { callbacks.push(cb) };
     createClient.mockImplementation(() => {
       return {
         network: {
-          listen: listenFn,
-          config: () => Promise.resolve({
-            connections: [conn0],
-            hostname
-          }),
+          onNetworkEvent: onNetworkEventFn,
+          addresses: addressFn,
+          hostname: hostnameFn,
+          setUp: jest.fn().mockResolvedValue()
         }
       };
     });
@@ -65,7 +61,7 @@ describe("TargetIpsPopup", () => {
 
     const dialog = await screen.findByRole("dialog");
 
-    within(dialog).getByText(/Ip Addresses/);
+    within(dialog).getByText(/IP Addresses/);
     within(dialog).getByText("5.6.7.8/16");
 
     const closeButton = within(dialog).getByRole("button", { name: /Close/i });
@@ -76,33 +72,15 @@ describe("TargetIpsPopup", () => {
     });
   });
 
-  it("updates the IP if the connection changes", async () => {
-    installerRender(<TargetIpsPopup />);
-    await screen.findByRole("button", { name: /1.2.3.4\/24 \(example.net\)/i });
-    const updatedConn = {
-      ...conn0,
-      addresses: [{ address: "5.6.7.8", prefix: 24 }]
-    };
-
-    act(() => {
-      callbacks.connectionUpdated(updatedConn);
-    });
-    await screen.findByRole("button", { name: /5.6.7.8\/24 \(example.net\)/i });
-  });
-
-  it("updates the IP if the connection is replaced", async () => {
+  it("updates address and hostname if they change", async () => {
     installerRender(<TargetIpsPopup />);
     await screen.findByRole("button", { name: /1.2.3.4\/24 \(example.net\)/i });
-    const conn1 = {
-      ...conn0,
-      id: "2f1b1c0d-c835-479d-ae7d-e828bb4a75fa",
-      addresses: [{ address: "5.6.7.8", prefix: 24 }]
-    };
 
+    addressFn.mockReturnValue([{ address: "5.6.7.8", prefix: 24 }]);
+    hostnameFn.mockReturnValue("localhost.localdomain");
     act(() => {
-      callbacks.connectionAdded(conn1);
-      callbacks.connectionRemoved(conn0.id);
+      callbacks.forEach(cb => cb());
     });
-    await screen.findByRole("button", { name: /5.6.7.8\/24 \(example.net\)/i });
+    await screen.findByRole("button", { name: /5.6.7.8\/24 \(localhost.localdomain\)/i });
   });
 });

web/src/client/network.test.js

@@ -33,8 +33,8 @@ const conn = {
 
 const adapter = {
   setUp: jest.fn(),
-  activeConnections: jest.fn().mockResolvedValue([conn]),
-  hostname: jest.fn().mockResolvedValue("localhost"),
+  activeConnections: jest.fn().mockReturnValue([conn]),
+  hostname: jest.fn().mockReturnValue("localhost.localdomain"),
   subscribe: jest.fn(),
   getConnection: jest.fn(),
   addConnection: jest.fn(),
@@ -43,16 +43,25 @@ const adapter = {
 };
 
 describe("NetworkClient", () => {
-  describe("#config", () => {
-    it("returns an object containing the hostname, known IPv4 addresses, and active connections", async () => {
+  describe("#activeConnections", () => {
+    it("retuns the list of active connections from the adapter", () => {
       const client = new NetworkClient(adapter);
-      const config = await client.config();
+      const connections = client.activeConnections();
+      expect(connections).toEqual([conn]);
+    });
+  });
 
-      expect(config.hostname).toEqual("localhost");
-      expect(config.addresses).toEqual([
-        { address: "192.168.122.1", prefix: 24 }
-      ]);
-      expect(config.connections).toEqual([conn]);
+  describe("#addresses", () => {
+    it("returns the list of addresses", () => {
+      const client = new NetworkClient(adapter);
+      expect(client.addresses()).toEqual([{ address: "192.168.122.1", prefix: 24 }]);
+    });
+  });
+
+  describe("#hostname", () => {
+    it("returns the hostname from the adapter", () => {
+      const client = new NetworkClient(adapter);
+      expect(client.hostname()).toEqual("localhost.localdomain");
     });
   });
 });