[CALCITE-5052] Allow Source based on a URL with jar: protocol

This allows dependent projects to run tests using Bazel. (Previously, DiffRepository would give errors because Bazel has packaged the .xml files it needs inside JAR files.) Close apache#2750
looker-open-source · Oct 17, 2024 · ccb609f · ccb609f
1 parent b3343d0
commit ccb609f
Show file tree

Hide file tree

Showing 3 changed files with 103 additions and 17 deletions.
diff --git a/core/src/main/java/org/apache/calcite/util/Sources.java b/core/src/main/java/org/apache/calcite/util/Sources.java
@@ -192,22 +192,30 @@ private File fileNonNull() {
     }
 
     private static @Nullable File urlToFile(URL url) {
-      if (!"file".equals(url.getProtocol())) {
+      switch (url.getProtocol()) {
+      case "jar":
+        try {
+          return urlToFile((new URI(url.getFile())).toURL());
+        } catch (MalformedURLException | URISyntaxException e) {
+          return null;
+        }
+      case "file":
+        URI uri;
+        try {
+          uri = url.toURI();
+        } catch (URISyntaxException e) {
+          throw new IllegalArgumentException("Unable to convert URL " + url + " to URI", e);
+        }
+        if (uri.isOpaque()) {
+          // It is like file:test%20file.c++
+          // getSchemeSpecificPart would return "test file.c++"
+          return new File(uri.getSchemeSpecificPart());
+        }
+        // See https://stackoverflow.com/a/17870390/1261287
+        return Paths.get(uri).toFile();
+      default:
         return null;
       }
-      URI uri;
-      try {
-        uri = url.toURI();
-      } catch (URISyntaxException e) {
-        throw new IllegalArgumentException("Unable to convert URL " + url + " to URI", e);
-      }
-      if (uri.isOpaque()) {
-        // It is like file:test%20file.c++
-        // getSchemeSpecificPart would return "test file.c++"
-        return new File(uri.getSchemeSpecificPart());
-      }
-      // See https://stackoverflow.com/a/17870390/1261287
-      return Paths.get(uri).toFile();
     }
 
     private static URL fileToUrl(File file) {

diff --git a/core/src/test/java/org/apache/calcite/util/SourceTest.java b/core/src/test/java/org/apache/calcite/util/SourceTest.java
@@ -15,8 +15,13 @@
  * limitations under the License.
  */
 package org.apache.calcite.util;
+
+import org.apache.commons.lang3.StringUtils;
+
 import com.google.common.io.CharSource;
 
+import net.hydromatic.foodmart.queries.FoodmartQuerySet;
+
 import org.junit.jupiter.api.Disabled;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.params.ParameterizedTest;
@@ -25,20 +30,28 @@
 
 import java.io.BufferedReader;
 import java.io.File;
+import java.io.FileOutputStream;
 import java.io.IOException;
 import java.io.InputStreamReader;
+import java.io.OutputStreamWriter;
+import java.io.PrintWriter;
 import java.io.Reader;
+import java.net.MalformedURLException;
+import java.net.URI;
 import java.net.URISyntaxException;
 import java.net.URL;
 import java.nio.charset.StandardCharsets;
 import java.util.Arrays;
+import java.util.regex.Pattern;
 import java.util.stream.Stream;
 
 import static org.apache.calcite.util.Sources.file;
 import static org.apache.calcite.util.Sources.of;
 import static org.apache.calcite.util.Sources.url;
 
 import static org.hamcrest.CoreMatchers.is;
+import static org.hamcrest.CoreMatchers.not;
+import static org.hamcrest.CoreMatchers.notNullValue;
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.Matchers.hasToString;
 import static org.junit.jupiter.api.Assertions.assertNotNull;
@@ -120,6 +133,56 @@ void testAbsoluteFileToUrl(String path, String expectedUrl) throws URISyntaxExce
         is(absoluteFile.getAbsolutePath()));
   }
 
