web: Fix storage devices (#1168)

## Problem Some properties of `StorageDevice` have a wrong type (e.g., `#wires` or `#devices`). The UI was implemented according to the wrong type, which raises a runtime error. ## Solution Fix the wrong types, improve the type checking and adapt the UI components. Bonus: Unused components `DeviceList` and `DeviceSelector` were dropped. ## Testing * Added and adapted unit tests. * Tested manually.
agama-project · Apr 24, 2024 · da1840a · da1840a
2 parents 22b4845 + 9ead1db
commit da1840a
Show file tree

Hide file tree

Showing 8 changed files with 420 additions and 420 deletions.
diff --git a/web/src/assets/styles/blocks.scss b/web/src/assets/styles/blocks.scss
@@ -329,28 +329,6 @@ ul[data-type="agama/list"][role="grid"] {
   }
 }
 
-// Each kind of list/selector has its way of laying out their items
-[data-items-type="agama/storage-devices"] {
-  display: grid;
-  gap: var(--spacer-smaller);
-  grid-template-columns: 1fr 2fr 2fr;
-  grid-template-areas: "type-and-size drive-info drive-content";
-
-  svg {
-    vertical-align: inherit;
-  }
-
-  > div {
-    margin-block-end: 0;
-  }
-
-  > :first-child {
-    align-self: center;
-    text-align: center;
-    justify-self: start;
-  }
-}
-
 [data-items-type="agama/space-policies"] {
   // It works with the default styling
 }

diff --git a/web/src/client/storage.js b/web/src/client/storage.js
@@ -62,8 +62,8 @@ const ZFCP_DISK_IFACE = "org.opensuse.Agama.Storage1.ZFCP.Disk";
  * @property {string} [transport]
  * @property {boolean} [sdCard]
  * @property {boolean} [dellBOSS]
- * @property {string[]} [devices] - RAID devices (only for "raid" and "md" types)
- * @property {string[]} [wires] - Multipath wires (only for "multipath" type)
+ * @property {StorageDevice[]} [devices] - RAID devices (only for "raid" and "md" types)
+ * @property {StorageDevice[]} [wires] - Multipath wires (only for "multipath" type)
  * @property {string} [level] - MD RAID level (only for "md" type)
  * @property {string} [uuid]
  * @property {number} [start] - First block of the region (only for block devices)
@@ -99,6 +99,7 @@ const ZFCP_DISK_IFACE = "org.opensuse.Agama.Storage1.ZFCP.Disk";
  * @property {number} sid
  * @property {string} type
  * @property {string} [mountPath]
+ * @property {string} [label]
  *
  * @typedef {object} ProposalResult
  * @property {ProposalSettings} settings
