Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reusing schemas end up with no index creation #13054

Closed
1 task done
alegria20 opened this issue Feb 20, 2023 · 6 comments
Closed
1 task done

Reusing schemas end up with no index creation #13054

alegria20 opened this issue Feb 20, 2023 · 6 comments
Labels
help This issue can likely be resolved in GitHub issues. No bug fixes, features, or docs necessary

Comments

@alegria20
Copy link

alegria20 commented Feb 20, 2023

Prerequisites

  • I have written a descriptive issue title

Mongoose version

6.8.1

Node.js version

16.18.0

MongoDB version

5.0.14

Operating system

Linux

Operating system version (i.e. 20.04, 11.3, 10)

No response

Issue

I am a new user of mongoose library, and I have the following topic.

I have the need to reuse properties of schemas (I assume this is a pretty common need, please correct me if my assumption is wrong), e.g. :

const investorLegalIdProperty = {
  type: String,
  required: true,
  unique: true,
  minlength: 5,
  maxlenght: 50,
};

Then I have schemas that use this property as follows:

const investorSchema = new mongoose.Schema({
    name: {
      type: String,
      required: true,
      minlength: 2,
      maxlenght: 50,
    },
    familyName: {
      type: String,
      required: true,
      minlength: 2,
      maxlenght: 50,
    },
    legalId: investorLegalIdProperty,
  },
);

This approach was (in my naive mind) very convenient way to reuse the investorLegalIdProperty in different schemas, and has been working fine for the [type], the [minlenght] and the [maxlenght]. However that is not the case for [required] and [unique], which are completely ignored and indexes are not created accordingly.

My questions are:

  1. How can I make sure the required and unique values are not ignored?
  2. Is it a common need to reause properties of schemas? Is the approach I have followed a valid approach?
  3. Or is there any better or more common approach for this matter?

Thanks for any support.

@alegria20 alegria20 added help This issue can likely be resolved in GitHub issues. No bug fixes, features, or docs necessary help wanted labels Feb 20, 2023
@alegria20
Copy link
Author

alegria20 commented Feb 20, 2023

When I was reusing the schemaProperties somewhere else, I was there stating unique: false. As I am quite new to JS, I didnt pay attention that the variable seemed to be passed by reference which apply the modification to the original object.

So, for one side I am happy that I fixed the issue, but I would like to still mantain the questions related to "how to reuse schemas and schema properties".

Edit:
For instance, The main reason I started using properties aside, was for this specific case:

