Skip to content

Commit

Permalink
bug :: preserve original behavior for getStringParameter (#1034)
Browse files Browse the repository at this point in the history
  • Loading branch information
@dev-mlb
dev-mlb authored Dec 31, 2024
1 parent ace38d3 commit 3c93fee
Show file tree
Hide file tree
Showing 6 changed files with 297 additions and 10 deletions.
2 changes: 1 addition & 1 deletion src/main/java/emissary/core/BaseDataObject.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -972,7 +972,7 @@ public String getParameterAsString(final String key) {
if (obj.size() > 1) {
logger.warn("Multiple values for parameter, returning the first - parameter:{}, number of values:{}", key, obj.size());
}
return StringUtils.trimToNull(obj.stream().findFirst().orElse(null));
return obj.stream().findFirst().orElse(null);
}

/**
Expand Down
32 changes: 27 additions & 5 deletions src/main/java/emissary/core/IBaseDataObject.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
import emissary.directory.DirectoryEntry;

import org.apache.commons.collections4.CollectionUtils;
import org.apache.commons.lang3.StringUtils;

import java.io.IOException;
import java.io.InputStream;
Expand All @@ -14,6 +13,7 @@
import java.util.Collections;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.Set;
import java.util.UUID;
import java.util.stream.Collectors;
Expand Down Expand Up @@ -358,7 +358,7 @@ enum MergePolicy {
*/
@Deprecated
default String getStringParameter(final String key) {
return getParameterAsConcatString(key);
return getStringParameter(key, DEFAULT_PARAM_SEPARATOR);
}

/**
Expand All @@ -370,8 +370,27 @@ default String getStringParameter(final String key) {
* @deprecated use {@link #getParameterAsConcatString(String, String)}
*/
@Deprecated
@Nullable
default String getStringParameter(final String key, final String sep) {
return getParameterAsConcatString(key, sep);
final List<Object> obj = getParameter(key);
if (obj == null) {
return null;
} else if (obj.isEmpty()) {
return null;
} else if ((obj.size() == 1) && (obj.get(0) instanceof String)) {
return (String) obj.get(0);
} else if ((obj.size() == 1) && (obj.get(0) == null)) {
return null;
} else {
final StringBuilder sb = new StringBuilder();
for (final Object item : obj) {
if (sb.length() > 0) {
sb.append(sep);
}
sb.append(item);
}
return sb.toString();
}
}

/**
Expand Down Expand Up @@ -418,8 +437,11 @@ default String getParameterAsConcatString(final String key) {
*/
@Nullable
default String getParameterAsConcatString(final String key, final String sep) {
final var strParameter = String.join(sep, getParameterAsStrings(key));
return StringUtils.isBlank(strParameter) ? null : strParameter;
Collection<String> strings = getParameterAsStrings(key);
if (strings.stream().anyMatch(Objects::nonNull)) {
return String.join(sep, strings);
}
return null;
}

/**
Expand Down
73 changes: 69 additions & 4 deletions src/test/java/emissary/core/BaseDataObjectTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -505,12 +505,24 @@ void testSetParameters() {
void testStringParameterOnNonStringValue() {
this.b.putParameter("A", 1L);
assertEquals("1", this.b.getStringParameter("A"), "Non-string parameters must call toString method");
assertEquals("1", this.b.getParameterAsConcatString("A"), "Non-string parameters must call toString method");
assertEquals("1", this.b.getParameterAsString("A"), "Non-string parameters must call toString method");
}

@Test
void testStringParameterOnNullValue() {
this.b.putParameter("A", null);
assertNull(this.b.getStringParameter("A"), "Null parameter must be returned as null");
assertNull(this.b.getParameterAsConcatString("A"), "Null parameter must be returned as null");
assertNull(this.b.getParameterAsString("A"), "Null parameter must be returned as null");
}

@Test
void testStringParameterOnEmptyValue() {
this.b.putParameter("A", "");
assertEquals("", this.b.getStringParameter("A"), "Empty parameter must be returned as empty string");
assertEquals("", this.b.getParameterAsConcatString("A"), "Empty parameter must be returned as empty string");
assertEquals("", this.b.getParameterAsString("A"), "Empty parameter must be returned as empty string");
}

@Test
Expand Down Expand Up @@ -795,8 +807,14 @@ void testMergeParameters() {
this.b.mergeParameters(map);

assertEquals("uno", this.b.getStringParameter("ONE"), "When merging parameters previous values must override");
assertEquals("uno", this.b.getParameterAsConcatString("ONE"), "When merging parameters previous values must override");
assertEquals("uno", this.b.getParameterAsString("ONE"), "When merging parameters previous values must override");
assertEquals("deux", this.b.getStringParameter("TWO"), "When merging parameters previous values must override");
assertEquals("deux", this.b.getParameterAsConcatString("TWO"), "When merging parameters previous values must override");
assertEquals("deux", this.b.getParameterAsString("TWO"), "When merging parameters previous values must override");
assertEquals("tres", this.b.getStringParameter("THREE"), "When merging parameters new keys must be stored");
assertEquals("tres", this.b.getParameterAsConcatString("THREE"), "When merging parameters new keys must be stored");
assertEquals("tres", this.b.getParameterAsString("THREE"), "When merging parameters new keys must be stored");
}

@Test
Expand All @@ -813,8 +831,14 @@ void testPutParametersWithPolicy() {
this.b.putParameters(map, IBaseDataObject.MergePolicy.KEEP_ALL);

assertEquals("uno;uno", this.b.getStringParameter("ONE"), "When specifying KEEP_ALL values must all stay");
assertEquals("uno;uno", this.b.getParameterAsConcatString("ONE"), "When specifying KEEP_ALL values must all stay");
assertEquals("uno", this.b.getParameterAsString("ONE"), "When specifying KEEP_ALL first value should be retained");
assertEquals("deux;dos", this.b.getStringParameter("TWO"), "When specifying KEEP_ALL values must all stay");
assertEquals("deux;dos", this.b.getParameterAsConcatString("TWO"), "When specifying KEEP_ALL values must all stay");
assertEquals("deux", this.b.getParameterAsString("TWO"), "When specifying KEEP_ALL first value should be retained");
assertEquals("tres", this.b.getStringParameter("THREE"), "When specifying KEEP_ALL new keys must be stored");
assertEquals("tres", this.b.getParameterAsConcatString("THREE"), "When specifying KEEP_ALL new keys must be stored");
assertEquals("tres", this.b.getParameterAsString("THREE"), "When specifying KEEP_ALL first value should be retained");
}

@Test
Expand Down Expand Up @@ -844,15 +868,25 @@ void testPutParametersWithMultimapAsMap() {
void testParameters() {
this.b.putParameter("ME", "YOU");
assertEquals("YOU", this.b.getStringParameter("ME"), "Gotten parameter");
assertEquals("YOU", this.b.getParameterAsConcatString("ME"), "Gotten parameter");
assertEquals("YOU", this.b.getParameterAsString("ME"), "Gotten parameter");
final Map<String, Object> map = new HashMap<>();
map.put("ONE", "uno");
map.put("TWO", "dos");
map.put("THREE", "tres");
this.b.putParameters(map);
assertEquals("uno", this.b.getStringParameter("ONE"), "Map put parameter gotten");
assertEquals("uno", this.b.getParameterAsConcatString("ONE"), "Map put parameter gotten");
assertEquals("uno", this.b.getParameterAsString("ONE"), "Map put parameter gotten");
assertEquals("dos", this.b.getStringParameter("TWO"), "Map put parameter gotten");
assertEquals("dos", this.b.getParameterAsConcatString("TWO"), "Map put parameter gotten");
assertEquals("dos", this.b.getParameterAsString("TWO"), "Map put parameter gotten");
assertEquals("tres", this.b.getStringParameter("THREE"), "Map put parameter gotten");
assertEquals("tres", this.b.getParameterAsConcatString("THREE"), "Map put parameter gotten");
assertEquals("tres", this.b.getParameterAsString("THREE"), "Map put parameter gotten");
assertEquals("YOU", this.b.getStringParameter("ME"), "Gotten parameter");
assertEquals("YOU", this.b.getParameterAsConcatString("ME"), "Gotten parameter");
assertEquals("YOU", this.b.getParameterAsString("ME"), "Gotten parameter");

// Deletes
this.b.deleteParameter("THREE");
Expand All @@ -861,6 +895,8 @@ void testParameters() {
// Overwrite
this.b.putParameter("ME", "THEM");
assertEquals("THEM", this.b.getStringParameter("ME"), "Gotten parameter");
assertEquals("THEM", this.b.getParameterAsConcatString("ME"), "Gotten parameter");
assertEquals("THEM", this.b.getParameterAsString("ME"), "Gotten parameter");

// Clear
this.b.clearParameters();
Expand Down Expand Up @@ -895,14 +931,18 @@ void testAppendDuplicateParameters() {
this.b.appendParameter("YO", "GABBA");
this.b.appendParameter("YO", "GABBA");
assertEquals("GABBA;GABBA", this.b.getStringParameter("YO"), "Appended duplicate parameters should be preserved");
assertEquals("GABBA;GABBA", this.b.getParameterAsConcatString("YO"), "Appended duplicate parameters should be preserved");
assertEquals("GABBA", this.b.getParameterAsString("YO"), "Appended duplicate parameters first value should be preserved");
assertTrue(this.b.hasParameter("YO"), "HasParameter should be true");
}

@Test
void testAppendUniqueParameters() {
this.b.appendUniqueParameter("YO", "GABBA");
this.b.appendUniqueParameter("YO", "GABBA");
assertEquals("GABBA", this.b.getStringParameter("YO"), "Appended unique parameters should be collapsed");
assertEquals("GABBA", this.b.getStringParameter("YO"), "Appended unique parameters should be collapsed");
assertEquals("GABBA", this.b.getParameterAsConcatString("YO"), "Appended unique parameters should be collapsed");
assertEquals("GABBA", this.b.getParameterAsString("YO"), "Appended unique parameters should be collapsed");
assertTrue(this.b.hasParameter("YO"), "HasParameter should be true");
}

Expand All @@ -920,6 +960,12 @@ void testParametersWithMixtureOfSingleValuesAndLists() {
assertEquals("FOO1;FOO2;FOO3;FOO4",
this.b.getStringParameter("FOO"),
"Returned string should be combination of initial list and added value");
assertEquals("FOO1;FOO2;FOO3;FOO4",
this.b.getParameterAsConcatString("FOO"),
"Returned string should be combination of initial list and added value");
assertEquals("FOO1",
this.b.getParameterAsString("FOO"),
"Returned string should be first value from combination of initial list and added value");
}

@Test
Expand All @@ -934,6 +980,10 @@ void testParametersWithMixtureOfSingleValuesAndSets() {
assertEquals(3, this.b.getParameter("FOO").size(), "Returned list size should match what was put in");
this.b.appendParameter("FOO", "FOO4");
assertEquals("FOO1;FOO2;FOO3;FOO4", this.b.getStringParameter("FOO"), "Returned string should be combination of initial set and added value");
assertEquals("FOO1;FOO2;FOO3;FOO4", this.b.getParameterAsConcatString("FOO"),
"Returned string should be combination of initial set and added value");
assertEquals("FOO1", this.b.getParameterAsString("FOO"),
"Returned string should be first value from combination of initial set and added value");
}

@Test
Expand Down Expand Up @@ -965,27 +1015,35 @@ void testAppendParameter() {
this.b.putParameter("ME", "YOU");
this.b.appendParameter("ME", "FOO");
assertEquals("YOU;FOO", this.b.getStringParameter("ME"), "Appended parameter value");
assertEquals("YOU;FOO", this.b.getParameterAsConcatString("ME"), "Appended parameter value");
assertEquals("YOU", this.b.getParameterAsString("ME"), "Appended parameter value");
}

@Test
void testAppendParameterIterables() {
this.b.putParameter("ME", "YOU");
this.b.appendParameter("ME", Arrays.asList("FOO", "BAR", "BAZ"));
assertEquals("YOU;FOO;BAR;BAZ", this.b.getStringParameter("ME"), "Appended parameter value");
assertEquals("YOU;FOO;BAR;BAZ", this.b.getParameterAsConcatString("ME"), "Appended parameter value");
assertEquals("YOU", this.b.getParameterAsString("ME"), "First value from appended parameter value");

final Set<String> s = new TreeSet<>();
s.add("ZAB");
s.add("RAB");
s.add("OOF");
this.b.appendParameter("ME", s);

assertEquals("YOU;FOO;BAR;BAZ;OOF;RAB;ZAB", this.b.getStringParameter("ME"), "Appended set paramter value");
assertEquals("YOU;FOO;BAR;BAZ;OOF;RAB;ZAB", this.b.getStringParameter("ME"), "Appended set parameter value");
assertEquals("YOU;FOO;BAR;BAZ;OOF;RAB;ZAB", this.b.getParameterAsConcatString("ME"), "Appended set parameter value");
assertEquals("YOU", this.b.getParameterAsString("ME"), "First value from appended set parameter value");
}

@Test
void testAppendParameterOntoEmpty() {
this.b.appendParameter("ME", "FOO");
assertEquals("FOO", this.b.getStringParameter("ME"), "Appended parameter value");
assertEquals("FOO", this.b.getParameterAsConcatString("ME"), "Appended parameter value");
assertEquals("FOO", this.b.getParameterAsString("ME"), "First value from appended parameter value");
}


Expand Down Expand Up @@ -1346,6 +1404,7 @@ void testGetParameterAsString() throws IOException {
this.b.appendParameter("A", "THREE");
assertEquals("1", this.b.getParameterAsString("A"));
assertEquals("1;TWO;THREE", this.b.getParameterAsConcatString("A"));
assertEquals("1;TWO;THREE", this.b.getStringParameter("A"));
LogbackTester.SimplifiedLogEvent logEvent = new LogbackTester.SimplifiedLogEvent(Level.WARN,
"Multiple values for parameter, returning the first - parameter:A, number of values:3", null);
logbackTester.checkLogList(Collections.singletonList(logEvent));
Expand All @@ -1354,27 +1413,33 @@ void testGetParameterAsString() throws IOException {
this.b.putParameter("A", 2L);
assertEquals("2", this.b.getParameterAsString("A"));
assertEquals("2", this.b.getParameterAsConcatString("A"));
assertEquals("2", this.b.getStringParameter("A"));

this.b.putParameter("A", "THREE");
assertEquals("THREE", this.b.getParameterAsString("A"));
assertEquals("THREE", this.b.getParameterAsConcatString("A"));
assertEquals("THREE", this.b.getStringParameter("A"));

this.b.putParameter("A", null);
assertNull(this.b.getParameterAsString("A"));
assertNull(this.b.getParameterAsConcatString("A"));
assertNull(this.b.getStringParameter("A"));

this.b.putParameter("A", "");
assertNull(this.b.getParameterAsString("A"));
assertNull(this.b.getParameterAsConcatString("A"));
assertEquals("", this.b.getParameterAsString("A"));
assertEquals("", this.b.getParameterAsConcatString("A"));
assertEquals("", this.b.getStringParameter("A"));

assertNull(this.b.getParameterAsString("DNE"));
assertNull(this.b.getParameterAsConcatString("DNE"));
assertNull(this.b.getStringParameter("DNE"));

this.b.putParameter("A", null);
this.b.appendParameter("A", "FOUR");
this.b.appendParameter("A", " ");
assertEquals("null", this.b.getParameterAsString("A"));
assertEquals("null;FOUR; ", this.b.getParameterAsConcatString("A"));
assertEquals("null;FOUR; ", this.b.getStringParameter("A"));
}

}
Loading

0 comments on commit 3c93fee

Please sign in to comment.