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

SERVER-9790 Add this.$db || db.toString() to DBRef string representation #638

Closed
wants to merge 4 commits into from

Conversation

sl33nyc
Copy link
Contributor

@sl33nyc sl33nyc commented Feb 18, 2014

If the optional $db argument is specified in the $DBRef, prepend it before the collection name. Otherwise, prepend the current database before the collection name.

@@ -393,6 +398,10 @@ if (typeof(DBRef) != "undefined"){
return this.toString();
}

DBRef.prototype.getDb = function(){
return this.$db || db.toString();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do not default to current DB if undefined.

@scotthernandez
Copy link
Contributor

There should also be jstests for this. See:
jstests/shelltypes.js
jstests/constructors.js

@sl33nyc
Copy link
Contributor Author

sl33nyc commented Feb 19, 2014

Removed default DBRef.$db, added optional $db argument to DBRef v8 constructor, and more jstests.


db.getMongo().getDB("otherdb").dropDatabase();
var objid = new ObjectId();
db.getMongo().getDB("otherdb").getCollection("othercoll").insert({_id:objid, field:"value"});
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

consider using db.getSiblingDB("otherdb") instead of getMongo().getDB(). This is the convention in most of the JS tests.

@sl33nyc
Copy link
Contributor Author

sl33nyc commented Jun 5, 2014

@benety, one more pass please.

@benety
Copy link
Contributor

benety commented Jun 5, 2014

LGTM. @scotthernandez ?

}

DBRef.prototype.tojson = function(indent){
return this.toString();
}

DBRef.prototype.getDb = function(){
return this.$db || null;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably more correct to replace null with undefined, to indicate it is not set.

Or return the current db. If that is done then fetch can be changed to:
return db.getSiblingDB(getDb()).getCollection(getCollection()).find({ _id : this.$id }):

@scotthernandez
Copy link
Contributor

LGTM, aside from my previous note.

@@ -386,13 +386,18 @@ if (typeof(DBRef) != "undefined"){
DBRef.prototype.fetch = function(){
assert(this.$ref, "need a ns");
assert(this.$id, "need an id");
return db[ this.$ref ].findOne({ _id : this.$id });
var coll = this.$db ? db.getSiblingDB(this.$db).getCollection(this.$ref) : db[this.$ref];
return coll.findOne({ _id : this.$id }):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

colon at end of line?

@sl33nyc
Copy link
Contributor Author

sl33nyc commented Jun 5, 2014

Whoops! Typo fixed.

@benety
Copy link
Contributor

benety commented Jun 6, 2014

Thanks. Running patch through CI now.

@benety
Copy link
Contributor

benety commented Jun 6, 2014

Hi Stephen,

This pull request has been merged in commit 0cc8d91.

Thank you for contributing!
Ben

@benety benety closed this Jun 6, 2014
jiongle1 pushed a commit to scantist-ossops-m2/mongo that referenced this pull request Mar 30, 2024
jiongle1 pushed a commit to scantist-ossops-m2/mongo that referenced this pull request Mar 30, 2024
jiongle1 pushed a commit to scantist-ossops-m2/mongo that referenced this pull request Mar 30, 2024
Use atomic operations to swap new page structures into place.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants