Routing-controllers: Controller Inheritance support

[jselesan]

Hi there! It would be nice to support controller inheritance, which can allow create (for example) a CrudController with CRUD methods for a REST API, calling a service to fetch data, and then inherit from that controller for each specific entity type

[MichalLytek]

Prefer composition over inheritance - put your CRUD methods logic into generic services and inject it to the controller.

[jselesan]

Yes @19majkel94, is what I'm doing, but I have to repeat the route handlers on every controller. Not a big deal, but a lot of repeated code.

[ssljivic]

Good point @jselesan

[MichalLytek]

CrudController with CRUD methods for a REST API, calling a service to fetch data, and then inherit from that controller for each specific entity type
I have to repeat the route handlers

So you want something like a generic controller? Or can you show me your current use case and how this feature should looks&works 😉

[jselesan]

I think something like this (may not compile ;))

export abstract class CrudController {
  private _service: CrudService;
  protected setService(service: CrudService) {
    this._service = service;
  }

 @Get("/")
 all() {
   return this.service.getAll();
 }  

 /* the other methods are similar */
}

@JsonController("/employees")
export class EmployeesController extends CrudController {
  constructor(private service: EmployeesService) { //EmployeesService should extend CrudService
    this.setService(service);
  }
}

This way, EmployeesController will expose all() method, that can be match to a GET / request

I'm not sure if it is possible, is just an idea :)

[pleerock]

@jselesan code you provided does not look good even if it was implementable. In my experience inheritance in controllers never bring somethings really valuable, its only makes things complex and unpredictable. Controllers should be simple as hell and if you have duplication in logic - simply extract this logic into services - thats correct places for such logic. If you have duplication in decorators and routes - thats not an issue, having duplicated route decorators just makes things more explicit. Really really no need to make things complex (and make life harder for everyone except you) with such patterns.

[MichalLytek]

If you have duplication in decorators and routes - thats not an issue, having duplicated route decorators just makes things more explicit.

+1000 😉

I also have something like @Get("/") all() { return this.service.getAll(); }
in almost every controller but it's not a big deal if you have repeated logic extracted into services.

@jselesan
Imagine that you use the extending pattern to reduce boilerplate. So in some time you can get the inheritance chain that consist of 10 extending controllers and there are difference for every controller. So instead of look into controller mapped to REST API resource path and see what's happening there (like reading a doc) you have to traverse all the way up to see all actions, etc. Good luck with debugging 😉

For easier development I recomend you to create your own template or even snippet for IDE so you can just type the service name and type for import and inject, controller name or path and the base thing will be configured in 30s 😃

[pleerock]

Feel free to continue discussion

[95gabor]

+1

[alexey-pelykh]

Actually this makes sense in a bit more complex implementation like:

import Bluebird from 'bluebird'
import { Request } from 'express'
import { Get, Post, Put, Delete, Body, Param, Req, OnNull, NotFoundError } from 'routing-controllers'
import ResourceRepositoryService from '../services/ResourceRepositoryService'
import { ResourceNotFoundError } from '../errors'
import Dto, { DtoClassType } from './Dto'

export type ResourceRepositoryServiceClass<
  TService extends ResourceRepositoryService<TEntity, TId, TContent>,
  TEntity = {},
  TId = {},
  TContent = {}
> = Function & { prototype: TService }

export default function ResourceController<
  TDto extends Dto<TDto, TObject>,
  TContentDto extends Dto<TContentDto, TContentObject>,
  TService extends ResourceRepositoryService<TEntity, TId, TContent>,
  TEntity = {},
  TId = {},
  TContent = {},
  TObject = {},
  TContentObject = {}
