changeset 962:3e7fde939ccc

TW-49811 prohibit agent-side checkout when 2 roots are checked out into the same dir
author Dmitry Neverov <dmitry.neverov@gmail.com>
date Tue, 25 Apr 2017 13:01:25 +0200
parents 075f3521bd02
children e9987cf08919 d0a236443fc9
files mercurial-agent/src/jetbrains/buildServer/buildTriggers/vcs/mercurial/MercurialAgentSideVcsSupport.java mercurial-tests/src/jetbrains/buildServer/buildTriggers/vcs/mercurial/AgentSideCheckoutTest.java
diffstat 2 files changed, 172 insertions(+), 3 deletions(-) [+]
line wrap: on
line diff
--- a/mercurial-agent/src/jetbrains/buildServer/buildTriggers/vcs/mercurial/MercurialAgentSideVcsSupport.java	Wed Apr 05 10:36:29 2017 +0200
+++ b/mercurial-agent/src/jetbrains/buildServer/buildTriggers/vcs/mercurial/MercurialAgentSideVcsSupport.java	Tue Apr 25 13:01:25 2017 +0200
@@ -25,6 +25,7 @@
 import org.jetbrains.annotations.NotNull;
 
 import java.io.File;
+import java.util.*;
 
 public class MercurialAgentSideVcsSupport extends AgentVcsSupport implements UpdateByIncludeRules2 {
 
@@ -64,16 +65,61 @@
       return AgentCheckoutAbility.noVcsClientOnAgent(e.getMessage());
     }
 
+    Set<String> targetDirs = new HashSet<String>();
     try {
       for (IncludeRule rule : checkoutRules.getRootIncludeRules()) {
         MercurialIncludeRuleUpdater.checkRuleIsValid(rule);
+        targetDirs.add(rule.getTo());
       }
-      return AgentCheckoutAbility.canCheckout();
     } catch (VcsException e) {
       return AgentCheckoutAbility.notSupportedCheckoutRules(e.getMessage());
     }
+
+    List<VcsRootEntry> mercurialEntries = getMercurialEntries(build);
+    if (mercurialEntries.size() > 1) {
+      for (VcsRootEntry entry : mercurialEntries) {
+        VcsRoot otherRoot = entry.getVcsRoot();
+        if (vcsRoot.equals(otherRoot))
+          continue;
+        for (IncludeRule rule : getOtherRootRules(entry.getCheckoutRules().getRootIncludeRules())) {
+          if (targetDirs.contains(rule.getTo()))
+            return AgentCheckoutAbility.canNotCheckout("Cannot checkout VCS root '" + vcsRoot.getName() + "' into the same directory as VCS root '" + otherRoot.getName() + "'");
+        }
+      }
+    }
+
+    return AgentCheckoutAbility.canCheckout();
   }
 
