Refactor dlayer-source API to be less opinionated

src/packages/dlayer-source/index.js

@@ -1,9 +1,8 @@
 "use strict";
 
-const Promise = require("bluebird");
-const syncpipe = require("syncpipe");
 const util = require("util");
 const Result = require("@joepie91/result");
+const mapObject = require("map-obj");
 
 const ID = Symbol("dlayer-source object ID");
 const AllowErrors = Symbol("dlayer-source allow-errors marker");
@@ -11,73 +10,56 @@ const AllowErrors = Symbol("dlayer-source allow-errors marker");
 // TODO: Make more readable
 // TODO: Refactor allowErrors logic so that it's actually part of the internal $getProperty implementation in dlayer itself, and this abstraction uses that tool?
 
-module.exports = {
+module.exports = function dlayerSource(source, properties) {
-	withSources: function withSources(schemaObject) {
+	// contextName, { targetProperty: sourceProperty }
-		let { $sources, ... rest } = schemaObject;
-	
-		let generatedProperties = syncpipe($sources ?? {}, [
-			(_) => Object.entries(_),
-			(_) => _.flatMap(([ source, properties ]) => {
-				return Object.entries(properties).map(([ property, selector ]) => {
-					let getter = function (_args, context) {
-						return Promise.try(() => {
-							if (properties[ID] != null) {
-								let dataSource = context[source];
 
-								if (dataSource != null) {
+	return mapObject(properties, (property, selector) => {
-									// console.log(`Calling source '${source}' with ID ${util.inspect(properties[ID])}`);
+		return [
-									return Result.wrapAsync(() => dataSource.load(properties[ID]));
+			property,
-								} else {
+			async function (_args, context) {
-									throw new Error(`Attempted to read from context property '${source}', but no such property exists`);
+				let dataSource = context[source];
-								}
+				let sourceID = properties[ID];
-							} else {
+				let allowErrors = (properties[AllowErrors] === true);
-								// FIXME: Better error message
+				let isCustomSelector = (typeof selector !== "string");
-								throw new Error(`Must specify a dlayer-source ID`);
-							}
-						}).then((result) => {
-							// console.log(`Result [${source}|${util.inspect(properties[ID])}] ${util.inspect(result)}`);
 
-							// The AllowErrors option is set when a source definition has its own way to deal with (allowable) errors. Instead of simply propagating the error for all affected attributes, it calls the attribute handlers with the Result (or returns `undefined` if only a property is specified).
+				if (dataSource == null) {
-							// TODO: How to deal with null results? Allow them or not? Make it an option?
+					throw new Error(`Attempted to read from context property '${source}', but no such property exists`);
-							if (result.isOK && result.value() == null) {
+				} else if (sourceID == null) {
-								// TODO: Change implementation to allow `Result.ok(null|undefined)` but not `null|undefined` directly?
+					// FIXME: Better error message
-								throw new Error(`Null-ish result returned for ID ${util.inspect(properties[ID])} from source at context property '${source}'; this is not allowed, and there is probably a bug in your code. Please file a ticket if you have a good usecase for null-ish results!`);
+					throw new Error(`Must specify a dlayer-source ID`);
-							} else if (properties[AllowErrors] === true && typeof selector !== "string") {
+				} else {
-								// Custom selectors always receive the Result as-is
+					let result = await Result.wrapAsync(() => dataSource.load(sourceID));
-								return selector(result);
-							} else if (result.isError) {
-								if (properties[AllowErrors] === true) {
-									return undefined;
-								} else {
-									// This is equivalent to a `throw`, and so we just propagate it
-									return result;
-								}
-							} else {
-								// This is to support property name shorthand used in place of a selector function
-								if (typeof selector === "string") {
-									return result.value()[selector];
-								} else {
-									return selector(result.value(), context);
-								}
-							}
-						});
-					};
-
-					return [ property, getter ];
-				});
-			}),
-			(_) => Object.fromEntries(_)
-		]);
-	
-		// NOTE: We always specify the generated properties first, so that properties can be overridden by explicit known values to bypass the source lookup, if needed by the implementation
-		return {
-			... generatedProperties,
-			... rest
-		};
-	},
-	ID: ID,
-	AllowErrors: AllowErrors
-};
 
+					// The AllowErrors option is set when a source definition has its own way to deal with (allowable) errors. Instead of simply propagating the error for all affected attributes, it calls the attribute handlers with the Result (or returns `undefined` if only a property is specified).
+					// TODO: How to deal with null results? Allow them or not? Make it an option?
 
+					if (result.isOK && result.value() == null) {
+						// TODO: Change implementation to allow `Result.ok(null|undefined)` but not `null|undefined` directly?
+						throw new Error(`Null-ish result returned for ID ${util.inspect(properties[ID])} from source at context property '${source}'; this is not allowed, and there is probably a bug in your code. Please file a ticket if you have a good usecase for null-ish results!`);
+					} else if (allowErrors === true && isCustomSelector) {
+						// Custom selectors always receive the Result as-is (note that this has to come before the error case handling!)
+						return selector(result, context);
+					} else if (result.isError) {
+						if (allowErrors === true) {
+							// TODO: Does this actually make sense?
+							return undefined;
+						} else {
+							// This is equivalent to a `throw`, and so we just propagate it
+							return result;
+						}
+					} else if (isCustomSelector) {
+						return selector(result.value(), context);
+					} else {
+						// This is to support property name shorthand used in place of a selector function
+						return result.value()[selector];
+					}
+				}
+			}
+		];
+	});
+};
 
