Skip to content

Commit bb4cf65

Browse files
committed
fix: handle use case where data fetched is null to avoid exceptions
Signed-off-by: TessaIO <ahmedgrati1999@gmail.com>
1 parent 8e417d2 commit bb4cf65

File tree

2 files changed

+27
-29
lines changed

2 files changed

+27
-29
lines changed

extensions-core/lookups-cached-single/src/main/java/org/apache/druid/server/lookup/LoadingLookup.java

+12-10
Original file line numberDiff line numberDiff line change
@@ -28,10 +28,11 @@
2828

2929
import javax.annotation.Nullable;
3030
import java.util.Collections;
31+
import java.util.HashMap;
32+
import java.util.HashSet;
3133
import java.util.List;
3234
import java.util.Map;
3335
import java.util.Set;
34-
import java.util.HashSet;
3536
import java.util.concurrent.Callable;
3637
import java.util.concurrent.ExecutionException;
3738
import java.util.concurrent.atomic.AtomicBoolean;
@@ -74,16 +75,19 @@ public String apply(@Nullable final String key)
7475
// otherwise null will be replaced with empty string in nullToEmptyIfNeeded above.
7576
return null;
7677
}
77-
78-
final String presentVal;
79-
try {
80-
presentVal = loadingCache.get(keyEquivalent, new ApplyCallable(keyEquivalent));
78+
String presentVal = loadingCache.getIfPresent(keyEquivalent);
79+
if (presentVal != null) {
8180
return NullHandling.emptyToNullIfNeeded(presentVal);
8281
}
83-
catch (ExecutionException e) {
84-
LOGGER.debug("value not found for key [%s]", key);
82+
83+
String val = this.dataFetcher.fetch(keyEquivalent);
84+
if (val == null) {
8585
return null;
8686
}
87+
Map<String, String> map = new HashMap<>();
88+
map.put(keyEquivalent, val);
89+
loadingCache.putAll(map);
90+
return NullHandling.emptyToNullIfNeeded(val);
8791
}
8892

8993
@Override
@@ -131,9 +135,7 @@ public Set<String> keySet()
131135
Iterable<Map.Entry<String, String>> data = this.dataFetcher.fetchAll();
132136
Set<String> set = new HashSet<>();
133137
if (data != null) {
134-
data.forEach(each -> {
135-
set.add(each.getKey());
136-
});
138+
data.forEach(each -> set.add(each.getKey()));
137139
}
138140
return set;
139141
}

extensions-core/lookups-cached-single/src/test/java/org/apache/druid/server/lookup/LoadingLookupTest.java

+15-19
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@
1919

2020
package org.apache.druid.server.lookup;
2121

22-
import com.amazonaws.transform.MapEntry;
2322
import com.google.common.collect.ImmutableMap;
2423
import com.google.common.collect.ImmutableSet;
2524
import org.apache.druid.common.config.NullHandling;
@@ -31,7 +30,11 @@
3130
import org.junit.Test;
3231
import org.junit.rules.ExpectedException;
3332

34-
import java.util.*;
33+
import java.util.AbstractMap;
34+
import java.util.Arrays;
35+
import java.util.Collections;
36+
import java.util.Iterator;
37+
import java.util.Map;
3538
import java.util.concurrent.Callable;
3639
import java.util.concurrent.ExecutionException;
3740

@@ -48,7 +51,7 @@ public class LoadingLookupTest extends InitializedNullHandlingTest
4851
@Test
4952
public void testApplyEmptyOrNull() throws ExecutionException
5053
{
51-
EasyMock.expect(lookupCache.get(EasyMock.eq(""), EasyMock.anyObject(Callable.class)))
54+
EasyMock.expect(lookupCache.getIfPresent(EasyMock.eq("")))
5255
.andReturn("empty").atLeastOnce();
5356
EasyMock.replay(lookupCache);
5457
Assert.assertEquals("empty", loadingLookup.apply(""));
@@ -74,7 +77,7 @@ public void testUnapplyNull()
7477
@Test
7578
public void testApply() throws ExecutionException
7679
{
77-
EasyMock.expect(lookupCache.get(EasyMock.eq("key"), EasyMock.anyObject(Callable.class))).andReturn("value").once();
80+
EasyMock.expect(lookupCache.getIfPresent(EasyMock.eq("key"))).andReturn("value").once();
7881
EasyMock.replay(lookupCache);
7982
Assert.assertEquals(ImmutableMap.of("key", "value"), loadingLookup.applyAll(ImmutableSet.of("key")));
8083
EasyMock.verify(lookupCache);
@@ -101,17 +104,6 @@ public void testClose()
101104
EasyMock.verify(lookupCache, reverseLookupCache);
102105
}
103106

104-
@Test
105-
public void testApplyWithExecutionError() throws ExecutionException
106-
{
107-
EasyMock.expect(lookupCache.get(EasyMock.eq("key"), EasyMock.anyObject(Callable.class)))
108-
.andThrow(new ExecutionException(null))
109-
.once();
110-
EasyMock.replay(lookupCache);
111-
Assert.assertNull(loadingLookup.apply("key"));
112-
EasyMock.verify(lookupCache);
113-
}
114-
115107
@Test
116108
public void testUnApplyWithExecutionError() throws ExecutionException
117109
{
@@ -161,9 +153,13 @@ public void testFetchAll()
161153
EasyMock.verify(dataFetcher);
162154
}
163155

164-
public int getIteratorSize(Iterator<Map.Entry<String, String>> it) {
165-
int i = 0;
166-
for ( ; it.hasNext() ; ++i ) it.next();
167-
return i;
156+
public int getIteratorSize(Iterator<Map.Entry<String, String>> it)
157+
{
158+
int sum = 0;
159+
while (it.hasNext()) {
160+
sum++;
161+
it.next();
162+
}
163+
return sum;
168164
}
169165
}

0 commit comments

Comments
 (0)