+
+  @NotNull
+  private List<VcsRootEntry> getMercurialEntries(@NotNull AgentRunningBuild build) {
+    List<VcsRootEntry> result = new ArrayList<VcsRootEntry>();
+    for (VcsRootEntry entry : build.getVcsRootEntries()) {
+      if (Constants.VCS_NAME.equals(entry.getVcsRoot().getVcsName()))
+        result.add(entry);
+    }
+    return result;
+  }
+
+
+  @NotNull
+  private List<IncludeRule> getOtherRootRules(@NotNull List<IncludeRule> rules) {
+    List<IncludeRule> result = new ArrayList<IncludeRule>();
+    for (IncludeRule rule : rules) {
+      try {
+        MercurialIncludeRuleUpdater.checkRuleIsValid(rule);
+      } catch (VcsException e) {
+        //return empty root rules to skip checks; appropriate cannot checkout reason
+        //will be returned during otherRoot's canCheckout() call
+        return Collections.emptyList();
+      }
+      result.add(rule);
+    }
+    return result;
+  }
+
+
   private void registerExtensions(@NotNull VcsRoot root, @NotNull CheckoutRules checkoutRules, @NotNull MercurialIncludeRuleUpdater updater) {
     CheckoutInfo info = new CheckoutInfo(myRepoFactory, new HgVcsRoot(root), checkoutRules);
     for (MercurialExtension ext :myExtentionManager.getExtensionsForCheckout(info)) {
--- a/mercurial-tests/src/jetbrains/buildServer/buildTriggers/vcs/mercurial/AgentSideCheckoutTest.java	Wed Apr 05 10:36:29 2017 +0200
+++ b/mercurial-tests/src/jetbrains/buildServer/buildTriggers/vcs/mercurial/AgentSideCheckoutTest.java	Tue Apr 25 13:01:25 2017 +0200
@@ -15,6 +15,7 @@
  */
 package jetbrains.buildServer.buildTriggers.vcs.mercurial;
 
+import jetbrains.buildServer.TestInternalProperties;
 import jetbrains.buildServer.agent.*;
 import jetbrains.buildServer.agent.vcs.AgentCheckoutAbility;
 import jetbrains.buildServer.buildTriggers.vcs.mercurial.command.CommandSettingsForRootImpl;
@@ -31,6 +32,7 @@
 import org.jmock.Expectations;
 import org.jmock.Mockery;
 import org.testng.annotations.BeforeMethod;
+import org.testng.annotations.DataProvider;
 import org.testng.annotations.Test;
 
 import java.io.File;
@@ -40,8 +42,9 @@
 import java.util.Map;
 import java.util.concurrent.*;
 
+import static java.util.Arrays.asList;
 import static jetbrains.buildServer.buildTriggers.vcs.mercurial.Util.copyRepository;
-import static org.hamcrest.MatcherAssert.*;
+import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.text.StringContains.containsString;
 
 /**
@@ -63,6 +66,7 @@
   @BeforeMethod
   protected void setUp() throws Exception {
     super.setUp();
+    TestInternalProperties.init();
 
     myContext = new Mockery();
 
@@ -118,6 +122,85 @@
     assertThat(agentCheckoutAbility.getCanNotCheckoutReason().getDetails(), containsString("Invalid include rule: subdir=>subdir2"));
   }
 
+
+  @DataProvider
+  public static Object[][] severalRootsSetups() throws Exception {
+    return new Object[][]{
+            new Object[]{new Setup()
+                    .setShouldFail(true)
+            },
+            new Object[]{new Setup()
+                    .setCheckoutRules1("+:.=>dir")
+                    .setCheckoutRules2("+:.=>dir")
+                    .setShouldFail(true)
+            },
+            new Object[]{new Setup()
+                    .setCheckoutRules1("+:.=>dir1")
+                    .setCheckoutRules2("+:.=>dir2\n+:.=>dir1")
+                    .setShouldFail(true)
+            },
+            new Object[]{new Setup()
+                    .setCheckoutRules2("+:.=>dir1")
+                    .setCheckoutRules2("+:.=>dir2")
+                    .setShouldFail(false)
+            }
+    };
+  }
+
+
+  @TestFor(issues = "TW-49811")
+  @Test(dataProvider = "severalRootsSetups")
+  public void auto_checkout_many_roots(@NotNull Setup setup) throws Exception {
+    VcsRootImpl root1 = new VcsRootBuilder().withId(1).withUrl("http://some.org/repo1").build();
+    VcsRootImpl root2 = new VcsRootBuilder().withId(2).withUrl("http://some.org/repo2").build();
+
+    CheckoutRules rules1 = new CheckoutRules(setup.getCheckoutRules1());
+    CheckoutRules rules2 = new CheckoutRules(setup.getCheckoutRules2());
+    AgentRunningBuild build = myContext.mock(AgentRunningBuild.class, "build" + myBuildCounter++);
+    myContext.checking(new Expectations() {{
+      allowing(build).getVcsRootEntries();
+      will(returnValue(asList(new VcsRootEntry(root1, rules1), new VcsRootEntry(root2, rules2))));
+    }});
+
+    AgentCheckoutAbility canCheckout1 = myVcsSupport.canCheckout(root1, rules1, build);
+    AgentCheckoutAbility canCheckout2 = myVcsSupport.canCheckout(root2, rules2, build);
+
+    if (setup.isShouldFail()) {
+      assertThat(canCheckout1.getCanNotCheckoutReason().getDetails(), CoreMatchers.equalTo(
+              "Cannot checkout VCS root '" + root1.getName() + "' into the same directory as VCS root '" + root2.getName() + "'"));
+      assertThat(canCheckout2.getCanNotCheckoutReason().getDetails(), CoreMatchers.equalTo(
+              "Cannot checkout VCS root '" + root2.getName() + "' into the same directory as VCS root '" + root1.getName() + "'"));
+    } else {
+      assertNull(canCheckout1.getCanNotCheckoutReason());
+      assertNull(canCheckout2.getCanNotCheckoutReason());
+    }
+  }
+
+
+  @TestFor(issues = "TW-49811")
+  public void auto_checkout_many_roots_ignore_broken() throws Exception {
+    //root1 and root2 have intersecting checkout dir (dir1), but
+    //root2 also has an unsupported checkout rule; we should ignore
+    //root2 during canCheckout() call for root1
+    VcsRootImpl root1 = new VcsRootBuilder().withId(1).withUrl("http://some.org/repo1").build();
+    VcsRootImpl root2 = new VcsRootBuilder().withId(2).withUrl("http://some.org/repo2").build();
+
+    CheckoutRules rules1 = new CheckoutRules("+:.=>dir1");
+    CheckoutRules rules2 = new CheckoutRules("+:.=>dir1\n+:dir2=>dir3");
+    AgentRunningBuild build = myContext.mock(AgentRunningBuild.class, "build" + myBuildCounter++);
+    myContext.checking(new Expectations() {{
+      allowing(build).getVcsRootEntries();
+      will(returnValue(asList(new VcsRootEntry(root1, rules1), new VcsRootEntry(root2, rules2))));
+    }});
+
+    AgentCheckoutAbility canCheckout1 = myVcsSupport.canCheckout(root1, rules1, build);
+    AgentCheckoutAbility canCheckout2 = myVcsSupport.canCheckout(root2, rules2, build);
+    assertNull(canCheckout1.getCanNotCheckoutReason());
+    assertThat(canCheckout2.getCanNotCheckoutReason().getType(), CoreMatchers.equalTo(AgentCanNotCheckoutReason.NOT_SUPPORTED_CHECKOUT_RULES));
+    assertThat(canCheckout2.getCanNotCheckoutReason().getDetails(), containsString("Invalid include rule: dir2=>dir3"));
+  }
+
+
   public void checkout_on_agent() throws IOException, VcsException {
     testUpdate(createVcsRoot(simpleRepo()), "4:b06a290a363b", "cleanPatch1/after", new IncludeRule(".", ".", null));
   }
@@ -178,7 +261,6 @@
       allowing(build).getBuildLogger(); will(returnValue(myLogger));
       allowing(build).getSharedConfigParameters(); will(returnValue(sharedConfigParameters));
     }});
-    assertNull(myVcsSupport.canCheckout(vcsRoot, new CheckoutRules(""), build).getCanNotCheckoutReason());
     myVcsSupport.getUpdater(vcsRoot, new CheckoutRules(""), version, myWorkDir, build, false).process(includeRule, actualWorkDir);
 
     File hgDir = new File(actualWorkDir, ".hg");
@@ -322,5 +404,46 @@
     return "mercurial-tests/testData";
   }
 
+  private static class Setup {
+    private String myCheckoutRules1 = CheckoutRules.DEFAULT.getAsString();
+    private String myCheckoutRules2 = CheckoutRules.DEFAULT.getAsString();
+    private boolean myShouldFail;
 
+    @NotNull
+    public String getCheckoutRules1() {
+      return myCheckoutRules1;
+    }
+
+    @NotNull
+    public Setup setCheckoutRules1(@NotNull String checkoutRules1) {
+      myCheckoutRules1 = checkoutRules1;
+      return this;
+    }
+
+    @NotNull
+    public String getCheckoutRules2() {
+      return myCheckoutRules2;
+    }
+
+    @NotNull
+    public Setup setCheckoutRules2(@NotNull String checkoutRules2) {
+      myCheckoutRules2 = checkoutRules2;
+      return this;
+    }
+
+    public boolean isShouldFail() {
+      return myShouldFail;
+    }
+
+    @NotNull
+    public Setup setShouldFail(boolean shouldFail) {
+      myShouldFail = shouldFail;
+      return this;
+    }
+
+    @Override
+    public String toString() {
+      return "rules1: '" + myCheckoutRules1 + "', rules2: '" + myCheckoutRules2 + "'";
+    }
+  }
 }