From 0a88e0d2b8dd1790386de1d26af1701d904d71bf Mon Sep 17 00:00:00 2001 From: Adrian Gonzalez Date: Mon, 4 Dec 2017 17:21:37 -0500 Subject: [PATCH] Mirror specific refspecs instead of using regular expressions --- README.md | 6 +- pom.xml | 2 +- .../bitbucket/hook/MirrorRepositoryHook.java | 93 ++++++---------- .../i18n/stash-hook-mirror.properties | 4 +- .../static/mirror-repository-hook.soy | 20 ++-- .../hook/DefaultPasswordEncryptorTest.java | 9 +- .../hook/MirrorRepositoryHookTest.java | 105 ++++-------------- 7 files changed, 79 insertions(+), 160 deletions(-) diff --git a/README.md b/README.md index 8e39884..da82a7b 100644 --- a/README.md +++ b/README.md @@ -12,11 +12,15 @@ The following is a plugin for Atlassian Bitbucket Server to provide repository m - 'pi' reinstalls the plugin into the running product instance * `atlas-help` -- prints description for all commands in the SDK -Full documentation is always available at: +Full documentation is always available at: https://developer.atlassian.com/display/DOCS/Introduction+to+the+Atlassian+Plugin+SDK +Soy documentation is available here: +https://bitbucket.org/atlassian/aui-adg/wiki/versions/product-version-matrix +https://bitbucket.org/atlassian/aui/src/master/src/soy/form.soy + The plugin can be found on the Atlassian Marketplace here: https://marketplace.atlassian.com/plugins/com.englishtown.stash-hook-mirror diff --git a/pom.xml b/pom.xml index 1ad4217..48cea25 100644 --- a/pom.xml +++ b/pom.xml @@ -33,7 +33,7 @@ 5.0.2 ${bitbucket.version} 3.1.0 - 6.3.0 + 6.3.7 diff --git a/src/main/java/com/englishtown/bitbucket/hook/MirrorRepositoryHook.java b/src/main/java/com/englishtown/bitbucket/hook/MirrorRepositoryHook.java index 152cd14..37a4cfb 100644 --- a/src/main/java/com/englishtown/bitbucket/hook/MirrorRepositoryHook.java +++ b/src/main/java/com/englishtown/bitbucket/hook/MirrorRepositoryHook.java @@ -3,14 +3,20 @@ package com.englishtown.bitbucket.hook; import com.atlassian.bitbucket.hook.repository.AsyncPostReceiveRepositoryHook; import com.atlassian.bitbucket.hook.repository.RepositoryHookContext; import com.atlassian.bitbucket.i18n.I18nService; -import com.atlassian.bitbucket.repository.*; -import com.atlassian.bitbucket.scm.*; +import com.atlassian.bitbucket.repository.RefChange; +import com.atlassian.bitbucket.repository.Repository; +import com.atlassian.bitbucket.repository.RepositoryService; +import com.atlassian.bitbucket.scm.CommandExitHandler; +import com.atlassian.bitbucket.scm.DefaultCommandExitHandler; +import com.atlassian.bitbucket.scm.ScmCommandBuilder; +import com.atlassian.bitbucket.scm.ScmService; import com.atlassian.bitbucket.scm.git.command.GitScmCommandBuilder; import com.atlassian.bitbucket.setting.RepositorySettingsValidator; import com.atlassian.bitbucket.setting.Settings; import com.atlassian.bitbucket.setting.SettingsValidationErrors; import com.atlassian.sal.api.pluginsettings.PluginSettings; import com.atlassian.sal.api.pluginsettings.PluginSettingsFactory; +import com.google.common.base.Strings; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -21,8 +27,6 @@ import java.net.URISyntaxException; import java.util.*; import java.util.concurrent.ScheduledExecutorService; import java.util.concurrent.TimeUnit; -import java.util.regex.Pattern; -import java.util.regex.PatternSyntaxException; public class MirrorRepositoryHook implements AsyncPostReceiveRepositoryHook, RepositorySettingsValidator { @@ -31,15 +35,16 @@ public class MirrorRepositoryHook implements AsyncPostReceiveRepositoryHook, Rep String username; String password; String suffix; - String branchesIncludePattern; + String refspec; } public static final String PLUGIN_SETTINGS_KEY = "com.englishtown.stash.hook.mirror"; static final String SETTING_MIRROR_REPO_URL = "mirrorRepoUrl"; static final String SETTING_USERNAME = "username"; static final String SETTING_PASSWORD = "password"; - static final String SETTING_BRANCHES_INCLUDE_PATTERN = "branchesIncludePattern"; + static final String SETTING_REFSPEC = "refspec"; static final int MAX_ATTEMPTS = 5; + static final String DEFAULT_REFSPEC = "+refs/heads/*:refs/heads/*"; private final ScmService scmService; private final I18nService i18nService; @@ -97,12 +102,12 @@ public class MirrorRepositoryHook implements AsyncPostReceiveRepositoryHook, Rep List mirrorSettings = getMirrorSettings(context.getSettings()); for (MirrorSettings settings : mirrorSettings) { - runMirrorCommand(settings, context.getRepository(), refChanges); + runMirrorCommand(settings, context.getRepository()); } } - void runMirrorCommand(MirrorSettings settings, final Repository repository, Collection refChanges) { + void runMirrorCommand(MirrorSettings settings, final Repository repository) { if (repositoryService.isEmpty(repository)) { return; } @@ -129,16 +134,23 @@ public class MirrorRepositoryHook implements AsyncPostReceiveRepositoryHook, Rep // Call push command with the prune flag and refspecs for heads and tags // Do not use the mirror flag as pull-request refs are included builder.command("push") - .argument("--prune") // this deletes locally deleted branches - .argument("--atomic") // use an atomic transaction to have a consistent state - .argument(authenticatedUrl) - .argument("--force") // Canonical repository should always take precedence over mirror - .argument("+refs/tags/*:refs/tags/*") // and tags - .argument("+refs/notes/*:refs/notes/*"); // and notes - // add branch arguments - addBranchArguments(settings, refChanges, builder); + .argument("--prune") // this deletes locally deleted branches + .argument("--atomic") // use an atomic transaction to have a consistent state + .argument(authenticatedUrl) + .argument("--force") // Canonical repository should always take precedence over mirror + .argument("+refs/tags/*:refs/tags/*") // and tags + .argument("+refs/notes/*:refs/notes/*"); // and notes + + // Add refspec args + String refspecs = Strings.isNullOrEmpty(settings.refspec) ? DEFAULT_REFSPEC : settings.refspec; + for (String refspec : refspecs.split("\\s|\\n")) { + if (!Strings.isNullOrEmpty(refspec)) { + builder.argument(refspec); + } + } + builder.errorHandler(passwordHandler) - .exitHandler(passwordHandler); + .exitHandler(passwordHandler); String result = builder.build(passwordHandler).call(); @@ -207,7 +219,7 @@ public class MirrorRepositoryHook implements AsyncPostReceiveRepositoryHook, Rep if (ok) { updateSettings(mirrorSettings, settings); for (MirrorSettings ms : mirrorSettings) { - runMirrorCommand(ms, repository, Collections.EMPTY_LIST); + runMirrorCommand(ms, repository); } } @@ -232,7 +244,7 @@ public class MirrorRepositoryHook implements AsyncPostReceiveRepositoryHook, Rep ms.mirrorRepoUrl = settings.getString(SETTING_MIRROR_REPO_URL + suffix, ""); ms.username = settings.getString(SETTING_USERNAME + suffix, ""); ms.password = settings.getString(SETTING_PASSWORD + suffix, ""); - ms.branchesIncludePattern = (settings.getString(SETTING_BRANCHES_INCLUDE_PATTERN + suffix, "")); + ms.refspec = (settings.getString(SETTING_REFSPEC + suffix, "")); ms.suffix = String.valueOf(count++); results.add(ms); @@ -285,12 +297,10 @@ public class MirrorRepositoryHook implements AsyncPostReceiveRepositoryHook, Rep ms.password = ms.username = ""; } - if (!ms.branchesIncludePattern.isEmpty()) { - try { - Pattern.compile(ms.branchesIncludePattern); - } catch (PatternSyntaxException e) { + if (!ms.refspec.isEmpty()) { + if (!ms.refspec.contains(":")) { result = false; - errors.addFieldError(SETTING_BRANCHES_INCLUDE_PATTERN + ms.suffix, "This is not a valid regular expression."); + errors.addFieldError(SETTING_REFSPEC + ms.suffix, "A refspec should be in the form :."); } } @@ -306,7 +316,7 @@ public class MirrorRepositoryHook implements AsyncPostReceiveRepositoryHook, Rep values.put(SETTING_MIRROR_REPO_URL + ms.suffix, ms.mirrorRepoUrl); values.put(SETTING_USERNAME + ms.suffix, ms.username); values.put(SETTING_PASSWORD + ms.suffix, (ms.password.isEmpty() ? ms.password : passwordEncryptor.encrypt(ms.password))); - values.put(SETTING_BRANCHES_INCLUDE_PATTERN + ms.suffix, ms.branchesIncludePattern); + values.put(SETTING_REFSPEC + ms.suffix, ms.refspec); } // Unfortunately the settings are stored in an immutable map, so need to cheat with reflection @@ -330,37 +340,4 @@ public class MirrorRepositoryHook implements AsyncPostReceiveRepositoryHook, Rep return new PasswordHandler(password, new DefaultCommandExitHandler(i18nService)); } - private void addBranchArguments(MirrorSettings settings, Collection refChanges, CommandBuilder builder) { - Map branchModifyArguments = new HashMap<>(); - Map branchDeleteArguments = new HashMap<>(); - - // if an empty list of RefChanges was provided we assume this was caused by triggering after a config change. - // same if no branch pattern was specified we sync all branches - if(refChanges.isEmpty() || settings.branchesIncludePattern.isEmpty()) { - builder.argument("+refs/heads/*:refs/heads/*"); - } else { - for (RefChange refChange : refChanges) { - MinimalRef ref = refChange.getRef(); - String displayId = ref.getDisplayId(); - // branch operations - if (ref.getType().equals(StandardRefType.BRANCH)) { - if (displayId.matches(settings.branchesIncludePattern)) { - if (refChange.getType().equals(RefChangeType.DELETE) && !branchDeleteArguments.containsKey(displayId)) { - branchDeleteArguments.put(displayId, "+:refs/heads/" + displayId); - } else if ((refChange.getType().equals(RefChangeType.ADD) || refChange.getType().equals(RefChangeType.UPDATE)) && !branchModifyArguments.containsKey(displayId)) { - branchModifyArguments.put(displayId, "+refs/heads/" + displayId + ":refs/heads/" + displayId); - } - } - } - } - - for (String key : branchDeleteArguments.keySet()) { - builder.argument(branchDeleteArguments.get(key)); - } - - for (String key : branchModifyArguments.keySet()) { - builder.argument(branchModifyArguments.get(key)); - } - } - } } diff --git a/src/main/resources/i18n/stash-hook-mirror.properties b/src/main/resources/i18n/stash-hook-mirror.properties index f66005e..e2f9657 100644 --- a/src/main/resources/i18n/stash-hook-mirror.properties +++ b/src/main/resources/i18n/stash-hook-mirror.properties @@ -11,5 +11,5 @@ mirror-repository-hook.username.description=The username to use for pushing to t mirror-repository-hook.password.label=Password mirror-repository-hook.password.description=The password to use for pushing to the mirror over http(s) -mirror-repository-hook.branchesIncludePattern.label=Branches Include Pattern -mirror-repository-hook.branchesIncludePattern.description=Regex pattern for branches to include +mirror-repository-hook.refspec.label=Refspec +mirror-repository-hook.refspec.description=The git refspec(s) to mirror (defaults to +refs/heads/*:refs/heads/*) diff --git a/src/main/resources/static/mirror-repository-hook.soy b/src/main/resources/static/mirror-repository-hook.soy index 95986b3..e9c3bec 100644 --- a/src/main/resources/static/mirror-repository-hook.soy +++ b/src/main/resources/static/mirror-repository-hook.soy @@ -48,7 +48,7 @@ {/param} {param isRequired: true /} {param descriptionText: getText('mirror-repository-hook.mirrorRepoUrl.description') /} - {param extraClasses: 'long et-mirror-repo' /} + {param extraClasses: 'et-mirror-repo' /} {param errorTexts: $errors ? $errors['mirrorRepoUrl' + $index] : null /} {/call} {call aui.form.textField} @@ -58,7 +58,6 @@ {getText('mirror-repository-hook.username.label')} {/param} {param descriptionText: getText('mirror-repository-hook.username.description') /} - {param extraClasses: 'long' /} {param errorTexts: $errors ? $errors['username' + $index] : null /} {/call} {call aui.form.passwordField} @@ -68,20 +67,17 @@ {getText('mirror-repository-hook.password.label')} {/param} {param descriptionText: getText('mirror-repository-hook.password.description') /} - {param extraClasses: 'long' /} {param errorTexts: $errors ? $errors['password' + $index] : null /} {/call} - {call aui.form.textField} - {param id: 'branchesIncludePattern' + $index /} - {param value: $config['branchesIncludePattern' + $index] /} + {call aui.form.textareaField} + {param id: 'refspec' + $index /} + {param value: $config['refspec' + $index] /} {param labelContent} - {getText('mirror-repository-hook.branchesIncludePattern.label')} + {getText('mirror-repository-hook.refspec.label')} {/param} - {param descriptionText: getText('mirror-repository-hook.branchesIncludePattern.description') /} - {param extraClasses: 'long' /} - {param errorTexts: $errors ? $errors['branchesIncludePattern' + $index] : null /} + {param descriptionText: getText('mirror-repository-hook.refspec.description') /} + {param fieldWidth: 'long' /} + {param errorTexts: $errors ? $errors['refspec' + $index] : null /} {/call} {/template} - - diff --git a/src/test/java/com/englishtown/bitbucket/hook/DefaultPasswordEncryptorTest.java b/src/test/java/com/englishtown/bitbucket/hook/DefaultPasswordEncryptorTest.java index b945d92..9de97ca 100644 --- a/src/test/java/com/englishtown/bitbucket/hook/DefaultPasswordEncryptorTest.java +++ b/src/test/java/com/englishtown/bitbucket/hook/DefaultPasswordEncryptorTest.java @@ -2,10 +2,11 @@ package com.englishtown.bitbucket.hook; import com.atlassian.sal.api.pluginsettings.PluginSettings; import org.junit.Before; +import org.junit.Rule; import org.junit.Test; -import org.junit.runner.RunWith; import org.mockito.Mock; -import org.mockito.runners.MockitoJUnitRunner; +import org.mockito.junit.MockitoJUnit; +import org.mockito.junit.MockitoRule; import static org.junit.Assert.*; import static org.mockito.Mockito.*; @@ -13,11 +14,13 @@ import static org.mockito.Mockito.*; /** * DefaultPasswordEncryptor unit tests */ -@RunWith(MockitoJUnitRunner.class) public class DefaultPasswordEncryptorTest { private final static String CRYPTO_KEY = "m3ys5YexQc7irRlmJeCwAw=="; + @Rule + public MockitoRule mockitoRule = MockitoJUnit.rule(); + @Mock private PluginSettings pluginSettings; diff --git a/src/test/java/com/englishtown/bitbucket/hook/MirrorRepositoryHookTest.java b/src/test/java/com/englishtown/bitbucket/hook/MirrorRepositoryHookTest.java index d07f950..be49d32 100644 --- a/src/test/java/com/englishtown/bitbucket/hook/MirrorRepositoryHookTest.java +++ b/src/test/java/com/englishtown/bitbucket/hook/MirrorRepositoryHookTest.java @@ -2,7 +2,8 @@ package com.englishtown.bitbucket.hook; import com.atlassian.bitbucket.hook.repository.RepositoryHookContext; import com.atlassian.bitbucket.i18n.I18nService; -import com.atlassian.bitbucket.repository.*; +import com.atlassian.bitbucket.repository.Repository; +import com.atlassian.bitbucket.repository.RepositoryService; import com.atlassian.bitbucket.scm.CommandErrorHandler; import com.atlassian.bitbucket.scm.CommandExitHandler; import com.atlassian.bitbucket.scm.CommandOutputHandler; @@ -14,15 +15,18 @@ import com.atlassian.bitbucket.setting.SettingsValidationErrors; import com.atlassian.sal.api.pluginsettings.PluginSettings; import com.atlassian.sal.api.pluginsettings.PluginSettingsFactory; import org.junit.Before; +import org.junit.Rule; import org.junit.Test; -import org.junit.runner.RunWith; import org.mockito.ArgumentCaptor; +import org.mockito.ArgumentMatchers; import org.mockito.Captor; -import org.mockito.Matchers; import org.mockito.Mock; -import org.mockito.runners.MockitoJUnitRunner; +import org.mockito.junit.MockitoJUnit; +import org.mockito.junit.MockitoRule; -import java.util.*; +import java.util.ArrayList; +import java.util.HashMap; +import java.util.Map; import java.util.concurrent.ScheduledExecutorService; import java.util.concurrent.TimeUnit; @@ -32,12 +36,14 @@ import static org.mockito.Mockito.*; /** * Unit tests for {@link MirrorRepositoryHook} */ -@RunWith(MockitoJUnitRunner.class) public class MirrorRepositoryHookTest { private MirrorRepositoryHook hook; private GitScmCommandBuilder builder; + @Rule + public MockitoRule mockitoRule = MockitoJUnit.rule(); + @Mock private ScmService scmService; @Mock @@ -60,7 +66,7 @@ public class MirrorRepositoryHookTest { private final String username = "test-user"; private final String password = "test-password"; private final String repository = "https://test-user:test-password@bitbucket-mirror.englishtown.com/scm/test/test.git"; - private final String branchesIncludePattern ="master"; + private final String refspec = "+refs/heads/master:refs/heads/master +refs/heads/develop:refs/heads/develop"; @Captor ArgumentCaptor argumentCaptor; @@ -94,44 +100,6 @@ public class MirrorRepositoryHookTest { verifyExecutor(); } - @Test - public void testPostReceiveWithBranchesPatternUpdateOperation() throws Exception { - when(passwordEncryptor.decrypt(anyString())).thenReturn(password); - - Repository repo = mock(Repository.class); - RefChange refChange = mock(RefChange.class); - MinimalRef ref = mock(MinimalRef.class); - - List refChanges = new ArrayList<>(); - refChanges.add(refChange); - when(refChange.getRef()).thenReturn(ref); - when(refChange.getType()).thenReturn(RefChangeType.UPDATE); - when(ref.getType()).thenReturn(StandardRefType.BRANCH); - when(ref.getDisplayId()).thenReturn("master"); - - hook.postReceive(buildContext(repo), refChanges); - verifyExecutorWithBranchesPatternUpdateOperation(); - } - - @Test - public void testPostReceiveWithBranchesPatternDeleteOperation() throws Exception { - when(passwordEncryptor.decrypt(anyString())).thenReturn(password); - - Repository repo = mock(Repository.class); - RefChange refChange = mock(RefChange.class); - MinimalRef ref = mock(MinimalRef.class); - - List refChanges = new ArrayList<>(); - refChanges.add(refChange); - when(refChange.getRef()).thenReturn(ref); - when(refChange.getType()).thenReturn(RefChangeType.DELETE); - when(ref.getType()).thenReturn(StandardRefType.BRANCH); - when(ref.getDisplayId()).thenReturn("master"); - - hook.postReceive(buildContext(repo), refChanges); - verifyExecutorWithBranchesPatternDeleteOperation(); - } - @Test public void testEmptyRepositoriesNotMirrored() { Repository repo = mock(Repository.class); @@ -139,7 +107,7 @@ public class MirrorRepositoryHookTest { hook.postReceive(buildContext(repo), new ArrayList<>()); - verify(executor, never()).submit(Matchers.any()); + verify(executor, never()).submit(ArgumentMatchers.any()); } @Test @@ -152,7 +120,7 @@ public class MirrorRepositoryHookTest { ms.mirrorRepoUrl = mirrorRepoUrlHttp; ms.username = username; ms.password = password; - hook.runMirrorCommand(ms, mock(Repository.class), Collections.emptyList()); + hook.runMirrorCommand(ms, mock(Repository.class)); verify(executor).submit(argumentCaptor.capture()); Runnable runnable = argumentCaptor.getValue(); @@ -192,40 +160,11 @@ public class MirrorRepositoryHookTest { verify(builder, times(1)).argument(eq("--prune")); verify(builder, times(1)).argument(eq("--atomic")); verify(builder, times(1)).argument(eq(repository)); - verify(builder, times(1)).argument(eq("+refs/heads/*:refs/heads/*")); + verify(builder, times(1)).argument(eq("--force")); verify(builder, times(1)).argument(eq("+refs/tags/*:refs/tags/*")); - verify(cmd, times(1)).call(); - - } - - private void verifyExecutorWithBranchesPatternUpdateOperation() throws Exception { - - verify(executor).submit(argumentCaptor.capture()); - Runnable runnable = argumentCaptor.getValue(); - runnable.run(); - - verify(builder, times(1)).command(eq("push")); - verify(builder, times(1)).argument(eq("--prune")); - verify(builder, times(1)).argument(eq("--atomic")); - verify(builder, times(1)).argument(eq(repository)); + verify(builder, times(1)).argument(eq("+refs/notes/*:refs/notes/*")); verify(builder, times(1)).argument(eq("+refs/heads/master:refs/heads/master")); - verify(builder, times(1)).argument(eq("+refs/tags/*:refs/tags/*")); - verify(cmd, times(1)).call(); - - } - - private void verifyExecutorWithBranchesPatternDeleteOperation() throws Exception { - - verify(executor).submit(argumentCaptor.capture()); - Runnable runnable = argumentCaptor.getValue(); - runnable.run(); - - verify(builder, times(1)).command(eq("push")); - verify(builder, times(1)).argument(eq("--prune")); - verify(builder, times(1)).argument(eq("--atomic")); - verify(builder, times(1)).argument(eq(repository)); - verify(builder, times(1)).argument(eq("+:refs/heads/master")); - verify(builder, times(1)).argument(eq("+refs/tags/*:refs/tags/*")); + verify(builder, times(1)).argument(eq("+refs/heads/develop:refs/heads/develop")); verify(cmd, times(1)).call(); } @@ -270,9 +209,9 @@ public class MirrorRepositoryHookTest { .thenReturn("") .thenReturn(password); - when(settings.getString(eq(MirrorRepositoryHook.SETTING_BRANCHES_INCLUDE_PATTERN + "0"), eq(""))) + when(settings.getString(eq(MirrorRepositoryHook.SETTING_REFSPEC + "0"), eq(""))) .thenReturn("??") - .thenReturn("master") + .thenReturn("+refs/heads/master:refs/heads/master") .thenReturn(""); Repository repo = mock(Repository.class); @@ -289,7 +228,7 @@ public class MirrorRepositoryHookTest { verify(errors).addFieldError(eq(MirrorRepositoryHook.SETTING_MIRROR_REPO_URL + "0"), anyString()); verify(errors, never()).addFieldError(eq(MirrorRepositoryHook.SETTING_USERNAME + "0"), anyString()); verify(errors, never()).addFieldError(eq(MirrorRepositoryHook.SETTING_PASSWORD + "0"), anyString()); - verify(errors).addFieldError(eq(MirrorRepositoryHook.SETTING_BRANCHES_INCLUDE_PATTERN + "0"), anyString()); + verify(errors).addFieldError(eq(MirrorRepositoryHook.SETTING_REFSPEC + "0"), anyString()); errors = mock(SettingsValidationErrors.class); hook.validate(settings, errors, repo); @@ -347,7 +286,7 @@ public class MirrorRepositoryHookTest { when(settings.getString(eq(MirrorRepositoryHook.SETTING_MIRROR_REPO_URL), eq(""))).thenReturn(mirrorRepoUrlHttp); when(settings.getString(eq(MirrorRepositoryHook.SETTING_USERNAME), eq(""))).thenReturn(username); when(settings.getString(eq(MirrorRepositoryHook.SETTING_PASSWORD), eq(""))).thenReturn(password); - when(settings.getString(eq(MirrorRepositoryHook.SETTING_BRANCHES_INCLUDE_PATTERN), eq(""))).thenReturn(branchesIncludePattern); + when(settings.getString(eq(MirrorRepositoryHook.SETTING_REFSPEC), eq(""))).thenReturn(refspec); return settings; } }