From 9b51c34d7bf3de7d54afc7aecb873428f9a48986 Mon Sep 17 00:00:00 2001 From: Patrick Coray Date: Thu, 3 Nov 2016 00:47:40 +0100 Subject: [PATCH 1/3] Delete basedir when clone fails --- .../server/environment/JGitEnvironmentRepository.java | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/spring-cloud-config-server/src/main/java/org/springframework/cloud/config/server/environment/JGitEnvironmentRepository.java b/spring-cloud-config-server/src/main/java/org/springframework/cloud/config/server/environment/JGitEnvironmentRepository.java index fefdc8b1..015615c5 100644 --- a/spring-cloud-config-server/src/main/java/org/springframework/cloud/config/server/environment/JGitEnvironmentRepository.java +++ b/spring-cloud-config-server/src/main/java/org/springframework/cloud/config/server/environment/JGitEnvironmentRepository.java @@ -377,7 +377,13 @@ public class JGitEnvironmentRepository extends AbstractScmEnvironmentRepository if (hasText(getUsername())) { setCredentialsProvider(clone); } - return clone.call(); + try { + return clone.call(); + } + catch (GitAPIException e) { + deleteBaseDirIfExists(); + throw e; + } } private void deleteBaseDirIfExists() { From 8e98a309d75ab51ca76128bb796d774781962d87 Mon Sep 17 00:00:00 2001 From: Patrick Coray Date: Thu, 3 Nov 2016 01:18:48 +0100 Subject: [PATCH 2/3] Add test to check if baseDir is deleted when clone fails --- .../JGitEnvironmentRepositoryTests.java | 60 +++++++++++++------ 1 file changed, 43 insertions(+), 17 deletions(-) diff --git a/spring-cloud-config-server/src/test/java/org/springframework/cloud/config/server/environment/JGitEnvironmentRepositoryTests.java b/spring-cloud-config-server/src/test/java/org/springframework/cloud/config/server/environment/JGitEnvironmentRepositoryTests.java index 197439aa..42608e96 100644 --- a/spring-cloud-config-server/src/test/java/org/springframework/cloud/config/server/environment/JGitEnvironmentRepositoryTests.java +++ b/spring-cloud-config-server/src/test/java/org/springframework/cloud/config/server/environment/JGitEnvironmentRepositoryTests.java @@ -16,25 +16,9 @@ package org.springframework.cloud.config.server.environment; -import java.io.File; -import java.io.IOException; - -import org.eclipse.jgit.api.CloneCommand; -import org.eclipse.jgit.api.Git; -import org.eclipse.jgit.api.Status; -import org.eclipse.jgit.api.StatusCommand; -import org.eclipse.jgit.lib.Ref; -import org.eclipse.jgit.lib.Repository; -import org.eclipse.jgit.lib.StoredConfig; -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.test.ConfigServerTestUtils; -import org.springframework.core.env.StandardEnvironment; - import static org.hamcrest.Matchers.is; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertThat; @@ -46,6 +30,24 @@ 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; + +import org.eclipse.jgit.api.CloneCommand; +import org.eclipse.jgit.api.Git; +import org.eclipse.jgit.api.Status; +import org.eclipse.jgit.api.StatusCommand; +import org.eclipse.jgit.api.errors.TransportException; +import org.eclipse.jgit.lib.Ref; +import org.eclipse.jgit.lib.Repository; +import org.eclipse.jgit.lib.StoredConfig; +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.test.ConfigServerTestUtils; +import org.springframework.core.env.StandardEnvironment; + /** * @author Dave Syer * @@ -329,6 +331,30 @@ public class JGitEnvironmentRepositoryTests { assertThat("shouldPull was false", shouldPull, is(true)); } + + @Test + public void shouldDeleteBaseDirWhenCloneFails() 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); + when(mockCloneCommand.call()).thenThrow(new TransportException("failed to clone")); + + JGitEnvironmentRepository envRepository = new JGitEnvironmentRepository( + this.environment); + envRepository.setGitFactory(new MockGitFactory(mockGit, mockCloneCommand)); + envRepository.setUri("http://somegitserver/somegitrepo"); + envRepository.setBasedir(this.basedir); + + try { + envRepository.findOne("bar", "staging", "master"); + } catch (Exception ex) { + // expected + } + assertFalse("baseDir should be deleted when clone fails", this.basedir.exists()); + + } class MockGitFactory extends JGitEnvironmentRepository.JGitFactory { From 0cd7bf991c37908ef0dfbff1596eca4719eaae54 Mon Sep 17 00:00:00 2001 From: Patrick Coray Date: Thu, 3 Nov 2016 01:29:01 +0100 Subject: [PATCH 3/3] Keep formatting of file --- .../JGitEnvironmentRepositoryTests.java | 35 ++++++++++--------- 1 file changed, 18 insertions(+), 17 deletions(-) diff --git a/spring-cloud-config-server/src/test/java/org/springframework/cloud/config/server/environment/JGitEnvironmentRepositoryTests.java b/spring-cloud-config-server/src/test/java/org/springframework/cloud/config/server/environment/JGitEnvironmentRepositoryTests.java index 42608e96..49f5a436 100644 --- a/spring-cloud-config-server/src/test/java/org/springframework/cloud/config/server/environment/JGitEnvironmentRepositoryTests.java +++ b/spring-cloud-config-server/src/test/java/org/springframework/cloud/config/server/environment/JGitEnvironmentRepositoryTests.java @@ -16,20 +16,6 @@ package org.springframework.cloud.config.server.environment; -import static org.hamcrest.Matchers.is; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertNotNull; -import static org.junit.Assert.assertNull; -import static org.junit.Assert.assertThat; -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; @@ -48,6 +34,20 @@ import org.springframework.cloud.config.environment.Environment; import org.springframework.cloud.config.server.test.ConfigServerTestUtils; import org.springframework.core.env.StandardEnvironment; +import static org.hamcrest.Matchers.is; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertThat; +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; + /** * @author Dave Syer * @@ -349,11 +349,12 @@ public class JGitEnvironmentRepositoryTests { try { envRepository.findOne("bar", "staging", "master"); - } catch (Exception ex) { - // expected + } + catch (Exception ex) { + // expected - ignore } - assertFalse("baseDir should be deleted when clone fails", this.basedir.exists()); + assertFalse("baseDir should be deleted when clone fails", this.basedir.exists()); } class MockGitFactory extends JGitEnvironmentRepository.JGitFactory {