Skip to content

Commit e279f35

Browse files
nahsranahsra
committed
add tests and more stable behavior
1 parent e261a7f commit e279f35

File tree

6 files changed

+153
-47
lines changed

6 files changed

+153
-47
lines changed

‎framework/codemodder-base/src/main/java/io/codemodder/CodemodPackageUpdateResult.java‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,13 +6,16 @@
66
import java.util.List;
77
import java.util.Set;
88

9-
/** A */
9+
/** A model of a codemod's updating of packages. */
1010
public interface CodemodPackageUpdateResult {
1111

12+
/** A structured description of what we were able to do. */
1213
List<CodeTFPackageAction> packageActions();
1314

15+
/** The changes that were made to the manifest file. */
1416
List<CodeTFChangesetEntry> manifestChanges();
1517

18+
/** The set of files that we attempted to update, but failed. */
1619
Set<Path> filesFailedToChange();
1720

1821
static CodemodPackageUpdateResult from(

‎plugins/codemodder-plugin-maven/src/main/java/io/codemodder/plugins/maven/DefaultPOMDependencyUpdater.java‎

Lines changed: 26 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@
2020
import org.slf4j.LoggerFactory;
2121

2222
/** POMDependencyUpdater is responsible for updating Maven POM files with new dependencies. */
23-
class DefaultPOMDependencyUpdater implements POMDependencyUpdater {
23+
final class DefaultPOMDependencyUpdater implements POMDependencyUpdater {
2424
private final PomFileFinder pomFileFinder;
2525

2626
private Optional<Path> maybePomFile;
@@ -63,17 +63,19 @@ class DefaultPOMDependencyUpdater implements POMDependencyUpdater {
6363
* @param file The specific POM file to update.
6464
* @param dependencies The list of new dependencies to be added.
6565
* @return A DependencyUpdateResult containing information about the update process.
66-
* @throws IOException If an I/O error occurs.
67-
* @throws XMLStreamException If an error occurs during XML stream processing.
68-
* @throws DocumentException If an error occurs while parsing the document.
69-
* @throws URISyntaxException If there is an issue with the URI syntax.
7066
*/
7167
@NotNull
7268
public DependencyUpdateResult execute(
73-
final Path projectDir, final Path file, final List<DependencyGAV> dependencies)
74-
throws IOException, XMLStreamException, DocumentException, URISyntaxException {
75-
if (isEmptyPomFile(projectDir, file)) {
76-
LOG.trace("Pom file was empty for {}", file);
69+
final Path projectDir, final Path file, final List<DependencyGAV> dependencies) {
70+
71+
// if we can't find a pom, we want to skip this file
72+
try {
73+
if (isEmptyPomFile(projectDir, file)) {
74+
LOG.trace("Pom file was empty for {}", file);
75+
return DependencyUpdateResult.EMPTY_UPDATE;
76+
}
77+
} catch (IOException e) {
78+
LOG.warn("Not all Maven dependencies could be found", e);
7779
return DependencyUpdateResult.EMPTY_UPDATE;
7880
}
7981

@@ -84,7 +86,17 @@ public DependencyUpdateResult execute(
8486
skippedDependencies = new ArrayList<>();
8587
injectedDependencies = new ArrayList<>();
8688
erroredFiles = new LinkedHashSet<>();
87-
foundDependenciesMapped = new AtomicReference<>(pomOperator.getAllFoundDependencies());
89+
90+
// get all the existing dependencies, if we're not able to find them, we can't update
91+
Collection<DependencyGAV> allFoundDependencies;
92+
try {
93+
allFoundDependencies = pomOperator.getAllFoundDependencies();
94+
} catch (IOException | DocumentException | XMLStreamException | URISyntaxException e) {
95+
LOG.warn("Problem calculating all dependencies", e);
96+
return DependencyUpdateResult.EMPTY_UPDATE;
97+
}
98+
99+
foundDependenciesMapped = new AtomicReference<>(allFoundDependencies);
88100
LOG.trace("Beginning dependency set size: {}", foundDependenciesMapped.get().size());
89101

90102
dependencies.forEach(
@@ -97,7 +109,6 @@ public DependencyUpdateResult execute(
97109
}
98110

99111
final ProjectModel modifiedProjectModel = pomOperator.addDependency(newDependencyGAV);
100-
101112
if (modifiedProjectModel == null) {
102113
LOG.trace("POM file didn't need modification or it failed?");
103114
return;
@@ -107,16 +118,14 @@ public DependencyUpdateResult execute(
107118

108119
modifyPomFiles(projectDir, modifiedProjectModel, newDependencyGAV);
109120

121+
// recalculate the dependencies after our addition
110122
final Collection<DependencyGAV> newDependencySet =
111123
pomOperator.getAllFoundDependencies();
112-
113124
LOG.trace("New dependency set size: {}", newDependencySet.size());
114-
115125
foundDependenciesMapped.set(newDependencySet);
116-
} catch (DocumentException | IOException | URISyntaxException | XMLStreamException e) {
117-
LOG.error("Unexpected problem getting on pom operator", e);
118-
throw new MavenProvider.DependencyUpdateException(
119-
"Failure while executing pom operator: ", e);
126+
127+
} catch (Exception e) {
128+
LOG.error("Problem getting on pom operator", e);
120129
}
121130
});
122131

‎plugins/codemodder-plugin-maven/src/main/java/io/codemodder/plugins/maven/MavenProvider.java‎

Lines changed: 17 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -57,17 +57,21 @@ public void modify(final Path path, final byte[] contents) throws IOException {
5757
private final POMDependencyUpdater pomDependencyUpdater;
5858
private final PomFileFinder pomFileFinder;
5959

60+
MavenProvider(
61+
final PomFileFinder pomFileFinder, final POMDependencyUpdater pomDependencyUpdater) {
62+
this.pomFileFinder = Objects.requireNonNull(pomFileFinder);
63+
this.pomDependencyUpdater = Objects.requireNonNull(pomDependencyUpdater);
64+
}
65+
6066
MavenProvider(
6167
final PomModifier pomModifier,
6268
final PomFileFinder pomFileFinder,
6369
final DependencyDescriptor dependencyDescriptor,
6470
final ArtifactInjectionPositionFinder positionFinder) {
65-
Objects.requireNonNull(pomModifier);
66-
Objects.requireNonNull(pomFileFinder);
67-
this.pomFileFinder = pomFileFinder;
68-
this.pomDependencyUpdater =
71+
this(
72+
pomFileFinder,
6973
new DefaultPOMDependencyUpdater(
70-
new CodeTFGenerator(positionFinder, dependencyDescriptor), pomFileFinder, pomModifier);
74+
new CodeTFGenerator(positionFinder, dependencyDescriptor), pomFileFinder, pomModifier));
7175
}
7276

7377
MavenProvider(
@@ -113,20 +117,17 @@ public Optional<Path> findForFile(final Path projectDir, final Path file) throws
113117
}
114118
}
115119

120+
/**
121+
* This method must not throw exception -- it should capture failures in its model and bubble up
122+
* normal results.
123+
*/
116124
@Override
117125
public DependencyUpdateResult updateDependencies(
118126
final Path projectDir, final Path file, final List<DependencyGAV> dependencies) {
119-
try {
120-
String dependenciesStr =
121-
dependencies.stream().map(DependencyGAV::toString).collect(Collectors.joining(","));
122-
LOG.trace("Updating dependencies for {} in {}: {}", file, projectDir, dependenciesStr);
123-
final DependencyUpdateResult dependencyUpdateResult =
124-
pomDependencyUpdater.execute(projectDir, file, dependencies);
125-
LOG.trace("Dependency update result: {}", dependencyUpdateResult);
126-
return dependencyUpdateResult;
127-
} catch (Exception e) {
128-
throw new DependencyUpdateException("Failure when updating dependencies", e);
129-
}
127+
String dependenciesStr =
128+
dependencies.stream().map(DependencyGAV::toString).collect(Collectors.joining(","));
129+
LOG.trace("Updating dependencies for {} in {}: {}", file, projectDir, dependenciesStr);
130+
return pomDependencyUpdater.execute(projectDir, file, dependencies);
130131
}
131132

132133
@Override

‎plugins/codemodder-plugin-maven/src/main/java/io/codemodder/plugins/maven/POMDependencyUpdater.java‎

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2,15 +2,10 @@
22

33
import io.codemodder.DependencyGAV;
44
import io.codemodder.DependencyUpdateResult;
5-
import java.io.IOException;
6-
import java.net.URISyntaxException;
75
import java.nio.file.Path;
86
import java.util.List;
9-
import javax.xml.stream.XMLStreamException;
10-
import org.dom4j.DocumentException;
117

128
interface POMDependencyUpdater {
139

14-
DependencyUpdateResult execute(Path projectDir, Path file, List<DependencyGAV> dependencies)
15-
throws IOException, XMLStreamException, DocumentException, URISyntaxException;
10+
DependencyUpdateResult execute(Path projectDir, Path file, List<DependencyGAV> dependencies);
1611
}

‎plugins/codemodder-plugin-maven/src/test/java/io/codemodder/plugins/maven/DefaultPOMDependencyUpdaterTest.java‎

Lines changed: 99 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,99 @@
1+
package io.codemodder.plugins.maven;
2+
3+
import static org.assertj.core.api.Assertions.assertThat;
4+
import static org.mockito.ArgumentMatchers.any;
5+
import static org.mockito.Mockito.*;
6+
7+
import io.codemodder.DependencyDescriptor;
8+
import io.codemodder.DependencyGAV;
9+
import io.codemodder.DependencyUpdateResult;
10+
import java.io.IOException;
11+
import java.nio.file.Files;
12+
import java.nio.file.Path;
13+
import java.util.List;
14+
import java.util.Optional;
15+
import org.junit.jupiter.api.BeforeEach;
16+
import org.junit.jupiter.api.Test;
17+
import org.junit.jupiter.api.io.TempDir;
18+
19+
final class DefaultPOMDependencyUpdaterTest {
20+
21+
private ArtifactInjectionPositionFinder positionFinder;
22+
private CodeTFGenerator codeTFGenerator;
23+
private DependencyDescriptor dependencyDescriptor;
24+
private PomFileFinder pomFileFinder;
25+
private MavenProvider.PomModifier pomModifier;
26+
private Path projectDir;
27+
private Path pomPath;
28+
private DefaultPOMDependencyUpdater updater;
29+
30+
@BeforeEach
31+
void setup(@TempDir Path tempDir) throws IOException {
32+
positionFinder = mock(ArtifactInjectionPositionFinder.class);
33+
dependencyDescriptor = DependencyDescriptor.createMarkdownDescriptor();
34+
pomFileFinder = mock(PomFileFinder.class);
35+
pomModifier = mock(MavenProvider.PomModifier.class);
36+
this.projectDir = tempDir;
37+
build();
38+
}
39+
40+
void build() throws IOException {
41+
pomPath = projectDir.resolve("pom.xml");
42+
43+
if (!Files.exists(pomPath)) {
44+
Files.createFile(pomPath);
45+
}
46+
Files.writeString(
47+
pomPath,
48+
"""
49+
<project>
50+
<modelVersion>4.0.0</modelVersion>
51+
<groupId>io.codemodder</groupId>
52+
<artifactId>test</artifactId>
53+
<version>1.0.0</version>
54+
<dependencies>
55+
56+
</dependencies>
57+
</project>
58+
""");
59+
60+
when(pomFileFinder.findForFile(any(), any())).thenReturn(Optional.ofNullable(pomPath));
61+
codeTFGenerator = new CodeTFGenerator(positionFinder, dependencyDescriptor);
62+
when(positionFinder.find(any(), any())).thenReturn(7);
63+
updater = new DefaultPOMDependencyUpdater(codeTFGenerator, pomFileFinder, pomModifier);
64+
}
65+
66+
@Test
67+
void it_works() {
68+
DependencyUpdateResult result =
69+
updater.execute(projectDir, pomPath, List.of(DependencyGAV.OWASP_XSS_JAVA_ENCODER));
70+
assertThat(result.erroredFiles()).isEmpty();
71+
assertThat(result.injectedPackages()).containsExactly(DependencyGAV.OWASP_XSS_JAVA_ENCODER);
72+
assertThat(result.skippedPackages()).isEmpty();
73+
assertThat(result.packageChanges()).hasSize(1);
74+
}
75+
76+
@Test
77+
void it_doesnt_propagate_update_exceptions_when_no_pom_found() throws IOException {
78+
when(pomFileFinder.findForFile(any(), any()))
79+
.thenThrow(new IOException("blows up during finding"));
80+
DependencyUpdateResult result =
81+
updater.execute(projectDir, pomPath, List.of(DependencyGAV.OWASP_XSS_JAVA_ENCODER));
82+
assertThat(result.erroredFiles()).isEmpty();
83+
assertThat(result.injectedPackages()).isEmpty();
84+
assertThat(result.skippedPackages()).isEmpty();
85+
assertThat(result.packageChanges()).isEmpty();
86+
}
87+
88+
@Test
89+
void it_doesnt_propagate_update_exceptions_when_pom_io_error() throws IOException {
90+
build();
91+
doThrow(new IOException("blows up during modification")).when(pomModifier).modify(any(), any());
92+
DependencyUpdateResult result =
93+
updater.execute(projectDir, pomPath, List.of(DependencyGAV.OWASP_XSS_JAVA_ENCODER));
94+
assertThat(result.erroredFiles()).hasSize(1);
95+
assertThat(result.injectedPackages()).isEmpty();
96+
assertThat(result.skippedPackages()).isEmpty();
97+
assertThat(result.packageChanges()).isEmpty();
98+
}
99+
}

‎plugins/codemodder-plugin-maven/src/test/java/io/codemodder/plugins/maven/MavenProviderTest.java‎

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -153,7 +153,7 @@ void it_returns_changeset_when_no_issues() throws IOException {
153153

154154
// we've updated all the poms, so we merge this with the pre-existing one change
155155
List<CodeTFChangesetEntry> changes = result.packageChanges();
156-
assertThat(changes.size()).isEqualTo(2);
156+
assertThat(changes).hasSize(2);
157157

158158
// we injected all the dependencies, total success!
159159
assertThat(result.injectedPackages()).containsOnly(marsDependency1, marsDependency2);
@@ -204,15 +204,15 @@ void it_places_library_facts_in_correct_pom_place(
204204

205205
// we only injected the toolkit -- verify that
206206
List<DependencyGAV> injectedPackages = result.injectedPackages();
207-
assertThat(injectedPackages.size()).isEqualTo(1);
207+
assertThat(injectedPackages).hasSize(1);
208208
DependencyGAV injectedPackage = injectedPackages.get(0);
209209
assertThat(injectedPackage).isEqualTo(DependencyGAV.JAVA_SECURITY_TOOLKIT);
210210

211211
List<CodeTFChangesetEntry> changesets = result.packageChanges();
212-
assertThat(changesets.size()).isEqualTo(1);
212+
assertThat(changesets).hasSize(1);
213213
CodeTFChangesetEntry change = changesets.get(0);
214214
List<CodeTFChange> changes = change.getChanges();
215-
assertThat(changes.size()).isEqualTo(1);
215+
assertThat(changes).hasSize(1);
216216
CodeTFChange lineChange = changes.get(0);
217217
assertThat(lineChange.getDescription()).contains("License: ");
218218
assertThat(lineChange.getLineNumber()).isEqualTo(libraryFactsLineTarget);
@@ -402,7 +402,7 @@ void it_updates_poms_correctly() throws IOException {
402402
provider.updateDependencies(
403403
projectDir, module1Pom, List.of(marsDependency1, marsDependency2, venusDependency));
404404

405-
assertThat(result.packageChanges().size()).isEqualTo(3);
405+
assertThat(result.packageChanges()).hasSize(3);
406406

407407
CodeTFChangesetEntry changesetEntry = result.packageChanges().iterator().next();
408408
assertThat(changesetEntry.getPath()).isEqualTo("module1/pom.xml");
@@ -423,8 +423,7 @@ void it_finds_correct_poms() throws IOException {
423423
Pair.of(jspFile, Optional.of(rootPom)),
424424
Pair.of(irrelevantFile, Optional.of(rootPom)))) {
425425
Optional<Path> pom = pomFinder.findForFile(this.projectDir, pair.getLeft());
426-
assertThat(pom.isPresent()).isTrue();
427-
assertThat(pom.get()).isEqualTo(pair.getRight().get());
426+
assertThat(pom).contains(pair.getRight().get());
428427
}
429428
}
430429

0 commit comments

Comments
 (0)