From ab8876219f8590cb1927d0a867d58b6d2acd5c92 Mon Sep 17 00:00:00 2001 From: Sam Brannen Date: Fri, 19 Jul 2019 17:14:07 +0200 Subject: [PATCH] Polish contribution See gh-1835 --- .../test/context/jdbc/Sql.java | 37 +++++++------ .../jdbc/SqlScriptsTestExecutionListener.java | 51 +++++++++--------- ...epeatableSqlAnnotationSqlScriptsTests.java | 5 -- .../test/context/jdbc/SqlMethodMergeTest.java | 30 ----------- .../context/jdbc/SqlMethodMergeTests.java | 51 ++++++++++++++++++ .../context/jdbc/SqlMethodOverrideTest.java | 30 ----------- .../context/jdbc/SqlMethodOverrideTests.java | 54 +++++++++++++++++++ 7 files changed, 150 insertions(+), 108 deletions(-) delete mode 100644 spring-test/src/test/java/org/springframework/test/context/jdbc/SqlMethodMergeTest.java create mode 100644 spring-test/src/test/java/org/springframework/test/context/jdbc/SqlMethodMergeTests.java delete mode 100644 spring-test/src/test/java/org/springframework/test/context/jdbc/SqlMethodOverrideTest.java create mode 100644 spring-test/src/test/java/org/springframework/test/context/jdbc/SqlMethodOverrideTests.java diff --git a/spring-test/src/main/java/org/springframework/test/context/jdbc/Sql.java b/spring-test/src/main/java/org/springframework/test/context/jdbc/Sql.java index 27e7e29248..d1348ac13a 100644 --- a/spring-test/src/main/java/org/springframework/test/context/jdbc/Sql.java +++ b/spring-test/src/main/java/org/springframework/test/context/jdbc/Sql.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2018 the original author or authors. + * Copyright 2002-2019 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -32,7 +32,7 @@ import org.springframework.core.annotation.AliasFor; * database during integration tests. * *

