From 22fe29e8aa09fd1a77831639e4777011c5d10053 Mon Sep 17 00:00:00 2001
From: Googler <intellij-github@google.com>
Date: Mon, 23 Oct 2023 10:11:45 -0700
Subject: [PATCH] Split `ArtifactTracker` interface

into `ArtifactTracker` and `RenderJarArtifactTracker` interfaces.

Even though their implementations can share a common infrastructure they do
not need to share it. These interfaces are used by different callers and
thus making them separate makes it easier to understand the intended usage
of each method.

1. Do not rename `ArtifactTracker` to `DependencyArtifactTracker` to avoid
   too much noise in this change.

2. Do not split the implementation yet. A few more changes are needed before it
   can happen.

PiperOrigin-RevId: 575854279
---
 .../RenderJarClassFileFinder.java             |  8 ++--
 .../model/idea/BlazeClassJarProvider.java     |  7 +++-
 .../blaze/base/qsync/ArtifactTracker.java     | 27 +-----------
 .../qsync/ArtifactTrackerUpdateResult.java    | 41 +++++++++++++++++++
 .../idea/blaze/base/qsync/ProjectLoader.java  |  1 +
 .../blaze/base/qsync/QuerySyncManager.java    |  5 +++
 .../blaze/base/qsync/QuerySyncProject.java    |  7 ++++
 .../base/qsync/RenderJarArtifactTracker.java  | 34 +++++++++++++++
 .../base/qsync/RenderJarTrackerImpl.java      | 10 ++---
 .../base/qsync/cache/ArtifactTrackerImpl.java | 14 ++++---
 .../qsync/cache/ArtifactTrackerImplTest.java  | 10 ++---
 11 files changed, 117 insertions(+), 47 deletions(-)
 create mode 100644 base/src/com/google/idea/blaze/base/qsync/ArtifactTrackerUpdateResult.java
 create mode 100644 base/src/com/google/idea/blaze/base/qsync/RenderJarArtifactTracker.java

diff --git a/aswb/src/com/google/idea/blaze/android/projectsystem/RenderJarClassFileFinder.java b/aswb/src/com/google/idea/blaze/android/projectsystem/RenderJarClassFileFinder.java
index 2f582036eaa..b58fca0edbc 100644
--- a/aswb/src/com/google/idea/blaze/android/projectsystem/RenderJarClassFileFinder.java
+++ b/aswb/src/com/google/idea/blaze/android/projectsystem/RenderJarClassFileFinder.java
@@ -30,9 +30,9 @@
 import com.google.idea.blaze.base.ideinfo.TargetKey;
 import com.google.idea.blaze.base.io.VirtualFileSystemProvider;
 import com.google.idea.blaze.base.model.BlazeProjectData;
-import com.google.idea.blaze.base.qsync.ArtifactTracker;
 import com.google.idea.blaze.base.qsync.QuerySync;
 import com.google.idea.blaze.base.qsync.QuerySyncManager;
+import com.google.idea.blaze.base.qsync.RenderJarArtifactTracker;
 import com.google.idea.blaze.base.settings.Blaze;
 import com.google.idea.blaze.base.settings.BlazeImportSettings.ProjectType;
 import com.google.idea.blaze.base.sync.BlazeSyncModificationTracker;
