sequelize / sequelize-typescript

Decorators and some other features for sequelize
MIT License
2.79k stars 280 forks source link

Pass model class to the constructor of an abstract class and use it #72

Closed TalissonJunior closed 7 years ago

TalissonJunior commented 7 years ago

What i want to do?

I´m trying to create a generic base repository class that receives a Model in the constructor, so that others repository inherit from the base respository ´passing their model and get some boilerplate like CRUD.

Cardapio.ts - My Model

import {Table, Column, Model, PrimaryKey,AutoIncrement} from 'sequelize-typescript';

@Table({ tableName: 'cardapio',})
export class Cardapio extends Model<Cardapio> {

  @PrimaryKey
  @AutoIncrement
  @Column
  id: number;

  @Column
  nome: string;

  @Column
  descricao: string;

  @Column
  valor: number;
}

IBaseRepository.ts - interface class to make sure that base class will implement all the methods

export interface IBaseRepository<T>{
    GetByID(identifier: number): T;
    ListAll() : T[];
    Insert(model : T): Boolean;
    Delete(identifier : number) : Boolean;
    Update(identifier:number ,model : T): Boolean;
}

BaseRepository.ts - Base repository that contains the generic code for all my repository class

import {Connection,log} from '../../config';
import {Sequelize, Model} from 'sequelize-typescript';
import {IBaseRepository} from '../Interfaces/IBaseRepository';
import {Cardapio} from '../../Models/Database/Cardapio';

export abstract class BaseRepository<T> implements IBaseRepository<T>{

    private _model:any;
    private connection:any;
    private _sequelize:Sequelize;

    public get sequelize(){return this._sequelize };

       // trying to receive model here, it could be a person model, car model ....
    // and it will be receive from the model repository , in this case CardapioRepository will pass the model
    constructor(model:any){
       this._model = model;
       this._sequelize = new Sequelize(new Connection().Development());

       this._sequelize
        .authenticate().then(() => {
            log.infoc(() => "Connection has been established successfully.");
        })
        .catch(err => {
            log.error('Unable to connect to the database:', err);
        });
    }

    GetByID(identifier: number){
        return this._model.findById(identifier)
        .then(model => {          
            return model;
        });
    }

    ListAll(){

        // well i get and error here, typescript doesn´t recognize function findAll, and tells me to add an expression  like 'this._model.findAll({where:{id:{gt:0}}}).all<this._model>({expression here});'

        return this._model.findAll({where:{id:{gt:0}}}).all<this._model>();
    }

    Insert(model: Model<T>){

      return  model.save();
    }

    Delete(identifier: number){
        return true;
    }

    Update(identifier:number, model: T){
        return true;
    }

}

CardapioRepository - repository class that will have all the baseRepository methods,

import {BaseRepository} from './BaseRepository';
import {Cardapio} from '../../Models/DataBase/Cardapio';

export class CardapioRepository extends BaseRepository<Cardapio>{

    constructor(){
        //here we pass the model, to be able to reuse all the methods from the baseRepository
        super(Cardapio);
    }

}

I tried to change my private _model type to Model located in baserepository, but it didn´t work. What am i doing wrong? could anyone tell me please? if you want to test the code and run it i can post it on github so you can clone it.

RobinBuschmann commented 7 years ago

Hey @TalissonJunior can you provide the actual error code thrown by the typescript compiler please?

This part looks wrong to me: all<this._model>(); You cannot set a variable reference here, but a type only. Like all<string> or all<typeof Model>.

TalissonJunior commented 7 years ago

@RobinBuschmann Yeah sure, here is the log. image

its complaining about the all<this._model>();

I created a repository on github with the project, it would be really awesome if you could clone it from https://github.com/TalissonJunior/BaseWebApi---Nodejs-Typescript to check the error by yourself and help me out

all you have to do is run:

npm install -- to install all the dependencies.

and then run:

gulp serve -- to start server at http://localhost:3000/

and to test you can use http://localhost:3000/api/places if everything rights it should return a empty array.

but it isn´t right.

RobinBuschmann commented 7 years ago

Thanks for you example project. As I said: You cannot set a variable reference in all, but a type only like all<string> or all<typeof Model>. So all<this.model> is no valid typescript code. You could use all<any> instead. Since there is no benefit here from passing the type. Nevertheless calling all on the returned promise of findAll is not necessary at all. You can simply write findAll. So you can omit all here.

TalissonJunior commented 7 years ago

Solved!, Thanks man you solved my problem!

TalissonJunior commented 7 years ago

@RobinBuschmann Hey man do you know why my ListAll() function works when i call it directly with my Place Model ex:

 ListAll(){
        //THIS WORKS!
        return Place.findAll({where:{id:{gt:0}}});
   }

and it doesn´t when i create a global private _model variable , populate it with the Place Model inside of my constructor and use it on my listAll() function ? ex:

 ListAll(){
        //THIS DOESN´T WORK!, this._model is being populate with my Place model inside of constructor
        return this._model.findAll({where:{id:{gt:0}}});
    }

CODE THAT WORKS!

BaseRepository.ts

import {IBaseRepository} from '../Interfaces/IBaseRepository';
import {Model} from 'sequelize-typescript';
import {Place} from '../../Models/Database/Place';

