From 036cbbd17fb16e59ac7e6a544e1c9a78f4af813a Mon Sep 17 00:00:00 2001 From: Damon Douglas Date: Sun, 7 Jul 2024 15:18:20 -0700 Subject: [PATCH 1/7] Stage PrismRunner implementation and dependencies --- .../org/apache/beam/runners/prism/PrismPipelineOptions.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/runners/prism/java/src/main/java/org/apache/beam/runners/prism/PrismPipelineOptions.java b/runners/prism/java/src/main/java/org/apache/beam/runners/prism/PrismPipelineOptions.java index ec0f8beb620a..1b665603b8b6 100644 --- a/runners/prism/java/src/main/java/org/apache/beam/runners/prism/PrismPipelineOptions.java +++ b/runners/prism/java/src/main/java/org/apache/beam/runners/prism/PrismPipelineOptions.java @@ -35,7 +35,7 @@ public interface PrismPipelineOptions extends PortablePipelineOptions { void setPrismLocation(String prismLocation); @Description( - "Override the SDK's version for deriving the Github Release URLs for " + "Override the SDK\\'s version for deriving the Github Release URLs for " + "downloading a zipped prism binary, for the current platform. If " + "set to a Github Release page URL, then it will use that release page as a base when constructing the download URL.") String getPrismVersionOverride(); From 5ee5fc37d9cc16f8539b765d6fb9e8c2a2be3a0e Mon Sep 17 00:00:00 2001 From: Damon Douglas Date: Sun, 7 Jul 2024 15:42:20 -0700 Subject: [PATCH 2/7] Locate and download Prism binary --- .../beam/runners/prism/PrismLocator.java | 253 ++++++++++++++++++ .../beam/runners/prism/PrismLocatorTest.java | 77 ++++++ 2 files changed, 330 insertions(+) create mode 100644 runners/prism/java/src/main/java/org/apache/beam/runners/prism/PrismLocator.java create mode 100644 runners/prism/java/src/test/java/org/apache/beam/runners/prism/PrismLocatorTest.java diff --git a/runners/prism/java/src/main/java/org/apache/beam/runners/prism/PrismLocator.java b/runners/prism/java/src/main/java/org/apache/beam/runners/prism/PrismLocator.java new file mode 100644 index 000000000000..d5b148318f29 --- /dev/null +++ b/runners/prism/java/src/main/java/org/apache/beam/runners/prism/PrismLocator.java @@ -0,0 +1,253 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.beam.runners.prism; + +import static org.apache.beam.sdk.util.Preconditions.checkStateNotNull; +import static org.apache.beam.vendor.guava.v32_1_2_jre.com.google.common.base.Preconditions.checkArgument; +import static org.apache.beam.vendor.guava.v32_1_2_jre.com.google.common.base.Preconditions.checkState; + +import java.io.IOException; +import java.io.InputStream; +import java.io.OutputStream; +import java.net.URL; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.Paths; +import java.nio.file.attribute.PosixFilePermission; +import java.nio.file.attribute.PosixFilePermissions; +import java.util.Iterator; +import java.util.Set; +import java.util.function.BiConsumer; +import java.util.zip.ZipEntry; +import java.util.zip.ZipInputStream; +import org.apache.beam.sdk.util.ReleaseInfo; +import org.apache.beam.vendor.guava.v32_1_2_jre.com.google.common.base.Splitter; +import org.apache.beam.vendor.guava.v32_1_2_jre.com.google.common.base.Strings; +import org.apache.beam.vendor.guava.v32_1_2_jre.com.google.common.hash.HashCode; +import org.apache.beam.vendor.guava.v32_1_2_jre.com.google.common.hash.Hashing; +import org.apache.beam.vendor.guava.v32_1_2_jre.com.google.common.io.ByteStreams; + +/** + * Locates a Prism executable based on a user's default operating system and architecture + * environment or a {@link PrismPipelineOptions#getPrismLocation()} override. Handles the download, + * unzip, {@link PosixFilePermissions}, as needed. For {@link #GITHUB_DOWNLOAD_PREFIX} sources, + * additionally performs a SHA512 verification. + */ +class PrismLocator { + static final String OS_NAME_PROPERTY = "os.name"; + static final String ARCH_PROPERTY = "os.arch"; + static final String USER_HOME_PROPERTY = "user.home"; + + private static final String ZIP_EXT = "zip"; + private static final String SHA512_EXT = "sha512"; + private static final ReleaseInfo RELEASE_INFO = ReleaseInfo.getReleaseInfo(); + private static final String PRISM_BIN_PATH = ".apache_beam/cache/prism/bin"; + private static final Set PERMS = + PosixFilePermissions.fromString("rwxr-xr-x"); + private static final String GITHUB_DOWNLOAD_PREFIX = + "https://github.com/apache/beam/releases/download"; + private static final String GITHUB_TAG_PREFIX = "https://github.com/apache/beam/releases/tag"; + + private final PrismPipelineOptions options; + + PrismLocator(PrismPipelineOptions options) { + this.options = options; + } + + /** + * Downloads and prepares a Prism executable for use with the {@link PrismRunner}, executed by the + * {@link PrismExecutor}. The returned {@link String} is the absolute path to the Prism + * executable. + */ + String resolve() throws IOException { + + String from = + String.format("%s/v%s/%s.zip", GITHUB_DOWNLOAD_PREFIX, getSDKVersion(), buildFileName()); + + if (!Strings.isNullOrEmpty(options.getPrismLocation())) { + checkArgument( + !options.getPrismLocation().startsWith(GITHUB_TAG_PREFIX), + "Provided --prismLocation URL is not an Apache Beam Github " + + "Release page URL or download URL: ", + from); + + from = options.getPrismLocation(); + } + + String fromFileName = getNameWithoutExtension(from); + Path to = Paths.get(userHome(), PRISM_BIN_PATH, fromFileName); + + if (Files.exists(to)) { + return to.toString(); + } + + createDirectoryIfNeeded(to); + + if (from.startsWith("http")) { + String result = resolve(new URL(from), to); + checkState(Files.exists(to), "Resolved location does not exist: %s", result); + return result; + } + + String result = resolve(Paths.get(from), to); + checkState(Files.exists(to), "Resolved location does not exist: %s", result); + return result; + } + + private String resolve(URL from, Path to) throws IOException { + if (from.toString().startsWith(GITHUB_DOWNLOAD_PREFIX)) { + URL shaSumReference = new URL(from + "." + SHA512_EXT); + validateShaSum512(shaSumReference, from); + } + + BiConsumer downloadFn = PrismLocator::download; + if (from.getPath().endsWith(ZIP_EXT)) { + downloadFn = PrismLocator::unzip; + } + downloadFn.accept(from, to); + + Files.setPosixFilePermissions(to, PERMS); + + return to.toString(); + } + + private String resolve(Path from, Path to) throws IOException { + + BiConsumer copyFn = PrismLocator::copy; + if (from.endsWith(ZIP_EXT)) { + copyFn = PrismLocator::unzip; + } + + copyFn.accept(from.toUri().toURL().openStream(), to); + ByteStreams.copy(from.toUri().toURL().openStream(), Files.newOutputStream(to)); + Files.setPosixFilePermissions(to, PERMS); + + return to.toString(); + } + + String buildFileName() { + String version = getSDKVersion(); + return String.format("apache_beam-v%s-prism-%s-%s", version, os(), arch()); + } + + private static void unzip(URL from, Path to) { + try { + unzip(from.openStream(), to); + } catch (IOException e) { + throw new RuntimeException(e); + } + } + + private static void unzip(InputStream from, Path to) { + try (OutputStream out = Files.newOutputStream(to)) { + ZipInputStream zis = new ZipInputStream(from); + for (ZipEntry entry = zis.getNextEntry(); entry != null; entry = zis.getNextEntry()) { + InputStream in = ByteStreams.limit(zis, entry.getSize()); + ByteStreams.copy(in, out); + } + } catch (IOException e) { + throw new RuntimeException(e); + } + } + + private static void copy(InputStream from, Path to) { + try { + ByteStreams.copy(from, Files.newOutputStream(to)); + } catch (IOException e) { + throw new RuntimeException(e); + } + } + + private static void download(URL from, Path to) { + try { + ByteStreams.copy(from.openStream(), Files.newOutputStream(to)); + } catch (IOException e) { + throw new RuntimeException(e); + } + } + + private static void validateShaSum512(URL shaSumReference, URL source) throws IOException { + try (InputStream in = shaSumReference.openStream()) { + String rawContent = new String(ByteStreams.toByteArray(in), StandardCharsets.UTF_8); + checkState(!Strings.isNullOrEmpty(rawContent)); + String reference = ""; + Iterator split = Splitter.onPattern("\\s+").split(rawContent).iterator(); + if (split.hasNext()) { + reference = split.next(); + } + checkState(!Strings.isNullOrEmpty(reference)); + + HashCode toVerify = Hashing.sha512().hashBytes(ByteStreams.toByteArray(source.openStream())); + checkState( + reference.equals(toVerify.toString()), + "Expected sha512 derived from: %s does not equal expected: %s, got: %s", + source, + reference, + toVerify.toString()); + } + } + + private static String getNameWithoutExtension(String path) { + return org.apache.beam.vendor.guava.v32_1_2_jre.com.google.common.io.Files + .getNameWithoutExtension(path); + } + + private String getSDKVersion() { + if (Strings.isNullOrEmpty(options.getPrismVersionOverride())) { + return RELEASE_INFO.getSdkVersion(); + } + return options.getPrismVersionOverride(); + } + + private static String os() { + String result = mustGetPropertyAsLowerCase(OS_NAME_PROPERTY); + if (result.contains("mac")) { + return "darwin"; + } + return result; + } + + private static String arch() { + String result = mustGetPropertyAsLowerCase(ARCH_PROPERTY); + if (result.contains("aarch")) { + return "arm64"; + } + return result; + } + + private static String userHome() { + return mustGetPropertyAsLowerCase(USER_HOME_PROPERTY); + } + + private static String mustGetPropertyAsLowerCase(String name) { + return checkStateNotNull(System.getProperty(name), "System property: " + name + " not set") + .toLowerCase(); + } + + private static void createDirectoryIfNeeded(Path path) throws IOException { + Path parent = path.getParent(); + if (parent == null) { + return; + } + if (parent.toFile().exists()) { + return; + } + Files.createDirectories(parent); + } +} diff --git a/runners/prism/java/src/test/java/org/apache/beam/runners/prism/PrismLocatorTest.java b/runners/prism/java/src/test/java/org/apache/beam/runners/prism/PrismLocatorTest.java new file mode 100644 index 000000000000..e1a34bb0dc49 --- /dev/null +++ b/runners/prism/java/src/test/java/org/apache/beam/runners/prism/PrismLocatorTest.java @@ -0,0 +1,77 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.beam.runners.prism; + +import static com.google.common.truth.Truth.assertThat; +import static org.apache.beam.runners.prism.PrismRunnerTest.getLocalPrismBuildOrIgnoreTest; + +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.Paths; +import org.apache.beam.sdk.options.PipelineOptionsFactory; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.JUnit4; + +/** Tests for {@link PrismLocator}. */ +@RunWith(JUnit4.class) +public class PrismLocatorTest { + + @Test + public void givenVersionOverride_thenResolves() throws IOException { + PrismPipelineOptions options = options(); + options.setPrismVersionOverride("2.57.0"); + PrismLocator underTest = new PrismLocator(options); + String got = underTest.resolve(); + assertThat(got).contains(".apache_beam/cache/prism/bin/"); + assertThat(got).contains("2.57.0"); + Path gotPath = Paths.get(got); + assertThat(Files.exists(gotPath)).isTrue(); + Files.delete(gotPath); + } + + @Test + public void givenHttpPrismLocationOption_thenResolves() throws IOException { + PrismPipelineOptions options = options(); + options.setPrismLocation( + "https://github.com/apache/beam/releases/download/v2.57.0/apache_beam-v2.57.0-prism-darwin-arm64.zip"); + PrismLocator underTest = new PrismLocator(options); + String got = underTest.resolve(); + assertThat(got).contains(".apache_beam/cache/prism/bin/"); + Path gotPath = Paths.get(got); + assertThat(Files.exists(gotPath)).isTrue(); + Files.delete(gotPath); + } + + @Test + public void givenFilePrismLocationOption_thenResolves() throws IOException { + PrismPipelineOptions options = options(); + options.setPrismLocation(getLocalPrismBuildOrIgnoreTest()); + PrismLocator underTest = new PrismLocator(options); + String got = underTest.resolve(); + assertThat(got).contains(".apache_beam/cache/prism/bin/"); + Path gotPath = Paths.get(got); + assertThat(Files.exists(gotPath)).isTrue(); + Files.delete(gotPath); + } + + private static PrismPipelineOptions options() { + return PipelineOptionsFactory.create().as(PrismPipelineOptions.class); + } +} From e0a355ff20753d1ea800cde3ad443e9484c53e8e Mon Sep 17 00:00:00 2001 From: Damon Douglas Date: Mon, 8 Jul 2024 18:09:35 +0000 Subject: [PATCH 3/7] Sync with head --- .../org/apache/beam/runners/prism/PrismPipelineOptions.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/runners/prism/java/src/main/java/org/apache/beam/runners/prism/PrismPipelineOptions.java b/runners/prism/java/src/main/java/org/apache/beam/runners/prism/PrismPipelineOptions.java index 1b665603b8b6..ec0f8beb620a 100644 --- a/runners/prism/java/src/main/java/org/apache/beam/runners/prism/PrismPipelineOptions.java +++ b/runners/prism/java/src/main/java/org/apache/beam/runners/prism/PrismPipelineOptions.java @@ -35,7 +35,7 @@ public interface PrismPipelineOptions extends PortablePipelineOptions { void setPrismLocation(String prismLocation); @Description( - "Override the SDK\\'s version for deriving the Github Release URLs for " + "Override the SDK's version for deriving the Github Release URLs for " + "downloading a zipped prism binary, for the current platform. If " + "set to a Github Release page URL, then it will use that release page as a base when constructing the download URL.") String getPrismVersionOverride(); From c57e0391ab71b5a696c7ca13ac38c22dbe72a993 Mon Sep 17 00:00:00 2001 From: Damon Douglas Date: Tue, 9 Jul 2024 15:29:16 +0000 Subject: [PATCH 4/7] Remove redundant check --- .../main/java/org/apache/beam/runners/prism/PrismLocator.java | 3 --- 1 file changed, 3 deletions(-) diff --git a/runners/prism/java/src/main/java/org/apache/beam/runners/prism/PrismLocator.java b/runners/prism/java/src/main/java/org/apache/beam/runners/prism/PrismLocator.java index d5b148318f29..cd98ba8c75a2 100644 --- a/runners/prism/java/src/main/java/org/apache/beam/runners/prism/PrismLocator.java +++ b/runners/prism/java/src/main/java/org/apache/beam/runners/prism/PrismLocator.java @@ -245,9 +245,6 @@ private static void createDirectoryIfNeeded(Path path) throws IOException { if (parent == null) { return; } - if (parent.toFile().exists()) { - return; - } Files.createDirectories(parent); } } From f0b51a0d2b1cd457befb5306645e12f75cc4ef46 Mon Sep 17 00:00:00 2001 From: Damon Douglas Date: Wed, 10 Jul 2024 16:10:11 +0000 Subject: [PATCH 5/7] Remove sha verification; delete files in test setup --- .../beam/runners/prism/PrismLocator.java | 41 ++----------------- .../beam/runners/prism/PrismLocatorTest.java | 27 +++++++++--- 2 files changed, 26 insertions(+), 42 deletions(-) diff --git a/runners/prism/java/src/main/java/org/apache/beam/runners/prism/PrismLocator.java b/runners/prism/java/src/main/java/org/apache/beam/runners/prism/PrismLocator.java index cd98ba8c75a2..23c6d9e6faec 100644 --- a/runners/prism/java/src/main/java/org/apache/beam/runners/prism/PrismLocator.java +++ b/runners/prism/java/src/main/java/org/apache/beam/runners/prism/PrismLocator.java @@ -25,22 +25,17 @@ import java.io.InputStream; import java.io.OutputStream; import java.net.URL; -import java.nio.charset.StandardCharsets; import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.Paths; import java.nio.file.attribute.PosixFilePermission; import java.nio.file.attribute.PosixFilePermissions; -import java.util.Iterator; import java.util.Set; import java.util.function.BiConsumer; import java.util.zip.ZipEntry; import java.util.zip.ZipInputStream; import org.apache.beam.sdk.util.ReleaseInfo; -import org.apache.beam.vendor.guava.v32_1_2_jre.com.google.common.base.Splitter; import org.apache.beam.vendor.guava.v32_1_2_jre.com.google.common.base.Strings; -import org.apache.beam.vendor.guava.v32_1_2_jre.com.google.common.hash.HashCode; -import org.apache.beam.vendor.guava.v32_1_2_jre.com.google.common.hash.Hashing; import org.apache.beam.vendor.guava.v32_1_2_jre.com.google.common.io.ByteStreams; /** @@ -55,9 +50,8 @@ class PrismLocator { static final String USER_HOME_PROPERTY = "user.home"; private static final String ZIP_EXT = "zip"; - private static final String SHA512_EXT = "sha512"; private static final ReleaseInfo RELEASE_INFO = ReleaseInfo.getReleaseInfo(); - private static final String PRISM_BIN_PATH = ".apache_beam/cache/prism/bin"; + static final String PRISM_BIN_PATH = ".apache_beam/cache/prism/bin"; private static final Set PERMS = PosixFilePermissions.fromString("rwxr-xr-x"); private static final String GITHUB_DOWNLOAD_PREFIX = @@ -71,9 +65,8 @@ class PrismLocator { } /** - * Downloads and prepares a Prism executable for use with the {@link PrismRunner}, executed by the - * {@link PrismExecutor}. The returned {@link String} is the absolute path to the Prism - * executable. + * Downloads and prepares a Prism executable for use with the {@link PrismRunner}. The returned + * {@link String} is the absolute path to the Prism executable. */ String resolve() throws IOException { @@ -111,11 +104,6 @@ String resolve() throws IOException { } private String resolve(URL from, Path to) throws IOException { - if (from.toString().startsWith(GITHUB_DOWNLOAD_PREFIX)) { - URL shaSumReference = new URL(from + "." + SHA512_EXT); - validateShaSum512(shaSumReference, from); - } - BiConsumer downloadFn = PrismLocator::download; if (from.getPath().endsWith(ZIP_EXT)) { downloadFn = PrismLocator::unzip; @@ -182,27 +170,6 @@ private static void download(URL from, Path to) { } } - private static void validateShaSum512(URL shaSumReference, URL source) throws IOException { - try (InputStream in = shaSumReference.openStream()) { - String rawContent = new String(ByteStreams.toByteArray(in), StandardCharsets.UTF_8); - checkState(!Strings.isNullOrEmpty(rawContent)); - String reference = ""; - Iterator split = Splitter.onPattern("\\s+").split(rawContent).iterator(); - if (split.hasNext()) { - reference = split.next(); - } - checkState(!Strings.isNullOrEmpty(reference)); - - HashCode toVerify = Hashing.sha512().hashBytes(ByteStreams.toByteArray(source.openStream())); - checkState( - reference.equals(toVerify.toString()), - "Expected sha512 derived from: %s does not equal expected: %s, got: %s", - source, - reference, - toVerify.toString()); - } - } - private static String getNameWithoutExtension(String path) { return org.apache.beam.vendor.guava.v32_1_2_jre.com.google.common.io.Files .getNameWithoutExtension(path); @@ -231,7 +198,7 @@ private static String arch() { return result; } - private static String userHome() { + static String userHome() { return mustGetPropertyAsLowerCase(USER_HOME_PROPERTY); } diff --git a/runners/prism/java/src/test/java/org/apache/beam/runners/prism/PrismLocatorTest.java b/runners/prism/java/src/test/java/org/apache/beam/runners/prism/PrismLocatorTest.java index e1a34bb0dc49..0644e910fc21 100644 --- a/runners/prism/java/src/test/java/org/apache/beam/runners/prism/PrismLocatorTest.java +++ b/runners/prism/java/src/test/java/org/apache/beam/runners/prism/PrismLocatorTest.java @@ -18,13 +18,19 @@ package org.apache.beam.runners.prism; import static com.google.common.truth.Truth.assertThat; +import static org.apache.beam.runners.prism.PrismLocator.PRISM_BIN_PATH; +import static org.apache.beam.runners.prism.PrismLocator.userHome; import static org.apache.beam.runners.prism.PrismRunnerTest.getLocalPrismBuildOrIgnoreTest; import java.io.IOException; +import java.nio.file.FileVisitResult; import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.Paths; +import java.nio.file.SimpleFileVisitor; +import java.nio.file.attribute.BasicFileAttributes; import org.apache.beam.sdk.options.PipelineOptionsFactory; +import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.JUnit4; @@ -33,17 +39,30 @@ @RunWith(JUnit4.class) public class PrismLocatorTest { + @Before + public void setup() throws IOException { + Files.walkFileTree( + Paths.get(userHome(), PRISM_BIN_PATH), + new SimpleFileVisitor() { + @Override + public FileVisitResult visitFile(Path file, BasicFileAttributes attrs) + throws IOException { + Files.delete(file); + return FileVisitResult.CONTINUE; + } + }); + } + @Test public void givenVersionOverride_thenResolves() throws IOException { PrismPipelineOptions options = options(); options.setPrismVersionOverride("2.57.0"); PrismLocator underTest = new PrismLocator(options); String got = underTest.resolve(); - assertThat(got).contains(".apache_beam/cache/prism/bin/"); + assertThat(got).contains(PRISM_BIN_PATH); assertThat(got).contains("2.57.0"); Path gotPath = Paths.get(got); assertThat(Files.exists(gotPath)).isTrue(); - Files.delete(gotPath); } @Test @@ -56,7 +75,6 @@ public void givenHttpPrismLocationOption_thenResolves() throws IOException { assertThat(got).contains(".apache_beam/cache/prism/bin/"); Path gotPath = Paths.get(got); assertThat(Files.exists(gotPath)).isTrue(); - Files.delete(gotPath); } @Test @@ -65,10 +83,9 @@ public void givenFilePrismLocationOption_thenResolves() throws IOException { options.setPrismLocation(getLocalPrismBuildOrIgnoreTest()); PrismLocator underTest = new PrismLocator(options); String got = underTest.resolve(); - assertThat(got).contains(".apache_beam/cache/prism/bin/"); + assertThat(got).contains(PRISM_BIN_PATH); Path gotPath = Paths.get(got); assertThat(Files.exists(gotPath)).isTrue(); - Files.delete(gotPath); } private static PrismPipelineOptions options() { From d4091521e53f3a6e6f7e886c76b3dd7a1c7972de Mon Sep 17 00:00:00 2001 From: Damon Douglas Date: Wed, 10 Jul 2024 16:41:26 +0000 Subject: [PATCH 6/7] Remove destination dir; check exists --- .../beam/runners/prism/PrismLocator.java | 8 ++++++-- .../beam/runners/prism/PrismLocatorTest.java | 18 ++++++++++++------ 2 files changed, 18 insertions(+), 8 deletions(-) diff --git a/runners/prism/java/src/main/java/org/apache/beam/runners/prism/PrismLocator.java b/runners/prism/java/src/main/java/org/apache/beam/runners/prism/PrismLocator.java index 23c6d9e6faec..f32e4d88f42b 100644 --- a/runners/prism/java/src/main/java/org/apache/beam/runners/prism/PrismLocator.java +++ b/runners/prism/java/src/main/java/org/apache/beam/runners/prism/PrismLocator.java @@ -51,7 +51,7 @@ class PrismLocator { private static final String ZIP_EXT = "zip"; private static final ReleaseInfo RELEASE_INFO = ReleaseInfo.getReleaseInfo(); - static final String PRISM_BIN_PATH = ".apache_beam/cache/prism/bin"; + private static final String PRISM_BIN_PATH = ".apache_beam/cache/prism/bin"; private static final Set PERMS = PosixFilePermissions.fromString("rwxr-xr-x"); private static final String GITHUB_DOWNLOAD_PREFIX = @@ -103,6 +103,10 @@ String resolve() throws IOException { return result; } + static Path prismBinDirectory() { + return Paths.get(userHome(), PRISM_BIN_PATH); + } + private String resolve(URL from, Path to) throws IOException { BiConsumer downloadFn = PrismLocator::download; if (from.getPath().endsWith(ZIP_EXT)) { @@ -198,7 +202,7 @@ private static String arch() { return result; } - static String userHome() { + private static String userHome() { return mustGetPropertyAsLowerCase(USER_HOME_PROPERTY); } diff --git a/runners/prism/java/src/test/java/org/apache/beam/runners/prism/PrismLocatorTest.java b/runners/prism/java/src/test/java/org/apache/beam/runners/prism/PrismLocatorTest.java index 0644e910fc21..69c558eb983f 100644 --- a/runners/prism/java/src/test/java/org/apache/beam/runners/prism/PrismLocatorTest.java +++ b/runners/prism/java/src/test/java/org/apache/beam/runners/prism/PrismLocatorTest.java @@ -18,8 +18,7 @@ package org.apache.beam.runners.prism; import static com.google.common.truth.Truth.assertThat; -import static org.apache.beam.runners.prism.PrismLocator.PRISM_BIN_PATH; -import static org.apache.beam.runners.prism.PrismLocator.userHome; +import static org.apache.beam.runners.prism.PrismLocator.prismBinDirectory; import static org.apache.beam.runners.prism.PrismRunnerTest.getLocalPrismBuildOrIgnoreTest; import java.io.IOException; @@ -39,10 +38,12 @@ @RunWith(JUnit4.class) public class PrismLocatorTest { + private static final Path DESTINATION_DIRECTORY = prismBinDirectory(); + @Before public void setup() throws IOException { Files.walkFileTree( - Paths.get(userHome(), PRISM_BIN_PATH), + DESTINATION_DIRECTORY, new SimpleFileVisitor() { @Override public FileVisitResult visitFile(Path file, BasicFileAttributes attrs) @@ -51,15 +52,18 @@ public FileVisitResult visitFile(Path file, BasicFileAttributes attrs) return FileVisitResult.CONTINUE; } }); + + Files.deleteIfExists(DESTINATION_DIRECTORY); } @Test public void givenVersionOverride_thenResolves() throws IOException { + assertThat(Files.exists(DESTINATION_DIRECTORY)).isFalse(); PrismPipelineOptions options = options(); options.setPrismVersionOverride("2.57.0"); PrismLocator underTest = new PrismLocator(options); String got = underTest.resolve(); - assertThat(got).contains(PRISM_BIN_PATH); + assertThat(got).contains(DESTINATION_DIRECTORY.toString()); assertThat(got).contains("2.57.0"); Path gotPath = Paths.get(got); assertThat(Files.exists(gotPath)).isTrue(); @@ -67,23 +71,25 @@ public void givenVersionOverride_thenResolves() throws IOException { @Test public void givenHttpPrismLocationOption_thenResolves() throws IOException { + assertThat(Files.exists(DESTINATION_DIRECTORY)).isFalse(); PrismPipelineOptions options = options(); options.setPrismLocation( "https://github.com/apache/beam/releases/download/v2.57.0/apache_beam-v2.57.0-prism-darwin-arm64.zip"); PrismLocator underTest = new PrismLocator(options); String got = underTest.resolve(); - assertThat(got).contains(".apache_beam/cache/prism/bin/"); + assertThat(got).contains(DESTINATION_DIRECTORY.toString()); Path gotPath = Paths.get(got); assertThat(Files.exists(gotPath)).isTrue(); } @Test public void givenFilePrismLocationOption_thenResolves() throws IOException { + assertThat(Files.exists(DESTINATION_DIRECTORY)).isFalse(); PrismPipelineOptions options = options(); options.setPrismLocation(getLocalPrismBuildOrIgnoreTest()); PrismLocator underTest = new PrismLocator(options); String got = underTest.resolve(); - assertThat(got).contains(PRISM_BIN_PATH); + assertThat(got).contains(DESTINATION_DIRECTORY.toString()); Path gotPath = Paths.get(got); assertThat(Files.exists(gotPath)).isTrue(); } From 89a6c037a529e50d2386797e85c492a33b1d2255 Mon Sep 17 00:00:00 2001 From: Damon Douglas Date: Wed, 10 Jul 2024 16:56:25 +0000 Subject: [PATCH 7/7] Add tests for 404 and tag prefix --- .../beam/runners/prism/PrismLocatorTest.java | 47 ++++++++++++++----- 1 file changed, 36 insertions(+), 11 deletions(-) diff --git a/runners/prism/java/src/test/java/org/apache/beam/runners/prism/PrismLocatorTest.java b/runners/prism/java/src/test/java/org/apache/beam/runners/prism/PrismLocatorTest.java index 69c558eb983f..982a8bfd657c 100644 --- a/runners/prism/java/src/test/java/org/apache/beam/runners/prism/PrismLocatorTest.java +++ b/runners/prism/java/src/test/java/org/apache/beam/runners/prism/PrismLocatorTest.java @@ -20,6 +20,7 @@ import static com.google.common.truth.Truth.assertThat; import static org.apache.beam.runners.prism.PrismLocator.prismBinDirectory; import static org.apache.beam.runners.prism.PrismRunnerTest.getLocalPrismBuildOrIgnoreTest; +import static org.junit.Assert.assertThrows; import java.io.IOException; import java.nio.file.FileVisitResult; @@ -42,18 +43,20 @@ public class PrismLocatorTest { @Before public void setup() throws IOException { - Files.walkFileTree( - DESTINATION_DIRECTORY, - new SimpleFileVisitor() { - @Override - public FileVisitResult visitFile(Path file, BasicFileAttributes attrs) - throws IOException { - Files.delete(file); - return FileVisitResult.CONTINUE; - } - }); + if (Files.exists(DESTINATION_DIRECTORY)) { + Files.walkFileTree( + DESTINATION_DIRECTORY, + new SimpleFileVisitor() { + @Override + public FileVisitResult visitFile(Path file, BasicFileAttributes attrs) + throws IOException { + Files.delete(file); + return FileVisitResult.CONTINUE; + } + }); - Files.deleteIfExists(DESTINATION_DIRECTORY); + Files.delete(DESTINATION_DIRECTORY); + } } @Test @@ -94,6 +97,28 @@ public void givenFilePrismLocationOption_thenResolves() throws IOException { assertThat(Files.exists(gotPath)).isTrue(); } + @Test + public void givenGithubTagPrismLocationOption_thenThrows() { + PrismPipelineOptions options = options(); + options.setPrismLocation( + "https://github.com/apache/beam/releases/tag/v2.57.0/apache_beam-v2.57.0-prism-darwin-amd64.zip"); + PrismLocator underTest = new PrismLocator(options); + IllegalArgumentException error = + assertThrows(IllegalArgumentException.class, underTest::resolve); + assertThat(error.getMessage()) + .contains( + "Provided --prismLocation URL is not an Apache Beam Github Release page URL or download URL"); + } + + @Test + public void givenPrismLocation404_thenThrows() { + PrismPipelineOptions options = options(); + options.setPrismLocation("https://example.com/i/dont/exist.zip"); + PrismLocator underTest = new PrismLocator(options); + RuntimeException error = assertThrows(RuntimeException.class, underTest::resolve); + assertThat(error.getMessage()).contains("NotFoundException"); + } + private static PrismPipelineOptions options() { return PipelineOptionsFactory.create().as(PrismPipelineOptions.class); }