From 648f43c22d002ef9b6757b3d72caa59e6baa492e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sascha=20Kr=C3=BCger?= Date: Fri, 6 May 2016 16:42:54 +0200 Subject: [PATCH] Improve logging of JGit exceptions In some situations the JGit library can fail to update the local Git repository. While the config server should gracefully handle these situations some added information from the root cause might be helpful to pinpoint why changes in the remote repository do not get reflected in the REST API. fixes gh-364 --- .../JGitEnvironmentRepository.java | 23 ++++++++++++++----- .../MultipleJGitEnvironmentRepository.java | 12 ++++++++-- .../JGitEnvironmentRepositoryTests.java | 15 ++++++++++++ 3 files changed, 42 insertions(+), 8 deletions(-) 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 5ba854ed..890f5347 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 @@ -225,13 +225,20 @@ public class JGitEnvironmentRepository extends AbstractScmEnvironmentRepository /* for testing */ boolean shouldPull(Git git, Ref ref) throws GitAPIException { boolean shouldPull; Status gitStatus = git.status().call(); - if (this.isForcePull() && !gitStatus.isClean()) { + boolean isWorkingTreeClean = gitStatus.isClean(); + String originUrl = git.getRepository().getConfig().getString("remote", "origin", + "url"); + + if (this.forcePull && !isWorkingTreeClean) { shouldPull = true; logDirty(gitStatus); } else { - shouldPull = gitStatus.isClean() && ref != null && git.getRepository() - .getConfig().getString("remote", "origin", "url") != null; + shouldPull = isWorkingTreeClean && ref != null && originUrl != null; + } + if (!isWorkingTreeClean && !this.forcePull) { + this.logger.info("Cannot pull from remote " + originUrl + + ", the working tree is not clean."); } return shouldPull; } @@ -270,9 +277,13 @@ public class JGitEnvironmentRepository extends AbstractScmEnvironmentRepository pull.call(); } catch (Exception e) { - this.logger.warn("Could not pull remote for " + label + " (current ref=" + ref - + "), remote: " + git.getRepository().getConfig().getString("remote", - "origin", "url")); + this.logger + .warn("Could not pull remote for " + label + " (current ref=" + ref + + "), remote: " + + git.getRepository().getConfig().getString("remote", + "origin", "url") + + ", cause: (" + e.getClass().getSimpleName() + ") " + + e.getMessage()); } } diff --git a/spring-cloud-config-server/src/main/java/org/springframework/cloud/config/server/environment/MultipleJGitEnvironmentRepository.java b/spring-cloud-config-server/src/main/java/org/springframework/cloud/config/server/environment/MultipleJGitEnvironmentRepository.java index d5dc30ee..1d278f3f 100644 --- a/spring-cloud-config-server/src/main/java/org/springframework/cloud/config/server/environment/MultipleJGitEnvironmentRepository.java +++ b/spring-cloud-config-server/src/main/java/org/springframework/cloud/config/server/environment/MultipleJGitEnvironmentRepository.java @@ -100,6 +100,11 @@ public class MultipleJGitEnvironmentRepository extends JGitEnvironmentRepository } } catch (Exception e) { + if (logger.isDebugEnabled()) { + this.logger.debug("Cannot retrieve resource locations from " + + candidate.getUri() + ", cause: (" + + e.getClass().getSimpleName() + ") " + e.getMessage()); + } continue; } } @@ -130,8 +135,11 @@ public class MultipleJGitEnvironmentRepository extends JGitEnvironmentRepository } } catch (Exception e) { - this.logger.info( - "Cannot load configuration from " + candidate.getUri()); + if (logger.isDebugEnabled()) { + this.logger.debug("Cannot load configuration from " + + candidate.getUri() + ", cause: (" + + e.getClass().getSimpleName() + ") " + e.getMessage()); + } continue; } } 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 966cfd32..197439aa 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 @@ -239,8 +239,13 @@ public class JGitEnvironmentRepositoryTests { Git git = mock(Git.class); StatusCommand statusCommand = mock(StatusCommand.class); Status status = mock(Status.class); + Repository repository = mock(Repository.class); + StoredConfig storedConfig = mock(StoredConfig.class); when(git.status()).thenReturn(statusCommand); + when(git.getRepository()).thenReturn(repository); + when(repository.getConfig()).thenReturn(storedConfig); + when(storedConfig.getString("remote", "origin", "url")).thenReturn("http://example/git"); when(statusCommand.call()).thenReturn(status); when(status.isClean()).thenReturn(false); @@ -258,8 +263,13 @@ public class JGitEnvironmentRepositoryTests { Git git = mock(Git.class); StatusCommand statusCommand = mock(StatusCommand.class); Status status = mock(Status.class); + Repository repository = mock(Repository.class); + StoredConfig storedConfig = mock(StoredConfig.class); when(git.status()).thenReturn(statusCommand); + when(git.getRepository()).thenReturn(repository); + when(repository.getConfig()).thenReturn(storedConfig); + when(storedConfig.getString("remote", "origin", "url")).thenReturn("http://example/git"); when(statusCommand.call()).thenReturn(status); when(status.isClean()).thenReturn(true); @@ -278,8 +288,13 @@ public class JGitEnvironmentRepositoryTests { StatusCommand statusCommand = mock(StatusCommand.class); Status status = mock(Status.class); Ref ref = mock(Ref.class); + Repository repository = mock(Repository.class); + StoredConfig storedConfig = mock(StoredConfig.class); when(git.status()).thenReturn(statusCommand); + when(git.getRepository()).thenReturn(repository); + when(repository.getConfig()).thenReturn(storedConfig); + when(storedConfig.getString("remote", "origin", "url")).thenReturn("http://example/git"); when(statusCommand.call()).thenReturn(status); when(status.isClean()).thenReturn(false);