Skip to content

Commit 2daa835

Browse files
pacmano1pacmano1
committed
Return Optional from metadata column validator
Review feedback: as a new public method, findUnknownColumn should not return a nullable String. It now returns Optional<String>, the javadoc gained @param/@return tags, and the stale class doc describing the removed REST-layer check was corrected. Signed-off-by: Finnegan's Owner <44065187+pacmano1@users.noreply.github.com>
1 parent 8fe38a2 commit 2daa835

3 files changed

Lines changed: 37 additions & 28 deletions

File tree

‎server/src/main/java/com/mirth/connect/server/controllers/DonkeyMessageController.java‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
import java.util.Map;
2121
import java.util.Map.Entry;
2222
import java.util.NavigableMap;
23+
import java.util.Optional;
2324
import java.util.Set;
2425
import java.util.TreeMap;
2526
import java.util.TreeSet;
@@ -690,9 +691,9 @@ private List<MessageSearchResult> searchMessages(MessageFilter filter, String ch
690691
* regardless of the resulting status, because the throw happens before any query runs.
691692
*/
692693
private void validateMetaDataColumns(String channelId, MessageFilter filter) {
693-
String unknownColumn = MetaDataColumnValidator.findUnknownColumn(filter, () -> ControllerFactory.getFactory().createChannelController().getMetaDataColumns(channelId));
694-
if (unknownColumn != null) {
695-
throw new MetaDataColumnException(channelId, unknownColumn);
694+
Optional<String> unknownColumn = MetaDataColumnValidator.findUnknownColumn(filter, () -> ControllerFactory.getFactory().createChannelController().getMetaDataColumns(channelId));
695+
if (unknownColumn.isPresent()) {
696+
throw new MetaDataColumnException(channelId, unknownColumn.get());
696697
}
697698
}
698699

‎server/src/main/java/com/mirth/connect/server/util/MetaDataColumnValidator.java‎

Lines changed: 21 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55

66
import java.util.HashSet;
77
import java.util.List;
8+
import java.util.Optional;
89
import java.util.Set;
910
import java.util.function.Supplier;
1011

@@ -21,34 +22,41 @@
2122
*
2223
* <p>
2324
* This is a pure check: it never throws and never looks anything up. Callers pass in the channel's
24-
* defined columns and decide what to do with an unknown column - the REST layer returns 400, the
25-
* controller layer throws as a last-resort backstop for callers that bypass the REST layer.
25+
* defined columns and decide what to do with an unknown column - the controller rejects the search
26+
* by throwing a MetaDataColumnException before any query runs.
2627
* </p>
2728
*/
2829
public final class MetaDataColumnValidator {
2930

3031
private MetaDataColumnValidator() {}
3132

3233
/**
33-
* Returns the first metadata column name referenced by the filter that is not defined on the
34-
* channel, or {@code null} if every referenced column is valid. Column names are matched exactly
35-
* against the channel's (upper-cased) column names; a {@code null} referenced name is treated as
36-
* unknown and returned as the string {@code "null"} so the result stays unambiguous.
34+
* Finds the first metadata column name referenced by the filter that is not defined on the
35+
* channel. Column names are matched exactly against the channel's (upper-cased) column names; a
36+
* {@code null} referenced name is treated as unknown and reported as the string {@code "null"}.
3737
*
3838
* <p>
3939
* The defined columns are supplied lazily and are only requested when the filter actually
4040
* references a custom column, so a search that uses none costs no channel lookup.
4141
* </p>
42+
*
43+
* @param filter the message search filter to check; may be {@code null}, which validates
44+
* trivially
45+
* @param definedColumnsSupplier supplies the channel's defined metadata columns; only invoked
46+
* when the filter references a custom column, and may return {@code null} for a
47+
* channel with no columns
48+
* @return the first unknown column name referenced by the filter, or {@link Optional#empty()}
49+
* if every referenced column is defined on the channel
4250
*/
43-
public static String findUnknownColumn(MessageFilter filter, Supplier<List<MetaDataColumn>> definedColumnsSupplier) {
51+
public static Optional<String> findUnknownColumn(MessageFilter filter, Supplier<List<MetaDataColumn>> definedColumnsSupplier) {
4452
if (filter == null) {
45-
return null;
53+
return Optional.empty();
4654
}
4755

4856
boolean hasMetaDataSearch = CollectionUtils.isNotEmpty(filter.getMetaDataSearch());
4957
boolean hasTextSearchColumns = CollectionUtils.isNotEmpty(filter.getTextSearchMetaDataColumns());
5058
if (!hasMetaDataSearch && !hasTextSearchColumns) {
51-
return null;
59+
return Optional.empty();
5260
}
5361

5462
List<MetaDataColumn> definedColumns = definedColumnsSupplier.get();
@@ -64,22 +72,22 @@ public static String findUnknownColumn(MessageFilter filter, Supplier<List<MetaD
6472
if (hasMetaDataSearch) {
6573
for (MetaDataSearchElement element : filter.getMetaDataSearch()) {
6674
if (element == null) {
67-
return "null";
75+
return Optional.of("null");
6876
}
6977
if (element.getColumnName() == null || !allowedColumns.contains(element.getColumnName())) {
70-
return String.valueOf(element.getColumnName());
78+
return Optional.of(String.valueOf(element.getColumnName()));
7179
}
7280
}
7381
}
7482

7583
if (hasTextSearchColumns) {
7684
for (String columnName : filter.getTextSearchMetaDataColumns()) {
7785
if (columnName == null || !allowedColumns.contains(columnName)) {
78-
return String.valueOf(columnName);
86+
return Optional.of(String.valueOf(columnName));
7987
}
8088
}
8189
}
8290

83-
return null;
91+
return Optional.empty();
8492
}
8593
}

‎server/src/test/java/com/mirth/connect/server/util/MetaDataColumnValidatorTest.java‎

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -5,12 +5,12 @@
55

66
import static org.junit.Assert.assertEquals;
77
import static org.junit.Assert.assertFalse;
8-
import static org.junit.Assert.assertNull;
98
import static org.junit.Assert.assertTrue;
109

1110
import java.util.ArrayList;
1211
import java.util.Arrays;
1312
import java.util.List;
13+
import java.util.Optional;
1414
import java.util.concurrent.atomic.AtomicBoolean;
1515
import java.util.function.Supplier;
1616

@@ -40,15 +40,15 @@ private static Supplier<List<MetaDataColumn>> supplier(List<MetaDataColumn> colu
4040

4141
@Test
4242
public void nullFilterReturnsNull() {
43-
assertNull(MetaDataColumnValidator.findUnknownColumn(null, () -> definedColumns("STATUS")));
43+
assertEquals(Optional.empty(), MetaDataColumnValidator.findUnknownColumn(null, () -> definedColumns("STATUS")));
4444
}
4545

4646
@Test
4747
public void noReferencedColumnsReturnsNullAndSkipsLookup() {
4848
AtomicBoolean invoked = new AtomicBoolean(false);
4949
MessageFilter filter = new MessageFilter();
5050

51-
assertNull(MetaDataColumnValidator.findUnknownColumn(filter, supplier(definedColumns("STATUS"), invoked)));
51+
assertEquals(Optional.empty(), MetaDataColumnValidator.findUnknownColumn(filter, supplier(definedColumns("STATUS"), invoked)));
5252
assertFalse("Channel columns must not be looked up when the filter references none", invoked.get());
5353
}
5454

@@ -58,7 +58,7 @@ public void definedMetaDataSearchColumnReturnsNull() {
5858
MessageFilter filter = new MessageFilter();
5959
filter.setMetaDataSearch(Arrays.asList(new MetaDataSearchElement("STATUS", "EQUAL", "x", false)));
6060

61-
assertNull(MetaDataColumnValidator.findUnknownColumn(filter, supplier(definedColumns("STATUS"), invoked)));
61+
assertEquals(Optional.empty(), MetaDataColumnValidator.findUnknownColumn(filter, supplier(definedColumns("STATUS"), invoked)));
6262
assertTrue("A referenced column must trigger the lookup", invoked.get());
6363
}
6464

@@ -67,47 +67,47 @@ public void unknownMetaDataSearchColumnIsReturned() {
6767
MessageFilter filter = new MessageFilter();
6868
filter.setMetaDataSearch(Arrays.asList(new MetaDataSearchElement("EVIL\" OR '1'='1", "EQUAL", "x", false)));
6969

70-
assertEquals("EVIL\" OR '1'='1", MetaDataColumnValidator.findUnknownColumn(filter, () -> definedColumns("STATUS")));
70+
assertEquals(Optional.of("EVIL\" OR '1'='1"), MetaDataColumnValidator.findUnknownColumn(filter, () -> definedColumns("STATUS")));
7171
}
7272

7373
@Test
7474
public void nonUpperCaseColumnIsReturned() {
7575
MessageFilter filter = new MessageFilter();
7676
filter.setMetaDataSearch(Arrays.asList(new MetaDataSearchElement("status", "EQUAL", "x", false)));
7777

78-
assertEquals("status", MetaDataColumnValidator.findUnknownColumn(filter, () -> definedColumns("STATUS")));
78+
assertEquals(Optional.of("status"), MetaDataColumnValidator.findUnknownColumn(filter, () -> definedColumns("STATUS")));
7979
}
8080

8181
@Test
8282
public void nullColumnNameIsReturnedAsNullString() {
8383
MessageFilter filter = new MessageFilter();
8484
filter.setMetaDataSearch(Arrays.asList(new MetaDataSearchElement(null, "EQUAL", "x", false)));
8585

86-
assertEquals("null", MetaDataColumnValidator.findUnknownColumn(filter, () -> definedColumns("STATUS")));
86+
assertEquals(Optional.of("null"), MetaDataColumnValidator.findUnknownColumn(filter, () -> definedColumns("STATUS")));
8787
}
8888

8989
@Test
9090
public void definedTextSearchColumnReturnsNull() {
9191
MessageFilter filter = new MessageFilter();
9292
filter.setTextSearchMetaDataColumns(new ArrayList<String>(Arrays.asList("STATUS")));
9393

94-
assertNull(MetaDataColumnValidator.findUnknownColumn(filter, () -> definedColumns("STATUS")));
94+
assertEquals(Optional.empty(), MetaDataColumnValidator.findUnknownColumn(filter, () -> definedColumns("STATUS")));
9595
}
9696

9797
@Test
9898
public void unknownTextSearchColumnIsReturned() {
9999
MessageFilter filter = new MessageFilter();
100100
filter.setTextSearchMetaDataColumns(new ArrayList<String>(Arrays.asList("BOGUS")));
101101

102-
assertEquals("BOGUS", MetaDataColumnValidator.findUnknownColumn(filter, () -> definedColumns("STATUS")));
102+
assertEquals(Optional.of("BOGUS"), MetaDataColumnValidator.findUnknownColumn(filter, () -> definedColumns("STATUS")));
103103
}
104104

105105
@Test
106106
public void channelWithNoColumnsRejectsAnyReferencedColumn() {
107107
MessageFilter filter = new MessageFilter();
108108
filter.setMetaDataSearch(Arrays.asList(new MetaDataSearchElement("STATUS", "EQUAL", "x", false)));
109109

110-
assertEquals("STATUS", MetaDataColumnValidator.findUnknownColumn(filter, () -> null));
110+
assertEquals(Optional.of("STATUS"), MetaDataColumnValidator.findUnknownColumn(filter, () -> null));
111111
}
112112

113113
@Test
@@ -118,7 +118,7 @@ public void nullSearchElementIsRejectedNotThrown() {
118118
filter.setMetaDataSearch(elements);
119119

120120
// A null element in the list must be treated as unknown (returned), never an NPE.
121-
assertEquals("null", MetaDataColumnValidator.findUnknownColumn(filter, () -> definedColumns("STATUS")));
121+
assertEquals(Optional.of("null"), MetaDataColumnValidator.findUnknownColumn(filter, () -> definedColumns("STATUS")));
122122
}
123123

124124
@Test
@@ -131,6 +131,6 @@ public void nullDefinedColumnEntryIsIgnoredNotThrown() {
131131
columns.add(new MetaDataColumn("STATUS", MetaDataColumnType.STRING, null));
132132

133133
// A null entry in the channel's columns must be skipped, not cause an NPE; STATUS still validates.
134-
assertNull(MetaDataColumnValidator.findUnknownColumn(filter, () -> columns));
134+
assertEquals(Optional.empty(), MetaDataColumnValidator.findUnknownColumn(filter, () -> columns));
135135
}
136136
}

0 commit comments

Comments
 (0)