mongoose-sequence icon indicating copy to clipboard operation
mongoose-sequence copied to clipboard

Embedded document: add auto increment id does not work.

Open Brampage opened this issue 7 years ago • 13 comments

The example shows that the auto increment does not work on embedded documents.

This does not work: The parent document implementation with mongoose-sequence The embedded document implementation

The parent document's auto increment id does work, however when I add an auto increment id on my embedded document I get this error: Error: Counter already defined for field "_id" at Sequence.getInstance (C:\Users\\Desktop\Programming\MonDoAPI\node_modules\mongoose-sequence\lib\sequence.js:69:15) at Schema.plugin (C:\Users\...\Desktop\Programming\MonDoAPI\node_modules\mongoose\lib\schema.js:1139:3) at Object.<anonymous> (C:\Users\...\Desktop\Programming\MonDoAPI\models\sensorModule.model.js:21:19) at Module._compile (module.js:570:32) at Object.Module._extensions..js (module.js:579:10) at Module.load (module.js:487:32) at tryModuleLoad (module.js:446:12) at Function.Module._load (module.js:438:3) at Module.require (module.js:497:17) at require (internal/module.js:20:19) at Object.<anonymous> (C:\Users\...\Desktop\Programming\MonDoAPI\controllers\sensorModule.controller.js:1:84) at Module._compile (module.js:570:32) at Object.Module._extensions..js (module.js:579:10) at Module.load (module.js:487:32) at tryModuleLoad (module.js:446:12) at Function.Module._load (module.js:438:3) [nodemon] app crashed - waiting for file changes before starting...

Brampage avatar Feb 14 '17 15:02 Brampage

Hello. Please, can you try to specify the id in the options of the plugin? Like That:

// for the sensorModule schema
SensorModelSchema.plugin(AutoIncrement, {id: 'sensor_model_id_counter'});

// and for the sensor schema
SensorSchema.plugin(AutoIncrement, {id: 'sensor_id_counter'});

ramiel avatar Feb 15 '17 15:02 ramiel

Any news on this? Did it worked?

ramiel avatar Mar 12 '17 21:03 ramiel

@ramiel

I noticed that you're using a SequenceArchive singleton to verify existing fields. Which means that whenever a new Schema sets the same field as counter, the singleton checks the fields based on all the mongoose defined Schemas.

I also noticed that the method existSequence only checks for the field, not the Schema itself. Maybe if we also check the Schema's name we could fix this issue?

SequenceArchive.prototype.existsSequence = function(id) {
    var seq;
    for (var i = 0, len = this.sequences.length; i < len; i++) {
        seq = this.sequences[i];
        if (_.isEqual(seq.id, id)) // Only checks the associated id
            return true;
    }

    return false;
};

That might trigger the error that @Castrovalva is getting.

rfns avatar Mar 14 '17 13:03 rfns

I think this won't fix the problem. If we change the way you say it will avoid the problem from @Castrovalva but then it will increment the same counter for the two different schemas. Maybe I'm wrong or I can't see what you're sayng (easily both) and so if you want to provide a PR with the modification you're welcome. In any case my idea is to

  • Have the id to be mandatory or
  • Generate a better id (if not provided) depending on schema/field

I prefer the first solution as it is robust even for cross-application counters.

ramiel avatar Mar 14 '17 14:03 ramiel

You're right. I reviewed the collection where the counters are being saved and noticed that mongoose-sequence works by using property names only, ignoring the collection/schema's name. This would indeed cause problems for usages other than the OP.

By enforcing a mandatory rule for the id name, wouldn't that generate a breaking change? I guess most of the users are using _id or id as counter for convenience sake.

Maybe I could do a PR to add the schema's name to the counters collection, this follows your second idea.

Now the problem would be how to put that property into an existing counter collection. That might also break the current implementations.

rfns avatar Mar 20 '17 12:03 rfns

Yes I know enforcing the id is a breaking change and so the new version would be incompatible with the previous. As you say, considering the schema name can lead to another breaking change. Sadly I think the problem is due to a poor design of mine. I think I'll go for the mandatory id solution because it is easier and it has the same guarantees than the other. And, as a bonus point, it will not break any implementation which already set the id. If you think we can manage adding the schema name to the counter without breaking any implementation, please send your PR, it would be awesome!

ramiel avatar Mar 22 '17 10:03 ramiel

Enforcing the name ids would not cause any database breaking change (only API), however it would keep the rubbish counters.

Uhm... what if when boostraping the plugin it could fetch the highest ids from each plugin enabled schema and assume that this would be the correct one to reassign to the Counters collection? I mean, counters are incremental only right? So the plugin shouldn't care about the lower ids, neither the database nor the application.

So maybe it could do something like that:

  1. Checks if the current schema is already up-to-date with the new format.
  2. If not, findOne with sort({ _id: -1 }) for each plugin enabled schema.
  3. Stores a reference to this collection and it's id (counter).
  4. ~After all references are stored, checks the Counter collection for the conflicting ids.~
  5. ~If there's a conflict then updates the counter with n + 1, where n = current schema id + index of conflicting schema (that's to assure the plugin wont use another matching id).~
  6. Adds the schema or colllection name property using the stored reference name and updates the counter related to the schema.

The reference could be something like this:

{ name: "Collection", counter: 2382732 }

I'm gonna fork this project and start implementing this idea after your thoughts.

Ahh, btw this might works for both approaches.

rfns avatar Apr 10 '17 18:04 rfns

i am still facing ths issue . any solution ?

vamshi9666 avatar May 05 '18 12:05 vamshi9666

If you specify the Id for the counter you should have no problem as said in other comments.

ramiel avatar May 05 '18 12:05 ramiel

It would be great if someone posts solution for this

darsh-sn avatar Nov 26 '18 09:11 darsh-sn

For someone with the same problem, and who did not understand the answer as I did in the beginning, this was my solution:

My code with the error:


const commentsSchema = new Schema({
    _id: Number,
   some...
});
commentsSchema.plugin(AutoIncrement);

module.exports = mongoose.model("Comments", commentsSchema);


and it was fixed by changing the autoincrement part to this: commentsSchema.plugin(AutoIncrement, {id: '<autoincrement_name>', inc_field: '_id'});

for example: commentsSchema.plugin(AutoIncrement, {id: 'comments_id_counter', inc_field: '_id'});

OrlandoRibera avatar Nov 09 '20 18:11 OrlandoRibera

For someone with the same problem, and who did not understand the answer as I did in the beginning, this was my solution:

My code with the error:


const commentsSchema = new Schema({
    _id: Number,
   some...
});
commentsSchema.plugin(AutoIncrement);

module.exports = mongoose.model("Comments", commentsSchema);

and it was fixed by changing the autoincrement part to this: commentsSchema.plugin(AutoIncrement, {id: '<autoincrement_name>', inc_field: '_id'});

for example: commentsSchema.plugin(AutoIncrement, {id: 'comments_id_counter', inc_field: '_id'});

This resolved my issue.

neilthawani avatar Jan 05 '21 22:01 neilthawani

Thanks, that solved my problem!

Facundojimenez avatar Aug 03 '22 19:08 Facundojimenez