From b33cc345508a8dd078f5d773a0e9a8b6e02eeb42 Mon Sep 17 00:00:00 2001 From: Sven Slootweg Date: Tue, 27 Jun 2023 15:10:13 +0200 Subject: [PATCH] Internal refactor of dlayer, to make feature changes easier --- src/packages/dlayer/cursor.js | 57 +++++++--- src/packages/dlayer/index.js | 195 ++++++++++++++++++++-------------- 2 files changed, 160 insertions(+), 92 deletions(-) diff --git a/src/packages/dlayer/cursor.js b/src/packages/dlayer/cursor.js index e41d22d..92d016a 100644 --- a/src/packages/dlayer/cursor.js +++ b/src/packages/dlayer/cursor.js @@ -2,22 +2,54 @@ // Simple data type to represent a query path and corresponding schema path tied together, because these are basically always used together, and it would bloat up the implementation code otherwise -function createInstance({ queryPath, schemaPath, queryObject, schemaObject }) { - return { +function createInstance({ queryPath, schemaPath, queryObject, schemaObject, parent }) { + let self; + // eslint-disable-next-line no-return-assign + return self = { queryPath: queryPath, schemaPath: schemaPath, query: queryObject, schema: schemaObject, - child: function (queryKey, schemaKey, { queryOverride, schemaOverride } = {}) { + child: function (queryKey) { + let newQueryPath = (queryKey != null) + ? queryPath.concat([ queryKey ]) + : queryPath; + + let newQueryObject = (queryKey != null) + ? queryObject[queryKey] + : queryObject; + + // TODO: Is this correct even when the queryKey is null, and so we remain at the same object? + // $key is used for handling aliases + let effectiveSchemaKey = (newQueryObject?.$key != null) + ? newQueryObject.$key + : queryKey; + + let newSchemaPath = (effectiveSchemaKey != null) + ? schemaPath.concat([ effectiveSchemaKey ]) + : schemaPath; + + let newSchemaObject = (effectiveSchemaKey != null) + ? schemaObject[effectiveSchemaKey] + : schemaObject; + + return createInstance({ + queryPath: newQueryPath, + schemaPath: newSchemaPath, + queryObject: newQueryObject, + schemaObject: newSchemaObject, + parent: self + }); + }, + parent: parent, + override: function ({ query, schema }) { return createInstance({ - queryPath: (queryKey != null) - ? queryPath.concat([ queryKey ]) - : queryPath, - schemaPath: (schemaKey != null) - ? schemaPath.concat([ schemaKey ]) - : schemaPath, - queryObject: queryOverride ?? queryObject[queryKey], - schemaObject: schemaOverride ?? schemaObject[schemaKey] + queryPath: queryPath, + schemaPath: schemaPath, + queryObject: query ?? queryObject, + schemaObject: schema ?? schemaObject, + // An override doesn't change the path, so the parent shouldn't change either + parent: self.parent }); }, toPathString: function () { @@ -40,6 +72,7 @@ module.exports = function createCursor({ query, schema }) { queryPath: [], schemaPath: [], queryObject: query, - schemaObject: schema + schemaObject: schema, + parent: undefined }); }; diff --git a/src/packages/dlayer/index.js b/src/packages/dlayer/index.js index d747a88..00341c4 100644 --- a/src/packages/dlayer/index.js +++ b/src/packages/dlayer/index.js @@ -2,6 +2,7 @@ const Promise = require("bluebird"); const mapObject = require("map-obj"); +const syncpipe = require("syncpipe"); const Result = require("../result"); const createCursor = require("./cursor"); @@ -14,6 +15,21 @@ const createCursor = require("./cursor"); // FIXME: Allow setting an evaluation depth limit for queries, to limit eg. recursion // FIXME: recurseDepth, recurseLabel/recurseGoto +/* Data structure design: + +- The process starts with: + 1. A query tree, a nested object representing the query from the user + 2. A schema tree, an abstract tree of nested objects specified by the API; notably, the full tree is not known upfront, and parts may be discovered asynchronously or even generated dynamically + 3. A cursor pointing at the root +- All of these objects are immutable, including the cursor (a child cursor is created, you don't mutate the existing cursor) +- The cursor is an object that represents a specific position in the query *and* schema tree; it has no understanding of the API structure, nor any ability to evaluate any handlers. All it does is keep track of what part of the query and schema tree we're currently dealing with, accounting for aliases where appropriate, and ensuring that the two pointers move in tandem. +- We recurse into the query tree according to the query the user specified, moving along in the schema tree accordingly. Branches that are specified in the schema tree but not in the query tree, will not be visited. +- At each step along the way, we have a cursor pointing at the schema item being processed currently; that schema item can be some static subtree, or eg. a value generated by a previous handler. +- From that cursor, for each step, we generate an 'instruction'; this is a parsed representation of that query item's querying rules, as well as information on what to do with the result once that item has been evaluated. This instruction is used to evaluate the relevant handler and then continue recursing with child rules (or, in the case of an actual recursive query, duplicating the previous rules). +- From the result, we then generate a new cursor for the child items. And so on, and so forth. + +*/ + /* Recursion design: When setting `$recurse: true` on a child property, the parent schema gets duplicated with the child schema merged into it, and the resulting combined schema is used for the recursive fetching. Because the child schema can explicitly set properties to false, this allows for both "fetch in parent but not in recursed children" cases (true in parent, false in child) and "fetch in recursed children but not in parent" cases (unspecified or false in parent, true in child). The schema merging will eventually become deep-merging, when multi-level recursion is implemented (ie. the possibility to recurse indirectly). @@ -80,15 +96,15 @@ function analyzeSubquery(subquery) { return { isRecursive, allowErrors, hasChildKeys, isLeaf, args }; } -function analyzeQueryKey(cursor, queryKey) { - let childCursor = cursor.child(queryKey, null); - let schemaKey = childCursor.query?.$key ?? queryKey; // $key is for handling aliases - let handler = cursor.child(queryKey, schemaKey).schema ?? cursor.schema.$anyKey; +function makeInstruction(cursor, queryKey) { + // let schemaKey = cursor.childQuery(queryKey)?.$key ?? queryKey; // $key is for handling aliases + let childCursor = cursor.child(queryKey); + let handler = childCursor.schema ?? cursor.schema.$anyKey; // TODO: Maybe clean up all the .child stuff here by moving the `$key` logic into the cursor implementation instead, as it seems like nothing else in dlayer needs to care about the aliasing return { ... analyzeSubquery(childCursor.query), - schemaKey: schemaKey, + cursor: childCursor, handler: handler }; } @@ -101,87 +117,104 @@ function assignErrorPath(error, cursor) { } } -// MARKER: build a sample todo-list schema for testing out fetches, mutations, and combinations of them, including on collections - -function evaluate(cursor, context) { - // map query object -> result object - return asyncMapObject(cursor.query, (queryKey, subquery) => { - let shouldFetch = (subquery !== false); - - if (!shouldFetch || specialKeyRegex.test(queryKey)) { - // When constructing the result object, we only care about the 'real' keys, not about special meta-keys like $key; those get processed in the actual resolution logic itself. - return mapObject.mapObjectSkip; - } else { - let { schemaKey, handler, args, isRecursive, allowErrors, isLeaf } = analyzeQueryKey(cursor, queryKey); - - if (handler != null) { - let promise = Promise.try(() => { - // This calls the data provider in the schema - return Result.wrapAsync(() => maybeCall(handler, [ args, context ], cursor.schema)); - }).then((result) => { - if (result.isOK) { - let value = result.value(); - - return Promise.try(() => { - if (!isLeaf && value != null) { - let effectiveSubquery = (isRecursive) - ? { ... cursor.query, ... subquery } - : subquery; - - return mapMaybeArray(value, (item, i) => { - // NOTE: We're adding `i` to the query path for user feedback purposes, but we're not *actually* diving down into that property on the query object; the queryOverride doesn't just handle recursion, it also ensures that the 'original' subquery is passed in regardless of what the path suggests - // TODO: schemaOverride here is used to pass in the (asynchronously/lazily) resolved result, which the cursor implementation wouldn't have access to otherwise; need to somehow make it clearer in the API design that the automatic 'schema navigation' is only used for simple objects - maybe not call it an 'override' but instead just something like newSchema and newQuery? - let subCursor = (i != null) - ? cursor - .child(queryKey, schemaKey) - .child(i, i, { - queryOverride: effectiveSubquery, - schemaOverride: item - }) - : cursor - .child(queryKey, schemaKey, { - queryOverride: effectiveSubquery, - schemaOverride: item - }); - - return evaluate(subCursor, context); - }); - } else { - // null / undefined are returned as-is, so are leaves - return value; - } - }).then((evaluated) => { - // FIXME: Verify that this is still necessary here - if (allowErrors) { - return Result.ok(evaluated); - } else { - return evaluated; - } - }); - } else { - let error = result.error(); - - if (error.__dlayerAcceptableError === true) { - if (allowErrors === true) { - return Result.error(error.inner); - } else { - throw error.inner; - } +// TODO: build a sample todo-list schema for testing out fetches, mutations, and combinations of them, including on collections + +function makeEnvironment(context) { + function callHandler(instruction) { + // NOTE: cursor is assumed to already be the key of the child + let { schemaKey, handler, args, allowErrors, cursor } = instruction; + + if (handler != null) { + return Promise.try(() => { + // This calls the data provider in the schema + return Result.wrapAsync(() => maybeCall(handler, [ args, context ], cursor.parent.schema)); + }).then((result) => { + if (result.isOK) { + return result.value(); + } else { + let error = result.error(); + + if (error.__dlayerAcceptableError === true) { + if (allowErrors === true) { + return Result.error(error.inner); } else { - throw error; + throw error.inner; } + } else { + throw error; } - }).tapCatch((error) => { - // FIXME: Chain properly - assignErrorPath(error, cursor.child(queryKey, schemaKey)); + } + }).tapCatch((error) => { + // FIXME: Chain properly + assignErrorPath(error, cursor); + }); + } else { + throw new Error(`No key '${schemaKey}' exists in the schema`); + } + } + + // FIXME: instruction abstraction? + + function applyToResultValue(instruction, value, query) { + let { cursor } = instruction; + + if (Array.isArray(value)) { + return Promise.map(value, (item, i) => { + let itemCursor = cursor + .child(i, i) + .override({ + query: query, + schema: item + }); + + return applyRules(itemCursor); + }); + } else { + let itemCursor = cursor + .override({ + query: query, + schema: value }); - return [ queryKey, promise ]; + return applyRules(itemCursor); + } + } + + function applyRules(cursor) { + // map query object -> result object + return asyncMapObject(cursor.query, (queryKey, subquery) => { + let shouldFetch = (subquery !== false); + + if (!shouldFetch || specialKeyRegex.test(queryKey)) { + // When constructing the result object, we only care about the 'real' keys, not about special meta-keys like $key; those get processed in the actual resolution logic itself. + return mapObject.mapObjectSkip; } else { - throw new Error(`No key '${schemaKey}' exists in the schema`); + // FIXME: This is hacky, and should be made more ergonomic... + return [ + queryKey, + Promise.try(async () => { + let instruction = makeInstruction(cursor, queryKey); + let value = await callHandler(instruction); + + let effectiveSubquery = (instruction.isRecursive) + ? { ... cursor.query, ... subquery } + : subquery; + + let finalValue = (instruction.isLeaf || value == null) + ? value + : applyToResultValue(instruction, value, effectiveSubquery); + + // FIXME: We're absorbing Result.errors here, but that's a bit weird. We should probably be consistently carrying Result values throughout the implementation, and only unwrap them at the last moment? + return (instruction.allowErrors) + ? Result.ok(finalValue) + : finalValue; + }) + ]; } - } - }); + }); + } + + return applyRules; } module.exports = function createDLayer(options) { @@ -233,8 +266,10 @@ module.exports = function createDLayer(options) { schema: options.schema }); + let evaluate = makeEnvironment(combinedContext); + // FIXME: Currently, top-level errors do not get a path property assigned to them, because that assignment happens on nested calls above - return evaluate(cursor, combinedContext); + return evaluate(cursor); } }; };