Strange behaviour when referencing 'model.modelName'

[LucioMF]

Versions

• NodeJS: 10.15.3
• Typegoose(NPM): 6.2.2
• mongoose: 5.8.10
• mongodb: 4.2.1
• nestjs / nestjs-typegoose: 6.11.4 / 7.0.0

What is the Problem?

I recently upgrade to version 6
I was using the "modelName" property of the model, to tag my endpoints in Swagger. But with the new version this seems to somehow interfere with the queries (of any kind) to the database. They stay frozen and never respond

Code Example

Base Model:

import { prop, modelOptions } from '@typegoose/typegoose';

@modelOptions({
    schemaOptions: {
        toJSON: {
            virtuals: true,
            getters: true,
        },
        toObject: {
            virtuals: true,
            getters: true,
        },
    },
})
export class BaseModel<T> {
    @prop({ default: Date.now() })
    createdAt?: Date;

    @prop({ default: Date.now() })
    updatedAt?: Date;

    id?: string;
}

User Model:

import { BaseModel } from '../../shared/base.model';
import { prop, ReturnModelType, Ref, arrayProp, getModelForClass } from '@typegoose/typegoose';

export class User extends BaseModel<User> {
    @prop({ lowercase: true, trim: true, required: [true, 'username is required'], minlength: [4, 'Must be at least 4 characters'], unique: true })
    username!: string;

    @prop({ required: [true, 'password is required'], minlength: [6, 'Must be at least 6 characters'] })
    password!: string;

    @prop()
    firstName?: string;

    @prop()
    lastName?: string;

    get fullName(): string {
        return `${this.firstName} ${this.lastName}`;
    }

    static get model(): ReturnModelType<typeof User> {
        /* old version way */
        // return new User().getModelForClass(User);
        return getModelForClass(User);
    }

    static get modelName(): string {
        /* If I put a breakpoint here I can see that the model does have the modelName property,
            and that has the expected valueo */
        return this.model.modelName;

       /* If I return it like follow it works perfectly */
       // return 'User';
    }
}

User service:

import {
    Injectable,
} from '@nestjs/common';
import { InjectModel } from 'nestjs-typegoose';
import { User } from './models/user.model';
import { ReturnModelType } from '@typegoose/typegoose';

@Injectable()
export class UserService {
    protected model;
    constructor(
        @InjectModel(User) userModel: ReturnModelType<typeof User>,
    ) {
        this.model = userModel;
    }

    async findAll(filter = {}): Promise<DocumentType<User>[]> {
        return this.model.find(filter).exec();
    }

}

User Controller:

import {
    Controller,
    HttpStatus,
    Res,
    Get,
} from '@nestjs/common';
import { UserService } from './user.service';
import { ApiTags, ApiResponse } from '@nestjs/swagger';
import { User } from './models/user.model';

@Controller('user')
@ApiTags(User.modelName) /* Here I use it */
export class UserController {
    constructor(
        private readonly userService: UserService,
    ) {  }

    @Get('/listusers')
    @ApiResponse({ status: HttpStatus.OK, type: String })
    @ApiResponse({ status: HttpStatus.BAD_REQUEST })
    async listAllUsers(
        @Res() res,
    ) {
        const users = await this.userService.findAll();
        return res.send(users);
    }
}

Steps to reproduce

Run the code above (or some similar) and call ''listusers" method. It will get frozen in the findAll query.
Now replace the line return this.model.modelName; in the virtual get property of the model by return 'User'; and it will work.

Do you know why it happenes?

no

[hasezoey]

simple:
you are overwriting the static property modelName that is provided by mongoose with your own, and so cause an infinite loop trying to read the property modelName, you can fix this by either renaming this function or just using the normal doc.constructor.modelName / model.modelName

PS: i edited your issue to have code-highlighting

[LucioMF]

you can fix this by either renaming this function or just using the normal doc.constructor.modelName / model.modelName

Neither solution works.
It only works when, in one way or another, I give the hardocode string to swagger.
It's weird because when I refer to model.modelName, swagger does work, and I can see the name of the model in the tag entering localhost: 8080/api/docs. But it makes all mongo queries freeze

