Put all connecting to Maven logic in DefaultModelResolver

Making the other classes much more testable!

--
MOS_MIGRATED_REVID=118203206

6 files changed, 94 insertions, 47 deletions

diff --git a/src/tools/generate_workspace/src/main/java/com/google/devtools/build/workspace/GenerateWorkspace.java b/src/tools/generate_workspace/src/main/java/com/google/devtools/build/workspace/GenerateWorkspace.java
index 84bbc0b6eb..e78233d01d 100644
--- a/src/tools/generate_workspace/src/main/java/com/google/devtools/build/workspace/GenerateWorkspace.java
+++ b/src/tools/generate_workspace/src/main/java/com/google/devtools/build/workspace/GenerateWorkspace.java
@@ -23,6 +23,7 @@ import com.google.devtools.build.lib.vfs.FileSystem;
 import com.google.devtools.build.lib.vfs.JavaIoFileSystem;
 import com.google.devtools.build.lib.vfs.Path;
 import com.google.devtools.build.lib.vfs.UnixFileSystem;
+import com.google.devtools.build.workspace.maven.DefaultModelResolver;
 import com.google.devtools.build.workspace.maven.Resolver;
 import com.google.devtools.common.options.OptionsParser;
 
@@ -82,7 +83,7 @@ public class GenerateWorkspace {
   private GenerateWorkspace(String outputDir) {
     this.handler = new EventHandler();
     this.fileSystem = getFileSystem();
-    this.resolver = new Resolver(handler);
+    this.resolver = new Resolver(handler, new DefaultModelResolver());
     if (outputDir.isEmpty()) {
       this.outputDir = fileSystem.getPath(Files.createTempDir().toString());
     } else {
diff --git a/src/tools/generate_workspace/src/main/java/com/google/devtools/build/workspace/maven/DefaultModelResolver.java b/src/tools/generate_workspace/src/main/java/com/google/devtools/build/workspace/maven/DefaultModelResolver.java
index d00d714506..2bd3acd46d 100644
--- a/src/tools/generate_workspace/src/main/java/com/google/devtools/build/workspace/maven/DefaultModelResolver.java
+++ b/src/tools/generate_workspace/src/main/java/com/google/devtools/build/workspace/maven/DefaultModelResolver.java
@@ -20,10 +20,24 @@ import com.google.common.collect.Maps;
 import com.google.common.collect.Sets;
 import com.google.devtools.build.lib.bazel.repository.MavenConnector;
 
+import com.google.devtools.build.lib.events.Event;
+import com.google.devtools.build.lib.events.EventHandler;
+import org.apache.maven.model.Model;
 import org.apache.maven.model.Parent;
 import org.apache.maven.model.Repository;
+import org.apache.maven.model.building.DefaultModelBuilder;
+import org.apache.maven.model.building.DefaultModelBuilderFactory;
+import org.apache.maven.model.building.DefaultModelBuildingRequest;
+import org.apache.maven.model.building.FileModelSource;
+import org.apache.maven.model.building.ModelBuildingException;
+import org.apache.maven.model.building.ModelBuildingResult;
 import org.apache.maven.model.building.ModelSource;
 import org.apache.maven.model.building.UrlModelSource;
+import org.apache.maven.model.composition.DefaultDependencyManagementImporter;
+import org.apache.maven.model.management.DefaultDependencyManagementInjector;
+import org.apache.maven.model.management.DefaultPluginManagementInjector;
+import org.apache.maven.model.plugin.DefaultPluginConfigurationExpander;
+import org.apache.maven.model.profile.DefaultProfileSelector;
 import org.apache.maven.model.resolution.ModelResolver;
 import org.apache.maven.model.resolution.UnresolvableModelException;
 import org.eclipse.aether.artifact.Artifact;
@@ -44,17 +58,26 @@ public class DefaultModelResolver implements ModelResolver {
 
   private final Set<Repository> repositories;
   private final Map<String, ModelSource> ruleNameToModelSource;
+  private final DefaultModelBuilder modelBuilder;
 
   public DefaultModelResolver() {
     repositories = Sets.newHashSet();
     repositories.add(MavenConnector.getMavenCentral());
     ruleNameToModelSource = Maps.newHashMap();
+    modelBuilder = new DefaultModelBuilderFactory().newInstance()
+        .setProfileSelector(new DefaultProfileSelector())
+        .setPluginConfigurationExpander(new DefaultPluginConfigurationExpander())
+        .setPluginManagementInjector(new DefaultPluginManagementInjector())
+        .setDependencyManagementImporter(new DefaultDependencyManagementImporter())
+        .setDependencyManagementInjector(new DefaultDependencyManagementInjector());
   }
 
   private DefaultModelResolver(
-      Set<Repository> repositories, Map<String, ModelSource> ruleNameToModelSource) {
+      Set<Repository> repositories, Map<String, ModelSource> ruleNameToModelSource,
+      DefaultModelBuilder modelBuilder) {
     this.repositories = repositories;
     this.ruleNameToModelSource = ruleNameToModelSource;
+    this.modelBuilder = modelBuilder;
   }
 
   public ModelSource resolveModel(Artifact artifact) throws UnresolvableModelException {
@@ -145,7 +168,7 @@ public class DefaultModelResolver implements ModelResolver {
 
   @Override
   public ModelResolver newCopy() {
-    return new DefaultModelResolver(repositories, ruleNameToModelSource);
+    return new DefaultModelResolver(repositories, ruleNameToModelSource, modelBuilder);
   }
 
   /**
@@ -167,4 +190,38 @@ public class DefaultModelResolver implements ModelResolver {
     }
     return false;
   }
+
+  public Model getEffectiveModel(ModelSource modelSource, EventHandler handler) {
+    DefaultModelBuildingRequest request = new DefaultModelBuildingRequest();
+    request.setModelResolver(this);
+    request.setModelSource(modelSource);
+    Model model;
+    try {
+      ModelBuildingResult result = modelBuilder.build(request);
+      model = result.getEffectiveModel();
+    } catch (ModelBuildingException | IllegalArgumentException e) {
+      // IllegalArg can be thrown if the parent POM cannot be resolved.
+      handler.handle(Event.error("Unable to resolve Maven model from " + modelSource.getLocation()
+          + ": " + e.getMessage()));
+      return null;
+    }
+    return model;
+  }
+
+  public Model getRawModel(FileModelSource fileModelSource, EventHandler handler) {
+    DefaultModelBuildingRequest request = new DefaultModelBuildingRequest();
+    request.setModelResolver(this);
+    request.setModelSource(fileModelSource);
+    Model model;
+    try {
+      ModelBuildingResult result = modelBuilder.build(request);
+      model = result.getRawModel();
+    } catch (ModelBuildingException | IllegalArgumentException e) {
+      // IllegalArg can be thrown if the parent POM cannot be resolved.
+      handler.handle(Event.error("Unable to resolve raw Maven model from "
+          + fileModelSource.getLocation() + ": " + e.getMessage()));
+      return null;
+    }
+    return model;
+  }
 }
diff --git a/src/tools/generate_workspace/src/main/java/com/google/devtools/build/workspace/maven/Resolver.java b/src/tools/generate_workspace/src/main/java/com/google/devtools/build/workspace/maven/Resolver.java
index d379deca5e..18b61d7977 100644
--- a/src/tools/generate_workspace/src/main/java/com/google/devtools/build/workspace/maven/Resolver.java
+++ b/src/tools/generate_workspace/src/main/java/com/google/devtools/build/workspace/maven/Resolver.java
@@ -25,21 +25,11 @@ import org.apache.maven.model.Dependency;
 import org.apache.maven.model.Model;
 import org.apache.maven.model.Parent;
 import org.apache.maven.model.Repository;
-import org.apache.maven.model.building.DefaultModelBuilder;
-import org.apache.maven.model.building.DefaultModelBuilderFactory;
-import org.apache.maven.model.building.DefaultModelBuildingRequest;
 import org.apache.maven.model.building.DefaultModelProcessor;
 import org.apache.maven.model.building.FileModelSource;
-import org.apache.maven.model.building.ModelBuildingException;
-import org.apache.maven.model.building.ModelBuildingResult;
 import org.apache.maven.model.building.ModelSource;
-import org.apache.maven.model.composition.DefaultDependencyManagementImporter;
 import org.apache.maven.model.io.DefaultModelReader;
 import org.apache.maven.model.locator.DefaultModelLocator;
-import org.apache.maven.model.management.DefaultDependencyManagementInjector;
-import org.apache.maven.model.management.DefaultPluginManagementInjector;
-import org.apache.maven.model.plugin.DefaultPluginConfigurationExpander;
-import org.apache.maven.model.profile.DefaultProfileSelector;
 import org.apache.maven.model.resolution.UnresolvableModelException;
 import org.eclipse.aether.artifact.Artifact;
 import org.eclipse.aether.artifact.DefaultArtifact;
@@ -90,7 +80,6 @@ public class Resolver {
   private static final String COMPILE_SCOPE = "compile";
 
   private final EventHandler handler;
-  private final DefaultModelBuilder modelBuilder;
   private final DefaultModelResolver modelResolver;
 
   private final List<String> headers;
@@ -98,17 +87,11 @@ public class Resolver {
   private final Map<String, Rule> deps;
   private Set<Rule> rootDependencies;
 
-  public Resolver(EventHandler handler) {
+  public Resolver(EventHandler handler, DefaultModelResolver resolver) {
     this.handler = handler;
     this.headers = Lists.newArrayList();
     this.deps = Maps.newHashMap();
-    this.modelBuilder = new DefaultModelBuilderFactory().newInstance()
-        .setProfileSelector(new DefaultProfileSelector())
-        .setPluginConfigurationExpander(new DefaultPluginConfigurationExpander())
-        .setPluginManagementInjector(new DefaultPluginManagementInjector())
-        .setDependencyManagementImporter(new DefaultDependencyManagementImporter())
-        .setDependencyManagementInjector(new DefaultDependencyManagementInjector());
-    this.modelResolver = new DefaultModelResolver();
+    this.modelResolver = resolver;
     this.rootDependencies = Sets.newTreeSet();
   }
 
@@ -204,17 +187,8 @@ public class Resolver {
    */
   @Nullable
   public Model resolveEffectiveModel(ModelSource modelSource, Set<String> exclusions, Rule parent) {
-    DefaultModelBuildingRequest request = new DefaultModelBuildingRequest();
-    request.setModelResolver(modelResolver);
-    request.setModelSource(modelSource);
-    Model model;
-    try {
-      ModelBuildingResult result = modelBuilder.build(request);
-      model = result.getEffectiveModel();
-    } catch (ModelBuildingException | IllegalArgumentException e) {
-      // IllegalArg can be thrown if the parent POM cannot be resolved.
-      handler.handle(Event.error("Unable to resolve Maven model from " + modelSource.getLocation()
-          + ": " + e.getMessage()));
+    Model model = modelResolver.getEffectiveModel(modelSource, handler);
+    if (model == null) {
       return null;
     }
     for (Repository repo : model.getRepositories()) {
@@ -268,19 +242,7 @@ public class Resolver {
    * Find the POM files for a given pom's parent(s) and submodules.
    */
   private void resolveSourceLocations(FileModelSource fileModelSource) {
-    DefaultModelBuildingRequest request = new DefaultModelBuildingRequest();
-    request.setModelResolver(modelResolver);
-    request.setModelSource(fileModelSource);
-    Model model;
-    try {
-      ModelBuildingResult result = modelBuilder.build(request);
-      model = result.getRawModel();
-    } catch (ModelBuildingException | IllegalArgumentException e) {
-      // IllegalArg can be thrown if the parent POM cannot be resolved.
-      handler.handle(Event.error("Unable to resolve raw Maven model from "
-          + fileModelSource.getLocation() + ": " + e.getMessage()));
-      return;
-    }
+    Model model = modelResolver.getRawModel(fileModelSource, handler);
 
     // Self.
     Parent parent = model.getParent();
diff --git a/src/tools/generate_workspace/src/test/java/com/google/devtools/build/workspace/WorkspaceFileGeneratorTest.java b/src/tools/generate_workspace/src/test/java/com/google/devtools/build/workspace/WorkspaceFileGeneratorTest.java
index 3ff548c078..77ac7828b0 100644
--- a/src/tools/generate_workspace/src/test/java/com/google/devtools/build/workspace/WorkspaceFileGeneratorTest.java
+++ b/src/tools/generate_workspace/src/test/java/com/google/devtools/build/workspace/WorkspaceFileGeneratorTest.java
@@ -18,6 +18,7 @@ import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertTrue;
 
 import com.google.devtools.build.lib.events.StoredEventHandler;
+import com.google.devtools.build.workspace.maven.DefaultModelResolver;
 import com.google.devtools.build.workspace.maven.Resolver;
 
 import org.junit.Test;
@@ -65,7 +66,7 @@ public class WorkspaceFileGeneratorTest {
     pomWriter.close();
 
     StoredEventHandler handler = new StoredEventHandler();
-    Resolver resolver = new Resolver(handler);
+    Resolver resolver = new Resolver(handler, new DefaultModelResolver());
     String outputFile = tmpdir + "/output";
     PrintStream outputStream = new PrintStream(outputFile);
     resolver.resolvePomDependencies(tmpdir.getAbsolutePath());
diff --git a/src/tools/generate_workspace/src/test/java/com/google/devtools/build/workspace/maven/BUILD b/src/tools/generate_workspace/src/test/java/com/google/devtools/build/workspace/maven/BUILD
index edce307ff2..d4df1085e3 100644
--- a/src/tools/generate_workspace/src/test/java/com/google/devtools/build/workspace/maven/BUILD
+++ b/src/tools/generate_workspace/src/test/java/com/google/devtools/build/workspace/maven/BUILD
@@ -17,6 +17,7 @@ java_test(
         "//src/main/java/com/google/devtools/build/lib:events",
         "//src/tools/generate_workspace/src/main/java/com/google/devtools/build/workspace/maven",
         "//third_party:junit4",
+        "//third_party:mockito",
         "//third_party:truth",
     ],
 )
diff --git a/src/tools/generate_workspace/src/test/java/com/google/devtools/build/workspace/maven/ResolverTest.java b/src/tools/generate_workspace/src/test/java/com/google/devtools/build/workspace/maven/ResolverTest.java
index 20318ec802..b830bcad8f 100644
--- a/src/tools/generate_workspace/src/test/java/com/google/devtools/build/workspace/maven/ResolverTest.java
+++ b/src/tools/generate_workspace/src/test/java/com/google/devtools/build/workspace/maven/ResolverTest.java
@@ -16,9 +16,15 @@ package com.google.devtools.build.workspace.maven;
 
 import static com.google.common.truth.Truth.assertThat;
 
+import com.google.devtools.build.lib.events.StoredEventHandler;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.junit.runners.JUnit4;
+import org.mockito.Mockito;
+
+import java.io.ByteArrayOutputStream;
+import java.io.PrintStream;
+import java.nio.charset.Charset;
 
 /**
  * Tests for {@link Resolver}.
@@ -38,4 +44,23 @@ public class ResolverTest {
     assertThat(Resolver.getSha1Url("http://example.pom/foo.pom", "jar"))
         .isEqualTo("http://example.pom/foo.jar.sha1");
   }
+
+  @Test
+  public void testArtifactResolution() throws Exception {
+    StoredEventHandler handler = new StoredEventHandler();
+    DefaultModelResolver modelResolver = Mockito.mock(DefaultModelResolver.class);
+    Resolver resolver = new Resolver(handler, modelResolver);
+    resolver.resolveArtifact("x:y:1.2.3");
+
+    ByteArrayOutputStream baos = new ByteArrayOutputStream();
+    PrintStream ps = new PrintStream(baos);
+    resolver.writeWorkspace(ps);
+    String content = baos.toString(String.valueOf(Charset.defaultCharset()));
+    assertThat(content).contains("maven_jar(\n"
+        + "    name = \"x_y\",\n"
+        + "    artifact = \"x:y:1.2.3\",\n"
+        + ")"
+    );
+    assertThat(handler.hasErrors()).isFalse();
+  }
 }