Skip to content

Commit fe74bda

Browse files
aldrinlealJacoco Coverage Update Action
andauthored
Use pom-operator, Take 2 (pixee#16)
* Implements pom-operator * Codifies formatting rules for `pom.xml` in `.editorconfig` Co-authored-by: Jacoco Coverage Update Action <pixee@users.noreply.github.com>
1 parent b39b2d9 commit fe74bda

11 files changed

Lines changed: 170 additions & 310 deletions

File tree

.editorconfig

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
# EditorConfig is awesome: https://EditorConfig.org
2+
3+
# top-most EditorConfig file
4+
root = true
5+
6+
# Unix-style newlines with a newline ending every file
7+
# Set default charset
8+
[*]
9+
end_of_line = lf
10+
insert_final_newline = true
11+
charset = utf-8
12+
13+
# 4 space indentation
14+
[*.xml]
15+
indent_style = space
16+
indent_size = 2

.github/badges/branches.svg

Lines changed: 1 addition & 1 deletion
Loading

.github/badges/jacoco.svg

Lines changed: 1 addition & 1 deletion
Loading

.github/workflows/build.yml

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,29 +2,40 @@ name: Java CI
22

33
on: [push]
44

5+
env:
6+
MAVEN_OPTS: "-Dhttps.protocols=TLSv1.2"
7+
58
jobs:
69
build:
710
runs-on: ubuntu-latest
811

912
steps:
1013
- uses: actions/checkout@v2
14+
1115
- name: "Set up JDK 11"
1216
uses: actions/setup-java@v2
1317
with:
1418
java-version: '11'
1519
distribution: 'adopt'
1620

21+
- name: Set up Maven
22+
uses: stCarolas/setup-maven@v4.4
23+
with:
24+
maven-version: 3.8.2
25+
1726
- name: "Setting up Artifactory authentication"
1827
uses: s4u/maven-settings-action@v2
1928
with:
20-
servers: '[{"id": "central", "username": "${env.ARTIFACTORY_USER}", "password": "${env.ARTIFACTORY_TOKEN}"}]'
29+
servers: |
30+
[
31+
{"id": "pixee-libs-release", "username": "${{ secrets.ARTIFACTORY_USER }}", "password": "${{ secrets.ARTIFACTORY_TOKEN }}"},
32+
{"id": "pixee-libs-snapshot", "username": "${{ secrets.ARTIFACTORY_USER }}", "password": "${{ secrets.ARTIFACTORY_TOKEN }}"}
33+
]
2134
githubServer: false
2235

2336
- name: "Build with Maven"
24-
env:
25-
ARTIFACTORY_USER: ${{ secrets.ARTIFACTORY_USER }}
26-
ARTIFACTORY_TOKEN: ${{ secrets.ARTIFACTORY_TOKEN }}
27-
run: mvn --batch-mode --update-snapshots -Pci verify
37+
run: |
38+
mvn --batch-mode --update-snapshots -Pci verify
2839
2940
- name: "Generate Coverage Badge"
3041
id: jacoco

.gitignore

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,4 +27,8 @@ hs_err_pid*
2727

2828
target/
2929

30-
dependency-reduced-pom.xml
30+
dependency-reduced-pom.xml
31+
32+
.secrets
33+
34+
output-*.txt

pom.xml

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,11 @@
9191
<artifactId>java-security-toolkit</artifactId>
9292
<version>${versions.jst}</version>
9393
</dependency>
94+
<dependency>
95+
<groupId>io.openpixee</groupId>
96+
<artifactId>java-security-toolkit-xstream</artifactId>
97+
<version>${versions.jst-xstream}</version>
98+
</dependency>
9499
<dependency>
95100
<groupId>io.github.pixee</groupId>
96101
<artifactId>codetf-java</artifactId>

src/main/java/io/openpixee/java/CodeTFReportGenerator.java

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -146,8 +146,11 @@ private List<CodeTFResult> toCodeTFResults(
146146
weave.changeCode(),
147147
"fill description for " + weave.changeCode()))
148148
.collect(Collectors.toList());
149-
CodeTFResult result = new CodeTFResult(path, diff, changes);
150-
codetfResults.add(result);
149+
150+
if (StringUtils.isNotBlank(diff)) {
151+
CodeTFResult result = new CodeTFResult(path, diff, changes);
152+
codetfResults.add(result);
153+
}
151154
}
152155
return Collections.unmodifiableList(codetfResults);
153156
}

src/main/java/io/openpixee/java/protections/DependencyInjectingVisitor.java

Lines changed: 56 additions & 145 deletions
Original file line numberDiff line numberDiff line change
@@ -10,40 +10,27 @@
1010
import io.openpixee.java.FileWeavingContext;
1111
import io.openpixee.java.Weave;
1212
import io.openpixee.java.WeavingResult;
13+
import io.openpixee.maven.operator.POMOperator;
14+
import io.openpixee.maven.operator.ProjectModel;
15+
import io.openpixee.maven.operator.ProjectModelFactory;
1316
import java.io.*;
14-
import java.nio.charset.StandardCharsets;
17+
import java.nio.charset.Charset;
1518
import java.nio.file.Files;
1619
import java.util.*;
1720
import java.util.stream.Collectors;
18-
import javax.xml.transform.*;
19-
import javax.xml.transform.stream.StreamResult;
20-
import javax.xml.transform.stream.StreamSource;
21-
import org.apache.commons.io.FileUtils;
2221
import org.apache.commons.io.IOUtils;
23-
import org.apache.maven.model.Dependency;
24-
import org.apache.maven.model.Model;
25-
import org.apache.maven.model.io.xpp3.MavenXpp3Reader;
26-
import org.apache.maven.model.io.xpp3.MavenXpp3Writer;
27-
import org.codehaus.plexus.util.xml.pull.XmlPullParserException;
2822
import org.slf4j.Logger;
2923
import org.slf4j.LoggerFactory;
3024

