[JAVA-609] ObjectId equals(...) broken Created: 24/Jul/12  Updated: 31/Mar/15  Resolved: 25/Jun/13

Status: Closed
Project: Java Driver
Component/s: API
Affects Version/s: 2.7.3
Fix Version/s: 3.0.0

Type: Bug Priority: Critical - P2
Reporter: Oliver Gierke Assignee: Jeffrey Yemin
Resolution: Done Votes: 0
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified


 Description   

ObjectId's equals(...) method is implemented invalidly. equals(...) methods need to be symmetrical, which means if A.equals(B) is true, B.equals(A) needs to be true as well. This is not the case as the implementation massages String values into ObjectId instances and thus surprisingly the following code succeeds:

ObjectId left = new ObjectId();
String right = left.toString();
 
assertThat(left, is(right)); // succeeds

This breaks once you test for symmetry:

assertThat(right, is(left)); // fails



 Comments   
Comment by Jeffrey Yemin [ 31/Mar/15 ]

Closing all resolved 3.0.0 issues, as 3.0.0 has been tagged and released.

Comment by auto [ 30/May/13 ]

Author:

{u'username': u'jyemin', u'name': u'Jeff Yemin', u'email': u'jeff.yemin@10gen.com'}

Message: JAVA-609, JAVA-586, JAVA-749: Major changes to ObjectId class. The major change is that ObjectId now conforms to the spec. Instead of representing as 3 4-byte value, it now properly uses a 4-byte timestamp, 3-byte machine identifier, 2-byte process identifier, and 3-byte counter.

The logic for generating machine identifier has changed. Since we are java 6 minimum now, using NetworkInterface.getHardwareAddress() for more uniqueness.

The logic for generating process identifier has changed. It attempts to get the actual numeric process id from JMX, falling back to a string hash only if if it can't. It looks for a pattern in the process name of <process_id>@<host_name>. No longer using the classloader hash either, since each class loader will have a separate instance of NEXT_COUNTER, which is seeded with a random number and so is very unlikely to clash.

Removed a bunch of stuff

Removed support for 'babble' format.
Removed constructors that didn't conform to the spec (machine identifier only, instead of machine identifer and process identifier)
Removed support for isNew/notNew. I don't see a need for this. It was only used in save method, and it seems the only purpose is to decide whether to do an insert of an upsert. I don't see any harm in always doing an upsert. This also has
the nice property of making ObjectId immutable.
Removed a bunch of poorly named methods: _flip(int), _inc(), _machine(), _time()
Removed massageToObjectId(java.lang.Object)' factory method

Other non-backwards compatible changes:

equals method will no longer compare equal to anything but an ObjectId

Additionally, added getters that conform to the spec: timestamp, machineIdentifier, processIdentifier, and counter. Deprecated the old ones.
Branch: 3.0.x
https://github.com/mongodb/mongo-java-driver/commit/aa4b4355c808e045ce00cc1dfbdd0bf78fb6901a

Comment by Jeffrey Yemin [ 27/Aug/12 ]

This is incorrect behavior, but have to be careful since changing it could break existing clients who are relying on it.

Generated at Thu Feb 08 08:52:41 UTC 2024 using Jira 9.7.1#970001-sha1:2222b88b221c4928ef0de3161136cc90c8356a66.