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

Allow declaration of custom ID scalar type #1058

Closed
hrmcdonald opened this issue Apr 12, 2021 · 9 comments
Closed

Allow declaration of custom ID scalar type #1058

hrmcdonald opened this issue Apr 12, 2021 · 9 comments
Labels
enhancement New feature or request in progress

Comments

@hrmcdonald
Copy link

hrmcdonald commented Apr 12, 2021

Is your feature request related to a problem? Please describe.

We would like to be able to specify a custom "ID" scalar type other than the default GraphQL ID scalar type. The default CRUD input types nestjs-query generates are fine other than the ID field automatically placed on a few types. This would prevent us from the mess of having to declare or generate custom input types for few different operations.

Have you read the Contributing Guidelines?

yes!

Describe the solution you'd like

The default input type class factories already accept inputs. It should be relatively easy to pass an ID type from the resolver options to these factories.

I believe this would really only affect the generation of:

  • DeleteOneInputType
  • UpdateOneInputType
  • RelationInputType
  • RelationsInputType
  • FindOneArgsType

Describe alternatives you've considered

I've built out an pipe that inspects value classes and uses the NestJS TypeMetadataStorage information on InputType values to determine when an argument or any of its child properties are ID types. I can then serialize and deserialize these properties before they hit any handlers. The problem with this is that besides the obvious hacky-ness of this solution, it's quite inefficient.

Additional context

I'll investigate what it would take to create a PR for this feature myself soon if you think this is something you'd be willing to include in the library, but I wanted to get a ticket up first.


btw - This is such an amazingly well made library. I'd love to contribute monetarily to its continued development if you ever enabled something like that!

@hrmcdonald hrmcdonald added the enhancement New feature or request label Apr 12, 2021
@doug-martin
Copy link
Owner

@hrmcdonald thank you for submitting an enhacement request!

This makes sense to me, I'm curious (if you can share) what is your ID scalar type? I like trying to keep everything type safe and currently the services all expect string | number for ids.

@hrmcdonald
Copy link
Author

hrmcdonald commented Apr 13, 2021

