CrudRequest - Seeking thoughts and feedback

[mkelandis]

Team - I am occasionally scraping against CrudRequest. Things that I want to do which are hard:

• formulate a filtering query based on standard query string params using ?thing=a&thing2=b&...
• make service to service calls, formulating a query without replicating CrudOptions with repo calls

To this end I've hacked together a CrudReqBuilder class (This is a WIP, crud-req-builder.ts):

/**
 * Builder Utility for CrudRequest...
 * - make a CrudRequest from Options when no controller is involved.
 * - make a CrudRequest from an existing CrudRequest (defensive copy) when a controller is involved.
 * - add support for injecting standard query string parameters as $and search criteria (?id=1&user=7&...)
 * @see: https://github.com/nestjsx/crud/blob/master/packages/crud/src/interceptors/crud-request.interceptor.ts
 */
export class CrudReqBuilder {

  static readonly crudRequestInterceptor = new CrudRequestInterceptor();
  static readonly DEFAULT_ROUTES = {
    getManyBase: { interceptors: [], decorators: [] },
    getOneBase: { interceptors: [], decorators: [] },
    createOneBase: { interceptors: [], decorators: [], returnShallow: false },
    createManyBase: { interceptors: [], decorators: [] },
    updateOneBase: { interceptors: [], decorators: [], allowParamsOverride: false, returnShallow: false },
    replaceOneBase: { interceptors: [], decorators: [], allowParamsOverride: false, returnShallow: false },
    deleteOneBase: { interceptors: [], decorators: [], returnDeleted: false }
  };

  options: CrudRequestOptions;
  parsed: ParsedRequestParams;

  static withRequest(crudRequest: CrudRequest): CrudReqBuilder {
    const builder = new CrudReqBuilder();
    builder.parsed = clone(crudRequest.parsed);
    builder.options = clone(crudRequest.options);
    return builder;
  }

  static withOptions(crudOptions: CrudOptions): CrudReqBuilder {

    const builder = new CrudReqBuilder();
    const { query, routes, params } = clone(crudOptions);
    builder.options = { query, routes, params} as CrudOptions;
    Object.assign(builder.options.routes, this.DEFAULT_ROUTES);

    const parser = RequestQueryParser.create();
    parser.parseQuery({});
    parser.search = { $and: this.crudRequestInterceptor.getSearch(parser, builder.options, undefined) };

    const crudRequest = this.crudRequestInterceptor.getCrudRequest(parser, builder.options);
    builder.options = crudRequest.options;
    builder.parsed = crudRequest.parsed;
    return builder;
  }

  query(query: any, validArgs?: string[]): CrudReqBuilder {

    this.check();

    const conditions: any = [];
    for (const key of Object.keys(query)) {
      if (validArgs && !validArgs.includes(key)) {
        continue;
      }
      const condition = {};
      if (query[key] instanceof Array) {
        condition[key] = { $in: query[key] };
      } else {
        condition[key] = query[key];
      }
      conditions.push(condition);
    }

    // Explanation - the empty search array contains [0: null, 1: {}]
    // We need to splice in the new terms, replacing the first element and terminate with the {}
    this.parsed.search.$and.splice(0, 1, ...conditions);

    return this;
  }

  limit(limit: number): CrudReqBuilder {
    this.parsed.limit = limit;
    return this;
  }

  check() {
    if (!this.options || !this.parsed) {
      throw new Error('Builder must be initialized withRequest() or withOptions()');
    }
  }

  build(): CrudRequest {
    this.check();
    return {options: this.options, parsed: this.parsed};
  }
}

CrudOptions need to be stored externally from the Controller, so they can be passed to the Builder... Example (user-crud.ts):

export const userCrudOptions: CrudOptions = {
  model: { type: User },
  params: {},
  query: {
    join: {
      accounts: {
        eager: true,
        exclude: ['dateCreated', 'dateUpdated']
      },
      roles: {
        eager: true,
        exclude: ['dateCreated', 'dateUpdated']
      },
      addresses: {
        eager: true,
        exclude: ['dateCreated', 'dateUpdated']
      }
    }
  },
  routes: { only: ['getManyBase', 'getOneBase', 'createOneBase', 'replaceOneBase', 'deleteOneBase'] },
  validation: { whitelist: true }
};

