getDiscriminatorModelForClass doesn't respect the model's existingConnection option

[CasperLinden2]

Versions

• System: linux
• NodeJS: v18.13.0
• Typescript: 4.9.4

• Compiler / Transpiler: tsc
• Typegoose(NPM): 10.1.0
• mongoose: 6.9.0
• mongodb: 4.13.0

What is the Problem?

I would expect getDiscriminatorModelForClass to use the same connection that the base model uses. But that doesn't happen, even if you explicitly specify it.

Code Example

const db = await mongoose.createConnection('mongodb://127.0.0.1:27017/database1').asPromise();

const model1 = getModelForClass(ModelOne, {
    existingConnection: db.useDB('database2')
});

// Model 1 works fine

const model2 = getDiscriminatorModelForClass(ModelOne, ModelTwo, {
    existingConnection: db.useDB('database2')
});

// model2 is using database1

Do you know why it happens?

no

---

[hasezoey]

I would expect getDiscriminatorModelForClass to use the same connection that the base model uses

it should, from what i know

even if you explicitly specify it.

from what i know existingMongoose and existingConnection cannot be set differently for a discriminator

const model2 = getDiscriminatorModelForClass(ModelOne, ModelTwo, {

are you sure you want a discriminator on ModelOne instead of model1?

[CasperLinden2]

@hasezoey Sorry, that was my lazy translation from the real source.

I've dug deeper and found that it was my bug, however, it would be super helpful if this spat out a warning ;)

import {getDiscriminatorModelForClass, getModelForClass, prop} from "@typegoose/typegoose";
import mongoose from "mongoose";

export class ModelOne
{
    @prop()
    public var1: string;
}

export class ModelTwo extends ModelOne
{
    @prop()
    public var2: string;
}


async function test()
{
    const db = await mongoose.createConnection('mongodb://127.0.0.1:27017/database1', {}).asPromise();

    // To reproduce the issue we have to first get the model from the first db
    getModelForClass(ModelTwo, {
        existingConnection: db
    });

    const newDB = db.useDb('database2');
    const model1 = getModelForClass(ModelOne, {
        existingConnection: newDB
    });
    const model2 = getDiscriminatorModelForClass(model1, ModelTwo, {
        existingConnection: newDB
    });

    const test = new model2();
    test.var2 = 'test';
    await test.save();

    // Model2 will be created on database1
}
test().catch((e) =>
{
    console.error(e);
})

[hasezoey]

with 0599ef2 there is documentation in getDiscriminatorModelForClass noting that existing* options will not be used from the discriminator, it also adds a runtime warning as W002.
this change will be in version 10.1.1

[github-actions]

🎉 This issue has been resolved in version 10.1.1 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀

[CasperLinden2]

Actually, I don't think the document change covers this entirely.

I think the issue is that you can't define the same model on multiple connections/databases.

I think we also need Warn if Model was already declared on a different database

.. Or in fact, throw an error, since this is always going to result in unexpected behaviour

[hasezoey]

I think the issue is that you can't define the same model on multiple connections/databases.

no that is possible, that is a different issue - which is already documented as Models with same name
TL;DR: it is because typegoose does its own model / class caching, which is based on model name, to be able to retrieve a class for a registered model

or if this is not what you mean, please clarify

[CasperLinden2]

Well, from my example script

I first get ModelTwo on database1

getModelForClass(ModelTwo, {
        existingConnection: db
});

Then a subsequent attempt to get ModelTwo on database2 results in it ending up on database1 despite it deriving from a model which is successfully established on database2

 const model2 = getDiscriminatorModelForClass(model1, ModelTwo, {
        existingConnection: newDB
    });

[CasperLinden2]

So actually because of this double-initialisation it results in the opposite behaviour than is documented by your changes

[hasezoey]

opposite behaviour than is documented by your changes

the documented behavior is not for double defining the same model name

i will consider adding a way to disable caching so this can be allowed (while disabling the getting from cache)

---

Edit:

Then a subsequent attempt to get ModelTwo on database2 results in it ending up on database1

actually it does not create a new model, see

typegoose/src/typegoose.ts

Lines 427 to 429 in 4be10be

	if (models.has(name)) {
	return models.get(name) as ReturnModelType<U, QueryHelpers>;
	}

[hasezoey]

released typegoose v10.2.0-beta.1 which includes a way to disable caching, please test it and report if this solves your issue

for now the beta documentation can be found at Github Models with same name: Disable Caching

[CasperLinden2]

Thanks! Could this be an option that could be passed to getModelForClass and getDiscriminatorModelForClass so we don't have to disable caching globally?

[hasezoey]

locally disabled cache is added with 78ac3bc, which will be in 10.2.0-beta.2

please test if this is properly working

for now the beta documentation can be found at Github Models with same name: Disable Caching AND Github modelOptions: disableCaching

(also the global option has been renamed from disableCaching to disableGlobalCaching)