@@ -243,41 +244,48 @@ class DevicesManager {
    * @returns {Promise<StorageDevice[]>}
    */
   async getDevices() {
+    /** @type {(path: string, dbusDevices: object[]) => StorageDevice} */
     const buildDevice = (path, dbusDevices) => {
-      const addDeviceProperties = (device, dbusProperties) => {
-        device.sid = dbusProperties.SID.v;
-        device.name = dbusProperties.Name.v;
-        device.description = dbusProperties.Description.v;
+      /** @type {(device: StorageDevice, deviceProperties: object) => void} */
+      const addDeviceProperties = (device, deviceProperties) => {
+        device.sid = deviceProperties.SID.v;
+        device.name = deviceProperties.Name.v;
+        device.description = deviceProperties.Description.v;
       };
 
-      const addDriveProperties = (device, dbusProperties) => {
+      /** @type {(device: StorageDevice, driveProperties: object) => void} */
+      const addDriveProperties = (device, driveProperties) => {
         device.isDrive = true;
-        device.type = dbusProperties.Type.v;
-        device.vendor = dbusProperties.Vendor.v;
-        device.model = dbusProperties.Model.v;
-        device.driver = dbusProperties.Driver.v;
-        device.bus = dbusProperties.Bus.v;
-        device.busId = dbusProperties.BusId.v;
-        device.transport = dbusProperties.Transport.v;
-        device.sdCard = dbusProperties.Info.v.SDCard.v;
-        device.dellBOSS = dbusProperties.Info.v.DellBOSS.v;
+        device.type = driveProperties.Type.v;
+        device.vendor = driveProperties.Vendor.v;
+        device.model = driveProperties.Model.v;
+        device.driver = driveProperties.Driver.v;
+        device.bus = driveProperties.Bus.v;
+        device.busId = driveProperties.BusId.v;
+        device.transport = driveProperties.Transport.v;
+        device.sdCard = driveProperties.Info.v.SDCard.v;
+        device.dellBOSS = driveProperties.Info.v.DellBOSS.v;
       };
 
+      /** @type {(device: StorageDevice, raidProperties: object) => void} */
       const addRAIDProperties = (device, raidProperties) => {
         device.devices = raidProperties.Devices.v.map(d => buildDevice(d, dbusDevices));
       };
 
+      /** @type {(device: StorageDevice, multipathProperties: object) => void} */
       const addMultipathProperties = (device, multipathProperties) => {
         device.wires = multipathProperties.Wires.v.map(d => buildDevice(d, dbusDevices));
       };
 
+      /** @type {(device: StorageDevice, mdProperties: object) => void} */
       const addMDProperties = (device, mdProperties) => {
         device.type = "md";
         device.level = mdProperties.Level.v;
         device.uuid = mdProperties.UUID.v;
         device.devices = mdProperties.Devices.v.map(d => buildDevice(d, dbusDevices));
       };
 
+      /** @type {(device: StorageDevice, blockProperties: object) => void} */
       const addBlockProperties = (device, blockProperties) => {
         device.active = blockProperties.Active.v;
         device.encrypted = blockProperties.Encrypted.v;
@@ -289,22 +297,26 @@ class DevicesManager {
         device.udevPaths = blockProperties.UdevPaths.v;
       };
 
+      /** @type {(device: StorageDevice, partitionProperties: object) => void} */
       const addPartitionProperties = (device, partitionProperties) => {
         device.type = "partition";
         device.isEFI = partitionProperties.EFI.v;
       };
 
+      /** @type {(device: StorageDevice, lvmVgProperties: object) => void} */
       const addLvmVgProperties = (device, lvmVgProperties) => {
         device.type = "lvmVg";
         device.size = lvmVgProperties.Size.v;
         device.physicalVolumes = lvmVgProperties.PhysicalVolumes.v.map(d => buildDevice(d, dbusDevices));
         device.logicalVolumes = lvmVgProperties.LogicalVolumes.v.map(d => buildDevice(d, dbusDevices));
       };
 
+      /** @type {(device: StorageDevice) => void} */
       const addLvmLvProperties = (device) => {
         device.type = "lvmLv";
       };
 
+      /** @type {(device: StorageDevice, ptableProperties: object) => void} */
       const addPtableProperties = (device, ptableProperties) => {
         const buildPartitionSlot = ([start, size]) => ({ start, size });
         const partitions = ptableProperties.Partitions.v.map(p => buildDevice(p, dbusDevices));
@@ -316,6 +328,7 @@ class DevicesManager {
         };
       };
 
+      /** @type {(device: StorageDevice, filesystemProperties: object) => void} */
       const addFilesystemProperties = (device, filesystemProperties) => {
         const buildMountPath = path => path.length > 0 ? path : undefined;
         const buildLabel = label => label.length > 0 ? label : undefined;
@@ -327,15 +340,17 @@ class DevicesManager {
         };
       };
 
+      /** @type {(device: StorageDevice, componentProperties: object) => void} */
       const addComponentProperties = (device, componentProperties) => {
         device.component = {
           type: componentProperties.Type.v,
           deviceNames: componentProperties.DeviceNames.v
         };
       };
 
+      /** @type {StorageDevice} */
       const device = {
-        sid: path.split("/").pop(),
+        sid: Number(path.split("/").pop()),
         name: "",
         description: "",
         isDrive: false,

diff --git a/web/src/client/storage.test.js b/web/src/client/storage.test.js
@@ -25,6 +25,10 @@
 import DBusClient from "./dbus";
 import { StorageClient } from "./storage";
 
+/**
+ * @typedef {import("~/client/storage").StorageDevice} StorageDevice
+ */
+
 jest.mock("./dbus");
 
 const cockpitProxies = {};
@@ -35,6 +39,7 @@ let managedObjects = {};
 
 // System devices
 
+/** @type {StorageDevice}  */
 const sda = {
   sid: 59,
   isDrive: true,
@@ -59,6 +64,7 @@ const sda = {
   udevPaths: ["pci-0000:00-12", "pci-0000:00-12-ata"],
 };
 
+/** @type {StorageDevice}  */
 const sda1 = {
   sid: 60,
   isDrive: false,
@@ -76,6 +82,7 @@ const sda1 = {
   isEFI: false
 };
 
+/** @type {StorageDevice}  */
 const sda2 = {
   sid: 61,
   isDrive: false,
@@ -93,6 +100,7 @@ const sda2 = {
   isEFI: false
 };
 
+/** @type {StorageDevice}  */
 const sdb = {
   sid: 62,
   isDrive: true,
@@ -117,6 +125,7 @@ const sdb = {
   udevPaths: ["pci-0000:00-19"]
 };
 
+/** @type {StorageDevice}  */
 const sdc = {
   sid: 63,
   isDrive: true,
@@ -141,6 +150,7 @@ const sdc = {
   udevPaths: []
 };
 
+/** @type {StorageDevice}  */
 const sdd = {
   sid: 64,
   isDrive: true,
@@ -165,6 +175,7 @@ const sdd = {
   udevPaths: []
 };
 
+/** @type {StorageDevice}  */
 const sde = {
   sid: 65,
   isDrive: true,
@@ -189,6 +200,7 @@ const sde = {
   udevPaths: []
 };
 
+/** @type {StorageDevice}  */
 const md0 = {
   sid: 66,
   isDrive: false,
@@ -202,12 +214,14 @@ const md0 = {
   start: 0,
   encrypted: false,
   recoverableSize: 0,
+  devices: [],
   systems : ["openSUSE Leap 15.2"],
   udevIds: [],
   udevPaths: [],
   filesystem: { sid: 100, type: "ext4", mountPath: "/test", label: "system" }
 };
 
+/** @type {StorageDevice}  */
 const raid = {
   sid: 67,
   isDrive: true,
@@ -227,11 +241,13 @@ const raid = {
   start: 0,
   encrypted: false,
   recoverableSize: 0,
+  devices: [],
   systems : [],
   udevIds: [],
   udevPaths: []
 };
 
+/** @type {StorageDevice}  */
 const multipath = {
   sid: 68,
   isDrive: true,
@@ -256,6 +272,7 @@ const multipath = {
   udevPaths: []
 };
 
+/** @type {StorageDevice}  */
 const dasd = {
   sid: 69,
   isDrive: true,
@@ -280,6 +297,7 @@ const dasd = {
   udevPaths: []
 };
 
+/** @type {StorageDevice}  */
 const sdf = {
   sid: 70,
   isDrive: true,
@@ -304,6 +322,7 @@ const sdf = {
   udevPaths: []
 };
 
+/** @type {StorageDevice}  */
 const sdf1 = {
   sid: 71,
   isDrive: false,
@@ -321,6 +340,7 @@ const sdf1 = {
   isEFI: false
 };
 
+/** @type {StorageDevice}  */
 const lvmVg = {
   sid: 72,
   isDrive: false,
@@ -330,6 +350,7 @@ const lvmVg = {
   size: 512
 };
 
+/** @type {StorageDevice}  */
 const lvmLv1 = {
   sid: 73,
   isDrive: false,
@@ -414,6 +435,7 @@ const systemDevices = {
 //
 // Using a single device because most of the checks are already done with system devices.
 
+/** @type {StorageDevice}  */
 const sdbStaging = {
   sid: 62,
   isDrive: true,