Skip to content

Commit 4affd18

Browse files
andrzejj0andrzejj0
authored
Resolves #1270: Ensuring thread safety when VersionsHelper (#1273)
1 parent 6bd8d68 commit 4affd18

6 files changed

Lines changed: 326 additions & 4 deletions

File tree

‎versions-enforcer/src/main/java/org/codehaus/mojo/versions/enforcer/MaxDependencyUpdates.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -297,7 +297,7 @@ public MaxDependencyUpdates(
297297
* Creates the VersionsHelper object
298298
* @return VersionsHelper object
299299
*/
300-
private VersionsHelper createVersionsHelper(String serverId, String rulesUri, RuleSet ruleSet)
300+
private synchronized VersionsHelper createVersionsHelper(String serverId, String rulesUri, RuleSet ruleSet)
301301
throws EnforcerRuleError {
302302
try {
303303
Log log = new PluginLogWrapper(getLog());

‎versions-maven-plugin/src/main/java/org/codehaus/mojo/versions/AbstractVersionsReport.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -167,7 +167,7 @@ protected AbstractVersionsReport(
167167
this.rendererFactory = rendererFactory;
168168
}
169169

170-
public VersionsHelper getHelper() throws MavenReportException {
170+
public synchronized VersionsHelper getHelper() throws MavenReportException {
171171
if (helper == null) {
172172
try {
173173
RuleService ruleService = new RulesServiceBuilder()

‎versions-maven-plugin/src/main/java/org/codehaus/mojo/versions/AbstractVersionsUpdaterMojo.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -207,7 +207,7 @@ protected AbstractVersionsUpdaterMojo(
207207

208208
protected abstract boolean getAllowSnapshots();
209209

210-
public VersionsHelper getHelper() throws MojoExecutionException {
210+
public synchronized VersionsHelper getHelper() throws MojoExecutionException {
211211
if (helper == null) {
212212
RuleService ruleService = new RulesServiceBuilder()
213213
.withMavenSession(session)

‎versions-maven-plugin/src/test/java/org/codehaus/mojo/versions/TestWagonStub.java‎

Lines changed: 134 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,134 @@
1+
package org.codehaus.mojo.versions;
2+
3+
import java.io.File;
4+
import java.util.Collections;
5+
import java.util.List;
6+
import java.util.function.Consumer;
7+
8+
import org.apache.maven.wagon.ResourceDoesNotExistException;
9+
import org.apache.maven.wagon.TransferFailedException;
10+
import org.apache.maven.wagon.Wagon;
11+
import org.apache.maven.wagon.authentication.AuthenticationInfo;
12+
import org.apache.maven.wagon.authorization.AuthorizationException;
13+
import org.apache.maven.wagon.events.SessionListener;
14+
import org.apache.maven.wagon.events.TransferListener;
15+
import org.apache.maven.wagon.proxy.ProxyInfo;
16+
import org.apache.maven.wagon.proxy.ProxyInfoProvider;
17+
import org.apache.maven.wagon.repository.Repository;
18+
19+
class TestWagonStub implements Wagon {
20+
private final Consumer<File> getter;
21+
22+
TestWagonStub(Consumer<File> getter) {
23+
this.getter = getter;
24+
}
25+
26+
@Override
27+
public void get(String resourceName, File destination)
28+
throws TransferFailedException, ResourceDoesNotExistException, AuthorizationException {
29+
getter.accept(destination);
30+
}
31+
32+
@Override
33+
public boolean getIfNewer(String resourceName, File destination, long timestamp) {
34+
return false;
35+
}
36+
37+
@Override
38+
public void put(File source, String destination)
39+
throws TransferFailedException, ResourceDoesNotExistException, AuthorizationException {}
40+
41+
@Override
42+
public void putDirectory(File sourceDirectory, String destinationDirectory) {}
43+
44+
@Override
45+
public boolean resourceExists(String resourceName) {
46+
return false;
47+
}
48+
49+
@Override
50+
public List<String> getFileList(String destinationDirectory) {
51+
return Collections.emptyList();
52+
}
53+
54+
@Override
55+
public boolean supportsDirectoryCopy() {
56+
return false;
57+
}
58+
59+
@Override
60+
public Repository getRepository() {
61+
return null;
62+
}
63+
64+
@Override
65+
public void connect(Repository source) {}
66+
67+
@Override
68+
public void connect(Repository source, ProxyInfo proxyInfo) {}
69+
70+
@Override
71+
public void connect(Repository source, ProxyInfoProvider proxyInfoProvider) {}
72+
73+
@Override
74+
public void connect(Repository source, AuthenticationInfo authenticationInfo) {}
75+
76+
@Override
77+
public void connect(Repository source, AuthenticationInfo authenticationInfo, ProxyInfo proxyInfo) {}
78+
79+
@Override
80+
public void connect(
81+
Repository source, AuthenticationInfo authenticationInfo, ProxyInfoProvider proxyInfoProvider) {}
82+
83+
@Override
84+
public void openConnection() {}
85+
86+
@Override
87+
public void disconnect() {}
88+
89+
@Override
90+
public void setTimeout(int timeoutValue) {}
91+
92+
@Override
93+
public int getTimeout() {
94+
return 0;
95+
}
96+
97+
@Override
98+
public void setReadTimeout(int timeoutValue) {}
99+
100+
@Override
101+
public int getReadTimeout() {
102+
return 0;
103+
}
104+
105+
@Override
106+
public void addSessionListener(SessionListener listener) {}
107+
108+
@Override
109+
public void removeSessionListener(SessionListener listener) {}
110+
111+
@Override
112+
public boolean hasSessionListener(SessionListener listener) {
113+
return false;
114+
}
115+
116+
@Override
117+
public void addTransferListener(TransferListener listener) {}
118+
119+
@Override
120+
public void removeTransferListener(TransferListener listener) {}
121+
122+
@Override
123+
public boolean hasTransferListener(TransferListener listener) {
124+
return false;
125+
}
126+
127+
@Override
128+
public boolean isInteractive() {
129+
return false;
130+
}
131+
132+
@Override
133+
public void setInteractive(boolean interactive) {}
134+
}

‎versions-maven-plugin/src/test/java/org/codehaus/mojo/versions/UseLatestVersionsRaceConditionTest.java‎

Lines changed: 178 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,178 @@
1+
package org.codehaus.mojo.versions;
2+
3+
import java.nio.charset.StandardCharsets;
4+
import java.nio.file.Files;
5+
import java.util.Arrays;
6+
import java.util.Collections;
7+
import java.util.concurrent.atomic.AtomicBoolean;
8+
9+
import org.apache.maven.artifact.handler.manager.ArtifactHandlerManager;
10+
import org.apache.maven.model.Model;
11+
import org.apache.maven.plugin.MojoExecution;
12+
import org.apache.maven.plugin.MojoExecutionException;
13+
import org.apache.maven.plugin.logging.Log;
14+
import org.apache.maven.project.MavenProject;
15+
import org.apache.maven.wagon.Wagon;
16+
import org.codehaus.mojo.versions.api.PomHelper;
17+
import org.codehaus.mojo.versions.change.DefaultDependencyVersionChange;
18+
import org.codehaus.mojo.versions.utils.ArtifactFactory;
19+
import org.codehaus.mojo.versions.utils.DependencyBuilder;
20+
import org.codehaus.mojo.versions.utils.MockUtils;
21+
import org.codehaus.mojo.versions.utils.TestChangeRecorder;
22+
import org.codehaus.plexus.component.configurator.expression.ExpressionEvaluator;
23+
import org.junit.Before;
24+
import org.junit.Test;
25+
import org.mockito.Mock;
26+
import org.mockito.MockedStatic;
27+
import org.mockito.Mockito;
28+
29+
import static java.util.Collections.emptyList;
30+
import static org.apache.maven.artifact.Artifact.SCOPE_COMPILE;
31+
import static org.apache.maven.plugin.testing.ArtifactStubFactory.setVariableValueToObject;
32+
import static org.codehaus.mojo.versions.utils.MockUtils.mockArtifactHandlerManager;
33+
import static org.codehaus.mojo.versions.utils.MockUtils.mockMavenSession;
34+
import static org.hamcrest.MatcherAssert.assertThat;
35+
import static org.hamcrest.Matchers.allOf;
36+
import static org.hamcrest.Matchers.hasItem;
37+
import static org.hamcrest.Matchers.is;
38+
import static org.mockito.ArgumentMatchers.any;
39+
import static org.mockito.ArgumentMatchers.anyString;
40+
import static org.mockito.Mockito.mockStatic;
41+
import static org.mockito.MockitoAnnotations.openMocks;
42+
43+
public class UseLatestVersionsRaceConditionTest {
44+
protected UseLatestVersionsMojoBase mojo;
45+
46+
protected TestChangeRecorder changeRecorder;
47+
48+
@Mock
49+
protected Log log;
50+
51+
protected PomHelper pomHelper;
52+
53+
protected ArtifactFactory artifactFactory;
54+
55+
@Mock
56+
protected ExpressionEvaluator expressionEvaluator;
57+
58+
@Before
59+
public void setUp() throws Exception {
60+
openMocks(this);
61+
changeRecorder = new TestChangeRecorder();
62+
ArtifactHandlerManager artifactHandlerManager = mockArtifactHandlerManager();
63+
artifactFactory = new ArtifactFactory(artifactHandlerManager);
64+
pomHelper = new PomHelper(artifactFactory, expressionEvaluator);
65+
mojo = createMojo();
66+
mojo.mojoExecution = Mockito.mock(MojoExecution.class);
67+
}
68+
69+
private static String getRulesString() {
70+
return "<ruleset>\n"
71+
+ " <ignoreVersions>\n"
72+
+ " <ignoreVersion type=\"regex\">.*-alpha</ignoreVersion>\n"
73+
+ " <ignoreVersion type=\"regex\">.*-beta</ignoreVersion>\n"
74+
+ " <ignoreVersion type=\"illegalType\">illegalVersion</ignoreVersion>\n"
75+
+ " </ignoreVersions>\n"
76+
+ " <rules>\n"
77+
+ " <rule groupId=\"com.mycompany.maven\">\n"
78+
+ " <ignoreVersions>\n"
79+
+ " <ignoreVersion>one</ignoreVersion>\n"
80+
+ " <ignoreVersion>two</ignoreVersion>\n"
81+
+ " <ignoreVersion>1.2.0</ignoreVersion>\n"
82+
+ " <ignoreVersion type=\"illegalType\">illegalVersion</ignoreVersion>\n"
83+
+ " </ignoreVersions>\n"
84+
+ " </rule>\n"
85+
+ " </rules>\n"
86+
+ "</ruleset>";
87+
}
88+
89+
protected UseLatestVersionsMojoBase createMojo() throws IllegalAccessException, MojoExecutionException {
90+
AtomicBoolean inUse = new AtomicBoolean(false);
91+
Wagon testWagon = new TestWagonStub(file -> {
92+
try {
93+
assertThat("Resource is in use", inUse.compareAndSet(false, true), is(true));
94+
Files.write(file.toPath(), getRulesString().getBytes(StandardCharsets.UTF_8));
95+
Thread.sleep(200);
96+
} catch (Exception e) {
97+
e.printStackTrace();
98+
} finally {
99+
inUse.set(false);
100+
}
101+
});
102+
return new UseLatestVersionsMojo(
103+
artifactFactory,
104+
MockUtils.mockAetherRepositorySystem(),
105+
Collections.singletonMap("proto", testWagon),
106+
changeRecorder.asTestMap()) {
107+
{
108+
reactorProjects = emptyList();
109+
MavenProject project = new MavenProject() {
110+
{
111+
setModel(new Model() {
112+
{
113+
setGroupId("default-group");
114+
setArtifactId("project-artifact");
115+
setVersion("1.0.0-SNAPSHOT");
116+
117+
setDependencies(Arrays.asList(
118+
DependencyBuilder.newBuilder()
119+
.withGroupId("default-group")
120+
.withArtifactId("artifactA")
121+
.withVersion("1.0.0")
122+
.withType("pom")
123+
.withClassifier("default")
124+
.withScope(SCOPE_COMPILE)
125+
.build(),
126+
DependencyBuilder.newBuilder()
127+
.withGroupId("default-group")
128+
.withArtifactId("artifactB")
129+
.withVersion("1.0.0")
130+
.withType("pom")
131+
.withClassifier("default")
132+
.withScope(SCOPE_COMPILE)
133+
.build(),
134+
DependencyBuilder.newBuilder()
135+
.withGroupId("default-group")
136+
.withArtifactId("artifactC")
137+
.withVersion("1.0.0")
138+
.withType("pom")
139+
.withClassifier("default")
140+
.withScope(SCOPE_COMPILE)
141+
.build()));
142+
}
143+
});
144+
}
145+
};
146+
setProject(project);
147+
session = mockMavenSession();
148+
setVariableValueToObject(this, "rulesUri", "proto://localhost/rules.xml");
149+
}
150+
};
151+
}
152+
153+
@Test
154+
public void testConcurrency() throws Exception {
155+
try (MockedStatic<PomHelper> pomHelper = mockStatic(PomHelper.class)) {
156+
pomHelper
157+
.when(() -> PomHelper.setDependencyVersion(any(), any(), any(), any(), any(), any(), any()))
158+
.thenReturn(true);
159+
pomHelper
160+
.when(() -> PomHelper.getRawModel(any(MavenProject.class)))
161+
.thenReturn(mojo.getProject().getModel());
162+
pomHelper
163+
.when(() -> PomHelper.setProjectParentVersion(any(), anyString()))
164+
.thenReturn(true);
165+
mojo.update(null);
166+
}
167+
168+
assertThat(
169+
changeRecorder.getChanges(),
170+
allOf(
171+
hasItem(new DefaultDependencyVersionChange(
172+
"default-group", "artifactA",
173+
"1.0.0", "2.0.0")),
174+
hasItem(new DefaultDependencyVersionChange(
175+
"default-group", "artifactB",
176+
"1.0.0", "1.1.0"))));
177+
}
178+
}

‎versions-test/src/main/java/org/codehaus/mojo/versions/utils/MockUtils.java‎

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,9 @@
3232
import org.codehaus.plexus.i18n.I18N;
3333
import org.eclipse.aether.RepositorySystem;
3434
import org.eclipse.aether.RepositorySystemSession;
35+
import org.eclipse.aether.repository.AuthenticationSelector;
36+
import org.eclipse.aether.repository.MirrorSelector;
37+
import org.eclipse.aether.repository.ProxySelector;
3538
import org.eclipse.aether.resolution.ArtifactRequest;
3639
import org.eclipse.aether.resolution.ArtifactResolutionException;
3740
import org.eclipse.aether.resolution.ArtifactResult;
@@ -183,11 +186,18 @@ public static MavenSession mockMavenSession() {
183186
*/
184187
public static MavenSession mockMavenSession(MavenProject project) {
185188
MavenSession session = mock(MavenSession.class);
186-
when(session.getRepositorySession()).thenReturn(mock(RepositorySystemSession.class));
189+
RepositorySystemSession repositorySystemSession = mock(RepositorySystemSession.class);
190+
when(session.getRepositorySession()).thenReturn(repositorySystemSession);
187191
when(session.getCurrentProject()).thenReturn(project);
188192
Properties emptyProperties = new Properties();
189193
when(session.getUserProperties()).thenReturn(emptyProperties);
190194
when(session.getSystemProperties()).thenReturn(emptyProperties);
195+
ProxySelector proxySelector = mock(ProxySelector.class);
196+
when(repositorySystemSession.getProxySelector()).thenReturn(proxySelector);
197+
AuthenticationSelector authenticationSelector = mock(AuthenticationSelector.class);
198+
when(repositorySystemSession.getAuthenticationSelector()).thenReturn(authenticationSelector);
199+
MirrorSelector mirrorSelector = mock(MirrorSelector.class);
200+
when(repositorySystemSession.getMirrorSelector()).thenReturn(mirrorSelector);
191201
return session;
192202
}
193203
}

0 commit comments

Comments
 (0)