Support parsing of name-ver spec

2 files changed, 46 insertions, 63 deletions

diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/yum/YumPackageName.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/yum/YumPackageName.java
index 24dfda0470f..5b32f50ad49 100644
--- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/yum/YumPackageName.java
+++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/yum/YumPackageName.java
@@ -11,7 +11,7 @@ import java.util.regex.Pattern;
 import java.util.stream.Collectors;
 
 /**
- * YUM package name.
+ * A YUM/DNF package name.
  *
  * <p>From yum(8): YUM package names are used with install, update, remove, list, info etc
  * with any of the following as well as globs of any of the following, with any of the
@@ -41,9 +41,10 @@ public class YumPackageName {
             Arrays.stream(Architecture.values()).map(Architecture::name).collect(Collectors.joining("|"));
     private static final Pattern ARCHITECTURE_PATTERN = Pattern.compile("\\.(" + ARCHITECTURES_OR + "|\\*)$");
     private static final Pattern EPOCH_PATTERN = Pattern.compile("^((.+)-)?([0-9]+)$");
-    private static final Pattern NAME_VER_REL_PATTERN = Pattern.compile("^((.+)-)?" +
-            "([+()a-z0-9._]*[0-9][a-z0-9._]*)-" + // ver contains at least one digit
-            "([+()a-z0-9._]*[0-9][a-z0-9._]*)$"); // rel contains at least one digit
+    private static final Pattern NAME_VER_REL_PATTERN =
+            Pattern.compile("^(.+?)?" +                                 // name
+                            "-([+()a-z0-9._]*[0-9]+\\.[a-z0-9._]*)" +   // ver: contains at least one digit and dot
+                            "(?:-([+()a-z0-9._]*[0-9][a-z0-9._]*))?$"); // rel: contains at least one digit
     private static final Pattern NAME_PATTERN = Pattern.compile("^[+()a-zA-Z0-9._-]+$");
 
     private final Optional<String> epoch;
@@ -90,7 +91,6 @@ public class YumPackageName {
                            Optional<String> architecture) {
         if (Strings.isNullOrEmpty(name))
             throw new IllegalArgumentException("name cannot be null or empty");
-
         this.epoch = epoch;
         this.name = name;
         this.version = version;
@@ -106,6 +106,7 @@ public class YumPackageName {
      * <ol>
      *     <li>name
      *     <li>name.arch
+     *     <li>name-ver
      *     <li>name-ver-rel
      *     <li>name-ver-rel.arch
      *     <li>name-epoch:ver-rel.arch
@@ -120,14 +121,7 @@ public class YumPackageName {
         Optional<String> epoch = Optional.empty();
         String name = null;
 
-        //       packageSpec                spec
-        //  name                       name
-        //  name.arch                  name.arch
-        //  name-ver-rel               name-ver-rel
-        //  name-ver-rel.arch          name-ver-rel.arch
-        //  name-epoch:ver-rel.arch    name-epoch:ver-rel.arch
-        //  epoch:name-ver-rel.arch    epoch:name-ver-rel.arch
-
+        // Parse epoch and remove it from spec
         int epochColon = spec.indexOf(':');
         if (epochColon >= 0) {
             Matcher epochMatcher = EPOCH_PATTERN.matcher(spec.substring(0, epochColon));
@@ -138,17 +132,12 @@ public class YumPackageName {
             name = epochMatcher.group(2);
             epoch = Optional.of(epochMatcher.group(3));
 
-            spec = spec.substring(epochColon + 1);
+            spec = epochColon == 0
+                    ? spec.substring(epochColon + 1)
+                    : spec.substring(0, epochColon - 1) + spec.substring(epochColon + 1);
         }
 
-        //       packageSpec                spec
-        //  name                       name
-        //  name.arch                  name.arch
-        //  name-ver-rel               name-ver-rel
-        //  name-ver-rel.arch          name-ver-rel.arch
-        //  name-epoch:ver-rel.arch    ver-rel.arch (non-null name)
-        //  epoch:name-ver-rel.arch    name-ver-rel.arch
-
+        // Parse architecture and remove it from spec
         Optional<String> architecture = Optional.empty();
         Matcher architectureMatcher = ARCHITECTURE_PATTERN.matcher(spec);
         if (architectureMatcher.find()) {
@@ -156,44 +145,27 @@ public class YumPackageName {
             spec = spec.substring(0, architectureMatcher.start());
         }
 
-        //       packageSpec                spec
-        //  name                       name
-        //  name.arch                  name
-        //  name-ver-rel               name-ver-rel
-        //  name-ver-rel.arch          name-ver-rel
-        //  name-epoch:ver-rel.arch    ver-rel (non-null name)
-        //  epoch:name-ver-rel.arch    name-ver-rel
-
+        // Parse name, version and release and remove the latter two from spec
         Optional<String> version = Optional.empty();
         Optional<String> release = Optional.empty();
         Matcher matcher = NAME_VER_REL_PATTERN.matcher(spec);
         if (matcher.find()) {
-            // spec format one of:
-            //  1. name-ver-rel
-            //  2. ver-rel
-
-            spec = matcher.group(2);
+            spec = matcher.group(1);
             if (spec == null) {
                 if (name == null) {
                     throw new IllegalArgumentException("No package name was found: " + packageSpec);
                 }
                 spec = name; // makes spec hold the package name in all cases below.
-            } else if (name != null) {
-                throw new IllegalArgumentException("Ambiguous package names were found for " +
-                        packageSpec + ": '" + name + "' and '" + spec + "'");
             }
 
-            version = Optional.of(matcher.group(3));
-            release = Optional.of(matcher.group(4));
+            version = Optional.of(matcher.group(2));
+            release = Optional.ofNullable(matcher.group(3));
         }
 
-        //       packageSpec                spec
-        //  name                       name
-        //  name.arch                  name
-        //  name-ver-rel               name
-        //  name-ver-rel.arch          name
-        //  name-epoch:ver-rel.arch    name
-        //  epoch:name-ver-rel.arch    name
+        // Set default epoch if we have a version
+        if (version.isPresent() && epoch.isEmpty()) {
+            epoch = Optional.of("0");
+        }
 
         if (!NAME_PATTERN.matcher(spec).find()) {
             throw new IllegalArgumentException("Bad package name in " + packageSpec + ": '" + spec + "'");
@@ -221,11 +193,10 @@ public class YumPackageName {
     /** Return package name, omitting components that are not specified. */
     public String toName() {
         StringBuilder builder = new StringBuilder();
-        boolean isBare = version.isEmpty() && release.isEmpty() && architecture.isEmpty();
         char nextDelimiter;
         builder.append(name);
         // Fully versioned package names must always include epoch in Yum 4
-        epoch.or(() -> Optional.of("0").filter(v -> !isBare))
+        epoch.or(() -> Optional.of("0").filter(v -> version.isPresent()))
              .ifPresent(ep -> builder.append('-').append(ep));
         nextDelimiter = ':';
         version.ifPresent(s -> builder.append(nextDelimiter).append(s));
diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/task/util/yum/YumPackageNameTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/task/util/yum/YumPackageNameTest.java
index bcec086b4af..a4dc3ac359a 100644
--- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/task/util/yum/YumPackageNameTest.java
+++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/task/util/yum/YumPackageNameTest.java
@@ -60,11 +60,21 @@ public class YumPackageNameTest {
                 null,
                 null,
                 "x86_64",
-                "docker-engine-selinux-0.x86_64",
+                "docker-engine-selinux.x86_64",
                 null);
 
+        // name-ver
+        verifyPackageName("docker-engine-selinux-1.12.6",
+                          "0",
+                          "docker-engine-selinux",
+                          "1.12.6",
+                          null,
+                          null,
+                          "docker-engine-selinux-0:1.12.6",
+                          null);
+
         // name-ver-rel
-        verifyPackageName("docker-engine-selinux-0:1.12.6-1.el7",
+        verifyPackageName("docker-engine-selinux-1.12.6-1.el7",
                 "0",
                 "docker-engine-selinux",
                 "1.12.6",
@@ -74,7 +84,7 @@ public class YumPackageNameTest {
                 "docker-engine-selinux-0:1.12.6-1.el7.*");
 
         // name-ver-rel.arch
-        verifyPackageName("docker-engine-selinux-0:1.12.6-1.el7.x86_64",
+        verifyPackageName("docker-engine-selinux-1.12.6-1.el7.x86_64",
                 "0",
                 "docker-engine-selinux",
                 "1.12.6",
@@ -115,30 +125,32 @@ public class YumPackageNameTest {
                                    String toName,
                                    String toVersionName) {
         YumPackageName yumPackageName = YumPackageName.fromString(input);
-        verifyValue(epoch, yumPackageName.getEpoch());
-        verifyValue(name, Optional.of(yumPackageName.getName()));
-        verifyValue(version, yumPackageName.getVersion());
-        verifyValue(release, yumPackageName.getRelease());
-        verifyValue(architecture, yumPackageName.getArchitecture());
-        verifyValue(toName, Optional.of(yumPackageName.toName()));
+        assertPackageField("epoch", epoch, yumPackageName.getEpoch());
+        assertPackageField("name", name, Optional.of(yumPackageName.getName()));
+        assertPackageField("version", version, yumPackageName.getVersion());
+        assertPackageField("release", release, yumPackageName.getRelease());
+        assertPackageField("architecture", architecture, yumPackageName.getArchitecture());
+        assertPackageField("toName()", toName, Optional.of(yumPackageName.toName()));
 
         if (toVersionName == null) {
             try {
                 yumPackageName.toVersionLockName();
                 fail();
             } catch (IllegalStateException e) {
-                assertThat(e.getMessage(), containsStringIgnoringCase("Version is missing "));
+                assertTrue("Exception message contains expected substring: " + e.getMessage(),
+                           e.getMessage().contains("Version is missing ") ||
+                           e.getMessage().contains("Release is missing "));
             }
         } else {
             assertEquals(toVersionName, yumPackageName.toVersionLockName());
         }
     }
 
-    private void verifyValue(String value, Optional<String> actual) {
-        if (value == null) {
-            assertFalse(actual.isPresent());
+    private void assertPackageField(String field, String expected, Optional<String> actual) {
+        if (expected == null) {
+            assertFalse(field + " is not present", actual.isPresent());
         } else {
-            assertEquals(value, actual.get());
+            assertEquals(field + " has expected value", expected, actual.get());
         }
     }