Merge pull request #626 from vasilievip/fixErrorLogging

* fixErrorLogging:
  Add stacktrace logging
This commit is contained in:
Spencer Gibb
2017-03-31 14:17:50 -06:00
9 changed files with 95 additions and 34 deletions

View File

@@ -77,8 +77,13 @@ public class CipherEnvironmentEncryptor implements EnvironmentEncryptor {
catch (Exception e) {
value = "<n/a>";
name = "invalid." + name;
logger.warn("Cannot decrypt key: " + key + " (" + e.getClass()
+ ": " + e.getMessage() + ")");
String message = "Cannot decrypt key: " + key + " (" + e.getClass()
+ ": " + e.getMessage() + ")";
if (logger.isDebugEnabled()) {
logger.debug(message, e);
} else if (logger.isWarnEnabled()) {
logger.warn(message);
}
}
map.put(name, value);
}

View File

@@ -125,19 +125,14 @@ public class EncryptionController {
public String encrypt(@PathVariable String name, @PathVariable String profiles,
@RequestBody String data, @RequestHeader("Content-Type") MediaType type) {
checkEncryptorInstalled(name, profiles);
try {
String input = stripFormData(data, type, false);
Map<String, String> keys = this.helper.getEncryptorKeys(name, profiles,
input);
String textToEncrypt = this.helper.stripPrefix(input);
String encrypted = this.helper.addPrefix(keys,
this.encryptor.locate(keys).encrypt(textToEncrypt));
logger.info("Encrypted data");
return encrypted;
}
catch (IllegalArgumentException e) {
throw new InvalidCipherException();
}
String input = stripFormData(data, type, false);
Map<String, String> keys = this.helper.getEncryptorKeys(name, profiles,
input);
String textToEncrypt = this.helper.stripPrefix(input);
String encrypted = this.helper.addPrefix(keys,
this.encryptor.locate(keys).encrypt(textToEncrypt));
logger.info("Encrypted data");
return encrypted;
}
@RequestMapping(value = "decrypt", method = RequestMethod.POST)
@@ -161,7 +156,8 @@ public class EncryptionController {
logger.info("Decrypted cipher data");
return decrypted;
}
catch (IllegalArgumentException e) {
catch (IllegalArgumentException|IllegalStateException e) {
logger.error("Cannot decrypt key:" + name + ", value:" + data, e);
throw new InvalidCipherException();
}
}
@@ -241,4 +237,4 @@ class KeyNotAvailableException extends RuntimeException {
@SuppressWarnings("serial")
class InvalidCipherException extends RuntimeException {
}
}

View File

@@ -197,7 +197,7 @@ public class JGitEnvironmentRepository extends AbstractScmEnvironmentRepository
return git.getRepository().getRef("HEAD").getObjectId().getName();
}
catch (RefNotFoundException e) {
throw new NoSuchLabelException("No such label: " + label);
throw new NoSuchLabelException("No such label: " + label, e);
}
catch (GitAPIException e) {
throw new IllegalStateException("Cannot clone or checkout repository", e);
@@ -302,14 +302,15 @@ public class JGitEnvironmentRepository extends AbstractScmEnvironmentRepository
setCredentialsProvider(fetch);
FetchResult result = fetch.call();
if(result.getTrackingRefUpdates() != null && result.getTrackingRefUpdates().size() > 0) {
this.logger.info("Fetched for remote " + label + " and found " + result.getTrackingRefUpdates().size()
logger.info("Fetched for remote " + label + " and found " + result.getTrackingRefUpdates().size()
+ " updates");
}
return result;
}
catch (Exception ex) {
this.logger.warn("Could not fetch remote for " + label + " remote: " + git
.getRepository().getConfig().getString("remote", "origin", "url"));
String message = "Could not fetch remote for " + label + " remote: " + git
.getRepository().getConfig().getString("remote", "origin", "url");
warn(message, ex);
return null;
}
}
@@ -325,8 +326,9 @@ public class JGitEnvironmentRepository extends AbstractScmEnvironmentRepository
return result;
}
catch (Exception ex) {
this.logger.warn("Could not merge remote for " + label + " remote: " + git
.getRepository().getConfig().getString("remote", "origin", "url"));
String message = "Could not merge remote for " + label + " remote: " + git
.getRepository().getConfig().getString("remote", "origin", "url");
warn(message, ex);
return null;
}
}
@@ -343,9 +345,10 @@ public class JGitEnvironmentRepository extends AbstractScmEnvironmentRepository
return resetRef;
}
catch (Exception ex) {
this.logger.warn("Could not reset to remote for " + label + " (current ref="
String message = "Could not reset to remote for " + label + " (current ref="
+ ref + "), remote: " + git.getRepository().getConfig()
.getString("remote", "origin", "url"));
.getString("remote", "origin", "url");
warn(message, ex);
return null;
}
}
@@ -449,10 +452,9 @@ public class JGitEnvironmentRepository extends AbstractScmEnvironmentRepository
return status.call().isClean();
}
catch (Exception e) {
this.logger
.warn("Could not execute status command on local repository. Cause: ("
+ e.getClass().getSimpleName() + ") " + e.getMessage());
String message = "Could not execute status command on local repository. Cause: ("
+ e.getClass().getSimpleName() + ") " + e.getMessage();
warn(message, e);
return false;
}
}
@@ -486,6 +488,13 @@ public class JGitEnvironmentRepository extends AbstractScmEnvironmentRepository
return false;
}
protected void warn(String message, Exception ex) {
logger.warn(message);
if (logger.isDebugEnabled()) {
logger.debug("Stacktrace for: " + message, ex);
}
}
/**
* Wraps the static method calls to {@link org.eclipse.jgit.api.Git} and
* {@link org.eclipse.jgit.api.CloneCommand} allowing for easier unit testing.

View File

@@ -119,7 +119,7 @@ public class MultipleJGitEnvironmentRepository extends JGitEnvironmentRepository
if (logger.isDebugEnabled()) {
this.logger.debug("Cannot retrieve resource locations from "
+ candidate.getUri() + ", cause: ("
+ e.getClass().getSimpleName() + ") " + e.getMessage());
+ e.getClass().getSimpleName() + ") " + e.getMessage(), e);
}
continue;
}
@@ -154,7 +154,7 @@ public class MultipleJGitEnvironmentRepository extends JGitEnvironmentRepository
if (logger.isDebugEnabled()) {
this.logger.debug("Cannot load configuration from "
+ candidate.getUri() + ", cause: ("
+ e.getClass().getSimpleName() + ") " + e.getMessage());
+ e.getClass().getSimpleName() + ") " + e.getMessage(), e);
}
continue;
}

View File

@@ -27,4 +27,8 @@ public class NoSuchLabelException extends RepositoryException {
super(string);
}
public NoSuchLabelException(String string, Exception e) {
super(string, e);
}
}

View File

@@ -27,4 +27,8 @@ public class RepositoryException extends RuntimeException {
super(string);
}
public RepositoryException(String message, Throwable cause) {
super(message, cause);
}
}

View File

@@ -146,9 +146,13 @@ public class SvnKitEnvironmentRepository extends AbstractScmEnvironmentRepositor
return version.toString();
}
catch (Exception e) {
this.logger.warn("Could not update remote for " + label + " (current local="
+ getWorkingDirectory().getPath() + "), remote: " + this.getUri()
+ ")");
String message = "Could not update remote for " + label + " (current local="
+ getWorkingDirectory().getPath() + "), remote: " + this.getUri() + ")";
if (logger.isDebugEnabled()) {
logger.debug(message, e);
} else if (logger.isWarnEnabled()) {
logger.warn(message);
}
}
final SVNStatus status = SVNClientManager.newInstance().getStatusClient()

View File

@@ -55,6 +55,21 @@ public class EncryptionControllerTests {
this.controller.decrypt("foo", MediaType.TEXT_PLAIN);
}
@Test(expected = InvalidCipherException.class)
public void shouldThrowExceptionOnDecryptInvalidData() {
this.controller = new EncryptionController(
new SingleTextEncryptorLocator(new RsaSecretEncryptor()));
controller.decrypt("foo", MediaType.TEXT_PLAIN);
}
@Test(expected = InvalidCipherException.class)
public void shouldThrowExceptionOnDecryptWrongKey() {
RsaSecretEncryptor encryptor = new RsaSecretEncryptor();
this.controller = new EncryptionController(
new SingleTextEncryptorLocator(new RsaSecretEncryptor()));
controller.decrypt(encryptor.encrypt("foo"), MediaType.TEXT_PLAIN);
}
@Test
public void sunnyDayRsaKey() {
this.controller = new EncryptionController(

View File

@@ -23,6 +23,7 @@ import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
import org.apache.commons.logging.Log;
import org.eclipse.jgit.api.CheckoutCommand;
import org.eclipse.jgit.api.CloneCommand;
import org.eclipse.jgit.api.FetchCommand;
@@ -72,7 +73,9 @@ import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertThat;
import static org.mockito.Matchers.any;
import static org.mockito.Matchers.anyString;
import static org.mockito.Matchers.eq;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.mockingDetails;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
@@ -703,6 +706,27 @@ public class JGitEnvironmentRepositoryTests {
}
}
@Test
public void shouldPrintStacktraceIfDebugEnabled() throws Exception {
final Log mockLogger = mock(Log.class);
JGitEnvironmentRepository envRepository = new JGitEnvironmentRepository(this.environment){
@Override
public void afterPropertiesSet() throws Exception {
this.logger = mockLogger;
}
};
envRepository.afterPropertiesSet();
when(mockLogger.isDebugEnabled()).thenReturn(true);
envRepository.warn("", new RuntimeException());
verify(mockLogger).warn(eq(""));
verify(mockLogger).debug(eq("Stacktrace for: "), any(RuntimeException.class));
int numberOfInvocations = mockingDetails(mockLogger).getInvocations().size();
assertEquals("should call isDebugEnabled warn and debug", 3, numberOfInvocations);
}
class MockCloneCommand extends CloneCommand {
private Git mockGit;