/**
 * Base abstract class that implements all the boilerplate methods (Create,Update,insert,ListAll,GetByID)
 * of IBaseRepository, so that other repositories get this methods implemented.
 */
export abstract class BaseRepository<T> implements IBaseRepository<T>{

    GetByID(identifier: number){

        return true;
    }

    ListAll(){
        //THIS WORKS!
        return Place.findAll({where:{id:{gt:0}}});
    }

    Insert(model: Model<T>){
        return  model.save();
    }

    //TODO
    Delete(identifier: number){
        return true;
    }

    //TODO
    Update(identifier:number, model: T){
        return true;
    }

}

CODE THAT DOESN´T WORK~

BaseRepository.ts

import {IBaseRepository} from '../Interfaces/IBaseRepository';
import {Model} from 'sequelize-typescript';
import {Place} from '../../Models/Database/Place';

/**
 * Base abstract class that implements all the boilerplate methods (Create,Update,insert,ListAll,GetByID)
 * of IBaseRepository, so that other repositories get this methods implemented.
 */
export abstract class BaseRepository<T> implements IBaseRepository<T>{

    private _model:any;

    /**
     * getting the current model from PlaceRepository
     */
    constructor(model:any){
       this._model = model;
    }

    GetByID(identifier: number){

        return this._model.findById(identifier)
        .then(model => {          
            return model;
        });
    }

    ListAll(){
        //THIS DOESN´T WORK!
        return this._model.findAll({where:{id:{gt:0}}});
    }

    Insert(model: Model<T>){
        return  model.save();
    }

    //TODO
    Delete(identifier: number){
        return true;
    }

    //TODO
    Update(identifier:number, model: T){
        return true;
    }

}

PlaceRepository.ts

import {BaseRepository} from './BaseRepository';
import {Place} from '../../Models/DataBase/Place';

/**
 * Place repository that inherits BaseRepository passing a Place Model,
 * to receive all the boilerplate Methods (Create,Update,insert,ListAll,GetByID) of BaseRepository 
 */
export class PlaceRepository extends BaseRepository<Place>{
    constructor(){
        //passing model to BaseRepository.ts
        super(Place);
    }

}

log error:

image

And Place model is already add to sequelize if not the first code would trow the same error.

RobinBuschmann commented 7 years ago

Hmm, this is strange. Did you initialize Place in both cases with

import {Sequelize} from 'sequelize-typescript';
new Sequelize({
  modelPaths: [__dirname + 'path/to/place/and/other/models'],
  ...
}) 

?

TalissonJunior commented 7 years ago

i did,

this is my app.ts that initialize the server.

import "reflect-metadata"; // this shim is required
import {createExpressServer} from "routing-controllers";
import { PlaceController } from "./Controller/PlaceController";
import { Sequelize } from "sequelize-typescript";
import { Connection } from "./config";

// creates express app, registers all controller routes and returns you express app instance
const app = createExpressServer({
   routePrefix: "/api",
   controllers: [PlaceController] // we specify controllers we want to use
});

// run express application on port 3000
app.listen(3000);

new Sequelize(new Connection().Development());

and this is my config.ts where i get my connection object

import {Sequelize} from 'sequelize-typescript';
import {Category,CategoryLogger,CategoryServiceFactory,CategoryDefaultConfiguration,LogLevel} from "typescript-logging";

/**
 * Connections to dabase
 */
export class Connection {

    private modelPaths = [__dirname + "/Models/Database"];

    Development(){
        return {
            name: "db_teste",
            username: "a28301_teste",
            password: "teste",
            host:"myhost",
            dialect:"mysql",
            modelPaths: this.modelPaths

        }
    }

    Production(){
        return {
            name: "db_production",
            username: "user name",
            password: "password",
            host:"server/host",
            dialect:"mysql",
            modelPaths: this.modelPaths
        }
    }

    Localhost(){
        return {
            database: "db_snack",
            username: "root",
            password: "1234",
            host:"localhost",
            dialect: "mysql",
            modelPaths: this.modelPaths
        }
    } 
}

I have a link to the repo here if you want to check by yourself :https://github.com/TalissonJunior/BaseWebApi---Nodejs-Typescript

RobinBuschmann commented 7 years ago

Finally, I got it! And it was a very tricky one.

The path to Place in PlaceRepository is wrong. Your PlaceRepository

import {BaseRepository} from './BaseRepository';
import {Place} from '../../Models/DataBase/Place'; // <--- here

export class PlaceRepository extends BaseRepository<Place>{
    constructor(){
        super(Place);
    }
}

It is Database not DataBase. I don't really know what is going on with this typo, but it seems so, that a copy of Place was created from this wrong path. So that the actual Place was initialized but the copy was not. This is strange. On what OS are you working? I'm on MacOS. I don't know if it is a node.js issue or one of MacOS's file system. Any ideas?

TalissonJunior commented 7 years ago

Woooohh, that was really tricky, i'm really glad that you found that and resolve the problem , thank you so much man.

And, I'm on windows 8, It might be an nodejs error, as you said nodejs probably created a copy from the wrong Path. I Will try to take a deep search on that error, and If i find something i tell you.

thoaiky1992 commented 3 years ago

I have just encountered a case like this, please guide me on how to solve it

Zubayer94 commented 1 year ago

If you set the model type as any private _model:any; You will lose model static method suggestions like create, findOrCreate. findAll and so on. What is the way around?