@@ -143,12 +143,12 @@ public VirtualFile findClass(String fqcn) {
 
     if (Blaze.getProjectType(project).equals(ProjectType.QUERY_SYNC)) {
       if (QuerySync.isComposeEnabled()) {
-        ArtifactTracker artifactTracker =
-            QuerySyncManager.getInstance(project).getArtifactTracker();
+        RenderJarArtifactTracker renderJarArtifactTracker =
+            QuerySyncManager.getInstance(project).getRenderJarArtifactTracker();
         // TODO(b/283280194): Setup fqcn -> target and target -> Render jar mappings to avoid
         // iterating over all render jars when trying to locate class for fqcn.
         // TODO(b/284002836): Collect metrics on time taken to iterate over the jars
-        for (File renderJar : artifactTracker.getRenderJars()) {
+        for (File renderJar : renderJarArtifactTracker.getRenderJars()) {
           VirtualFile renderResolveJarVf =
               VirtualFileSystemProvider.getInstance().getSystem().findFileByIoFile(renderJar);
           if (renderResolveJarVf != null) {
diff --git a/aswb/src/com/google/idea/blaze/android/sync/model/idea/BlazeClassJarProvider.java b/aswb/src/com/google/idea/blaze/android/sync/model/idea/BlazeClassJarProvider.java
index 4081f820525..7c4bb24eefa 100644
--- a/aswb/src/com/google/idea/blaze/android/sync/model/idea/BlazeClassJarProvider.java
+++ b/aswb/src/com/google/idea/blaze/android/sync/model/idea/BlazeClassJarProvider.java
@@ -32,10 +32,12 @@
 import com.google.idea.blaze.base.ideinfo.TargetKey;
 import com.google.idea.blaze.base.ideinfo.TargetMap;
 import com.google.idea.blaze.base.model.BlazeProjectData;
+import com.google.idea.blaze.base.qsync.QuerySync;
 import com.google.idea.blaze.base.qsync.ArtifactTracker;
 import com.google.idea.blaze.base.qsync.QuerySyncManager;
 import com.google.idea.blaze.base.settings.Blaze;
 import com.google.idea.blaze.base.settings.BlazeImportSettings.ProjectType;
+import com.google.idea.blaze.base.qsync.RenderJarArtifactTracker;
 import com.google.idea.blaze.base.sync.data.BlazeProjectDataManager;
 import com.google.idea.blaze.base.sync.workspace.ArtifactLocationDecoder;
 import com.google.idea.blaze.base.targetmaps.TransitiveDependencyMap;
@@ -74,8 +76,9 @@ public List<File> getModuleExternalLibraries(Module module) {
       // As Query Sync has a single workspace module but multiple resource modules
       // (TODO(b/283282438): for setting up the resources). All render jars are mapped to the same
       // workspace module
-      ArtifactTracker artifactTracker = QuerySyncManager.getInstance(project).getArtifactTracker();
-      return artifactTracker.getRenderJars();
+      RenderJarArtifactTracker renderJarArtifactTracker =
+          QuerySyncManager.getInstance(project).getRenderJarArtifactTracker();
+      return renderJarArtifactTracker.getRenderJars();
     }
 
     BlazeProjectData blazeProjectData =
diff --git a/base/src/com/google/idea/blaze/base/qsync/ArtifactTracker.java b/base/src/com/google/idea/blaze/base/qsync/ArtifactTracker.java
index d63e71a6694..5bb057287e0 100644
--- a/base/src/com/google/idea/blaze/base/qsync/ArtifactTracker.java
+++ b/base/src/com/google/idea/blaze/base/qsync/ArtifactTracker.java
@@ -15,8 +15,6 @@
  */
 package com.google.idea.blaze.base.qsync;
 
-import com.google.auto.value.AutoValue;
-import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableSet;
 import com.google.idea.blaze.base.scope.BlazeContext;
 import com.google.idea.blaze.common.Context;
@@ -25,7 +23,6 @@
 import com.google.idea.blaze.qsync.cc.CcDependenciesInfo;
 import com.google.idea.blaze.qsync.project.BuildGraphData;
 import com.google.idea.blaze.qsync.project.ProjectProto;
-import java.io.File;
 import java.io.IOException;
 import java.nio.file.Path;
 import java.util.Optional;
@@ -38,12 +35,8 @@ public interface ArtifactTracker {
   void clear() throws IOException;
 
   /** Fetches, caches and sets up new artifacts. */
-  UpdateResult update(Set<Label> targets, OutputInfo outputInfo, BlazeContext context)
-      throws BuildException;
-
-  /** Fetches, caches and sets up new render jar artifacts. */
-  UpdateResult update(Set<Label> targets, RenderJarInfo renderJarInfo, BlazeContext context)
-      throws BuildException;
+  ArtifactTrackerUpdateResult update(
+      Set<Label> targets, OutputInfo outputInfo, BlazeContext context) throws BuildException;
 
   /**
    * Makes the project snapshot reflect the current state of tracked artifacts.
@@ -90,22 +83,6 @@ ProjectProto.Project updateProjectProto(
    */
   CcDependenciesInfo getCcDependenciesInfo();
 
-  /** A data class representing the result of updating artifacts. */
-  @AutoValue
-  abstract class UpdateResult {
-    public abstract ImmutableSet<Path> updatedFiles();
-
-    public abstract ImmutableSet<String> removedKeys();
-
-    public static UpdateResult create(
-        ImmutableSet<Path> updatedFiles, ImmutableSet<String> removedKeys) {
-      return new AutoValue_ArtifactTracker_UpdateResult(updatedFiles, removedKeys);
-    }
-  }
-
-  /** Returns the list of render jars */
-  ImmutableList<File> getRenderJars();
-
   /** Returns the count of .jar files. */
   Integer getJarsCount();
 }
diff --git a/base/src/com/google/idea/blaze/base/qsync/ArtifactTrackerUpdateResult.java b/base/src/com/google/idea/blaze/base/qsync/ArtifactTrackerUpdateResult.java
new file mode 100644
index 00000000000..26f434892d2
--- /dev/null
+++ b/base/src/com/google/idea/blaze/base/qsync/ArtifactTrackerUpdateResult.java
@@ -0,0 +1,41 @@
+/*
+ * Copyright 2023 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.idea.blaze.base.qsync;
+
+import com.google.auto.value.AutoValue;
+import com.google.common.collect.ImmutableSet;
+import java.nio.file.Path;
+
+/**
+ * A result of updating artifact cache.
+ *
+ * <p>Instances of this class describe the difference between the state of the artifact cache before
+ * and after the operation. They do not represent the result of build action that produced these
+ * artifacts.
+ *
+ * <p><b>DO NOT USE</b> for purposes other than logging or displaying the result in the UI.
+ */
+@AutoValue
+public abstract class ArtifactTrackerUpdateResult {
+  public abstract ImmutableSet<Path> updatedFiles();
+
+  public abstract ImmutableSet<String> removedKeys();
+
+  public static ArtifactTrackerUpdateResult create(
+      ImmutableSet<Path> updatedFiles, ImmutableSet<String> removedKeys) {
+    return new AutoValue_ArtifactTrackerUpdateResult(updatedFiles, removedKeys);
+  }
+}
diff --git a/base/src/com/google/idea/blaze/base/qsync/ProjectLoader.java b/base/src/com/google/idea/blaze/base/qsync/ProjectLoader.java
index 6623e581cf4..0f6dd27a122 100644
--- a/base/src/com/google/idea/blaze/base/qsync/ProjectLoader.java
+++ b/base/src/com/google/idea/blaze/base/qsync/ProjectLoader.java
@@ -179,6 +179,7 @@ public QuerySyncProject loadProject(BlazeContext context) throws IOException {
             importSettings,
             workspaceRoot,
             artifactTracker,
+            artifactTracker,
             dependencyTracker,
             renderJarTracker,
             projectQuerier,
diff --git a/base/src/com/google/idea/blaze/base/qsync/QuerySyncManager.java b/base/src/com/google/idea/blaze/base/qsync/QuerySyncManager.java
index bf6e64428c9..8f3110b9db3 100644
--- a/base/src/com/google/idea/blaze/base/qsync/QuerySyncManager.java
+++ b/base/src/com/google/idea/blaze/base/qsync/QuerySyncManager.java
@@ -152,6 +152,11 @@ public ArtifactTracker getArtifactTracker() {
     return loadedProject.getArtifactTracker();
   }
 
+  public RenderJarArtifactTracker getRenderJarArtifactTracker() {
+    assertProjectLoaded();
+    return loadedProject.getRenderJarArtifactTracker();
+  }
+
   public SourceToTargetMap getSourceToTargetMap() {
     assertProjectLoaded();
     return loadedProject.getSourceToTargetMap();
diff --git a/base/src/com/google/idea/blaze/base/qsync/QuerySyncProject.java b/base/src/com/google/idea/blaze/base/qsync/QuerySyncProject.java
index ac2ee944aec..6aed6fa0dbe 100644
--- a/base/src/com/google/idea/blaze/base/qsync/QuerySyncProject.java
+++ b/base/src/com/google/idea/blaze/base/qsync/QuerySyncProject.java
@@ -83,6 +83,7 @@ public class QuerySyncProject {
   private final BlazeImportSettings importSettings;
   private final WorkspaceRoot workspaceRoot;
   private final ArtifactTracker artifactTracker;
+  private final RenderJarArtifactTracker renderJarArtifactTracker;
   private final DependencyTracker dependencyTracker;
   private final RenderJarTracker renderJarTracker;
   private final ProjectQuerier projectQuerier;
@@ -107,6 +108,7 @@ public QuerySyncProject(
       BlazeImportSettings importSettings,
       WorkspaceRoot workspaceRoot,
       ArtifactTracker artifactTracker,
+      RenderJarArtifactTracker renderJarArtifactTracker,
       DependencyTracker dependencyTracker,
       RenderJarTracker renderJarTracker,
       ProjectQuerier projectQuerier,
@@ -125,6 +127,7 @@ public QuerySyncProject(
     this.importSettings = importSettings;
     this.workspaceRoot = workspaceRoot;
     this.artifactTracker = artifactTracker;
+    this.renderJarArtifactTracker = renderJarArtifactTracker;
     this.dependencyTracker = dependencyTracker;
     this.renderJarTracker = renderJarTracker;
     this.projectQuerier = projectQuerier;
@@ -180,6 +183,10 @@ public ArtifactTracker getArtifactTracker() {
     return artifactTracker;
   }
 
+  public RenderJarArtifactTracker getRenderJarArtifactTracker() {
+    return renderJarArtifactTracker;
+  }
+
   public SourceToTargetMap getSourceToTargetMap() {
     return sourceToTargetMap;
   }
diff --git a/base/src/com/google/idea/blaze/base/qsync/RenderJarArtifactTracker.java b/base/src/com/google/idea/blaze/base/qsync/RenderJarArtifactTracker.java
new file mode 100644
index 00000000000..7a4bfd59d08
--- /dev/null
+++ b/base/src/com/google/idea/blaze/base/qsync/RenderJarArtifactTracker.java
@@ -0,0 +1,34 @@
+/*
+ * Copyright 2023 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.idea.blaze.base.qsync;
+
+import com.google.common.collect.ImmutableList;
+import com.google.idea.blaze.base.scope.BlazeContext;
+import com.google.idea.blaze.common.Label;
+import com.google.idea.blaze.exception.BuildException;
+import java.io.File;
+import java.util.Set;
+
+/** A local cache of built render jarss. */
+public interface RenderJarArtifactTracker {
+
+  /** Fetches, caches and sets up new render jar artifacts. */
+  ArtifactTrackerUpdateResult update(
+      Set<Label> targets, RenderJarInfo renderJarInfo, BlazeContext context) throws BuildException;
+
+  /** Returns the list of render jars */
+  ImmutableList<File> getRenderJars();
+}
diff --git a/base/src/com/google/idea/blaze/base/qsync/RenderJarTrackerImpl.java b/base/src/com/google/idea/blaze/base/qsync/RenderJarTrackerImpl.java
index 169aa62a949..bcc1d482349 100644
--- a/base/src/com/google/idea/blaze/base/qsync/RenderJarTrackerImpl.java
+++ b/base/src/com/google/idea/blaze/base/qsync/RenderJarTrackerImpl.java
@@ -18,7 +18,6 @@
 import com.google.common.base.Preconditions;
 import com.google.idea.blaze.base.bazel.BazelExitCode;
 import com.google.idea.blaze.base.logging.utils.querysync.BuildDepsStatsScope;
-import com.google.idea.blaze.base.qsync.ArtifactTracker.UpdateResult;
 import com.google.idea.blaze.base.scope.BlazeContext;
 import com.google.idea.blaze.common.Label;
 import com.google.idea.blaze.common.PrintOutput;
@@ -39,15 +38,15 @@ public class RenderJarTrackerImpl implements RenderJarTracker {
 
   private final BlazeProject blazeProject;
   private final RenderJarBuilder renderJarBuilder;
-  private final ArtifactTracker artifactTracker;
+  private final RenderJarArtifactTracker renderJarArtifactTracker;
 
   public RenderJarTrackerImpl(
       BlazeProject blazeProject,
       RenderJarBuilder renderJarBuilder,
-      ArtifactTracker artifactTracker) {
+      RenderJarArtifactTracker renderJarArtifactTracker) {
     this.blazeProject = blazeProject;
     this.renderJarBuilder = renderJarBuilder;
-    this.artifactTracker = artifactTracker;
+    this.renderJarArtifactTracker = renderJarArtifactTracker;
   }
 
   /** Builds the render jars of the given files and adds then to the cache */
@@ -95,7 +94,8 @@ public void buildRenderJarForFile(BlazeContext context, List<Path> workspaceRela
       context.output(PrintOutput.error("There were build errors when generating render jar."));
     }
 
-    UpdateResult updateResult = artifactTracker.update(targets, renderJarInfo, context);
+    ArtifactTrackerUpdateResult updateResult =
+        renderJarArtifactTracker.update(targets, renderJarInfo, context);
     if (updateResult.updatedFiles().isEmpty()) {
       context.output(
           PrintOutput.log(
diff --git a/base/src/com/google/idea/blaze/base/qsync/cache/ArtifactTrackerImpl.java b/base/src/com/google/idea/blaze/base/qsync/cache/ArtifactTrackerImpl.java
index 5b14703c6ba..64a53e71576 100644
--- a/base/src/com/google/idea/blaze/base/qsync/cache/ArtifactTrackerImpl.java
+++ b/base/src/com/google/idea/blaze/base/qsync/cache/ArtifactTrackerImpl.java
@@ -52,8 +52,10 @@
 import com.google.idea.blaze.base.logging.utils.querysync.BuildDepsStats;
 import com.google.idea.blaze.base.logging.utils.querysync.BuildDepsStatsScope;
 import com.google.idea.blaze.base.qsync.ArtifactTracker;
+import com.google.idea.blaze.base.qsync.ArtifactTrackerUpdateResult;
 import com.google.idea.blaze.base.qsync.OutputGroup;
 import com.google.idea.blaze.base.qsync.OutputInfo;
+import com.google.idea.blaze.base.qsync.RenderJarArtifactTracker;
 import com.google.idea.blaze.base.qsync.RenderJarInfo;
 import com.google.idea.blaze.base.qsync.cache.ArtifactFetcher.ArtifactDestination;
 import com.google.idea.blaze.base.qsync.cache.FileCache.CacheLayout;
@@ -106,7 +108,7 @@
  *
  * <p>This class maps all the targets that have been built to their artifacts.
  */
-public class ArtifactTrackerImpl implements ArtifactTracker {
+public class ArtifactTrackerImpl implements ArtifactTracker, RenderJarArtifactTracker {
 
   private static final BoolExperiment ATTACH_DEP_SRCJARS =
       new BoolExperiment("querysync.attach.dep.srcjars", true);
@@ -377,7 +379,7 @@ private ImmutableMap<Path, Path> prepareFinalLayouts(
 
   /** Fetches, caches and sets up new render jar artifacts. */
   @Override
-  public UpdateResult update(
+  public ArtifactTrackerUpdateResult update(
       Set<Label> targets, RenderJarInfo renderJarInfo, BlazeContext outerContext)
       throws BuildException {
     ImmutableMap<OutputArtifact, OutputArtifactDestinationAndLayout> artifactMap;
@@ -389,7 +391,7 @@ public UpdateResult update(
     try (BlazeContext context = BlazeContext.create(outerContext)) {
       ImmutableMap<Path, Path> updated = cache(context, artifactMap);
       saveState();
-      return UpdateResult.create(updated.keySet(), ImmutableSet.of());
+      return ArtifactTrackerUpdateResult.create(updated.keySet(), ImmutableSet.of());
     } catch (ExecutionException | IOException e) {
       throw new BuildException(e);
     }
@@ -397,8 +399,8 @@ public UpdateResult update(
 
   /** Fetches, caches and sets up new artifacts. */
   @Override
-  public UpdateResult update(Set<Label> targets, OutputInfo outputInfo, BlazeContext outerContext)
-      throws BuildException {
+  public ArtifactTrackerUpdateResult update(
+      Set<Label> targets, OutputInfo outputInfo, BlazeContext outerContext) throws BuildException {
     ImmutableMap<OutputArtifact, OutputArtifactDestinationAndLayout> artifactMap;
     try {
       artifactMap = outputInfoToArtifactMap(outputInfo);
@@ -417,7 +419,7 @@ public UpdateResult update(Set<Label> targets, OutputInfo outputInfo, BlazeConte
       ccDepencenciesInfo = ccDepsBuilder.build();
 
       saveState();
-      return UpdateResult.create(updated.keySet(), ImmutableSet.of());
+      return ArtifactTrackerUpdateResult.create(updated.keySet(), ImmutableSet.of());
     } catch (ExecutionException | IOException e) {
       throw new BuildException(e);
     }
diff --git a/base/tests/unittests/com/google/idea/blaze/base/qsync/cache/ArtifactTrackerImplTest.java b/base/tests/unittests/com/google/idea/blaze/base/qsync/cache/ArtifactTrackerImplTest.java
index 0e12f77e175..d56cea755c1 100644
--- a/base/tests/unittests/com/google/idea/blaze/base/qsync/cache/ArtifactTrackerImplTest.java
+++ b/base/tests/unittests/com/google/idea/blaze/base/qsync/cache/ArtifactTrackerImplTest.java
@@ -28,7 +28,7 @@
 import com.google.devtools.intellij.qsync.ArtifactTrackerData.JavaTargetArtifacts;
 import com.google.idea.blaze.base.command.buildresult.OutputArtifact;
 import com.google.idea.blaze.base.filecache.ArtifactState;
-import com.google.idea.blaze.base.qsync.ArtifactTracker.UpdateResult;
+import com.google.idea.blaze.base.qsync.ArtifactTrackerUpdateResult;
 import com.google.idea.blaze.base.qsync.GroupedOutputArtifacts;
 import com.google.idea.blaze.base.qsync.OutputGroup;
 import com.google.idea.blaze.base.qsync.OutputInfo;
@@ -113,7 +113,7 @@ public void failed_fetch_resets_metadata() throws Exception {
             ProjectDefinition.EMPTY);
     artifactTracker.initialize();
 
-    final UpdateResult unused =
+    final ArtifactTrackerUpdateResult unused =
         artifactTracker.update(
             ImmutableSet.of(Label.of("//test:test")),
             OutputInfo.builder()
@@ -129,7 +129,7 @@ public void failed_fetch_resets_metadata() throws Exception {
 
     testArtifactFetcher.shouldFail = true;
     try {
-      final UpdateResult unused2 =
+      final ArtifactTrackerUpdateResult unused2 =
           artifactTracker.update(
               ImmutableSet.of(Label.of("//test:test2")),
               OutputInfo.builder()
@@ -221,7 +221,7 @@ public void library_sources() throws Throwable {
             ProjectDefinition.EMPTY);
     artifactTracker.initialize();
 
-    final UpdateResult unused =
+    final ArtifactTrackerUpdateResult unused =
         artifactTracker.update(
             ImmutableSet.of(Label.of("//test:test"), Label.of("//test:anothertest")),
             OutputInfo.builder()
@@ -276,7 +276,7 @@ public void library_sources_unknown_lib() throws Throwable {
             ProjectDefinition.EMPTY);
     artifactTracker.initialize();
 
-    final UpdateResult unused =
+    final ArtifactTrackerUpdateResult unused =
         artifactTracker.update(
             ImmutableSet.of(Label.of("//test:test"), Label.of("//test:anothertest")),
             OutputInfo.builder()