3125
/**
3226
* This is an important default weaver that will inject the dependencies that the weaves require.
3327
*/
3428
public final class DependencyInjectingVisitor implements FileBasedVisitor {
35-
36-
private final PomRewriterStrategy pomRewriterStrategy;
3729
private final WeavingResult emptyWeaveResult;
3830
private boolean scanned;
3931
private Map<File, Set<DependencyGAV>> pomsToUpdate;
4032

4133
public DependencyInjectingVisitor() {
42-
this(new MavenXpp3RewriterStrategy());
43-
}
44-
45-
DependencyInjectingVisitor(PomRewriterStrategy pomRewriterStrategy) {
46-
this.pomRewriterStrategy = Objects.requireNonNull(pomRewriterStrategy);
4734
this.emptyWeaveResult =
4835
WeavingResult.createDefault(Collections.emptySet(), Collections.emptySet());
4936
this.scanned = false;
@@ -55,87 +42,6 @@ public String ruleId() {
5542
return pomInjectionRuleId;
5643
}
5744

58-
/**
59-
* There are many different ways we could choose to rewrite the POM to contain our dependency.
60-
* There are many tradeoffs between them as to version coverage, accuracy, robustness,
61-
* "disruptiveness" of the patch, etc., so we keep multiple strategies around until we probably
62-
* have to hand-develop a solution that ticks all our boxes.
63-
*/
64-
public interface PomRewriterStrategy {
65-
/**
66-
* Given a model of the POM, and the string contents itself, return the String version of the
67-
* POM, rewritten to contain our dependency.
68-
*/
69-
String rewritePom(String pomContentsAsString, Model model) throws IOException;
70-
}
71-
72-
/**
73-
* {@inheritDoc}
74-
*
75-
* <p>Uses the {@link MavenXpp3Writer} to rewrite the {@link Model} with our dependency added in.
76-
*/
77-
public static class MavenXpp3RewriterStrategy implements PomRewriterStrategy {
78-
@Override
79-
public String rewritePom(final String pomContentsAsString, final Model model)
80-
throws IOException {
81-
var writer = new MavenXpp3Writer();
82-
var rewrittenPom = new StringWriter();
83-
writer.write(rewrittenPom, model);
84-
return rewrittenPom.toString();
85-
}
86-
}
87-
88-
/**
89-
* {@inheritDoc}
90-
*
91-
* <p>Uses an XSLT transformation to convert the POM to inject the dependency.
92-
*
93-
* <p>TODO: This doesn't work yet -- the XSL needs some love. It only correctly injects a new
94-
* dependencies block when there is none.
95-
*/
96-
public static class XslTransformingStrategy implements PomRewriterStrategy {
97-
98-
@Override
99-
public String rewritePom(final String pomContentsAsString, final Model model)
100-
throws IOException {
101-
var factory = TransformerFactory.newInstance();
102-
var injectionXsl =
103-
IOUtils.toString(
104-
new InputStreamReader(
105-
Objects.requireNonNull(
106-
getClass().getResourceAsStream("/inject-dependency.xsl"),
107-
"dependency injection xsl not found")));
108-
var xslt = new StreamSource(new StringReader(injectionXsl));
109-
try {
110-
var transformer = factory.newTransformer(xslt);
111-
var text = new StreamSource(new StringReader(pomContentsAsString));
112-
var sw = new StringWriter();
113-
transformer.transform(text, new StreamResult(sw));
114-
final String dependenciesBlock =
115-
createDependency(projectGroup, projectArtifactId, projectVersion);
116-
var transformedText = sw.toString();
117-
transformedText = transformedText.replace("%PIXEE_DEPENDENCY_INFO%", dependenciesBlock);
118-
return transformedText;
119-
} catch (TransformerException e) {
120-
throw new IOException(e);
121-
}
122-
}
123-
124-
private String createDependency(
125-
final String group, final String artifact, final String version) {
126-
StringBuilder sb = new StringBuilder();
127-
sb.append(nl);
128-
sb.append(" <dependencies>").append(nl);
129-
sb.append(" <dependency>").append(nl);
130-
sb.append(" <groupId>").append(group).append("</groupId>").append(nl);
131-
sb.append(" <artifactId>").append(artifact).append("</artifactId>").append(nl);
132-
sb.append(" <version>").append(version).append("</version>").append(nl);
133-
sb.append(" </dependency>").append(nl);
134-
sb.append(" </dependencies>").append(nl);
135-
return sb.toString();
136-
}
137-
}
138-
13945
@Override
14046
public WeavingResult visitRepositoryFile(
14147
final File repositoryRoot,
@@ -211,65 +117,70 @@ private Map<File, Set<DependencyGAV>> scan(
211117

212118
@VisibleForTesting
213119
ChangedFile transformPomIfNeeded(final File file, final Set<DependencyGAV> dependenciesToAdd)
214-
throws IOException, XmlPullParserException {
215-
var pomContents = IOUtils.toString(new FileReader(file));
216-
var reader = new MavenXpp3Reader();
217-
var model = reader.read(new StringReader(pomContents));
218-
219-
boolean addedDependencies = false;
220-
for (final DependencyGAV dependencyToAdd : dependenciesToAdd) {
221-
var hasDependencyAlready =
222-
model.getDependencies().stream()
223-
.anyMatch(
224-
dep ->
225-
dependencyToAdd.group().equals(dep.getGroupId())
226-
&& dependencyToAdd.artifact().equals(dep.getArtifactId()));
227-
228-
if (!hasDependencyAlready) {
229-
LOG.debug("Adding dependency {}:{}", dependencyToAdd.group(), dependencyToAdd.artifact());
230-
addedDependencies = true;
231-
var dependency = new Dependency();
232-
dependency.setArtifactId(dependencyToAdd.artifact());
233-
dependency.setGroupId(dependencyToAdd.group());
234-
dependency.setVersion(dependencyToAdd.version());
235-
model.addDependency(dependency);
236-
} else {
237-
LOG.debug(
238-
"Not weaving pom since it contained dependency {}:{}",
239-
dependencyToAdd.group(),
240-
dependencyToAdd.artifact());
241-
}
242-
}
243-
244-
if (!addedDependencies) {
245-
LOG.debug("No new dependencies needed");
120+
throws IOException {
121+
/*
122+
* Short-circuit things
123+
*/
124+
if (null == dependenciesToAdd || dependenciesToAdd.isEmpty()) {
246125
return null;
247126
}
248127

249-
var rewrittenPomContents = pomRewriterStrategy.rewritePom(pomContents, model);
128+
List<io.openpixee.maven.operator.Dependency> mappedDependencies =
129+
dependenciesToAdd.stream()
130+
.map(
131+
dependencyGAV ->
132+
new io.openpixee.maven.operator.Dependency(
133+
dependencyGAV.group(),
134+
dependencyGAV.artifact(),
135+
dependencyGAV.version(),
136+
null,
137+
null,
138+
null))
139+
.collect(Collectors.toList());
140+
141+
var originalPomContents =
142+
IOUtils.readLines(new FileInputStream(file), Charset.defaultCharset());
143+
144+
final File lastPomFile = File.createTempFile("pom", ".xml");
145+
146+
IOUtils.copy(new FileInputStream(file), new FileOutputStream(lastPomFile));
147+
148+
mappedDependencies.forEach(
149+
newDependency -> {
150+
ProjectModel projectModel =
151+
ProjectModelFactory.load(lastPomFile)
152+
.withDependency(newDependency)
153+
.withOverrideIfAlreadyExists(true)
154+
.withSkipIfNewer(true)
155+
.withUseProperties(true)
156+
.build();
157+
158+
boolean result = POMOperator.modify(projectModel);
159+
160+
if (result) {
161+
try {
162+
IOUtils.write(projectModel.getResultPom().asXML(), new FileOutputStream(lastPomFile));
163+
} catch (IOException e) {
164+
throw new RuntimeException(e);
165+
}
166+
}
167+
});
250168

251-
File modifiedPom = File.createTempFile("pom", ".xml");
252-
FileUtils.write(modifiedPom, rewrittenPomContents, StandardCharsets.UTF_8);
169+
var finalPomContents =
170+
IOUtils.readLines(new FileInputStream(lastPomFile), Charset.defaultCharset());
253171

254-
/*
255-
* We have to calculate the line position to associate with this diff. This is already calculated when the final
256-
* report is built later, so we should refactor.
257-
*
258-
* This is also wasteful because it duplicates reading the both files' contents into memory.
259-
*/
260-
List<String> original = Files.readAllLines(file.toPath());
261-
List<String> patched = Files.readAllLines(modifiedPom.toPath());
262-
Patch<String> patch = DiffUtils.diff(original, patched);
263-
if (patch.getDeltas().isEmpty()) {
264-
LOG.warn("For some reason there was no pom delta");
172+
if (finalPomContents.equals(originalPomContents)) {
265173
return null;
266174
}
267175

176+
Patch<String> patch = DiffUtils.diff(originalPomContents, finalPomContents);
177+
268178
AbstractDelta<String> delta = patch.getDeltas().get(0);
269179
int position = 1 + delta.getSource().getPosition();
180+
270181
return ChangedFile.createDefault(
271182
file.getAbsolutePath(),
272-
modifiedPom.getAbsolutePath(),
183+
lastPomFile.getAbsolutePath(),
273184
Weave.from(position, pomInjectionRuleId));
274185
}
275186

0 commit comments

Comments
 (0)