From 84f678a06bb787641f492633552fa0de5e506d17 Mon Sep 17 00:00:00 2001 From: Sam Brannen Date: Sun, 13 Apr 2014 14:51:42 +0200 Subject: [PATCH] Assert preconditions in ResourceDatabasePopulator Prior to this commit ResourceDatabasePopulator did not assert any preconditions for constructor and method arguments. Consequently, errors would not be encountered until the populator was executed. This commit addresses this issue by ensuring that preconditions for constructor and method arguments are asserted immediately, throwing IllegalArgumentExceptions where appropriate. Issue: SPR-11690 --- .../datasource/init/DatabasePopulator.java | 2 +- .../init/ResourceDatabasePopulator.java | 46 +++++++--- .../init/ResourceDatabasePopulatorTests.java | 88 +++++++++++++++++++ 3 files changed, 122 insertions(+), 14 deletions(-) create mode 100644 spring-jdbc/src/test/java/org/springframework/jdbc/datasource/init/ResourceDatabasePopulatorTests.java diff --git a/spring-jdbc/src/main/java/org/springframework/jdbc/datasource/init/DatabasePopulator.java b/spring-jdbc/src/main/java/org/springframework/jdbc/datasource/init/DatabasePopulator.java index b6b9f2d205..d72ee0cab3 100644 --- a/spring-jdbc/src/main/java/org/springframework/jdbc/datasource/init/DatabasePopulator.java +++ b/spring-jdbc/src/main/java/org/springframework/jdbc/datasource/init/DatabasePopulator.java @@ -40,7 +40,7 @@ public interface DatabasePopulator { * {@link ResourceDatabasePopulator} and {@link DatabasePopulatorUtils} wrap * all {@code SQLExceptions} in {@code ScriptExceptions}. * @param connection the JDBC connection to use to populate the db; already - * configured and ready to use + * configured and ready to use; never {@code null} * @throws SQLException if an unrecoverable data access exception occurs * during database population * @throws ScriptException in all other error cases diff --git a/spring-jdbc/src/main/java/org/springframework/jdbc/datasource/init/ResourceDatabasePopulator.java b/spring-jdbc/src/main/java/org/springframework/jdbc/datasource/init/ResourceDatabasePopulator.java index 8451c985f9..8bd50ebc8e 100644 --- a/spring-jdbc/src/main/java/org/springframework/jdbc/datasource/init/ResourceDatabasePopulator.java +++ b/spring-jdbc/src/main/java/org/springframework/jdbc/datasource/init/ResourceDatabasePopulator.java @@ -16,7 +16,6 @@ package org.springframework.jdbc.datasource.init; - import java.sql.Connection; import java.util.ArrayList; import java.util.Arrays; @@ -26,6 +25,7 @@ import javax.sql.DataSource; import org.springframework.core.io.Resource; import org.springframework.core.io.support.EncodedResource; +import org.springframework.util.Assert; /** * Populates, initializes, or cleans up a database using SQL scripts defined in @@ -85,7 +85,7 @@ public class ResourceDatabasePopulator implements DatabasePopulator { */ public ResourceDatabasePopulator(Resource... scripts) { this(); - this.scripts = Arrays.asList(scripts); + setScripts(scripts); } /** @@ -96,7 +96,8 @@ public class ResourceDatabasePopulator implements DatabasePopulator { * statement can be ignored * @param sqlScriptEncoding the encoding for the supplied SQL scripts, if * different from the platform encoding; may be {@code null} - * @param scripts the scripts to execute to initialize or populate the database + * @param scripts the scripts to execute to initialize or populate the database; + * never {@code null} * @since 4.0.3 */ public ResourceDatabasePopulator(boolean continueOnError, boolean ignoreFailedDrops, String sqlScriptEncoding, @@ -109,26 +110,29 @@ public class ResourceDatabasePopulator implements DatabasePopulator { /** * Add a script to execute to initialize or populate the database. - * @param script the path to an SQL script + * @param script the path to an SQL script; never {@code null} */ public void addScript(Resource script) { - this.scripts.add(script); + Assert.notNull(script, "script must not be null"); + getScripts().add(script); } /** * Add multiple scripts to execute to initialize or populate the database. - * @param scripts the scripts to execute + * @param scripts the scripts to execute; never {@code null} */ public void addScripts(Resource... scripts) { - this.scripts.addAll(Arrays.asList(scripts)); + assertContentsOfScriptArray(scripts); + getScripts().addAll(Arrays.asList(scripts)); } /** * Set the scripts to execute to initialize or populate the database, * replacing any previously added scripts. - * @param scripts the scripts to execute + * @param scripts the scripts to execute; never {@code null} */ public void setScripts(Resource... scripts) { + assertContentsOfScriptArray(scripts); this.scripts = Arrays.asList(scripts); } @@ -165,11 +169,13 @@ public class ResourceDatabasePopulator implements DatabasePopulator { * Set the start delimiter that identifies block comments within the SQL * scripts. *

Defaults to {@code "/*"}. - * @param blockCommentStartDelimiter the start delimiter for block comments + * @param blockCommentStartDelimiter the start delimiter for block comments; + * never {@code null} or empty * @since 4.0.3 * @see #setBlockCommentEndDelimiter */ public void setBlockCommentStartDelimiter(String blockCommentStartDelimiter) { + Assert.hasText(blockCommentStartDelimiter, "blockCommentStartDelimiter must not be null or empty"); this.blockCommentStartDelimiter = blockCommentStartDelimiter; } @@ -177,11 +183,13 @@ public class ResourceDatabasePopulator implements DatabasePopulator { * Set the end delimiter that identifies block comments within the SQL * scripts. *

Defaults to "*/". - * @param blockCommentEndDelimiter the end delimiter for block comments + * @param blockCommentEndDelimiter the end delimiter for block comments; + * never {@code null} or empty * @since 4.0.3 * @see #setBlockCommentStartDelimiter */ public void setBlockCommentEndDelimiter(String blockCommentEndDelimiter) { + Assert.hasText(blockCommentEndDelimiter, "blockCommentEndDelimiter must not be null or empty"); this.blockCommentEndDelimiter = blockCommentEndDelimiter; } @@ -212,7 +220,8 @@ public class ResourceDatabasePopulator implements DatabasePopulator { */ @Override public void populate(Connection connection) throws ScriptException { - for (Resource script : this.scripts) { + Assert.notNull(connection, "Connection must not be null"); + for (Resource script : getScripts()) { ScriptUtils.executeSqlScript(connection, encodeScript(script), this.continueOnError, this.ignoreFailedDrops, this.commentPrefix, this.separator, this.blockCommentStartDelimiter, this.blockCommentEndDelimiter); @@ -223,23 +232,34 @@ public class ResourceDatabasePopulator implements DatabasePopulator { * Execute this {@code ResourceDatabasePopulator} against the given * {@link DataSource}. *

Delegates to {@link DatabasePopulatorUtils#execute}. - * @param dataSource the {@code DataSource} to execute against + * @param dataSource the {@code DataSource} to execute against; never {@code null} * @throws ScriptException if an error occurs * @since 4.1 * @see #populate(Connection) */ public void execute(DataSource dataSource) throws ScriptException { + Assert.notNull(dataSource, "DataSource must not be null"); DatabasePopulatorUtils.execute(this, dataSource); } + final List getScripts() { + return this.scripts; + } + /** * {@link EncodedResource} is not a sub-type of {@link Resource}. Thus we * always need to wrap each script resource in an {@code EncodedResource} * using the configured {@linkplain #setSqlScriptEncoding encoding}. - * @param script the script to wrap + * @param script the script to wrap; never {@code null} */ private EncodedResource encodeScript(Resource script) { + Assert.notNull(script, "script must not be null"); return new EncodedResource(script, this.sqlScriptEncoding); } + private void assertContentsOfScriptArray(Resource... scripts) { + Assert.notNull(scripts, "scripts must not be null"); + Assert.noNullElements(scripts, "scripts array must not contain null elements"); + } + } diff --git a/spring-jdbc/src/test/java/org/springframework/jdbc/datasource/init/ResourceDatabasePopulatorTests.java b/spring-jdbc/src/test/java/org/springframework/jdbc/datasource/init/ResourceDatabasePopulatorTests.java new file mode 100644 index 0000000000..3e4d5a7efa --- /dev/null +++ b/spring-jdbc/src/test/java/org/springframework/jdbc/datasource/init/ResourceDatabasePopulatorTests.java @@ -0,0 +1,88 @@ +/* + * Copyright 2002-2014 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 + * + * 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 org.springframework.jdbc.datasource.init; + +import org.junit.Test; +import org.springframework.core.io.ClassPathResource; +import org.springframework.core.io.Resource; + +import static org.junit.Assert.*; + +/** + * Unit tests for {@link ResourceDatabasePopulator}. + * + * @author Sam Brannen + * @since 4.1 + * @see AbstractDatabasePopulatorTests + */ +public class ResourceDatabasePopulatorTests { + + private static final Resource script1 = resource("script1"); + private static final Resource script2 = resource("script2"); + + + @Test(expected = IllegalArgumentException.class) + public void constructWithNullResource() { + new ResourceDatabasePopulator((Resource) null); + } + + @Test(expected = IllegalArgumentException.class) + public void constructWithNullResourceArray() { + new ResourceDatabasePopulator((Resource[]) null); + } + + @Test + public void constructWithResource() { + ResourceDatabasePopulator databasePopulator = new ResourceDatabasePopulator(script1); + assertEquals(1, databasePopulator.getScripts().size()); + } + + @Test + public void constructWithMulipleResources() { + ResourceDatabasePopulator databasePopulator = new ResourceDatabasePopulator(script1, script2); + assertEquals(2, databasePopulator.getScripts().size()); + } + + @Test(expected = IllegalArgumentException.class) + public void addScriptsWithNullResource() { + ResourceDatabasePopulator databasePopulator = new ResourceDatabasePopulator(); + databasePopulator.addScripts((Resource) null); + } + + @Test(expected = IllegalArgumentException.class) + public void addScriptsWithNullResourceArray() { + ResourceDatabasePopulator databasePopulator = new ResourceDatabasePopulator(); + databasePopulator.addScripts((Resource[]) null); + } + + @Test(expected = IllegalArgumentException.class) + public void setScriptsWithNullResource() { + ResourceDatabasePopulator databasePopulator = new ResourceDatabasePopulator(); + databasePopulator.setScripts((Resource) null); + } + + @Test(expected = IllegalArgumentException.class) + public void setScriptsWithNullResourceArray() { + ResourceDatabasePopulator databasePopulator = new ResourceDatabasePopulator(); + databasePopulator.setScripts((Resource[]) null); + } + + private static Resource resource(String path) { + return new ClassPathResource(path); + } + +}