Add support for `ObjectId` #70

stIncMale · 2025-03-17T14:38:56Z

src/main/java/com/mongodb/hibernate/internal/type/ObjectIdJdbcType.java

src/main/java/com/mongodb/hibernate/jdbc/MongoPreparedStatement.java

src/main/java/com/mongodb/hibernate/internal/type/MqlType.java

stIncMale · 2025-03-27T05:09:40Z

src/test/java/com/mongodb/hibernate/jdbc/MongoPreparedStatementTests.java

+                var expectedDoc = new BsonDocument()
+                        .append("insert", new BsonString("items"))
+                        .append(
+                                "documents",
+                                new BsonArray(List.of(new BsonDocument()
+                                        .append("string1", new BsonString("s1"))
+                                        .append("string2", new BsonString("s2"))
+                                        .append("int32", new BsonInt32(1))
+                                        .append("boolean", BsonBoolean.TRUE)
+                                        .append(
+                                                "stringAndObjectId",
+                                                new BsonArray(List.of(
+                                                        new BsonString("array element"),
+                                                        new BsonObjectId(new ObjectId(1, 2)))))
+                                        .append("objectId", new BsonObjectId(new ObjectId(2, 0))))));


This way it is easier to express what exactly we are expecting than with a string.

NathanQingyangXu · 2025-03-27T19:26:40Z

Looks great overall. Only some minor comments are left.

NathanQingyangXu · 2025-03-27T18:42:56Z

src/main/java/com/mongodb/hibernate/internal/type/MqlType.java

