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

Accessing to resolver metadata in middleware #123

Open
erencay opened this issue Aug 10, 2018 · 14 comments
Open

Accessing to resolver metadata in middleware #123

erencay opened this issue Aug 10, 2018 · 14 comments
Labels
Community 👨‍👧 Something initiated by a community Enhancement 🆕 New feature or request
Milestone

Comments

@erencay
Copy link

erencay commented Aug 10, 2018

Providing metadata/resolver information into a middleware operation might have some benefits for extended use cases.

Example Use Case:
In this example I wanted to change level field to a fixed value if user is not an admin.

Decorator example;

function ValueUnlessAdmin<T>(val: any) {
    return function(inputCls: any, field: any) {
        Reflect.defineMetadata("valueUnlessAdmin", val, inputCls.constructor, field);
    };
}

InputType example;

@InputType()
class CreateUserInput {
    @Field()
    @ValueUnlessAdmin("member") // Creates metadata
    level: string;
}

Middleware example;

class SetValues implements MiddlewareInterface<Cntx> {
    async use({ context, args }: ResolverData<Cntx>, next: NextFn) {
        // At this point one might want to use metadata
        return next();
    }
}

export const schema = buildSchemaSync({
    resolvers: [UserResolver],
    globalMiddlewares: [SetValues],
});

Here's a suggested workaround by @19majkel94

> technically you can create a class decorator to mark an input class that it has special decorator like @ValueUnlessAdmin
> they both can use Reflect.defineMetadata to store some data attached
> and then in middleware you can browse the info object to search if the mutation use an input type with this custom decorator
> middlewares don't know if next is a middleware or a resolver so they have no access to compiled resolver params
> so you can't get array of params with instances of classes and make some checks
> and input type field have no middlewares or guards and input types are only data transfer object, they're not executed, no logic there

This workaround did the trick. That being said, it could be easier to realize such intentions and may be more if metadata/resolver information can be accessed in the middleware.

@MichalLytek MichalLytek added Enhancement 🆕 New feature or request Community 👨‍👧 Something initiated by a community Discussion 💬 Brainstorm about the idea labels Aug 10, 2018
@MichalLytek MichalLytek added this to the Future release milestone Aug 10, 2018
@MichalLytek
Copy link
Owner

Here is a quick snippet with my vision of the API:

const setValues: MiddlewareFn<Cntx> = ({ context, args, resolverMetadata }, next) => {
  // if resolverMetadata is defined, the next in chain is a resolver
  if (resolverMetadata) {
    // clone original args data object
    const customArgs = { ...args };
    for (const argMetadata of resolverMetadata.args) {
      // if arg is an input
      if (argMetadata.isInput) {
        // load custom metadata value from custom storage using InputType class
        const unlessAdminMetadatas = loadMetadataFromCustomStorage(argMetadata.classType);
        // do the logic - replace with the value when not admin
        for (const metadata of unlessAdminMetadatas) {
          if (!context.user.roles.include("admin")) {
            customArgs[argMetadata.argName][metadata.fieldName] = metadata.value;
          }
        }
      }
    }
    // call the next middleware (resolver) with replaced args object
    return next({ args: customArgs });
  }
  // if not, normally call next
  return next();
};

It's just an idea how to implement it exposing the resolverMetadata object, a draft of the metadata.

@MichalLytek
Copy link
Owner

MichalLytek commented Oct 24, 2018

I am revisiting this issue and I am thinking about a different way as injecting built metadata into middlewares is not so easy.

What if:

@ValueUnlessAdmin("member") // Creates metadata
level: string;

would use #124 to add some metadata to the GraphQLObject/InputType field?
This would solve:

At this point one might want to use metadata

Because this should be available in an info property of the middleware. You would have to navigate throught the resolve info tree, find all input types with fields that has provided metadata and do the conversion.

@erencay What do you think? Would it solve your use case? 🤔

@MichalLytek
Copy link
Owner

After a small research, it looks like #124 will allow to attach the custom data to the GraphQLObjectType and others, so it will be easily accessible in middlewares:

export const SampleMiddleware: MiddlewareFn = async ({ info }, next) => {
  const resolverMetadata = info.parentType.getFields()[info.fieldName];
  // here we can access `resolverMetadata.complexity` or other metadata
  return next();
};

