Skip to content

Commit ccc1b5d

Browse files
philwebbphilwebb
committed
Don't call Startable.start() for already started containers
Add a new `TestcontainersStartup.start` static method and update the existing start methods so that `Startable.start()` is only called when the container is not already running. Prior to this commit, we assumed that `Startable.start()` calls were idempotent and could be safely made multiple times. Whilst this appears to be true for stock `GenericContainer` based startables, users may have their own `start()` method that does not expect to be called multiple times. The implemented detection logic will not be applied if a `Startable` is not also a `Container`. In these cases, the implementation will need to deal directly with multiple `start()` calls. Fixed gh-43253
1 parent 65a862c commit ccc1b5d

File tree

5 files changed

+253
-7
lines changed

5 files changed

+253
-7
lines changed

‎spring-boot-project/spring-boot-testcontainers/src/dockerTest/java/org/springframework/boot/testcontainers/service/connection/ServiceConnectionStartsConnectionOnceIntegrationTest.java‎

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,76 @@
1+
/*
2+
* Copyright 2012-2024 the original author or authors.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* https://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
package org.springframework.boot.testcontainers.service.connection;
18+
19+
import java.util.concurrent.atomic.AtomicInteger;
20+
21+
import org.junit.jupiter.api.Test;
22+
import org.testcontainers.containers.PostgreSQLContainer;
23+
import org.testcontainers.junit.jupiter.Container;
24+
import org.testcontainers.junit.jupiter.Testcontainers;
25+
import org.testcontainers.utility.DockerImageName;
26+
27+
import org.springframework.boot.autoconfigure.ImportAutoConfiguration;
28+
import org.springframework.boot.autoconfigure.jdbc.DataSourceAutoConfiguration;
29+
import org.springframework.boot.testsupport.container.TestImage;
30+
import org.springframework.context.annotation.Configuration;
31+
import org.springframework.test.context.junit.jupiter.SpringJUnitConfig;
32+
33+
import static org.assertj.core.api.Assertions.assertThat;
34+
35+
/**
36+
* Integration tests to ensure containers are started only once.
37+
*
38+
* @author Phillip Webb
39+
*/
40+
@SpringJUnitConfig
41+
@Testcontainers(disabledWithoutDocker = true)
42+
class ServiceConnectionStartsConnectionOnceIntegrationTest {
43+
44+
@Container
45+
@ServiceConnection
46+
static final StartCountingPostgreSQLContainer postgres = TestImage
47+
.container(StartCountingPostgreSQLContainer.class);
48+
49+
@Test
50+
void startedOnlyOnce() {
51+
assertThat(postgres.startCount.get()).isOne();
52+
}
53+
54+
@Configuration(proxyBeanMethods = false)
55+
@ImportAutoConfiguration(DataSourceAutoConfiguration.class)
56+
static class TestConfiguration {
57+
58+
}
59+
60+
static class StartCountingPostgreSQLContainer extends PostgreSQLContainer<StartCountingPostgreSQLContainer> {
61+
62+
final AtomicInteger startCount = new AtomicInteger();
63+
64+
StartCountingPostgreSQLContainer(DockerImageName dockerImageName) {
65+
super(dockerImageName);
66+
}
67+
68+
@Override
69+
public void start() {
70+
this.startCount.incrementAndGet();
71+
super.start();
72+
}
73+
74+
}
75+
76+
}

‎spring-boot-project/spring-boot-testcontainers/src/main/java/org/springframework/boot/testcontainers/lifecycle/TestcontainersLifecycleBeanPostProcessor.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,7 @@ public Object postProcessAfterInitialization(Object bean, String beanName) throw
9797
}
9898
else if (this.startables.get() == Startables.STARTED) {
9999
logger.trace(LogMessage.format("Starting container %s", beanName));
100-
startableBean.start();
100+
TestcontainersStartup.start(startableBean);
101101
}
102102
}
103103
return bean;

‎spring-boot-project/spring-boot-testcontainers/src/main/java/org/springframework/boot/testcontainers/lifecycle/TestcontainersStartup.java‎

Lines changed: 75 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2012-2023 the original author or authors.
2+
* Copyright 2012-2024 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -17,7 +17,13 @@
1717
package org.springframework.boot.testcontainers.lifecycle;
1818

1919
import java.util.Collection;
20+
import java.util.HashMap;
21+
import java.util.LinkedHashSet;
22+
import java.util.Map;
23+
import java.util.Set;
24+
import java.util.stream.Collectors;
2025

26+
import org.testcontainers.containers.Container;
2127
import org.testcontainers.lifecycle.Startable;
2228
import org.testcontainers.lifecycle.Startables;
2329

