Skip to content
This repository has been archived by the owner on Jun 29, 2021. It is now read-only.

collection._makeNewID should have an option for blank ObjectID #428

Open
jankapunkt opened this issue Apr 14, 2021 · 5 comments
Open

collection._makeNewID should have an option for blank ObjectID #428

jankapunkt opened this issue Apr 14, 2021 · 5 comments

Comments

@jankapunkt
Copy link

Currently a new collection's idGeneration options allow either the default Meteor String _id or a Mongo ObjectID with a ranomd 24byte hex String:

switch (options.idGeneration) {
case 'MONGO':
  this._makeNewID = function () {
    var src = name ? DDP.randomStream('/collection/' + name) : Random.insecure;
    return new Mongo.ObjectID(src.hexString(24));
  };
  break;
case 'STRING':
default:
  this._makeNewID = function () {
    var src = name ? DDP.randomStream('/collection/' + name) : Random.insecure;
    return src.id();
  };
  break;
}

Defined here: https://github.com/meteor/meteor/blob/devel/packages/mongo/collection.js#L61

However, I have to create ObjectIDs without any parameter to conform with the way they are created by other services we are running. I would like to add another parameter 'MONGO_PLAIN' that returns an ObjectID using the default Mongo-builtin algorithm:

case 'MONGO_PLAIN':
  this._makeNewID = function () {
    return new Mongo.ObjectID();
  };
  break;

I could provide a PR + docs update. Any objections against this?

@jankapunkt
Copy link
Author

Ok now I'm confused. I created a custom wrapper for EJSON to support native Mongo, which itself runs fine:

import { MongoInternals } from 'meteor/mongo'
import { Random } from 'meteor/random'
import { EJSON } from 'meteor/ejson'

const ObjectID = MongoInternals.NpmModule.ObjectID

class NativeObjectId extends ObjectID {
  toString () {
    const str = this.toHexString()
    return `ObjectID("${str}")`
  }

  clone () {
    return new ObjectID(this.toHexString())
  }

  typeName () {
    return 'nativeoid'
  }

  equals (other) {
    return other instanceof ObjectID && this.toHexString() === other.toHexString()
  }

  toJSONValue () {
    return this.toHexString()
  }
}

EJSON.addType('nativeoid', str => new NativeObjectId(str))

module.exports = {
  ObjectID: (str = undefined) => new NativeObjectId(str)
}

and I also can monkey-patch _makeNewID when creating a new collection:

import { ObjectID } from 'path/to/myObjectId'

const collection = new Mongo.Collection(null)
collection._makeNewID = function () {
  return new ObjectID()
}

but on insert I get Error: Meteor does not currently support objects other than ObjectID as ids. So I checked the code and found that MongoId (which is the cause of this error) is baked into the system super deep:

Therefore Meteor prevents me from it's very core to use simple native Mongo ObjectId() and I don't understand the reason why.

@sebakerckhof
Copy link

There's a lot of places in the Meteor code that will fail when using native object ids. For starters, everything that relies on the interface MongoId provides. e.g. IdMap, which is used in a lot places (minimongo collections, observer multiplexers, some parts of the cursor functionality, oplog/polling query drivers, ...)

So let's say you change all that, then it's still a question whether it will work on the client side.
If it works, it's by a stroke of luck that Meteor includes the right polyfills (like for Buffer) and that these polyfills are ok (some functionality is just not possible on the client-side and therefore some methods are just stubs).

But even then, it means we would now be including some huge polyfills (like Buffer) by default. Which I don't think would be acceptable.

@jankapunkt
Copy link
Author

I see so it's mainly the Buffer dependency that would leak into the client that prevents this?

But the feature of isomorphic DBs is deprecated since allow/deny became deprecated, right? Wouldn't it work out to use native ObjectID on the Server and use the MongoID as a wrapper on the client? Just curious to find a way to make it work.

@chmelevskij
Copy link

Question, so for example, is this error related to the issue?

"MinimongoError: The _id field cannot be changed from {_id: "ObjectID("607a9d6e8e9250758fc00e16")"} to {_id: "607a9d6e8e9250758fc00e16"} [409]"

@StorytellerCZ
Copy link
Collaborator

@jankapunkt I would like to see this added as there is a clear use case for it. Though as @sebakerckhof pointed out there are significant hurdles to overcome. I would like to start a larger project of improving Mongo and MiniMongo in Meteor to bring many of the new things from MongoDB over to Meteor and I think this should be one of the things to tackle.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants