Change the FF --jar option into --classpath.

Instead of requiring the caller to provide a single self-contained jar,
we can allow a general classpath. That means for example that we could
provide the jar-file or classes directory from the compilation of a user's
function, plus the jars containing the function's dependencies.

PiperOrigin-RevId: 293704294

Author: eamonnmcmanus

invoker/core/src/main/java/com/google/cloud/functions/invoker/runner/Invoker.java

@@ -14,6 +14,8 @@
 
 package com.google.cloud.functions.invoker.runner;
 
+import static java.util.stream.Collectors.toList;
+
 import com.beust.jcommander.JCommander;
 import com.beust.jcommander.Parameter;
 import com.beust.jcommander.ParameterException;
@@ -28,13 +30,18 @@
 import com.google.cloud.functions.invoker.NewHttpFunctionExecutor;
 import java.io.File;
 import java.io.IOException;
+import java.io.UncheckedIOException;
 import java.lang.reflect.Method;
+import java.net.MalformedURLException;
 import java.net.URL;
 import java.net.URLClassLoader;
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.nio.file.Paths;
+import java.util.ArrayList;
 import java.util.Arrays;
+import java.util.Collections;
+import java.util.List;
 import java.util.Map;
 import java.util.Objects;
 import java.util.Optional;
@@ -91,13 +98,18 @@ private static class Options {
         System.getenv().getOrDefault("FUNCTION_TARGET", "TestFunction.function");
 
     @Parameter(
-        description = "Name of a jar file that contains the function to execute. This must be"
-            + " self-contained: either it must be a \"fat jar\" which bundles the dependencies"
-            + " of all of the function code, or it must use the Class-Path attribute in the jar"
-            + " manifest to point to those dependencies.",
-        names = "--jar"
+        description = "List of files or directories where the compiled Java classes making up"
+            + " the function will be found. This functions like the -classpath option to the"
+            + " java command. It is a list of filenames separated by '${path.separator}'."
+            + " If an entry in the list names a directory then the class foo.bar.Baz will be looked"
+            + " for in foo${file.separator}bar${file.separator}Baz.class under that"
+            + " directory. If an entry in the list names a file and that file is a jar file then"
+            + " class foo.bar.Baz will be looked for in an entry foo/bar/Baz.class in that jar"
+            + " file. If an entry is a directory followed by '${file.separator}*' then every file"
+            + " in the directory whose name ends with '.jar' will be searched for classes.",
+        names = "--classpath"
     )
-    private String jar = null;
+    private String classPath = null;
 
     @Parameter(
         names = "--help", help = true
@@ -124,12 +136,12 @@ static Optional<Invoker> makeInvoker(Map<String, String> environment, String...
     try {
       jCommander.parse(args);
     } catch (ParameterException e) {
-      jCommander.usage();
+      usage(jCommander);
       throw e;
     }
 
     if (options.help) {
-      jCommander.usage();
+      usage(jCommander);
       return Optional.empty();
     }
 
@@ -138,15 +150,15 @@ static Optional<Invoker> makeInvoker(Map<String, String> environment, String...
       port = Integer.parseInt(options.port);
     } catch (NumberFormatException e) {
       System.err.println("--port value should be an integer: " + options.port);
-      jCommander.usage();
+      usage(jCommander);
       throw e;
     }
     String functionTarget = options.target;
     Path standardFunctionJarPath = Paths.get("function/function.jar");
-    Optional<String> functionJarPath =
+    Optional<String> functionClasspath =
         Arrays.asList(
-            options.jar,
-            environment.get("FUNCTION_JAR"),
+            options.classPath,
+            environment.get("FUNCTION_CLASSPATH"),
             Files.exists(standardFunctionJarPath) ? standardFunctionJarPath.toString() : null)
             .stream()
             .filter(Objects::nonNull)
@@ -156,28 +168,37 @@ static Optional<Invoker> makeInvoker(Map<String, String> environment, String...
             port,
             functionTarget,
             environment.get("FUNCTION_SIGNATURE_TYPE"),
-            functionJarPath);
+            functionClasspath);
     return Optional.of(invoker);
   }
 
+  private static void usage(JCommander jCommander) {
+    StringBuilder usageBuilder = new StringBuilder();
+    jCommander.getUsageFormatter().usage(usageBuilder);
+    String usage = usageBuilder.toString()
+        .replace("${file.separator}", File.separator)
+        .replace("${path.separator}", File.pathSeparator);
+    jCommander.getConsole().println(usage);
+  }
+
   private static boolean isLocalRun() {
     return System.getenv("K_SERVICE") == null;
   }
 
   private final Integer port;
   private final String functionTarget;
   private final String functionSignatureType;
-  private final Optional<String> functionJarPath;
+  private final Optional<String> functionClasspath;
 
   public Invoker(
       Integer port,
       String functionTarget,
       String functionSignatureType,
-      Optional<String> functionJarPath) {
+      Optional<String> functionClasspath) {
     this.port = port;
     this.functionTarget = functionTarget;
     this.functionSignatureType = functionSignatureType;
-    this.functionJarPath = functionJarPath;
+    this.functionClasspath = functionClasspath;
   }
 
   Integer getPort() {
@@ -192,8 +213,8 @@ String getFunctionSignatureType() {
     return functionSignatureType;
   }
 
-  Optional<String> getFunctionJarPath() {
-    return functionJarPath;
+  Optional<String> getFunctionClasspath() {
+    return functionClasspath;
   }
 
   public void startServer() throws Exception {
@@ -203,21 +224,11 @@ public void startServer() throws Exception {
     context.setContextPath("/");
     server.setHandler(context);
 
-    Optional<File> functionJarFile =
-        functionJarPath.isPresent()
-            ? Optional.of(new File(functionJarPath.get()))
-            : Optional.empty();
-    if (functionJarFile.isPresent() && !functionJarFile.get().exists()) {
-      throw new IllegalArgumentException(
-          "functionJarPath points to an non-existent file: "
-              + functionJarFile.get().getAbsolutePath());
-    }
-
     ClassLoader runtimeLoader = getClass().getClassLoader();
     ClassLoader classLoader;
-    if (functionJarFile.isPresent()) {
+    if (functionClasspath.isPresent()) {
       ClassLoader parent = new OnlyApiClassLoader(runtimeLoader);
-      classLoader = new URLClassLoader(new URL[]{functionJarFile.get().toURI().toURL()}, parent);
+      classLoader = new URLClassLoader(classpathToUrls(functionClasspath.get()), parent);
     } else {
       classLoader = runtimeLoader;
     }
@@ -279,6 +290,39 @@ public void startServer() throws Exception {
     server.join();
   }
 
+  static URL[] classpathToUrls(String classpath) throws IOException {
+    String[] components = classpath.split(File.pathSeparator);
+    List<URL> urls = new ArrayList<>();
+    for (String component : components) {
+      if (component.endsWith(File.separator + "*")) {
+        urls.addAll(jarsIn(component.substring(0, component.length() - 2)));
+      } else {
+        Path path = Paths.get(component);
+        if (Files.exists(path)) {
+          urls.add(path.toUri().toURL());
+        }
+      }
+    }
+    return urls.toArray(new URL[0]);
+  }
+
+  private static List<URL> jarsIn(String dir) throws IOException {
+    Path path = Paths.get(dir);
+    if (!Files.isDirectory(path)) {
+      return Collections.emptyList();
+    }
+    return Files.list(path)
+        .filter(p -> p.getFileName().toString().endsWith(".jar"))
+        .map(p -> {
+          try {
+            return p.toUri().toURL();
+          } catch (MalformedURLException e) {
+            throw new UncheckedIOException(e);
+          }
+        })
+        .collect(toList());
+  }
+
   private void logServerInfo() {
     if (isLocalRun()) {
       logger.log(Level.INFO, "Serving function...");

invoker/core/src/test/java/com/google/cloud/functions/invoker/IntegrationTest.java

@@ -330,24 +330,24 @@ private String functionJarString() throws IOException {
   }
 
   /**
-   * Tests that if we launch an HTTP function with {@code --jar}, then the function code cannot
-   * see the classes from the runtime. This is allows us to avoid conflicts between versions of
-   * libraries that we use in the runtime and different versions of the same libraries that the
+   * Tests that if we launch an HTTP function with {@code --classpath}, then the function code
+   * cannot see the classes from the runtime. This is allows us to avoid conflicts between versions
+   * of libraries that we use in the runtime and different versions of the same libraries that the
    * function might use.
    */
   @Test
-  public void jarOptionHttp() throws Exception {
+  public void classpathOptionHttp() throws Exception {
     testHttpFunction("com.example.functionjar.Foreground",
-        ImmutableList.of("--jar", functionJarString()),
+        ImmutableList.of("--classpath", functionJarString()),
         TestCase.builder()
             .setUrl("/?class=" + INTERNAL_CLASS.getName())
             .setExpectedResponseText("OK")
             .build());
   }
 
-  /** Like {@link #jarOptionHttp} but for background functions. */
+  /** Like {@link #classpathOptionHttp} but for background functions. */
   @Test
-  public void jarOptionBackground() throws Exception {
+  public void classpathOptionBackground() throws Exception {
     Gson gson = new Gson();
     URL resourceUrl = getClass().getResource("/adder_gcf_ga_event.json");
     assertThat(resourceUrl).isNotNull();
@@ -356,7 +356,7 @@ public void jarOptionBackground() throws Exception {
     JsonObject jsonData = json.getAsJsonObject("data");
     jsonData.addProperty("class", INTERNAL_CLASS.getName());
     testBackgroundFunction("com.example.functionjar.Background",
-        ImmutableList.of("--jar", functionJarString()),
+        ImmutableList.of("--classpath", functionJarString()),
         TestCase.builder().setRequestText(json.toString()).build());
   }
 

invoker/core/src/test/java/com/google/cloud/functions/invoker/runner/InvokerTest.java

@@ -1,12 +1,16 @@
 package com.google.cloud.functions.invoker.runner;
 
 import static com.google.common.truth.Truth.assertThat;
+import static com.google.common.truth.Truth.assertWithMessage;
 import static com.google.common.truth.Truth8.assertThat;
 
 import java.io.ByteArrayOutputStream;
+import java.io.File;
 import java.io.IOException;
 import java.io.PrintStream;
+import java.net.URL;
 import java.nio.charset.StandardCharsets;
+import java.util.Arrays;
 import java.util.Collections;
 import java.util.Map;
 import java.util.Optional;
@@ -24,6 +28,7 @@ public void help() throws IOException {
     });
     assertThat(help).contains("Usage:");
     assertThat(help).contains("--target");
+    assertThat(help).containsMatch("separated\\s+by\\s+'" + File.pathSeparator + "'");
   }
 
   @Test
@@ -64,22 +69,40 @@ public void explicitSignatureType() {
   }
 
   @Test
-  public void defaultJar() {
+  public void defaultClasspath() {
     Optional<Invoker> invoker = Invoker.makeInvoker();
-    assertThat(invoker.get().getFunctionJarPath()).isEmpty();
+    assertThat(invoker.get().getFunctionClasspath()).isEmpty();
   }
 
+  private static final String FAKE_CLASSPATH =
+      "/foo/bar/baz.jar" + File.pathSeparator + "/some/directory";
+
   @Test
-  public void explicitJarViaEnvironment() {
-    Map<String, String> env = Collections.singletonMap("FUNCTION_JAR", "/foo/bar/baz.jar");
+  public void explicitClasspathViaEnvironment() {
+    Map<String, String> env = Collections.singletonMap("FUNCTION_CLASSPATH", FAKE_CLASSPATH);
     Optional<Invoker> invoker = Invoker.makeInvoker(env);
-    assertThat(invoker.get().getFunctionJarPath()).hasValue("/foo/bar/baz.jar");
+    assertThat(invoker.get().getFunctionClasspath()).hasValue(FAKE_CLASSPATH);
   }
 
   @Test
-  public void explicitJarViaOption() {
-    Optional<Invoker> invoker = Invoker.makeInvoker("--jar", "/foo/bar/baz.jar");
-    assertThat(invoker.get().getFunctionJarPath()).hasValue("/foo/bar/baz.jar");
+  public void explicitClasspathViaOption() {
+    Optional<Invoker> invoker = Invoker.makeInvoker("--classpath", FAKE_CLASSPATH);
+    assertThat(invoker.get().getFunctionClasspath()).hasValue(FAKE_CLASSPATH);
+  }
+
+  @Test
+  public void classpathToUrls() throws Exception {
+    String classpath =
+        "../testfunction/target/test-classes" + File.pathSeparator + "../testfunction/target/lib/*";
+    URL[] urls = Invoker.classpathToUrls(classpath);
+    assertWithMessage(Arrays.toString(urls)).that(urls.length).isGreaterThan(2);
+    File classesDir = new File(urls[0].toURI());
+    assertWithMessage(classesDir.toString()).that(classesDir.isDirectory()).isTrue();
+    for (int i = 1; i < urls.length; i++) {
+      URL url = urls[i];
+      assertThat(url.toString()).endsWith(".jar");
+      assertWithMessage(url.toString()).that(new File(url.toURI()).isFile()).isTrue();
+    }
   }
 
   private static String captureOutput(Runnable operation) throws IOException {