From f3a151114145d487edf35ab550a4d53d914ee4db Mon Sep 17 00:00:00 2001
From: Matthew McEachen <matthew@photostructure.com>
Date: Thu, 21 Nov 2024 22:43:53 -0800
Subject: [PATCH] refactor: update test scripts and improve volume metadata
 handling - on linux, we now try to backfill label and uuid by traversing
 /dev/disks-by-* symlinks.

---
 package.json                                  |  2 +-
 .../{mtab.test.ts => linux_mtab.test.ts}      |  2 +-
 src/common/metadata_worker.h                  |  6 +-
 src/exports.ts                                | 21 ++++++-
 src/linux/dev_disk.ts                         | 55 +++++++++++++++++++
 src/linux/mtab.ts                             | 11 ++--
 src/linux/volume_metadata.cpp                 |  4 +-
 src/volume_metadata.ts                        |  2 +-
 8 files changed, 91 insertions(+), 12 deletions(-)
 rename src/__tests__/{mtab.test.ts => linux_mtab.test.ts} (99%)
 create mode 100644 src/linux/dev_disk.ts

diff --git a/package.json b/package.json
index 6efcdf9..93c44fe 100644
--- a/package.json
+++ b/package.json
@@ -44,7 +44,7 @@
     "watch": "tsc -p tsconfig.test.json --watch",
     "docs": "typedoc --out docs src/index.ts",
     "jest:coverage": "jest --coverage",
-    "jest:watch": "jest --watch",
+    "jest:watch": "npm t -- --watch",
     "jest:clear": "jest --clearCache",
     "// tests": "is called by .github/workflows/test.yml",
     "tests": "run-s test test:memory",
diff --git a/src/__tests__/mtab.test.ts b/src/__tests__/linux_mtab.test.ts
similarity index 99%
rename from src/__tests__/mtab.test.ts
rename to src/__tests__/linux_mtab.test.ts
index 8178d46..4ee30a6 100644
--- a/src/__tests__/mtab.test.ts
+++ b/src/__tests__/linux_mtab.test.ts
@@ -1,4 +1,4 @@
-// src/__tests__/mtab.test.ts
+// src/__tests__/linux_mtab.test.ts
 import { formatMtab, parseMtab } from "../linux/mtab.js";
 
 describe("mtab", () => {
diff --git a/src/common/metadata_worker.h b/src/common/metadata_worker.h
index 7a5c6d5..721346b 100644
--- a/src/common/metadata_worker.h
+++ b/src/common/metadata_worker.h
@@ -45,7 +45,10 @@ class MetadataWorkerBase : public Napi::AsyncWorker {
                           ? env.Null()
                           : Napi::String::New(env, metadata.uri));
     result.Set("status", Napi::String::New(env, metadata.status));
-    result.Set("remote", Napi::Boolean::New(env, metadata.remote));
+
+    if (metadata.remote) {
+      result.Set("remote", Napi::Boolean::New(env, metadata.remote));
+    }
     result.Set("remoteHost", metadata.remoteHost.empty()
                                  ? env.Null()
                                  : Napi::String::New(env, metadata.remoteHost));
@@ -59,4 +62,5 @@ class MetadataWorkerBase : public Napi::AsyncWorker {
 
   void OnOK() override { deferred_.Resolve(CreateResultObject()); }
 }; // class MetadataWorkerBase
+
 } // namespace FSMeta
\ No newline at end of file
diff --git a/src/exports.ts b/src/exports.ts
index 78a9dad..61f07db 100644
--- a/src/exports.ts
+++ b/src/exports.ts
@@ -7,6 +7,7 @@ import { filterMountPoints, filterTypedMountPoints } from "./config_filters.js";
 import { defer } from "./defer.js";
 import { WrappedError } from "./error.js";
 import { isRemoteFsType } from "./fs_type.js";
+import { getLabelFromDevDisk, getUuidFromDevDisk } from "./linux/dev_disk.js";
 import { getLinuxMountPoints } from "./linux/mount_points.js";
 import { isRemoteFSInfo, parseFsSpec, parseMtab } from "./linux/mtab.js";
 import { normalizeMountPoint } from "./mount_point.js";
@@ -132,11 +133,13 @@ export class ExportsImpl {
         const entry = entries.find((e) => e.fs_file === mountPoint);
 
         if (entry != null) {
+          device = entry.fs_spec;
           mtabInfo.fileSystem = entry.fs_vfstype;
           if (isRemoteFSInfo(entry)) {
             mtabInfo.remote = true;
             mtabInfo.remoteHost = entry.remoteHost;
             mtabInfo.remoteShare = entry.remoteShare;
+            console.log("mtab found remote", { mountPoint, entry });
           }
         }
       } catch (error) {
@@ -174,11 +177,27 @@ export class ExportsImpl {
       mountPoint,
       remote,
     }) as unknown as VolumeMetadata;
+
+    // Backfill if blkid or gio failed us:
+    if (isLinux && isNotBlank(device)) {
+      if (isBlank(result.uuid)) {
+        // Sometimes blkid doesn't have the UUID in cache. Try to get it from
+        // /dev/disk/by-uuid:
+        result.uuid = await getUuidFromDevDisk(device);
+      }
+      if (isBlank(result.label)) {
+        result.label = await getLabelFromDevDisk(device);
+      }
+    }
+
+    // Fix microsoft UUID format:
     result.uuid = extractUUID(result.uuid) ?? result.uuid;
+
+    // Normalize remote share path
     if (isNotBlank(result.remoteShare)) {
       result.remoteShare = normalizeMountPoint(result.remoteShare);
     }
 
-    return result;
+    return compactValues(result) as unknown as VolumeMetadata;
   }
 }
