santiq / bulletproof-nodejs

Implementation of a bulletproof node.js API 🛡️
https://softwareontheroad.com/ideal-nodejs-project-structure/
MIT License
5.49k stars 1.15k forks source link

How to call the service locator and declare services ? Should I use the `new` keyword ? #12

Closed lineldcosta closed 5 years ago

lineldcosta commented 5 years ago

Hello Santiago,

Hey i am using your nice code structure to build my new application, i am new to typescript and typedi. I am stuck at injecting database connections at model I am using postgresql database

Below file path i am adding postgres db connection

bulletproof-nodejs/src/loaders/dependencyInjector.ts Container.set('db', db);

and in models folder

import { Service, Inject, Container } from 'typedi';
import { Pool } from 'pg';

class Status {

  @Inject('db')
  private db1;
  constructor() {
  }

  public async getApiStatus(): Promise<string> {
    console.log(this.db1); // **not working**
    let pool: Pool = Container.get('db');  // **working fine**
    //console.log(this.pool);
    return new Promise<string>((resolve, reject) => {
      pool.query('select * from status;', (error, results) => {
        if (error) {
          throw error;
        }
        console.log(results.rows);
      });

      resolve('success');
    });
  }
}

export default new Status();

I wanted to somehow inject the database into the model using @inject.. and without adding container.get('db') at each methods and access the database using this.db1 Could you please give me some hints to achieve this.

santiq commented 5 years ago

I think it is because of how you are exporting your model.

You are instantiating the Status class at the moment of export export default new Status() so @inject cannot be executed because it's only called when the service is decorated with @Service and instantiated through a service locator.

So, you can refactor your code in this way

import { Service, Inject, Container } from 'typedi';
import { Pool } from 'pg';
@Service() // notice this change
class Status {

  constructor(
     @Inject('db') private db1;
) {
  }

  public async getApiStatus(): Promise<string> {
    console.log(this.db1);
    let pool: Pool = this.db1; 

    return new Promise<string>((resolve, reject) => {
      pool.query('select * from status;', (error, results) => {
        if (error) {
          throw error;
        }
        console.log(results.rows);
      });

      resolve('success');
    });
  }
}

export default Status; // notice this change

And then to get an instance of the Status class use

import { Container } from 'typedi';
import Status from '../models/status';

const statusInstance = Container.get(Status); 
lineldcosta commented 5 years ago

Thanks for the update.

i am using below service to use the above models as mentioned in the bulletproof-nodejs.

// path-to-folder/services/status/index.ts

import { Service, Inject } from 'typedi';

@Service()
export default class StatusService {
  constructor(@Inject('statusModel') private statusModel) {}
  public async status() {
    try {
      let apiStatus = await this.statusModel.getApiStatus();
      //throw 'error occured';
      return {
        apiStatus: apiStatus,
      };
    } catch (e) {
      throw e;
    }
  }
}

In this case it will throw the error TypeError: this.statusModel.getApiStatus is not a function

but you have used mongoose database and works fine!

santiq commented 5 years ago

I think is not related to the database you choose.

How are you instantiating the StatusService ?

Do you use the service locator? Container.get('statusService')

Or do you use the class constructor? new StatusService( new StatusModel( new DatabaseConnection() ) )

Also, notice that in your example, you are using @Inject('statusModel') but you need to declare that string 'statusModel' to be the id of the class to instantiate otherwise the dependency injector will not find it.

import { Service, Inject, Container } from 'typedi';
import { Pool } from 'pg';
@Service('statusModel') // notice this change, now the service has a name
class Status {

  constructor(
     @Inject('db') private db1;
) {
  }