Usage Example:

• filtering params...(controller code)

  crudReq = CrudReqBuilder.withRequest(crudReq)
  .query(req.query, ['scriptHcl', 'applicantId'])
  .build();
  req.log.debug(JSON.stringify(crudReq));
  const scriptResponseSets = await super.getManyBqpBase(req, crudReq);

• service to service (hand-rolled service method)

  async findByHcl(hcl: string): Promise<Script> {
  const crudRequestByHcl = CrudReqBuilder.withOptions(scriptCrudOptions()).query({hcl}).build();
  const script = await super.getOneOrFail(crudRequestByHcl);
  await this.hydrateScript(script);
  return script;
  }

This class has also been useful in unit tests running against the service when no controller is involved.

I think long term we could consider organizing the code in the request interceptor such that building a CrudRequest could be handled outside of the context of a Controller. The code above has a lot of limitations and assumptions because the necessary input to a CrudRequest is kinda sprinkled deeply through the code path the interceptor follows. (default routes vs only/excluded, parsed.filters vs parsed.search for example). A real CrudRequestBuilder provided by the library for use externally, which is also used internally to marshal an HTTP Request to a CrudRequest, could be very useful.

Let me know if there is an easy way I'm missing to do this stuff. Just looking for feedback and wanting to open the discussion up to see how other folks might be solving similar probs.

Thanks, -Mike

[hakimio]

formulate a filtering query based on standard query string params using ?thing=a&thing2=b&...

Why? crud library has perfectly usable search and filter query params.

make service to service calls

Crud library is not intended for that. You can take a look at microservices and maybe gRPC for that use case. For frontend usage crud-request library works great.

[mkelandis]

"...crud library has perfectly usable search and filter query params."

^-- Because REST is a powerful concept. It gives me a predictable way to reason, and make assumptions about API endpoints by simply knowing about the nouns in the system.

For example, if I know about a resource called "books" I can now generally formulate a query to retrieve the books in the system without inventing any new verbs.

GET /books

...the next cognitive leap I might make is to return books by a particular author. ok, here is what I would assume based on what I know about REST APIs...

GET /books?author=Stephen%20King

...I want to be able to support that idea but must shoehorn into the crud way:

GET /books?filter=author||$eq||Stephen%20King

^-- not an assumption I would make about a REST API. I should be able to easily transform some arguments on an endpoint into the syntax that nestjs/crud wants - but I keep finding over and over again that it's extremely difficult. This is just one minor example.

I think the thing I am scraping against is not how powerful this library is but how extensible it isn't .

I respect mightily what has been done here by all the contributors. That being said, I think that a home-grown solution is going to be the best option for my team.

[hakimio]

I think that a home-grown solution is going to be the best option for my team.

Good luck with that. If you do make another custom REST solution, I am pretty sure you'll have better understanding why the crud library was built the way it was and why your way of adding filters and other features might not be the best way.

[michaelyali]

Hi @mkelandis Thanks for raising this topic and sorry for my late response. At first, I wanted to break down your points, but after finally reading everything I made a conclusion that your suggestion is even more non-NestJs way than this library is (I would really LOVE to see and use truly NetsJs way to create CRUD apps with all the functionality that we have currently have in this lib).

But even if there is a possibility that I agree to change everything to apply your suggestions, this lib will have so many breaking changes (again) so that I'm afraid it'll become a completely new one. I don't plan to have SUCH BIG breaking changes anymore in the future.

But nevertheless, answering your questions (or points):

formulate a filtering query based on standard query string params using ?thing=a&thing2=b&...

We can definitely add this. It's not a problem at all.

make service to service calls, formulating a query without replicating CrudOptions with repo calls

I've always have been saying that my main intentions of creating CrudService were that it should handle requests from the CrudController. It has never intended to have all the crud functionality without being called outside a CrudController.

But I'll think how can I add a possibility of composing parsed request object so that you can do service to service calls.

Cheers.

[mkelandis]

Thanks @zMotivat0r -- I think my intent here was to open up the discussion. Feel free to close this issue out. Best Regards -Mike