+
+    @Override
+    public String getVendor() {
+        return "MongoDB";


optional. move it to MongoConstants?

Done in 4dbfcaa. The rest of the relevant changes is in #79.

NathanQingyangXu · 2025-03-27T18:49:19Z

src/main/java/com/mongodb/hibernate/internal/type/MqlType.java

+    }
+
+    MqlType(int type) {
+        this.type = type;


from my understanding, code or typeCode is more accurate. SQLType or MqlType is a wrapper of the code or number, but the type per se is not equal to code or number.

We might need to rename the two static methods as well:

maxHibernateSqlType -> maxHibernateSqlTypeCode

minType -> minMqlTypeCode

I agree. I used type because that's how java.sql.JDBCType names it (see JDBCType.type), and java.sql.JDBCType is the closest thing to MqlType. The JDBC naming of this concept is inconsistent:

The non-API part of the JDBC specification calls it "type code".

java.sql.Types documents each constant it defines as "type code", yet the class itself is not called TypeCodes, but Types.

Seemingly, as a result of the previous item, java.sql.JDBCType calls its code field JDBCType.type.

java.sql.SQLType documents the same thing as "vendor specific type number", and exposes the method SQLType.getVendorTypeNumber.

Done in d0fd672.

src/main/java/com/mongodb/hibernate/internal/type/ObjectIdJavaType.java

NathanQingyangXu · 2025-03-27T19:01:39Z

src/integrationTest/java/com/mongodb/hibernate/id/ObjectIdFieldTypeIntegrationTests.java

+        @Id
+        ObjectId id;
+
+        ObjectId v;


It would be great to add testing case covering such edge case: persisting non-id field with null value.

It might be better orgnaized to create two separate testing case suites, one of which focuses on id and the other focuses on non-id field.

NathanQingyangXu · 2025-03-27T19:37:37Z

src/main/java/com/mongodb/hibernate/internal/type/ObjectIdJdbcType.java

+import org.hibernate.type.descriptor.jdbc.BasicExtractor;
+import org.hibernate.type.descriptor.jdbc.JdbcType;
+
+public final class ObjectIdJdbcType implements JdbcType {


optional. It would be great to implement toString() if only for better debugging purpose.

src/main/java/com/mongodb/hibernate/internal/type/ObjectIdJavaType.java

NathanQingyangXu · 2025-03-28T16:48:28Z

Just found an edge case wherein explicit @JavaType annotation is used, the getById() testing case would fail:

@Entity
    @Table(name = "items")
    static class Item {
        @Id
        ObjectId id;

        @JavaType(ObjectIdJavaType.class)
        ObjectId v;
    }

NathanQingyangXu · 2025-03-28T17:10:28Z

src/main/java/com/mongodb/hibernate/internal/type/ObjectIdJavaType.java

+    public static final ObjectIdJavaType INSTANCE = new ObjectIdJavaType();
+
+    private ObjectIdJavaType() {
+        super(ObjectId.class, ImmutableMutabilityPlan.instance());


it doesn't hurt to specify ImmutableMutabilityPlan above, but given it is default plan, super(ObjectId.class) seems to suffice.

Done in d11dbcf.

stIncMale self-assigned this Mar 17, 2025

stIncMale force-pushed the HIBERNATE-23 branch from b7516fb to 290a31e Compare March 17, 2025 14:54

NathanQingyangXu reviewed Mar 18, 2025

View reviewed changes

src/main/java/com/mongodb/hibernate/internal/type/ObjectIdJdbcType.java Show resolved Hide resolved

NathanQingyangXu reviewed Mar 18, 2025

View reviewed changes

src/main/java/com/mongodb/hibernate/internal/type/ObjectIdJdbcType.java Show resolved Hide resolved

NathanQingyangXu reviewed Mar 18, 2025

View reviewed changes

src/main/java/com/mongodb/hibernate/jdbc/MongoPreparedStatement.java Show resolved Hide resolved

NathanQingyangXu reviewed Mar 18, 2025

View reviewed changes

src/main/java/com/mongodb/hibernate/internal/type/MqlType.java Show resolved Hide resolved

Add support for ObjectId

8ac53b2

stIncMale force-pushed the HIBERNATE-23 branch from 290a31e to 8ac53b2 Compare March 26, 2025 23:38

stIncMale added 3 commits March 26, 2025 18:54

Add ObjectIdFieldTypeIntegrationTests.getById

bff3df1

Test MongoPreparedStatement.setObject

fdefbd9

Test MongoResultSet.getObject

b9346b8

stIncMale force-pushed the HIBERNATE-23 branch from 4313e2a to b9346b8 Compare March 27, 2025 04:21

Add internal docs

effeb17

stIncMale commented Mar 27, 2025

View reviewed changes

stIncMale marked this pull request as ready for review March 27, 2025 05:10

stIncMale requested a review from NathanQingyangXu March 27, 2025 05:10

Do minor improvements

33d95e3

NathanQingyangXu requested changes Mar 27, 2025

View reviewed changes

NathanQingyangXu requested changes Mar 28, 2025

View reviewed changes

NathanQingyangXu reviewed Mar 28, 2025

View reviewed changes

stIncMale added 3 commits March 30, 2025 13:53

Move the "MongoDB" literal from MqlType to MongoConstants

4dbfcaa

Rename type -> code

d0fd672

Rely on the default MutabilityPlan

d11dbcf

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for `ObjectId` #70

Add support for `ObjectId` #70

stIncMale commented Mar 17, 2025

stIncMale Mar 27, 2025

NathanQingyangXu commented Mar 27, 2025

NathanQingyangXu Mar 27, 2025

stIncMale Mar 30, 2025

NathanQingyangXu Mar 27, 2025

stIncMale Mar 30, 2025

NathanQingyangXu Mar 27, 2025

NathanQingyangXu Mar 27, 2025

NathanQingyangXu commented Mar 28, 2025

NathanQingyangXu Mar 28, 2025

stIncMale Mar 31, 2025

Add support for ObjectId #70

Are you sure you want to change the base?

Add support for ObjectId #70

Conversation

stIncMale commented Mar 17, 2025

Choose a reason for hiding this comment

NathanQingyangXu commented Mar 27, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

NathanQingyangXu commented Mar 28, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Add support for `ObjectId` #70

Add support for `ObjectId` #70