Unwrap exceptions from RuntimeException in URIExtractionNamespaceCacheFactory.populateCache() (part of #3667) (#3668)

* Unwrap exceptions from RuntimeException in URIExtractionNamespaceCacheFactory.populateCache()

* Fix tests

extensions-core/lookups-cached-global/src/main/java/io/druid/server/lookup/namespace/URIExtractionNamespaceCacheFactory.java

@@ -19,7 +19,6 @@
 
 package io.druid.server.lookup.namespace;
 
-import com.google.common.base.Throwables;
 import com.google.common.io.ByteSource;
 import com.google.inject.Inject;
 import io.druid.data.SearchableVersionedDataFinder;
@@ -99,14 +98,12 @@ public class URIExtractionNamespaceCacheFactory implements ExtractionNamespaceCa
       );
 
       if (uri == null) {
-        throw new RuntimeException(
-            new FileNotFoundException(
-                String.format(
-                    "Could not find match for pattern `%s` in [%s] for %s",
-                    versionRegex,
-                    originalUri,
-                    extractionNamespace
-                )
+        throw new FileNotFoundException(
+            String.format(
+                "Could not find match for pattern `%s` in [%s] for %s",
+                versionRegex,
+                originalUri,
+                extractionNamespace
             )
         );
       }
@@ -116,71 +113,66 @@ public class URIExtractionNamespaceCacheFactory implements ExtractionNamespaceCa
 
     final String uriPath = uri.getPath();
 
-    try {
-      return RetryUtils.retry(
-          new Callable<String>()
+    return RetryUtils.retry(
+        new Callable<String>()
+        {
+          @Override
+          public String call() throws Exception
           {
-            @Override
-            public String call() throws Exception
-            {
-              final String version = puller.getVersion(uri);
-              try {
-                // Important to call equals() against version because lastVersion could be null
-                if (version.equals(lastVersion)) {
-                  log.debug(
-                      "URI [%s] for namespace [%s] has the same last modified time [%s] as the last cached. " +
-                      "Skipping ",
-                      uri.toString(),
-                      id,
-                      version
-                  );
-                  return lastVersion;
-                }
+            final String version = puller.getVersion(uri);
+            try {
+              // Important to call equals() against version because lastVersion could be null
+              if (version.equals(lastVersion)) {
+                log.debug(
+                    "URI [%s] for namespace [%s] has the same last modified time [%s] as the last cached. " +
+                    "Skipping ",
+                    uri.toString(),
+                    id,
+                    version
+                );
+                return lastVersion;
               }
-              catch (NumberFormatException ex) {
-                log.debug(ex, "Failed to get last modified timestamp. Assuming no timestamp");
-              }
-              final ByteSource source;
-              if (CompressionUtils.isGz(uriPath)) {
-                // Simple gzip stream
-                log.debug("Loading gz");
-                source = new ByteSource()
-                {
-                  @Override
-                  public InputStream openStream() throws IOException
-                  {
-                    return CompressionUtils.gzipInputStream(puller.getInputStream(uri));
-                  }
-                };
-              } else {
-                source = new ByteSource()
-                {
-                  @Override
-                  public InputStream openStream() throws IOException
-                  {
-                    return puller.getInputStream(uri);
-                  }
-                };
-              }
-              final MapPopulator.PopulateResult populateResult = new MapPopulator<>(
-                  extractionNamespace.getNamespaceParseSpec()
-                                     .getParser()
-              ).populate(source, cache);
-              log.info(
-                  "Finished loading %,d values from %,d lines for namespace [%s]",
-                  populateResult.getEntries(),
-                  populateResult.getLines(),
-                  id
-              );
-              return version;
             }
-          },
-          puller.shouldRetryPredicate(),
-          DEFAULT_NUM_RETRIES
-      );
-    }
-    catch (Exception e) {
-      throw Throwables.propagate(e);
-    }
+            catch (NumberFormatException ex) {
+              log.debug(ex, "Failed to get last modified timestamp. Assuming no timestamp");
+            }
+            final ByteSource source;
+            if (CompressionUtils.isGz(uriPath)) {
+              // Simple gzip stream
+              log.debug("Loading gz");
+              source = new ByteSource()
+              {
+                @Override
+                public InputStream openStream() throws IOException
+                {
+                  return CompressionUtils.gzipInputStream(puller.getInputStream(uri));
+                }
+              };
+            } else {
+              source = new ByteSource()
+              {
+                @Override
+                public InputStream openStream() throws IOException
+                {
+                  return puller.getInputStream(uri);
+                }
+              };
+            }
+            final MapPopulator.PopulateResult populateResult = new MapPopulator<>(
+                extractionNamespace.getNamespaceParseSpec()
+                                   .getParser()
+            ).populate(source, cache);
+            log.info(
+                "Finished loading %,d values from %,d lines for namespace [%s]",
+                populateResult.getEntries(),
+                populateResult.getLines(),
+                id
+            );
+            return version;
+          }
+        },
+        puller.shouldRetryPredicate(),
+        DEFAULT_NUM_RETRIES
+    );
   }
 }

extensions-core/lookups-cached-global/src/test/java/io/druid/server/lookup/namespace/URIExtractionNamespaceCacheFactoryTest.java

@@ -40,15 +40,12 @@ import io.druid.server.lookup.namespace.cache.NamespaceExtractionCacheManagersTe
 import io.druid.server.lookup.namespace.cache.OffHeapNamespaceExtractionCacheManager;
 import io.druid.server.lookup.namespace.cache.OnHeapNamespaceExtractionCacheManager;
 import io.druid.server.metrics.NoopServiceEmitter;
-import org.hamcrest.BaseMatcher;
-import org.hamcrest.Description;
 import org.joda.time.Period;
 import org.junit.After;
 import org.junit.Assert;
 import org.junit.Before;
 import org.junit.Rule;
 import org.junit.Test;
-import org.junit.rules.ExpectedException;
 import org.junit.rules.TemporaryFolder;
 import org.junit.runner.RunWith;
 import org.junit.runners.Parameterized;
@@ -279,8 +276,6 @@ public class URIExtractionNamespaceCacheFactoryTest
 
   @Rule
   public TemporaryFolder temporaryFolder = new TemporaryFolder();
-  @Rule
-  public ExpectedException expectedException = ExpectedException.none();
 
   private final String suffix;
   private final Function<File, OutputStream> outStreamSupplier;
@@ -413,7 +408,7 @@ public class URIExtractionNamespaceCacheFactoryTest
     Assert.assertEquals(null, map.get("baz"));
   }
 