> (
  resourceServiceIdentifier: ResourceRepositoryServiceClass<TService>,
  dtoClass: DtoClassType<TDto>,
  contentDtoClass: DtoClassType<TContentDto>
) {
  abstract class ResourceController<
    TController
  > {
    protected readonly packDto = Dto.pack.bind(dtoClass)
    protected readonly unpackDto = Dto.unpack.bind(dtoClass)
    protected readonly packContentDto = Dto.pack.bind(contentDtoClass)
    protected readonly unpackContentDto = Dto.unpack.bind(contentDtoClass)

    constructor (
      protected readonly resourceService: TService
    ) {}

    @Post()
    public async create (
      @Body({ required: true }) data: TContentDto
    ): Promise<TDto> {
      const resource = await this.resourceService.create(this.unpackContentDto(data))
      return this.packDto(resource)
    }

    @Get('/:id')
    @OnNull(404)
    public async get (
      @Param('id') id: number
    ): Promise<TDto | null> {
      const resource = await this.resourceService.get(id as any)
      if (!resource) {
        return null
      }

      return this.packDto(resource)
    }

    @Put('/:id')
    public async update (
      @Param('id') id: number,
      @Body({ required: true }) data: TContentDto
    ): Promise<TDto> {
      try {
        const resource = await this.resourceService.update(id as any, this.unpackContentDto(data))
        return this.packDto(resource)
      } catch (err) {
        if (err instanceof ResourceNotFoundError) {
          throw new NotFoundError(err.message)
        }
        throw err
      }
    }

    @Delete('/:id')
    public async delete (
      @Param('id') id: number
    ): Promise<void> {
      try {
        await this.resourceService.delete(id as any)
      } catch (err) {
        if (err instanceof ResourceNotFoundError) {
          throw new NotFoundError(err.message)
        }
        throw err
      }
    }

    @Get()
    public async query (
      @Req() request: Request
    ): Promise<TDto[]> {
      const resources = this.resourceService.query()

      return Bluebird.map(resources, resource => this.packDto(resource))
    }

    @Get('/count')
    public async count (
      @Req() request: Request
    ): Promise<number> {
      return this.resourceService.count()
    }
  }

  return ResourceController
}

[alexey-pelykh]

@19majkel94 btw with yours type-graphql this approach worked almost like a charm (except https://github.com/19majkel94/type-graphql/issues/89) =)

[alexey-pelykh]

My suggestion would be to add an extends: Function[] options to the Controller and JsonController, and check that in MetadataBuilder.createControllers to assemble actions from extended controllers as well

[alexey-pelykh]

@19majkel94 any hints on how this may be implemented in routing-controllers? :)

[MichalLytek]

@alexey-pelykh Not really, I don't know this codebase very well so it's hard for me to dig in and implement this. Also as pleerock is against, it might not be a good idea 😜

[alexey-pelykh]

@19majkel94 I wonder what changed your mind!

[lixaotec]

Dear fellows, @alexey-pelykh , @pleerock ,

Is this abstract controller functional?

Sorry to repoen the issue, I was wondering how to base controllers, services and repository for a plenty similar CRUD.

@alexey-pelykh example seems very interesting as it aggregates a lot of work that is repeatable for cruds.

Where can I find more information or samples, would you provide @alexey-pelykh .

Appreciate your work

[alexey-pelykh]

@lixaotec it wasn't last time I visited the feature, I ended up having MyEntityController + MyEntityCrudController

[closertb]

so, is there a way ？ i also meet this problem.

BaseControll

import { Model } from 'sequelize-typescript';
import { AnyObject } from '../config/global';
import Repository from "./Repository";

export default class Controller {
    constructor(public repository: Repository) {
      this.repository = repository;
    }

    async all(body: object) {
      const res = await this.repository.findAll(body);
      return res;
    }

    async findById(id: number) {
      const res = await this.repository.findOne(id);
      return res;
    }

    async one(body: AnyObject) {
      const { id = -1 } = body;
      if (typeof +id !== 'number' || id < 0) {
        return {
          status: 'error',
          code: 1001,
          message: 'id无效'
        };
      }
      const res = await this.repository.findOne(+id);
      return res;
    }

    async save(body: Model) {
      await this.repository.save(body);
      return { message: 'success' };
    }

    async update(body: AnyObject) {
      await this.repository.update(body);
      return { message: 'success' };
    }
}

ServiceController

import { JsonController, Post, Body } from "routing-controllers";
import { Service } from "typedi";
import Controller from '../Controller';
import { AnyObject } from '../../config/global';
import Model from "./model";
import ServiceRepository from "./repository";

@Service()
@JsonController('/service')
export default class RuleController extends Controller {
  constructor(repository: ServiceRepository) {
    super(repository);
  }
  @Post("/query")
  async all(@Body() body: object) {
    return super.all(body);
  }

  @Post("/detail")
  async one(@Body() body: AnyObject) {
    return super.one(body);
  }

  @Post("/save")
  async save(@Body() body: Model) {
    return super.save(body);
  }

  @Post("/update")
  async update(@Body() body: AnyObject) {
    return super.update(body);
  }
}

I must repeat the code in every service

[github-actions[bot]]

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.