Skip to content

Commit a5f541c

Browse files
sanagaraj-pivotalsanagaraj-pivotal
authored
Don't add dependency twice
closes #309
1 parent 4155124 commit a5f541c

File tree

2 files changed

+167
-86
lines changed

2 files changed

+167
-86
lines changed

‎components/sbm-core/src/main/java/org/springframework/sbm/build/impl/OpenRewriteMavenBuildFile.java

Lines changed: 25 additions & 86 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,7 @@
5858
import java.util.HashSet;
5959
import java.util.Iterator;
6060
import java.util.List;
61+
import java.util.Map;
6162
import java.util.Optional;
6263
import java.util.Set;
6364
import java.util.stream.Collectors;
@@ -105,7 +106,6 @@ protected List<SourceFile> visit(List<SourceFile> before, ExecutionContext ctx)
105106
Optional<MavenResolutionResult> mavenModels = MavenBuildFileUtil.findMavenResolution(mavenFiles.get(i));
106107
Optional<MavenResolutionResult> newMavenModels = MavenBuildFileUtil.findMavenResolution(newMavenFiles.get(i));
107108
mavenFiles.get(i).withMarkers(Markers.build(Arrays.asList(newMavenModels.get())));
108-
// Xml.Document m = mavenFiles.get(i).getMarkers().withModel(newMavenFiles.get(i).getModel());
109109
// FIXME: 497 verify correctness
110110
mavenFiles.set(i, newMavenFiles.get(i));
111111
}
@@ -157,8 +157,27 @@ public MavenResolutionResult getPom() {
157157

158158
@Override
159159
public void addDependency(Dependency dependency) {
160-
addDependencyInner(dependency);
161-
eventPublisher.publishEvent(new DependenciesChangedEvent(getResolvedDependenciesPaths()));
160+
if (!containsDependency(dependency)) {
161+
addDependencyInner(dependency);
162+
eventPublisher.publishEvent(new DependenciesChangedEvent(getResolvedDependenciesPaths()));
163+
}
164+
}
165+
166+
private boolean containsDependency(Dependency dependency) {
167+
List<ResolvedDependency> listToSearch;
168+
169+
Map<Scope, List<ResolvedDependency>> projectDependencies = getPom().getDependencies();
170+
171+
listToSearch = dependency.getScope() == null ? projectDependencies.get(Scope.Compile) :
172+
projectDependencies.get(Scope.fromName(dependency.getScope()));
173+
174+
return listToSearch
175+
.stream()
176+
.anyMatch(
177+
d -> d.getArtifactId().equals(dependency.getArtifactId())
178+
&& d.getGroupId().equals(dependency.getGroupId())
179+
&& d.getVersion().equals(dependency.getVersion())
180+
);
162181
}
163182

164183
@Override
@@ -218,21 +237,20 @@ public List<Dependency> getDeclaredDependencies(Scope... scopes) {
218237
// returns dependencies as declared in xml
219238
List<org.openrewrite.maven.tree.Dependency> requestedDependencies = getPom().getPom().getRequestedDependencies();
220239
// FIXME: #7 use getPom().getDependencies() instead ?
221-
List<Dependency> declaredDependenciesWithEffectiveVersions = requestedDependencies.stream()
240+
return requestedDependencies.stream()
222241
.filter(d -> {
223242
if(scopes.length == 0) {
224243
return true;
225244
} else {
226245
// FIXME: scope test should also return compile!
227-
return Arrays.asList(scopes).stream().anyMatch(scope -> {
246+
return Arrays.stream(scopes).anyMatch(scope -> {
228247
String effectiveScope = d.getScope() == null ? "compile" : d.getScope();
229248
return scope.toString().equalsIgnoreCase(effectiveScope);
230249
});
231250
}
232251
})
233-
.map(d -> mapDependency(d))
252+
.map(this::mapDependency)
234253
.collect(Collectors.toList());
235-
return declaredDependenciesWithEffectiveVersions;
236254
}
237255

238256
/**
@@ -371,7 +389,6 @@ private String scopeString(Scope scope) {
371389
}
372390

373391
protected void addDependenciesInner(List<Dependency> dependencies) {
374-
// dependencies = dependencies.stream().filter(d -> hasExactDeclaredDependency(d)).collect(Collectors.toList());
375392
if (!dependencies.isEmpty()) {
376393
Recipe r = getAddDependencyRecipe(dependencies.get(0));
377394
dependencies.stream().skip(1).forEach(d -> r.doNext(getAddDependencyRecipe(d)));
@@ -385,35 +402,9 @@ protected void addDependenciesInner(List<Dependency> dependencies) {
385402
excludeDependenciesInner(exclusions);
386403

387404
updateClasspathRegistry();
388-
389-
// javaParser.
390-
391-
/*
392-
Field classpathField = ReflectionUtils.findField(Java11Parser.class, "classpath");
393-
ReflectionUtils.makeAccessible(classpathField);
394-
Object field1 = ReflectionUtils.getField(classpathField, ((RewriteJavaParser)javaParser).getJavaParser());
395-
Collection<Path> field = (Collection<Path>) field1;
396-
if(field1 == null) {
397-
398-
}
399-
400-
field.addAll(ClasspathRegistry.getInstance().getCurrentDependencies());
401-
javaParser.setClasspath(field);
402-
403-
// TODO: #7 update classpath for JavaParser, publish event that classpath changed which triggers a recompile
404-
405-
timeExceeded = System.currentTimeMillis() - before;
406-
System.out.println("Took " + (timeExceeded/1000) + " sec.");
407-
408-
*/
409405
}
410406
}
411407

412-
private boolean hasEffectiveDependency(Dependency d) {
413-
return getEffectiveDependencies().stream()
414-
.anyMatch(dep -> d.getCoordinates().equals(dep.getCoordinates()));
415-
}
416-
417408
/**
418409
* Does not updateClasspathRegistry
419410
*/
@@ -524,56 +515,6 @@ public List<Path> getResolvedDependenciesPaths() {
524515
return getPom().getDependencies().get(Scope.Provided).stream()
525516
.map(rewriteMavenArtifactDownloader::downloadArtifact)
526517
.collect(Collectors.toList());
527-
528-
/*
529-
Field classpathField = ReflectionUtils.findField(Java11Parser.class, "classpath");
530-
ReflectionUtils.makeAccessible(classpathField);
531-
Object field1 = ReflectionUtils.getField(classpathField, ((RewriteJavaParser)javaParser).getJavaParser());
532-
Collection<Path> field = (Collection<Path>) field1;
533-
return new ArrayList<>(field);
534-
535-
*/
536-
537-
// return new ArrayList<>(ClasspathRegistry.getInstance().getCurrentDependencies());
538-
//
539-
// try {
540-
// MavenResolvedArtifact[] artifacts = new MavenResolvedArtifact[0];
541-
// File file = getAbsolutePath().toFile();
542-
// if (file.exists()) {
543-
// MavenWorkingSession mavenWorkingSession = new MavenWorkingSessionImpl().loadPomFromFile(file);
544-
// if (!mavenWorkingSession.getDeclaredDependencies().isEmpty()) {
545-
// artifacts = org.jboss.shrinkwrap.resolver.api.maven.Maven.resolver().loadPomFromFile(file)
546-
// .importDependencies(ScopeType.values())
547-
// .resolve().withTransitivity().asResolvedArtifact();
548-
// }
549-
// } else {
550-
// List<MavenDependency> mavenDependencies = getDeclaredDependencies().stream()
551-
// .map(d -> MavenDependencies.createDependency(
552-
// MavenCoordinates.createCoordinate(
553-
// d.getGroupId(), d.getArtifactId(), d.getVersion(),
554-
// d.getType() == null ? PackagingType.JAR : PackagingType.of(d.getType()),
555-
// d.getClassifier()),
556-
// ScopeType.fromScopeType(d.getScope()),
557-
// false))
558-
// .collect(Collectors.toList());
559-
//
560-
// if (mavenDependencies.isEmpty()) {
561-
// return Collections.emptyList();
562-
// }
563-
//
564-
// artifacts = org.jboss.shrinkwrap.resolver.api.maven.Maven.resolver()
565-
// .addDependencies(mavenDependencies)
566-
// .resolve().withTransitivity().asResolvedArtifact();
567-
// }
568-
//
569-
// return Arrays.stream(artifacts)
570-
// .map(a -> a.asFile().toPath())
571-
// .collect(Collectors.toList());
572-
// } catch (
573-
// Exception e) {
574-
// throw new RuntimeException(e);
575-
// }
576-
577518
}
578519

579520
@Override
@@ -697,8 +638,6 @@ public void upgradeParentVersion(String version) {
697638
apply(
698639
new UpgradeParentVersion(parent.getGroupId(), parent.getArtifactId(), version, null)
699640
);
700-
// List<Xml.Document> parse = MavenParser.builder().build().parseInputs(List.of(new Parser.Input(getAbsolutePath(), () -> new ByteArrayInputStream(print().getBytes(StandardCharsets.UTF_8)))), getAbsoluteProjectDir(), executionContext);
701-
// replaceWith(parse.get(0));
702641
}
703642
}
704643