@@ -40,7 +46,7 @@ public enum TestcontainersStartup {
4046

4147
@Override
4248
void start(Collection<? extends Startable> startables) {
43-
startables.forEach(Startable::start);
49+
startables.forEach(TestcontainersStartup::start);
4450
}
4551

4652
},
@@ -52,7 +58,8 @@ void start(Collection<? extends Startable> startables) {
5258

5359
@Override
5460
void start(Collection<? extends Startable> startables) {
55-
Startables.deepStart(startables).join();
61+
SingleStartables singleStartables = new SingleStartables();
62+
Startables.deepStart(startables.stream().map(singleStartables::getOrCreate)).join();
5663
}
5764

5865
};
@@ -91,4 +98,69 @@ private static String getCanonicalName(String name) {
9198
return canonicalName.toString();
9299
}
93100

101+
/**
102+
* Start the given {@link Startable} unless is's detected as already running.
103+
* @param startable the startable to start
104+
* @since 3.4.1
105+
*/
106+
public static void start(Startable startable) {
107+
if (!isRunning(startable)) {
108+
startable.start();
109+
}
110+
}
111+
112+
private static boolean isRunning(Startable startable) {
113+
try {
114+
return (startable instanceof Container<?> container) && container.isRunning();
115+
}
116+
catch (Throwable ex) {
117+
return false;
118+
119+
}
120+
}
121+
122+
/**
123+
* Tracks and adapts {@link Startable} instances to use
124+
* {@link TestcontainersStartup#start(Startable)} so containers are only started once
125+
* even when calling {@link Startables#deepStart(java.util.stream.Stream)}.
126+
*/
127+
private static final class SingleStartables {
128+
129+
private final Map<Startable, SingleStartable> adapters = new HashMap<>();
130+
131+
SingleStartable getOrCreate(Startable startable) {
132+
return this.adapters.computeIfAbsent(startable, this::create);
133+
}
134+
135+
private SingleStartable create(Startable startable) {
136+
return new SingleStartable(this, startable);
137+
}
138+
139+
record SingleStartable(SingleStartables singleStartables, Startable startable) implements Startable {
140+
141+
@Override
142+
public Set<Startable> getDependencies() {
143+
Set<Startable> dependencies = this.startable.getDependencies();
144+
if (dependencies.isEmpty()) {
145+
return dependencies;
146+
}
147+
return dependencies.stream()
148+
.map(this.singleStartables::getOrCreate)
149+
.collect(Collectors.toCollection(LinkedHashSet::new));
150+
}
151+
152+
@Override
153+
public void start() {
154+
TestcontainersStartup.start(this.startable);
155+
}
156+
157+
@Override
158+
public void stop() {
159+
this.startable.stop();
160+
}
161+
162+
}
163+
164+
}
165+
94166
}

‎spring-boot-project/spring-boot-testcontainers/src/main/java/org/springframework/boot/testcontainers/service/connection/ContainerConnectionDetailsFactory.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@
3333
import org.springframework.boot.autoconfigure.service.connection.ConnectionDetailsFactory;
3434
import org.springframework.boot.origin.Origin;
3535
import org.springframework.boot.origin.OriginProvider;
36+
import org.springframework.boot.testcontainers.lifecycle.TestcontainersStartup;
3637
import org.springframework.context.ApplicationContext;
3738
import org.springframework.context.ApplicationContextAware;
3839
import org.springframework.core.ResolvableType;
@@ -188,7 +189,7 @@ protected final C getContainer() {
188189
Assert.state(this.container != null,
189190
"Container cannot be obtained before the connection details bean has been initialized");
190191
if (this.container instanceof Startable startable) {
191-
startable.start();
192+
TestcontainersStartup.start(startable);
192193
}
193194
return this.container;
194195
}

‎spring-boot-project/spring-boot-testcontainers/src/test/java/org/springframework/boot/testcontainers/lifecycle/TestcontainersStartupTests.java‎

Lines changed: 99 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2012-2023 the original author or authors.
2+
* Copyright 2012-2024 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -17,10 +17,13 @@
1717
package org.springframework.boot.testcontainers.lifecycle;
1818

1919
import java.util.ArrayList;
20+
import java.util.Collection;
2021
import java.util.List;
22+
import java.util.Set;
2123
import java.util.concurrent.atomic.AtomicInteger;
2224

2325
import org.junit.jupiter.api.Test;
26+
import org.testcontainers.containers.GenericContainer;
2427
import org.testcontainers.lifecycle.Startable;
2528

