From 63e90d7d27869fd50057ed9be2f79f75690cfc51 Mon Sep 17 00:00:00 2001 From: Dave Syer Date: Fri, 25 Sep 2015 09:26:49 +0100 Subject: [PATCH] Remove extra call to git fetch A pull is going to happen anyway, so it's probably just unnecessary overhead. Fixes gh-227 --- .../server/JGitEnvironmentRepository.java | 20 +-- .../JGitEnvironmentRepositoryTests.java | 121 +++++++++--------- 2 files changed, 65 insertions(+), 76 deletions(-) diff --git a/spring-cloud-config-server/src/main/java/org/springframework/cloud/config/server/JGitEnvironmentRepository.java b/spring-cloud-config-server/src/main/java/org/springframework/cloud/config/server/JGitEnvironmentRepository.java index 20b19647..94c3fbda 100644 --- a/spring-cloud-config-server/src/main/java/org/springframework/cloud/config/server/JGitEnvironmentRepository.java +++ b/spring-cloud-config-server/src/main/java/org/springframework/cloud/config/server/JGitEnvironmentRepository.java @@ -27,7 +27,6 @@ import org.apache.commons.logging.LogFactory; import org.eclipse.jgit.api.CheckoutCommand; import org.eclipse.jgit.api.CloneCommand; import org.eclipse.jgit.api.CreateBranchCommand.SetupUpstreamMode; -import org.eclipse.jgit.api.FetchCommand; import org.eclipse.jgit.api.Git; import org.eclipse.jgit.api.ListBranchCommand; import org.eclipse.jgit.api.ListBranchCommand.ListMode; @@ -63,9 +62,9 @@ public class JGitEnvironmentRepository extends AbstractScmEnvironmentRepository private static final String FILE_URI_PREFIX = "file:"; /** - * Timeout (in seconds) for obtaining HTTP or SSH connection (if applicable) + * Timeout (in seconds) for obtaining HTTP or SSH connection (if applicable). Default 5 seconds. */ - private int timeout = 0; + private int timeout = 5; private boolean initialized; @@ -243,7 +242,6 @@ public class JGitEnvironmentRepository extends AbstractScmEnvironmentRepository private Git openGitRepository() throws IOException { Git git = this.gitFactory.getGitByOpen(getWorkingDirectory()); - tryFetch(git); return git; } @@ -268,20 +266,6 @@ public class JGitEnvironmentRepository extends AbstractScmEnvironmentRepository return clone.call(); } - private void tryFetch(Git git) { - try { - FetchCommand fetch = git.fetch(); - setTimeout(fetch); - if (hasText(getUsername())) { - setCredentialsProvider(fetch); - } - fetch.call(); - } - catch (Exception e) { - logger.warn("Remote repository not available"); - } - } - private void deleteBaseDirIfExists() { if (getBasedir().exists()) { try { diff --git a/spring-cloud-config-server/src/test/java/org/springframework/cloud/config/server/JGitEnvironmentRepositoryTests.java b/spring-cloud-config-server/src/test/java/org/springframework/cloud/config/server/JGitEnvironmentRepositoryTests.java index 13d2b39c..7f29b7b5 100644 --- a/spring-cloud-config-server/src/test/java/org/springframework/cloud/config/server/JGitEnvironmentRepositoryTests.java +++ b/spring-cloud-config-server/src/test/java/org/springframework/cloud/config/server/JGitEnvironmentRepositoryTests.java @@ -16,8 +16,14 @@ package org.springframework.cloud.config.server; -import static org.junit.Assert.*; -import static org.mockito.Mockito.*; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; +import static org.mockito.Matchers.any; +import static org.mockito.Matchers.anyString; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; import java.io.File; import java.io.IOException; @@ -28,8 +34,6 @@ import org.eclipse.jgit.util.FileUtils; import org.junit.Before; import org.junit.Test; import org.springframework.cloud.config.environment.Environment; -import org.springframework.cloud.config.server.ConfigServerTestUtils; -import org.springframework.cloud.config.server.JGitEnvironmentRepository; import org.springframework.core.env.StandardEnvironment; /** @@ -40,134 +44,133 @@ public class JGitEnvironmentRepositoryTests { private StandardEnvironment environment = new StandardEnvironment(); private JGitEnvironmentRepository repository = new JGitEnvironmentRepository( - environment); + this.environment); private File basedir = new File("target/config"); @Before public void init() throws Exception { String uri = ConfigServerTestUtils.prepareLocalRepo(); - repository.setUri(uri); - if (basedir.exists()) { - FileUtils.delete(basedir, FileUtils.RECURSIVE | FileUtils.RETRY); + this.repository.setUri(uri); + if (this.basedir.exists()) { + FileUtils.delete(this.basedir, FileUtils.RECURSIVE | FileUtils.RETRY); } } @Test public void vanilla() { - repository.findOne("bar", "staging", "master"); - Environment environment = repository.findOne("bar", "staging", "master"); + this.repository.findOne("bar", "staging", "master"); + Environment environment = this.repository.findOne("bar", "staging", "master"); assertEquals(2, environment.getPropertySources().size()); - assertEquals(repository.getUri() + "/bar.properties", environment + assertEquals(this.repository.getUri() + "/bar.properties", environment .getPropertySources().get(0).getName()); } @Test public void nested() throws IOException { String uri = ConfigServerTestUtils.prepareLocalRepo("another-config-repo"); - repository.setUri(uri); - repository.setSearchPaths(new String[] {"sub"}); - repository.findOne("bar", "staging", "master"); - Environment environment = repository.findOne("bar", "staging", "master"); + this.repository.setUri(uri); + this.repository.setSearchPaths(new String[] {"sub"}); + this.repository.findOne("bar", "staging", "master"); + Environment environment = this.repository.findOne("bar", "staging", "master"); assertEquals(2, environment.getPropertySources().size()); - assertEquals(repository.getUri() + "/sub/application.yml", environment + assertEquals(this.repository.getUri() + "/sub/application.yml", environment .getPropertySources().get(0).getName()); } @Test public void nestedPattern() throws IOException { String uri = ConfigServerTestUtils.prepareLocalRepo("another-config-repo"); - repository.setUri(uri); - repository.setSearchPaths(new String[] {"sub*"}); - repository.findOne("bar", "staging", "master"); - Environment environment = repository.findOne("bar", "staging", "master"); + this.repository.setUri(uri); + this.repository.setSearchPaths(new String[] {"sub*"}); + this.repository.findOne("bar", "staging", "master"); + Environment environment = this.repository.findOne("bar", "staging", "master"); assertEquals(2, environment.getPropertySources().size()); - assertEquals(repository.getUri() + "/sub/application.yml", environment + assertEquals(this.repository.getUri() + "/sub/application.yml", environment .getPropertySources().get(0).getName()); } @Test public void branch() { - repository.setBasedir(basedir); - Environment environment = repository.findOne("bar", "staging", "raw"); + this.repository.setBasedir(this.basedir); + Environment environment = this.repository.findOne("bar", "staging", "raw"); assertEquals(2, environment.getPropertySources().size()); - assertEquals(repository.getUri() + "/bar.properties", environment + assertEquals(this.repository.getUri() + "/bar.properties", environment .getPropertySources().get(0).getName()); } @Test public void tag() { - repository.setBasedir(basedir); - Environment environment = repository.findOne("bar", "staging", "foo"); + this.repository.setBasedir(this.basedir); + Environment environment = this.repository.findOne("bar", "staging", "foo"); assertEquals(2, environment.getPropertySources().size()); - assertEquals(repository.getUri() + "/bar.properties", environment + assertEquals(this.repository.getUri() + "/bar.properties", environment .getPropertySources().get(0).getName()); } @Test public void basedir() { - repository.setBasedir(basedir); - repository.findOne("bar", "staging", "master"); - Environment environment = repository.findOne("bar", "staging", "master"); + this.repository.setBasedir(this.basedir); + this.repository.findOne("bar", "staging", "master"); + Environment environment = this.repository.findOne("bar", "staging", "master"); assertEquals(2, environment.getPropertySources().size()); - assertEquals(repository.getUri() + "/bar.properties", environment + assertEquals(this.repository.getUri() + "/bar.properties", environment .getPropertySources().get(0).getName()); } @Test public void basedirExists() throws Exception { - assertTrue(basedir.mkdirs()); - assertTrue(new File(basedir, ".nothing").createNewFile()); - repository.setBasedir(basedir); - repository.findOne("bar", "staging", "master"); - Environment environment = repository.findOne("bar", "staging", "master"); + assertTrue(this.basedir.mkdirs()); + assertTrue(new File(this.basedir, ".nothing").createNewFile()); + this.repository.setBasedir(this.basedir); + this.repository.findOne("bar", "staging", "master"); + Environment environment = this.repository.findOne("bar", "staging", "master"); assertEquals(2, environment.getPropertySources().size()); - assertEquals(repository.getUri() + "/bar.properties", environment + assertEquals(this.repository.getUri() + "/bar.properties", environment .getPropertySources().get(0).getName()); } - + @Test public void uriWithHostOnly() throws Exception { - repository.setUri("git://localhost"); - assertEquals("git://localhost/", repository.getUri()); + this.repository.setUri("git://localhost"); + assertEquals("git://localhost/", this.repository.getUri()); } @Test public void uriWithHostAndPath() throws Exception { - repository.setUri("git://localhost/foo/"); - assertEquals("git://localhost/foo", repository.getUri()); + this.repository.setUri("git://localhost/foo/"); + assertEquals("git://localhost/foo", this.repository.getUri()); } - + @Test - public void afterPropertiesSet_CloneOnStartTrue_CloneAndFetchCalled() + public void afterPropertiesSet_CloneOnStartTrue_CloneAndFetchCalled() throws Exception { Git mockGit = mock(Git.class); CloneCommand mockCloneCommand = mock(CloneCommand.class); - + when(mockCloneCommand.setURI(anyString())).thenReturn(mockCloneCommand); when(mockCloneCommand.setDirectory(any(File.class))).thenReturn(mockCloneCommand); JGitEnvironmentRepository envRepository = new JGitEnvironmentRepository( - environment); + this.environment); envRepository.setGitFactory(new MockGitFactory(mockGit, mockCloneCommand)); envRepository.setUri("http://somegitserver/somegitrepo"); envRepository.setCloneOnStart(true); envRepository.afterPropertiesSet(); verify(mockCloneCommand, times(1)).call(); - verify(mockGit, times(1)).fetch(); } @Test - public void afterPropertiesSet_CloneOnStartFalse_CloneAndFetchNotCalled() + public void afterPropertiesSet_CloneOnStartFalse_CloneAndFetchNotCalled() throws Exception { Git mockGit = mock(Git.class); CloneCommand mockCloneCommand = mock(CloneCommand.class); - + when(mockCloneCommand.setURI(anyString())).thenReturn(mockCloneCommand); when(mockCloneCommand.setDirectory(any(File.class))).thenReturn(mockCloneCommand); JGitEnvironmentRepository envRepository = new JGitEnvironmentRepository( - environment); + this.environment); envRepository.setGitFactory(new MockGitFactory(mockGit, mockCloneCommand)); envRepository.setUri("http://somegitserver/somegitrepo"); envRepository.afterPropertiesSet(); @@ -176,16 +179,16 @@ public class JGitEnvironmentRepositoryTests { } @Test - public void afterPropertiesSet_CloneOnStartTrueWithFileURL_CloneAndFetchNotCalled() + public void afterPropertiesSet_CloneOnStartTrueWithFileURL_CloneAndFetchNotCalled() throws Exception { Git mockGit = mock(Git.class); CloneCommand mockCloneCommand = mock(CloneCommand.class); - + when(mockCloneCommand.setURI(anyString())).thenReturn(mockCloneCommand); when(mockCloneCommand.setDirectory(any(File.class))).thenReturn(mockCloneCommand); JGitEnvironmentRepository envRepository = new JGitEnvironmentRepository( - environment); + this.environment); envRepository.setGitFactory(new MockGitFactory(mockGit, mockCloneCommand)); envRepository.setUri("file://somefilesystem/somegitrepo"); envRepository.setCloneOnStart(true); @@ -195,21 +198,23 @@ public class JGitEnvironmentRepositoryTests { } class MockGitFactory extends JGitEnvironmentRepository.JGitFactory { - + private Git mockGit; private CloneCommand mockCloneCommand; - + public MockGitFactory (Git mockGit, CloneCommand mockCloneCommand) { this.mockGit = mockGit; this.mockCloneCommand = mockCloneCommand; } - + + @Override public Git getGitByOpen(File file) throws IOException { - return mockGit; + return this.mockGit; } + @Override public CloneCommand getCloneCommandByCloneRepository() { - return mockCloneCommand; + return this.mockCloneCommand; } } }