From 2be2c4546ae4300fd46cd106f4e9085e28eb205c Mon Sep 17 00:00:00 2001 From: lutovich Date: Thu, 31 May 2018 23:20:10 +0200 Subject: [PATCH] Fix config file as URL on Windows Configuration for various formatter steps can be located in separate files. Such files might live in the local filesystem and be identified as simple paths. They can also be remote files identified by a URL. Both types of files are copied to the output directory using Maven's `ResourceManager` which can handle both local and remote files. Previously, plugin tried to deduce the output file name based on the configured string. This was done using `FileUtils#filename()` method that expects to only work with system file separators. Issue occurred on Windows when URL was specified. Path separator on Windows is `\` and in URL it is `/`. Code wasn't able to extract the last path of the URL that denotes the file name. Instead, it treated the whole URL as a file name. URLs contain chars invalid for Windows paths and an exception was thrown on attempt to create the file. This commit fixes the problem by making code use predefined name pattern for all resolved resources. Name of the output file will not be guessed based on the configured path or URL. It will just be unique and have 'spotless-resource-' prefix. --- plugin-maven/CHANGES.md | 2 ++ .../diffplug/spotless/maven/FileLocator.java | 9 +++--- .../spotless/maven/FileLocatorTest.java | 30 ++++++++++++++----- 3 files changed, 29 insertions(+), 12 deletions(-) diff --git a/plugin-maven/CHANGES.md b/plugin-maven/CHANGES.md index fcdbaf2d39..b4d630cf5f 100644 --- a/plugin-maven/CHANGES.md +++ b/plugin-maven/CHANGES.md @@ -2,6 +2,8 @@ ### Version 1.10.0-SNAPSHOT - TBD ([javadoc](https://diffplug.github.io/spotless/javadoc/spotless-maven-plugin/snapshot/), [snapshot](https://oss.sonatype.org/content/repositories/snapshots/com/diffplug/spotless/spotless-maven-plugin/)) +* Fixed a bug in configuration file resolution on Windows when file is denoted by a URL. ([#254](https://github.com/diffplug/spotless/pull/254)) + ### Version 1.0.0.BETA5 - May 14th 2018 ([javadoc](https://diffplug.github.io/spotless/javadoc/spotless-maven-plugin/1.0.0.BETA5/), [jcenter](https://bintray.com/diffplug/opensource/spotless-maven-plugin/1.0.0.BETA5)) * Fixed a bug in `LicenseHeaderStep` which caused an exception with some malformed date-aware licenses. ([#222](https://github.com/diffplug/spotless/pull/222)) diff --git a/plugin-maven/src/main/java/com/diffplug/spotless/maven/FileLocator.java b/plugin-maven/src/main/java/com/diffplug/spotless/maven/FileLocator.java index 237d815ed0..0d28c68ee1 100644 --- a/plugin-maven/src/main/java/com/diffplug/spotless/maven/FileLocator.java +++ b/plugin-maven/src/main/java/com/diffplug/spotless/maven/FileLocator.java @@ -15,9 +15,10 @@ */ package com.diffplug.spotless.maven; -import static com.diffplug.common.base.Strings.*; +import static com.diffplug.common.base.Strings.isNullOrEmpty; import java.io.File; +import java.util.UUID; import org.codehaus.plexus.resource.ResourceManager; import org.codehaus.plexus.resource.loader.FileResourceCreationException; @@ -26,6 +27,8 @@ public class FileLocator { + static final String TMP_RESOURCE_FILE_PREFIX = "spotless-resource-"; + private final ResourceManager resourceManager; public FileLocator(ResourceManager resourceManager) { @@ -48,9 +51,7 @@ public File locateFile(String path) { } private static String tmpOutputFileName(String path) { - String nameWithExtension = FileUtils.filename(path); String extension = FileUtils.extension(path); - String name = nameWithExtension.replace('.' + extension, ""); - return name + '-' + System.currentTimeMillis() + '.' + extension; + return TMP_RESOURCE_FILE_PREFIX + UUID.randomUUID() + '.' + extension; } } diff --git a/plugin-maven/src/test/java/com/diffplug/spotless/maven/FileLocatorTest.java b/plugin-maven/src/test/java/com/diffplug/spotless/maven/FileLocatorTest.java index d2a54e8c0c..c2c1dfe380 100644 --- a/plugin-maven/src/test/java/com/diffplug/spotless/maven/FileLocatorTest.java +++ b/plugin-maven/src/test/java/com/diffplug/spotless/maven/FileLocatorTest.java @@ -15,14 +15,13 @@ */ package com.diffplug.spotless.maven; +import static com.diffplug.spotless.maven.FileLocator.TMP_RESOURCE_FILE_PREFIX; import static org.assertj.core.api.Assertions.assertThat; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNull; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.eq; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.when; +import static org.mockito.Mockito.*; import java.io.File; import java.nio.file.Paths; @@ -47,17 +46,32 @@ public void locateNull() { } @Test - public void locateValidFile() throws Exception { - String path = Paths.get("tmp", "configs", "my-config.xml").toString(); - File tmpOutputFile = new File("tmp-my-config.xml"); + public void locateXmlFile() throws Exception { + testFileLocator(Paths.get("tmp", "configs", "my-config.xml").toString(), "xml"); + } + + @Test + public void locatePropertiesFile() throws Exception { + testFileLocator(Paths.get("home", "ubuntu", "my-other-config.properties").toString(), "properties"); + } + + @Test + public void locateConfFileWithIncorrectSeparators() throws Exception { + String oppositeSeparator = "/".equals(File.separator) ? "\\" : "/"; + String path = "tmp" + oppositeSeparator + "configs" + oppositeSeparator + "hello.conf"; + + testFileLocator(path, "conf"); + } + + private void testFileLocator(String path, String extension) throws Exception { + File tmpOutputFile = new File("tmp-file"); when(resourceManager.getResourceAsFile(any(), any())).thenReturn(tmpOutputFile); File locatedFile = fileLocator.locateFile(path); - assertEquals(tmpOutputFile, locatedFile); ArgumentCaptor argCaptor = ArgumentCaptor.forClass(String.class); verify(resourceManager).getResourceAsFile(eq(path), argCaptor.capture()); - assertThat(argCaptor.getValue()).startsWith("my-config").endsWith(".xml"); + assertThat(argCaptor.getValue()).startsWith(TMP_RESOURCE_FILE_PREFIX).endsWith('.' + extension); } }