HIBERNATE-66: Building index and unique constraint for Mongo collection

[apmasell]

No description provided.

[Copilot]

Pull request overview

This PR implements Hibernate schema export support for MongoDB indexes/unique constraints by emitting createIndexes/dropIndexes admin commands as MQL JSON and teaching the JDBC layer to execute those commands against MongoDB.

Changes:

• Added a generic MongoIndexExporter to translate Hibernate Index/UniqueKey metadata into MongoDB createIndexes/dropIndexes commands.
• Implemented admin-command execution in MongoStatement.execute(String) via a new AdminCommand decoder/executor.
• Added an integration test to verify index creation and to assert unsupported index options are rejected.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
src/main/java/com/mongodb/hibernate/internal/translate/MongoIndexExporter.java	New exporter that serializes Hibernate index metadata into MongoDB index admin commands (JSON).
src/main/java/com/mongodb/hibernate/internal/jdbc/MongoStatement.java	Enables Statement.execute(String) to run MongoDB admin commands decoded from JSON.
src/main/java/com/mongodb/hibernate/internal/jdbc/AdminCommand.java	New decoder/executor for supported admin commands (createIndexes, dropIndexes).
src/main/java/com/mongodb/hibernate/internal/dialect/MongoDialect.java	Hooks Hibernate schema tooling to use MongoDB-specific exporters for indexes and unique keys; no-op table exporter.
src/integrationTest/java/com/mongodb/hibernate/query/IndexIntegrationTests.java	Integration coverage for index/unique index creation and for rejecting unsupported index options.

[strogiyotec]

I really like the idea with <T extends Exportable> to support both index and unique index

But can we move implementations to the separate classes instead of anonymous classes ?

My thinking process is that we don't have a unit test for getSqlCreateStrings as we do for records where we check the generated string command

Once we have two separate implementations in their own classes we can then add unit tests

   @Test
      void testSingleKeyUnique() {
          var exporter = exporter(true, "books", Optional.of("uniq_isbn"), "", new BsonElement("isbn", new BsonInt32(1)));
          assertThat(exporter.getSqlCreateStrings(UNUSED, null, null))
                  .containsExactly(
                          "{\"createIndexes\": \"books\", \"indexes\": [{\"key\": {\"isbn\": 1}, \"name\": \"uniq_isbn\", \"unique\": true}]}");
      }

      @Test
      void testMultipleKeysNonUnique() {
          var exporter = exporter(
                  false,
                  "books",
                  Optional.of("idx_publisher_author"),
                  "",
                  new BsonElement("publisher", new BsonInt32(1)),
                  new BsonElement("author", new BsonInt32(1)));
          assertThat(exporter.getSqlCreateStrings(UNUSED, null, null))
                  .containsExactly(
                          "{\"createIndexes\": \"books\", \"indexes\": [{\"key\": {\"publisher\": 1, \"author\": 1}, \"name\": \"idx_publisher_author\",
  \"unique\": false}]}");
      }

[apmasell]

Per discussion, these aren't actually sent to the server, they are parsed by AdminCommand, so we don't need that kind of check. I've added a note.

[Copilot]

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.

[jyemin]

I'm still reviewing the PR, but just want to note that CI is failing due to the HIBERNATE-204 commit that requires corresponding changes to the new integration test.

There are three things to do here, and each only surfaces once the previous is fixed:

1. TestCommandListener → @InjectCommandHistory CommandHistory, with commandHistory.getCommands() in place of registry.requireService(...).getStartedCommands().
2. @InjectMongoCollection fields must now be instance, not static — otherwise IllegalStateException: … must not be static — use an instance field to avoid races under parallel execution.
3. inRegistry builds its registry by hand, so it has to apply the per-class contributor itself:

  .applySetting(MONGO_CONFIGURATION_CONTRIBUTOR_KEY,
                MongoExtension.configurationContributorForClass(IndexIntegrationTests.class))

[jyemin]

I was having trouble writing my findings in prose, so instead I decided to put what I understand the requirements to be into the integration test itself and pushed it to a branch on my fork. If we can first agree on the test surface, then the implementation will follow naturally from there:

https://github.com/jyemin/mongo-hibernate/blob/HIBERNATE-66-new-tests/src/integrationTest/java/com/mongodb/hibernate/query/IndexIntegrationTests.java

There are thirteen tests in the updated class, of which six are failing.

Failing:

Test	What is wrong
descendingIndexes	direction ignored on @Index
uniqueDescendingIndexes	direction ignored on @Index(unique = true)
columnLevelUnique	@Column(unique = true) emits no index at all
indexesFollowTheQualifiedCollectionName	indexes created on the unqualified collection name, so they land on a different collection from the data
InvalidMappings.indexOnUnmappedColumn	a columnList entry naming an unmapped column is not rejected
InvalidMappings.uniqueConstraintOnUnmappedColumn	the same through columnNames

Passing, so that what already works is guarded rather than bundled together with what does not: ascendingIndexes, uniqueConstraints, unnamedDeclarationsAreNamedByHibernate, createDropLifecycle, and the three cases in Unsupported.

Three things about the shape of the tests, since they differ from what is on the branch now.

Each test asserts both the emitted createIndexes command and the resulting index set on the server, through listIndexes. "The server accepted it" and "the server built what the mapping asked for" are different claims, and the second is more interesting, but the command is what our code actually produces and some of it is invisible in server state: one createIndexes per index and several batched into one command look identical afterwards. A createIndexes(collection, name, unique, key...) helper builds the expected command, so asserting both costs one line per test.

Comparisons are order-preserving. Commands are compared as JSON and each key document is flattened to an ordered list of field:direction pairs, because BsonDocument implements Map, so comparing documents directly ignores field order, and on a compound index the field order decides which sorts and prefix queries the index can serve. On the current branch you can swap the expectation for idx_on_multi_cols to {author: 1, publisher: 1} and it still passes.

Each group of declarations gets its own entity and collection. Partly so one defect cannot hide the state of everything else, and partly because MongoDB rejects a second index with the same key pattern under a different name, so declarations that overlap on fields cannot share a collection. That is also why @Column(unique = true) needs a field no other uniqueness declaration touches. A @BeforeEach drops those collections, since MongoExtension empties collections between tests but leaves their indexes standing, and the drop half of create-drop does not run when a SessionFactory fails to open, which is what the negative tests provoke.

I also deleted testGarbageDirection. An invalid direction is not a reachable input: IndexBinder.initializeColumns recognizes only a trailing asc or desc and leaves anything else as part of the column name, so that test was really exercising the column-name regex and would fail as soon as the regex goes away. What it was standing in front of is a real gap, which is now InvalidMappings.

Nine of my earlier threads are resolved. Four are still open, with replies on each.

[jyemin]

Correctness guarantees look good now. I found just one issue with default schema that needs a behavioral change.

This review focuses more on design and project conventions

[jyemin]

A few of the comment threads have not been addressed yet, so sending this back.

[jyemin]

LGTM