Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Document connection handling in ScriptUtils.executeSqlScript() methods [SPR-12908] #17507

Closed
spring-issuemaster opened this issue Apr 14, 2015 · 4 comments

Comments

Projects
None yet
2 participants
@spring-issuemaster
Copy link
Collaborator

commented Apr 14, 2015

Marcel Stör opened SPR-12908 and commented

The Javadoc for the deprecated JdbcTestUtils#executeSqlScript methods is missing information that's IMO crucial for the migration to ScriptUtils#executeSqlScript.

I migrated something like

JdbcTemplate template = new JdbcTemplate(dataSource);
org.springframework.core.io.Resource sqlScript = context.getResource(scriptLocation);
JdbcTestUtils.executeSqlScript(template, sqlScript, false);

to

try {
  ScriptUtils.executeSqlScript(dataSource.getConnection(), sqlScript);
} catch (ScriptException | SQLException e) {
  throw new RuntimeException("Failed to execute script '" + sqlScript.getFilename() + "'.", e);
}

It took me quite a while to figure out why the underlying connection pool kept throwing "Timeout waiting for idle object" exceptions.

Reason: in contrast to using JdbcTemplate with JdbcTestUtils you need to close the Connection manually if you work with ScriptUtils.

Hence, my new test method should be

try (Connection connection = dataSource.getConnection()) {
  ScriptUtils.executeSqlScript(connection, sqlScript);
} catch (ScriptException | SQLException e) {
  throw new RuntimeException("Failed to execute script '" + sqlScript.getFilename() + "'.", e);
}

It's trivial to understand why this has to be the way it is but I think it's important enough to deserve a specific remark in the \@deprecated messages (or ScriptUtils#executeSqlScript).


Affects: 4.0.3

Referenced from: commits 1226596

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Apr 14, 2015

Sam Brannen commented

Point taken. I'll augment the Javadoc for ScriptUtils#executeSqlScript accordingly.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Apr 14, 2015

Sam Brannen commented

As an alternative to your try-with-resources approach, you can make use of DatabasePopulatorUtils which handles closing the connection for you.

This is in fact what ResourceDatabasePopulator.execute(DataSource) does behind the scenes, and the following is how JdbcTestUtils.executeSqlScript(JdbcTemplate, EncodedResource, boolean) was refactored:

public static void executeSqlScript(JdbcTemplate jdbcTemplate, EncodedResource resource, boolean continueOnError) throws DataAccessException {
	new ResourceDatabasePopulator(continueOnError, false, resource.getEncoding(), resource.getResource()).execute(jdbcTemplate.getDataSource());
}

Similarly, AbstractTransactionalDataSourceSpringContextTests.executeSqlScript(String, boolean) was refactored as follows:

Resource resource = this.applicationContext.getResource(sqlResourcePath);
new ResourceDatabasePopulator(continueOnError, false, this.sqlScriptEncoding, resource).execute(jdbcTemplate.getDataSource());

So, you could simplify your try-with-resources approach like this:

new ResourceDatabasePopulator(sqlScript).execute(dataSource);
@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Apr 14, 2015

Marcel Stör commented

Oh, great, thanks for the hint.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Apr 14, 2015

Sam Brannen commented

Resolved as described in GitHub commit 1226596:

Document connection handling in ScriptUtils

This commit improves the documentation for ScriptUtils by explicitly
mentioning that a JDBC Connection supplied to the one of the
executeSqlScript() methods will not be released automatically.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.