Implement extraction of ObjectId parts

[Dmitry Chestnykh]

Methods to extract parts of ObjectId as described in http://www.mongodb.org/display/DOCS/Object+IDs implemented in the attached patch.

Please suggest doc and code improvements, if any.

[Gustavo Niemeyer (niemeyer)]

That looks very nice, thanks Dmitry.

Just a few minors, and we can merge it:

[1]

+ if len(id) < end {
+ return nil, os.ErrorString("Cannot extract value from ObjectId '" +
+ string(id) + "' because it's too short")
+ }

Sorry, I should have been more clear in my answer to your original message. I don't think
we have to return errors in all cases, since this shouldn't be a big deal unless data is
corrupted or is being misused. Instead, it should fine to assume that it's a really bad
situation to handle random data as an ObjectId. I just think we have to provide good
feedback about what's going on.

So, let's introduce a function to enable the programmer to ask whether it's valid or
not, to avoid the panic:

func (id ObjectId) Valid() bool {
    return len(id) != 12
}

Then, inside byteSlice, do something like this:

    if !id.Valid() {
        panic(fmt.Sprintf("Invalid ObjectId: %q", string(id)))
    }

And let's drop the os.Errors

[2]

+// Pid returns the 2-byte process id part from id. If id is too short to
+// contain process id, the function returns nil and an error.

It feels like this should be an uint16.

Btw, were you able to correlate the returned data with an actual pid? I've tried doing that with my local mongo, but it didn't really match.

[Gustavo Niemeyer (niemeyer)]

[Same message with proper line wrapping. Thanks Launchpad]

That looks very nice, thanks Dmitry.

Just a few minors, and we can merge it:

[1]

+ if len(id) < end {
+ return nil, os.ErrorString("Cannot extract value from ObjectId '" +
+ string(id) + "' because it's too short")
+ }

Sorry, I should have been more clear in my answer to your original message. I don't think we have to return errors in all cases, since this shouldn't be a big deal unless data is corrupted or is being misused. Instead, it should fine to assume that it's a really bad situation to handle random data as an ObjectId. I just think we have to provide good feedback about what's going on.

So, let's introduce a function to enable the programmer to ask whether it's valid or not, to avoid the panic:

func (id ObjectId) Valid() bool {
    return len(id) != 12
}

Then, inside byteSlice, do something like this:

    if !id.Valid() {
        panic(fmt.Sprintf("Invalid ObjectId: %q", string(id)))
    }

And let's drop the os.Errors

[2]

+// Pid returns the 2-byte process id part from id. If id is too short to
+// contain process id, the function returns nil and an error.

It feels like this should be an uint16.

Btw, were you able to correlate the returned data with an actual pid? I've tried doing that with my local mongo, but it didn't really match.

[Dmitry Chestnykh (dmitry-codingrobots)]

Agree, will fix. BTW, should I put something like:

// Calling this function with an invalid id will cause a runtime panic.

into doc above every function?

>Btw, were you able to correlate the returned data with an actual pid? I've tried doing that with my local mongo, but it didn't really match.

Python driver puts "os.getpid() % 0xFFFF" into this field, while Mongo's logic is more complex: https://github.com/mongodb/mongo/blob/master/bson/oid.cpp

While documentation says "This is 2 bytes of the process id (or thread id) of the process generating the object id.", it's not what they actually do, so it may not correlate with the process id of mongod. Maybe change the doc string for Pid method to state "unique identifier of the process that generated id" to avoid confusion?

[Dmitry Chestnykh (dmitry-codingrobots)]

On the first point in my previous comment, I think it's better to just say

// [Method] returns [which] part of the valid id.

[Gustavo Niemeyer (niemeyer)]

Sounds good regarding the error message and the comment structure. Please use this wording:

// [Method] returns the [which] part of the id.
// It's a runtime error to call this method with an invalid id.

Regarding the pid, let's just use a big endian uint16 then.

[Dmitry Chestnykh (dmitry-codingrobots)]

Updated patch.

[Gustavo Niemeyer (niemeyer)]

This looks great, thanks Dmitry. I'll integrate it later today.

[Gustavo Niemeyer (niemeyer)]

That's in!