-  @Test
+  @Test(expected = FileNotFoundException.class)
   public void testMissing() throws Exception
   {
     URIExtractionNamespace badNamespace = new URIExtractionNamespace(
@@ -425,18 +420,10 @@ public class URIExtractionNamespaceCacheFactoryTest
     );
     Assert.assertTrue(new File(namespace.getUri()).delete());
     ConcurrentMap<String, String> map = new ConcurrentHashMap<>();
-    try {
-      factory.populateCache(id, badNamespace, null, map);
-    }
-    catch (RuntimeException e) {
-      Assert.assertNotNull(e.getCause());
-      Assert.assertEquals(FileNotFoundException.class, e.getCause().getClass());
-      return;
-    }
-    Assert.fail("Did not have exception");
+    factory.populateCache(id, badNamespace, null, map);
   }
 
-  @Test
+  @Test(expected = FileNotFoundException.class)
   public void testMissingRegex() throws Exception
   {
     String badId = "bad";
@@ -450,24 +437,6 @@ public class URIExtractionNamespaceCacheFactoryTest
     );
     Assert.assertTrue(new File(namespace.getUri()).delete());
     ConcurrentMap<String, String> map = new ConcurrentHashMap<>();
-    expectedException.expect(new BaseMatcher<Throwable>()
-    {
-      @Override
-      public void describeTo(Description description)
-      {
-
-      }
-
-      @Override
-      public boolean matches(Object o)
-      {
-        if (!(o instanceof Throwable)) {
-          return false;
-        }
-        final Throwable t = (Throwable) o;
-        return t.getCause() != null && t.getCause() instanceof FileNotFoundException;
-      }
-    });
     factory.populateCache(badId, badNamespace, null, map);
   }