[Suggestion] Filtering resolver properties

[joezappie]

This is in relation to #2595 but I'm making my own thread as this detours from their original request.

The proposed solution is a built in filter list for resolvers. There is one mention in the dove docs for a $resolve property, but no information on it so I'm assuming its up to developers to roll their own at this point. I think this is a common/generic enough feature that should be built into the schema API.

Proposal

I'm proposing two query parameters (though the names probably arnt the best):

• $resolve ($include?) - Pick which resolver properties to run
• $exclude - Run all resolver properties except in this list

These can either be an array of property names, or an object that includes what type of resolver is being run (data/query/result) and its array of properties. This is needed because you may have a query resolver with a result resolver and in that case you need to specify what resolver its for.

Querying Params

{ 
   query: {
       $resolve: ['prop1', 'prop2']
       // OR
       $resolve: {
            result: ['prop1', 'prop2']
       }
    }
}

Both $resolve and $exclude can be provided. Exclude is done as a 2nd pass to modify the selected properties, so in this case it would just further filter down the $resovle list.

Explicit inclusion I also included a helper function hasProperty(). This is useful for if you want to specify a property that should only run if its explicitly listed. Instead of having to list a bunch of resolve properties or specifically exclude the property property, this by default allows that. I'm using something similar where my frontend generally needs less information, but if I'm doing a backend service call I might want more details on top of the typical resolvers.

   customer_type: async (value, object, context) => {
      if (hasProperty(context, 'customer_type')) {
         // include detailed information about customer type only if declared in $resolve
      }
    },

Issues

While this is completely doable in userland, theres two things I don't like about my current solution.

1. Its messy to call the filter hook as it needs context but also needs to be passed to resolveResult/resolveQuery: find: [(context) => resolveResult(filterProperties(resolve({...}))(context))(context)],
2. Type of resolver needs to manually be defined. If this was built into the library, resolveResult could automatically check for the result include/exclude list. resolveResult(filterProperties(resolve({...}), 'result')(context))(context)

Currently Implementation

Tested exclude/include but not for running based on type of resolver.

/**
 * Filter for including properties. Modifies the Resolve's properties object to 
 * only have those in the properties array.
 * @param {Resolve} original The original Resolve instance
 * @param {Resolve} filtered The copied Resolve instance
 * @param {array} properties Array of properties to remove
 */
function filterInclude(original, filtered, properties) {
  if (Array.isArray(properties)) {
    // Clear the properties to add back in only the selected
    filtered.options.properties = {};

    properties.forEach((property) => {
      if (Object.hasOwnProperty.call(original.options.properties, property)) {
        // Add the property back to filtered resolver
        filtered.options.properties[property] = original.options.properties[property];
      }
    });
  } else if (properties === false) {
    // Don't resolve any properties
    filtered.options.properties = {};
  }
}

/**
 * Filter for excluding properties. Modifies the Resolve's properties object to 
 * exclude the ones passed in with the service request.
 * @param {Resolve} filtered The copied Resolve instance
 * @param {array} properties Array of properties to remove
 */
function filterExclude(filtered, properties) {
  if (Array.isArray(properties)) {
    properties.forEach((property) => {
      // Check if the exclude property exists and if so remove it
      if (Object.hasOwn(filtered.options.properties, property) === true) {
        delete filtered.options.properties[property];
      }
    });
  }
}

/**
 * Runs the filter on a Resolve object by copying its object and modifying its 
 * properties depending on the context.
 * @param {Resolve} resolver Instance of resolve
 * @param {string} type Type of Resolve (data/query/result)
 * @returns {function} Filtered resolve result function
 */
export const filterProperties = (resolver, type) => (context) => {
  // Get resolve and exclude properties
  const resolveProperties = context.params?.$resolve?.[type] || context.params?.$resolve;
  const excludeProperties = context.params?.$exclude?.[type] || context.params?.$exclude;

  if (resolveProperties !== undefined || excludeProperties !== undefined) {
    // Make shallow copy of the resolver to modify properties
    const filteredResolver = Object.assign(Object.create(resolver), resolver);
    filteredResolver.options = { ...resolver.options };
    filteredResolver.options.properties = { ...resolver.options.properties };

    // Run the filters
    filterInclude(resolver, filteredResolver, resolveProperties);
    filterExclude(filteredResolver, excludeProperties);

    return filteredResolver;
  }

  return resolver;
};

/**
 * Checks if a property should be resolved based on the context
 * @param {Object} context Feathers context
 * @param {string} property Name of property
 * @param {string} type Type of Resolve (data/query/result)
 * @returns {boolean} If property should be resolved
 */
export const hasProperty = function (context, property, type) {
  let has = property === true;

  // Check the include list
  if (has === false) {
    // Get the property list, optionally by type
    const include = context.params?.$resolve?.[type] || context.params?.$resolve;
    if (Array.isArray(include)) {
      // Check if the property is included
      has = include.includes(property);
    }
  }

  // Check that its not excluded
  if (has) {
    const exclude = context.params?.$exclude?.[type] || context.params?.$exclude;
    if (Array.isArray(exclude)) {
      // Check if the property is excluded
      has = !exclude.includes(property);
    }
  }

  return has;
};

/**
 * Pulls $resolve and $exclude out from query and adds them to params
 * @param {Object} context Feathers context
 * @returns {function} Filtered resolve result function
 */
export const resolveParamsFromClient = (context) => {
  // Pull $resolve and $exclude from query. Save as param
  paramsFromClient('$resolve', '$exclude')(context);
};

/**
 * Wrapper around resolveResult that includes filtering
 * @param {Resolve} resolver Instance of resolve
 * @returns {function} Filtered resolve result function
 */
export const resolveFilteredResult = (resolver) => (context) => resolveResult(filterProperties(resolver)(context))(context);

Note: I did include the resolveFilteredResult hook that wraps the resolveResult method. That does make my original point about it being messy less relevant but I still feel like this is a common enough thing to be warranted. If you're not using it, performance wise it would have a negligible impact.

[daffl]

So after a bit more discussion on Discord today and based on #2900 think that this can all be addressed now very flexibly with a $select query resolver. For example, the below query resolver, excludes the user by default unless explicitly selected (based on the original schema property names) and allows a $select: ['*'] for the standard properties:

const defaultSelect = Object.keys(messageSchema.properties).filter(key => key !== 'user')

export const loginQueryResolver = resolve<MessageQuery, HookContext>({
  properties: {
    $select: async (value) => {
      if (Array.isArray(value)) {
        return value.reduce((current, name) => [
          ...current, ...(name === '*' ? defaultSelect : [name])
        ], [])
      }
      return defaultSelect
    }
  }
})

[joezappie]

@daffl I'm finally getting around to updating to the release and switching my current codebase to use all these new features. I didn't really put much thought into it before as your example looked good.

I would like to make an app wide hook to add in an $unselect feature as that's a more common use case for me. 98% of the time I want to select everything, but sometimes when internally calling other services I want to exclude a resolver because it will result in a circular dependency.

My thought was to add a hook that converts $unselect to the $select notation by inverting it. The issue I'm having is, adding this as an app wide hook to add $unselect for all services, I have no good way of dynamically getting the schema that will be used to grab the properties. I thought about trying to loop through context.self.hooks but since resolveResult and resolveQuery return an anon function there's no good way to check if the hook contains a resolve. I'd also have to call the function, to grab the first parameter which feels hacky. Curious if you have any ideas on this. My other option is to keep doing it the way I'm doing it now and write a wrapper to resolveResult/resolveQuery that I use instead, which is where I'll probably go with this.

A question I have, does $select get passed to mongo for it to filter the document or does feathers filter it after getting the entire document?

I still think an $unselect feature would be generally beneficial. If I submitted a PR to add it, would you consider it? Id rather spend time working on something others might also get use out of then something just for me.

[joezappie]

Looking through the code, I see $select does get passed to the db query. Thinking about this more, an $unselect does pose an issue when it comes to SQL as it doesn't (at least the major ones) support column exclusion, only inclusion (though mongo and an in memory db could support it). It could work by doing the field removal in the final result, but that's probably not ideal.

Does the mongo adapter support the object syntax for $select such as { _id: 0, name: 1} directly? If resolve.ts could support either format, then the mongo notation could be used to exclude a resolver from running instead of a separate $unselect keyword, and could be a mongo only feature. This would be a big change though I'm guessing?

That all said, its my fault for not looking at this closer in December to give feedback when you were going through. I'm totally okay with just writing my own wrapper around resolveResult/resolveQuery, just wanted to put in my 2 cents even if its to late at this point. This issue could be closed now that property filtering is now doable.

[daffl]

I believe this would still be possible, although it'd have to be on the schema/service level since we need to know about all the properties (easier introspection is definitely something I've been thinking about though). Based on my above example, a $unselect might look like this:

const properties = Object.keys(messageSchema.properties)

export const loginQueryResolver = resolve<MessageQuery, HookContext>({
  $unselect: async () => undefined,
  $select: async (value, data) => properties.filter(prop => !(data.$unselect || []).includes(prop))
})

[joezappie]

You're right, better to do that as a resolver than making a wrapper for resolveResult/resolveQuery. I can still turn that into a service hook I call before resolveResult and pass it the schema there. My main concern was that it must be dealt with on the service level, instead of being able to add new global functionality. But since you're thinking about ways of making introspection easier, I'm good to go forward with you're resolver suggestion and consider that resolved.

Edit That said, the only other hang up I have is virtual properties with $select. Since querySyntax takes in the schema and doesn't know about the resolvers, ajv fails to validate when selecting a virtual property. Would be simple to make a conversion function that takes the resolver and spits out a schema with Type.Any for them just for generating the querySyntax, but do you have any thoughts on this? Maybe querySyntax should be able to take in an optional resolver as well?

const schema = Type.Object({
  _id: Type.ObjectId(),
  employeeId: Type.ObjectId(),
 });

const resolver = resolve({
  employee: virtual((object, context) => {
      return context.params.loader.service('api/org/employees').load(object.employeeId);
  }),
});

export const querySchema = Type.Intersect(
  [
    querySyntax(schema),
    Type.Object({}, { additionalProperties: false }),
  ],
  { additionalProperties: false } 
);

Took me a bit to realize this was what's going on.