+Object.assign(module.exports, {
+	ID: ID,
+	AllowErrors: AllowErrors
+});

src/packages/map-tree/index.js

@@ -1,18 +0,0 @@
-"use strict";
-
-// FIXME: Finish this later
-
-const { validateArguments } = require("@validatem/core");
-const isString = require("@validatem/is-string");
-const isFunction = require("@validatem/is-function");
-const required = require("@validatem/required");
-
-module.exports = function mapTree(tree, predicate, childrenProperty) {
-	validateArguments(arguments, {
-		tree: [ required ],
-		predicate: [ required, isFunction ],
-		childrenProperty: [ isString ]
-	});
-
-	
-};

src/packages/sysquery-block-devices/index.js

@@ -6,7 +6,6 @@ const matchValue = require("match-value");
 const memoizee = require("memoizee");
 const unreachable = require("@joepie91/unreachable")("@sysquery/block-devices");
 
-// TODO: Refactor dlayerSource to be object-mergeable instead of all-encompassing
 const dlayerSource = require("../dlayer-source");
 const All = require("../graphql-interface/symbols/all");
 const lsblk = require("../exec-lsblk");
@@ -52,28 +51,26 @@ module.exports = {
 	},
 	types: {
 		"sysquery.blockDevices.BlockDevice": function ({ name, path }) {
-			return dlayerSource.withSources({
+			return {
-				$sources: {
+				... dlayerSource("lsblk", {
-					lsblk: {
+					[dlayerSource.ID]: { name, path },
-						[dlayerSource.ID]: { name, path },
+					name: "name",
-						name: "name",
+					path: (device) => fs.realpath(device.path),
-						path: (device) => fs.realpath(device.path),
+					type: (device) => matchValue(device.type, {
-						type: (device) => matchValue(device.type, {
+						partition: "PARTITION",
-							partition: "PARTITION",
+						disk: "DISK",
-							disk: "DISK",
+						loopDevice: "LOOP_DEVICE"
-							loopDevice: "LOOP_DEVICE"
+					}),
-						}),
+					size: "size",
-						size: "size",
+					mountpoint: "mountpoint", // FIXME: Isn't this obsoleted by `mounts`?
-						mountpoint: "mountpoint", // FIXME: Isn't this obsoleted by `mounts`?
+					deviceNumber: "deviceNumber",
-						deviceNumber: "deviceNumber",
+					removable: "removable",
-						removable: "removable",
+					readOnly: "readOnly",
-						readOnly: "readOnly",
+					children: (device, { $make }) => device.children.map((child) => {
-						children: (device, { $make }) => device.children.map((child) => {
+						return $make("sysquery.blockDevices.BlockDevice", { name: child.name });
-							return $make("sysquery.blockDevices.BlockDevice", { name: child.name });
+					})
-						})
+				})
-					}
+			};
-				}
-			});
 		}
 	},
 	extensions: {

src/packages/sysquery-lvm/index.js

@@ -66,28 +66,26 @@ module.exports = {
 	},
 	types: {
 		"sysquery.lvm.PhysicalVolume": function PhysicalVolume({ path }) {
-			return dlayerSource.withSources({
+			return {
-				$sources: {
+				... dlayerSource("physicalVolumes", {
-					physicalVolumes: {
+					[dlayerSource.ID]: path,
-						[dlayerSource.ID]: path,
+					path: "path",
-						path: "path",
+					format: "format",
-						format: "format",
+					totalSpace: "totalSpace",
-						totalSpace: "totalSpace",
+					freeSpace: "freeSpace",
-						freeSpace: "freeSpace",
+					isExported: "isExported",
-						isExported: "isExported",
+					isMissing: "isMissing",
-						isMissing: "isMissing",
+					isAllocatable: "isAllocatable",
-						isAllocatable: "isAllocatable",
+					isDuplicate: "isDuplicate",
-						isDuplicate: "isDuplicate",
+					isUsed: "isUsed",
-						isUsed: "isUsed",
+					volumeGroup: (volume, { $make }) => {
-						volumeGroup: (volume, { $make }) => {
+						return $make("sysquery.lvm.VolumeGroup", { name: volume.volumeGroup });
-							return $make("sysquery.lvm.VolumeGroup", { name: volume.volumeGroup });
-						}
 					}
-				}
+				})
-			});
+			};
 		},
 		"sysquery.lvm.VolumeGroup": function VolumeGroup({ name }) {
-			return dlayerSource.withSources({
+			return {
 				physicalVolumes: function (_args, { physicalVolumes, $make }) {
 					return Promise.try(() => {
 						return physicalVolumes.load(All);
@@ -106,28 +104,26 @@ module.exports = {
 						return $make("sysquery.lvm.LogicalVolume", { path: volume.path });
 					});
 				},
-				$sources: {
+				... dlayerSource("volumeGroups", {
-					volumeGroups: {
+					[dlayerSource.ID]: name,
-						[dlayerSource.ID]: name,
+					name: "name",
-						name: "name",
+					totalSpace: "totalSpace",
-						totalSpace: "totalSpace",
+					freeSpace: "freeSpace",
-						freeSpace: "freeSpace",
+					physicalVolumeCount: "physicalVolumeCount",
-						physicalVolumeCount: "physicalVolumeCount",
+					logicalVolumeCount: "logicalVolumeCount",
-						logicalVolumeCount: "logicalVolumeCount",
+					snapshotCount: "snapshotCount",
-						snapshotCount: "snapshotCount",
+					isReadOnly: "isReadOnly",
-						isReadOnly: "isReadOnly",
+					isResizeable: "isResizeable",
-						isResizeable: "isResizeable",
+					isExported: "isExported",
-						isExported: "isExported",
+					isIncomplete: "isIncomplete",
-						isIncomplete: "isIncomplete",
+					allocationPolicy: "allocationPolicy",
-						allocationPolicy: "allocationPolicy",
+					mode: "mode"
-						mode: "mode"
+				})
-					}
+			};
-				}
-			});
 		},
 		"sysquery.lvm.LogicalVolume": function LogicalVolume({ path }) {
-			return dlayerSource.withSources({
+			return {
-				$sources: {
+				... dlayerSource("logicalVolumes", {
 					logicalVolumes: {
 						[dlayerSource.ID]: path,
 						path: "path",
@@ -176,8 +172,8 @@ module.exports = {
 							return $make("sysquery.lvm.VolumeGroup", { name: volume.volumeGroup });
 						}
 					}
-				}
+				})
-			});
+			};
 		}
 	}
 };

src/packages/sysquery-mounts/index.js

@@ -45,37 +45,35 @@ module.exports = {
 	},
 	types: {
 		"sysquery.mounts.Mount": function ({ mountpoint }) {
-			return dlayerSource.withSources({
+			return {
 				mountpoint: mountpoint,
-				$sources: {
+				... dlayerSource("findmnt", {
-					findmnt: {
+					[dlayerSource.ID]: mountpoint,
-						[dlayerSource.ID]: mountpoint,
+					id: "id",
-						id: "id",
+					// FIXME: Aren't we inferring the below somewhere else in the code, using the square brackets?
-						// FIXME: Aren't we inferring the below somewhere else in the code, using the square brackets?
+					type: (mount) => (mount.rootPath === "/")
-						type: (mount) => (mount.rootPath === "/")
+						? "ROOT_MOUNT"
-							? "ROOT_MOUNT"
+						: "SUBMOUNT",
-							: "SUBMOUNT",
+					filesystem: "filesystem",
-						filesystem: "filesystem",
+					options: "options",
-						options: "options",
+					label: "label",
-						label: "label",
+					uuid: "uuid",
-						uuid: "uuid",
+					partitionLabel: "partitionLabel",
-						partitionLabel: "partitionLabel",
+					partitionUUID: "partitionUUID",
-						partitionUUID: "partitionUUID",
+					deviceNumber: "deviceNumber",
-						deviceNumber: "deviceNumber",
+					sourceDevicePath: "sourceDevice",
-						sourceDevicePath: "sourceDevice",
+					totalSpace: "totalSpace",
-						totalSpace: "totalSpace",
+					freeSpace: "freeSpace",
-						freeSpace: "freeSpace",
+					usedSpace: "usedSpace",
-						usedSpace: "usedSpace",
+					rootPath: "rootPath",
-						rootPath: "rootPath",
+					taskID: "taskID",
-						taskID: "taskID",
+					optionalFields: "optionalFields",
-						optionalFields: "optionalFields",
+					propagationFlags: "propagationFlags",
-						propagationFlags: "propagationFlags",
+					children: (mount, { $make }) => mount.children.map((child) => {
-						children: (mount, { $make }) => mount.children.map((child) => {
+						return $make("sysquery.mounts.Mount", { mountpoint: child.mountpoint });
-							return $make("sysquery.mounts.Mount", { mountpoint: child.mountpoint });
+					})
-						})
+				})
-					}
+			};
-				}
-			});
 		},
 	},
 	extensions: {