Skip to content

Commit

Permalink
fix(typegoose::getDiscriminatorModelForClass): add warning when using…
Browse files Browse the repository at this point in the history
… different "existing*" options

fixes #789
  • Loading branch information
hasezoey committed Feb 5, 2023
1 parent 7b507d5 commit 0599ef2
Show file tree
Hide file tree
Showing 6 changed files with 189 additions and 1 deletion.
5 changes: 5 additions & 0 deletions docs/api/functions/getDiscriminatorModelForClass.md
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,11 @@ function getDiscriminatorModelForClass<U extends AnyParamConstructor<any>, Query

Option `value` is to overwrite the key the class is registered on as a discriminator, by default it is the generated model name, but can be overwritten with any string, recommended is to use a [string-`enum`](https://www.typescriptlang.org/docs/handbook/enums.html#string-enums) to keep track of names.

:::note
Note that [`existingConnection`](../decorators/modelOptions.md#existingconnection) and [`existingMongoose`](../decorators/modelOptions.md#existingmongoose) will not be used and instead will be registered on the `from` model's settings.
See [`Warning W002`](../../guides/error-warning-details.md#property-was-defined-differently-on-base-and-discriminator-w002).
:::

## Example

```ts
Expand Down
11 changes: 11 additions & 0 deletions docs/guides/error-warning-details.md
Original file line number Diff line number Diff line change
Expand Up @@ -538,3 +538,14 @@ Type of "${name}.${key}" is not ${type}, but includes the following ${extra} opt

Details:
The provided options (listed in the warning) do nothing with the provided `Type`

### Property was defined differently on base and discriminator [W002]

Warning:

```txt
Property "${property}" was defined on "${clName}", but is different from discriminator base "${fromName}", which is not supported! [W002]
```

Details:
The Property [`existingConnection`](../decorators/modelOptions.md#existingconnection) or [`existingMongoose`](../decorators/modelOptions.md#existingmongoose) were defined differently on the discriminator's base model and the discriminator itself, which is not supported.
12 changes: 12 additions & 0 deletions src/internal/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -700,6 +700,18 @@ export function warnNotCorrectTypeOptions(name: string, key: string, type: strin
}
}

/**
* Logs a warning for Discriminator setting a different "existing*" property than the base
* @param fromName Name of the Base Model
* @param clName Name of the Discriminator's class
* @param property The property defined that does not match
*/
export function warnNotMatchingExisting(fromName: string, clName: string, property: string) {
logger.warn(
`Property "${property}" was defined on "${clName}", but is different from discriminator base "${fromName}", which is not supported! [W002]`
);
}

/**
* Try to convert input "value" to a String, without it failing
* @param value The Value to convert to String
Expand Down
8 changes: 8 additions & 0 deletions src/typegoose.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import {
mapModelOptionsToNaming,
mergeMetadata,
mergeSchemaOptions,
warnNotMatchingExisting,
} from './internal/utils';

/* istanbul ignore next */
Expand Down Expand Up @@ -427,6 +428,13 @@ export function getDiscriminatorModelForClass<U extends AnyParamConstructor<any>
return models.get(name) as ReturnModelType<U, QueryHelpers>;
}

if (mergedOptions.existingConnection && mergedOptions.existingConnection !== from.db) {
warnNotMatchingExisting(from.modelName, getName(cl), 'existingConnection');
}
if (mergedOptions.existingMongoose && mergedOptions.existingMongoose !== from.base) {
warnNotMatchingExisting(from.modelName, getName(cl), 'existingMongoose');
}

const sch: mongoose.Schema<any> = buildSchema(cl, mergedOptions);

const mergeHooks = mergedOptions.options?.enableMergeHooks ?? false;
Expand Down
64 changes: 64 additions & 0 deletions test/tests/__snapshots__/warnings.test.ts.snap
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,70 @@ Array [

exports[`should throw a error if property is "Mixed" and Severity.ERROR is used 1`] = `"Setting \\"Mixed\\" is not allowed! (TestMixedError, test) [E017]"`;

exports[`should warn if "existingConnection" is defined differently on base and discriminator 1`] = `
[MockFunction] {
"calls": Array [
Array [
"Property \\"existingConnection\\" was defined on \\"DisDecoratorEC\\", but is different from discriminator base \\"DisBaseDecoratorEC\\", which is not supported! [W002]",
],
],
"results": Array [
Object {
"type": "return",
"value": undefined,
},
],
}
`;

exports[`should warn if "existingConnection" is defined differently on base and discriminator 2`] = `
[MockFunction] {
"calls": Array [
Array [
"Property \\"existingConnection\\" was defined on \\"DisParameterEC\\", but is different from discriminator base \\"DisBaseParameterEC\\", which is not supported! [W002]",
],
],
"results": Array [
Object {
"type": "return",
"value": undefined,
},
],
}
`;

exports[`should warn if "existingMongoose" is defined differently on base and discriminator 1`] = `
[MockFunction] {
"calls": Array [
Array [
"Property \\"existingMongoose\\" was defined on \\"DisDecoratorEM\\", but is different from discriminator base \\"DisBaseDecoratorEM\\", which is not supported! [W002]",
],
],
"results": Array [
Object {
"type": "return",
"value": undefined,
},
],
}
`;

exports[`should warn if "existingMongoose" is defined differently on base and discriminator 2`] = `
[MockFunction] {
"calls": Array [
Array [
"Property \\"existingMongoose\\" was defined on \\"DisParameterEM\\", but is different from discriminator base \\"DisBaseParameterEM\\", which is not supported! [W002]",
],
],
"results": Array [
Object {
"type": "return",
"value": undefined,
},
],
}
`;

exports[`should warn if "justOne" is defined, but no Virtual Populate Options 1`] = `
Array [
Array [
Expand Down
90 changes: 89 additions & 1 deletion test/tests/warnings.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { logger } from '../../src/logSettings';
import { buildSchema, modelOptions, mongoose, prop, Severity } from '../../src/typegoose';
import { buildSchema, getDiscriminatorModelForClass, getModelForClass, modelOptions, mongoose, prop, Severity } from '../../src/typegoose';
import * as utils from '../../src/internal/utils';

let spyWarn: jest.SpyInstance;
Expand Down Expand Up @@ -110,3 +110,91 @@ it('should warn if property is "Mixed" and a invalid Severity is used', () => {
expect(spyWarn).toHaveBeenCalledTimes(1);
expect(spyWarn.mock.calls).toMatchSnapshot();
});

it('should warn if "existingMongoose" is defined differently on base and discriminator', () => {
// test setting via decorator
{
@modelOptions({ existingMongoose: mongoose })
class DisBaseDecoratorEM {
@prop()
public baseDummy?: string;
}

@modelOptions({ existingMongoose: new mongoose.Mongoose() })
class DisDecoratorEM extends DisBaseDecoratorEM {
@prop()
public disDummy?: string;
}

const DisBaseModel = getModelForClass(DisBaseDecoratorEM);
getDiscriminatorModelForClass(DisBaseModel, DisDecoratorEM);

expect(spyWarn).toHaveBeenCalledTimes(1);
expect(spyWarn).toMatchSnapshot();
}

spyWarn.mockClear();

// test setting via options
{
class DisBaseParameterEM {
@prop()
public baseDummy?: string;
}

class DisParameterEM extends DisBaseParameterEM {
@prop()
public disDummy?: string;
}

const DisBaseModel = getModelForClass(DisBaseParameterEM, { existingMongoose: mongoose });
getDiscriminatorModelForClass(DisBaseModel, DisParameterEM, { existingMongoose: new mongoose.Mongoose() });

expect(spyWarn).toHaveBeenCalledTimes(1);
expect(spyWarn).toMatchSnapshot();
}
});

it('should warn if "existingConnection" is defined differently on base and discriminator', () => {
// test setting via decorator
{
@modelOptions({ existingConnection: mongoose.connection })
class DisBaseDecoratorEC {
@prop()
public baseDummy?: string;
}

@modelOptions({ existingConnection: mongoose.createConnection() })
class DisDecoratorEC extends DisBaseDecoratorEC {
@prop()
public disDummy?: string;
}

const DisBaseModel = getModelForClass(DisBaseDecoratorEC);
getDiscriminatorModelForClass(DisBaseModel, DisDecoratorEC);

expect(spyWarn).toHaveBeenCalledTimes(1);
expect(spyWarn).toMatchSnapshot();
}

spyWarn.mockClear();

// test setting via options
{
class DisBaseParameterEC {
@prop()
public baseDummy?: string;
}

class DisParameterEC extends DisBaseParameterEC {
@prop()
public disDummy?: string;
}

const DisBaseModel = getModelForClass(DisBaseParameterEC, { existingConnection: mongoose.connection });
getDiscriminatorModelForClass(DisBaseModel, DisParameterEC, { existingConnection: mongoose.createConnection() });

expect(spyWarn).toHaveBeenCalledTimes(1);
expect(spyWarn).toMatchSnapshot();
}
});

0 comments on commit 0599ef2

Please sign in to comment.