I will add this recipe to the docs describing the metadata feature, maybe in the future there will be another argument of middleware function with injected parsed data like this.

@glen-84
Copy link
Contributor

glen-84 commented Dec 28, 2019

My current use case is generating a TypeORM query builder, where I need access to the entity property name when given a schema name. For example, if the schema name is dob, but the entity property is dateOfBirth.

It might also be useful to have access to the object type class, to check if the object implements a Connection (like Relay) interface.

Is there no "supported" method of accessing this data? Should I assume that relying on:

import {getMetadataStorage} from "type-graphql/dist/metadata/getMetadataStorage";

... is Really Bad™?


Regarding your vision of the API, would it not make sense to leave ResolverData as it is (strictly the same as resolvers receive), and instead pass the resolverMetadata as a 3rd argument?

({ context, args }, next, resolverMetadata) => {}

I wasn't initially planning on using middleware, or a custom decorator, because I don't think that it's possible to do dependency injection into a decorator? The usage of my decorator currently looks like this:

@Query(() => User)
public async user(
    @Arg("id") _id: string,
    @QueryBuilder(User, (qb, {args}) => qb.where({id: args.id}))
    qb: SelectQueryBuilder<User>
): Promise<User> {
    const user = await qb.getOne();

    if (!user) {
        throw new Error("User not found.");
    }

    return user;
}

The unused @Arg seems unfortunate ... I assume that there's no way to specify this elsewhere?

I was initially just using a function within the body of the resolver, like this:

const qb = generateQueryBuilder(User, info, {id});

But even with this issue resolved, I would have no access to the resolver metadata.

At the moment the code uses:

import {getRepository} from "typeorm";

