From d0c259797a80676c242af57754fac0083b280636 Mon Sep 17 00:00:00 2001 From: Wilco Greven Date: Fri, 13 Nov 2015 20:13:13 +0100 Subject: [PATCH 1/2] [JENKINS-23666] Prefer property files in workspace over property file in current working directory. --- .../service/PropertiesVariablesRetriever.java | 8 +------ .../envinject/EnvInjectBuilderTest.java | 24 +++++++++++++++++++ 2 files changed, 25 insertions(+), 7 deletions(-) diff --git a/src/main/java/org/jenkinsci/plugins/envinject/service/PropertiesVariablesRetriever.java b/src/main/java/org/jenkinsci/plugins/envinject/service/PropertiesVariablesRetriever.java index 1400d8e5..47648197 100644 --- a/src/main/java/org/jenkinsci/plugins/envinject/service/PropertiesVariablesRetriever.java +++ b/src/main/java/org/jenkinsci/plugins/envinject/service/PropertiesVariablesRetriever.java @@ -71,13 +71,7 @@ public Map invoke(File base, VirtualChannel channel) throws IOEx } private File getFile(File base, String scriptFilePath) { - - File file = new File(scriptFilePath); - if (file.exists()) { - return file; - } - - file = new File(base, scriptFilePath); + File file = new File(new FilePath(base).child(scriptFilePath).getRemote()); return file.exists() ? file : null; } diff --git a/src/test/java/org/jenkinsci/plugins/envinject/EnvInjectBuilderTest.java b/src/test/java/org/jenkinsci/plugins/envinject/EnvInjectBuilderTest.java index fa4cf606..f1f378c4 100644 --- a/src/test/java/org/jenkinsci/plugins/envinject/EnvInjectBuilderTest.java +++ b/src/test/java/org/jenkinsci/plugins/envinject/EnvInjectBuilderTest.java @@ -24,7 +24,9 @@ package org.jenkinsci.plugins.envinject; import static org.junit.Assert.assertEquals; + import hudson.EnvVars; +import hudson.FilePath; import hudson.model.FreeStyleProject; import org.junit.Before; @@ -34,6 +36,8 @@ import org.jvnet.hudson.test.JenkinsRule; import org.jvnet.hudson.test.SingleFileSCM; +import java.io.File; + public class EnvInjectBuilderTest { @Rule public JenkinsRule j = new JenkinsRule(); @@ -59,6 +63,26 @@ public class EnvInjectBuilderTest { assertEquals("new", buildEnvVars().get("VAR")); } + @Test public void propertyFileInWorkspaceShouldTakePrecedenceOverAbsolutePropertyFile() throws Exception { + File propertyFile = File.createTempFile("test", "properties", new File(System.getProperty("user.dir"))); + propertyFile.deleteOnExit(); + new FilePath(propertyFile).write("SOURCE=user.dir", "UTF-8"); + p.setScm(new SingleFileSCM(propertyFile.getName(), "SOURCE=workspace")); + p.getBuildersList().add(new EnvInjectBuilder(propertyFile.getName(), null)); + + assertEquals("workspace", buildEnvVars().get("SOURCE")); + } + + @Test public void injectPropertiesUsingAbsoluteFileName() throws Exception { + File propertyFile = File.createTempFile("test", "properties"); + propertyFile.deleteOnExit(); + new FilePath(propertyFile).write("VAR=test", "UTF-8"); + + p.getBuildersList().add(new EnvInjectBuilder(propertyFile.getAbsolutePath(), null)); + + assertEquals("test", buildEnvVars().get("VAR")); + } + private EnvVars buildEnvVars() throws Exception { CaptureEnvironmentBuilder capture = new CaptureEnvironmentBuilder(); p.getBuildersList().add(capture); From 47ea88daaf2e0f792981482399f7e6236bd71dc2 Mon Sep 17 00:00:00 2001 From: Wilco Greven Date: Fri, 13 Nov 2015 21:26:40 +0100 Subject: [PATCH 2/2] Don't do everything inside the FileCallable, only load the property file. --- .../envinject/service/EnvInjectEnvVars.java | 26 ++------ .../service/PropertiesVariablesRetriever.java | 63 ++++++++++--------- 2 files changed, 39 insertions(+), 50 deletions(-) diff --git a/src/main/java/org/jenkinsci/plugins/envinject/service/EnvInjectEnvVars.java b/src/main/java/org/jenkinsci/plugins/envinject/service/EnvInjectEnvVars.java index f67d4b05..ff8ffbbf 100644 --- a/src/main/java/org/jenkinsci/plugins/envinject/service/EnvInjectEnvVars.java +++ b/src/main/java/org/jenkinsci/plugins/envinject/service/EnvInjectEnvVars.java @@ -43,19 +43,11 @@ public Map getEnvVarsPropertiesJobProperty(FilePath rootPath, Map propertiesContent, Map infraEnvVarsMaster, Map infraEnvVarsNode) throws EnvInjectException { - final Map resultMap = new LinkedHashMap(); - try { - if (loadFilesFromMaster) { - resultMap.putAll(Hudson.getInstance().getRootPath().act(new PropertiesVariablesRetriever(propertiesFilePath, propertiesContent, infraEnvVarsMaster, logger))); - } else { - resultMap.putAll(rootPath.act(new PropertiesVariablesRetriever(propertiesFilePath, propertiesContent, infraEnvVarsNode, logger))); - } - } catch (IOException e) { - throw new EnvInjectException(e); - } catch (InterruptedException e) { - throw new EnvInjectException(e); + if (loadFilesFromMaster) { + return new PropertiesVariablesRetriever(Hudson.getInstance().getRootPath(), propertiesFilePath, propertiesContent, infraEnvVarsMaster, logger).retrieve(); + } else { + return new PropertiesVariablesRetriever(rootPath, propertiesFilePath, propertiesContent, infraEnvVarsNode, logger).retrieve(); } - return resultMap; } @Nonnull @@ -64,15 +56,7 @@ public Map getEnvVarsFileProperty(@Nonnull FilePath rootPath, String propertiesFilePath, Map propertiesContent, Map currentEnvVars) throws EnvInjectException { - Map resultMap = new LinkedHashMap(); - try { - resultMap.putAll(rootPath.act(new PropertiesVariablesRetriever(propertiesFilePath, propertiesContent, currentEnvVars, logger))); - } catch (IOException e) { - throw new EnvInjectException(e); - } catch (InterruptedException e) { - throw new EnvInjectException(e); - } - return resultMap; + return new PropertiesVariablesRetriever(rootPath, propertiesFilePath, propertiesContent, currentEnvVars, logger).retrieve(); } public int executeScript(boolean loadFromMaster, diff --git a/src/main/java/org/jenkinsci/plugins/envinject/service/PropertiesVariablesRetriever.java b/src/main/java/org/jenkinsci/plugins/envinject/service/PropertiesVariablesRetriever.java index 47648197..043403a5 100644 --- a/src/main/java/org/jenkinsci/plugins/envinject/service/PropertiesVariablesRetriever.java +++ b/src/main/java/org/jenkinsci/plugins/envinject/service/PropertiesVariablesRetriever.java @@ -14,7 +14,9 @@ /** * @author Gregory Boissinot */ -public class PropertiesVariablesRetriever implements FilePath.FileCallable> { +public class PropertiesVariablesRetriever { + + private FilePath basePath; private String propertiesFilePath; @@ -24,55 +26,58 @@ public class PropertiesVariablesRetriever implements FilePath.FileCallable propertiesContent, Map currentEnvVars, EnvInjectLogger logger) { + public PropertiesVariablesRetriever(FilePath basePath, String propertiesFilePath, Map propertiesContent, Map currentEnvVars, EnvInjectLogger logger) { + this.basePath = basePath; this.propertiesFilePath = propertiesFilePath; this.propertiesContent = propertiesContent; this.currentEnvVars = currentEnvVars; this.logger = logger; } - public Map invoke(File base, VirtualChannel channel) throws IOException, InterruptedException { + public Map retrieve() throws EnvInjectException { Map result = new LinkedHashMap(); - try { - - PropertiesLoader loader = new PropertiesLoader(); + //Add the properties file + if (propertiesFilePath != null) { + String propertiesFilePathResolved = Util.replaceMacro(propertiesFilePath, currentEnvVars); + propertiesFilePathResolved = propertiesFilePathResolved.replace("\\", "/"); + FilePath remotePropertiesFilePath = new FilePath(basePath, propertiesFilePathResolved); - //Add the properties file - if (propertiesFilePath != null) { - String propertiesFilePathResolved = Util.replaceMacro(propertiesFilePath, currentEnvVars); - propertiesFilePathResolved = propertiesFilePathResolved.replace("\\", "/"); - File propertiesFile = getFile(base, propertiesFilePathResolved); - if (propertiesFile == null) { + try { + if (!remotePropertiesFilePath.exists()) { String message = String.format("The given properties file path '%s' doesn't exist.", propertiesFilePathResolved); logger.error(message); String patternMessage = String.format("Missing file path was resolved from pattern '%s' .", propertiesFilePath); logger.error(patternMessage); throw new EnvInjectException(message); } - logger.info(String.format("Injecting as environment variables the properties file path '%s'", propertiesFilePathResolved)); - result.putAll(loader.getVarsFromPropertiesFile(propertiesFile, currentEnvVars)); - logger.info("Variables injected successfully."); - } - //Add the properties content - if (propertiesContent != null) { - PropertiesGetter propertiesGetter = new PropertiesGetter(); - logger.info(String.format("Injecting as environment variables the properties content %n%s%n", propertiesGetter.getPropertiesContentFromMapObject(propertiesContent))); - result.putAll(propertiesContent); + logger.info(String.format("Injecting as environment variables the properties file path '%s'", propertiesFilePathResolved)); + result.putAll(remotePropertiesFilePath.act(new FilePath.FileCallable>() { + public Map invoke(File propertiesFile, VirtualChannel virtualChannel) throws IOException, InterruptedException { + try { + return new PropertiesLoader().getVarsFromPropertiesFile(propertiesFile, currentEnvVars); + } catch (EnvInjectException envEx) { + throw new IOException(envEx.getMessage()); + } + } + })); logger.info("Variables injected successfully."); + } catch (IOException e) { + throw new EnvInjectException(e); + } catch (InterruptedException e) { + throw new EnvInjectException(e); } + } - } catch (EnvInjectException envEx) { - throw new IOException(envEx.getMessage()); + //Add the properties content + if (propertiesContent != null) { + PropertiesGetter propertiesGetter = new PropertiesGetter(); + logger.info(String.format("Injecting as environment variables the properties content %n%s%n", propertiesGetter.getPropertiesContentFromMapObject(propertiesContent))); + result.putAll(propertiesContent); + logger.info("Variables injected successfully."); } return result; } - - private File getFile(File base, String scriptFilePath) { - File file = new File(new FilePath(base).child(scriptFilePath).getRemote()); - return file.exists() ? file : null; - } - }