From ccb609f1d092b249c10b6be7c6025464563600e9 Mon Sep 17 00:00:00 2001 From: Marieke Gueye Date: Tue, 22 Mar 2022 16:22:12 +0000 Subject: [PATCH] [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/calcite#2750 --- .../java/org/apache/calcite/util/Sources.java | 36 ++++++----- .../org/apache/calcite/util/SourceTest.java | 63 +++++++++++++++++++ .../apache/calcite/test/DiffRepository.java | 21 ++++++- 3 files changed, 103 insertions(+), 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 index 406b1e6ddc57..66b2ea8a4f07 100644 --- 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 index 6715a3622ad8..04336354476b 100644 --- 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,13 +30,19 @@ 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; @@ -39,6 +50,8 @@ 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 + * [CALCITE-5052] + * Allow Source based on a URL with jar: protocol. */ + @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 index 197cd971508c..d675ef616733 100644 --- 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,