[LucioMF]

A comment on the sidelines (correct me if I'm wrong): Unless github's policies have changed, a issue can only be reopened if it was yourself who closed it. Otherwise, only a repo collaborator can.

[hasezoey]

@LucioMF then just ask to reopen, dont create an new issue that is exactly the same

[hasezoey]

@LucioMF i just tested your provided code (without nest) and it works, no problem at all (the static modelName never gets called)

could you add the following lines before your find query (user service findAll):

console.log("default connection", mongoose.connection.readyState);
console.log("all other", mongoose.connections.map(x => x.readyState));

here the code:

// NodeJS: 12.14.1
// MongoDB: 4.2-bionic (Docker)
import { getModelForClass, prop, modelOptions, ReturnModelType } from "@typegoose/typegoose"; // @typegoose/typegoose@6.2.2
import * as mongoose from "mongoose"; // mongoose@5.8.9

@modelOptions({
  schemaOptions: {
    toJSON: {
      virtuals: true,
      getters: true,
    },
    toObject: {
      virtuals: true,
      getters: true,
    },
    timestamps: true
  },
})
export class BaseModel<T> {
  public createdAt?: Date;
  public updatedAt?: Date;

  public id?: string;
}

export class User extends BaseModel<User> {
  @prop({ lowercase: true, trim: true, required: [true, "username is required"], minlength: [1, "Must be at least 4 characters"] })
  public username!: string;

  @prop({ required: [true, "password is required"], minlength: [1, "Must be at least 6 characters"] })
  public password!: string;

  public static get model(): ReturnModelType<typeof User> {
    /* old version way */
    // return new User().getModelForClass(User);
    return getModelForClass(User);
  }

  public static get modelName(): string {
    // this function never got called
    console.log("p s g modelName");
    return this.model.modelName;
  }
}

export const UserModel = getModelForClass(User);

(async () => {
  await mongoose.connect(`mongodb://localhost:27017/`, { useNewUrlParser: true, dbName: "verify186", useCreateIndex: true, useUnifiedTopology: true });

  const user = await UserModel.create({ username: "user1", password: "somepwd" });
  console.log(user);

  const found = await UserModel.findById(user._id).exec();
  console.log(found);

  const foundAll = await UserModel.find({}).exec();
  console.log("foundall", foundAll);

  await mongoose.disconnect();
})();

the only thing i changed is to use the mongoose timestamps option, remove some properties that are not needed for reproduction, removed unique for testing and set minlength (for all) to 1

[LucioMF]

I don't see you call the static "modelName" in question.
I do it in Swagger's decorators, which I use in the UserController.

For example:

import {
    Controller,
    Get,
} from '@nestjs/common';
import { UserService } from './user.service';
import { ApiTags } from '@nestjs/swagger';
import { User } from './models/user.model';

@Controller('user')
@ApiTags(User.modelName) /* Here I use it */
export class UserController {
    constructor(
        private readonly userService: UserService,
    ) {  }

[hasezoey]

@LucioMF sorry, i never used swagger and know little about nestjs & graphql, but i tested logging this too (console.log("inital", UserModel.modelName)) but it never fired the static in question

Update: i renamed the function and noticed it is an static getter, which isnt supported by mongoose (just gets ignored), so either use an static function or some non-static getter

[hasezoey]

@LucioMF i would recommend opening an mongoose issue with an use case (i cant, because i dont know an use case that would need static getter & setters)
-> and i would recommend referencing this issue for future reference

[LucioMF]

Both "model" and "modelName" are static because they are class properties, I don't need Mongoose to recognize them as model properties.
When doing (console.log("inital", UserModel.modelName)), static "modelName" is not fired because you are trying to access it from the model (and model has his own modelName property), try doing the following to access it as a class static function:
(console.log("inital", User.modelName))
Where "User" is the exported class

[hasezoey]

@LucioMF why?

update: i tried it, still no problem

[LucioMF]

But the function is triggered, right?

I tried calling the function ONLY for a "console.log" and certainly no problem occurs.
The problem is when that same function is called from a Swagger decorator in the controller.
I don't know how to reproduce the issue without using Swagger.

Here you have a simple configuration to use Swagger:

import { NestFactory } from '@nestjs/core';
import { AppModule } from './app.module';
import { DocumentBuilder, SwaggerModule } from '@nestjs/swagger';

async function bootstrap() {
  const app = await NestFactory.create(AppModule);
  const hostDomain = 'http://localhost:8080';

  const swaggerOptions = new DocumentBuilder()
    .setTitle('titler')
    .setDescription('some description')
    .setVersion('1.0.0')
    .addTag('API')
    .build();

  const swaggerDoc = SwaggerModule.createDocument(app, swaggerOptions);

  app.use('/api/docs/swagger.json', (req, res) => {
    res.send(swaggerDoc);
  });

  SwaggerModule.setup('api/docs', app, null, {
    swaggerUrl: `${hostDomain}/api/docs/swagger.json`,
    explorer: true,
    swaggerOptions: {
      docExpansion: 'list',
      filter: true,
      showRequestDuration: true,
    },
  });

  app.enableCors();
  await app.listen(8080);
}
bootstrap();

Then you will be able to use Swagger's decorators in your controller and could see the result in localhost:8080/api/docs

[hasezoey]

@LucioMF could you make an repo so we have the same code and versions?

[LucioMF]

Sure: https://github.com/LucioMF/typegoose-swagger

pre-requirements:
Having a mongo server running to point to. The connectionString configuration is in app.module.ts

Instructions:

• Clone repo
• Download dependencies (npm i)
• Run it (npm run start:dev)
• POST or GET to localhost:8080/user
• To see Swagger documentation enter localhost:8080/api/docs

[hasezoey]

result of:

console.log("default connection", mongoose.connection.readyState);
console.log("all other", mongoose.connections.map(x => x.readyState));

is

default connection 0
all other [ 0, 1 ]

which means you have the problem of kpfromer/nestjs-typegoose#60 with further reading and trying to fix it would result in kpfromer/nestjs-typegoose#57

[hasezoey]

TL;DR: the connection made with TypegooseModule.forRoot is an non-default connection, and the model is associated with the default connection, which never gets connected so all queries stall and wait to be connected to the database (and naming connections with connectionName is somehow not supported, so the non-default connection can not be queried and identified easily)

[LucioMF]

Sorry, I still do not understand why queries only get stall when you want to access the "modelName" specifically (because if accessed from another side there is no problem) from Swagger decorators.
How is relationed one thing with other?
Also, if I don't try to access that model property from the decorators, the code works perfectly.
Should I get back to @nestjs/mongoose?
If it is a nestjs-typegoose bug, is there a fix planned?

[hasezoey]

is there a fix planned?

no there isnt (at least that i know of), because i still dont know much about nestjs and the nestjs-typegoose codebase i cant really help and kpfromer (the owner of nestjs-typegoose) is rather slow to respond
so yes, i would suggest for now switching to nestjs/mongoose with typegoose

[LucioMF]

Thanks.
Since it is a minor problem, and that the solution is not that bad... If I stay in nestjs-typegoose, and simply pass the model name as a hardcoded string, will I encounter a more "serious" problem in the future?

[hasezoey]

and simply pass the model name as a hardcoded string, will I encounter a more "serious" problem in the future?

if you want to name the class something like SomeClass the model name will be SomeClass and if you overwrite this (with @modelOptions) the name will change in that what you have set so if you dont use this kind, it is save to use, but i would not rely on hardcoded things

maybe try the getName function of typegoose (which is internal, but still can be imported) i guess from import { getName } from "@typegoose/typegoose/lib/internal/utils"

[LucioMF]

Thanks.
Using imported getName function, and passing the class as a parameter (because if you pass the model "this.model" returns the string "model") it seems to work as well.
Something like:

import { getName } from '@typegoose/typegoose/lib/internal/utils';

    static get modelName(): string {
        // return this.model.modelName; /*doesn't work*/
        // return 'User'; /*dirty workaround*/

        return getName(User); /*nice workaround*/
    }