+  /** Test case for
+   * <a href="https://issues.apache.org/jira/browse/CALCITE-5052">[CALCITE-5052]
+   * Allow Source based on a URL with jar: protocol</a>. */
+  @Test void testJarFileUrl() throws MalformedURLException {
+    // mock jar file
+    final String jarPath = "jar:file:sources!/abcdef.txt";
+    final URL url;
+    try {
+      url = new URI(jarPath).toURL();
+    } catch (URISyntaxException e) {
+      throw new RuntimeException(e);
+    }
+    final Source source = of(url);
+    assertThat("No file retrieved for Sources.of(file " + jarPath + ")",
+        source.file(), notNullValue());
+    assertThat("Sources.of(file " + jarPath + ").url()).file().getPath()",
+        slashify(source.file().getPath()),
+        is("sources!/abcdef.txt"));
+
+  }
+
+  /** Tests {@link Sources#of(URL)} with code similar to
+   * {@code DiffRepository.Key#toRepo()}. */
+  @Test void testJarFileUrlWrite() {
+    final URL refFile = FoodmartQuerySet.class.getResource("/queries.json");
+    assertThat(refFile, notNullValue());
+    final Source source = of(refFile);
+    final String refFilePath = source.file().getAbsolutePath();
+    final String logFilePath;
+    assertThat(StringUtils.containsIgnoreCase(refFilePath, ".jar!"), is(true));
+    // If the file is located in a JAR, we cannot write the file in place
+    // so we add it to the /tmp directory
+    // the expected output is /tmp/[jarname]/[path-to-file-in-jar/filename]_actual.json
+    logFilePath =
+        Pattern.compile(".*\\/(.*)\\.jar\\!(.*)\\.json")
+            .matcher(refFilePath)
+            .replaceAll("/tmp/$1$2_actual.json");
+    final File logFile = new File(logFilePath);
+    assertThat(refFile, not(is(logFile.getAbsolutePath())));
+    boolean b = logFile.getParentFile().mkdirs();
+    Util.discard(b);
+    try (FileOutputStream fos = new FileOutputStream(logFile);
+        OutputStreamWriter osw = new OutputStreamWriter(fos, StandardCharsets.UTF_8);
+        PrintWriter pw = new PrintWriter(osw)) {
+      pw.println("hello, world!");
+    } catch (IOException e) {
+      throw Util.throwAsRuntime(e);
+    }
+  }
+
   @Test void testAppendWithSpaces() {
     String fooRelative = "fo o+";
     String fooAbsolute = ROOT_PREFIX + "fo o+";

diff --git a/testkit/src/main/java/org/apache/calcite/test/DiffRepository.java b/testkit/src/main/java/org/apache/calcite/test/DiffRepository.java
@@ -23,6 +23,8 @@
 import org.apache.calcite.util.Util;
 import org.apache.calcite.util.XmlOutput;
 
+import org.apache.commons.lang3.StringUtils;
+
 import com.google.common.cache.CacheBuilder;
 import com.google.common.cache.CacheLoader;
 import com.google.common.cache.LoadingCache;
@@ -52,6 +54,7 @@
 import java.util.Objects;
 import java.util.SortedMap;
 import java.util.TreeMap;
+import java.util.regex.Pattern;
 import javax.xml.XMLConstants;
 import javax.xml.parsers.DocumentBuilder;
 import javax.xml.parsers.DocumentBuilderFactory;
@@ -909,6 +912,9 @@ String filter(
 
   /** Cache key. */
   private static class Key {
+    private static final Pattern JAR_EMBEDDED_XML_PATH =
+        Pattern.compile(".*\\/(.*)\\.jar\\!(.*)\\.xml");
+
     private final Class<?> clazz;
     private final @Nullable DiffRepository baseRepository;
     private final @Nullable Filter filter;
@@ -937,9 +943,18 @@ private static class Key {
     DiffRepository toRepo() {
       final URL refFile = findFile(clazz, ".xml");
       final String refFilePath = Sources.of(refFile).file().getAbsolutePath();
-      final String logFilePath = refFilePath
-          .replace("resources", "diffrepo")
-          .replace(".xml", "_actual.xml");
+      final String logFilePath;
+      if (StringUtils.containsIgnoreCase(refFilePath, ".jar!")) {
+        // If the file is located in a JAR, we cannot write the file in place
+        // so we add it to the /tmp directory
+        // the expected output is /tmp/[jarname]/[path-to-file-in-jar/filename]_actual.xml
+        logFilePath =
+            JAR_EMBEDDED_XML_PATH.matcher(refFilePath).replaceAll("/tmp/$1$2_actual.xml");
+      } else {
+        logFilePath = refFilePath
+            .replace("resources", "diffrepo")
+            .replace(".xml", "_actual.xml");
+      }
       final File logFile = new File(logFilePath);
       assert !refFilePath.equals(logFile.getAbsolutePath());
       return new DiffRepository(refFile, logFile, baseRepository, filter,