[java-mvn-enforcer] 03/04: JMVN-43 Pom 'Parser' fails ungracefully if it finds an undefined property
Rod Widdowson
rdw at steadingsoftware.com
Tue Oct 11 08:34:12 UTC 2022
This is an automated email from the git hooks/post-receive script.
rdw pushed a commit to branch main
in repository java-mvn-enforcer.
View the commit online:
http://git.shibboleth.net/view/?p=java-mvn-enforcer.git;a=commit;h=a24bd5e87093a2da4fa174b7d8231693323bdf83
commit a24bd5e87093a2da4fa174b7d8231693323bdf83
Author: Rod Widdowson <rdw at steadingsoftware.com>
AuthorDate: Mon Oct 10 15:02:17 2022 +0100
JMVN-43 Pom 'Parser' fails ungracefully if it finds an undefined property
https://shibboleth.atlassian.net/browse/JMVN-43
Push logging down to the 'parser' (it has to be passed in because
within maven we use a pseudo-logger) and convert Constraint
failures into error logs with defaults.
---
.../shibboleth/mvn/enforcer/impl/ParsedPom.java | 64 ++++++++++++++++------
.../mvn/enforcer/impl/ProjectPomContext.java | 10 ++--
2 files changed, 52 insertions(+), 22 deletions(-)
diff --git a/src/main/java/net/shibboleth/mvn/enforcer/impl/ParsedPom.java b/src/main/java/net/shibboleth/mvn/enforcer/impl/ParsedPom.java
index dd4dff7..5cd1ec9 100644
--- a/src/main/java/net/shibboleth/mvn/enforcer/impl/ParsedPom.java
+++ b/src/main/java/net/shibboleth/mvn/enforcer/impl/ParsedPom.java
@@ -35,21 +35,24 @@ import java.util.Set;
import javax.annotation.Nonnull;
import javax.annotation.Nullable;
+import org.slf4j.Logger;
import org.w3c.dom.Document;
import org.w3c.dom.Element;
import net.shibboleth.utilities.java.support.collection.Pair;
-import net.shibboleth.utilities.java.support.logic.Constraint;
import net.shibboleth.utilities.java.support.primitive.StringSupport;
import net.shibboleth.utilities.java.support.xml.ElementSupport;
import net.shibboleth.utilities.java.support.xml.ParserPool;
import net.shibboleth.utilities.java.support.xml.XMLParserException;
/**
- *
+ * Object to represent a pom file (it also does the parsing).
*/
public class ParsedPom {
+ /** Log. */
+ private final Logger log;
+
/** Compile dependencies - what we care about. */
private final Map<String, DependencyPomArtifact> compileDependencies = new HashMap<>();
@@ -87,6 +90,7 @@ public class ParsedPom {
* @param pomLoader how to get a pom (for BOM loading)
* @param pom the {@link Path} to the pom.
* @param pomName an ID for the pom
+ * @param thelog logger
* @param parentPomProperties if present it is properties from the parent (which might be empty), if null we are *only*
* looking for the parent pom coordinates.
* @param map Managed dependencies from parent
@@ -96,10 +100,12 @@ public class ParsedPom {
@Nonnull final MavenLoader pomLoader,
@Nonnull final Path pom,
@Nonnull final String pomName,
+ @Nonnull final Logger theLog,
@Nullable final Properties parentPomProperties,
@Nonnull final Map<String, DependencyPomArtifact> map)
throws Exception {
+ log = theLog;
managedDependencies = new HashMap<>(map);
sourcePomInfo = pomName;
@@ -124,7 +130,8 @@ public class ParsedPom {
return;
} else if (parentPomProperties.isEmpty()) {
if (parent != null) {
- final ParsedPom parsedParent = new ParsedPom(parsers, pomLoader, pomLoader.downloadArtifact(parent, "pom"), parent.toString(), new Properties(), Collections.emptyMap());
+ final ParsedPom parsedParent = new ParsedPom(parsers, pomLoader,
+ pomLoader.downloadArtifact(parent, "pom"), parent.toString(), theLog, new Properties(), Collections.emptyMap());
final Properties props = parsedParent.getProperties();
for (final Object p:props.keySet()) {
String pName = (String) p;
@@ -154,7 +161,8 @@ public class ParsedPom {
}
}
for (final DependencyPomArtifact bom : bomDependencies.values()) {
- final ParsedPom parsedBom = new ParsedPom(parsers, pomLoader, pomLoader.downloadArtifact(bom, "pom"), bom.toString(), new Properties(), Collections.emptyMap());
+ final ParsedPom parsedBom = new ParsedPom(parsers, pomLoader, pomLoader.downloadArtifact(bom, "pom"), bom.toString(),
+ log, new Properties(), Collections.emptyMap());
for (DependencyPomArtifact dep : parsedBom.getManagedDependencies().values()) {
addWithCheck(dep, managedDependencies);
}
@@ -170,7 +178,8 @@ public class ParsedPom {
for (final Element module: ElementSupport.getChildElementsByTagName(modules, "module")) {
final Path modulePath = pom.getParent().resolve(module.getTextContent()).resolve("pom.xml");
if (Files.exists(modulePath)) {
- final ParsedPom modulePom = new ParsedPom(parsers, pomLoader, modulePath, module.getTextContent(), properties, managedDependencies);
+ final ParsedPom modulePom = new ParsedPom(parsers, pomLoader, modulePath,
+ module.getTextContent(), log, properties, managedDependencies);
moduleCompiles.addAll(modulePom.getCompileDependencies());
moduleRuntimes.addAll(modulePom.getRuntimeDependencies());
generated.add(modulePom.getOurInfo());
@@ -191,7 +200,10 @@ public class ParsedPom {
*/
@Nonnull protected String getElementContent(final Element el) {
String remainingContents = StringSupport.trimOrNull(el.getTextContent());
- remainingContents = Constraint.isNotNull(remainingContents, "<" + el.getLocalName() + "> must have content");
+ if (remainingContents == null) {
+ log.error("{} : <{}> must have content", sourcePomInfo, el.getLocalName());
+ return "";
+ }
final StringBuilder contents = new StringBuilder();
for (int index = remainingContents.indexOf("${"); index >= 0; index = remainingContents.indexOf("${")) {
contents.append(remainingContents.substring(0, index));
@@ -201,7 +213,12 @@ public class ParsedPom {
break;
}
final String propName = remainingContents.substring(2, endIndex);
- contents.append(Constraint.isNotNull(properties.getProperty(propName), propName + " is not defined"));
+ String propValue = properties.getProperty(propName);
+ if (propValue == null) {
+ log.error("{}: Property {} is not defined", sourcePomInfo, propName);
+ propValue="<undefined>";
+ }
+ contents.append(propValue);
remainingContents = remainingContents.substring(endIndex+1);
}
contents.append(remainingContents);
@@ -385,12 +402,16 @@ public class ParsedPom {
} else if (parentArtifact != null) {
groupId = parentArtifact.getGroupId();
} else {
- Constraint.isGreaterThan(0, grps.size(), "<groupId> should exist in dependency");
+ if (grps.size() == 0) {
+ log.error("{}: <groupId> should exist in dependency", sourcePomInfo);
+ }
groupId = null;
}
final List<Element> arts = ElementSupport.getChildElementsByTagName(item, "artifactId");
- Constraint.isGreaterThan(0, arts.size(), "<artifactId> should exist in dependency");
+ if (arts.size() == 0) {
+ log.error("{}: <artifactId> should exist in dependency", sourcePomInfo);
+ }
artifactId = getElementContent(arts.get(0));
final List<Element> vers = ElementSupport.getChildElementsByTagName(item, "version");
@@ -408,11 +429,8 @@ public class ParsedPom {
}
final List<Element> clssfrs = ElementSupport.getChildElementsByTagName(item, "classifier");
- Constraint.isLessThanOrEqual(0, clssfrs.size(), ") or 1 <classifier> elements should exist in dependency");
- if (clssfrs.size() > 0) {
- classifier = getElementContent(clssfrs.get(0));
- } else {
- classifier = "";
+ if (clssfrs.size() > 1) {
+ log.error("{} 0 or 1 <classifier> elements should exist in dependency", sourcePomInfo);
}
List<Element> excls = ElementSupport.getChildElementsByTagName(item, "exclusions");
@@ -420,11 +438,21 @@ public class ParsedPom {
excls = ElementSupport.getChildElementsByTagName(excls.get(0), "exclusion");
for (Element e : excls) {
List<Element> els = ElementSupport.getChildElementsByTagName(e, "groupId");
- Constraint.isGreaterThan(0, els.size(), "<groupId> should exist in exclusion");
- final String grp = getElementContent(els.get(0));
+ final String grp;
+ if (els.size() == 0) {
+ grp = "";
+ log.error("{}: <groupId> should exist in exclusion\"", sourcePomInfo);
+ } else {
+ grp = getElementContent(els.get(0));
+ }
+ final String art;
els = ElementSupport.getChildElementsByTagName(e, "artifactId");
- Constraint.isGreaterThan(0, els.size(), "<artifactId> should exist in exclusion");
- final String art = getElementContent(els.get(0));
+ if (els.size() == 0) {
+ art = "";
+ log.error("{}: <artifactId> should exist in exclusion\"", sourcePomInfo);
+ } else {
+ art = getElementContent(els.get(0));
+ }
exclusions.add(new Pair<>(grp, art));
}
}
diff --git a/src/main/java/net/shibboleth/mvn/enforcer/impl/ProjectPomContext.java b/src/main/java/net/shibboleth/mvn/enforcer/impl/ProjectPomContext.java
index c5b002e..0fcd85c 100644
--- a/src/main/java/net/shibboleth/mvn/enforcer/impl/ProjectPomContext.java
+++ b/src/main/java/net/shibboleth/mvn/enforcer/impl/ProjectPomContext.java
@@ -157,8 +157,9 @@ public final class ProjectPomContext implements AutoCloseable {
try {
parentPom = new ParsedPom(parserPool,
mavenLoader,
- pomPath ,
- "provided pom",
+ pomPath,
+ "provided pom",
+ log,
null,
Collections.emptyMap());
} catch (final Exception e) {
@@ -178,14 +179,15 @@ public final class ProjectPomContext implements AutoCloseable {
}
try {
projectParent = new ParsedPom(parserPool, mavenLoader,
- parentPath, "parent pom.", new Properties(), Collections.emptyMap());
+ parentPath, "parent pom.", log,
+ new Properties(), Collections.emptyMap());
} catch (final Exception e) {
log.error("Could not parse {}:", parentPom.getParent(), e);
return false;
}
try {
parentPom = new ParsedPom(parserPool, mavenLoader, pomPath,
- "provided pom", projectParent.getProperties(),
+ "provided pom", log, projectParent.getProperties(),
projectParent.getManagedDependencies());
} catch (final Exception e) {
log.error("Could not load pom", e);
--
To stop receiving notification emails like this one, please contact
the administrator of this repository.
More information about the commits
mailing list