SWIFT-268 Store bson_oid_t in ObjectId rather than creating one each time we need it

[patrickfreed]

SWIFT-268

As part of this update, I refactored the public oid property on ObjectId that returns the string representation to hex, which is more descriptive. It also aligns with Go driver's Hex() and Java driver's toHexString().

I also refactored init(fromPointer:) to init(copying:), similar to the memory leak fix refactors.

[kmahar]

code changes lgtm, couple questions

[kmahar]

any opinions on whether we should keep this parameter label? I feel like just from is more in line with the rest of the driver, and the String isn't needed to disambiguate from the decodable initializer.

[patrickfreed]

I agree fromString should probably be replaced. In fact, I kind of think the entire initializer should be removed, and init?(ifValid: String) should be the only one, with the ifValid label being replaced with either from or hex or fromHex.

The libbson docs do not mention what happens if bson_oid_init_from_string is called with an invalid string, so I guess we can say it's undefined behavior. I really think we should minimize the amount of UB easily available through our API if at all possible. Thoughts?

[kmahar]

I like the idea of having a single failable initializer in the public API. that is consistent with how we handle Decimal128s.

[patrickfreed]

Refactored init?(ifValid: String) to just init?(_: String). Let me know if you think it'd be better with a label. Also, removed the un-failable init.

[kmahar]

I could go either way.... it might be useful as an indicator about the type of string, but maybe that's not that necessary.

[patrickfreed]

@mbroadst any strong opinions on this one? I feel like I could go either way too. The one benefit of doing it without a label is that it looks just like it would in the shell, though I doubt we generally make many driver design choices based on how the shell behaves.

[kmahar]

lgtm