There are actually two variants I am interested in implementing, one that simply maps strings from the client to numbers on the API. I implemented a variant of the nestjs-query query service interface for objection.js because it is our ORM of choice (we're potentially open to considering contributing this to the main repo if you are interested). Given that a lot of projects simply use auto incrementing IDs, having this scalar would help us remain type safe (even though most DBs do the type conversion for this basic scenario anyways, it can make things like testing more straight forward).

An iteration of that basic scalar I am interested in trying out however is one that uses something like a feistel cipher to obfuscate auto-incrementing IDs for added security so they cannot be scraped or guessed from the client in certain circumstances where that matters.

So neither of those would break the string | number expectation you mentioned, it would just enable us to easily map IDs to a different format for the client.

@ishwinder
Copy link

ishwinder commented May 2, 2021

+1 to feature request. We also serialize the ids as strings in our responses to obfuscate the auto increment ids. Right now we have to override the find by id resolver for every entity since the framework doesn't give us the flexibility to override FindOneArgsType to accept the id as scalar which we can serialize/deserialize as per our logic.

@ishwinder
Copy link

ishwinder commented May 2, 2021

@hrmcdonald can you help with sharing the interceptor if its not too much proprietary code ?

@hrmcdonald
Copy link
Author

hrmcdonald commented May 6, 2021

Sure thing, I'm hoping to set aside some time to put together a PR for this feature within the next couple of weeks.

@ishwinder sorry I mispoke when saying interceptor. It is actually a pipe. This might not be flexible enough, but you can try to tweak it to your needs:

import { PipeTransform, Injectable, ArgumentMetadata, Type } from '@nestjs/common';
import { TypeMetadataStorage, ID, InputType } from '@nestjs/graphql';
import { ObjectTypeMetadata } from '@nestjs/graphql/dist/schema-builder/metadata/object-type.metadata';
import * as nanomemoize from 'nano-memoize';

/**
 * NestJS Pipe that inspects an argument's type.
 * If it is an input type, the metadata will be pulled and inspected.
 * If there is an ID field, the pipe will attempt to parse it to a number.
 * IDs that cannot be parsed into numbers will passively fail and be returned as is.
 */
@Injectable()
export class ParseIDPipe implements PipeTransform {
  // Using classic for loops in this pipe because this is called a lot and they are faster

  transform(value: any, metadata: ArgumentMetadata) {
    const inputMeta = this.retrieveInputTypeMetadata(
      metadata.metatype,
      metadata.metatype.name
    );
    if (inputMeta) {
      return this.scanTypeMetadata(inputMeta, value);
    }
    return value;
  }

  /**
   * Recursively traverse type information merging inherited property metadata on the way
   * back up. The name is included as a param to support easy memoization.
   */
  retrieveInputTypeMetadata = nanomemoize((inputTypeClass: Type<any>, name: string) => {
    const metadata = TypeMetadataStorage.getInputTypeMetadataByTarget(inputTypeClass);
    if (metadata?.target) {
      const inheritedMetadata = this.retrieveInputTypeMetadata(
        (metadata.target as any).__proto__,
        (metadata.target as any).__proto__.name
      );
      if (inheritedMetadata) {
        const currentProps = [];
        for (let i = 0; i < metadata.properties.length; i++) {
          currentProps.push(metadata.properties[i]);
        }
        for (let i = 0; i < inheritedMetadata.properties.length; i++) {
          const checkProp = inheritedMetadata.properties[i];
          if (currentProps.indexOf(checkProp.name) === -1) {
            metadata.properties.push(checkProp);
          }
        }
        return metadata;
      }
      return metadata;
    }
    return metadata;
  });

  /** Scan all of the properties of a value for ID fields and InputType child fields */
  scanTypeMetadata(metadata: ObjectTypeMetadata, value: any) {
    if (value !== null && value !== undefined) {
      for (let i = 0; i < metadata.properties.length; i++) {
        const prop = metadata.properties[i];
        // Check if this field is an ID type
        const childSchemaType = prop.typeFn();
        if (childSchemaType === ID) {
          // [ID] types get set as ID in the metadata, we can infer an array from the object itself
          if (Array.isArray(value[prop.name])) {
            for (let j = 0; j < value[prop.name].length; j++) {
              value[prop.name][j] = this.parseIdValue(value[prop.name][j]);
            }
          } else {
            value[prop.name] = this.parseIdValue(value[prop.name]);
          }
        } else {
          // Recurse over child InputTypes
          const childInputMeta = this.retrieveInputTypeMetadata(
            childSchemaType as Type<any>,
            (childSchemaType as Type<any>).name
          );
          if (childInputMeta) {
            this.scanTypeMetadata(childInputMeta, value[prop.name]);
          }
        }
      }
    }

    return value;
  }

  /**
   * Safely attempt to parse a value to an integer. If that fails
   * just pass the value back as is to avoid further issues
   */
  parseIdValue(value) {
    if (value) {
      try {
        return parseInt(value, 10);
      } catch (e) {
        return value;
      }
    }
  }
}

@ishwinder
Copy link

ah .. scalar yeah, I do have a scalar for id as well going, I thought you had a pipe to intercept value classes and inspects the information on InputType values to determine when an argument or any of its child properties are ID types and serialize/deserialize them before handler is it.

@hrmcdonald
Copy link
Author

ah .. scalar yeah, I do have a scalar for id as well going, I thought you had a pipe to intercept value classes and inspects the information on InputType values to determine when an argument or any of its child properties are ID types and serialize/deserialize them before handler is it.

What a coincidence, I just reread your comment and realized I misunderstood before. I just updated my comment above with what you were looking for!

@doug-martin
Copy link
Owner

@hrmcdonald @ishwinder I realized I never went back and marked this issue as in progress (sorry about that). I hope to have the PR out later today or tomorrow.

To keep the number of potential ID type mismatches and option passing down to a minimum I added a new IDField decorator that will replace the usage of FilterableField when you have a custom ID type you want to use.

import { FilterableField, IDField } from '@nestjs-query/query-graphql';
import { ObjectType, GraphQLISODateTime } from '@nestjs/graphql';
import { CustomIDScalar } from '../../common/custom-id.scalar';

@ObjectType('TodoItem')
export class TodoItemDTO {
  @IDField(() => CustomIDScalar)
  id!: string;

  @FilterableField()
  title!: string;

  @FilterableField({ nullable: true })
  description?: string;

  @FilterableField()
  completed!: boolean;

  @FilterableField(() => GraphQLISODateTime, { filterOnly: true })
  created!: Date;

  @FilterableField(() => GraphQLISODateTime, { filterOnly: true })
  updated!: Date;
}

Just finishing up the examples and docs.

doug-martin added a commit that referenced this issue May 10, 2021
* Added new IDField decorator to allow specifying a custom ID type to be used for all inputs that require an ID
doug-martin added a commit that referenced this issue May 11, 2021
* Added new IDField decorator to allow specifying a custom ID type to be used for all inputs that require an ID
@doug-martin
Copy link
Owner

With v0.27.0 you can now use a custom graphql scalar type for id fields. This release addresses this by adding a new @IDField decorator.

For example, assume you have the following CustomIDScalar that obfuscates the id by base64 encoding it.

import { Scalar, CustomScalar } from '@nestjs/graphql';
import { Kind, ValueNode } from 'graphql';

@Scalar('CustomID')
export class CustomIDScalar implements CustomScalar<string, number> {
  description = 'ID custom scalar type';

  private idPrefix = 'id:';

  parseValue(value: string): number {
    // parse a `base64` encoded id from the client when provided as a variable
    return parseInt(Buffer.from(value, 'base64').toString('utf8').replace(this.idPrefix, ''), 10);
  }

  serialize(value: number): string {
    // serialize a number into the base64 representation
    return Buffer.from(`${this.idPrefix}${value}`, 'utf8').toString('base64');
  }

  parseLiteral(ast: ValueNode): number | null {
    // parse a `base64` encoded id from the client when hardcoded into the query
    if (ast.kind === Kind.STRING) {
      return this.parseValue(ast.value);
    }
    return null;
  }
}

You can now specify the custom id type for all query and mutation endpoints by using the @IDField decorator.

import { FilterableField, IDField } from '@nestjs-query/query-graphql';
import { ObjectType, GraphQLISODateTime } from '@nestjs/graphql';
import { CustomIDScalar } from '../../common/custom-id.scalar';

@ObjectType('TodoItem')
export class TodoItemDTO {
  @IDField(() => CustomIDScalar)
  id!: string;

  @FilterableField()
  title!: string;

  @FilterableField({ nullable: true })
  description?: string;

  @FilterableField()
  completed!: boolean;

  @FilterableField(() => GraphQLISODateTime, { filterOnly: true })
  created!: Date;

  @FilterableField(() => GraphQLISODateTime, { filterOnly: true })
  updated!: Date;
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request in progress
Projects
None yet
Development

No branches or pull requests

3 participants