Skip to content

Commit

Permalink
[FIXED JENKINS-31573] New line characters not parsing correctly (#78)
Browse files Browse the repository at this point in the history 
* Updated test to use LinkedHashMap since it expects order to be correct

* Used Java properties to parse content and files instead of custom class.

* Adding back in SortedProperties as a deprecated class
  • Loading branch information
@jones2026 @oleg-nenashev
jones2026 authored and oleg-nenashev committed Sep 18, 2016
1 parent 1955123 commit e64c47d
Show file tree
Hide file tree
Showing 4 changed files with 30 additions and 45 deletions.
45 changes: 13 additions & 32 deletions src/main/java/org/jenkinsci/plugins/envinject/service/PropertiesLoader.java
View file
Expand Up @@ -3,14 +3,14 @@

import hudson.Util;
import org.jenkinsci.lib.envinject.EnvInjectException;
import org.jenkinsci.plugins.envinject.util.SortedProperties;

import java.io.File;
import java.io.IOException;
import java.io.Serializable;
import java.io.StringReader;
import java.util.LinkedHashMap;
import java.util.Map;
import java.util.Properties;

/**
* @author Gregory Boissinot
Expand All @@ -26,29 +26,19 @@ public class PropertiesLoader implements Serializable {
* @throws EnvInjectException
*/
public Map<String, String> getVarsFromPropertiesFile(File propertiesFile, Map<String, String> currentEnvVars) throws EnvInjectException {

if (propertiesFile == null) {
throw new NullPointerException("The properties file object must be set.");
}
if (!propertiesFile.exists()) {
throw new IllegalArgumentException("The properties file object must be exist.");
}

Map<String, String> result = new LinkedHashMap<String, String>();

SortedProperties properties = new SortedProperties();
try {
String fileContent = Util.loadFile(propertiesFile);
String fileContentResolved = Util.replaceMacro(fileContent, currentEnvVars);
fileContentResolved = processPath(fileContentResolved);
properties.load(new StringReader(fileContentResolved));
return getVars(fileContent, currentEnvVars);
} catch (IOException ioe) {
throw new EnvInjectException("Problem occurs on loading content", ioe);
}
for (Map.Entry<Object, Object> entry : properties.entrySet()) {
result.put(processElement(entry.getKey()), processElement(entry.getValue()));
}
return result;
}

/**
Expand All @@ -60,20 +50,21 @@ public Map<String, String> getVarsFromPropertiesFile(File propertiesFile, Map<St
* @throws EnvInjectException
*/
public Map<String, String> getVarsFromPropertiesContent(String content, Map<String, String> currentEnvVars) throws EnvInjectException {

if (content == null) {
throw new NullPointerException("A properties content must be set.");
}
if (content.trim().length() == 0) {
throw new IllegalArgumentException("A properties content must be not empty.");
}

String contentResolved = Util.replaceMacro(content, currentEnvVars);
contentResolved = processPath(contentResolved);
return getVars(content, currentEnvVars);
}

private Map<String, String> getVars(String content, Map<String, String> currentEnvVars) throws EnvInjectException {
Map<String, String> result = new LinkedHashMap<String, String>();
StringReader stringReader = new StringReader(contentResolved);
SortedProperties properties = new SortedProperties();
StringReader stringReader = new StringReader(content);
Properties properties = new Properties();

try {
properties.load(stringReader);
} catch (IOException ioe) {
Expand All @@ -83,27 +74,17 @@ public Map<String, String> getVarsFromPropertiesContent(String content, Map<Stri
}

for (Map.Entry<Object, Object> entry : properties.entrySet()) {
result.put(processElement(entry.getKey()), processElement(entry.getValue()));
result.put(processElement(entry.getKey(), currentEnvVars), processElement(entry.getValue(), currentEnvVars));
}
return result;
}

private String processElement(Object prop) {
if (prop == null) {
private String processElement(Object prop, Map<String, String> currentEnvVars) {
String macroProcessedElement = Util.replaceMacro(String.valueOf(prop), currentEnvVars);
if (macroProcessedElement == null) {
return null;
}

return String.valueOf(prop).trim();
}

private String processPath(String content) {
if (content == null) {
return null;
}

content = content.replace("\\", "\\\\");
return content.replace("\\\\\n", "\\\n");
return macroProcessedElement.trim();
}

}

4 changes: 3 additions & 1 deletion src/main/java/org/jenkinsci/plugins/envinject/util/SortedProperties.java
View file
Expand Up @@ -8,7 +8,9 @@

/**
* @author Gregory Boissinot
* Use java.util.Properties now instead
*/
@Deprecated
public class SortedProperties extends LinkedHashMap<Object, Object> {


Expand Down Expand Up @@ -270,4 +272,4 @@ private String loadConvert(char[] in, int off, int len, char[] convtBuf) {
return new String(out, 0, outLen);
}

}
}
7 changes: 4 additions & 3 deletions src/test/java/org/jenkinsci/plugins/envinject/sevice/PropertiesGetterTest.java
View file
Expand Up @@ -5,6 +5,7 @@

import java.util.Collections;
import java.util.HashMap;
import java.util.LinkedHashMap;
import java.util.Map;

import static org.junit.Assert.*;
Expand Down Expand Up @@ -39,10 +40,10 @@ public void getPropertiesContentOneElement() {

@Test
public void getPropertiesContentThreeElements() {
Map<String, String> entryMap = new HashMap<String, String>();
entryMap.put("key1", "value1");
entryMap.put("key2", "value2");
Map<String, String> entryMap = new LinkedHashMap<String, String>();
entryMap.put("key3", "value3");
entryMap.put("key2", "value2");
entryMap.put("key1", "value1");
String content = propertiesGetter.getPropertiesContentFromMapObject(entryMap);
assertNotNull(content);
assertEquals("key3=value3\nkey2=value2\nkey1=value1", content);
Expand Down
19 changes: 10 additions & 9 deletions src/test/java/org/jenkinsci/plugins/envinject/sevice/PropertiesLoaderTest.java
View file
Expand Up @@ -125,15 +125,16 @@ public void contentWithNewlineInValues() throws Exception {

private void checkWithNewlineInValues(boolean fromFile) throws Exception {
// Create properties file containing backslash-escaped newlines
String content = "KEY1=line1\\\nline2\nKEY2= line1 \\\n line2 \nKEY3=line1\\\n\\\nline3";
String content = "KEY1=line1\\nline2\nKEY2= line3 \\n\\\n line4 \nKEY3=line5\\\n\\\nline6\nKEY4=line7\\\nline8\\n\\nline9";
Map<String, String> gatherVars = gatherEnvVars(fromFile, content, new HashMap<String, String>());
assertNotNull(gatherVars);
assertEquals(3, gatherVars.size());
assertEquals(4, gatherVars.size());

// Values should be trimmed at start & end, otherwise whitespace & newlines should be kept
assertEquals("line1\nline2", gatherVars.get("KEY1"));
assertEquals("line1 \nline2", gatherVars.get("KEY2"));
assertEquals("line1\n\nline3", gatherVars.get("KEY3"));
assertEquals("line3 \nline4", gatherVars.get("KEY2"));
assertEquals("line5line6", gatherVars.get("KEY3"));
assertEquals("line7line8\n\nline9", gatherVars.get("KEY4"));
}

@Test
Expand All @@ -147,17 +148,17 @@ public void contentWithVarsToResolve() throws Exception {
}

private void checkWithVarsToResolve(boolean fromFile) throws Exception {
String content = "KEY1 =${VAR1_TO_RESOLVE}\nKEY2=VALUE2\nKEY3=${VAR3_TO_RESOLVE}\\otherContent";
String content = "KEY1 =${VAR1_TO_RESOLVE}\nKEY2=https\\://github.com\nKEY3=${VAR3_TO_RESOLVE}\\\\otherContent";
Map<String, String> currentEnvVars = new HashMap<String, String>();
currentEnvVars.put("VAR1_TO_RESOLVE", "NEW_VALUE1");
currentEnvVars.put("VAR3_TO_RESOLVE", "NEW_VALUE3");
currentEnvVars.put("VAR1_TO_RESOLVE", "NEW_VALUE1 ");
currentEnvVars.put("VAR3_TO_RESOLVE", "C:\\Bench");

Map<String, String> gatherVars = gatherEnvVars(fromFile, content, currentEnvVars);
assertNotNull(gatherVars);
assertEquals(3, gatherVars.size());
assertEquals("NEW_VALUE1", gatherVars.get("KEY1"));
assertEquals("VALUE2", gatherVars.get("KEY2"));
assertEquals("NEW_VALUE3\\otherContent", gatherVars.get("KEY3"));
assertEquals("https://github.com", gatherVars.get("KEY2"));
assertEquals("C:\\Bench\\otherContent", gatherVars.get("KEY3"));
}

private Map<String, String> gatherEnvVars(boolean fromFile, String content2Load, Map<String, String> currentEnvVars) throws Exception {
Expand Down

0 comments on commit e64c47d

Please sign in to comment.