From 387d8087e95cbb994218f849d15d555a7eadc1af Mon Sep 17 00:00:00 2001 From: Derick Hermanson Date: Tue, 29 Aug 2023 10:38:21 -0500 Subject: [PATCH 1/3] [#1737] Property parse query string during native select query --- .../ReactiveNativeSelectQueryPlanImpl.java | 2 +- ...ativeQueryPlaceholderSubstitutionTest.java | 65 +++++++++++++++++++ 2 files changed, 66 insertions(+), 1 deletion(-) create mode 100644 hibernate-reactive-core/src/test/java/org/hibernate/reactive/NativeQueryPlaceholderSubstitutionTest.java diff --git a/hibernate-reactive-core/src/main/java/org/hibernate/reactive/query/sql/internal/ReactiveNativeSelectQueryPlanImpl.java b/hibernate-reactive-core/src/main/java/org/hibernate/reactive/query/sql/internal/ReactiveNativeSelectQueryPlanImpl.java index be7910d0e..28753683e 100644 --- a/hibernate-reactive-core/src/main/java/org/hibernate/reactive/query/sql/internal/ReactiveNativeSelectQueryPlanImpl.java +++ b/hibernate-reactive-core/src/main/java/org/hibernate/reactive/query/sql/internal/ReactiveNativeSelectQueryPlanImpl.java @@ -60,7 +60,7 @@ public ReactiveNativeSelectQueryPlanImpl( resultSetMapping.addAffectedTableNames( affectedTableNames, sessionFactory ); } this.affectedTableNames = affectedTableNames; - this.sql = sql; + this.sql = parser.process(); this.parameterList = parameterList; } diff --git a/hibernate-reactive-core/src/test/java/org/hibernate/reactive/NativeQueryPlaceholderSubstitutionTest.java b/hibernate-reactive-core/src/test/java/org/hibernate/reactive/NativeQueryPlaceholderSubstitutionTest.java new file mode 100644 index 000000000..43995d3e1 --- /dev/null +++ b/hibernate-reactive-core/src/test/java/org/hibernate/reactive/NativeQueryPlaceholderSubstitutionTest.java @@ -0,0 +1,65 @@ +/* Hibernate, Relational Persistence for Idiomatic Java + * + * SPDX-License-Identifier: Apache-2.0 + * Copyright: Red Hat Inc. and Hibernate Authors + */ +package org.hibernate.reactive; + +import static java.util.concurrent.TimeUnit.MINUTES; + +import io.vertx.junit5.Timeout; +import io.vertx.junit5.VertxTestContext; +import jakarta.persistence.Entity; +import jakarta.persistence.GeneratedValue; +import jakarta.persistence.Id; +import jakarta.persistence.Table; +import java.util.Collection; +import java.util.List; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Test; + +@Timeout(value = 10, timeUnit = MINUTES) +public class NativeQueryPlaceholderSubstitutionTest extends BaseReactiveTest { + + @Override + protected Collection> annotatedEntities() { + return List.of(Widget.class); + } + + @Test + public void testThatSchemaGetsSubstitutedDuringNativeSelectQuery(VertxTestContext context) { + + test(context, getSessionFactory().withSession(session -> + session.createNativeQuery("select count(*) from {h-schema}widgets", Integer.class) + .getSingleResult() + .thenAccept(result -> Assertions.assertEquals(0, result)) + )); + } + + @Test + public void testThatSchemaGetsSubstitutedDuringNativeNonSelectQuery(VertxTestContext context) { + + test(context, getSessionFactory().withSession(session -> + session.createNativeQuery("update {h-schema}widgets set id = 1") + .executeUpdate() + .thenAccept(result -> Assertions.assertEquals(0, result)) + )); + } + + @Entity(name = "Widget") + @Table(name = "widgets") + public static class Widget { + + @Id + @GeneratedValue + private Long id; + + @Override + public String toString() { + return "Widget{" + + "id=" + id + + '}'; + } + } + +} From a1410ce91739632dde9cf514c335e11a4e6cc390 Mon Sep 17 00:00:00 2001 From: Davide D'Alto Date: Thu, 31 Aug 2023 16:16:33 +0200 Subject: [PATCH 2/3] [#1737] Update test --- ...ativeQueryPlaceholderSubstitutionTest.java | 158 +++++++++++++----- 1 file changed, 113 insertions(+), 45 deletions(-) diff --git a/hibernate-reactive-core/src/test/java/org/hibernate/reactive/NativeQueryPlaceholderSubstitutionTest.java b/hibernate-reactive-core/src/test/java/org/hibernate/reactive/NativeQueryPlaceholderSubstitutionTest.java index 43995d3e1..1fa546a22 100644 --- a/hibernate-reactive-core/src/test/java/org/hibernate/reactive/NativeQueryPlaceholderSubstitutionTest.java +++ b/hibernate-reactive-core/src/test/java/org/hibernate/reactive/NativeQueryPlaceholderSubstitutionTest.java @@ -5,7 +5,17 @@ */ package org.hibernate.reactive; -import static java.util.concurrent.TimeUnit.MINUTES; +import java.util.Collection; +import java.util.List; + +import org.hibernate.boot.registry.StandardServiceRegistryBuilder; +import org.hibernate.cfg.Configuration; +import org.hibernate.reactive.provider.Settings; +import org.hibernate.reactive.testing.DBSelectionExtension; +import org.hibernate.reactive.testing.SqlStatementTracker; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.RegisterExtension; import io.vertx.junit5.Timeout; import io.vertx.junit5.VertxTestContext; @@ -13,53 +23,111 @@ import jakarta.persistence.GeneratedValue; import jakarta.persistence.Id; import jakarta.persistence.Table; -import java.util.Collection; -import java.util.List; -import org.junit.jupiter.api.Assertions; -import org.junit.jupiter.api.Test; +import static java.util.concurrent.TimeUnit.MINUTES; +import static org.assertj.core.api.Assertions.assertThat; +import static org.hibernate.reactive.containers.DatabaseConfiguration.DBType.POSTGRESQL; +import static org.hibernate.reactive.testing.DBSelectionExtension.runOnlyFor; + +/** + * The processing of the query is done by Hibernate ORM in {@link org.hibernate.reactive.query.sql.internal.ReactiveNativeSelectQueryPlanImpl} + * via the {@link org.hibernate.query.sql.internal.SQLQueryParser} for all databases. + *

+ * We are only testing this only on PostgreSQL, so that we can keep it simple. + * We might add the other databases when necessary + *

+ * + */ @Timeout(value = 10, timeUnit = MINUTES) public class NativeQueryPlaceholderSubstitutionTest extends BaseReactiveTest { - @Override - protected Collection> annotatedEntities() { - return List.of(Widget.class); - } - - @Test - public void testThatSchemaGetsSubstitutedDuringNativeSelectQuery(VertxTestContext context) { - - test(context, getSessionFactory().withSession(session -> - session.createNativeQuery("select count(*) from {h-schema}widgets", Integer.class) - .getSingleResult() - .thenAccept(result -> Assertions.assertEquals(0, result)) - )); - } - - @Test - public void testThatSchemaGetsSubstitutedDuringNativeNonSelectQuery(VertxTestContext context) { - - test(context, getSessionFactory().withSession(session -> - session.createNativeQuery("update {h-schema}widgets set id = 1") - .executeUpdate() - .thenAccept(result -> Assertions.assertEquals(0, result)) - )); - } - - @Entity(name = "Widget") - @Table(name = "widgets") - public static class Widget { - - @Id - @GeneratedValue - private Long id; - - @Override - public String toString() { - return "Widget{" + - "id=" + id + - '}'; - } - } + @RegisterExtension + public DBSelectionExtension dbRule = runOnlyFor( POSTGRESQL ); + + @Override + protected Collection> annotatedEntities() { + return List.of( Widget.class ); + } + + private static SqlStatementTracker sqlStatementTracker; + + @Override + protected Configuration constructConfiguration() { + Configuration configuration = super.constructConfiguration(); + configuration.setProperty( Settings.DEFAULT_SCHEMA, "public" ); + sqlStatementTracker = new SqlStatementTracker( + NativeQueryPlaceholderSubstitutionTest::isSelectOrUpdate, + configuration.getProperties() + ); + return configuration; + } + + @Override + protected void addServices(StandardServiceRegistryBuilder builder) { + sqlStatementTracker.registerService( builder ); + } + + private static boolean isSelectOrUpdate(String sql) { + return sql.startsWith( "select count" ) || sql.startsWith( "update " ); + } + + @Test + public void testSchemaPlaceHolderSubstitution(VertxTestContext context) { + sqlStatementTracker.clear(); + test( context, getSessionFactory() + .withSession( session -> session + .createNativeQuery( "select count(*) from {h-schema}widgets", Integer.class ) + .getSingleResult() + .thenAccept( result -> assertThat( result ).isZero() ) ) + .thenCompose( v -> getSessionFactory().withSession( session -> session + .createNativeQuery( "update {h-schema}widgets set id = 1" ) + .executeUpdate() + .thenAccept( result -> assertThat( result ).isZero() ) ) ) + .thenAccept( v -> assertThat( sqlStatementTracker.getLoggedQueries() ) + .containsExactly( "select count(*) from public.widgets", "update public.widgets set id = 1" ) ) + ); + } + + @Test + public void testDomainPlaceHolderSubstitution(VertxTestContext context) { + sqlStatementTracker.clear(); + test( context, getSessionFactory() + .withSession( session -> session + .createNativeQuery( "select count(*) from {h-domain}widgets", Integer.class ) + .getSingleResult() + .thenAccept( result -> assertThat( result ).isZero() ) ) + .thenCompose( v -> getSessionFactory().withSession( session -> session + .createNativeQuery( "update {h-domain}widgets set id = 1" ) + .executeUpdate() + .thenAccept( result -> assertThat( result ).isZero() ) ) ) + .thenAccept( v -> assertThat( sqlStatementTracker.getLoggedQueries() ) + .containsExactly( "select count(*) from public.widgets", "update public.widgets set id = 1" ) ) + ); + } + + @Test + public void testCatalogPlaceHolderSubstitution(VertxTestContext context) { + sqlStatementTracker.clear(); + test( context, getSessionFactory() + .withSession( session -> session + .createNativeQuery( "select count(*) from {h-catalog}widgets", Integer.class ) + .getSingleResult() + .thenAccept( result -> assertThat( result ).isZero() ) ) + .thenCompose( v -> getSessionFactory().withSession( session -> session + .createNativeQuery( "update {h-catalog}widgets set id = 1" ) + .executeUpdate() + .thenAccept( result -> assertThat( result ).isZero() ) ) ) + // PostgreSQL uses the schema, so that catalog property is null + .thenAccept( v -> assertThat( sqlStatementTracker.getLoggedQueries() ) + .containsExactly( "select count(*) from widgets", "update widgets set id = 1" ) ) + ); + } + @Entity(name = "Widget") + @Table(name = "widgets") + public static class Widget { + @Id + @GeneratedValue + private Long id; + } } From 8addc4aa81284a80c02fad8cfd5a51748f124747 Mon Sep 17 00:00:00 2001 From: Davide D'Alto Date: Thu, 31 Aug 2023 16:14:31 +0200 Subject: [PATCH 3/3] Remove DatabaseSelectionRule This was for JUnit 4. Now, with JUnit 5, we have `org.hibernate.reactive.testing.DBSelectionExtension` --- .../testing/DatabaseSelectionRule.java | 113 ------------------ .../testing/DatabaseSelectionRuleTests.java | 63 ---------- 2 files changed, 176 deletions(-) delete mode 100644 hibernate-reactive-core/src/test/java/org/hibernate/reactive/testing/DatabaseSelectionRule.java delete mode 100644 hibernate-reactive-core/src/test/java/org/hibernate/reactive/testing/DatabaseSelectionRuleTests.java diff --git a/hibernate-reactive-core/src/test/java/org/hibernate/reactive/testing/DatabaseSelectionRule.java b/hibernate-reactive-core/src/test/java/org/hibernate/reactive/testing/DatabaseSelectionRule.java deleted file mode 100644 index f00b853a3..000000000 --- a/hibernate-reactive-core/src/test/java/org/hibernate/reactive/testing/DatabaseSelectionRule.java +++ /dev/null @@ -1,113 +0,0 @@ -/* Hibernate, Relational Persistence for Idiomatic Java - * - * SPDX-License-Identifier: Apache-2.0 - * Copyright: Red Hat Inc. and Hibernate Authors - */ -package org.hibernate.reactive.testing; - -import java.util.Arrays; -import java.util.stream.Collectors; - -import org.hibernate.reactive.containers.DatabaseConfiguration; - -import org.junit.Assume; -import org.junit.rules.TestRule; -import org.junit.runner.Description; -import org.junit.runners.model.Statement; - -import org.hibernate.reactive.containers.DatabaseConfiguration.DBType; - -/** - * Define which db is enabled for the tests in a class. - *

- * Examples of use: - * - * {@code - * @Rule - * public DatabaseSelectionRule rule = DatabaseSelectionRule.skipTestsFor( DBType.POSTGRESQL ); - * } - * - * {@code - * @Rule - * public DatabaseSelectionRule rule = DatabaseSelectionRule.runOnlyFor( DBType.POSTGRESQL ); - * } - *

- * - * @see DBType - */ -public class DatabaseSelectionRule implements TestRule { - - /** - * The selected db for running the tests - */ - private final DBType selectedDb = DatabaseConfiguration.dbType(); - - /** - * Skip the tests for these dbs - */ - private final DBType[] skippable; - private final String description; - - private DatabaseSelectionRule(DBType[] skippable, String description) { - this.skippable = skippable; - this.description = description; - } - - /** - * Create a rule that will skip the tests for the selected dbs - * - * @param dbTypes the dbs we want to skip - * - * @return an instance of {@link DatabaseSelectionRule} - */ - public static DatabaseSelectionRule skipTestsFor(DBType... dbTypes) { - return new DatabaseSelectionRule( dbTypes, "Skip tests for " + Arrays.toString( dbTypes ) ); - } - - /** - * Create a rule that will run the tests only for the selected dbs - * - * @param dbTypes the dbs we want to use for running the tests - * - * @return an instance of {@link DatabaseSelectionRule} - */ - public static DatabaseSelectionRule runOnlyFor(DBType... dbTypes) { - DBType[] skippable = Arrays.stream( DBType.values() ) - .filter( dbType -> !Arrays.asList( dbTypes ).contains( dbType ) ) - .collect( Collectors.toList() ) - .toArray( new DBType[dbTypes.length] ); - return new DatabaseSelectionRule( skippable, "Run only for " + Arrays.toString( dbTypes ) ); - } - - @Override - public Statement apply(Statement base, Description description) { - if ( isSkippable( selectedDb ) ) { - return new SkipDB( selectedDb, description ); - } - return base; - } - - private boolean isSkippable(DBType dbType) { - for ( DBType db : skippable ) { - if ( db == selectedDb ) { - return true; - } - } - return false; - } - - private static class SkipDB extends Statement { - private final DBType db; - private Description description; - - private SkipDB(DBType db, Description description) { - this.db = db; - this.description = description; - } - - @Override - public void evaluate() { - Assume.assumeTrue( "Skipping test for " + db + ", rule: " + description, false ); - } - } -} diff --git a/hibernate-reactive-core/src/test/java/org/hibernate/reactive/testing/DatabaseSelectionRuleTests.java b/hibernate-reactive-core/src/test/java/org/hibernate/reactive/testing/DatabaseSelectionRuleTests.java deleted file mode 100644 index 46783b77c..000000000 --- a/hibernate-reactive-core/src/test/java/org/hibernate/reactive/testing/DatabaseSelectionRuleTests.java +++ /dev/null @@ -1,63 +0,0 @@ -/* Hibernate, Relational Persistence for Idiomatic Java - * - * SPDX-License-Identifier: Apache-2.0 - * Copyright: Red Hat Inc. and Hibernate Authors - */ -package org.hibernate.reactive.testing; - - -import org.hibernate.reactive.containers.DatabaseConfiguration; - -import org.junit.jupiter.api.Test; -import org.junit.jupiter.api.extension.RegisterExtension; - -import static org.assertj.core.api.Assertions.assertThat; -import static org.hibernate.reactive.containers.DatabaseConfiguration.DBType.MARIA; -import static org.hibernate.reactive.containers.DatabaseConfiguration.DBType.MYSQL; -import static org.hibernate.reactive.containers.DatabaseConfiguration.DBType.POSTGRESQL; -import static org.hibernate.reactive.containers.DatabaseConfiguration.dbType; -import static org.hibernate.reactive.testing.DBSelectionExtension.runOnlyFor; -import static org.hibernate.reactive.testing.DBSelectionExtension.skipTestsFor; - -public class DatabaseSelectionRuleTests { - - public static class SkipDBTest { - @RegisterExtension - public DBSelectionExtension dbSelection = skipTestsFor( POSTGRESQL ); - - @Test - public void shouldSkipPostgres() { - assertThat( dbType() ).isNotEqualTo( POSTGRESQL ); - } - } - - public static class SkipMultipleDBsTest { - @RegisterExtension - public DBSelectionExtension skipDbs = skipTestsFor( POSTGRESQL, MYSQL, MARIA ); - - @Test - public void shouldSkipPostgresMySQLAndMaria() { - assertThat( dbType() ).isNotIn( POSTGRESQL, MYSQL, MARIA ); - } - } - - public static class RunOnlyOnDBTest { - @RegisterExtension - public DBSelectionExtension skipDbs = runOnlyFor( POSTGRESQL ); - - @Test - public void shouldOnlyRunForPostgres() { - assertThat( dbType() ).isEqualTo( POSTGRESQL ); - } - } - - public static class RunOnlyOnDMultipleDBsTest { - @RegisterExtension - public DBSelectionExtension rule = runOnlyFor( POSTGRESQL, MYSQL, MARIA ); - - @Test - public void shouldOnlyRunForPostgresOrMySql() { - assertThat( DatabaseConfiguration.dbType() ).isIn( POSTGRESQL, MYSQL, MARIA ); - } - } -}