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

Support multiple init scripts in JdbcDatabaseContainer #7680

Merged
merged 2 commits into from
Jul 17, 2024

Conversation

savinov
Copy link
Contributor

@savinov savinov commented Oct 18, 2023

Inspired by PRs: #2578 and #1997
Closes #2232

@mrsofiane
Copy link

This feature is essential, the PR looks good, I hope it will be accepted as soon as possible, good work.

@ujjaldey
Copy link

ujjaldey commented May 4, 2024

This is a must-have feature. Could anyone please approve and merge this PR?

@sdavids
Copy link

sdavids commented May 15, 2024

Using private Map<String, String> initScriptPaths = new LinkedHashMap<>(); would support my use case #8634.

@eddumelendez eddumelendez added this to the next milestone Jul 17, 2024
@eddumelendez eddumelendez merged commit 6a07650 into testcontainers:main Jul 17, 2024
97 checks passed
@eddumelendez
Copy link
Member

Thanks for your contribution, @savinov !

@Asya-Vorobyova
Copy link

Looks like this change is breaking for the case when initScriptPath isn't provided. Then initScriptPaths is initialized with one null element, and then application failed when trying to get the resource in ScriptUtils.runInitScript() method.

@mikymigs
Copy link

Looks like this change is breaking for the case when initScriptPath isn't provided. Then initScriptPaths is initialized with one null element, and then application failed when trying to get the resource in ScriptUtils.runInitScript() method.

Same issue

@Asya-Vorobyova
Copy link

@savinov could somebody fix it, please? Thnx in advance :)

mmorshedi added a commit to mmorshedi/testcontainers-java that referenced this pull request Aug 15, 2024
In PR testcontainers#7680, a new feature was introduced to support multiple init scripts in JdbcDatabaseContainer. However, during this update, a critical null check was inadvertently removed from the `JdbcDatabaseContainer.runInitScriptIfRequired` method. This check was responsible for ignoring null values for the `initScript` parameter.

As a result, when no init script is defined, a null value is passed to `ScriptUtils.runInitScript`, leading to an exception being thrown and preventing the database container from starting.

This commit reinstates the missing null check, ensuring that undefined init scripts are properly handled. Additionally, a test has been added to verify that the issue is resolved and the database container can start without an init script.
if (initScriptPath != null) {
ScriptUtils.runInitScript(getDatabaseDelegate(), initScriptPath);
}
initScriptPaths.forEach(path -> ScriptUtils.runInitScript(getDatabaseDelegate(), path));

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we lost the null point checker here

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improve usability of SQL init scripts with multiple file support
8 participants