Skip to content

Commit fd00b26

Browse files
elliVMelliVM
authored
Handle possible null values in filter fields option (#599)
* account for JSON null values when parsing filter types * better defined exception messages * add null value tests for BloomFilterConfigurationTest * null safe compareTo * rebase and rename package name on new classes * clean up the equals method of ValidBloomFilterConfiguration * fix unit test assert throws exception types and messages
1 parent 028d8d6 commit fd00b26

6 files changed

Lines changed: 296 additions & 28 deletions

File tree

‎src/main/java/com/teragrep/pth_10/steps/teragrep/bloomfilter/BloomFilterConfiguration.java‎

Lines changed: 6 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@
4545
*/
4646
package com.teragrep.pth_10.steps.teragrep.bloomfilter;
4747

48+
import java.util.Comparator;
4849
import java.util.Objects;
4950

5051
public final class BloomFilterConfiguration implements Comparable<BloomFilterConfiguration> {
@@ -85,23 +86,11 @@ public int hashCode() {
8586

8687
@Override
8788
public int compareTo(final BloomFilterConfiguration other) {
88-
if (equals(other)) {
89-
return 0;
90-
}
91-
if (expected.equals(other.expected)) {
92-
// larger fpp results in a smaller filter bit size
93-
if (fpp < other.fpp) {
94-
return 1;
95-
}
96-
else {
97-
return -1;
98-
}
99-
}
100-
else if (expected > other.expected) {
101-
return 1;
102-
}
103-
else {
104-
return -1;
89+
if (other == null) {
90+
throw new IllegalArgumentException("Cannot compare against null");
10591
}
92+
return Comparator
93+
.comparing((BloomFilterConfiguration cnf) -> cnf.expected, Comparator.nullsLast(Long::compareTo))
94+
.thenComparing((BloomFilterConfiguration cnf) -> cnf.fpp, Comparator.nullsLast(Comparator.reverseOrder())).compare(this, other);
10695
}
10796
}

‎src/main/java/com/teragrep/pth_10/steps/teragrep/bloomfilter/FilterTypes.java‎

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -95,9 +95,8 @@ public SortedMap<Long, Double> sortedMap() {
9595
}
9696

9797
for (final BloomFilterConfiguration configuration : filterConfigurationList) {
98-
final Long expectedNumOfItems = configuration.expectedNumOfItems();
99-
final Double falsePositiveProbability = configuration.falsePositiveProbability();
100-
sizesMapFromJson.put(expectedNumOfItems, falsePositiveProbability);
98+
final ValidBloomFilterConfiguration validConfiguration = new ValidBloomFilterConfiguration(configuration);
99+
sizesMapFromJson.put(validConfiguration.expected(), validConfiguration.fpp());
101100
}
102101

103102
final boolean hasDuplicates = new HashSet<>(filterConfigurationList).size() != filterConfigurationList.size();
@@ -123,12 +122,19 @@ private String sizesJsonString() {
123122
final String BLOOM_NUMBER_OF_FIELDS_CONFIG_ITEM = "dpl.pth_06.bloom.db.fields";
124123
if (config.hasPath(BLOOM_NUMBER_OF_FIELDS_CONFIG_ITEM)) {
125124
jsonString = config.getString(BLOOM_NUMBER_OF_FIELDS_CONFIG_ITEM);
126-
if (jsonString == null || jsonString.isEmpty() || "null".equals(jsonString)) {
127-
throw new RuntimeException("Bloom filter size fields was not configured.");
125+
if (jsonString == null || jsonString.isEmpty()) {
126+
throw new IllegalArgumentException("Bloom filter size fields was not configured.");
127+
}
128+
if ("null".equals(jsonString)) {
129+
throw new IllegalArgumentException(
130+
"Option 'dpl.pth_06.bloom.db.fields' expected an JSON object but was 'null'"
131+
);
128132
}
129133
}
130134
else {
131-
throw new RuntimeException("Missing configuration item: '" + BLOOM_NUMBER_OF_FIELDS_CONFIG_ITEM + "'.");
135+
throw new IllegalArgumentException(
136+
"Missing configuration item: '" + BLOOM_NUMBER_OF_FIELDS_CONFIG_ITEM + "'."
137+
);
132138
}
133139
return jsonString;
134140
}

‎src/main/java/com/teragrep/pth_10/steps/teragrep/bloomfilter/ValidBloomFilterConfiguration.java‎

Lines changed: 100 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,100 @@
1+
/*
2+
* Teragrep Data Processing Language (DPL) translator for Apache Spark (pth_10)
3+
* Copyright (C) 2019-2025 Suomen Kanuuna Oy
4+
*
5+
* This program is free software: you can redistribute it and/or modify
6+
* it under the terms of the GNU Affero General Public License as published by
7+
* the Free Software Foundation, either version 3 of the License, or
8+
* (at your option) any later version.
9+
*
10+
* This program is distributed in the hope that it will be useful,
11+
* but WITHOUT ANY WARRANTY; without even the implied warranty of
12+
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
13+
* GNU Affero General Public License for more details.
14+
*
15+
* You should have received a copy of the GNU Affero General Public License
16+
* along with this program. If not, see <https://www.gnu.org/licenses/>.
17+
*
18+
*
19+
* Additional permission under GNU Affero General Public License version 3
20+
* section 7
21+
*
22+
* If you modify this Program, or any covered work, by linking or combining it
23+
* with other code, such other code is not for that reason alone subject to any
24+
* of the requirements of the GNU Affero GPL version 3 as long as this Program
25+
* is the same Program as licensed from Suomen Kanuuna Oy without any additional
26+
* modifications.
27+
*
28+
* Supplemented terms under GNU Affero General Public License version 3
29+
* section 7
30+
*
31+
* Origin of the software must be attributed to Suomen Kanuuna Oy. Any modified
32+
* versions must be marked as "Modified version of" The Program.
33+
*
34+
* Names of the licensors and authors may not be used for publicity purposes.
35+
*
36+
* No rights are granted for use of trade names, trademarks, or service marks
37+
* which are in The Program if any.
38+
*
39+
* Licensee must indemnify licensors and authors for any liability that these
40+
* contractual assumptions impose on licensors and authors.
41+
*
42+
* To the extent this program is licensed as part of the Commercial versions of
43+
* Teragrep, the applicable Commercial License may apply to this file if you as
44+
* a licensee so wish it.
45+
*/
46+
package com.teragrep.pth_10.steps.teragrep.bloomfilter;
47+
48+
import java.util.Objects;
49+
50+
public final class ValidBloomFilterConfiguration {
51+
52+
private final BloomFilterConfiguration configuration;
53+
54+
public ValidBloomFilterConfiguration(final BloomFilterConfiguration configuration) {
55+
this.configuration = configuration;
56+
}
57+
58+
private void validate() {
59+
if (configuration == null) {
60+
throw new IllegalArgumentException(
61+
"Option 'dpl.pth_06.bloom.db.fields' contains a 'null' filter configuration entry"
62+
);
63+
}
64+
}
65+
66+
public Long expected() {
67+
validate();
68+
final Long expected = configuration.expectedNumOfItems();
69+
if (expected == null) {
70+
throw new IllegalArgumentException("expected value in 'dpl.pth_06.bloom.db.fields' should not be 'null'");
71+
}
72+
return expected;
73+
}
74+
75+
public Double fpp() {
76+
validate();
77+
final Double fpp = configuration.falsePositiveProbability();
78+
if (fpp == null) {
79+
throw new IllegalArgumentException("fpp value in 'dpl.pth_06.bloom.db.fields' should not be 'null'");
80+
}
81+
return fpp;
82+
}
83+
84+
@Override
85+
public boolean equals(final Object o) {
86+
if (o == null) {
87+
return false;
88+
}
89+
if (getClass() != o.getClass()) {
90+
return false;
91+
}
92+
final ValidBloomFilterConfiguration that = (ValidBloomFilterConfiguration) o;
93+
return Objects.equals(configuration, that.configuration);
94+
}
95+
96+
@Override
97+
public int hashCode() {
98+
return Objects.hashCode(configuration);
99+
}
100+
}

‎src/test/java/com/teragrep/pth_10/steps/teragrep/bloomfilter/BloomFilterConfigurationTest.java‎

Lines changed: 26 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -96,22 +96,44 @@ public void testNonEquality() {
9696
public void testComparable() {
9797
final BloomFilterConfiguration base = new BloomFilterConfiguration(1000L, 0.02);
9898
final BloomFilterConfiguration equals = new BloomFilterConfiguration(1000L, 0.02);
99-
final BloomFilterConfiguration smaller = new BloomFilterConfiguration(500L, 0.02);
100-
final BloomFilterConfiguration larger = new BloomFilterConfiguration(2000L, 0.02);
99+
final BloomFilterConfiguration smallerExpected = new BloomFilterConfiguration(500L, 0.02);
100+
final BloomFilterConfiguration largerExpected = new BloomFilterConfiguration(2000L, 0.02);
101101
final BloomFilterConfiguration smallerFpp = new BloomFilterConfiguration(1000L, 0.01);
102102
final BloomFilterConfiguration largerFpp = new BloomFilterConfiguration(1000L, 0.03);
103103
final BloomFilterConfiguration bothLarger = new BloomFilterConfiguration(2000L, 0.03);
104104
final BloomFilterConfiguration bothSmaller = new BloomFilterConfiguration(500L, 0.01);
105105
final BloomFilterConfiguration smallerExpectedLargerFpp = new BloomFilterConfiguration(500L, 0.03);
106106
final BloomFilterConfiguration largerExpectedSmallerFpp = new BloomFilterConfiguration(2000L, 0.01);
107107
Assertions.assertEquals(0, base.compareTo(equals));
108-
Assertions.assertEquals(1, base.compareTo(smaller));
109-
Assertions.assertEquals(-1, base.compareTo(larger));
108+
Assertions.assertEquals(1, base.compareTo(smallerExpected));
109+
Assertions.assertEquals(-1, base.compareTo(largerExpected));
110110
Assertions.assertEquals(-1, base.compareTo(smallerFpp));
111111
Assertions.assertEquals(1, base.compareTo(largerFpp));
112112
Assertions.assertEquals(-1, base.compareTo(bothLarger));
113113
Assertions.assertEquals(1, base.compareTo(bothSmaller));
114114
Assertions.assertEquals(1, base.compareTo(smallerExpectedLargerFpp));
115115
Assertions.assertEquals(-1, base.compareTo(largerExpectedSmallerFpp));
116116
}
117+
118+
@Test
119+
public void testCompareNullValues() {
120+
final BloomFilterConfiguration base = new BloomFilterConfiguration(1000L, 0.02);
121+
final BloomFilterConfiguration expectedNull = new BloomFilterConfiguration(null, 0.02);
122+
final BloomFilterConfiguration fppNull = new BloomFilterConfiguration(1000L, null);
123+
final BloomFilterConfiguration bothNull = new BloomFilterConfiguration(null, null);
124+
Assertions.assertEquals(-1, base.compareTo(expectedNull));
125+
Assertions.assertEquals(-1, base.compareTo(fppNull));
126+
Assertions.assertEquals(-1, base.compareTo(bothNull));
127+
Assertions.assertEquals(-1, expectedNull.compareTo(bothNull));
128+
Assertions.assertEquals(-1, fppNull.compareTo(bothNull));
129+
}
130+
131+
@Test
132+
public void testCompareAgainstNullException() {
133+
final BloomFilterConfiguration base = new BloomFilterConfiguration(1000L, 0.02);
134+
final IllegalArgumentException exception = Assertions
135+
.assertThrows(IllegalArgumentException.class, () -> base.compareTo(null));
136+
final String expectedMessage = "Cannot compare against null";
137+
Assertions.assertEquals(expectedMessage, exception.getMessage());
138+
}
117139
}

‎src/test/java/com/teragrep/pth_10/steps/teragrep/bloomfilter/FilterTypesTest.java‎

Lines changed: 49 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -125,7 +125,11 @@ public void testSortedMapMethodWithNullStringJSON() {
125125
Exception exception = assertThrows(RuntimeException.class, () -> {
126126
filterTypes.sortedMap();
127127
});
128-
Assertions.assertEquals("Bloom filter size fields was not configured.", exception.getMessage());
128+
Assertions
129+
.assertEquals(
130+
"Option 'dpl.pth_06.bloom.db.fields' expected an JSON object but was 'null'",
131+
exception.getMessage()
132+
);
129133
}
130134

131135
@Test
@@ -198,6 +202,50 @@ public void testDuplicateValues() {
198202
Assertions.assertEquals(expectedMessage, exception.getMessage());
199203
}
200204

205+
@Test
206+
public void testNullExpectedValue() {
207+
final Properties properties = new Properties();
208+
properties.put("dpl.pth_06.bloom.db.fields", "[{expected: 1000, fpp: 0.01},{expected: null, fpp: 0.01}]");
209+
final Config config = ConfigFactory.parseProperties(properties);
210+
final FilterTypes filterTypes = new FilterTypes(config);
211+
final IllegalArgumentException exception = assertThrows(IllegalArgumentException.class, filterTypes::sortedMap);
212+
final String expectedMessage = "expected value in 'dpl.pth_06.bloom.db.fields' should not be 'null'";
213+
Assertions.assertEquals(expectedMessage, exception.getMessage());
214+
}
215+
216+
@Test
217+
public void testNullFppValue() {
218+
final Properties properties = new Properties();
219+
properties.put("dpl.pth_06.bloom.db.fields", "[{expected: 1000, fpp: 0.01},{expected: 2000, fpp: null}]");
220+
final Config config = ConfigFactory.parseProperties(properties);
221+
final FilterTypes filterTypes = new FilterTypes(config);
222+
final IllegalArgumentException exception = assertThrows(IllegalArgumentException.class, filterTypes::sortedMap);
223+
final String expectedMessage = "fpp value in 'dpl.pth_06.bloom.db.fields' should not be 'null'";
224+
Assertions.assertEquals(expectedMessage, exception.getMessage());
225+
}
226+
227+
@Test
228+
public void testNullFieldJson() {
229+
final Properties properties = new Properties();
230+
properties.put("dpl.pth_06.bloom.db.fields", "[{expected: 1000, fpp: 0.01},null]");
231+
final Config config = ConfigFactory.parseProperties(properties);
232+
final FilterTypes filterTypes = new FilterTypes(config);
233+
final IllegalArgumentException exception = assertThrows(IllegalArgumentException.class, filterTypes::sortedMap);
234+
final String expectedMessage = "Option 'dpl.pth_06.bloom.db.fields' contains a 'null' filter configuration entry";
235+
Assertions.assertEquals(expectedMessage, exception.getMessage());
236+
}
237+
238+
@Test
239+
public void testNullFilterTypeValueJson() {
240+
final Properties properties = new Properties();
241+
properties.put("dpl.pth_06.bloom.db.fields", "null");
242+
final Config config = ConfigFactory.parseProperties(properties);
243+
final FilterTypes filterTypes = new FilterTypes(config);
244+
final IllegalArgumentException exception = assertThrows(IllegalArgumentException.class, filterTypes::sortedMap);
245+
final String expectedMessage = "Option 'dpl.pth_06.bloom.db.fields' expected an JSON object but was 'null'";
246+
Assertions.assertEquals(expectedMessage, exception.getMessage());
247+
}
248+
201249
@Test
202250
public void testEquals() {
203251
Config config = ConfigFactory.parseProperties(defaultProperties());

‎src/test/java/com/teragrep/pth_10/steps/teragrep/bloomfilter/ValidBloomFilterConfigurationTest.java‎

Lines changed: 103 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,103 @@
1+
/*
2+
* Teragrep Data Processing Language (DPL) translator for Apache Spark (pth_10)
3+
* Copyright (C) 2019-2025 Suomen Kanuuna Oy
4+
*
5+
* This program is free software: you can redistribute it and/or modify
6+
* it under the terms of the GNU Affero General Public License as published by
7+
* the Free Software Foundation, either version 3 of the License, or
8+
* (at your option) any later version.
9+
*
10+
* This program is distributed in the hope that it will be useful,
11+
* but WITHOUT ANY WARRANTY; without even the implied warranty of
12+
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
13+
* GNU Affero General Public License for more details.
14+
*
15+
* You should have received a copy of the GNU Affero General Public License
16+
* along with this program. If not, see <https://www.gnu.org/licenses/>.
17+
*
18+
*
19+
* Additional permission under GNU Affero General Public License version 3
20+
* section 7
21+
*
22+
* If you modify this Program, or any covered work, by linking or combining it
23+
* with other code, such other code is not for that reason alone subject to any
24+
* of the requirements of the GNU Affero GPL version 3 as long as this Program
25+
* is the same Program as licensed from Suomen Kanuuna Oy without any additional
26+
* modifications.
27+
*
28+
* Supplemented terms under GNU Affero General Public License version 3
29+
* section 7
30+
*
31+
* Origin of the software must be attributed to Suomen Kanuuna Oy. Any modified
32+
* versions must be marked as "Modified version of" The Program.
33+
*
34+
* Names of the licensors and authors may not be used for publicity purposes.
35+
*
36+
* No rights are granted for use of trade names, trademarks, or service marks
37+
* which are in The Program if any.
38+
*
39+
* Licensee must indemnify licensors and authors for any liability that these
40+
* contractual assumptions impose on licensors and authors.
41+
*
42+
* To the extent this program is licensed as part of the Commercial versions of
43+
* Teragrep, the applicable Commercial License may apply to this file if you as
44+
* a licensee so wish it.
45+
*/
46+
package com.teragrep.pth_10.steps.teragrep.bloomfilter;
47+
48+
import nl.jqno.equalsverifier.EqualsVerifier;
49+
import org.junit.jupiter.api.Assertions;
50+
import org.junit.jupiter.api.Test;
51+
52+
public class ValidBloomFilterConfigurationTest {
53+
54+
@Test
55+
public void testValidConfiguration() {
56+
final ValidBloomFilterConfiguration configuration = new ValidBloomFilterConfiguration(
57+
new BloomFilterConfiguration(1000L, 0.01)
58+
);
59+
Assertions.assertEquals(1000L, configuration.expected());
60+
Assertions.assertEquals(0.01, configuration.fpp());
61+
}
62+
63+
@Test
64+
public void testNullBloomFilterConfiguration() {
65+
final ValidBloomFilterConfiguration configuration = new ValidBloomFilterConfiguration(null);
66+
final IllegalArgumentException exception = Assertions
67+
.assertThrows(IllegalArgumentException.class, configuration::expected);
68+
Assertions
69+
.assertEquals(
70+
"Option 'dpl.pth_06.bloom.db.fields' contains a 'null' filter configuration entry",
71+
exception.getMessage()
72+
);
73+
}
74+
75+
@Test
76+
public void testNullExpectedException() {
77+
final ValidBloomFilterConfiguration configuration = new ValidBloomFilterConfiguration(
78+
new BloomFilterConfiguration(null, 0.01)
79+
);
80+
final IllegalArgumentException exception = Assertions
81+
.assertThrows(IllegalArgumentException.class, configuration::expected);
82+
Assertions
83+
.assertEquals(
84+
"expected value in 'dpl.pth_06.bloom.db.fields' should not be 'null'", exception.getMessage()
85+
);
86+
}
87+
88+
@Test
89+
public void testNullFppException() {
90+
final ValidBloomFilterConfiguration configuration = new ValidBloomFilterConfiguration(
91+
new BloomFilterConfiguration(1000L, null)
92+
);
93+
final IllegalArgumentException exception = Assertions
94+
.assertThrows(IllegalArgumentException.class, configuration::fpp);
95+
Assertions
96+
.assertEquals("fpp value in 'dpl.pth_06.bloom.db.fields' should not be 'null'", exception.getMessage());
97+
}
98+
99+
@Test
100+
public void testContract() {
101+
EqualsVerifier.forClass(ValidBloomFilterConfiguration.class).verify();
102+
}
103+
}

0 commit comments

Comments
 (0)