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

Invalid keys error (DBRef issue) #148

Closed
websirnik opened this issue Oct 17, 2014 · 26 comments
Closed

Invalid keys error (DBRef issue) #148

websirnik opened this issue Oct 17, 2014 · 26 comments

Comments

@websirnik
Copy link

Just updated to 3.0.

Having the following error when saving document(even if nothing has changed):
http://f.cl.ly/items/3F1p45162Y1c1Y2B0Z3P/Screen%20Shot%202014-10-17%20at%2016.51.29.png

@jeromelebel
Copy link
Owner

Could you gie me the document?

@websirnik
Copy link
Author

@jeromelebel Here:

{
  "_id": ObjectId("542f5ef2c34fc26e1b8b71c8"),
  "created": new Date("2014-10-04T03:44:02+0100"),
  "isPublic": false,
  "postCount": 9,
  "slug": "b2b-top-picks",
  "title": "B2B Top Picks",
  "updated": new Date("2014-10-06T06:09:39+0100"),
  "user": {
    "$ref": "User",
    "$id": "5374d465c43a034f528b45ba",
    "$db": "db_name"
  }
}

@jeromelebel
Copy link
Owner

You are not supposed to have keys that start with $ But there is an issue in MongoHub since it's possible to insert a document with those keys.

@websirnik
Copy link
Author

@jeromelebel It's a Mongo thing when you are doing DB reference: http://docs.mongodb.org/manual/reference/database-references/#dbrefs
This should be supported.

In previous MongoHub 2.* version this was not an issue.

@jeromelebel
Copy link
Owner

The issue here, the document doesn't content a dbref, but it contains a value with "$ref", "$id" and "$db". When the document was inserted it should have been converted to a dbref. Was it inserted with mongohub?

MongoHub is based on the c driver. I think there is an issue with dbref that doesn't support database name. And the true is I didn't put a lot of time in dbref. I don't think it is well supported anyway.

@websirnik
Copy link
Author

May be I don't understand you. DBref itself is a convention. This is an implementation on DBref:

{
    "$ref": "User",
    "$id": "5374d465c43a034f528b45ba",
    "$db": "db_name"
}

Previously, it was supported in MongoHub(may be unintentionally:) ). Is it possible to revert the check for '$'?

@jeromelebel
Copy link
Owner

$ref, $id and $db are used as convention to represent a type that doesn't in pure json. if you use pure json to display a document, you should see $ref, $id, $db. If it is json supported by mongodb (like in the console) you should see DBRef().

The c driver doesn't seem to support the third parameter $db. In this page, there is no mention of $db too : http://docs.mongodb.org/manual/reference/mongodb-extended-json/#db-reference

So I'm not sure what to do yet with $db.

Also keys that starts with $ should not be allowed. MongoHub uses an API to insert new documents that doesn't check if there is a key with $. But to save a document, the API does the check. Therefore, you can create a document like that, but you can't modify.

