Skip to content

Commit 9cb926c

Browse files
whitewhite
white
authored and
white
committed
Fail-fast Parameter Validation
- PythonEmbed.Options.Builder: null checks for warmupScript(), warmupScripts(), onBeforeClose(), onAfterClose() - PythonEmbed.Options.Builder.build(): validate blank pythonExecutable, non-existent/not-directory venvPath, null env - PythonEmbedPool.Builder: null check for options parameter - Add 8 validation tests in PythonEmbedOptionsTest, 1 in PythonEmbedPoolTest - Update existing tests to reflect fail-fast behavior (IllegalArgumentException at builder level) - CI: add --refresh-dependencies to gradle test step
1 parent b10e200 commit 9cb926c

7 files changed

Lines changed: 117 additions & 28 deletions

File tree

‎python-embed-runtime/src/main/java/io/github/howtis/pythonembed/PythonEmbed.java‎

Lines changed: 30 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1339,12 +1339,23 @@ public Builder pythonExecutable(String value) {
13391339

13401340
/** Append a warmup script to execute after instance initialization. */
13411341
public Builder warmupScript(String script) {
1342+
if (script == null) {
1343+
throw new IllegalArgumentException("warmupScript must not be null");
1344+
}
13421345
this.warmupScripts.add(script);
13431346
return this;
13441347
}
13451348

13461349
/** Append multiple warmup scripts at once. */
13471350
public Builder warmupScripts(List<String> scripts) {
1351+
if (scripts == null) {
1352+
throw new IllegalArgumentException("warmupScripts must not be null");
1353+
}
1354+
for (String s : scripts) {
1355+
if (s == null) {
1356+
throw new IllegalArgumentException("warmup script must not be null");
1357+
}
1358+
}
13481359
this.warmupScripts.addAll(scripts);
13491360
return this;
13501361
}
@@ -1375,6 +1386,9 @@ public Builder env(Map<String, String> value) {
13751386
* Multiple hooks may be registered and will be called in order.
13761387
*/
13771388
public Builder onBeforeClose(CloseHook hook) {
1389+
if (hook == null) {
1390+
throw new IllegalArgumentException("close hook must not be null");
1391+
}
13781392
this.beforeCloseHooks.add(hook);
13791393
return this;
13801394
}
@@ -1384,6 +1398,9 @@ public Builder onBeforeClose(CloseHook hook) {
13841398
* Multiple hooks may be registered and will be called in order.
13851399
*/
13861400
public Builder onAfterClose(CloseHook hook) {
1401+
if (hook == null) {
1402+
throw new IllegalArgumentException("close hook must not be null");
1403+
}
13871404
this.afterCloseHooks.add(hook);
13881405
return this;
13891406
}
@@ -1393,7 +1410,9 @@ public Builder onAfterClose(CloseHook hook) {
13931410
*
13941411
* @return a new {@link Options} instance
13951412
* @throws IllegalArgumentException if {@code timeoutMs <= 0},
1396-
* {@code maxCodeLength <= 0}, or {@code startupTimeoutMs <= 0}
1413+
* {@code maxCodeLength <= 0}, or {@code startupTimeoutMs <= 0},
1414+
* or {@code pythonExecutable} is blank, or {@code venvPath}
1415+
* does not exist or is not a directory, or {@code env} is null
13971416
*/
13981417
public Options build() {
13991418
if (timeoutMs <= 0) {
@@ -1405,6 +1424,16 @@ public Options build() {
14051424
if (startupTimeoutMs <= 0) {
14061425
throw new IllegalArgumentException("startupTimeoutMs must be positive");
14071426
}
1427+
if (pythonExecutable != null && pythonExecutable.isBlank()) {
1428+
throw new IllegalArgumentException("pythonExecutable must not be blank");
1429+
}
1430+
if (venvPath != null && !java.nio.file.Files.isDirectory(venvPath)) {
1431+
throw new IllegalArgumentException(
1432+
"venvPath does not exist or is not a directory: " + venvPath);
1433+
}
1434+
if (env == null) {
1435+
throw new IllegalArgumentException("env must not be null");
1436+
}
14081437
return new Options(timeoutMs,
14091438
maxCodeLength, startupTimeoutMs, pythonExecutable,
14101439
List.copyOf(warmupScripts), lenientWarmup,

‎python-embed-runtime/src/main/java/io/github/howtis/pythonembed/PythonEmbedPool.java‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -189,6 +189,9 @@ private PythonEmbedPool(Builder b) {
189189
if (b.healthCheckIntervalMs < 0) {
190190
throw new IllegalArgumentException("healthCheckIntervalMs must be >= 0");
191191
}
192+
if (b.options == null) {
193+
throw new IllegalArgumentException("options must not be null");
194+
}
192195

193196
this.minPool = b.minPool;
194197
this.maxPool = b.maxPool;

‎python-embed-runtime/src/test/java/io/github/howtis/pythonembed/PythonEmbedExplicitPathTest.java‎

Lines changed: 10 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -102,27 +102,22 @@ void create_withEnvVars_passesToProcess() {
102102
}
103103

104104
@Test
105-
void create_nonexistentPath_throwsPythonExecutionException() {
105+
void build_nonexistentVenvPath_throwsIllegalArgumentException() {
106106
Path nonexistent = Path.of("/nonexistent/python-embed-venv-" + System.nanoTime());
107-
PythonExecutionException ex = assertThrows(PythonExecutionException.class,
108-
() -> PythonEmbed.create(
109-
PythonEmbed.Options.builder()
110-
.venvPath(nonexistent).build()));
111-
assertTrue(ex.getCause() instanceof IOException);
112-
assertTrue(ex.getCause().getMessage().contains("does not exist"));
107+
IllegalArgumentException ex = assertThrows(IllegalArgumentException.class,
108+
() -> PythonEmbed.Options.builder()
109+
.venvPath(nonexistent).build());
110+
assertTrue(ex.getMessage().contains("does not exist") || ex.getMessage().contains("not a directory"));
113111
}
114112

115113
@Test
116-
void create_pathIsFile_throwsPythonExecutionException(@TempDir Path tempDir) throws Exception {
114+
void build_venvPathIsFile_throwsIllegalArgumentException(@TempDir Path tempDir) throws Exception {
117115
Path aFile = tempDir.resolve("not-a-directory.txt");
118116
Files.createFile(aFile);
119-
PythonExecutionException ex = assertThrows(PythonExecutionException.class,
120-
() -> PythonEmbed.create(
121-
PythonEmbed.Options.builder()
122-
.venvPath(aFile).build()));
123-
assertTrue(ex.getCause() instanceof IOException);
124-
assertTrue(ex.getCause().getMessage().contains("does not exist")
125-
|| ex.getCause().getMessage().contains("not a directory"));
117+
IllegalArgumentException ex = assertThrows(IllegalArgumentException.class,
118+
() -> PythonEmbed.Options.builder()
119+
.venvPath(aFile).build());
120+
assertTrue(ex.getMessage().contains("does not exist") || ex.getMessage().contains("not a directory"));
126121
}
127122

128123
// ------------------------------------------------------------------

‎python-embed-runtime/src/test/java/io/github/howtis/pythonembed/PythonEmbedIntegrationTest.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -267,8 +267,8 @@ void options_defaults_workWithoutBuilder() {
267267
@Test
268268
@Timeout(value = 10, unit = TimeUnit.SECONDS)
269269
void options_withExplicitVenvPath() {
270-
// Test that builder with a non-existent venv path throws PythonExecutionException
271-
assertThrows(PythonExecutionException.class, () -> PythonEmbed.create(
270+
// Test that builder with a non-existent venv path fails fast
271+
assertThrows(IllegalArgumentException.class, () -> PythonEmbed.create(
272272
PythonEmbed.Options.builder()
273273
.venvPath(Path.of("/nonexistent/venv"))
274274
.build()));

‎python-embed-runtime/src/test/java/io/github/howtis/pythonembed/PythonEmbedOptionsTest.java‎

Lines changed: 62 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,18 @@
11
package io.github.howtis.pythonembed;
22

33
import org.junit.jupiter.api.Test;
4+
import org.junit.jupiter.api.io.TempDir;
45

56
import java.nio.file.Path;
7+
import java.util.Arrays;
68
import java.util.List;
79
import java.util.Map;
810

911
import static org.junit.jupiter.api.Assertions.assertEquals;
1012
import static org.junit.jupiter.api.Assertions.assertFalse;
1113
import static org.junit.jupiter.api.Assertions.assertNotNull;
1214
import static org.junit.jupiter.api.Assertions.assertNull;
15+
import static org.junit.jupiter.api.Assertions.assertThrows;
1316
import static org.junit.jupiter.api.Assertions.assertTrue;
1417

1518
class PythonEmbedOptionsTest {
@@ -168,13 +171,12 @@ void builder_lenientWarmup_setFalse() {
168171
// ---- venvPath ----
169172

170173
@Test
171-
void builder_venvPath_setsValue() {
172-
Path path = Path.of("/opt/venv");
174+
void builder_venvPath_setsValue(@TempDir Path tempDir) {
173175
PythonEmbed.Options opts = PythonEmbed.Options.builder()
174-
.venvPath(path)
176+
.venvPath(tempDir)
175177
.build();
176178

177-
assertEquals(path, opts.venvPath());
179+
assertEquals(tempDir, opts.venvPath());
178180
}
179181

180182
@Test
@@ -267,6 +269,59 @@ void builder_onAfterClose_emptyByDefault() {
267269
assertTrue(opts.afterCloseHooks().isEmpty());
268270
}
269271

272+
// ---- build() validation ----
273+
274+
@Test
275+
void build_pythonExecutable_blank_throws() {
276+
assertThrows(IllegalArgumentException.class,
277+
() -> PythonEmbed.Options.builder().pythonExecutable("").build());
278+
assertThrows(IllegalArgumentException.class,
279+
() -> PythonEmbed.Options.builder().pythonExecutable(" ").build());
280+
}
281+
282+
@Test
283+
void build_venvPath_nonexistent_throws() {
284+
Path nonexistent = Path.of("/nonexistent/venv/path");
285+
assertThrows(IllegalArgumentException.class,
286+
() -> PythonEmbed.Options.builder().venvPath(nonexistent).build());
287+
}
288+
289+
@Test
290+
void build_env_null_throws() {
291+
assertThrows(IllegalArgumentException.class,
292+
() -> PythonEmbed.Options.builder().env(null).build());
293+
}
294+
295+
@Test
296+
void warmupScript_null_throws() {
297+
assertThrows(IllegalArgumentException.class,
298+
() -> PythonEmbed.Options.builder().warmupScript(null));
299+
}
300+
301+
@Test
302+
void warmupScripts_null_throws() {
303+
assertThrows(IllegalArgumentException.class,
304+
() -> PythonEmbed.Options.builder().warmupScripts(null));
305+
}
306+
307+
@Test
308+
void warmupScripts_nullElement_throws() {
309+
assertThrows(IllegalArgumentException.class,
310+
() -> PythonEmbed.Options.builder().warmupScripts(Arrays.asList("import math", null)));
311+
}
312+
313+
@Test
314+
void onBeforeClose_null_throws() {
315+
assertThrows(IllegalArgumentException.class,
316+
() -> PythonEmbed.Options.builder().onBeforeClose(null));
317+
}
318+
319+
@Test
320+
void onAfterClose_null_throws() {
321+
assertThrows(IllegalArgumentException.class,
322+
() -> PythonEmbed.Options.builder().onAfterClose(null));
323+
}
324+
270325
// ---- build() immutability ----
271326

272327
@Test
@@ -296,7 +351,7 @@ void build_createsImmutableCopyOfHooks() {
296351
// ---- Builder chainability ----
297352

298353
@Test
299-
void builder_isChainable() {
354+
void builder_isChainable(@TempDir Path tempDir) {
300355
PythonEmbed.Options opts = PythonEmbed.Options.builder()
301356
.timeoutMs(60_000)
302357
.maxCodeLength(50_000)
@@ -305,7 +360,7 @@ void builder_isChainable() {
305360
.warmupScript("import math")
306361
.warmupScripts(List.of("import json"))
307362
.lenientWarmup(false)
308-
.venvPath(Path.of("/opt/venv"))
363+
.venvPath(tempDir)
309364
.env(Map.of("KEY", "VAL"))
310365
.onBeforeClose((embed, reason) -> {})
311366
.onAfterClose((embed, reason) -> {})
@@ -317,7 +372,7 @@ void builder_isChainable() {
317372
assertEquals("/usr/bin/python3", opts.pythonExecutable());
318373
assertEquals(List.of("import math", "import json"), opts.warmupScripts());
319374
assertFalse(opts.lenientWarmup());
320-
assertEquals(Path.of("/opt/venv"), opts.venvPath());
375+
assertEquals(tempDir, opts.venvPath());
321376
assertEquals(Map.of("KEY", "VAL"), opts.env());
322377
assertEquals(1, opts.beforeCloseHooks().size());
323378
assertEquals(1, opts.afterCloseHooks().size());

‎python-embed-runtime/src/test/java/io/github/howtis/pythonembed/PythonEmbedPoolTest.java‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -632,6 +632,12 @@ void maxPool_mustBeAtLeastMinPool() {
632632
assertThrows(IllegalArgumentException.class, () -> PythonEmbedPool.builder().minPool(3).maxPool(2).build());
633633
}
634634

635+
@Test
636+
void options_mustNotBeNull() {
637+
assertThrows(IllegalArgumentException.class,
638+
() -> PythonEmbedPool.builder().options(null).build());
639+
}
640+
635641
// ------------------------------------------------------------------
636642
// Options.Builder validation
637643
// ------------------------------------------------------------------

‎python-embed-spring-boot-starter/src/test/java/io/github/howtis/pythonembed/spring/PythonEmbedOptionsBuildTest.java‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
import io.github.howtis.pythonembed.PythonEmbed;
44
import org.junit.jupiter.api.Test;
5+
import org.junit.jupiter.api.io.TempDir;
56
import org.springframework.boot.context.properties.bind.Binder;
67
import org.springframework.boot.context.properties.source.MapConfigurationPropertySource;
78

@@ -32,10 +33,10 @@ void venvPathBlankUsesNoExplicitPath() {
3233
}
3334

3435
@Test
35-
void venvPathSetToCustomPath() {
36-
var props = bind(Map.of("python-embed.venv-path", "/opt/custom-venv"));
36+
void venvPathSetToCustomPath(@TempDir Path tempDir) {
37+
var props = bind(Map.of("python-embed.venv-path", tempDir.toString()));
3738
var options = PythonEmbedAutoConfiguration.buildOptions(props);
38-
assertThat(options.venvPath()).isEqualTo(Path.of("/opt/custom-venv"));
39+
assertThat(options.venvPath()).isEqualTo(tempDir);
3940
}
4041

4142
@Test

0 commit comments

Comments
 (0)