Migrate server classes to Flogger
This is the second part of the migration to Flogger. This change migrates all classes of the 'server' module to Flogger. Classes of the 'httpd' module have been migrated by the predecessor change. Other modules continue to use slf4j. They should be migrated by follow-up changes. During this migration we try to make the log statements more consistent: - avoid string concatenation - avoid usage of String.format(...) Change-Id: I233afc8360af7fef8640394204d0d676cc0d0001 Signed-off-by: Edwin Kempin <ekempin@google.com>
This commit is contained in:
Edwin Kempin
@@ -23,6 +23,7 @@ import com.google.common.annotations.VisibleForTesting;
|
||||
import com.google.common.base.Splitter;
|
||||
import com.google.common.collect.ImmutableList;
|
||||
import com.google.common.collect.Iterables;
|
||||
import com.google.common.flogger.FluentLogger;
|
||||
import com.google.gerrit.common.FooterConstants;
|
||||
import com.google.gerrit.common.Nullable;
|
||||
import com.google.gerrit.common.PageLinks;
|
||||
@@ -77,11 +78,9 @@ import org.eclipse.jgit.revwalk.FooterLine;
|
||||
import org.eclipse.jgit.revwalk.RevCommit;
|
||||
import org.eclipse.jgit.revwalk.RevWalk;
|
||||
import org.eclipse.jgit.util.SystemReader;
|
||||
import org.slf4j.Logger;
|
||||
import org.slf4j.LoggerFactory;
|
||||
|
||||
public class CommitValidators {
|
||||
private static final Logger log = LoggerFactory.getLogger(CommitValidators.class);
|
||||
private static final FluentLogger logger = FluentLogger.forEnclosingClass();
|
||||
|
||||
public static final Pattern NEW_PATCHSET_PATTERN =
|
||||
Pattern.compile("^" + REFS_CHANGES + "(?:[0-9][0-9]/)?([1-9][0-9]*)(?:/[1-9][0-9]*)?$");
|
||||
@@ -227,7 +226,10 @@ public class CommitValidators {
|
||||
messages.addAll(commitValidator.onCommitReceived(receiveEvent));
|
||||
}
|
||||
} catch (CommitValidationException e) {
|
||||
log.debug("CommitValidationException occurred: {}", e.getFullMessage(), e);
|
||||
logger
|
||||
.atFine()
|
||||
.withCause(e)
|
||||
.log("CommitValidationException occurred: %s", e.getFullMessage());
|
||||
// Keep the old messages (and their order) in case of an exception
|
||||
messages.addAll(e.getMessages());
|
||||
throw new CommitValidationException(e.getMessage(), messages);
|
||||
@@ -443,14 +445,14 @@ public class CommitValidators {
|
||||
throw new ConfigInvalidException("invalid project configuration");
|
||||
}
|
||||
} catch (ConfigInvalidException | IOException e) {
|
||||
log.error(
|
||||
"User "
|
||||
+ user.getLoggableName()
|
||||
+ " tried to push an invalid project configuration "
|
||||
+ receiveEvent.command.getNewId().name()
|
||||
+ " for project "
|
||||
+ receiveEvent.project.getName(),
|
||||
e);
|
||||
logger
|
||||
.atSevere()
|
||||
.withCause(e)
|
||||
.log(
|
||||
"User %s tried to push an invalid project configuration %s for project %s",
|
||||
user.getLoggableName(),
|
||||
receiveEvent.command.getNewId().name(),
|
||||
receiveEvent.project.getName());
|
||||
throw new CommitValidationException("invalid project configuration", messages);
|
||||
}
|
||||
}
|
||||
@@ -479,7 +481,7 @@ public class CommitValidators {
|
||||
} catch (AuthException e) {
|
||||
throw new CommitValidationException("you are not allowed to upload merges");
|
||||
} catch (PermissionBackendException e) {
|
||||
log.error("cannot check MERGE", e);
|
||||
logger.atSevere().withCause(e).log("cannot check MERGE");
|
||||
throw new CommitValidationException("internal auth error");
|
||||
}
|
||||
}
|
||||
@@ -554,7 +556,7 @@ public class CommitValidators {
|
||||
throw new CommitValidationException(
|
||||
"not Signed-off-by author/committer/uploader in commit message footer");
|
||||
} catch (PermissionBackendException e) {
|
||||
log.error("cannot check FORGE_COMMITTER", e);
|
||||
logger.atSevere().withCause(e).log("cannot check FORGE_COMMITTER");
|
||||
throw new CommitValidationException("internal auth error");
|
||||
}
|
||||
}
|
||||
@@ -590,7 +592,7 @@ public class CommitValidators {
|
||||
"invalid author",
|
||||
invalidEmail(receiveEvent.commit, "author", author, user, canonicalWebUrl));
|
||||
} catch (PermissionBackendException e) {
|
||||
log.error("cannot check FORGE_AUTHOR", e);
|
||||
logger.atSevere().withCause(e).log("cannot check FORGE_AUTHOR");
|
||||
throw new CommitValidationException("internal auth error");
|
||||
}
|
||||
}
|
||||
@@ -624,7 +626,7 @@ public class CommitValidators {
|
||||
"invalid committer",
|
||||
invalidEmail(receiveEvent.commit, "committer", committer, user, canonicalWebUrl));
|
||||
} catch (PermissionBackendException e) {
|
||||
log.error("cannot check FORGE_COMMITTER", e);
|
||||
logger.atSevere().withCause(e).log("cannot check FORGE_COMMITTER");
|
||||
throw new CommitValidationException("internal auth error");
|
||||
}
|
||||
}
|
||||
@@ -663,7 +665,7 @@ public class CommitValidators {
|
||||
gerritIdent.getEmailAddress(),
|
||||
RefPermission.FORGE_SERVER.name()));
|
||||
} catch (PermissionBackendException e) {
|
||||
log.error("cannot check FORGE_SERVER", e);
|
||||
logger.atSevere().withCause(e).log("cannot check FORGE_SERVER");
|
||||
throw new CommitValidationException("internal auth error");
|
||||
}
|
||||
}
|
||||
@@ -690,7 +692,7 @@ public class CommitValidators {
|
||||
return Collections.emptyList();
|
||||
} catch (IOException e) {
|
||||
String m = "error checking banned commits";
|
||||
log.warn(m, e);
|
||||
logger.atWarning().withCause(e).log(m);
|
||||
throw new CommitValidationException(m, e);
|
||||
}
|
||||
}
|
||||
@@ -729,7 +731,7 @@ public class CommitValidators {
|
||||
return msgs;
|
||||
} catch (IOException | ConfigInvalidException e) {
|
||||
String m = "error validating external IDs";
|
||||
log.warn(m, e);
|
||||
logger.atWarning().withCause(e).log(m);
|
||||
throw new CommitValidationException(m, e);
|
||||
}
|
||||
}
|
||||
@@ -787,7 +789,7 @@ public class CommitValidators {
|
||||
}
|
||||
} catch (IOException e) {
|
||||
String m = String.format("Validating update for account %s failed", accountId.get());
|
||||
log.error(m, e);
|
||||
logger.atSevere().withCause(e).log(m);
|
||||
throw new CommitValidationException(m, e);
|
||||
}
|
||||
return Collections.emptyList();
|
||||
|
||||
@@ -16,6 +16,7 @@ package com.google.gerrit.server.git.validators;
|
||||
|
||||
import com.google.common.base.Joiner;
|
||||
import com.google.common.collect.ImmutableList;
|
||||
import com.google.common.flogger.FluentLogger;
|
||||
import com.google.gerrit.extensions.api.projects.ProjectConfigEntryType;
|
||||
import com.google.gerrit.extensions.registration.DynamicMap;
|
||||
import com.google.gerrit.extensions.registration.DynamicMap.Entry;
|
||||
@@ -49,11 +50,9 @@ import java.util.List;
|
||||
import org.eclipse.jgit.errors.ConfigInvalidException;
|
||||
import org.eclipse.jgit.lib.Repository;
|
||||
import org.eclipse.jgit.revwalk.RevWalk;
|
||||
import org.slf4j.Logger;
|
||||
import org.slf4j.LoggerFactory;
|
||||
|
||||
public class MergeValidators {
|
||||
private static final Logger log = LoggerFactory.getLogger(MergeValidators.class);
|
||||
private static final FluentLogger logger = FluentLogger.forEnclosingClass();
|
||||
|
||||
private final DynamicSet<MergeValidationListener> mergeValidationListeners;
|
||||
private final ProjectConfigValidator.Factory projectConfigValidatorFactory;
|
||||
@@ -168,7 +167,7 @@ public class MergeValidators {
|
||||
} catch (AuthException e) {
|
||||
throw new MergeValidationException(SET_BY_ADMIN);
|
||||
} catch (PermissionBackendException e) {
|
||||
log.warn("Cannot check ADMINISTRATE_SERVER", e);
|
||||
logger.atWarning().withCause(e).log("Cannot check ADMINISTRATE_SERVER");
|
||||
throw new MergeValidationException("validation unavailable");
|
||||
}
|
||||
if (allUsersName.equals(destProject.getNameKey())
|
||||
@@ -280,7 +279,7 @@ public class MergeValidators {
|
||||
return;
|
||||
}
|
||||
} catch (IOException | OrmException e) {
|
||||
log.error("Cannot validate account update", e);
|
||||
logger.atSevere().withCause(e).log("Cannot validate account update");
|
||||
throw new MergeValidationException("account validation unavailable");
|
||||
}
|
||||
|
||||
@@ -291,7 +290,7 @@ public class MergeValidators {
|
||||
"invalid account configuration: " + Joiner.on("; ").join(errorMessages));
|
||||
}
|
||||
} catch (IOException e) {
|
||||
log.error("Cannot validate account update", e);
|
||||
logger.atSevere().withCause(e).log("Cannot validate account update");
|
||||
throw new MergeValidationException("account validation unavailable");
|
||||
}
|
||||
}
|
||||
|
||||
@@ -16,6 +16,7 @@ package com.google.gerrit.server.git.validators;
|
||||
import com.google.common.base.Predicate;
|
||||
import com.google.common.collect.ImmutableList;
|
||||
import com.google.common.collect.Iterables;
|
||||
import com.google.common.flogger.FluentLogger;
|
||||
import com.google.gerrit.extensions.registration.DynamicSet;
|
||||
import com.google.gerrit.extensions.restapi.AuthException;
|
||||
import com.google.gerrit.reviewdb.client.Account;
|
||||
@@ -34,12 +35,11 @@ import java.util.ArrayList;
|
||||
import java.util.List;
|
||||
import org.eclipse.jgit.lib.RefUpdate;
|
||||
import org.eclipse.jgit.transport.ReceiveCommand;
|
||||
import org.slf4j.Logger;
|
||||
import org.slf4j.LoggerFactory;
|
||||
|
||||
public class RefOperationValidators {
|
||||
private static final FluentLogger logger = FluentLogger.forEnclosingClass();
|
||||
|
||||
private static final GetErrorMessages GET_ERRORS = new GetErrorMessages();
|
||||
private static final Logger LOG = LoggerFactory.getLogger(RefOperationValidators.class);
|
||||
|
||||
public interface Factory {
|
||||
RefOperationValidators create(Project project, IdentifiedUser user, ReceiveCommand cmd);
|
||||
@@ -101,7 +101,7 @@ public class RefOperationValidators {
|
||||
String.format(
|
||||
"Ref \"%s\" %S in project %s validation failed",
|
||||
event.command.getRefName(), event.command.getType(), event.project.getName());
|
||||
LOG.error(header);
|
||||
logger.atSevere().log(header);
|
||||
throw new RefOperationValidationException(header, errors);
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user