diff --git a/src/linux/dev_disk.ts b/src/linux/dev_disk.ts
new file mode 100644
index 0000000..f47d990
--- /dev/null
+++ b/src/linux/dev_disk.ts
@@ -0,0 +1,55 @@
+import { Dirent } from "node:fs";
+import { readdir, readlink } from "node:fs/promises";
+import { join, resolve } from "node:path";
+
+/**
+ * Gets the UUID from symlinks for a given device path asynchronously
+ * @param devicePath The device path to look up
+ * @returns Promise that resolves to the UUID if found, empty string otherwise
+ */
+export function getUuidFromDevDisk(devicePath: string) {
+  return getBasenameLinkedTo("/dev/disk/by-uuid", resolve(devicePath)).catch(
+    () => undefined,
+  );
+}
+
+/**
+ * Gets the label from symlinks for a given device path asynchronously
+ * @param devicePath The device path to look up
+ * @returns Promise that resolves to the label if found, empty string otherwise
+ */
+export function getLabelFromDevDisk(devicePath: string) {
+  return getBasenameLinkedTo("/dev/disk/by-label", resolve(devicePath)).catch(
+    () => undefined,
+  );
+}
+
+async function getBasenameLinkedTo(
+  linkDir: string,
+  linkPath: string,
+): Promise<string | undefined> {
+  for await (const ea of readLinks(linkDir)) {
+    if (ea.linkTarget === linkPath) {
+      return ea.dirent.name;
+    }
+  }
+  return;
+}
+
+// only exposed for test mocking
+export async function* readLinks(
+  directory: string,
+): AsyncGenerator<{ dirent: Dirent; linkTarget: string }, void, unknown> {
+  for (const dirent of await readdir(directory, { withFileTypes: true })) {
+    if (dirent.isSymbolicLink()) {
+      try {
+        const linkTarget = resolve(
+          await readlink(join(directory, dirent.name)),
+        );
+        yield { dirent, linkTarget };
+      } catch {
+        // Ignore errors
+      }
+    }
+  }
+}
diff --git a/src/linux/mtab.ts b/src/linux/mtab.ts
index 38754df..56c3f10 100644
--- a/src/linux/mtab.ts
+++ b/src/linux/mtab.ts
@@ -1,5 +1,6 @@
 // src/linux/mtab.ts
 
+import { isRemoteFsType } from "../fs_type.js";
 import { normalizeLinuxMountPoint } from "../mount_point.js";
 import { toInt } from "../number.js";
 import { isObject } from "../object.js";
@@ -148,7 +149,7 @@ export function parseMtab(
       continue; // Skip malformed lines
     }
 
-    const mountEntry: MountEntry = {
+    const entry: MountEntry = {
       fs_spec: fields[0]!,
       fs_file: normalizeLinuxMountPoint(fields[1] ?? ""),
       fs_vfstype: fields[2]!,
@@ -157,11 +158,13 @@ export function parseMtab(
       fs_passno: toInt(fields[5]),
     };
 
-    const remoteInfo = parseFsSpec(mountEntry.fs_spec);
+    const remoteInfo = isRemoteFsType(entry.fs_vfstype)
+      ? parseFsSpec(entry.fs_spec)
+      : undefined;
     if (remoteInfo) {
-      entries.push({ ...mountEntry, ...remoteInfo });
+      entries.push({ ...entry, ...remoteInfo });
     } else {
-      entries.push(mountEntry);
+      entries.push(entry);
     }
   }
 
diff --git a/src/linux/volume_metadata.cpp b/src/linux/volume_metadata.cpp
index 26caaec..b2ef165 100644
--- a/src/linux/volume_metadata.cpp
+++ b/src/linux/volume_metadata.cpp
@@ -27,6 +27,7 @@ class LinuxMetadataWorker : public MetadataWorkerBase {
       }
 
       uint64_t blockSize = vfs.f_frsize ? vfs.f_frsize : vfs.f_bsize;
+      metadata.remote = false;
       metadata.size = static_cast<double>(blockSize) * vfs.f_blocks;
       metadata.available = static_cast<double>(blockSize) * vfs.f_bavail;
       metadata.used =
@@ -60,14 +61,11 @@ class LinuxMetadataWorker : public MetadataWorkerBase {
           metadata.status = std::string("Blkid warning: ") + e.what();
         }
       }
-
     } catch (const std::exception &e) {
       SetError(e.what());
     }
   }
 
-  void OnOK() override { deferred_.Resolve(CreateResultObject()); }
-
 private:
   VolumeMetadataOptions options_;
 };
diff --git a/src/volume_metadata.ts b/src/volume_metadata.ts
index 137153b..0a5e93d 100644
--- a/src/volume_metadata.ts
+++ b/src/volume_metadata.ts
@@ -16,7 +16,7 @@ export interface VolumeMetadata {
   /**
    * The name of the partition
    */
-  label?: string;
+  label?: string | undefined;
   /**
    * Total size in bytes
    */