  public async getApiStatus(): Promise<string> {
lineldcosta commented 5 years ago

Hey as per your instructions, i have changed the code, its working can you just check services folder and core(models) folder once and please let me know anything i need to update.

Below is the git ink https://github.com/lineldcosta/sample-nodejs

santiq commented 5 years ago

@lineldcosta Seems pretty good!

lineldcosta commented 5 years ago

Thank you boss!!!! i'm happy!

lineldcosta commented 5 years ago

Hi, Sorry to disturb you again! :-)

I am facing some difficulties in integrating unit-testing for services folder(error : TypeError: this.db.query is not a function) I have added integration test. But unit testing for services folder, not getting enough idea how to implement it. I have added the repo https://github.com/lineldcosta/sample-node-testing.git Can you please give some idea for unit testing services and models services folder: /tests/services/status/index.spec.ts models folder: /tests/core/status/index.spec.ts

santiq commented 5 years ago

Hi @lineldcosta, I'm glad to help you.

I totally forget to add tests examples to this repository, maybe next week I'll add a few examples.

For what I see in your code, you shouldn't call the containers in your tests, but mock every dependency of your service class.

Because one of the benefits of using Dependency Injection is to be able to use any implementation of the dependency as long as you don't change the interfaces and response types.

See this example:

Here we have a simple service class that handles user signup, so it creates a user in the database and also returns an access token like a JWT.

src/services/auth.ts

class AuthService {
constructor( @Inject('userModel') private userModel )

public SignUp(userObject) {
  const userRecord =  this.userModel.create(userObject)
  return { userRecord, JWT: '12345678.1234578.2134578' };
}
}

A good unit-test should only test the logic of the Service Unit, without calling the actual database. (that would be an integration test or even an end-to-end test)

I'm using mocks and spies from Jest but I'm sure you can find something equivalent in other tests frameworks.

tests/unit/services/auth


import UserService from '../../../src/services/auth';

describe('User Service Unit tests', () => {

  describe('Signup ', () => {
   const  userObject = { email: 'santiago@softwareontheroad.com', password: '12345678' };

   const userModelMock = {
     create: jest.fn().mockReturnValue(userObject) // <- Important, I'm creating a mock of the method that we know will be called. And also, mocking the response.
   }

   const userServiceInstance = new UserService(userModelMock);

   const result = userServiceInstance.SignUp(userObject);

   expect(result.user.email).toBeDefined();
   expect(result.user.password).toBeDefined();
   expect(result.jwt).toBeDefined();

    // Assert that the underlaying dependencie was called
   expect(userModelMock.create).toBeCalled();  
  })

The reason to not test the create method from the model, it's because the library already has those tests, you shouldn't worry about them!

What do you think?

lineldcosta commented 5 years ago

Looks cool!

I understand the concept now :-) I think i was doing integration test for every module till now. Thanks for the answer. I will try to integrate the concept you explained.

pprathameshmore commented 3 years ago

How to test chained mongoose functions?

 try {
            return await this.assetModel
                .find(filter)
                .populate('owner')
                .sort(sort)
                .limit(limit)
                .skip(page);
        } catch (error) {
            throw error;
        }
santiq commented 3 years ago

@pprathameshmore You shouldn't test a library, or third-party code, those are supposed to have their own test coverage.

pprathameshmore commented 3 years ago

I am trying to test using your approach, for single-function it works fine. But when trying to test this function. I am getting this error.

This is my code:

   test('should return assets collection', async () => {
        const assetsReturnedMockValue = [
            {
                type: 'Application',
                id: '5fedc45308510741a40f0934',
                assetId: 'SLKTM123',
                name: 'Application',
                deleted: false,
                risk: 9,
                owner: null,
            },
            {
                type: 'Application',
                id: '5fedc45308510741a40f0934',
                assetId: 'SLKTM123',
                name: 'Application',
                deleted: false,
                risk: 9,
                owner: null,
            },
        ];

        const assetModelMock = {
            find: jest.fn().mockReturnValue(assetsReturnedMockValue),
        };

        const assetServiceInstance = new AssetServices(
            assetModelMock,
            assetModelMock
        );

        const result = await assetServiceInstance.getAssets();

        expect(result).toBeDefined();
        //Assert that the underlying dependencies was called
        expect(assetModelMock.find).toBeCalled();
    });

Error: image

pprathameshmore commented 3 years ago

Whenever try to test only output, getting the above error

pprathameshmore commented 3 years ago

@santiq How to test dependent Services?

image