Method-level declarations override class-level declarations by default. - * This behaviour can be adjusted via {@link MergeMode} + * This behavior can be adjusted by setting the {@link #mergeMode}. * *

Script execution is performed by the {@link SqlScriptsTestExecutionListener}, * which is enabled by default. @@ -54,6 +54,7 @@ import org.springframework.core.annotation.AliasFor; * composed annotations with attribute overrides. * * @author Sam Brannen + * @author Dmitry Semukhin * @since 4.1 * @see SqlConfig * @see SqlGroup @@ -138,6 +139,16 @@ public @interface Sql { */ ExecutionPhase executionPhase() default ExecutionPhase.BEFORE_TEST_METHOD; + /** + * Indicates whether this {@code @Sql} annotation should be merged with + * class-level {@code @Sql} annotations or override them. + *

The merge mode is ignored if declared in a class-level {@code @Sql} + * annotation. + *

Defaults to {@link MergeMode#OVERRIDE OVERRIDE} for backwards compatibility. + * @since 5.2 + */ + MergeMode mergeMode() default MergeMode.OVERRIDE; + /** * Local configuration for the SQL scripts and statements declared within * this {@code @Sql} annotation. @@ -147,13 +158,6 @@ public @interface Sql { */ SqlConfig config() default @SqlConfig; - /** - * Indicates whether this annotation should be merged with upper-level annotations - * or override them. - *

Defaults to {@link MergeMode#OVERRIDE}. - */ - MergeMode mergeMode() default MergeMode.OVERRIDE; - /** * Enumeration of phases that dictate when SQL scripts are executed. @@ -174,22 +178,23 @@ public @interface Sql { } /** - * Enumeration of modes that dictate whether or not - * declared SQL {@link #scripts} and {@link #statements} are merged - * with the upper-level annotations. + * Enumeration of modes that dictate whether method-level {@code @Sql} + * declarations are merged with class-level {@code @Sql} declarations. + * @since 5.2 */ enum MergeMode { /** - * Indicates that locally declared SQL {@link #scripts} and {@link #statements} - * should override the upper-level (e.g. Class-level) annotations. + * Indicates that method-level {@code @Sql} declarations should override + * class-level {@code @Sql} declarations. */ OVERRIDE, /** - * Indicates that locally declared SQL {@link #scripts} and {@link #statements} - * should be merged the upper-level (e.g. Class-level) annotations. + * Indicates that method-level {@code @Sql} declarations should be merged + * with class-level {@code @Sql} declarations. */ MERGE } + } diff --git a/spring-test/src/main/java/org/springframework/test/context/jdbc/SqlScriptsTestExecutionListener.java b/spring-test/src/main/java/org/springframework/test/context/jdbc/SqlScriptsTestExecutionListener.java index 0ad5919761..06751e0f64 100644 --- a/spring-test/src/main/java/org/springframework/test/context/jdbc/SqlScriptsTestExecutionListener.java +++ b/spring-test/src/main/java/org/springframework/test/context/jdbc/SqlScriptsTestExecutionListener.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2018 the original author or authors. + * Copyright 2002-2019 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -26,13 +26,13 @@ import javax.sql.DataSource; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; -import org.jetbrains.annotations.NotNull; import org.springframework.context.ApplicationContext; import org.springframework.core.annotation.AnnotatedElementUtils; import org.springframework.core.io.ByteArrayResource; import org.springframework.core.io.ClassPathResource; import org.springframework.core.io.Resource; import org.springframework.jdbc.datasource.init.ResourceDatabasePopulator; +import org.springframework.lang.NonNull; import org.springframework.lang.Nullable; import org.springframework.test.context.TestContext; import org.springframework.test.context.jdbc.Sql.ExecutionPhase; @@ -84,6 +84,7 @@ import org.springframework.util.StringUtils; * locate these beans. * * @author Sam Brannen + * @author Dmitry Semukhin * @since 4.1 * @see Sql * @see SqlConfig @@ -111,7 +112,7 @@ public class SqlScriptsTestExecutionListener extends AbstractTestExecutionListen * {@link TestContext} before the current test method. */ @Override - public void beforeTestMethod(TestContext testContext) throws Exception { + public void beforeTestMethod(TestContext testContext) { executeSqlScripts(testContext, ExecutionPhase.BEFORE_TEST_METHOD); } @@ -120,7 +121,7 @@ public class SqlScriptsTestExecutionListener extends AbstractTestExecutionListen * {@link TestContext} after the current test method. */ @Override - public void afterTestMethod(TestContext testContext) throws Exception { + public void afterTestMethod(TestContext testContext) { executeSqlScripts(testContext, ExecutionPhase.AFTER_TEST_METHOD); } @@ -128,14 +129,14 @@ public class SqlScriptsTestExecutionListener extends AbstractTestExecutionListen * Execute SQL scripts configured via {@link Sql @Sql} for the supplied * {@link TestContext} and {@link ExecutionPhase}. */ - private void executeSqlScripts(TestContext testContext, ExecutionPhase executionPhase) throws Exception { - Set methodLevelSqls = getScriptsFromElement(testContext.getTestMethod()); + private void executeSqlScripts(TestContext testContext, ExecutionPhase executionPhase) { + Set methodLevelSqls = getSqlAnnotationsFor(testContext.getTestMethod()); List methodLevelOverrides = methodLevelSqls.stream() - .filter(s -> s.executionPhase() == executionPhase) - .filter(s -> s.mergeMode() == Sql.MergeMode.OVERRIDE) - .collect(Collectors.toList()); + .filter(s -> s.executionPhase() == executionPhase) + .filter(s -> s.mergeMode() == Sql.MergeMode.OVERRIDE) + .collect(Collectors.toList()); if (methodLevelOverrides.isEmpty()) { - executeScripts(getScriptsFromElement(testContext.getTestClass()), testContext, executionPhase, true); + executeScripts(getSqlAnnotationsFor(testContext.getTestClass()), testContext, executionPhase, true); executeScripts(methodLevelSqls, testContext, executionPhase, false); } else { executeScripts(methodLevelOverrides, testContext, executionPhase, false); @@ -143,20 +144,19 @@ public class SqlScriptsTestExecutionListener extends AbstractTestExecutionListen } /** - * Get SQL scripts configured via {@link Sql @Sql} for the supplied + * Get the {@link Sql @Sql} annotations declared on the supplied * {@link AnnotatedElement}. */ - private Set getScriptsFromElement(AnnotatedElement annotatedElement) throws Exception { + private Set getSqlAnnotationsFor(AnnotatedElement annotatedElement) { return AnnotatedElementUtils.getMergedRepeatableAnnotations(annotatedElement, Sql.class, SqlGroup.class); } /** - * Execute given {@link Sql @Sql} scripts. - * {@link AnnotatedElement}. + * Execute SQL scripts for the supplied {@link Sql @Sql} annotations. */ - private void executeScripts(Iterable scripts, TestContext testContext, ExecutionPhase executionPhase, - boolean classLevel) - throws Exception { + private void executeScripts( + Iterable scripts, TestContext testContext, ExecutionPhase executionPhase, boolean classLevel) { + for (Sql sql : scripts) { executeSqlScripts(sql, executionPhase, testContext, classLevel); } @@ -172,8 +172,8 @@ public class SqlScriptsTestExecutionListener extends AbstractTestExecutionListen * @param testContext the current {@code TestContext} * @param classLevel {@code true} if {@link Sql @Sql} was declared at the class level */ - private void executeSqlScripts(Sql sql, ExecutionPhase executionPhase, TestContext testContext, boolean classLevel) - throws Exception { + private void executeSqlScripts( + Sql sql, ExecutionPhase executionPhase, TestContext testContext, boolean classLevel) { if (executionPhase != sql.executionPhase()) { return; @@ -185,8 +185,6 @@ public class SqlScriptsTestExecutionListener extends AbstractTestExecutionListen mergedSqlConfig, executionPhase, testContext)); } - final ResourceDatabasePopulator populator = configurePopulator(mergedSqlConfig); - String[] scripts = getScripts(sql, testContext, classLevel); scripts = TestContextResourceUtils.convertToClasspathResourcePaths(testContext.getTestClass(), scripts); List scriptResources = TestContextResourceUtils.convertToResourceList( @@ -197,6 +195,8 @@ public class SqlScriptsTestExecutionListener extends AbstractTestExecutionListen scriptResources.add(new ByteArrayResource(stmt.getBytes(), "from inlined SQL statement: " + stmt)); } } + + ResourceDatabasePopulator populator = configurePopulator(mergedSqlConfig); populator.setScripts(scriptResources.toArray(new Resource[0])); if (logger.isDebugEnabled()) { logger.debug("Executing SQL scripts: " + ObjectUtils.nullSafeToString(scriptResources)); @@ -237,16 +237,13 @@ public class SqlScriptsTestExecutionListener extends AbstractTestExecutionListen TransactionDefinition.PROPAGATION_REQUIRED); TransactionAttribute txAttr = TestContextTransactionUtils.createDelegatingTransactionAttribute( testContext, new DefaultTransactionAttribute(propagation)); - new TransactionTemplate(txMgr, txAttr).execute(status -> { - populator.execute(finalDataSource); - return null; - }); + new TransactionTemplate(txMgr, txAttr).execute(() -> populator.execute(finalDataSource)); } } - @NotNull + @NonNull private ResourceDatabasePopulator configurePopulator(MergedSqlConfig mergedSqlConfig) { - final ResourceDatabasePopulator populator = new ResourceDatabasePopulator(); + ResourceDatabasePopulator populator = new ResourceDatabasePopulator(); populator.setSqlScriptEncoding(mergedSqlConfig.getEncoding()); populator.setSeparator(mergedSqlConfig.getSeparator()); populator.setCommentPrefix(mergedSqlConfig.getCommentPrefix()); diff --git a/spring-test/src/test/java/org/springframework/test/context/jdbc/RepeatableSqlAnnotationSqlScriptsTests.java b/spring-test/src/test/java/org/springframework/test/context/jdbc/RepeatableSqlAnnotationSqlScriptsTests.java index cbc6944a31..c1b827a1b1 100644 --- a/spring-test/src/test/java/org/springframework/test/context/jdbc/RepeatableSqlAnnotationSqlScriptsTests.java +++ b/spring-test/src/test/java/org/springframework/test/context/jdbc/RepeatableSqlAnnotationSqlScriptsTests.java @@ -25,7 +25,6 @@ import org.junit.runners.MethodSorters; import org.springframework.test.annotation.DirtiesContext; import org.springframework.test.context.ContextConfiguration; import org.springframework.test.context.junit4.AbstractTransactionalJUnit4SpringContextTests; -import org.springframework.test.jdbc.JdbcTestUtils; import static org.assertj.core.api.Assertions.assertThat; @@ -59,10 +58,6 @@ public class RepeatableSqlAnnotationSqlScriptsTests extends AbstractTransactiona assertNumUsers(2); } - protected int countRowsInTable(String tableName) { - return JdbcTestUtils.countRowsInTable(this.jdbcTemplate, tableName); - } - protected void assertNumUsers(int expected) { assertThat(countRowsInTable("user")).as("Number of rows in the 'user' table.").isEqualTo(expected); } diff --git a/spring-test/src/test/java/org/springframework/test/context/jdbc/SqlMethodMergeTest.java b/spring-test/src/test/java/org/springframework/test/context/jdbc/SqlMethodMergeTest.java deleted file mode 100644 index 3a23a9db87..0000000000 --- a/spring-test/src/test/java/org/springframework/test/context/jdbc/SqlMethodMergeTest.java +++ /dev/null @@ -1,30 +0,0 @@ -package org.springframework.test.context.jdbc; - -import org.junit.Test; -import org.springframework.test.annotation.DirtiesContext; -import org.springframework.test.context.ContextConfiguration; -import org.springframework.test.context.junit4.AbstractTransactionalJUnit4SpringContextTests; - -import static org.junit.Assert.assertEquals; - -/** - * Test to verify method level merge of @Sql annotations. - * - * @author Dmitry Semukhin - */ -@ContextConfiguration(classes = EmptyDatabaseConfig.class) -@Sql(value = {"schema.sql", "data-add-catbert.sql"}) -@DirtiesContext -public class SqlMethodMergeTest extends AbstractTransactionalJUnit4SpringContextTests { - - @Test - @Sql(value = "data-add-dogbert.sql", mergeMode = Sql.MergeMode.MERGE) - public void testMerge() { - assertNumUsers(2); - } - - protected void assertNumUsers(int expected) { - assertEquals("Number of rows in the 'user' table.", expected, countRowsInTable("user")); - } - -} diff --git a/spring-test/src/test/java/org/springframework/test/context/jdbc/SqlMethodMergeTests.java b/spring-test/src/test/java/org/springframework/test/context/jdbc/SqlMethodMergeTests.java new file mode 100644 index 0000000000..453c44af76 --- /dev/null +++ b/spring-test/src/test/java/org/springframework/test/context/jdbc/SqlMethodMergeTests.java @@ -0,0 +1,51 @@ +/* + * Copyright 2002-2019 the original author or authors. + * + * 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 + * + * https://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 org.springframework.test.context.jdbc; + +import org.junit.Test; + +import org.springframework.test.annotation.DirtiesContext; +import org.springframework.test.context.ContextConfiguration; +import org.springframework.test.context.junit4.AbstractTransactionalJUnit4SpringContextTests; + +import static org.junit.Assert.assertEquals; +import static org.springframework.test.context.jdbc.Sql.MergeMode.MERGE; + +/** + * Transactional integration tests for {@link Sql @Sql} that verify proper + * merging support for class-level and method-level declarations. + * + * @author Dmitry Semukhin + * @author Sam Brannen + * @since 5.2 + */ +@ContextConfiguration(classes = EmptyDatabaseConfig.class) +@Sql({ "schema.sql", "data-add-catbert.sql" }) +@DirtiesContext +public class SqlMethodMergeTests extends AbstractTransactionalJUnit4SpringContextTests { + + @Test + @Sql(scripts = "data-add-dogbert.sql", mergeMode = MERGE) + public void testMerge() { + assertNumUsers(2); + } + + protected void assertNumUsers(int expected) { + assertEquals("Number of rows in the 'user' table.", expected, countRowsInTable("user")); + } + +} diff --git a/spring-test/src/test/java/org/springframework/test/context/jdbc/SqlMethodOverrideTest.java b/spring-test/src/test/java/org/springframework/test/context/jdbc/SqlMethodOverrideTest.java deleted file mode 100644 index 6f4d8023c4..0000000000 --- a/spring-test/src/test/java/org/springframework/test/context/jdbc/SqlMethodOverrideTest.java +++ /dev/null @@ -1,30 +0,0 @@ -package org.springframework.test.context.jdbc; - -import org.junit.Test; -import org.springframework.test.annotation.DirtiesContext; -import org.springframework.test.context.ContextConfiguration; -import org.springframework.test.context.junit4.AbstractTransactionalJUnit4SpringContextTests; - -import static org.junit.Assert.assertEquals; - -/** - * Test to verify method level override of @Sql annotations. - * - * @author Dmitry Semukhin - */ -@ContextConfiguration(classes = EmptyDatabaseConfig.class) -@Sql(value = {"schema.sql", "data-add-catbert.sql"}) -@DirtiesContext -public class SqlMethodOverrideTest extends AbstractTransactionalJUnit4SpringContextTests { - - @Test - @Sql(value = {"schema.sql", "data.sql", "data-add-dogbert.sql", "data-add-catbert.sql"}, mergeMode = Sql.MergeMode.OVERRIDE) - public void testMerge() { - assertNumUsers(3); - } - - protected void assertNumUsers(int expected) { - assertEquals("Number of rows in the 'user' table.", expected, countRowsInTable("user")); - } - -} diff --git a/spring-test/src/test/java/org/springframework/test/context/jdbc/SqlMethodOverrideTests.java b/spring-test/src/test/java/org/springframework/test/context/jdbc/SqlMethodOverrideTests.java new file mode 100644 index 0000000000..a33731dcd5 --- /dev/null +++ b/spring-test/src/test/java/org/springframework/test/context/jdbc/SqlMethodOverrideTests.java @@ -0,0 +1,54 @@ +/* + * Copyright 2002-2019 the original author or authors. + * + * 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 + * + * https://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 org.springframework.test.context.jdbc; + +import org.junit.Test; + +import org.springframework.test.annotation.DirtiesContext; +import org.springframework.test.context.ContextConfiguration; +import org.springframework.test.context.junit4.AbstractTransactionalJUnit4SpringContextTests; + +import static org.junit.Assert.assertEquals; +import static org.springframework.test.context.jdbc.Sql.MergeMode.OVERRIDE; + +/** + * Transactional integration tests for {@link Sql @Sql} that verify proper + * overriding support for class-level and method-level declarations. + * + * @author Dmitry Semukhin + * @author Sam Brannen + * @since 5.2 + */ +@ContextConfiguration(classes = EmptyDatabaseConfig.class) +@Sql({ "schema.sql", "data-add-catbert.sql" }) +@DirtiesContext +public class SqlMethodOverrideTests extends AbstractTransactionalJUnit4SpringContextTests { + + @Test + @Sql( + scripts = { "schema.sql", "data.sql", "data-add-dogbert.sql", "data-add-catbert.sql" }, + mergeMode = OVERRIDE + ) + public void methodLevelSqlScriptsOverrideClassLevelScripts() { + assertNumUsers(3); + } + + protected void assertNumUsers(int expected) { + assertEquals("Number of rows in the 'user' table.", expected, countRowsInTable("user")); + } + +}