[MBUILDCACHE-73] Unify property value conversions in saving and restoration

Author: pdudits

src/main/java/org/apache/maven/buildcache/BuildCacheMojosExecutionStrategy.java

@@ -22,14 +22,12 @@
 import javax.inject.Inject;
 import javax.inject.Named;
 
-import java.io.File;
 import java.nio.file.Path;
 import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
 
-import org.apache.commons.lang3.ArrayUtils;
 import org.apache.commons.lang3.Strings;
 import org.apache.maven.SessionScoped;
 import org.apache.maven.buildcache.artifact.ArtifactRestorationReport;
@@ -381,19 +379,8 @@ boolean isParamsMatched(
             final String currentValue;
             try {
                 Object value = ReflectionUtils.getValueIncludingSuperclasses(propertyName, mojo);
-
-                if (value instanceof File) {
-                    Path baseDirPath = project.getBasedir().toPath();
-                    Path path = ((File) value).toPath();
-                    currentValue = normalizedPath(path, baseDirPath);
-                } else if (value instanceof Path) {
-                    Path baseDirPath = project.getBasedir().toPath();
-                    currentValue = normalizedPath(((Path) value), baseDirPath);
-                } else if (value != null && value.getClass().isArray()) {
-                    currentValue = ArrayUtils.toString(value);
-                } else {
-                    currentValue = String.valueOf(value);
-                }
+                Path baseDirPath = project.getBasedir().toPath();
+                currentValue = DtoUtils.normalizeValue(value, baseDirPath);
             } catch (IllegalAccessException e) {
                 LOGGER.error("Cannot extract plugin property {} from mojo {}", propertyName, mojo, e);
                 return false;
@@ -419,33 +406,6 @@ boolean isParamsMatched(
         return true;
     }
 
-    /**
-     * Best effort to normalize paths from Mojo fields.
-     * - all absolute paths under project root to be relativized for portability
-     * - redundant '..' and '.' to be removed to have consistent views on all paths
-     * - all relative paths are considered portable and should not be touched
-     * - absolute paths outside of project directory could not be deterministically
-     * relativized and not touched
-     */
-    private static String normalizedPath(Path path, Path baseDirPath) {
-        boolean isProjectSubdir = path.isAbsolute() && path.startsWith(baseDirPath);
-        if (LOGGER.isDebugEnabled()) {
-            LOGGER.debug(
-                    "normalizedPath isProjectSubdir {} path '{}' - baseDirPath '{}', path.isAbsolute() {}, path.startsWith(baseDirPath) {}",
-                    isProjectSubdir,
-                    path,
-                    baseDirPath,
-                    path.isAbsolute(),
-                    path.startsWith(baseDirPath));
-        }
-        Path preparedPath = isProjectSubdir ? baseDirPath.relativize(path) : path;
-        String normalizedPath = preparedPath.normalize().toString();
-        if (LOGGER.isDebugEnabled()) {
-            LOGGER.debug("normalizedPath '{}' - {} return {}", path, baseDirPath, normalizedPath);
-        }
-        return normalizedPath;
-    }
-
     private enum CacheRestorationStatus {
         SUCCESS,
         FAILURE,

src/main/java/org/apache/maven/buildcache/CacheControllerImpl.java

@@ -685,7 +685,7 @@ private void recordMojoProperties(CompletedExecution execution, MojoExecutionEve
         final Object mojo = executionEvent.getMojo();
 
         final File baseDir = executionEvent.getProject().getBasedir();
-        final String baseDirPath = FilenameUtils.normalizeNoEndSeparator(baseDir.getAbsolutePath()) + File.separator;
+        final Path baseDirPath = baseDir.toPath();
 
         final List<Parameter> parameters = mojoExecution.getMojoDescriptor().getParameters();
         for (Parameter parameter : parameters) {

src/main/java/org/apache/maven/buildcache/xml/DtoUtils.java

@@ -20,6 +20,8 @@
 
 import javax.annotation.Nonnull;
 
+import java.io.File;
+import java.nio.file.Path;
 import java.util.List;
 
 import org.apache.commons.lang3.ArrayUtils;
@@ -109,14 +111,11 @@ public static Dependency createDependency(Artifact artifact) {
     }
 
     public static void addProperty(
-            CompletedExecution execution, String propertyName, Object value, String baseDirPath, boolean tracked) {
+            CompletedExecution execution, String propertyName, Object value, Path baseDirPath, boolean tracked) {
         final PropertyValue valueType = new PropertyValue();
         valueType.setName(propertyName);
-        if (value != null && value.getClass().isArray()) {
-            value = ArrayUtils.toString(value);
-        }
-        final String valueText = String.valueOf(value);
-        valueType.setValue(Strings.CS.remove(valueText, baseDirPath));
+        final String valueText = normalizeValue(value, baseDirPath);
+        valueType.setValue(valueText);
         valueType.setTracked(tracked);
         execution.addProperty(valueType);
     }
@@ -173,4 +172,46 @@ public static Artifact copy(Artifact artifact) {
         copy.setFileSize(artifact.getFileSize());
         return copy;
     }
+
+    public static String normalizeValue(Object value, Path baseDirPath) {
+        final String currentValue;
+        if (value instanceof File) {
+            Path path = ((File) value).toPath();
+            currentValue = normalizedPath(path, baseDirPath);
+        } else if (value instanceof Path) {
+            currentValue = normalizedPath(((Path) value), baseDirPath);
+        } else if (value != null && value.getClass().isArray()) {
+            currentValue = ArrayUtils.toString(value);
+        } else {
+            currentValue = String.valueOf(value);
+        }
+        return currentValue;
+    }
+
+    /**
+     * Best effort to normalize paths from Mojo fields.
+     * - all absolute paths under project root to be relativized for portability
+     * - redundant '..' and '.' to be removed to have consistent views on all paths
+     * - all relative paths are considered portable and should not be touched
+     * - absolute paths outside of project directory could not be deterministically
+     * relativized and not touched
+     */
+    private static String normalizedPath(Path path, Path baseDirPath) {
+        boolean isProjectSubdir = path.isAbsolute() && path.startsWith(baseDirPath);
+        if (LOGGER.isDebugEnabled()) {
+            LOGGER.debug(
+                    "normalizedPath isProjectSubdir {} path '{}' - baseDirPath '{}', path.isAbsolute() {}, path.startsWith(baseDirPath) {}",
+                    isProjectSubdir,
+                    path,
+                    baseDirPath,
+                    path.isAbsolute(),
+                    path.startsWith(baseDirPath));
+        }
+        Path preparedPath = isProjectSubdir ? baseDirPath.relativize(path) : path;
+        String normalizedPath = preparedPath.normalize().toString();
+        if (LOGGER.isDebugEnabled()) {
+            LOGGER.debug("normalizedPath '{}' - {} return {}", path, baseDirPath, normalizedPath);
+        }
+        return normalizedPath;
+    }
 }