... but I wonder if the entity manager should instead be dependency-injected (which I don't think is possible when using a decorator).

Maybe it's possible to inject the entity manager into the resolver, and then pass that as an argument to the decorator, like:

@QueryBuilder(this.entityManager, User, (qb, {args}) => qb.where({id: args.id}))

Or:

@QueryBuilder(this.entityManager.getRepository(User), (qb, {args}) => qb.where({id: args.id}))

(untested)

I'm just a bit concerned that too much is happening within the decorator usage, as opposed to within the resolver body.

@MichalLytek
Copy link
Owner

MichalLytek commented Dec 28, 2019

I was initially just using a function within the body of the resolver

It's the best solution, much more readable, less error-prone, easier to understand as no magic behind, which I can't say about that super weird:

@QueryBuilder(this.entityManager.getRepository(User), (qb, {args}) => qb.where({id: args.id}))

My current use case is generating a TypeORM query builder, where I need access to the entity property name when given a schema name.

What exactly do you want to magically auto generate? Only select optimizations?
With #10 you would just receive the object with list of object type property names that the client has requested, so you could use them directly in the FindOptions or in query builder.

@glen-84
Copy link
Contributor

glen-84 commented Dec 28, 2019

super weird

The 2nd argument is just an optional initialization callback that allows you to set up the query builder (in this example, adding the id condition to the WHERE clause).

I guess one benefit to this is not having to always pull out the GraphQLResolveInfo in each resolver method.

But ya, I wasn't sure if it was the right approach, but you made it sound like middleware would be the only option (although I suppose a simple [global?] middleware could place the resolver metadata in an argument or in the context).

What exactly do you want to magically auto generate? Only select optimizations?

  • The exact fields selected.
  • Possibly the fields depended on by "computed" fields
    • (think fullName depending on firstName and lastName).
  • Left-joined mostly to-one relations and their specific fields.
  • Handling of Relay-like connections and cursor-based (keyset) pagination.
  • Filters/conditions
  • Ordering

With #10 you would just receive the object with list of object type property names that the client has requested

I'm doing this with graphql-parse-resolve-info.

@MichalLytek
Copy link
Owner

but you made it sound like middleware would be the only option

No, that's why I didn't propose to

instead pass the resolverMetadata as a 3rd argument?

But to make them part of the ResolverData so it would be available in parameter decorator, middleware, resolver, etc.

I'm doing this with graphql-parse-resolve-info.

You don't have schema to class property name mapping applied there.

What exactly do you want to magically auto generate?

I would rather use Warthog to have them auto generated under the hood or some common generated/generic args/input types to just pass them to the custom repository in resolver.

@glen-84
Copy link
Contributor

glen-84 commented Dec 29, 2019

You don't have schema to class property name mapping applied there.

I know, that's why I need the resolver metadata.

I would rather use Warthog to have them auto generated under the hood

I'm trying to avoid two things:

  1. Opinionated/inflexible libraries and frameworks: I don't want to be forced to use PostgreSQL, or Yarn, etc., and I want to have control over the queries and data loading, and to be able to use things like connections and keyset pagination.
  2. One-man projects with little to no funding: This is sometimes unavoidable, but in this case I don't specifically need what Warthog offers, so there's no reason to increase risk exposure.

or some common generated/generic args/input types to just pass them to the custom repository in resolver.

I'm not really sure what you mean by this.

@MichalLytek
Copy link
Owner

I'm not really sure what you mean by this.

Generic input or args, like FindOneInput(User) or PaginationArgs(Post) to avoid DRY. Then you can have the generic repository that will take the args, transform to a query and then returns the entity. So you can just call the repo in body of resolver method, instead of trying to hook into args, metadata and inject a prepared query builder.

@glen-84
Copy link
Contributor

glen-84 commented Jan 10, 2020

@MichalLytek,

I think I'd prefer to have more specific input and argument types, similar to Prisma and Warthog.

I guess this means that I'll also have to have a code generation step (similar to Warthog).

However, regardless of this aspect, I still need access to TypeGraphQL metadata, in at least two places: A CLI/code-gen step, and in middleware/decorators in order to access GraphQL -> class property mappings (among other things, potentially) in resolvers.

Which of these options do you suggest that I use?:

  1. Create custom decorators like Warthog does, that wrap TypeGraphQL (and possibly TypeORM) decorators, and maintain their own representation of the metadata.
    • Lots of extra work.
    • Duplication of metadata that already exists.
  2. import {getMetadataStorage} from "type-graphql/dist/metadata/getMetadataStorage";
    • Not officially supported/discouraged/not stable?
  3. Use Reflect.getMetadata to read the "raw" metadata.
    • Potentially unstable, as storage format may change.
  4. Repeat the necessary metadata in @Extensions({fieldAlias: "foo", fieldType: FieldType}).
    • Redundant, ugly.
  5. Wait for a stable API for accessing this metadata in TypeGraphQL.
    • Note: For the code generation case, this needs to be accessible outside of middleware as well.
    • Is this planned? Is it going to happen any time soon, or should I look at other options?

@MichalLytek
Copy link
Owner

I think the point nr 1 is the answer. You provide your own decorators, you translate its options into orm/graphql layer options, store additional info in your metadata storage (own class or Reflect.metadata()) and do the magic you need in build step, middlewares or something.

@glen-84
Copy link
Contributor

glen-84 commented Jan 12, 2020

I'm not convinced that option 1 is ideal, because:

  • It adds an otherwise-unnecessary layer of abstraction over TypeORM and TypeGraphQL.
    • It makes things less explicit, or more "magical".
  • If I'm not mistaken, it prevents you from adding API-only fields, as a single decorator must imply that both a TypeORM column, as well as a GraphQL field, should be added.
    • Warthog suggests that you use the TypeGraphQL decorator in these cases, but surely then you're no longer to able to access the metadata? What if you need to access the GraphQL field name (for example)?
  • As mentioned above, it means re-recording metadata that already exists.
  • My requirements (for now, field name mapping + possibly accessing field types) are too simple to justify creating such a "framework".

Could you please consider providing a formal API for accessing TypeGraphQL metadata (globally and via middleware)?

(BTW: This doesn't really seem like the same request as #123 or #134 – should I create a new issue for this?)

@MichalLytek
Copy link
Owner

MichalLytek commented Jan 12, 2020

Could you please consider providing a formal API for accessing TypeGraphQL metadata

This won't happen in the nearest time as I'm working on vNext with a complete redesign of metadata and pipeline. I have to make the internals stable first before exposing them to the outside world.

it prevents you from adding API-only fields

@ModelField({ columnOnly: true })

@glen-84
Copy link
Contributor

glen-84 commented Jan 12, 2020

I have to make the internals stable first before exposing them to the outside world.

All I ask is that you keep this request in the back of your mind while working on vNext. 🙂

@ModelField({ columnOnly: true })

Ya, I forgot to mention that alternative. The other points still apply though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Community 👨‍👧 Something initiated by a community Enhancement 🆕 New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants