Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Aggregation #2693

Draft
wants to merge 1 commit into
base: devel
Choose a base branch
from
Draft

Aggregation #2693

wants to merge 1 commit into from

Conversation

ErikDakoda
Copy link
Contributor

  • This PR enables you to query data on the server with a schema that does not match a database collection - for example an aggregation or an array of objects
  • PR for the documentation is here
  • Added new function registerCustomQuery(), to add a custom GraphQL type generated from a schema object and a resolver to query the data
  • Added new function registerCustomDefaultFragment(), to generate a default fragment from the same schema
  • Updated multi2 to be compatible with custom queries - you can now specify a typeName instead of a collection
  • Updated createSchema() to support SimpleSchema Shorthand Definitions
  • Added defaultCanRead parameter to createSchema()

 * This PR enables you to query data on the server with a schema that does not match a database collection - for example an aggregation or an array of objects
 * PR for the documentation is [here](VulcanJS/vulcan-docs#169)
 * Added new function `registerCustomQuery()`, to add a custom GraphQL type generated from a schema object and a resolver to query the data
 * Added new function `registerCustomDefaultFragment()`, to generate a default fragment from the same schema
 * Updated `multi2` to be compatible with custom queries - you can now specify a `typeName` instead of a `collection`
 * Updated `createSchema()` to support [SimpleSchema Shorthand Definitions](https://github.com/aldeed/simpl-schema#shorthand-definitions)
 * Added `defaultCanRead` parameter to `createSchema()`
@ErikDakoda ErikDakoda marked this pull request as draft February 11, 2021 23:51
@ErikDakoda
Copy link
Contributor Author

ErikDakoda commented Feb 11, 2021

@eric-burel Before I complete this I would like your feedback - because it was very useful last time. Originally I was going to call this registerAggregation() but then I realized that it has wider applications than just aggregation - though I am afraid the name registerCustomQuery() is not as clear.

Of course feedback from everybody is requested and welcome.

@eric-burel
Copy link
Contributor

eric-burel commented Feb 12, 2021

Thanks a lot! It will take me some time to review because this is an interesting feature but with lot of consequences. So I'll try to go step by step, first understanding the scope/use cases.

Do I get it correctly if I say that the point of this feature, is to be able to reuse useMulti on those custom data? Because currently we already have the ability to create custom resolvers, and I think we do use that at Live for good for this use case (@EloyID) ?
I guess you can't use a custom multi resolver tied to a collection, because this will have the side effect of creating an empty collection in Mongo?

Note that in Vulcan Next those problem are kind of obsolete, because you are able to create "Models" without Mongo => it's quite easy to create an "Agent" model like in your example whose data is stored in a local array. Instead of creating a custom resolver, you create a custom connector, replacing the db level logic (literraly the model in an MVC approach, instead of the Controller). There are premises of this in Vulcan Meteor but the limitation is that you can't create connectors in user land (eg storing in Redis, storing locally, etc. etc.).

@EloyID
Copy link
Contributor

EloyID commented Feb 12, 2021

Hi, for me this is very useful

I had to create a query only because we wanted to filter by the field of a related collection, and then you lose all the advantages of useMulti. I think I did not code it very nicely so I had cache problems.

The only problem I can see it's that when aggregating, if you have to query all the docs and then pass them to apply new filtering; it won't be very performing for big databases. I don't understand well useMulti, but can't we just give the option to inject some aggregation steps in the useMulti2 query to create the type of documents we want and then we let the input filters work ?

Hope it helps, nice feature!

@eric-burel
Copy link
Contributor

eric-burel commented Feb 12, 2021

This would raise security issues, you don't want users to be able to compute any kind of aggregates based on client-side inputs. Technically we could, but you'd need to create a whole new layer of security to check if the user is allowed to compute some aggregations, and I think it would be extremely difficult/maybe not even possible. So you have to do it server side and create a new resolver. Aggregations are quite complex in Mongo.

@EloyID
Copy link
Contributor

EloyID commented Feb 12, 2021

But as you can alreday create hasOne or hasMany relationship you could config the fields of the related collection to be also filterable

@Neobii
Copy link
Contributor

Neobii commented Mar 5, 2021

If using $lookup is one of the main use cases, perhaps it can be tied into the relations part of the schema definition?

@ErikDakoda
Copy link
Contributor Author

ErikDakoda commented Mar 5, 2021

@EloyID

The only problem I can see it's that when aggregating, if you have to query all the docs and then pass them to apply new filtering; it won't be very performing for big databases. I don't understand well useMulti, but can't we just give the option to inject some aggregation steps in the useMulti2 query to create the type of documents we want and then we let the input filters work ?

Actually you can pass an input prop to the aggregation, like you can to any multi query, which can include a filter:

const input = {
  filter: {
    supplierCode: { _eq: "190" }
  },
};
const queryOptions = {
  typeName: 'ScrubErrorsSummaryAggregate',
  fragmentName: 'ScrubErrorsSummaryAggregateFragment',
  input,
};
const useMultiResults = useMulti2(queryOptions);

The input is passed to the custom query resolver, where you can use it to modify your aggregation:

const resolver = async function (root, { input }, context) {
  const { filter } = input;
  const { currentUser } = context;
  
  const pipeline = [
    // pipeline stages
  ];
  
  if (filter?.supplierCode?._eq) {
    pipeline.splice(0, 0, {
      $match: {
        supplierId: filter.supplierCode._eq,
      },
    });
  }
  
  let docs = await Products.aggregateAsync(pipeline);
  // enforce doc and field-level permissions if necessary
  return { results: docs, totalCount: docs.length };
};

const filterable = [{
  name: 'supplierCode',
  type: 'String',
}];

registerCustomQuery({
  typeName: 'ScrubErrorsSummaryAggregate',
  description: 'Aggregate products grouped and scrub error',
  schema: scrubErrorsSummaryAggregateSchema,
  resolver,
  filterable,
});

I will probably add a full working example in Vulcan Starter.

@Neobii Right now you have to write the pipeline stages yourself, but you are right, in the future we could maybe auto-generate them using relations and other info in the schema.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants