Skip to content

Commit

Permalink
fix: handle use case where data fetched is null to avoid exceptions
Browse files Browse the repository at this point in the history
Signed-off-by: TessaIO <ahmedgrati1999@gmail.com>
  • Loading branch information
TessaIO committed Mar 19, 2024
1 parent 8e417d2 commit a646858
Show file tree
Hide file tree
Showing 2 changed files with 13 additions and 24 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -27,11 +27,7 @@
import org.apache.druid.server.lookup.cache.loading.LoadingCache;

import javax.annotation.Nullable;
import java.util.Collections;
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.HashSet;
import java.util.*;
import java.util.concurrent.Callable;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.atomic.AtomicBoolean;
Expand Down Expand Up @@ -74,16 +70,20 @@ public String apply(@Nullable final String key)
// otherwise null will be replaced with empty string in nullToEmptyIfNeeded above.
return null;
}

final String presentVal;
try {
presentVal = loadingCache.get(keyEquivalent, new ApplyCallable(keyEquivalent));
String presentVal = loadingCache.getIfPresent(keyEquivalent);
if (presentVal != null) {
return NullHandling.emptyToNullIfNeeded(presentVal);
}
catch (ExecutionException e) {
LOGGER.debug("value not found for key [%s]", key);

String val = this.dataFetcher.fetch(keyEquivalent);
LOGGER.info("VAL is [%s] AND KEY IS [%s]", val, keyEquivalent);
if (val == null) {
return null;
}
Map<String, String> map = new HashMap<String, String>();
map.put(keyEquivalent, val);
loadingCache.putAll(map);
return NullHandling.emptyToNullIfNeeded(val);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ public class LoadingLookupTest extends InitializedNullHandlingTest
@Test
public void testApplyEmptyOrNull() throws ExecutionException
{
EasyMock.expect(lookupCache.get(EasyMock.eq(""), EasyMock.anyObject(Callable.class)))
EasyMock.expect(lookupCache.getIfPresent(EasyMock.eq("")))
.andReturn("empty").atLeastOnce();
EasyMock.replay(lookupCache);
Assert.assertEquals("empty", loadingLookup.apply(""));
Expand All @@ -74,7 +74,7 @@ public void testUnapplyNull()
@Test
public void testApply() throws ExecutionException
{
EasyMock.expect(lookupCache.get(EasyMock.eq("key"), EasyMock.anyObject(Callable.class))).andReturn("value").once();
EasyMock.expect(lookupCache.getIfPresent(EasyMock.eq("key"))).andReturn("value").once();
EasyMock.replay(lookupCache);
Assert.assertEquals(ImmutableMap.of("key", "value"), loadingLookup.applyAll(ImmutableSet.of("key")));
EasyMock.verify(lookupCache);
Expand All @@ -101,17 +101,6 @@ public void testClose()
EasyMock.verify(lookupCache, reverseLookupCache);
}

@Test
public void testApplyWithExecutionError() throws ExecutionException
{
EasyMock.expect(lookupCache.get(EasyMock.eq("key"), EasyMock.anyObject(Callable.class)))
.andThrow(new ExecutionException(null))
.once();
EasyMock.replay(lookupCache);
Assert.assertNull(loadingLookup.apply("key"));
EasyMock.verify(lookupCache);
}

@Test
public void testUnApplyWithExecutionError() throws ExecutionException
{
Expand Down

0 comments on commit a646858

Please sign in to comment.