‎components/sbm-core/src/test/java/org/springframework/sbm/project/buildfile/OpenRewriteMavenBuildFileTest.java

Lines changed: 142 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717

1818
import org.intellij.lang.annotations.Language;
1919
import org.junit.jupiter.api.Disabled;
20+
import org.junit.jupiter.api.Nested;
2021
import org.junit.jupiter.api.Tag;
2122
import org.junit.jupiter.api.Test;
2223
import org.mockito.ArgumentCaptor;
@@ -88,6 +89,147 @@ void coordinatesForRequestedDependencies_withVersionProperty_shouldHaveResolvedV
8889
assertThat(buildFile.getRequestedDependencies().get(0).getCoordinates()).isEqualTo("javax.validation:validation-api:2.0.1.Final");
8990
}
9091

92+
@Nested
93+
class HandlingDuplicatedDependencyTest {
94+
95+
@Test
96+
void shouldNotAddDependencyWhenAlreadyExists() {
97+
@Language("xml")
98+
String applicationPom = """
99+
<?xml version="1.0" encoding="UTF-8"?>
100+
<project xmlns="http://maven.apache.org/POM/4.0.0"
101+
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
102+
xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
103+
<modelVersion>4.0.0</modelVersion>
104+
<groupId>org.example</groupId>
105+
<artifactId>some-module</artifactId>
106+
<version>1.0-SNAPSHOT</version>
107+
<properties>
108+
<maven.compiler.source>17</maven.compiler.source>
109+
<maven.compiler.target>17</maven.compiler.target>
110+
</properties>
111+
<dependencies>
112+
<dependency>
113+
<groupId>javax.validation</groupId>
114+
<artifactId>validation-api</artifactId>
115+
<version>2.0.1.Final</version>
116+
</dependency>
117+
</dependencies>
118+
</project>
119+
""";
120+
121+
BuildFile buildFile = TestProjectContext
122+
.buildProjectContext()
123+
.withMavenRootBuildFileSource(applicationPom)
124+
.build()
125+
.getApplicationModules()
126+
.list()
127+
.get(0)
128+
.getBuildFile();
129+
130+
buildFile.addDependency(Dependency.fromCoordinates("javax.validation:validation-api:2.0.1.Final"));
131+
buildFile.addDependency(Dependency.fromCoordinates("javax.validation:validation-api:2.0.1.Final"));
132+
assertThat(buildFile.getDeclaredDependencies()).hasSize(1);
133+
}
134+
135+
@Test
136+
void shouldDuplicateDependencyWithDifferentScope() {
137+
@Language("xml")
138+
String applicationPom = """
139+
<?xml version="1.0" encoding="UTF-8"?>
140+
<project xmlns="http://maven.apache.org/POM/4.0.0"
141+
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
142+
xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
143+
<modelVersion>4.0.0</modelVersion>
144+
<groupId>org.example</groupId>
145+
<artifactId>some-module</artifactId>
146+
<version>1.0-SNAPSHOT</version>
147+
<properties>
148+
<maven.compiler.source>17</maven.compiler.source>
149+
<maven.compiler.target>17</maven.compiler.target>
150+
</properties>
151+
<dependencies>
152+
<dependency>
153+
<groupId>javax.validation</groupId>
154+
<artifactId>validation-api</artifactId>
155+
<version>2.0.1.Final</version>
156+
<scope>test</scope>
157+
</dependency>
158+
</dependencies>
159+
</project>
160+
""";
161+
162+
BuildFile buildFile = TestProjectContext
163+
.buildProjectContext()
164+
.withMavenRootBuildFileSource(applicationPom)
165+
.build()
166+
.getApplicationModules()
167+
.list()
168+
.get(0)
169+
.getBuildFile();
170+
171+
buildFile.addDependency(Dependency.fromCoordinates("javax.validation:validation-api:2.0.1.Final"));
172+
assertThat(buildFile.getDeclaredDependencies()).hasSize(2);
173+
}
174+
175+
@Test
176+
void shouldNotDuplicateDependencyInManagedDependencySetting() {
177+
178+
@Language("xml")
179+
String applicationPom = """
180+
<?xml version="1.0" encoding="UTF-8"?>
181+
<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
182+
xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 https://maven.apache.org/xsd/maven-4.0.0.xsd">
183+
<modelVersion>4.0.0</modelVersion>
184+
<parent>
185+
<groupId>org.springframework.boot</groupId>
186+
<artifactId>spring-boot-starter-parent</artifactId>
187+
<version>2.7.4</version>
188+
<relativePath/> <!-- lookup parent from repository -->
189+
</parent>
190+
<groupId>com.example</groupId>
191+
<artifactId>demo</artifactId>
192+
<version>0.0.1-SNAPSHOT</version>
193+
<name>demo</name>
194+
<description>Demo project for Spring Boot</description>
195+
<properties>
196+
<java.version>17</java.version>
197+
</properties>
198+
<dependencies>
199+
<dependency>
200+
<groupId>org.springframework.boot</groupId>
201+
<artifactId>spring-boot-starter</artifactId>
202+
</dependency>
203+
<dependency>
204+
<groupId>org.springframework.boot</groupId>
205+
<artifactId>spring-boot-starter-test</artifactId>
206+
<scope>test</scope>
207+
</dependency>
208+
</dependencies>
209+
210+
</project>
211+
""";
212+
213+
BuildFile buildFile = TestProjectContext
214+
.buildProjectContext()
215+
.withMavenRootBuildFileSource(applicationPom)
216+
.build()
217+
.getApplicationModules()
218+
.list()
219+
.get(0)
220+
.getBuildFile();
221+
222+
buildFile.addDependency(Dependency.fromCoordinates("org.springframework.boot:spring-boot-starter:2.7.4"));
223+
Dependency springBootStarterTest = Dependency
224+
.fromCoordinates("org.springframework.boot:spring-boot-starter-test:2.7.4");
225+
springBootStarterTest.setScope("test");
226+
buildFile.addDependency(springBootStarterTest);
227+
228+
assertThat(buildFile.getDeclaredDependencies()).hasSize(2);
229+
}
230+
}
231+
232+
91233
@Test
92234
@Tag("integration")
93235
void testGetResolvedDependenciesPaths() {

0 commit comments

Comments
 (0)