const investorSchema = new mongoose.Schema({
    name: {
      type: String,
      required: true,
      minlength: 2,
      maxlenght: 50,
    },
    familyName: {
      type: String,
      required: true,
      minlength: 2,
      maxlenght: 50,
    },
    legalId: {
      type: String,
      required: true,
      unique: true,
      minlength: 5,
      maxlenght: 50,
  },
);
const transferSchema = new mongoose.Schema({
  sender: {
    type: investorSchema,
    required: true,
    unique: false,
  },
  receiver: {
    type: investorSchema,
    required: true,
    unique: false,
  },
  amount: {
    type: Number,
    required: true,
    unique: false,
  },
});

The issue with this code that made me look for alternatives is that the legalId is defined as unique in the investorSchema, then I got duplicate issue when storing transfers with same investors.

@vkarpov15 vkarpov15 added this to the 6.9.5 milestone Feb 20, 2023
@vkarpov15 vkarpov15 added the needs repro script Maybe a bug, but no repro script. The issue reporter should create a script that demos the issue label Feb 20, 2023
@alegria20
Copy link
Author

alegria20 commented Feb 20, 2023

Just in case this might help someone else in the future, I could fix the issue by using clone(), as follows:

const investorSchemaModif = investorSchema.clone();
investorSchemaModif.path('legalId').index(false);

@vkarpov15
Copy link
Collaborator

We'll keep this open for now, this may be a bug

@vkarpov15 vkarpov15 reopened this Feb 20, 2023
@IslandRhythms
Copy link
Collaborator

IslandRhythms commented Feb 21, 2023

if you uncomment the lines, it will throw an error. Otherwise, it will behave like nothing is wrong.
Labeling as a bug because https://mongoosejs.com/docs/api.html#model_Model-init

const mongoose = require('mongoose');

const investorLegalIdProperty = {
  type: String,
  required: true,
  unique: true,
  minlength: 5,
  maxlenght: 50,
};

const investorSchema = new mongoose.Schema({
  name: {
    type: String,
    required: true,
    minlength: 2,
    maxlenght: 50,
  },
  familyName: {
    type: String,
    required: true,
    minlength: 2,
    maxlenght: 50,
  },
  legalId: investorLegalIdProperty,
});

const testSchema = new mongoose.Schema({
  age: Number,
  legalId: investorLegalIdProperty
});

const Test = mongoose.model('Test', testSchema);
const Investor = mongoose.model('Investor', investorSchema);
async function run() {
  await mongoose.connect('mongodb://localhost:27017');
  await mongoose.connection.dropDatabase();
  // await Test.init();
  // const indexes = await Test.listIndexes();
  // console.log(indexes);

  // await Investor.init();
  // const index = await Investor.listIndexes();
  // console.log(index);
  await Test.create({
    age: 1,
    legalId: 'THEID'
  });
  await Investor.create({
    name: 'Test',
    familyName: 'Testserson',
    legalId: 'THEID'
  });
  await Test.create({
    age: 1,
    legalId: 'THEID'
  });
  await Investor.create({
    name: 'Test',
    familyName: 'Testserson',
    legalId: 'THEID'
  });
  console.log('done');
}

run();

@IslandRhythms IslandRhythms added confirmed-bug We've confirmed this is a bug in Mongoose and will fix it. and removed help This issue can likely be resolved in GitHub issues. No bug fixes, features, or docs necessary help wanted needs repro script Maybe a bug, but no repro script. The issue reporter should create a script that demos the issue labels Feb 21, 2023
@lpizzinidev
Copy link
Contributor

@IslandRhythms @vkarpov15

Your script is not throwing an error if you leave the line un-commented because the dropDatabase will cancel the unique keys constraints.
If you init both models again it will correctly throw an error since you are violating the unique: true constraint on the create operations.

If we run:

const mongoose = require('mongoose');

const investorLegalIdProperty = {
  type: String,
  required: true,
  unique: true,
  minlength: 5,
  maxlenght: 50,
};

const investorSchema = new mongoose.Schema({
  name: {
    type: String,
    required: true,
    minlength: 2,
    maxlenght: 50,
  },
  familyName: {
    type: String,
    required: true,
    minlength: 2,
    maxlenght: 50,
  },
  legalId: investorLegalIdProperty,
});

const testSchema = new mongoose.Schema({
  age: Number,
  legalId: investorLegalIdProperty
});

const Test = mongoose.model('Test', testSchema);
const Investor = mongoose.model('Investor', investorSchema);

async function run() {
  await mongoose.connect('mongodb://localhost:27017');
  await mongoose.connection.dropDatabase();
  await Test.init();
  const indexes = await Test.listIndexes();
  console.log(indexes);

  await Investor.init();
  const index = await Investor.listIndexes();
  console.log(index);
  
  await Test.create({
    age: 1,
    legalId: 'THEID'
  });
  await Investor.create({
    name: 'Test',
    familyName: 'Testserson',
    legalId: 'THEID'
  });
  await Test.create({
    age: 1,
    legalId: 'THEID2'
  });
  await Investor.create({
    name: 'Test',
    familyName: 'Testserson',
    legalId: 'THEID2'
  });
  console.log('done');

  process.exit(0);
}

run();

We won't get any errors.
So, in my opinion, there are no bugs here.

@vkarpov15
Copy link
Collaborator

@lpizzinidev is right, @IslandRhythms 's repro script is incorrect. If you don't wait for indexes to finish building, then you won't get unique constraint.

@vkarpov15 vkarpov15 removed this from the 6.10.2 milestone Mar 7, 2023
@vkarpov15 vkarpov15 added help This issue can likely be resolved in GitHub issues. No bug fixes, features, or docs necessary and removed confirmed-bug We've confirmed this is a bug in Mongoose and will fix it. labels Mar 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help This issue can likely be resolved in GitHub issues. No bug fixes, features, or docs necessary
Projects
None yet
Development

No branches or pull requests

4 participants