From 6bb7138e60a88a72a983f2bd0b06cf2e8c6711ab Mon Sep 17 00:00:00 2001 From: Nathan Xu Date: Sun, 2 Mar 2025 08:17:56 -0500 Subject: [PATCH 1/9] basic updating translation --- ...asicCrudTests.java => BasicCRUDTests.java} | 50 ++++++++++++- .../translate/AbstractMqlTranslator.java | 65 +++++++++++----- .../translate/NoopJdbcMutationOperation.java | 74 ------------------- .../translate/TableMutationMqlTranslator.java | 12 --- .../translate/mongoast/AstDocument.java | 2 +- .../translate/mongoast/AstFieldUpdate.java | 27 +++++++ .../mongoast/command/AstUpdateCommand.java | 57 ++++++++++++++ .../command/AstUpdateCommandTests.java | 56 ++++++++++++++ 8 files changed, 233 insertions(+), 110 deletions(-) rename src/integrationTest/java/com/mongodb/hibernate/{BasicCrudTests.java => BasicCRUDTests.java} (75%) delete mode 100644 src/main/java/com/mongodb/hibernate/internal/translate/NoopJdbcMutationOperation.java create mode 100644 src/main/java/com/mongodb/hibernate/internal/translate/mongoast/AstFieldUpdate.java create mode 100644 src/main/java/com/mongodb/hibernate/internal/translate/mongoast/command/AstUpdateCommand.java create mode 100644 src/test/java/com/mongodb/hibernate/internal/translate/mongoast/command/AstUpdateCommandTests.java diff --git a/src/integrationTest/java/com/mongodb/hibernate/BasicCrudTests.java b/src/integrationTest/java/com/mongodb/hibernate/BasicCRUDTests.java similarity index 75% rename from src/integrationTest/java/com/mongodb/hibernate/BasicCrudTests.java rename to src/integrationTest/java/com/mongodb/hibernate/BasicCRUDTests.java index ca39dd2c..ee5262c3 100644 --- a/src/integrationTest/java/com/mongodb/hibernate/BasicCrudTests.java +++ b/src/integrationTest/java/com/mongodb/hibernate/BasicCRUDTests.java @@ -32,6 +32,7 @@ import java.util.ArrayList; import java.util.List; import org.bson.BsonDocument; +import org.hibernate.testing.jdbc.SQLStatementInspector; import org.hibernate.testing.orm.junit.DomainModel; import org.hibernate.testing.orm.junit.SessionFactory; import org.hibernate.testing.orm.junit.SessionFactoryScope; @@ -41,10 +42,12 @@ import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; -@SessionFactory(exportSchema = false) -@DomainModel(annotatedClasses = {BasicCrudTests.Book.class, BasicCrudTests.BookWithEmbeddedField.class}) -class BasicCrudTests implements SessionFactoryScopeAware { +@SessionFactory(exportSchema = false, useCollectingStatementInspector = true) +@DomainModel(annotatedClasses = {BasicCRUDTests.Book.class, BasicCRUDTests.BookWithEmbeddedField.class}) +class BasicCRUDTests implements SessionFactoryScopeAware { @AutoClose private MongoClient mongoClient; @@ -143,7 +146,6 @@ void testEntityWithEmbeddedFieldInsertion() { @Nested class DeleteTests { - @Test void testSimpleDeletion() { @@ -170,6 +172,46 @@ void testSimpleDeletion() { } } + @Nested + class UpdateTests { + + @ParameterizedTest(name = "merge: {0}") + @ValueSource(booleans = {true, false}) + void testSimpleUpdate(boolean merge) { + var statementInspector = sessionFactoryScope.getStatementInspector(SQLStatementInspector.class); + statementInspector.clear(); + sessionFactoryScope.inTransaction(session -> { + var book = new Book(); + book.id = 1; + book.title = "War and Peace"; + book.author = "Leo Tolstoy"; + book.publishYear = 1867; + session.persist(book); + session.flush(); + + book.title = "Insurrection"; + book.publishYear = 1899; + if (merge) { + session.merge(book); + } + }); + + assertCollectionContainsOnly( + BsonDocument.parse( + """ + {"_id": 1, "author": "Leo Tolstoy", "publishYear": 1899, "title": "Insurrection"}\ + """)); + assertThat(statementInspector.getSqlQueries()) + .containsExactly( + """ + {"insert": "books", "documents": [{"author": {"$undefined": true}, "publishYear": {"$undefined": true}, "title": {"$undefined": true}, "_id": {"$undefined": true}}]}\ + """, + """ + {"update": "books", "updates": [{"q": {"_id": {"$eq": {"$undefined": true}}}, "u": {"$set": {"author": {"$undefined": true}, "publishYear": {"$undefined": true}, "title": {"$undefined": true}}}, "multi": true}]}\ + """); + } + } + private List getCollectionDocuments() { var documents = new ArrayList(); collection.find().sort(Sorts.ascending("_id")).into(documents); diff --git a/src/main/java/com/mongodb/hibernate/internal/translate/AbstractMqlTranslator.java b/src/main/java/com/mongodb/hibernate/internal/translate/AbstractMqlTranslator.java index 496a242f..c086b315 100644 --- a/src/main/java/com/mongodb/hibernate/internal/translate/AbstractMqlTranslator.java +++ b/src/main/java/com/mongodb/hibernate/internal/translate/AbstractMqlTranslator.java @@ -26,12 +26,15 @@ import com.mongodb.hibernate.internal.service.StandardServiceRegistryScopedState; import com.mongodb.hibernate.internal.translate.mongoast.AstDocument; import com.mongodb.hibernate.internal.translate.mongoast.AstElement; +import com.mongodb.hibernate.internal.translate.mongoast.AstFieldUpdate; import com.mongodb.hibernate.internal.translate.mongoast.AstNode; import com.mongodb.hibernate.internal.translate.mongoast.AstParameterMarker; import com.mongodb.hibernate.internal.translate.mongoast.command.AstDeleteCommand; import com.mongodb.hibernate.internal.translate.mongoast.command.AstInsertCommand; +import com.mongodb.hibernate.internal.translate.mongoast.command.AstUpdateCommand; import com.mongodb.hibernate.internal.translate.mongoast.filter.AstComparisonFilterOperation; import com.mongodb.hibernate.internal.translate.mongoast.filter.AstFieldOperationFilter; +import com.mongodb.hibernate.internal.translate.mongoast.filter.AstFilter; import com.mongodb.hibernate.internal.translate.mongoast.filter.AstFilterFieldPath; import java.io.IOException; import java.io.StringWriter; @@ -118,6 +121,7 @@ import org.hibernate.sql.exec.spi.JdbcOperation; import org.hibernate.sql.exec.spi.JdbcParameterBinder; import org.hibernate.sql.model.ast.ColumnWriteFragment; +import org.hibernate.sql.model.ast.RestrictedTableMutation; import org.hibernate.sql.model.internal.OptionalTableUpdate; import org.hibernate.sql.model.internal.TableDeleteCustomSql; import org.hibernate.sql.model.internal.TableDeleteStandard; @@ -200,7 +204,7 @@ R acceptAndYield(SqlAstNode node, AstVisitorValueDescriptor< } // ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ - // Table Mutation: insertion + // Table Mutation: insert @Override public void visitStandardTableInsert(TableInsertStandard tableInsert) { @@ -229,30 +233,63 @@ public void visitColumnWriteFragment(ColumnWriteFragment columnWriteFragment) { } // ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ - // Table Mutation: deletion + // Table Mutation: delete @Override public void visitStandardTableDelete(TableDeleteStandard tableDelete) { if (tableDelete.getWhereFragment() != null) { throw new FeatureNotSupportedException(); } + var keyFilter = getKeyFilter(tableDelete); + astVisitorValueHolder.yield( + COLLECTION_MUTATION, + new AstDeleteCommand(tableDelete.getMutatingTable().getTableName(), keyFilter)); + } + + // ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + // Table Mutation: update + + @Override + public void visitStandardTableUpdate(TableUpdateStandard tableUpdate) { + if (tableUpdate.getWhereFragment() != null) { + throw new FeatureNotSupportedException(); + } + doVisitTableUpdate(tableUpdate); + } + + @Override + public void visitOptionalTableUpdate(OptionalTableUpdate tableUpdate) { + doVisitTableUpdate(tableUpdate); + } + + private void doVisitTableUpdate(RestrictedTableMutation tableUpdate) { + var keyFilter = getKeyFilter(tableUpdate); + var updates = new ArrayList(); + tableUpdate.forEachValueBinding((position, valueBinding) -> { + var columnExpression = valueBinding.getColumnReference().getColumnExpression(); + var astValue = acceptAndYield(valueBinding.getValueExpression(), FIELD_VALUE); + updates.add(new AstFieldUpdate(columnExpression, astValue)); + }); + astVisitorValueHolder.yield( + COLLECTION_MUTATION, + new AstUpdateCommand(tableUpdate.getMutatingTable().getTableName(), keyFilter, updates)); + } - if (tableDelete.getNumberOfOptimisticLockBindings() > 0) { + private AstFilter getKeyFilter(RestrictedTableMutation tableMutation) { + if (tableMutation.getNumberOfOptimisticLockBindings() > 0) { throw new FeatureNotSupportedException("TODO-HIBERNATE-51 https://jira.mongodb.org/browse/HIBERNATE-51"); } - if (tableDelete.getNumberOfKeyBindings() > 1) { + if (tableMutation.getNumberOfKeyBindings() > 1) { throw new FeatureNotSupportedException("MongoDB doesn't support '_id' spanning multiple columns"); } - assertTrue(tableDelete.getNumberOfKeyBindings() == 1); - var keyBinding = tableDelete.getKeyBindings().get(0); + assertTrue(tableMutation.getNumberOfKeyBindings() == 1); + var keyBinding = tableMutation.getKeyBindings().get(0); - var tableName = tableDelete.getMutatingTable().getTableName(); var astFilterFieldPath = new AstFilterFieldPath(keyBinding.getColumnReference().getColumnExpression()); var astValue = acceptAndYield(keyBinding.getValueExpression(), FIELD_VALUE); - var keyFilter = new AstFieldOperationFilter(astFilterFieldPath, new AstComparisonFilterOperation(EQ, astValue)); - astVisitorValueHolder.yield(COLLECTION_MUTATION, new AstDeleteCommand(tableName, keyFilter)); + return new AstFieldOperationFilter(astFilterFieldPath, new AstComparisonFilterOperation(EQ, astValue)); } @Override @@ -606,16 +643,6 @@ public void visitCustomTableDelete(TableDeleteCustomSql tableDeleteCustomSql) { throw new FeatureNotSupportedException(); } - @Override - public void visitStandardTableUpdate(TableUpdateStandard tableUpdateStandard) { - throw new FeatureNotSupportedException("TODO-HIBERNATE-19 https://jira.mongodb.org/browse/HIBERNATE-19"); - } - - @Override - public void visitOptionalTableUpdate(OptionalTableUpdate optionalTableUpdate) { - throw new FeatureNotSupportedException(); - } - @Override public void visitCustomTableUpdate(TableUpdateCustomSql tableUpdateCustomSql) { throw new FeatureNotSupportedException(); diff --git a/src/main/java/com/mongodb/hibernate/internal/translate/NoopJdbcMutationOperation.java b/src/main/java/com/mongodb/hibernate/internal/translate/NoopJdbcMutationOperation.java deleted file mode 100644 index 75a7e7a6..00000000 --- a/src/main/java/com/mongodb/hibernate/internal/translate/NoopJdbcMutationOperation.java +++ /dev/null @@ -1,74 +0,0 @@ -/* - * Copyright 2025-present MongoDB, Inc. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package com.mongodb.hibernate.internal.translate; - -import java.util.List; -import org.hibernate.engine.jdbc.mutation.ParameterUsage; -import org.hibernate.jdbc.Expectation; -import org.hibernate.sql.exec.spi.JdbcParameterBinder; -import org.hibernate.sql.model.MutationTarget; -import org.hibernate.sql.model.MutationType; -import org.hibernate.sql.model.TableMapping; -import org.hibernate.sql.model.jdbc.JdbcMutationOperation; -import org.hibernate.sql.model.jdbc.JdbcValueDescriptor; -import org.jspecify.annotations.NullUnmarked; - -@NullUnmarked -final class NoopJdbcMutationOperation implements JdbcMutationOperation { - - NoopJdbcMutationOperation() {} - - @Override - public String getSqlString() { - return ""; - } - - @Override - public List getParameterBinders() { - return List.of(); - } - - @Override - public boolean isCallable() { - return false; - } - - @Override - public Expectation getExpectation() { - return null; - } - - @Override - public MutationType getMutationType() { - return null; - } - - @Override - public MutationTarget getMutationTarget() { - return null; - } - - @Override - public TableMapping getTableDetails() { - return null; - } - - @Override - public JdbcValueDescriptor findValueDescriptor(String columnName, ParameterUsage usage) { - return null; - } -} diff --git a/src/main/java/com/mongodb/hibernate/internal/translate/TableMutationMqlTranslator.java b/src/main/java/com/mongodb/hibernate/internal/translate/TableMutationMqlTranslator.java index 838313e6..7b9ecc27 100644 --- a/src/main/java/com/mongodb/hibernate/internal/translate/TableMutationMqlTranslator.java +++ b/src/main/java/com/mongodb/hibernate/internal/translate/TableMutationMqlTranslator.java @@ -22,8 +22,6 @@ import org.hibernate.engine.spi.SessionFactoryImplementor; import org.hibernate.query.spi.QueryOptions; import org.hibernate.sql.exec.spi.JdbcParameterBindings; -import org.hibernate.sql.model.ast.TableDelete; -import org.hibernate.sql.model.ast.TableInsert; import org.hibernate.sql.model.ast.TableMutation; import org.hibernate.sql.model.jdbc.JdbcMutationOperation; import org.jspecify.annotations.Nullable; @@ -38,20 +36,10 @@ final class TableMutationMqlTranslator extends } @Override - @SuppressWarnings("unchecked") public O translate(@Nullable JdbcParameterBindings jdbcParameterBindings, QueryOptions queryOptions) { assertNull(jdbcParameterBindings); // QueryOptions class is not applicable to table mutation so a dummy value is always passed in - if (tableMutation instanceof TableInsert || tableMutation instanceof TableDelete) { - return translateTableMutation(); - } else { - // TODO-HIBERNATE-19 https://jira.mongodb.org/browse/HIBERNATE-19 - return (O) new NoopJdbcMutationOperation(); - } - } - - private O translateTableMutation() { var rootAstNode = acceptAndYield(tableMutation, COLLECTION_MUTATION); return tableMutation.createMutationOperation(renderMongoAstNode(rootAstNode), getParameterBinders()); } diff --git a/src/main/java/com/mongodb/hibernate/internal/translate/mongoast/AstDocument.java b/src/main/java/com/mongodb/hibernate/internal/translate/mongoast/AstDocument.java index 82e20331..cc285426 100644 --- a/src/main/java/com/mongodb/hibernate/internal/translate/mongoast/AstDocument.java +++ b/src/main/java/com/mongodb/hibernate/internal/translate/mongoast/AstDocument.java @@ -19,7 +19,7 @@ import java.util.List; import org.bson.BsonWriter; -public record AstDocument(List elements) implements AstValue { +public record AstDocument(List elements) implements AstValue { @Override public void render(BsonWriter writer) { writer.writeStartDocument(); diff --git a/src/main/java/com/mongodb/hibernate/internal/translate/mongoast/AstFieldUpdate.java b/src/main/java/com/mongodb/hibernate/internal/translate/mongoast/AstFieldUpdate.java new file mode 100644 index 00000000..39ca2e1f --- /dev/null +++ b/src/main/java/com/mongodb/hibernate/internal/translate/mongoast/AstFieldUpdate.java @@ -0,0 +1,27 @@ +/* + * Copyright 2025-present MongoDB, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.mongodb.hibernate.internal.translate.mongoast; + +import org.bson.BsonWriter; + +public record AstFieldUpdate(String name, AstValue value) implements AstNode { + @Override + public void render(BsonWriter writer) { + writer.writeName(name); + value.render(writer); + } +} diff --git a/src/main/java/com/mongodb/hibernate/internal/translate/mongoast/command/AstUpdateCommand.java b/src/main/java/com/mongodb/hibernate/internal/translate/mongoast/command/AstUpdateCommand.java new file mode 100644 index 00000000..51967163 --- /dev/null +++ b/src/main/java/com/mongodb/hibernate/internal/translate/mongoast/command/AstUpdateCommand.java @@ -0,0 +1,57 @@ +/* + * Copyright 2025-present MongoDB, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.mongodb.hibernate.internal.translate.mongoast.command; + +import com.mongodb.hibernate.internal.translate.mongoast.AstFieldUpdate; +import com.mongodb.hibernate.internal.translate.mongoast.AstNode; +import com.mongodb.hibernate.internal.translate.mongoast.filter.AstFilter; +import java.util.List; +import org.bson.BsonWriter; + +public record AstUpdateCommand(String collection, AstFilter filter, List updates) implements AstNode { + @Override + public void render(BsonWriter writer) { + writer.writeStartDocument(); + { + writer.writeString("update", collection); + writer.writeName("updates"); + writer.writeStartArray(); + { + writer.writeStartDocument(); + { + writer.writeName("q"); + filter.render(writer); + writer.writeName("u"); + writer.writeStartDocument(); + { + writer.writeName("$set"); + writer.writeStartDocument(); + { + updates.forEach(update -> update.render(writer)); + } + writer.writeEndDocument(); + } + writer.writeEndDocument(); + writer.writeBoolean("multi", true); + } + writer.writeEndDocument(); + } + writer.writeEndArray(); + } + writer.writeEndDocument(); + } +} diff --git a/src/test/java/com/mongodb/hibernate/internal/translate/mongoast/command/AstUpdateCommandTests.java b/src/test/java/com/mongodb/hibernate/internal/translate/mongoast/command/AstUpdateCommandTests.java new file mode 100644 index 00000000..fd083785 --- /dev/null +++ b/src/test/java/com/mongodb/hibernate/internal/translate/mongoast/command/AstUpdateCommandTests.java @@ -0,0 +1,56 @@ +/* + * Copyright 2025-present MongoDB, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.mongodb.hibernate.internal.translate.mongoast.command; + +import static com.mongodb.hibernate.internal.translate.mongoast.AstNodeAssertions.assertRender; + +import com.mongodb.hibernate.internal.translate.mongoast.AstFieldUpdate; +import com.mongodb.hibernate.internal.translate.mongoast.AstLiteralValue; +import com.mongodb.hibernate.internal.translate.mongoast.filter.AstComparisonFilterOperation; +import com.mongodb.hibernate.internal.translate.mongoast.filter.AstComparisonFilterOperator; +import com.mongodb.hibernate.internal.translate.mongoast.filter.AstFieldOperationFilter; +import com.mongodb.hibernate.internal.translate.mongoast.filter.AstFilter; +import com.mongodb.hibernate.internal.translate.mongoast.filter.AstFilterFieldPath; +import java.util.List; +import org.bson.BsonInt64; +import org.bson.BsonString; +import org.junit.jupiter.api.Test; + +class AstUpdateCommandTests { + + @Test + void testRendering() { + + var collection = "books"; + var astFieldUpdate1 = new AstFieldUpdate("title", new AstLiteralValue(new BsonString("War and Peace"))); + var astFieldUpdate2 = new AstFieldUpdate("author", new AstLiteralValue(new BsonString("Leo Tolstoy"))); + + final AstFilter filter; + filter = new AstFieldOperationFilter( + new AstFilterFieldPath("_id"), + new AstComparisonFilterOperation( + AstComparisonFilterOperator.EQ, new AstLiteralValue(new BsonInt64(12345L)))); + + var updateCommand = new AstUpdateCommand(collection, filter, List.of(astFieldUpdate1, astFieldUpdate2)); + + final String expectedJson = + """ + {"update": "books", "updates": [{"q": {"_id": {"$eq": 12345}}, "u": {"$set": {"title": "War and Peace", "author": "Leo Tolstoy"}}, "multi": true}]}\ + """; + assertRender(expectedJson, updateCommand); + } +} From 9a1fe83033ed31762d766f3b9ea84c3bd897ed32 Mon Sep 17 00:00:00 2001 From: Nathan Xu Date: Mon, 3 Mar 2025 10:01:23 -0500 Subject: [PATCH 2/9] exclude 'merge' out of this PR scope --- .../com/mongodb/hibernate/BasicCRUDTests.java | 11 +++------- .../translate/AbstractMqlTranslator.java | 20 ++++++++----------- 2 files changed, 11 insertions(+), 20 deletions(-) diff --git a/src/integrationTest/java/com/mongodb/hibernate/BasicCRUDTests.java b/src/integrationTest/java/com/mongodb/hibernate/BasicCRUDTests.java index ee5262c3..6decb232 100644 --- a/src/integrationTest/java/com/mongodb/hibernate/BasicCRUDTests.java +++ b/src/integrationTest/java/com/mongodb/hibernate/BasicCRUDTests.java @@ -42,8 +42,6 @@ import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; -import org.junit.jupiter.params.ParameterizedTest; -import org.junit.jupiter.params.provider.ValueSource; @SessionFactory(exportSchema = false, useCollectingStatementInspector = true) @DomainModel(annotatedClasses = {BasicCRUDTests.Book.class, BasicCRUDTests.BookWithEmbeddedField.class}) @@ -175,9 +173,8 @@ void testSimpleDeletion() { @Nested class UpdateTests { - @ParameterizedTest(name = "merge: {0}") - @ValueSource(booleans = {true, false}) - void testSimpleUpdate(boolean merge) { + @Test + void testSimpleUpdate() { var statementInspector = sessionFactoryScope.getStatementInspector(SQLStatementInspector.class); statementInspector.clear(); sessionFactoryScope.inTransaction(session -> { @@ -191,9 +188,7 @@ void testSimpleUpdate(boolean merge) { book.title = "Insurrection"; book.publishYear = 1899; - if (merge) { - session.merge(book); - } + session.merge(book); }); assertCollectionContainsOnly( diff --git a/src/main/java/com/mongodb/hibernate/internal/translate/AbstractMqlTranslator.java b/src/main/java/com/mongodb/hibernate/internal/translate/AbstractMqlTranslator.java index c086b315..399250f6 100644 --- a/src/main/java/com/mongodb/hibernate/internal/translate/AbstractMqlTranslator.java +++ b/src/main/java/com/mongodb/hibernate/internal/translate/AbstractMqlTranslator.java @@ -254,22 +254,13 @@ public void visitStandardTableUpdate(TableUpdateStandard tableUpdate) { if (tableUpdate.getWhereFragment() != null) { throw new FeatureNotSupportedException(); } - doVisitTableUpdate(tableUpdate); - } - - @Override - public void visitOptionalTableUpdate(OptionalTableUpdate tableUpdate) { - doVisitTableUpdate(tableUpdate); - } - - private void doVisitTableUpdate(RestrictedTableMutation tableUpdate) { var keyFilter = getKeyFilter(tableUpdate); - var updates = new ArrayList(); - tableUpdate.forEachValueBinding((position, valueBinding) -> { + var updates = new ArrayList(tableUpdate.getNumberOfValueBindings()); + for (var valueBinding : tableUpdate.getValueBindings()) { var columnExpression = valueBinding.getColumnReference().getColumnExpression(); var astValue = acceptAndYield(valueBinding.getValueExpression(), FIELD_VALUE); updates.add(new AstFieldUpdate(columnExpression, astValue)); - }); + } astVisitorValueHolder.yield( COLLECTION_MUTATION, new AstUpdateCommand(tableUpdate.getMutatingTable().getTableName(), keyFilter, updates)); @@ -643,6 +634,11 @@ public void visitCustomTableDelete(TableDeleteCustomSql tableDeleteCustomSql) { throw new FeatureNotSupportedException(); } + @Override + public void visitOptionalTableUpdate(OptionalTableUpdate optionalTableUpdate) { + throw new FeatureNotSupportedException(); + } + @Override public void visitCustomTableUpdate(TableUpdateCustomSql tableUpdateCustomSql) { throw new FeatureNotSupportedException(); From bbb6fee372eb7a4f398c2edb663cf0971704a564 Mon Sep 17 00:00:00 2001 From: Nathan Xu Date: Mon, 3 Mar 2025 10:02:54 -0500 Subject: [PATCH 3/9] revert back naming changing for BasicCrudTests --- .../hibernate/{BasicCRUDTests.java => BasicCrudTests.java} | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) rename src/integrationTest/java/com/mongodb/hibernate/{BasicCRUDTests.java => BasicCrudTests.java} (98%) diff --git a/src/integrationTest/java/com/mongodb/hibernate/BasicCRUDTests.java b/src/integrationTest/java/com/mongodb/hibernate/BasicCrudTests.java similarity index 98% rename from src/integrationTest/java/com/mongodb/hibernate/BasicCRUDTests.java rename to src/integrationTest/java/com/mongodb/hibernate/BasicCrudTests.java index 6decb232..521fa950 100644 --- a/src/integrationTest/java/com/mongodb/hibernate/BasicCRUDTests.java +++ b/src/integrationTest/java/com/mongodb/hibernate/BasicCrudTests.java @@ -44,8 +44,8 @@ import org.junit.jupiter.api.Test; @SessionFactory(exportSchema = false, useCollectingStatementInspector = true) -@DomainModel(annotatedClasses = {BasicCRUDTests.Book.class, BasicCRUDTests.BookWithEmbeddedField.class}) -class BasicCRUDTests implements SessionFactoryScopeAware { +@DomainModel(annotatedClasses = {BasicCrudTests.Book.class, BasicCrudTests.BookWithEmbeddedField.class}) +class BasicCrudTests implements SessionFactoryScopeAware { @AutoClose private MongoClient mongoClient; From e988b33351b82cccddaa3a87d6e71c3eb58a2eb1 Mon Sep 17 00:00:00 2001 From: Nathan Xu Date: Wed, 5 Mar 2025 14:41:20 -0500 Subject: [PATCH 4/9] Update src/integrationTest/java/com/mongodb/hibernate/BasicCrudIntegrationTests.java Co-authored-by: Maxim Katcharov --- .../java/com/mongodb/hibernate/BasicCrudIntegrationTests.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/integrationTest/java/com/mongodb/hibernate/BasicCrudIntegrationTests.java b/src/integrationTest/java/com/mongodb/hibernate/BasicCrudIntegrationTests.java index 353ed430..ee596e76 100644 --- a/src/integrationTest/java/com/mongodb/hibernate/BasicCrudIntegrationTests.java +++ b/src/integrationTest/java/com/mongodb/hibernate/BasicCrudIntegrationTests.java @@ -186,7 +186,7 @@ void testSimpleUpdate() { session.persist(book); session.flush(); - book.title = "Insurrection"; + book.title = "Resurrection"; book.publishYear = 1899; session.merge(book); }); @@ -194,7 +194,7 @@ void testSimpleUpdate() { assertCollectionContainsOnly( BsonDocument.parse( """ - {"_id": 1, "author": "Leo Tolstoy", "publishYear": 1899, "title": "Insurrection"}\ + {"_id": 1, "author": "Leo Tolstoy", "publishYear": 1899, "title": "Resurrection"}\ """)); assertThat(statementInspector.getSqlQueries()) .containsExactly( From c55bde93c80f8d84f5ca0d5b2c70572e799a7f80 Mon Sep 17 00:00:00 2001 From: Nathan Xu Date: Fri, 7 Mar 2025 11:28:59 -0500 Subject: [PATCH 5/9] Update src/integrationTest/java/com/mongodb/hibernate/BasicCrudIntegrationTests.java Co-authored-by: Valentin Kovalenko --- .../java/com/mongodb/hibernate/BasicCrudIntegrationTests.java | 1 - 1 file changed, 1 deletion(-) diff --git a/src/integrationTest/java/com/mongodb/hibernate/BasicCrudIntegrationTests.java b/src/integrationTest/java/com/mongodb/hibernate/BasicCrudIntegrationTests.java index ee596e76..41798279 100644 --- a/src/integrationTest/java/com/mongodb/hibernate/BasicCrudIntegrationTests.java +++ b/src/integrationTest/java/com/mongodb/hibernate/BasicCrudIntegrationTests.java @@ -188,7 +188,6 @@ void testSimpleUpdate() { book.title = "Resurrection"; book.publishYear = 1899; - session.merge(book); }); assertCollectionContainsOnly( From c6791de58988970b3db5f28d576ebbd66a8cfe56 Mon Sep 17 00:00:00 2001 From: Nathan Xu Date: Fri, 7 Mar 2025 11:09:53 -0500 Subject: [PATCH 6/9] add update testing case to test `@DynamicUpdate` --- .../hibernate/BasicCrudIntegrationTests.java | 57 ++++++++++++++++++- 1 file changed, 54 insertions(+), 3 deletions(-) diff --git a/src/integrationTest/java/com/mongodb/hibernate/BasicCrudIntegrationTests.java b/src/integrationTest/java/com/mongodb/hibernate/BasicCrudIntegrationTests.java index 41798279..b195e473 100644 --- a/src/integrationTest/java/com/mongodb/hibernate/BasicCrudIntegrationTests.java +++ b/src/integrationTest/java/com/mongodb/hibernate/BasicCrudIntegrationTests.java @@ -32,6 +32,7 @@ import java.util.ArrayList; import java.util.List; import org.bson.BsonDocument; +import org.hibernate.annotations.DynamicUpdate; import org.hibernate.testing.jdbc.SQLStatementInspector; import org.hibernate.testing.orm.junit.DomainModel; import org.hibernate.testing.orm.junit.SessionFactory; @@ -45,7 +46,10 @@ @SessionFactory(exportSchema = false, useCollectingStatementInspector = true) @DomainModel( - annotatedClasses = {BasicCrudIntegrationTests.Book.class, BasicCrudIntegrationTests.BookWithEmbeddedField.class + annotatedClasses = { + BasicCrudIntegrationTests.Book.class, + BasicCrudIntegrationTests.BookWithEmbeddedField.class, + BasicCrudIntegrationTests.BookDynamicUpdated.class }) class BasicCrudIntegrationTests implements SessionFactoryScopeAware { @@ -204,6 +208,38 @@ void testSimpleUpdate() { {"update": "books", "updates": [{"q": {"_id": {"$eq": {"$undefined": true}}}, "u": {"$set": {"author": {"$undefined": true}, "publishYear": {"$undefined": true}, "title": {"$undefined": true}}}, "multi": true}]}\ """); } + + @Test + void testDynamicUpdate() { + var statementInspector = sessionFactoryScope.getStatementInspector(SQLStatementInspector.class); + statementInspector.clear(); + sessionFactoryScope.inTransaction(session -> { + var book = new BookDynamicUpdated(); + book.id = 1; + book.title = "War and Peace"; + book.author = "Leo Tolstoy"; + book.publishYear = 1899; + session.persist(book); + session.flush(); + + book.publishYear = 1867; + session.merge(book); + }); + + assertCollectionContainsOnly( + BsonDocument.parse( + """ + {"_id": 1, "author": "Leo Tolstoy", "publishYear": 1867, "title": "War and Peace"}\ + """)); + assertThat(statementInspector.getSqlQueries()) + .containsExactly( + """ + {"insert": "books", "documents": [{"author": {"$undefined": true}, "publishYear": {"$undefined": true}, "title": {"$undefined": true}, "_id": {"$undefined": true}}]}\ + """, + """ + {"update": "books", "updates": [{"q": {"_id": {"$eq": {"$undefined": true}}}, "u": {"$set": {"publishYear": {"$undefined": true}}}, "multi": true}]}\ + """); + } } private List getCollectionDocuments() { @@ -216,7 +252,7 @@ private void assertCollectionContainsOnly(BsonDocument expectedDoc) { assertThat(getCollectionDocuments()).asInstanceOf(LIST).singleElement().isEqualTo(expectedDoc); } - @Entity(name = "Book") + @Entity @Table(name = "books") static class Book { @Id @@ -230,7 +266,22 @@ static class Book { int publishYear; } - @Entity(name = "BookWithEmbeddedField") + @Entity + @Table(name = "books") + @DynamicUpdate + static class BookDynamicUpdated { + @Id + @Column(name = "_id") + int id; + + String title; + + String author; + + int publishYear; + } + + @Entity @Table(name = "books") static class BookWithEmbeddedField { @Id From 90d78760c805c79a372b8f3e334cfdef9d3592ac Mon Sep 17 00:00:00 2001 From: Nathan Xu Date: Fri, 7 Mar 2025 11:36:12 -0500 Subject: [PATCH 7/9] addresses code review comments --- .../hibernate/BasicCrudIntegrationTests.java | 24 +------------------ .../translate/AbstractMqlTranslator.java | 8 +++++++ 2 files changed, 9 insertions(+), 23 deletions(-) diff --git a/src/integrationTest/java/com/mongodb/hibernate/BasicCrudIntegrationTests.java b/src/integrationTest/java/com/mongodb/hibernate/BasicCrudIntegrationTests.java index b195e473..bd6fca82 100644 --- a/src/integrationTest/java/com/mongodb/hibernate/BasicCrudIntegrationTests.java +++ b/src/integrationTest/java/com/mongodb/hibernate/BasicCrudIntegrationTests.java @@ -33,7 +33,6 @@ import java.util.List; import org.bson.BsonDocument; import org.hibernate.annotations.DynamicUpdate; -import org.hibernate.testing.jdbc.SQLStatementInspector; import org.hibernate.testing.orm.junit.DomainModel; import org.hibernate.testing.orm.junit.SessionFactory; import org.hibernate.testing.orm.junit.SessionFactoryScope; @@ -44,7 +43,7 @@ import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; -@SessionFactory(exportSchema = false, useCollectingStatementInspector = true) +@SessionFactory(exportSchema = false) @DomainModel( annotatedClasses = { BasicCrudIntegrationTests.Book.class, @@ -179,8 +178,6 @@ class UpdateTests { @Test void testSimpleUpdate() { - var statementInspector = sessionFactoryScope.getStatementInspector(SQLStatementInspector.class); - statementInspector.clear(); sessionFactoryScope.inTransaction(session -> { var book = new Book(); book.id = 1; @@ -199,20 +196,10 @@ void testSimpleUpdate() { """ {"_id": 1, "author": "Leo Tolstoy", "publishYear": 1899, "title": "Resurrection"}\ """)); - assertThat(statementInspector.getSqlQueries()) - .containsExactly( - """ - {"insert": "books", "documents": [{"author": {"$undefined": true}, "publishYear": {"$undefined": true}, "title": {"$undefined": true}, "_id": {"$undefined": true}}]}\ - """, - """ - {"update": "books", "updates": [{"q": {"_id": {"$eq": {"$undefined": true}}}, "u": {"$set": {"author": {"$undefined": true}, "publishYear": {"$undefined": true}, "title": {"$undefined": true}}}, "multi": true}]}\ - """); } @Test void testDynamicUpdate() { - var statementInspector = sessionFactoryScope.getStatementInspector(SQLStatementInspector.class); - statementInspector.clear(); sessionFactoryScope.inTransaction(session -> { var book = new BookDynamicUpdated(); book.id = 1; @@ -223,7 +210,6 @@ void testDynamicUpdate() { session.flush(); book.publishYear = 1867; - session.merge(book); }); assertCollectionContainsOnly( @@ -231,14 +217,6 @@ void testDynamicUpdate() { """ {"_id": 1, "author": "Leo Tolstoy", "publishYear": 1867, "title": "War and Peace"}\ """)); - assertThat(statementInspector.getSqlQueries()) - .containsExactly( - """ - {"insert": "books", "documents": [{"author": {"$undefined": true}, "publishYear": {"$undefined": true}, "title": {"$undefined": true}, "_id": {"$undefined": true}}]}\ - """, - """ - {"update": "books", "updates": [{"q": {"_id": {"$eq": {"$undefined": true}}}, "u": {"$set": {"publishYear": {"$undefined": true}}}, "multi": true}]}\ - """); } } diff --git a/src/main/java/com/mongodb/hibernate/internal/translate/AbstractMqlTranslator.java b/src/main/java/com/mongodb/hibernate/internal/translate/AbstractMqlTranslator.java index 399250f6..58f22ea3 100644 --- a/src/main/java/com/mongodb/hibernate/internal/translate/AbstractMqlTranslator.java +++ b/src/main/java/com/mongodb/hibernate/internal/translate/AbstractMqlTranslator.java @@ -208,6 +208,10 @@ R acceptAndYield(SqlAstNode node, AstVisitorValueDescriptor< @Override public void visitStandardTableInsert(TableInsertStandard tableInsert) { + if (tableInsert.getReturningColumns() != null + && !tableInsert.getReturningColumns().isEmpty()) { + throw new FeatureNotSupportedException(); + } var tableName = tableInsert.getTableName(); var astElements = new ArrayList(tableInsert.getNumberOfValueBindings()); for (var columnValueBinding : tableInsert.getValueBindings()) { @@ -251,6 +255,10 @@ public void visitStandardTableDelete(TableDeleteStandard tableDelete) { @Override public void visitStandardTableUpdate(TableUpdateStandard tableUpdate) { + if (tableUpdate.getReturningColumns() != null + && !tableUpdate.getReturningColumns().isEmpty()) { + throw new FeatureNotSupportedException(); + } if (tableUpdate.getWhereFragment() != null) { throw new FeatureNotSupportedException(); } From 3c4e08ff07ce96f9a5a5f557cc62552f099ff933 Mon Sep 17 00:00:00 2001 From: Nathan Xu Date: Mon, 10 Mar 2025 15:38:04 -0400 Subject: [PATCH 8/9] changes as per code review comments --- .../com/mongodb/hibernate/BasicCrudIntegrationTests.java | 6 +++--- .../internal/translate/AbstractMqlTranslator.java | 8 ++++---- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/integrationTest/java/com/mongodb/hibernate/BasicCrudIntegrationTests.java b/src/integrationTest/java/com/mongodb/hibernate/BasicCrudIntegrationTests.java index ec8f5d0e..d6a1e886 100644 --- a/src/integrationTest/java/com/mongodb/hibernate/BasicCrudIntegrationTests.java +++ b/src/integrationTest/java/com/mongodb/hibernate/BasicCrudIntegrationTests.java @@ -45,7 +45,7 @@ annotatedClasses = { BasicCrudIntegrationTests.Book.class, BasicCrudIntegrationTests.BookWithEmbeddedField.class, - BasicCrudIntegrationTests.BookDynamicUpdated.class + BasicCrudIntegrationTests.BookDynamicallyUpdated.class }) @ExtendWith(MongoExtension.class) class BasicCrudIntegrationTests implements SessionFactoryScopeAware { @@ -183,7 +183,7 @@ void testSimpleUpdate() { @Test void testDynamicUpdate() { sessionFactoryScope.inTransaction(session -> { - var book = new BookDynamicUpdated(); + var book = new BookDynamicallyUpdated(); book.id = 1; book.title = "War and Peace"; book.author = "Leo Tolstoy"; @@ -229,7 +229,7 @@ static class Book { @Entity @Table(name = "books") @DynamicUpdate - static class BookDynamicUpdated { + static class BookDynamicallyUpdated { @Id @Column(name = "_id") int id; diff --git a/src/main/java/com/mongodb/hibernate/internal/translate/AbstractMqlTranslator.java b/src/main/java/com/mongodb/hibernate/internal/translate/AbstractMqlTranslator.java index 58f22ea3..e10d82c0 100644 --- a/src/main/java/com/mongodb/hibernate/internal/translate/AbstractMqlTranslator.java +++ b/src/main/java/com/mongodb/hibernate/internal/translate/AbstractMqlTranslator.java @@ -120,8 +120,9 @@ import org.hibernate.sql.ast.tree.update.UpdateStatement; import org.hibernate.sql.exec.spi.JdbcOperation; import org.hibernate.sql.exec.spi.JdbcParameterBinder; +import org.hibernate.sql.model.MutationOperation; +import org.hibernate.sql.model.ast.AbstractRestrictedTableMutation; import org.hibernate.sql.model.ast.ColumnWriteFragment; -import org.hibernate.sql.model.ast.RestrictedTableMutation; import org.hibernate.sql.model.internal.OptionalTableUpdate; import org.hibernate.sql.model.internal.TableDeleteCustomSql; import org.hibernate.sql.model.internal.TableDeleteStandard; @@ -255,8 +256,7 @@ public void visitStandardTableDelete(TableDeleteStandard tableDelete) { @Override public void visitStandardTableUpdate(TableUpdateStandard tableUpdate) { - if (tableUpdate.getReturningColumns() != null - && !tableUpdate.getReturningColumns().isEmpty()) { + if (tableUpdate.getNumberOfReturningColumns() > 0) { throw new FeatureNotSupportedException(); } if (tableUpdate.getWhereFragment() != null) { @@ -274,7 +274,7 @@ public void visitStandardTableUpdate(TableUpdateStandard tableUpdate) { new AstUpdateCommand(tableUpdate.getMutatingTable().getTableName(), keyFilter, updates)); } - private AstFilter getKeyFilter(RestrictedTableMutation tableMutation) { + private AstFilter getKeyFilter(AbstractRestrictedTableMutation tableMutation) { if (tableMutation.getNumberOfOptimisticLockBindings() > 0) { throw new FeatureNotSupportedException("TODO-HIBERNATE-51 https://jira.mongodb.org/browse/HIBERNATE-51"); } From 8803d302e73c484b59e158f88b85866b2b4c9e77 Mon Sep 17 00:00:00 2001 From: Nathan Xu Date: Tue, 11 Mar 2025 09:23:44 -0400 Subject: [PATCH 9/9] further code changes as per comments --- .../java/com/mongodb/hibernate/BasicCrudIntegrationTests.java | 4 ++-- .../hibernate/internal/translate/AbstractMqlTranslator.java | 3 +-- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/src/integrationTest/java/com/mongodb/hibernate/BasicCrudIntegrationTests.java b/src/integrationTest/java/com/mongodb/hibernate/BasicCrudIntegrationTests.java index d6a1e886..379fc596 100644 --- a/src/integrationTest/java/com/mongodb/hibernate/BasicCrudIntegrationTests.java +++ b/src/integrationTest/java/com/mongodb/hibernate/BasicCrudIntegrationTests.java @@ -202,13 +202,13 @@ void testDynamicUpdate() { } } - private List getCollectionDocuments() { + private static List getCollectionDocuments() { var documents = new ArrayList(); collection.find().sort(Sorts.ascending("_id")).into(documents); return documents; } - private void assertCollectionContainsOnly(BsonDocument expectedDoc) { + private static void assertCollectionContainsOnly(BsonDocument expectedDoc) { assertThat(getCollectionDocuments()).asInstanceOf(LIST).singleElement().isEqualTo(expectedDoc); } diff --git a/src/main/java/com/mongodb/hibernate/internal/translate/AbstractMqlTranslator.java b/src/main/java/com/mongodb/hibernate/internal/translate/AbstractMqlTranslator.java index e10d82c0..5aef9f25 100644 --- a/src/main/java/com/mongodb/hibernate/internal/translate/AbstractMqlTranslator.java +++ b/src/main/java/com/mongodb/hibernate/internal/translate/AbstractMqlTranslator.java @@ -209,8 +209,7 @@ R acceptAndYield(SqlAstNode node, AstVisitorValueDescriptor< @Override public void visitStandardTableInsert(TableInsertStandard tableInsert) { - if (tableInsert.getReturningColumns() != null - && !tableInsert.getReturningColumns().isEmpty()) { + if (tableInsert.getNumberOfReturningColumns() > 0) { throw new FeatureNotSupportedException(); } var tableName = tableInsert.getTableName();