So there are 2 issues. The first one, you should not be allowed to insert a document like that from mongohub. But you should write : {
"_id": ObjectId("542f5ef2c34fc26e1b8b71c8"),
"created": new Date("2014-10-04T03:44:02+0100"),
"isPublic": false,
"postCount": 9,
"slug": "b2b-top-picks",
"title": "B2B Top Picks",
"updated": new Date("2014-10-06T06:09:39+0100"),
"user": DBRef("User", "5374d465c43a034f528b45ba", "db_name")
}
(since with the MongoHub we don't use pure json but mongodb json)

The second issue is I don't know what to do the third parameter "db_name", since the c driver doesn't support it.

How do you use such a document like that?

@jeromelebel
Copy link
Owner

I created 2 issues in mongodb that I would need to solve this issue:
https://jira.mongodb.org/browse/CDRIVER-444
https://jira.mongodb.org/browse/CDRIVER-443

@websirnik
Copy link
Author

@jeromelebel Sidequestion, where can I download 2.* version?

@jeromelebel
Copy link
Owner

https://mongohub.s3.amazonaws.com/MongoHub-2.6.2.zip That's the only version available.

jeromelebel pushed a commit to jeromelebel/MongoObjCDriver that referenced this issue Nov 5, 2014
@jeromelebel
Copy link
Owner

It should be fixed with 3.0.7. I let you close this issue if it works for you.

@websirnik
Copy link
Author

Updated to 3.0.7. Getting the same error :(

@jeromelebel
Copy link
Owner

But when you try to modify a document, do you see:
"user": {
"$ref": "User",
"$id": "5374d465c43a034f528b45ba",
"$db": "db_name"
}

or:

"user": DBRef("db_name.User", "5374d465c43a034f528b45ba")

?

@websirnik
Copy link
Author

I see properties with $
screen_shot_2014-11-05_at_15_30_20

@jeromelebel
Copy link
Owner

and in the find tab, do you see "user" in your document as "DBRef()" or as an object with 3 items?

@websirnik
Copy link
Author

Same
screen_shot_2014-11-05_at_15_40_41

@jeromelebel
Copy link
Owner

The issue is mongodb is supposed to refuse any document with a key that starts with $.

Your document should be { "_id": ObjectId("xxx"), "user": DBRef("mydb.User", "yyy") } inside the database. But according to the screenshot, your document is { "_id": ObjectId("xxx"), "user": { "$ref":"User", "$db": "mydb", "$id": "yyy"} }

I'm not sure I want mongohub to change the structure of your document for you. And I can't remove the sanity check that are done in the c driver (the c driver is done by mongodb, not by me).

I wonder if your application would be still ok to receive DBRef() instead of {"$ref":...}

@jeromelebel
Copy link
Owner

So I found the solution for the issue I created to mongodb with https://jira.mongodb.org/browse/CDRIVER-444 (the way to display the database with DBRef() )

But https://jira.mongodb.org/browse/CDRIVER-443 is still pending (the sanity check for keys that start with $)

@websirnik
Copy link
Author

I don't want MongoHub to mess up with my data and auto-convert $ref to DBRef().
My app interacts with Mongo through Doctrine ODM, $ notation is coming from there.

@jeromelebel
Copy link
Owner

You should create a bug on Doctrine ODM, since they don't use DBRef() type. As soon as mongodb answers about the sanity check I will update MongoHub (if they removes the sanity check). If they don't, I won't be able to solve your issue, and Doctrine ODM will have to be fixed.

@jeromelebel
Copy link
Owner

If I'm not mistaken, Doctrine ODM is using the php driver for mongodb. And the issue seems to be in the php driver.
I created this issue: https://jira.mongodb.org/browse/PHP-1262

@jeromelebel jeromelebel changed the title Invalid keys error Invalid keys error (DBRef issue) Nov 5, 2014
@websirnik
Copy link
Author

I don't think it's a bug, since the $ notation is proposed by Mongo, and I guess it's up to drivers to support it or not.
At least MongoHub 2.* works well for me)

@jeromelebel
Copy link
Owner

It doesn't behave as you expect it, so I think it's a bug. The questions are what is the expected behavior, and what should be modify so everything works smoothly.

I think all drivers should behave the same so we get the same data inside the database. But anyway, I don't do the php driver, and I don't do the c driver. Both are done by mongodb. So they will change one or the other behavior.

It used to work in MongoHub 2 because I was using the legacy c driver.

Right now, it is possible to insert document with $ with MongoHub 3, but it's not possible to modify them because of the sanity check. The c driver has just been modify by the people of mongodb. Therefore soon, MongoHub will not be able to insert document with $:
https://jira.mongodb.org/browse/CDRIVER-443

It seems that mongodb people push this policy further. So it seems this bug is "the php driver should not have inserted documents with $ in it". So I hope they will fix the php driver to use the DBRef type instead of using this bad workaround with $ref, $db, and $id. This type has been created to store those information and it should be used.

I can't do nothing on MongoHub since I don't work on the c driver. I can't remove the sanity check.

@jmikola
Copy link

jmikola commented Nov 5, 2014

@jeromelebel: I just came across this issue from PHP-1262.

So it seems this bug is "the php driver should not have inserted documents with $ in it". So I hope they will fix the php driver to use the DBRef type instead of using this bad workaround with $ref, $db, and $id. This type has been created to store those information and it should be used.

@websirnik was correct in saying that DBRef is purely a convention. There is no DBRef BSON type.

The mongo shell happens to render DBRef objects as DBRef(<collection name>, <identifier value>), but the actual data stored in the BSON document is simply an embedded object. The drivers and server should make a special allowance for $ref and $id as the first and second fields, respectively, and an optional third field, $db. In normal documents (e.g. excluding database profiler records), this two or three-field sequence is the only time that $-prefixed field names are permitted. Additionally, since the DBRef is really just an embedded object, MongoDB allows additional field names in the object, which some ODMs use to store class names.

Unfortunately, the mongo shell only shows the collection name ($ref) and identifier ($id) when it renders these embedded objects as DBRef() on the command line. You'll notice that the optional $db field and any subsequent fields are never rendered, which is actually quite problematic. Consider:

$ mongo
MongoDB shell version: 2.6.4
connecting to: test
> db.foo.drop()
true
> db.foo.insert({user:{$ref:'users',$id:1,$db:'test',extra:'data'}})
WriteResult({ "nInserted" : 1 })
> db.foo.findOne()
{ "_id" : ObjectId("545a9bfc9499b864be6f3e1f"), "user" : DBRef("users", 1) }
>

The PHP driver's MongoDBRef class is simply a utility class for constructing embedded documents (i.e. associative arrays/hashes in PHP) that follow the DBRef convention. In this particular case, the PHP driver or Doctrine ODM is not to blame. Rather, I believe the C driver's strictness needs to be relaxed.

@jeromelebel
Copy link
Owner

Sorry, I made a mistake between BSON_TYPE_DBPOINTER type and DBRef(). But I still stuck with the c driver.

as soon as is resolved, I could do something with MongoHub: https://jira.mongodb.org/browse/CDRIVER-457

@jeromelebel
Copy link
Owner

Sorry about my confusion between the types. 3.0.8 with the latest c driver should work now for you.

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

No branches or pull requests

3 participants