2629
import org.springframework.mock.env.MockEnvironment;
@@ -39,6 +42,16 @@ class TestcontainersStartupTests {
3942

4043
private final AtomicInteger counter = new AtomicInteger();
4144

45+
@Test
46+
void startSingleStartsOnlyOnce() {
47+
TestStartable startable = new TestStartable();
48+
assertThat(startable.startCount).isZero();
49+
TestcontainersStartup.start(startable);
50+
assertThat(startable.startCount).isOne();
51+
TestcontainersStartup.start(startable);
52+
assertThat(startable.startCount).isOne();
53+
}
54+
4255
@Test
4356
void startWhenSquentialStartsSequentially() {
4457
List<TestStartable> startables = createTestStartables(100);
@@ -49,13 +62,70 @@ void startWhenSquentialStartsSequentially() {
4962
}
5063
}
5164

65+
@Test
66+
void startWhenSquentialStartsOnlyOnce() {
67+
List<TestStartable> startables = createTestStartables(10);
68+
for (int i = 0; i < startables.size(); i++) {
69+
assertThat(startables.get(i).getStartCount()).isZero();
70+
}
71+
TestcontainersStartup.SEQUENTIAL.start(startables);
72+
for (int i = 0; i < startables.size(); i++) {
73+
assertThat(startables.get(i).getStartCount()).isOne();
74+
}
75+
TestcontainersStartup.SEQUENTIAL.start(startables);
76+
for (int i = 0; i < startables.size(); i++) {
77+
assertThat(startables.get(i).getStartCount()).isOne();
78+
}
79+
}
80+
5281
@Test
5382
void startWhenParallelStartsInParallel() {
5483
List<TestStartable> startables = createTestStartables(100);
5584
TestcontainersStartup.PARALLEL.start(startables);
5685
assertThat(startables.stream().map(TestStartable::getThreadName)).hasSizeGreaterThan(1);
5786
}
5887

88+
@Test
89+
void startWhenParallelStartsOnlyOnce() {
90+
List<TestStartable> startables = createTestStartables(10);
91+
for (int i = 0; i < startables.size(); i++) {
92+
assertThat(startables.get(i).getStartCount()).isZero();
93+
}
94+
TestcontainersStartup.PARALLEL.start(startables);
95+
for (int i = 0; i < startables.size(); i++) {
96+
assertThat(startables.get(i).getStartCount()).isOne();
97+
}
98+
TestcontainersStartup.PARALLEL.start(startables);
99+
for (int i = 0; i < startables.size(); i++) {
100+
assertThat(startables.get(i).getStartCount()).isOne();
101+
}
102+
}
103+
104+
@Test
105+
void startWhenParallelStartsDependenciesOnlyOnce() {
106+
List<TestStartable> dependencies = createTestStartables(10);
107+
TestStartable first = new TestStartable(dependencies);
108+
TestStartable second = new TestStartable(dependencies);
109+
List<TestStartable> startables = List.of(first, second);
110+
assertThat(first.getStartCount()).isZero();
111+
assertThat(second.getStartCount()).isZero();
112+
for (int i = 0; i < startables.size(); i++) {
113+
assertThat(dependencies.get(i).getStartCount()).isZero();
114+
}
115+
TestcontainersStartup.PARALLEL.start(startables);
116+
assertThat(first.getStartCount()).isOne();
117+
assertThat(second.getStartCount()).isOne();
118+
for (int i = 0; i < startables.size(); i++) {
119+
assertThat(dependencies.get(i).getStartCount()).isOne();
120+
}
121+
TestcontainersStartup.PARALLEL.start(startables);
122+
assertThat(first.getStartCount()).isOne();
123+
assertThat(second.getStartCount()).isOne();
124+
for (int i = 0; i < startables.size(); i++) {
125+
assertThat(dependencies.get(i).getStartCount()).isOne();
126+
}
127+
}
128+
59129
@Test
60130
void getWhenNoPropertyReturnsDefault() {
61131
MockEnvironment environment = new MockEnvironment();
@@ -93,20 +163,43 @@ private List<TestStartable> createTestStartables(int size) {
93163
return testStartables;
94164
}
95165

96-
private final class TestStartable implements Startable {
166+
private class TestStartable extends GenericContainer<TestStartable> {
167+
168+
private int startCount;
97169

98170
private int index;
99171

100172
private String threadName;
101173

174+
TestStartable() {
175+
super("test");
176+
}
177+
178+
TestStartable(Collection<TestStartable> startables) {
179+
super("test");
180+
this.dependencies.addAll(startables);
181+
}
182+
183+
@Override
184+
public Set<Startable> getDependencies() {
185+
return this.dependencies;
186+
}
187+
102188
@Override
103189
public void start() {
190+
this.startCount++;
104191
this.index = TestcontainersStartupTests.this.counter.getAndIncrement();
105192
this.threadName = Thread.currentThread().getName();
106193
}
107194

108195
@Override
109196
public void stop() {
197+
this.startCount--;
198+
}
199+
200+
@Override
201+
public boolean isRunning() {
202+
return this.startCount > 0;
110203
}
111204

112205
int getIndex() {
@@ -117,6 +210,10 @@ String getThreadName() {
117210
return this.threadName;
118211
}
119212

213+
int getStartCount() {
214+
return this.startCount;
215+
}
216+
120217
}
121218

122219
}

0 commit comments

Comments
 (0)