From ea4a148d8623843ae33a528f044e1c318ef801b2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Pfeuffer?= Date: Tue, 28 May 2024 08:54:30 +0200 Subject: [PATCH] Fix determination of default branch for git config on push --- gradle/changelog/git_config_init.yaml | 2 + gradle/changelog/git_initialization.yaml | 3 + .../GitRepositoryConfigInitializer.java | 41 +++- .../GitRepositoryConfigInitializerTest.java | 220 +++++++++++------- 4 files changed, 169 insertions(+), 97 deletions(-) create mode 100644 gradle/changelog/git_config_init.yaml create mode 100644 gradle/changelog/git_initialization.yaml diff --git a/gradle/changelog/git_config_init.yaml b/gradle/changelog/git_config_init.yaml new file mode 100644 index 0000000000..1d71dac14c --- /dev/null +++ b/gradle/changelog/git_config_init.yaml @@ -0,0 +1,2 @@ +- type: fixed + description: Default branch evaluation on git config initialization diff --git a/gradle/changelog/git_initialization.yaml b/gradle/changelog/git_initialization.yaml new file mode 100644 index 0000000000..6f1b76d336 --- /dev/null +++ b/gradle/changelog/git_initialization.yaml @@ -0,0 +1,3 @@ +- type: fixed + description: Exception in SVN repositories due to incorrect git initialization (Backport from 2.47) + diff --git a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/GitRepositoryConfigInitializer.java b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/GitRepositoryConfigInitializer.java index edd56e2e74..6d986a5072 100644 --- a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/GitRepositoryConfigInitializer.java +++ b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/GitRepositoryConfigInitializer.java @@ -29,11 +29,16 @@ import com.google.common.base.Strings; import sonia.scm.EagerSingleton; import sonia.scm.api.v2.resources.GitRepositoryConfigStoreProvider; import sonia.scm.plugin.Extension; +import sonia.scm.repository.api.RepositoryService; +import sonia.scm.repository.api.RepositoryServiceFactory; import sonia.scm.store.ConfigurationStore; -import javax.inject.Inject; +import java.io.IOException; import java.util.Comparator; import java.util.List; +import java.util.Optional; + +import javax.inject.Inject; @Extension @EagerSingleton @@ -41,28 +46,40 @@ public class GitRepositoryConfigInitializer { private final GitRepositoryHandler repoHandler; private final GitRepositoryConfigStoreProvider storeProvider; + private final RepositoryServiceFactory serviceFactory; @Inject - public GitRepositoryConfigInitializer(GitRepositoryHandler repoHandler, GitRepositoryConfigStoreProvider storeProvider) { + public GitRepositoryConfigInitializer(GitRepositoryHandler repoHandler, GitRepositoryConfigStoreProvider storeProvider, RepositoryServiceFactory serviceFactory) { this.repoHandler = repoHandler; this.storeProvider = storeProvider; + this.serviceFactory = serviceFactory; } @Subscribe - public void initConfig(PostReceiveRepositoryHookEvent event) { - ConfigurationStore store = storeProvider.get(event.getRepository()); - GitRepositoryConfig repositoryConfig = store.get(); - if (repositoryConfig == null || Strings.isNullOrEmpty(repositoryConfig.getDefaultBranch())) { - List defaultBranchCandidates = event.getContext().getBranchProvider().getCreatedOrModified(); + public void initConfig(PostReceiveRepositoryHookEvent event) throws IOException { + if (GitRepositoryHandler.TYPE_NAME.equals(event.getRepository().getType())) { + ConfigurationStore store = storeProvider.get(event.getRepository()); + GitRepositoryConfig repositoryConfig = store.get(); + if (repositoryConfig == null || Strings.isNullOrEmpty(repositoryConfig.getDefaultBranch())) { + String defaultBranch; + try (RepositoryService service = serviceFactory.create(event.getRepository())) { + List branches = service.getBranchesCommand().getBranches().getBranches(); + Optional repoDefaultBranch = branches.stream().filter(Branch::isDefaultBranch).findFirst(); + if (repoDefaultBranch.isPresent()) { + defaultBranch = repoDefaultBranch.get().getName(); + } else { + defaultBranch = determineDefaultBranchFromPush(event); + } + } - String defaultBranch = determineDefaultBranch(defaultBranchCandidates); - - GitRepositoryConfig gitRepositoryConfig = new GitRepositoryConfig(defaultBranch); - store.set(gitRepositoryConfig); + GitRepositoryConfig gitRepositoryConfig = new GitRepositoryConfig(defaultBranch); + store.set(gitRepositoryConfig); + } } } - private String determineDefaultBranch(List defaultBranchCandidates) { + private String determineDefaultBranchFromPush(PostReceiveRepositoryHookEvent event) { + List defaultBranchCandidates = event.getContext().getBranchProvider().getCreatedOrModified(); String globalConfigDefaultBranch = repoHandler.getConfig().getDefaultBranch(); if (defaultBranchCandidates.contains(globalConfigDefaultBranch)) { return globalConfigDefaultBranch; diff --git a/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/GitRepositoryConfigInitializerTest.java b/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/GitRepositoryConfigInitializerTest.java index d89d8a42a4..bfe4f5499e 100644 --- a/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/GitRepositoryConfigInitializerTest.java +++ b/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/GitRepositoryConfigInitializerTest.java @@ -28,18 +28,24 @@ import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.Answers; import org.mockito.InjectMocks; import org.mockito.Mock; import org.mockito.junit.jupiter.MockitoExtension; import sonia.scm.api.v2.resources.GitRepositoryConfigStoreProvider; +import sonia.scm.repository.api.BranchesCommandBuilder; import sonia.scm.repository.api.HookBranchProvider; import sonia.scm.repository.api.HookContext; +import sonia.scm.repository.api.RepositoryService; +import sonia.scm.repository.api.RepositoryServiceFactory; import sonia.scm.store.ConfigurationStore; +import java.io.IOException; import java.util.List; import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.ArgumentMatchers.argThat; +import static org.mockito.Mockito.lenient; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; import static org.mockito.Mockito.verify; @@ -62,116 +68,160 @@ class GitRepositoryConfigInitializerTest { @Mock private PostReceiveRepositoryHookEvent event; + @Mock + private RepositoryServiceFactory serviceFactory; + @Mock + private RepositoryService service; + @Mock(answer = Answers.RETURNS_DEEP_STUBS) + private BranchesCommandBuilder branchesCommand; + @Mock + private Branches branches; + @InjectMocks private GitRepositoryConfigInitializer initializer; - @BeforeEach - void initMocks() { - when(storeProvider.get(REPOSITORY)).thenReturn(configStore); - when(event.getRepository()).thenReturn(REPOSITORY); - } - @Test - void shouldDoNothingIfDefaultBranchAlreadySet() { - when(configStore.get()).thenReturn(new GitRepositoryConfig("any")); + void shouldSkipNonGitRepositories() throws IOException { + REPOSITORY.setType("svn"); initializer.initConfig(event); verify(event, never()).getContext(); } + @BeforeEach + void initEvent() { + when(event.getRepository()).thenReturn(REPOSITORY); + } + @Nested - class WithGlobalConfig { + class ForGitRepositories { @BeforeEach - void initRepoHandler() { - GitConfig gitConfig = new GitConfig(); - gitConfig.setDefaultBranch("global_default"); - when(repoHandler.getConfig()).thenReturn(gitConfig); + void initMocks() throws IOException { + when(storeProvider.get(REPOSITORY)).thenReturn(configStore); + REPOSITORY.setType("git"); + lenient().when(serviceFactory.create(event.getRepository())).thenReturn(service); + lenient().when(service.getBranchesCommand()).thenReturn(branchesCommand); + lenient().when(branchesCommand.getBranches()).thenReturn(branches); } @Test - void shouldSetDefaultBranchIfNoGitConfigYet() { - when(configStore.get()).thenReturn(null); - initEvent(List.of("main")); + void shouldDoNothingIfDefaultBranchAlreadySet() throws IOException { + when(configStore.get()).thenReturn(new GitRepositoryConfig("any")); initializer.initConfig(event); - verify(event).getContext(); + verify(event, never()).getContext(); } - @Test - void shouldDetermineAndApplyDefaultBranch_GlobalDefault() { - initEvent(List.of("global_default", "main", "master")); - when(configStore.get()).thenReturn(new GitRepositoryConfig(null)); + @Nested + class WithGlobalConfig { - initializer.initConfig(event); + @BeforeEach + void initRepoHandler() { + GitConfig gitConfig = new GitConfig(); + gitConfig.setDefaultBranch("global_default"); + lenient().when(repoHandler.getConfig()).thenReturn(gitConfig); + } - verify(configStore).set(argThat(arg -> { - assertThat(arg.getDefaultBranch()).isEqualTo("global_default"); - return true; - })); + + @Test + void shouldSetDefaultBranchFromPushIfNoGitConfigYet() throws IOException { + when(configStore.get()).thenReturn(null); + initEvent(List.of("main")); + + initializer.initConfig(event); + + verify(event).getContext(); + } + + @Test + void shouldDetermineAndApplyDefaultBranch_GlobalDefault() throws IOException { + initEvent(List.of("global_default", "main", "master")); + when(configStore.get()).thenReturn(new GitRepositoryConfig(null)); + + initializer.initConfig(event); + + verify(configStore).set(argThat(arg -> { + assertThat(arg.getDefaultBranch()).isEqualTo("global_default"); + return true; + })); + } + + @Test + void shouldDetermineAndApplyDefaultBranch_Main() throws IOException { + initEvent(List.of("master", "main")); + when(configStore.get()).thenReturn(new GitRepositoryConfig(null)); + + initializer.initConfig(event); + + verify(configStore).set(argThat(arg -> { + assertThat(arg.getDefaultBranch()).isEqualTo("main"); + return true; + })); + } + + @Test + void shouldDetermineAndApplyDefaultBranch_Master() throws IOException { + initEvent(List.of("develop", "master")); + when(configStore.get()).thenReturn(new GitRepositoryConfig(null)); + + initializer.initConfig(event); + + verify(configStore).set(argThat(arg -> { + assertThat(arg.getDefaultBranch()).isEqualTo("master"); + return true; + })); + } + + @Test + void shouldDetermineAndApplyDefaultBranchFromRepository_Staging() throws IOException { + when(configStore.get()).thenReturn(null); + initEvent(List.of("master", "main")); + when(branches.getBranches()).thenReturn(List.of(new Branch("staging", "abc", true))); + + initializer.initConfig(event); + + verify(configStore).set(argThat(arg -> { + assertThat(arg.getDefaultBranch()).isEqualTo("staging"); + return true; + })); + } + + @Test + void shouldDetermineAndApplyDefaultBranch_BestMatch() throws IOException { + initEvent(List.of("test/123", "trillian", "dent")); + when(configStore.get()).thenReturn(new GitRepositoryConfig(null)); + + initializer.initConfig(event); + + verify(configStore).set(argThat(arg -> { + assertThat(arg.getDefaultBranch()).isEqualTo("dent"); + return true; + })); + } + + @Test + void shouldDetermineAndApplyDefaultBranch_AnyFallback() throws IOException { + initEvent(List.of("test/123", "x/y/z")); + when(configStore.get()).thenReturn(new GitRepositoryConfig(null)); + + initializer.initConfig(event); + + verify(configStore).set(argThat(arg -> { + assertThat(arg.getDefaultBranch()).isEqualTo("test/123"); + return true; + })); + } } - @Test - void shouldDetermineAndApplyDefaultBranch_Main() { - initEvent(List.of("master", "main")); - when(configStore.get()).thenReturn(new GitRepositoryConfig(null)); - - initializer.initConfig(event); - - verify(configStore).set(argThat(arg -> { - assertThat(arg.getDefaultBranch()).isEqualTo("main"); - return true; - })); - } - - @Test - void shouldDetermineAndApplyDefaultBranch_Master() { - initEvent(List.of("develop", "master")); - when(configStore.get()).thenReturn(new GitRepositoryConfig(null)); - - initializer.initConfig(event); - - verify(configStore).set(argThat(arg -> { - assertThat(arg.getDefaultBranch()).isEqualTo("master"); - return true; - })); - } - - @Test - void shouldDetermineAndApplyDefaultBranch_BestMatch() { - initEvent(List.of("test/123", "trillian", "dent")); - when(configStore.get()).thenReturn(new GitRepositoryConfig(null)); - - initializer.initConfig(event); - - verify(configStore).set(argThat(arg -> { - assertThat(arg.getDefaultBranch()).isEqualTo("dent"); - return true; - })); - } - - @Test - void shouldDetermineAndApplyDefaultBranch_AnyFallback() { - initEvent(List.of("test/123", "x/y/z")); - when(configStore.get()).thenReturn(new GitRepositoryConfig(null)); - - initializer.initConfig(event); - - verify(configStore).set(argThat(arg -> { - assertThat(arg.getDefaultBranch()).isEqualTo("test/123"); - return true; - })); + private void initEvent(List branches) { + HookContext hookContext = mock(HookContext.class); + lenient().when(event.getContext()).thenReturn(hookContext); + HookBranchProvider branchProvider = mock(HookBranchProvider.class); + lenient().when(hookContext.getBranchProvider()).thenReturn(branchProvider); + lenient().when(branchProvider.getCreatedOrModified()).thenReturn(branches); } } - - private void initEvent(List branches) { - HookContext hookContext = mock(HookContext.class); - when(event.getContext()).thenReturn(hookContext); - HookBranchProvider branchProvider = mock(HookBranchProvider.class); - when(hookContext.getBranchProvider()).thenReturn(branchProvider); - when(branchProvider.getCreatedOrModified()).thenReturn(branches); - } - }