Meteor-Community-Packages / ground-db

GroundDB is a thin layer providing Meteor offline database and methods
https://atmospherejs.com/ground/db
MIT License
572 stars 77 forks source link

MongoId strings not returning. #195

Open CyberCyclone opened 7 years ago

CyberCyclone commented 7 years ago

I have a Ground subscription that contains MongoIDs in string form as the main id. When I do a find, it lists it out, however when I do a find or findOne on the id, it doesn't return anything.

Here's an example of one that fails:

DRAFTS.findOne({_id:"58d887874e0ca825443236cf"})

I suspect that it has something to do with mongo-id package and this function: MongoID._looksLikeObjectID(id)

I haven't been able track it down at all though.

raix commented 7 years ago

Does DRAFTS.findOne('58d887874e0ca825443236cf') work as expected?

CyberCyclone commented 7 years ago

No, it behaves the same way that {_id:"58d887874e0ca825443236cf"} is. If the id is not 24 characters, then it works as expected. It's only when there are 24 characters where it fails.

Chun-Yang commented 7 years ago

The problem is in the MongoID package, idStringify added '-' before the id when fetching the document. LINE 64

MongoID.idStringify = function (id) {                                                                 // 56
  if (id instanceof MongoID.ObjectID) {                                                               // 57
    return id.valueOf();                                                                              // 58
  } else if (typeof id === 'string') {                                                                // 59
    if (id === "") {                                                                                  // 60
      return id;                                                                                      // 61
    } else if (id.substr(0, 1) === "-" || // escape previously dashed strings                         // 62
               id.substr(0, 1) === "~" || // escape escaped numbers, true, false                      // 63
               MongoID._looksLikeObjectID(id) || // escape object-id-form strings                     // 64
               id.substr(0, 1) === '{') { // escape object-form strings, for maybe implementing later
      return "-" + id;                                                                                // 66
    } else {                                                                                          // 67
      return id; // other strings go through unchanged.                                               // 68
    }                                                                                                 // 69
  } else if (id === undefined) {                                                                      // 70
    return '-';                                                                                       // 71
  } else if (typeof id === 'object' && id !== null) {                                                 // 72
    throw new Error("Meteor does not currently support objects other than ObjectID as ids");          // 73
  } else { // Numbers, true, false, null                                                              // 74
    return "~" + JSON.stringify(id);                                                                  // 75
  }                                                                                                   // 76
}; 

Meteor Minimongo works fine since it stores the ids with '-' prefix, but not for ground db.

Chun-Yang commented 7 years ago

It seems we should not use the custom strid, we should use MongoID.idStringify instead. https://github.com/GroundMeteor/db/blob/grounddb-caching-2016/lib/client/ground.db.js#L40

lucfranken commented 6 years ago

@Chun-Yang did you ever find a solution for this issue? We encounter the same with id's generated by:

new Meteor.Collection.ObjectID()._str

for example an ID:

8298e2f637a1a4b9e114abe8

24 characters it is.

If the issue is not resolvable it would be good to throw an error when an unrecognized ID type is sent into find() and findOne() methods. That way you understand as a developer that this is an ID issue and not a synchronization / data issue.