Add a mechanism for build event protocol events to upload files

This should be a no-op, mostly replacing PathConverter with
BuildEventArtifactUploader, since none of the implementations perform any
upload yet.

PiperOrigin-RevId: 200685325

9 files changed, 109 insertions, 46 deletions

diff --git a/src/test/java/com/google/devtools/build/lib/BUILD b/src/test/java/com/google/devtools/build/lib/BUILD
index 4d2f69b321..769ac2595e 100644
--- a/src/test/java/com/google/devtools/build/lib/BUILD
+++ b/src/test/java/com/google/devtools/build/lib/BUILD
@@ -43,6 +43,7 @@ filegroup(
         "//src/test/java/com/google/devtools/build/lib/bazel/repository/downloader:srcs",
         "//src/test/java/com/google/devtools/build/lib/bazel/repository:srcs",
         "//src/test/java/com/google/devtools/build/lib/buildeventservice:srcs",
+        "//src/test/java/com/google/devtools/build/lib/buildeventstream:srcs",
         "//src/test/java/com/google/devtools/build/lib/buildeventstream/transports:srcs",
         "//src/test/java/com/google/devtools/build/lib/buildtool:srcs",
         "//src/test/java/com/google/devtools/build/lib/profiler/callcounts:srcs",
@@ -1248,6 +1249,7 @@ java_test(
         "//src/main/java/com/google/devtools/build/lib:util",
         "//src/main/java/com/google/devtools/build/lib/actions",
         "//src/main/java/com/google/devtools/build/lib/authandtls",
+        "//src/main/java/com/google/devtools/build/lib/buildeventstream",
         "//src/main/java/com/google/devtools/build/lib/clock",
         "//src/main/java/com/google/devtools/build/lib/remote",
         "//src/main/java/com/google/devtools/build/lib/remote/blobstore",
diff --git a/src/test/java/com/google/devtools/build/lib/buildeventservice/BazelBuildEventServiceModuleTest.java b/src/test/java/com/google/devtools/build/lib/buildeventservice/BazelBuildEventServiceModuleTest.java
index 9811f41d95..37ac8f6c95 100644
--- a/src/test/java/com/google/devtools/build/lib/buildeventservice/BazelBuildEventServiceModuleTest.java
+++ b/src/test/java/com/google/devtools/build/lib/buildeventservice/BazelBuildEventServiceModuleTest.java
@@ -26,8 +26,9 @@ import com.google.devtools.build.lib.actions.ActionExecutedEvent;
 import com.google.devtools.build.lib.actions.ActionExecutedEvent.ErrorTiming;
 import com.google.devtools.build.lib.actions.util.ActionsTestUtil;
 import com.google.devtools.build.lib.authandtls.AuthAndTLSOptions;
+import com.google.devtools.build.lib.buildeventstream.BuildEventArtifactUploader;
+import com.google.devtools.build.lib.buildeventstream.BuildEventArtifactUploaderMap;
 import com.google.devtools.build.lib.buildeventstream.BuildEventProtocolOptions;
-import com.google.devtools.build.lib.buildeventstream.PathConverter;
 import com.google.devtools.build.lib.buildeventstream.transports.BinaryFormatFileTransport;
 import com.google.devtools.build.lib.buildeventstream.transports.BuildEventStreamOptions;
 import com.google.devtools.build.lib.buildeventstream.transports.JsonFormatFileTransport;
@@ -38,7 +39,6 @@ import com.google.devtools.build.lib.events.Reporter;
 import com.google.devtools.build.lib.runtime.BlazeModule.ModuleEnvironment;
 import com.google.devtools.build.lib.runtime.BuildEventStreamer;
 import com.google.devtools.build.lib.runtime.Command;
-import com.google.devtools.build.lib.vfs.Path;
 import com.google.devtools.common.options.Options;
 import com.google.devtools.common.options.OptionsBase;
 import com.google.devtools.common.options.OptionsParser;
@@ -74,14 +74,6 @@ public class BazelBuildEventServiceModuleTest {
         }
       };
 
-  private static final PathConverter PATH_CONVERTER =
-      new PathConverter() {
-        @Override
-        public String apply(Path path) {
-          return path.getPathString();
-        }
-      };
-
   private Reporter reporter;
 
   private BuildEventServiceOptions besOptions;
@@ -146,7 +138,9 @@ public class BazelBuildEventServiceModuleTest {
         commandLineReporter,
         moduleEnvironment,
         clock,
-        PATH_CONVERTER,
+        new BuildEventArtifactUploaderMap.Builder()
+            .add("", BuildEventArtifactUploader.LOCAL_FILES_UPLOADER)
+            .build(),
         reporter,
         /* buildRequestId= */ "foo",
         /* invocationId= */ "bar",
diff --git a/src/test/java/com/google/devtools/build/lib/buildeventstream/BUILD b/src/test/java/com/google/devtools/build/lib/buildeventstream/BUILD
new file mode 100644
index 0000000000..586bdff2c0
--- /dev/null
+++ b/src/test/java/com/google/devtools/build/lib/buildeventstream/BUILD
@@ -0,0 +1,25 @@
+package(
+    default_testonly = 1,
+    default_visibility = ["//src:__subpackages__"],
+)
+
+filegroup(
+    name = "srcs",
+    testonly = 0,
+    srcs = glob(["**"]),
+    visibility = ["//src/test/java/com/google/devtools/build/lib:__pkg__"],
+)
+
+java_test(
+    name = "BuildEventStreamTest",
+    srcs = glob(["*.java"]),
+    test_class = "com.google.devtools.build.lib.AllTests",
+    runtime_deps = ["//src/test/java/com/google/devtools/build/lib:test_runner"],
+    deps = [
+        "//src/main/java/com/google/devtools/build/lib/buildeventstream",
+        "//third_party:guava",
+        "//third_party:junit4",
+        "//third_party:mockito",
+        "//third_party:truth",
+    ],
+)
diff --git a/src/test/java/com/google/devtools/build/lib/buildeventstream/PathConverterTest.java b/src/test/java/com/google/devtools/build/lib/buildeventstream/PathConverterTest.java
new file mode 100644
index 0000000000..3601e3746a
--- /dev/null
+++ b/src/test/java/com/google/devtools/build/lib/buildeventstream/PathConverterTest.java
@@ -0,0 +1,39 @@
+// Copyright 2018 The Bazel Authors. All rights reserved.
+//
+// Licensed 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 com.google.devtools.build.lib.buildeventstream;
+
+import static com.google.common.truth.Truth.assertThat;
+
+import com.google.devtools.build.lib.buildeventstream.PathConverter.FileUriPathConverter;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.JUnit4;
+
+/** Tests for {@link PathConverter}. */
+@RunWith(JUnit4.class)
+public class PathConverterTest {
+
+  @Test
+  public void testPathToUriString() {
+    assertThat(FileUriPathConverter.pathToUriString("/foo/bar"))
+        .isEqualTo("file:///foo/bar");
+  }
+
+  @Test
+  public void testWIndowsPathWithWhitespace() {
+    // See https://blogs.msdn.microsoft.com/ie/2006/12/06/file-uris-in-windows/
+    assertThat(FileUriPathConverter.pathToUriString("C:/Temp/Foo Bar.txt"))
+        .isEqualTo("file:///C:/Temp/Foo%20Bar.txt");
+  }
+}
diff --git a/src/test/java/com/google/devtools/build/lib/buildeventstream/transports/BinaryFormatFileTransportTest.java b/src/test/java/com/google/devtools/build/lib/buildeventstream/transports/BinaryFormatFileTransportTest.java
index 51951e4f8a..1b85701a9b 100644
--- a/src/test/java/com/google/devtools/build/lib/buildeventstream/transports/BinaryFormatFileTransportTest.java
+++ b/src/test/java/com/google/devtools/build/lib/buildeventstream/transports/BinaryFormatFileTransportTest.java
@@ -15,6 +15,7 @@
 package com.google.devtools.build.lib.buildeventstream.transports;
 
 import static com.google.common.truth.Truth.assertThat;
+import static com.google.devtools.build.lib.buildeventstream.BuildEventArtifactUploader.LOCAL_FILES_UPLOADER;
 import static org.mockito.Mockito.when;
 
 import com.google.devtools.build.lib.buildeventstream.ArtifactGroupNamer;
@@ -25,7 +26,6 @@ import com.google.devtools.build.lib.buildeventstream.BuildEventStreamProtos;
 import com.google.devtools.build.lib.buildeventstream.BuildEventStreamProtos.BuildStarted;
 import com.google.devtools.build.lib.buildeventstream.BuildEventStreamProtos.Progress;
 import com.google.devtools.build.lib.buildeventstream.BuildEventStreamProtos.TargetComplete;
-import com.google.devtools.build.lib.buildeventstream.PathConverter;
 import com.google.devtools.common.options.Options;
 import java.io.File;
 import java.io.FileInputStream;
@@ -43,7 +43,7 @@ import org.mockito.Mock;
 import org.mockito.Mockito;
 import org.mockito.MockitoAnnotations;
 
-/** Tests {@link BinaryFormatFileTransport}. **/
+/** Tests {@link BinaryFormatFileTransport}. */
 @RunWith(JUnit4.class)
 public class BinaryFormatFileTransportTest {
   private final BuildEventProtocolOptions defaultOpts =
@@ -53,7 +53,6 @@ public class BinaryFormatFileTransportTest {
 
   @Mock public BuildEvent buildEvent;
 
-  @Mock public PathConverter pathConverter;
   @Mock public ArtifactGroupNamer artifactGroupNamer;
 
   @Before
@@ -76,7 +75,8 @@ public class BinaryFormatFileTransportTest {
             .build();
     when(buildEvent.asStreamProto(Matchers.<BuildEventContext>any())).thenReturn(started);
     BinaryFormatFileTransport transport =
-        new BinaryFormatFileTransport(output.getAbsolutePath(), defaultOpts, pathConverter);
+        new BinaryFormatFileTransport(
+            output.getAbsolutePath(), defaultOpts, LOCAL_FILES_UPLOADER, (e) -> {});
     transport.sendBuildEvent(buildEvent, artifactGroupNamer);
 
     BuildEventStreamProtos.BuildEvent progress =
@@ -113,7 +113,7 @@ public class BinaryFormatFileTransportTest {
             .build();
     when(buildEvent.asStreamProto(Matchers.<BuildEventContext>any())).thenReturn(started);
     BinaryFormatFileTransport transport =
-        new BinaryFormatFileTransport(path, defaultOpts, pathConverter);
+        new BinaryFormatFileTransport(path, defaultOpts, LOCAL_FILES_UPLOADER, (e) -> {});
     transport.sendBuildEvent(buildEvent, artifactGroupNamer);
 
     transport.close().get();
@@ -134,7 +134,8 @@ public class BinaryFormatFileTransportTest {
     when(buildEvent.asStreamProto(Matchers.<BuildEventContext>any())).thenReturn(started);
 
     BinaryFormatFileTransport transport =
-        new BinaryFormatFileTransport(output.getAbsolutePath(), defaultOpts, pathConverter);
+        new BinaryFormatFileTransport(
+            output.getAbsolutePath(), defaultOpts, LOCAL_FILES_UPLOADER, (e) -> {});
 
     // Close the stream.
     transport.out.close();
@@ -161,7 +162,8 @@ public class BinaryFormatFileTransportTest {
     when(buildEvent.asStreamProto(Matchers.<BuildEventContext>any())).thenReturn(started);
 
     BinaryFormatFileTransport transport =
-        new BinaryFormatFileTransport(output.getAbsolutePath(), defaultOpts, pathConverter);
+        new BinaryFormatFileTransport(
+            output.getAbsolutePath(), defaultOpts, LOCAL_FILES_UPLOADER, (e) -> {});
 
     transport.sendBuildEvent(buildEvent, artifactGroupNamer);
     Future<Void> closeFuture = transport.close();
diff --git a/src/test/java/com/google/devtools/build/lib/buildeventstream/transports/BuildEventTransportFactoryTest.java b/src/test/java/com/google/devtools/build/lib/buildeventstream/transports/BuildEventTransportFactoryTest.java
index e5d5133cca..5494684b3f 100644
--- a/src/test/java/com/google/devtools/build/lib/buildeventstream/transports/BuildEventTransportFactoryTest.java
+++ b/src/test/java/com/google/devtools/build/lib/buildeventstream/transports/BuildEventTransportFactoryTest.java
@@ -22,6 +22,7 @@ import com.google.common.collect.FluentIterable;
 import com.google.common.collect.ImmutableSet;
 import com.google.devtools.build.lib.buildeventstream.ArtifactGroupNamer;
 import com.google.devtools.build.lib.buildeventstream.BuildEvent;
+import com.google.devtools.build.lib.buildeventstream.BuildEventArtifactUploaderMap;
 import com.google.devtools.build.lib.buildeventstream.BuildEventContext;
 import com.google.devtools.build.lib.buildeventstream.BuildEventProtocolOptions;
 import com.google.devtools.build.lib.buildeventstream.BuildEventStreamProtos;
@@ -31,7 +32,7 @@ import com.google.devtools.build.lib.buildeventstream.PathConverter;
 import com.google.devtools.common.options.Options;
 import java.io.File;
 import java.io.IOException;
-import java.util.concurrent.Future;
+import java.util.concurrent.ExecutionException;
 import org.junit.After;
 import org.junit.Before;
 import org.junit.Rule;
@@ -44,7 +45,7 @@ import org.mockito.Mock;
 import org.mockito.Mockito;
 import org.mockito.MockitoAnnotations;
 
-/** Tests {@link BuildEventTransportFactory}. **/
+/** Tests {@link BuildEventTransportFactory}. */
 @RunWith(JUnit4.class)
 public class BuildEventTransportFactoryTest {
 
@@ -85,14 +86,18 @@ public class BuildEventTransportFactoryTest {
     Mockito.validateMockitoUsage();
   }
 
+  private BuildEventArtifactUploaderMap localFilesOnly() {
+    return new BuildEventArtifactUploaderMap.Builder().build();
+  }
+
   @Test
-  public void testCreatesTextFormatFileTransport() throws IOException {
+  public void testCreatesTextFormatFileTransport() throws Exception {
     File textFile = tmp.newFile();
     when(options.getBuildEventTextFile()).thenReturn(textFile.getAbsolutePath());
-    when(options.getBuildEventTextFilePathConversion()).thenReturn(true);
     when(options.getBuildEventBinaryFile()).thenReturn("");
     ImmutableSet<BuildEventTransport> transports =
-        BuildEventTransportFactory.createFromOptions(options, protocolOpts, pathConverter);
+        BuildEventTransportFactory.createFromOptions(
+            options, protocolOpts, localFilesOnly(), (e) -> {});
     assertThat(FluentIterable.from(transports).transform(GET_CLASS))
         .containsExactly(TextFormatFileTransport.class);
     sendEventsAndClose(buildEvent, transports);
@@ -100,13 +105,13 @@ public class BuildEventTransportFactoryTest {
   }
 
   @Test
-  public void testCreatesBinaryFormatFileTransport() throws IOException {
+  public void testCreatesBinaryFormatFileTransport() throws Exception {
     File binaryFile = tmp.newFile();
     when(options.getBuildEventTextFile()).thenReturn("");
     when(options.getBuildEventBinaryFile()).thenReturn(binaryFile.getAbsolutePath());
-    when(options.getBuildEventBinaryFilePathConversion()).thenReturn(true);
     ImmutableSet<BuildEventTransport> transports =
-        BuildEventTransportFactory.createFromOptions(options, protocolOpts, pathConverter);
+        BuildEventTransportFactory.createFromOptions(
+            options, protocolOpts, localFilesOnly(), (e) -> {});
     assertThat(FluentIterable.from(transports).transform(GET_CLASS))
         .containsExactly(BinaryFormatFileTransport.class);
     sendEventsAndClose(buildEvent, transports);
@@ -114,15 +119,14 @@ public class BuildEventTransportFactoryTest {
   }
 
   @Test
-  public void testCreatesAllTransports() throws IOException {
+  public void testCreatesAllTransports() throws Exception {
     File textFile = tmp.newFile();
     File binaryFile = tmp.newFile();
     when(options.getBuildEventTextFile()).thenReturn(textFile.getAbsolutePath());
     when(options.getBuildEventBinaryFile()).thenReturn(binaryFile.getAbsolutePath());
-    when(options.getBuildEventBinaryFilePathConversion()).thenReturn(true);
-    when(options.getBuildEventTextFilePathConversion()).thenReturn(true);
     ImmutableSet<BuildEventTransport> transports =
-        BuildEventTransportFactory.createFromOptions(options, protocolOpts, pathConverter);
+        BuildEventTransportFactory.createFromOptions(
+            options, protocolOpts, localFilesOnly(), (e) -> {});
     assertThat(FluentIterable.from(transports).transform(GET_CLASS))
         .containsExactly(TextFormatFileTransport.class, BinaryFormatFileTransport.class);
     sendEventsAndClose(buildEvent, transports);
@@ -134,23 +138,16 @@ public class BuildEventTransportFactoryTest {
   public void testCreatesNoTransports() throws IOException {
     when(options.getBuildEventTextFile()).thenReturn("");
     ImmutableSet<BuildEventTransport> transports =
-        BuildEventTransportFactory.createFromOptions(options, protocolOpts, pathConverter);
+        BuildEventTransportFactory.createFromOptions(
+            options, protocolOpts, localFilesOnly(), (e) -> {});
     assertThat(transports).isEmpty();
   }
 
-  @Test
-  public void testPathToUriString() {
-    // See https://blogs.msdn.microsoft.com/ie/2006/12/06/file-uris-in-windows/
-    assertThat(BuildEventTransportFactory.pathToUriString("C:/Temp/Foo Bar.txt"))
-        .isEqualTo("file:///C:/Temp/Foo%20Bar.txt");
-  }
-
   private void sendEventsAndClose(BuildEvent event, Iterable<BuildEventTransport> transports)
-      throws IOException{
+      throws InterruptedException, ExecutionException {
     for (BuildEventTransport transport : transports) {
       transport.sendBuildEvent(event, artifactGroupNamer);
-      @SuppressWarnings({"unused", "nullness"})
-      Future<?> possiblyIgnoredError = transport.close();
+      transport.close().get();
     }
   }
 }
diff --git a/src/test/java/com/google/devtools/build/lib/buildeventstream/transports/JsonFormatFileTransportTest.java b/src/test/java/com/google/devtools/build/lib/buildeventstream/transports/JsonFormatFileTransportTest.java
index e667a7d787..656f87c289 100644
--- a/src/test/java/com/google/devtools/build/lib/buildeventstream/transports/JsonFormatFileTransportTest.java
+++ b/src/test/java/com/google/devtools/build/lib/buildeventstream/transports/JsonFormatFileTransportTest.java
@@ -15,6 +15,7 @@
 package com.google.devtools.build.lib.buildeventstream.transports;
 
 import static com.google.common.truth.Truth.assertThat;
+import static com.google.devtools.build.lib.buildeventstream.BuildEventArtifactUploader.LOCAL_FILES_UPLOADER;
 import static org.mockito.Mockito.when;
 
 import com.google.devtools.build.lib.buildeventstream.ArtifactGroupNamer;
@@ -76,7 +77,8 @@ public class JsonFormatFileTransportTest {
             .build();
     when(buildEvent.asStreamProto(Matchers.<BuildEventContext>any())).thenReturn(started);
     JsonFormatFileTransport transport =
-        new JsonFormatFileTransport(output.getAbsolutePath(), defaultOpts, pathConverter);
+        new JsonFormatFileTransport(
+            output.getAbsolutePath(), defaultOpts, LOCAL_FILES_UPLOADER, (e) -> {});
     transport.sendBuildEvent(buildEvent, artifactGroupNamer);
 
     transport.close().get();
diff --git a/src/test/java/com/google/devtools/build/lib/buildeventstream/transports/TextFormatFileTransportTest.java b/src/test/java/com/google/devtools/build/lib/buildeventstream/transports/TextFormatFileTransportTest.java
index c3ad19d0d6..df84539c3a 100644
--- a/src/test/java/com/google/devtools/build/lib/buildeventstream/transports/TextFormatFileTransportTest.java
+++ b/src/test/java/com/google/devtools/build/lib/buildeventstream/transports/TextFormatFileTransportTest.java
@@ -15,6 +15,7 @@
 package com.google.devtools.build.lib.buildeventstream.transports;
 
 import static com.google.common.truth.Truth.assertThat;
+import static com.google.devtools.build.lib.buildeventstream.BuildEventArtifactUploader.LOCAL_FILES_UPLOADER;
 import static org.mockito.Mockito.when;
 
 import com.google.common.base.Joiner;
@@ -77,7 +78,8 @@ public class TextFormatFileTransportTest {
             .build();
     when(buildEvent.asStreamProto(Matchers.<BuildEventContext>any())).thenReturn(started);
     TextFormatFileTransport transport =
-        new TextFormatFileTransport(output.getAbsolutePath(), defaultOpts, pathConverter);
+        new TextFormatFileTransport(
+            output.getAbsolutePath(), defaultOpts, LOCAL_FILES_UPLOADER, (e) -> {});
     transport.sendBuildEvent(buildEvent, artifactGroupNamer);
 
     BuildEventStreamProtos.BuildEvent progress =
diff --git a/src/test/java/com/google/devtools/build/lib/remote/CasPathConverterTest.java b/src/test/java/com/google/devtools/build/lib/remote/CasPathConverterTest.java
index 885ae95698..21a5bb39a2 100644
--- a/src/test/java/com/google/devtools/build/lib/remote/CasPathConverterTest.java
+++ b/src/test/java/com/google/devtools/build/lib/remote/CasPathConverterTest.java
@@ -37,20 +37,20 @@ public class CasPathConverterTest {
   @Test
   public void noOptionsShouldntCrash() {
     converter.digestUtil = new DigestUtil(HashFunction.SHA256);
-    assertThat(converter.apply(fs.getPath("/foo"))).isNull();
+    assertThat(converter.apply(fs.getPath("/foo"))).isEqualTo("file:///foo");
   }
 
   @Test
   public void noDigestUtilShouldntCrash() {
     converter.options = Options.getDefaults(RemoteOptions.class);
-    assertThat(converter.apply(fs.getPath("/foo"))).isNull();
+    assertThat(converter.apply(fs.getPath("/foo"))).isEqualTo("file:///foo");
   }
 
   @Test
   public void disabledRemote() {
     converter.options = Options.getDefaults(RemoteOptions.class);
     converter.digestUtil = new DigestUtil(HashFunction.SHA256);
-    assertThat(converter.apply(fs.getPath("/foo"))).isNull();
+    assertThat(converter.apply(fs.getPath("/foo"))).isEqualTo("file:///foo");
   }
 
   @Test