Copyright plugin source.
Touched all the source for the copyright plugin for gerrit team review.
Omitted .../testdata/archives/ which contains binary archives
containing files from .../testdata/licenses/
Change-Id: Ifdc8aae584598ad4ce1f8c8372468b51fb593a4d
diff --git a/src/main/java/com/googlesource/gerrit/plugins/copyright/CheckConfig.java b/src/main/java/com/googlesource/gerrit/plugins/copyright/CheckConfig.java
index d9dd534..14c4e7f 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/copyright/CheckConfig.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/copyright/CheckConfig.java
@@ -4,7 +4,7 @@
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
-// http://www.apache.org/licenses/LICENSE-2.0
+// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
@@ -18,6 +18,7 @@
import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Preconditions;
+import com.google.common.base.Stopwatch;
import com.google.common.base.Strings;
import com.google.common.collect.ImmutableList;
import com.google.common.flogger.FluentLogger;
@@ -37,6 +38,7 @@
import java.nio.file.Paths;
import java.util.Arrays;
import java.util.Objects;
+import java.util.concurrent.TimeUnit;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
import org.eclipse.jgit.errors.ConfigInvalidException;
@@ -57,10 +59,11 @@
private static final String LABEL = "label";
private static final String PLUGIN = "plugin";
private static final int BUFFER_SIZE = 2048;
- private static final char[] BUFFER = new char[BUFFER_SIZE];
private String pluginName;
+ /** All-Projects project.config contents. */
private Config configProject;
+ /** Plugin config from All-Projects project.config file. */
ScannerConfig scannerConfig;
public CheckConfig(String pluginName, String projectConfigContents)
@@ -82,27 +85,6 @@
this.scannerConfig.readConfigFile(pluginConfig);
}
- @Override
- public boolean equals(Object other) {
- if (this == other) {
- return true;
- }
- if (other == null) {
- return false;
- }
- if (other instanceof CheckConfig) {
- CheckConfig that = (CheckConfig) other;
- return Objects.equals(this.pluginName, that.pluginName)
- && Objects.equals(this.scannerConfig, that.scannerConfig);
- }
- return false;
- }
-
- @Override
- public int hashCode() {
- return Objects.hash(pluginName, scannerConfig);
- }
-
/**
* Validates the final state of {@code trialConfig}.
*
@@ -111,6 +93,9 @@
*/
public static void checkProjectConfig(
CopyrightReviewApi reviewApi, boolean pluginEnabled, CheckConfig trialConfig) {
+ // Warn without blocking project.config pushes when plugin disabled across entire server.
+ ValidationMessage.Type errorWhenActive =
+ pluginEnabled ? ValidationMessage.Type.ERROR : ValidationMessage.Type.WARNING;
CurrentUser fromUser =
reviewApi == null
? null
@@ -127,7 +112,7 @@
? "the plugin"
: fromUser.getLoggableName())
+ " will vote on"),
- pluginEnabled ? ValidationMessage.Type.ERROR : ValidationMessage.Type.WARNING));
+ errorWhenActive));
} else {
String labelName = trialConfig.scannerConfig.reviewLabel.trim();
if (!trialConfig.configProject.getSubsections(LABEL).contains(labelName)) {
@@ -137,8 +122,10 @@
ScannerConfig.KEY_REVIEW_LABEL,
labelName,
"no [" + LABEL + " \"" + labelName + "\"] section configured."),
- pluginEnabled ? ValidationMessage.Type.ERROR : ValidationMessage.Type.WARNING));
+ errorWhenActive));
}
+
+ // Enforce at least 1 approver exists for the copyright review label for content changes.
String[] voters =
trialConfig.configProject.getStringList(
ACCESS, RefNames.REFS_HEADS + "*", "label-" + labelName);
@@ -160,8 +147,10 @@
+ " on "
+ RefNames.REFS_HEADS
+ "*"),
- pluginEnabled ? ValidationMessage.Type.ERROR : ValidationMessage.Type.WARNING));
+ errorWhenActive));
}
+
+ // Enforce an approver exists for the copyright review label for configuration changes.
voters =
trialConfig.configProject.getStringList(
ACCESS, RefNames.REFS_CONFIG, "label-" + labelName);
@@ -179,7 +168,7 @@
ScannerConfig.KEY_REVIEW_LABEL,
labelName,
"no configured approvers for " + labelName + " on " + RefNames.REFS_CONFIG),
- pluginEnabled ? ValidationMessage.Type.ERROR : ValidationMessage.Type.WARNING));
+ errorWhenActive));
}
}
if (trialConfig.scannerConfig.reviewers.isEmpty()) {
@@ -199,7 +188,7 @@
+ " non-interactive user with full voting permissions for the review label '"
+ trialConfig.scannerConfig.reviewLabel
+ "'"),
- pluginEnabled ? ValidationMessage.Type.ERROR : ValidationMessage.Type.WARNING));
+ errorWhenActive));
// TODO: inject ReviewerAdder into reviewApi, and use ReviewerAdder.prepare
// a la
// https://gerrit.googlesource.com/gerrit/+/refs/heads/master/java/com/google/gerrit/server/restapi/change/PostReview.java#265
@@ -252,7 +241,7 @@
* the output on success into the commit message.
*
* <p>This method scans the commit message to find the copied text. If the text was created for
- * the same pattern signagure, this method returns a single valid finding with the number of
+ * the same pattern signature, this method returns a single valid finding with the number of
* microseconds it took to scan a large file, which can be used to block patterns that cause
* excessive backtracking.
*
@@ -289,7 +278,6 @@
new CopyrightReviewApi.CommitMessageFinding(
commitMessage, m.group(), m.group(1), m.start(), m.end()));
}
- logger.atSevere().log("signature for %s is %s", m.group(1), signature);
builder.add(
new CopyrightReviewApi.CommitMessageFinding(
commitMessage, m.group(), m.start(), m.end()));
@@ -329,15 +317,14 @@
/** Checks whether {@code trialConfig} might cause excessive backtracking. */
private long timeLargeFileInMicros() throws IOException {
- long startNanos = System.nanoTime();
+ Stopwatch sw = Stopwatch.createStarted();
try {
IndexedLineReader file = largeFile();
scannerConfig.scanner.findMatches("file", -1, file);
} finally {
- long elapsedNanos = System.nanoTime() - startNanos;
- long elapsedMicros = elapsedNanos / 1000;
- logger.atFine().log("timeLargeFile %dms", elapsedMicros / 1000);
- return elapsedMicros;
+ sw.stop();
+ logger.atFine().log("timeLargeFile %dms", sw.elapsed(TimeUnit.MILLISECONDS));
+ return sw.elapsed(TimeUnit.MICROSECONDS);
}
}
@@ -436,7 +423,7 @@
/** Read the contents of a project.config file from {@code ilr}. */
private static String readProjectConfigFile(IndexedLineReader ilr) throws IOException {
StringBuilder sb = new StringBuilder();
- CharBuffer cb = CharBuffer.wrap(BUFFER);
+ CharBuffer cb = CharBuffer.wrap(new char[BUFFER_SIZE]);
while (ilr.read(cb) >= 0) {
cb.flip();
sb.append(cb);
@@ -459,6 +446,7 @@
return Hashing.farmHashFingerprint64().hashBytes(sb.toString().getBytes(UTF_8)).toString();
}
+ // TODO: move check from command-line tool to background thread with timeout.
/** Entry point for command-line tool to check for excessive backtracking. */
public static void main(String[] args) {
if (args.length != 2) {
diff --git a/src/main/java/com/googlesource/gerrit/plugins/copyright/CopyrightConfig.java b/src/main/java/com/googlesource/gerrit/plugins/copyright/CopyrightConfig.java
index 5c30033..af45703 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/copyright/CopyrightConfig.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/copyright/CopyrightConfig.java
@@ -4,7 +4,7 @@
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
-// http://www.apache.org/licenses/LICENSE-2.0
+// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
@@ -16,20 +16,18 @@
import static java.nio.charset.StandardCharsets.UTF_8;
-import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Strings;
import com.google.common.collect.ImmutableList;
import com.google.common.flogger.FluentLogger;
+import com.google.gerrit.common.Nullable;
import com.google.gerrit.extensions.annotations.PluginName;
import com.google.gerrit.extensions.api.changes.AddReviewerResult;
import com.google.gerrit.extensions.api.changes.ReviewResult;
import com.google.gerrit.extensions.events.GitReferenceUpdatedListener;
+import com.google.gerrit.extensions.events.LifecycleListener;
import com.google.gerrit.extensions.events.RevisionCreatedListener;
import com.google.gerrit.extensions.registration.DynamicSet;
import com.google.gerrit.extensions.restapi.RestApiException;
-import com.google.gerrit.metrics.Description;
-import com.google.gerrit.metrics.Description.Units;
-import com.google.gerrit.metrics.MetricMaker;
import com.google.gerrit.metrics.Timer0;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.reviewdb.client.RefNames;
@@ -51,7 +49,6 @@
import java.util.List;
import java.util.Map;
import java.util.Objects;
-import java.util.concurrent.TimeUnit;
import javax.inject.Inject;
import org.eclipse.jgit.errors.ConfigInvalidException;
import org.eclipse.jgit.lib.Constants;
@@ -65,9 +62,13 @@
/** Listener to manage configuration for enforcing review of copyright declarations and licenses. */
@Singleton
class CopyrightConfig
- implements CommitValidationListener, RevisionCreatedListener, GitReferenceUpdatedListener {
+ implements CommitValidationListener,
+ RevisionCreatedListener,
+ GitReferenceUpdatedListener,
+ LifecycleListener {
private static final FluentLogger logger = FluentLogger.forEnclosingClass();
+ /** Default value of timeTestMax configuration parameter for avoiding excessive backtracking. */
private final long DEFAULT_MAX_ELAPSED_SECONDS = 8;
private final Metrics metrics;
@@ -78,49 +79,21 @@
private final PluginConfigFactory pluginConfigFactory;
private final CopyrightReviewApi reviewApi;
- private PluginConfig gerritConfig;
- private CheckConfig checkConfig;
+ @Nullable private PluginConfig gerritConfig;
+ @Nullable private CheckConfig checkConfig;
static AbstractModule module() {
return new AbstractModule() {
@Override
protected void configure() {
DynamicSet.bind(binder(), CommitValidationListener.class).to(CopyrightConfig.class);
+ DynamicSet.bind(binder(), LifecycleListener.class).to(CopyrightConfig.class);
DynamicSet.bind(binder(), RevisionCreatedListener.class).to(CopyrightConfig.class);
DynamicSet.bind(binder(), GitReferenceUpdatedListener.class).to(CopyrightConfig.class);
}
};
}
- @Singleton
- private static class Metrics {
- final Timer0 readConfigTimer;
- final Timer0 checkConfigTimer;
- final Timer0 testConfigTimer;
-
- @Inject
- Metrics(MetricMaker metricMaker) {
- readConfigTimer =
- metricMaker.newTimer(
- "read_config_latency",
- new Description("Time spent reading and parsing plugin configurations")
- .setCumulative()
- .setUnit(Units.MICROSECONDS));
- checkConfigTimer =
- metricMaker.newTimer(
- "check_config_latency",
- new Description("Time spent testing proposed plugin configurations")
- .setCumulative()
- .setUnit(Units.MICROSECONDS));
- testConfigTimer =
- metricMaker.newTimer(
- "test_config_latency",
- new Description("Time spent testing configurations against difficult file pattern")
- .setCumulative()
- .setUnit(Units.MICROSECONDS));
- }
- }
-
@Inject
CopyrightConfig(
Metrics metrics,
@@ -129,8 +102,7 @@
GitRepositoryManager repoManager,
ProjectCache projectCache,
PluginConfigFactory pluginConfigFactory,
- CopyrightReviewApi reviewApi)
- throws IOException, ConfigInvalidException {
+ CopyrightReviewApi reviewApi) {
this.metrics = metrics;
this.allProjectsName = allProjectsName;
this.pluginName = pluginName;
@@ -138,40 +110,28 @@
this.projectCache = projectCache;
this.pluginConfigFactory = pluginConfigFactory;
this.reviewApi = reviewApi;
-
- long nanoStart = System.nanoTime();
- try {
- checkConfig = readConfig(projectCache.getAllProjects().getProject().getConfigRefState());
- } finally {
- long elapsedMicros = (System.nanoTime() - nanoStart) / 1000;
- metrics.readConfigTimer.record(elapsedMicros, TimeUnit.MICROSECONDS);
- }
- }
-
- private CopyrightConfig(
- MetricMaker metricMaker, CopyrightReviewApi reviewApi, String projectConfigContents)
- throws ConfigInvalidException {
- metrics = new Metrics(metricMaker);
- allProjectsName = new AllProjectsName("All-Projects");
- pluginName = "copyright";
- repoManager = null;
- projectCache = null;
- pluginConfigFactory = null;
- this.reviewApi = reviewApi;
- checkConfig = new CheckConfig(pluginName, projectConfigContents);
- }
-
- @VisibleForTesting
- static CopyrightConfig createTestInstance(
- MetricMaker metricMaker, CopyrightReviewApi reviewApi, String projectConfigContents)
- throws ConfigInvalidException {
- return new CopyrightConfig(metricMaker, reviewApi, projectConfigContents);
+ this.checkConfig = null;
}
ScannerConfig getScannerConfig() {
- return checkConfig.scannerConfig;
+ return checkConfig == null ? null : checkConfig.scannerConfig;
}
+ @Override
+ public void start() {
+ try (Timer0.Context ctx = metrics.readConfigTimer.start()) {
+ checkConfig = readConfig(projectCache.getAllProjects().getProject().getConfigRefState());
+ } catch (IOException | ConfigInvalidException e) {
+ logger.atSevere().withCause(e).log("unable to load configuration");
+ metrics.configurationErrors.increment(allProjectsName.get());
+ metrics.errors.increment();
+ return;
+ }
+ }
+
+ @Override
+ public void stop() {}
+
/** Listens for merges to /refs/meta/config on All-Projects to reload plugin configuration. */
@Override
public void onGitReferenceUpdated(GitReferenceUpdatedListener.Event event) {
@@ -181,17 +141,13 @@
if (!event.getProjectName().equals(allProjectsName.get())) {
return;
}
- long nanoStart = System.nanoTime();
- try {
- clearConfig();
+ try (Timer0.Context ctx = metrics.readConfigTimer.start()) {
checkConfig = readConfig(event.getNewObjectId());
} catch (IOException | ConfigInvalidException e) {
- logger.atSevere().withCause(e).log("%s plugin unable to load configuration", pluginName);
- checkConfig = null;
+ logger.atSevere().withCause(e).log("unable to load configuration");
+ metrics.configurationErrors.increment(allProjectsName.get());
+ metrics.errors.increment();
return;
- } finally {
- long elapsedMicros = (System.nanoTime() - nanoStart) / 1000;
- metrics.readConfigTimer.record(elapsedMicros, TimeUnit.MICROSECONDS);
}
}
@@ -205,67 +161,56 @@
if (!event.getProjectNameKey().equals(allProjectsName)) {
return Collections.emptyList();
}
- long readStart = System.nanoTime();
- long checkStart = -1;
- long elapsedMicros = -1;
CheckConfig trialConfig = null;
try {
- trialConfig = readConfig(event.commit.getName());
- elapsedMicros = (System.nanoTime() - readStart) / 1000;
- metrics.readConfigTimer.record(elapsedMicros, TimeUnit.MICROSECONDS);
- if (Objects.equals(trialConfig.scannerConfig, checkConfig.scannerConfig)) {
+ try (Timer0.Context ctx = metrics.readConfigTimer.start()) {
+ trialConfig = readConfig(event.commit.getName());
+ }
+ if (Objects.equals(
+ trialConfig.scannerConfig, checkConfig == null ? null : checkConfig.scannerConfig)) {
return Collections.emptyList();
}
- checkStart = System.nanoTime();
- long maxElapsedSeconds =
- gerritConfig == null
- ? DEFAULT_MAX_ELAPSED_SECONDS
- : gerritConfig.getLong(ScannerConfig.KEY_TIME_TEST_MAX, DEFAULT_MAX_ELAPSED_SECONDS);
- if (maxElapsedSeconds > 0
- && CheckConfig.hasScanner(trialConfig)
- && !CheckConfig.scannersEqual(trialConfig, checkConfig)) {
- String commitMessage = event.commit.getFullMessage();
- ImmutableList<CopyrightReviewApi.CommitMessageFinding> findings =
- trialConfig.checkCommitMessage(commitMessage);
- if (CheckConfig.mustReportFindings(findings, maxElapsedSeconds)) {
- throw reviewApi.getCommitMessageException(pluginName, findings, maxElapsedSeconds);
+ try (Timer0.Context ctx = metrics.checkConfigTimer.start()) {
+ long maxElapsedSeconds =
+ gerritConfig == null
+ ? DEFAULT_MAX_ELAPSED_SECONDS
+ : gerritConfig.getLong(
+ ScannerConfig.KEY_TIME_TEST_MAX, DEFAULT_MAX_ELAPSED_SECONDS);
+ if (maxElapsedSeconds > 0
+ && CheckConfig.hasScanner(trialConfig)
+ && !CheckConfig.scannersEqual(trialConfig, checkConfig)) {
+ String commitMessage = event.commit.getFullMessage();
+ ImmutableList<CopyrightReviewApi.CommitMessageFinding> findings =
+ trialConfig.checkCommitMessage(commitMessage);
+ if (CheckConfig.mustReportFindings(findings, maxElapsedSeconds)) {
+ throw reviewApi.getCommitMessageException(pluginName, findings, maxElapsedSeconds);
+ }
}
+ boolean pluginEnabled =
+ gerritConfig != null && gerritConfig.getBoolean(ScannerConfig.KEY_ENABLE, false);
+ CheckConfig.checkProjectConfig(reviewApi, pluginEnabled, trialConfig);
+ List<CommitValidationMessage> messages =
+ trialConfig == null || trialConfig.scannerConfig == null
+ ? Collections.emptyList()
+ : trialConfig.scannerConfig.messages;
+ checkForConfigErrors(trialConfig);
+ return messages;
}
- boolean pluginEnabled =
- gerritConfig != null && gerritConfig.getBoolean(ScannerConfig.KEY_ENABLE, false);
- CheckConfig.checkProjectConfig(reviewApi, pluginEnabled, trialConfig);
- return trialConfig == null || trialConfig.scannerConfig == null
- ? Collections.emptyList()
- : trialConfig.scannerConfig.messages;
} catch (IOException e) {
- logger.atSevere().withCause(e).log(
- "failed to read new project.config for %s plugin", pluginName);
- throw new CommitValidationException("failed to read new project.config", e);
+ logger.atSevere().withCause(e).log("failed to read new project.config");
+ throw new CommitValidationException(
+ pluginName + "plugin failed to read new project.config", e);
} catch (ConfigInvalidException e) {
- logger.atSevere().withCause(e).log("unable to parse %s plugin config", pluginName);
+ logger.atSevere().withCause(e).log("unable to parse plugin config");
if (trialConfig != null && trialConfig.scannerConfig != null) {
trialConfig.scannerConfig.messages.add(ScannerConfig.errorMessage(e.getMessage()));
+ metrics.configurationErrors.increment(allProjectsName.get());
+ metrics.errors.increment();
+ checkForConfigErrors(trialConfig);
return trialConfig.scannerConfig.messages;
} else {
- throw new CommitValidationException("unable to parse new project.config", e);
- }
- } finally {
- if (elapsedMicros < 0) {
- elapsedMicros = (System.nanoTime() - readStart) / 1000;
- metrics.readConfigTimer.record(elapsedMicros, TimeUnit.MICROSECONDS);
- } else if (checkStart >= 0) {
- long elapsedNanos = System.nanoTime() - checkStart;
- metrics.checkConfigTimer.record(elapsedNanos, TimeUnit.NANOSECONDS);
- }
- if (trialConfig != null
- && trialConfig.scannerConfig != null
- && trialConfig.scannerConfig.hasErrors()) {
- StringBuilder sb = new StringBuilder();
- sb.append("\nerror in ");
- sb.append(pluginName);
- sb.append(" plugin configuration");
- trialConfig.scannerConfig.appendMessages(sb);
- throw new CommitValidationException(sb.toString());
+ throw new CommitValidationException(
+ pluginName + "plugin unable to parse new project.config", e);
}
}
}
@@ -286,49 +231,49 @@
return;
}
// passed onCommitReceived so expect at worst only warnings here
- long readStart = System.nanoTime();
- long checkStart = -1;
- long elapsedMicros = -1;
CheckConfig trialConfig = null;
try {
- trialConfig = readConfig(event.getChange().currentRevision);
- elapsedMicros = (System.nanoTime() - readStart) / 1000;
- metrics.readConfigTimer.record(elapsedMicros, TimeUnit.MICROSECONDS);
- if (Objects.equals(trialConfig, checkConfig.scannerConfig)) {
+ try (Timer0.Context ctx = metrics.readConfigTimer.start()) {
+ trialConfig = readConfig(event.getChange().currentRevision);
+ }
+ if (Objects.equals(
+ trialConfig.scannerConfig, checkConfig == null ? null : checkConfig.scannerConfig)) {
return;
}
- checkStart = System.nanoTime();
- if (CheckConfig.hasScanner(trialConfig)
- && !CheckConfig.scannersEqual(trialConfig, checkConfig)) {
- long maxElapsedSeconds =
- gerritConfig == null
- ? DEFAULT_MAX_ELAPSED_SECONDS
- : gerritConfig.getLong(
- ScannerConfig.KEY_TIME_TEST_MAX, DEFAULT_MAX_ELAPSED_SECONDS);
- if (maxElapsedSeconds > 0) {
- String commitMessage = event.getRevision().commitWithFooters;
- ImmutableList<CopyrightReviewApi.CommitMessageFinding> findings =
- trialConfig.checkCommitMessage(commitMessage);
- ReviewResult result =
- reviewApi.reportCommitMessageFindings(
- pluginName,
- allProjectsName.get(),
- checkConfig == null ? null : checkConfig.scannerConfig,
- trialConfig.scannerConfig,
- event,
- findings,
- maxElapsedSeconds);
- logReviewResultErrors(event, result);
+ try (Timer0.Context ctx = metrics.checkConfigTimer.start()) {
+ if (CheckConfig.hasScanner(trialConfig)
+ && !CheckConfig.scannersEqual(trialConfig, checkConfig)) {
+ long maxElapsedSeconds =
+ gerritConfig == null
+ ? DEFAULT_MAX_ELAPSED_SECONDS
+ : gerritConfig.getLong(
+ ScannerConfig.KEY_TIME_TEST_MAX, DEFAULT_MAX_ELAPSED_SECONDS);
+ if (maxElapsedSeconds > 0) {
+ String commitMessage = event.getRevision().commitWithFooters;
+ ImmutableList<CopyrightReviewApi.CommitMessageFinding> findings =
+ trialConfig.checkCommitMessage(commitMessage);
+ ReviewResult result =
+ reviewApi.reportCommitMessageFindings(
+ pluginName,
+ allProjectsName.get(),
+ checkConfig == null ? null : checkConfig.scannerConfig,
+ trialConfig.scannerConfig,
+ event,
+ findings,
+ maxElapsedSeconds);
+ logReviewResultErrors(event, result);
+ }
}
+ boolean pluginEnabled =
+ gerritConfig != null && gerritConfig.getBoolean(ScannerConfig.KEY_ENABLE, false);
+ CheckConfig.checkProjectConfig(reviewApi, pluginEnabled, trialConfig);
+ return;
}
- boolean pluginEnabled =
- gerritConfig != null && gerritConfig.getBoolean(ScannerConfig.KEY_ENABLE, false);
- CheckConfig.checkProjectConfig(reviewApi, pluginEnabled, trialConfig);
- return;
} catch (RestApiException | ConfigInvalidException | IOException e) {
- logger.atSevere().withCause(e).log("%s plugin unable to read new configuration", pluginName);
- // throw IllegalStateException? RestApiException?
+ logger.atSevere().withCause(e).log("unable to read new configuration");
+ metrics.configurationErrors.increment(project);
+ metrics.errors.increment();
return;
} finally {
if (trialConfig != null
@@ -341,24 +286,17 @@
pluginName,
project,
ProjectConfig.PROJECT_CONFIG,
- checkConfig.scannerConfig,
+ checkConfig == null ? null : checkConfig.scannerConfig,
trialConfig.scannerConfig,
event);
logReviewResultErrors(event, result);
} catch (RestApiException e) {
- logger.atSevere().withCause(e).log(
- "%s plugin unable to read new configuration", pluginName);
- // throw IllegalStateException? RestApiException?
+ logger.atSevere().withCause(e).log("unable to report configuration findings");
+ metrics.postReviewErrors.increment(project);
+ metrics.errors.increment();
return;
}
}
- if (elapsedMicros < 0) {
- elapsedMicros = (System.nanoTime() - readStart) / 1000;
- metrics.readConfigTimer.record(elapsedMicros, TimeUnit.MICROSECONDS);
- } else if (checkStart >= 0) {
- long elapsedNanos = System.nanoTime() - checkStart;
- metrics.checkConfigTimer.record(elapsedNanos, TimeUnit.NANOSECONDS);
- }
}
}
@@ -381,18 +319,21 @@
projectState = projectCache.checkedGet(Project.nameKey(project));
} catch (IOException e) {
logger.atSevere().withCause(e).log("error getting project state of %s", project);
- // throw IllegalStateException? RestApiException?
+ metrics.projectStateErrors.increment(project);
+ metrics.errors.increment();
return scannerConfig.defaultEnable;
}
if (projectState == null) {
logger.atSevere().log("error getting project state of %s", project);
- // throw IllegalStateException? RestApiException?
+ metrics.projectStateErrors.increment(project);
+ metrics.errors.increment();
return scannerConfig.defaultEnable;
}
ProjectConfig projectConfig = projectState.getConfig();
if (projectConfig == null) {
logger.atWarning().log("error getting project config of %s", project);
- // throw IllegalStateException? RestApiException? return?
+ metrics.projectConfigErrors.increment(project);
+ metrics.errors.increment();
return scannerConfig.defaultEnable;
}
PluginConfig pluginConfig = projectConfig.getPluginConfig(pluginName);
@@ -403,6 +344,19 @@
return pluginConfig.getBoolean(ScannerConfig.KEY_ENABLE, scannerConfig.defaultEnable);
}
+ private void checkForConfigErrors(CheckConfig trialConfig) throws CommitValidationException {
+ if (trialConfig != null
+ && trialConfig.scannerConfig != null
+ && trialConfig.scannerConfig.hasErrors()) {
+ StringBuilder sb = new StringBuilder();
+ sb.append("\nerror in ");
+ sb.append(pluginName);
+ sb.append(" plugin configuration");
+ trialConfig.scannerConfig.appendMessages(sb);
+ throw new CommitValidationException(sb.toString());
+ }
+ }
+
/**
* Loads and compiles configured patterns from {@code ref/meta/All-Projects/project.config} and
* {@code gerrit.config}.
@@ -411,6 +365,7 @@
* @return the new scanner configuration to check
* @throws IOException if accessing the repository fails
*/
+ @Nullable
private CheckConfig readConfig(String projectConfigObjectId)
throws IOException, ConfigInvalidException {
CheckConfig checkConfig = null;
@@ -425,7 +380,6 @@
}
gerritConfig = pluginConfigFactory.getFromGerritConfig(pluginName, true);
if (gerritConfig == null) {
- // throw IllegalStateException? RestApiException?
checkConfig.scannerConfig.messages.add(
ScannerConfig.hintMessage(
"missing [plugin \"" + pluginName + "\"] section in gerrit.config"));
@@ -436,32 +390,31 @@
return checkConfig;
}
- /** Erases any prior configuration state. */
- private void clearConfig() {
- checkConfig = null;
- }
-
private void logReviewResultErrors(RevisionCreatedListener.Event event, ReviewResult result) {
if (!Strings.isNullOrEmpty(result.error)) {
logger.atSevere().log(
- "%s plugin revision %s: error posting review: %s",
- pluginName, event.getChange().currentRevision, result.error);
+ "revision %s: error posting review: %s", event.getChange().currentRevision, result.error);
+ metrics.postReviewErrors.increment(event.getChange().project);
+ metrics.errors.increment();
}
- for (Map.Entry<String, AddReviewerResult> entry : result.reviewers.entrySet()) {
- AddReviewerResult arr = entry.getValue();
- if (!Strings.isNullOrEmpty(arr.error)) {
- logger.atSevere().log(
- "%s plugin revision %s: error adding reviewer %s: %s",
- pluginName, event.getChange().currentRevision, entry.getKey(), arr.error);
+ if (result.reviewers != null) {
+ for (Map.Entry<String, AddReviewerResult> entry : result.reviewers.entrySet()) {
+ AddReviewerResult arr = entry.getValue();
+ if (!Strings.isNullOrEmpty(arr.error)) {
+ logger.atSevere().log(
+ "revision %s: error adding reviewer %s: %s",
+ event.getChange().currentRevision, entry.getKey(), arr.error);
+ metrics.addReviewerErrors.increment(event.getChange().project);
+ metrics.errors.increment();
+ }
}
}
}
private String readFileContents(Repository repo, ObjectId objectId, String filename)
throws IOException {
- RevWalk rw = new RevWalk(repo);
- RevTree tree = rw.parseTree(objectId);
- try (TreeWalk tw = TreeWalk.forPath(rw.getObjectReader(), filename, tree)) {
+ try (RevWalk rw = new RevWalk(repo);
+ TreeWalk tw = TreeWalk.forPath(rw.getObjectReader(), filename, rw.parseTree(objectId))) {
ObjectLoader loader = repo.open(tw.getObjectId(0), Constants.OBJ_BLOB);
return new String(loader.getCachedBytes(), UTF_8);
}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/copyright/CopyrightReviewApi.java b/src/main/java/com/googlesource/gerrit/plugins/copyright/CopyrightReviewApi.java
index 0c41d7d..de97d60 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/copyright/CopyrightReviewApi.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/copyright/CopyrightReviewApi.java
@@ -4,7 +4,7 @@
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
-// http://www.apache.org/licenses/LICENSE-2.0
+// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
@@ -24,9 +24,10 @@
import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Preconditions;
import com.google.common.base.Strings;
-import com.google.common.base.Throwables;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
+import com.google.gerrit.extensions.api.GerritApi;
+import com.google.gerrit.extensions.api.changes.ChangeApi;
import com.google.gerrit.extensions.api.changes.ReviewInput;
import com.google.gerrit.extensions.api.changes.ReviewInput.CommentInput;
import com.google.gerrit.extensions.api.changes.ReviewResult;
@@ -34,31 +35,17 @@
import com.google.gerrit.extensions.client.ReviewerState;
import com.google.gerrit.extensions.common.CommentInfo;
import com.google.gerrit.extensions.events.RevisionCreatedListener;
-import com.google.gerrit.extensions.restapi.IdString;
-import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
import com.google.gerrit.extensions.restapi.RestApiException;
-import com.google.gerrit.metrics.Counter0;
-import com.google.gerrit.metrics.Counter1;
-import com.google.gerrit.metrics.Description;
-import com.google.gerrit.metrics.Description.Units;
-import com.google.gerrit.metrics.Field;
-import com.google.gerrit.metrics.MetricMaker;
import com.google.gerrit.metrics.Timer0;
import com.google.gerrit.metrics.Timer1;
import com.google.gerrit.reviewdb.client.Account;
-import com.google.gerrit.reviewdb.client.Change;
-import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.IdentifiedUser;
-import com.google.gerrit.server.PatchSetUtil;
import com.google.gerrit.server.PluginUser;
-import com.google.gerrit.server.change.ChangeResource;
-import com.google.gerrit.server.change.RevisionResource;
import com.google.gerrit.server.git.validators.CommitValidationException;
import com.google.gerrit.server.git.validators.CommitValidationMessage;
-import com.google.gerrit.server.notedb.ChangeNotes;
-import com.google.gerrit.server.restapi.change.ListChangeComments;
-import com.google.gerrit.server.restapi.change.PostReview;
+import com.google.gerrit.server.util.ManualRequestContext;
+import com.google.gerrit.server.util.ThreadLocalRequestContext;
import com.google.inject.Provider;
import com.google.inject.Singleton;
import com.googlesource.gerrit.plugins.copyright.lib.CopyrightScanner.Match;
@@ -68,7 +55,6 @@
import java.util.List;
import java.util.Map;
import java.util.Objects;
-import java.util.concurrent.TimeUnit;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
import javax.inject.Inject;
@@ -82,61 +68,8 @@
private final Provider<PluginUser> pluginUserProvider;
private final Provider<CurrentUser> userProvider;
private final IdentifiedUser.GenericFactory identifiedUserFactory;
- private final ChangeNotes.Factory changeNotesFactory;
- private final ChangeResource.Factory changeResourceFactory;
- private final PatchSetUtil psUtil;
- private final PostReview postReview;
- private final ListChangeComments listChangeComments;
-
- @Singleton
- static class Metrics {
- final Counter0 reviewCount;
- final Counter0 commentCount;
- final Timer0 reviewTimer;
- final Counter1 reviewCountByProject;
- final Counter1 commentCountByProject;
- final Timer1 reviewTimerByProject;
-
- @Inject
- Metrics(MetricMaker metricMaker) {
- Field<String> project = Field.ofString("project", "project name");
- reviewCount =
- metricMaker.newCounter(
- "review_count",
- new Description("Total number of posted reviews").setRate().setUnit("reviews"));
- commentCount =
- metricMaker.newCounter(
- "comment_count",
- new Description("Total number of posted review comments")
- .setRate()
- .setUnit("comments"));
- reviewTimer =
- metricMaker.newTimer(
- "review_latency",
- new Description("Time spent posting reviews to revisions")
- .setCumulative()
- .setUnit(Units.MICROSECONDS));
- reviewCountByProject =
- metricMaker.newCounter(
- "review_count_by_project",
- new Description("Total number of posted reviews").setRate().setUnit("reviews"),
- project);
- commentCountByProject =
- metricMaker.newCounter(
- "comment_count_by_project",
- new Description("Total number of posted review comments")
- .setRate()
- .setUnit("comments"),
- project);
- reviewTimerByProject =
- metricMaker.newTimer(
- "review_latency_by_project",
- new Description("Time spent posting reviews to revisions")
- .setCumulative()
- .setUnit(Units.MICROSECONDS),
- project);
- }
- }
+ private final ThreadLocalRequestContext requestContext;
+ private final GerritApi gApi;
@Inject
CopyrightReviewApi(
@@ -144,20 +77,14 @@
Provider<PluginUser> pluginUserProvider,
Provider<CurrentUser> userProvider,
IdentifiedUser.GenericFactory identifiedUserFactory,
- ChangeNotes.Factory changeNotesFactory,
- ChangeResource.Factory changeResourceFactory,
- PatchSetUtil psUtil,
- PostReview postReview,
- ListChangeComments listChangeComments) {
+ ThreadLocalRequestContext requestContext,
+ GerritApi gApi) {
this.metrics = metrics;
this.pluginUserProvider = pluginUserProvider;
this.userProvider = userProvider;
this.identifiedUserFactory = identifiedUserFactory;
- this.changeNotesFactory = changeNotesFactory;
- this.changeResourceFactory = changeResourceFactory;
- this.psUtil = psUtil;
- this.postReview = postReview;
- this.listChangeComments = listChangeComments;
+ this.requestContext = requestContext;
+ this.gApi = gApi;
}
/**
@@ -199,7 +126,6 @@
Preconditions.checkArgument(findings.size() != 1 || !findings.get(0).isValid());
Preconditions.checkArgument(maxElapsedSeconds > 0);
- long startNanos = System.nanoTime();
metrics.reviewCount.increment();
metrics.reviewCountByProject.increment(project);
@@ -207,41 +133,43 @@
oldConfig != null && oldConfig.fromAccountId > 0
? oldConfig.fromAccountId
: newConfig.fromAccountId;
- ChangeResource change = getChange(event, fromAccountId);
- Map<String, List<CommentInfo>> priorReviewComments = getComments(change);
- if (priorReviewComments == null) {
- priorReviewComments = ImmutableMap.of();
- }
- HashSet<CommentInfo> priorComments = new HashSet<>();
- priorComments.addAll(priorReviewComments.get("/COMMIT_MSG"));
-
- ReviewInput ri = new ReviewInput();
- ri.message(getCommitMessageMessage(pluginName, findings, maxElapsedSeconds));
- ImmutableList<CommentInput> comments =
- ImmutableList.copyOf(
- getCommitMessageComments(pluginName, findings, maxElapsedSeconds).stream()
- .filter(ci -> !priorComments.contains(ci))
- .toArray(i -> new CommentInput[i]));
- if (!comments.isEmpty()) {
- ri.comments = ImmutableMap.of("/COMMIT_MSG", comments);
- }
- String label = "Code-Review";
- if (!Strings.isNullOrEmpty(oldConfig.reviewLabel)) {
- label = oldConfig.reviewLabel;
- } else if (!Strings.isNullOrEmpty(newConfig.reviewLabel)) {
- label = newConfig.reviewLabel;
- }
- int vote = (findings.size() != 1 || !findings.get(0).isValid()) ? -2 : 2;
- if (vote == 2) {
- long elapsedMicros = findings.get(0).elapsedMicros;
- if (elapsedMicros > maxElapsedSeconds * 1000000) {
- vote = -2;
- } else if (elapsedMicros > 2000000) {
- vote = 1;
+ try (ManualRequestContext ctx = getContext(fromAccountId)) {
+ ChangeApi cApi = gApi.changes().id(event.getChange().id);
+ Map<String, List<CommentInfo>> priorReviewComments = cApi.comments();
+ if (priorReviewComments == null) {
+ priorReviewComments = ImmutableMap.of();
}
+ HashSet<CommentInfo> priorComments = new HashSet<>();
+ priorComments.addAll(priorReviewComments.get("/COMMIT_MSG"));
+
+ ReviewInput ri = new ReviewInput();
+ ri.message(getCommitMessageMessage(pluginName, findings, maxElapsedSeconds));
+ ImmutableList<CommentInput> comments =
+ ImmutableList.copyOf(
+ getCommitMessageComments(pluginName, findings, maxElapsedSeconds).stream()
+ .filter(ci -> !priorComments.contains(ci))
+ .toArray(i -> new CommentInput[i]));
+ if (!comments.isEmpty()) {
+ ri.comments = ImmutableMap.of("/COMMIT_MSG", comments);
+ }
+ String label = "Code-Review";
+ if (!Strings.isNullOrEmpty(oldConfig.reviewLabel)) {
+ label = oldConfig.reviewLabel;
+ } else if (!Strings.isNullOrEmpty(newConfig.reviewLabel)) {
+ label = newConfig.reviewLabel;
+ }
+ int vote = (findings.size() != 1 || !findings.get(0).isValid()) ? -2 : 2;
+ if (vote == 2) {
+ long elapsedMicros = findings.get(0).elapsedMicros;
+ if (elapsedMicros > maxElapsedSeconds * 1000000) {
+ vote = -2;
+ } else if (elapsedMicros > 2000000) {
+ vote = 1;
+ }
+ }
+ ri.label(label, vote);
+ return cApi.current().review(ri);
}
- ri.label(label, vote);
- return review(change, ri);
}
/**
@@ -308,22 +236,23 @@
Preconditions.checkNotNull(newConfig.messages);
Preconditions.checkArgument(!newConfig.messages.isEmpty());
- long startNanos = System.nanoTime();
metrics.reviewCount.increment();
metrics.reviewCountByProject.increment(project);
- try {
- int fromAccountId =
- oldConfig != null && oldConfig.fromAccountId > 0
- ? oldConfig.fromAccountId
- : newConfig.fromAccountId;
- ChangeResource change = getChange(event, fromAccountId);
+ int fromAccountId =
+ oldConfig != null && oldConfig.fromAccountId > 0
+ ? oldConfig.fromAccountId
+ : newConfig.fromAccountId;
+ try (Timer0.Context t0 = metrics.reviewTimer.start();
+ Timer1.Context t1 = metrics.reviewTimerByProject.start(project);
+ ManualRequestContext ctx = getContext(fromAccountId)) {
+ ChangeApi cApi = gApi.changes().id(event.getChange().id);
StringBuilder message = new StringBuilder();
message.append(pluginName);
message.append(" plugin issues parsing new configuration");
ReviewInput ri = new ReviewInput().message(message.toString());
- Map<String, List<CommentInfo>> priorComments = getComments(change);
+ Map<String, List<CommentInfo>> priorComments = cApi.comments();
if (priorComments == null) {
priorComments = ImmutableMap.of();
}
@@ -350,11 +279,7 @@
}
metrics.commentCount.incrementBy((long) numComments);
metrics.commentCountByProject.incrementBy(project, (long) numComments);
- return review(change, ri);
- } finally {
- long elapsedMicros = (System.nanoTime() - startNanos) / 1000;
- metrics.reviewTimer.record(elapsedMicros, TimeUnit.MICROSECONDS);
- metrics.reviewTimerByProject.record(project, elapsedMicros, TimeUnit.MICROSECONDS);
+ return cApi.current().review(ri);
}
}
@@ -373,11 +298,12 @@
RevisionCreatedListener.Event event,
Map<String, ImmutableList<Match>> findings)
throws RestApiException {
- long startNanos = System.nanoTime();
metrics.reviewCount.increment();
metrics.reviewCountByProject.increment(project);
- try {
+ try (Timer0.Context t0 = metrics.reviewTimer.start();
+ Timer1.Context t1 = metrics.reviewTimerByProject.start(project);
+ ManualRequestContext ctx = getContext(scannerConfig.fromAccountId)) {
boolean tpAllowed = scannerConfig.isThirdPartyAllowed(project);
boolean reviewRequired = false;
for (Map.Entry<String, ImmutableList<Match>> entry : findings.entrySet()) {
@@ -395,7 +321,7 @@
break;
}
}
- ChangeResource change = getChange(event, scannerConfig.fromAccountId);
+ ChangeApi cApi = gApi.changes().id(event.getChange().id);
ReviewInput ri =
new ReviewInput()
.message("Copyright scan")
@@ -411,7 +337,7 @@
} else {
ri = ri.message("This change appears to comply with copyright requirements.");
}
- Map<String, List<CommentInfo>> priorComments = getComments(change);
+ Map<String, List<CommentInfo>> priorComments = cApi.comments();
if (priorComments == null) {
priorComments = ImmutableMap.of();
}
@@ -450,11 +376,7 @@
}
metrics.commentCount.incrementBy((long) numCommentsAdded);
metrics.commentCountByProject.incrementBy(project, (long) numCommentsAdded);
- return review(change, ri);
- } finally {
- long elapsedMicros = (System.nanoTime() - startNanos) / 1000;
- metrics.reviewTimer.record(elapsedMicros, TimeUnit.MICROSECONDS);
- metrics.reviewTimerByProject.record(project, elapsedMicros, TimeUnit.MICROSECONDS);
+ return cApi.current().review(ri);
}
}
@@ -631,26 +553,13 @@
return sb.toString();
}
- /**
- * Constructs a {@link com.google.gerrit.server.change.ChangeResource} from the notes log for the
- * change onto which a new revision was pushed.
- *
- * @param event describes the newly pushed revision
- * @param fromAccountId identifies the configured user to impersonate when sending review comments
- * @throws RestApiException if an error occurs looking up the notes log for the change
- */
- private ChangeResource getChange(RevisionCreatedListener.Event event, int fromAccountId)
- throws RestApiException {
- try {
- CurrentUser fromUser = getSendingUser(fromAccountId);
- ChangeNotes notes = changeNotesFactory.createChecked(Change.id(event.getChange()._number));
- return changeResourceFactory.create(notes, fromUser);
- } catch (Exception e) {
- Throwables.throwIfUnchecked(e);
- throw e instanceof RestApiException
- ? (RestApiException) e
- : new RestApiException("Cannot load change", e);
- }
+ private ManualRequestContext getContext(int fromAccountId) {
+ PluginUser pluginUser = pluginUserProvider.get();
+ CurrentUser user =
+ fromAccountId <= 0
+ ? userProvider.get()
+ : identifiedUserFactory.runAs(null, Account.id(fromAccountId), pluginUser);
+ return new ManualRequestContext(user, requestContext);
}
/**
@@ -665,44 +574,6 @@
return ri;
}
- /**
- * Retrieves all of the prior review comments already attached to {@code change}.
- *
- * @throws RestApiException if an error occurs retrieving the comments
- */
- private Map<String, List<CommentInfo>> getComments(ChangeResource change)
- throws RestApiException {
- try {
- return listChangeComments.apply(change);
- } catch (Exception e) {
- Throwables.throwIfUnchecked(e);
- throw e instanceof RestApiException
- ? (RestApiException) e
- : new RestApiException("Cannot list comments", e);
- }
- }
-
- /**
- * Adds the code review described by {@code ri} to the review thread of {@code change}.
- *
- * @throws RestApiException if an error occurs updating the review thread
- */
- private ReviewResult review(ChangeResource change, ReviewInput ri) throws RestApiException {
- try {
- PatchSet ps = psUtil.current(change.getNotes());
- if (ps == null) {
- throw new ResourceNotFoundException(IdString.fromDecoded("current"));
- }
- RevisionResource revision = RevisionResource.createNonCacheable(change, ps);
- return postReview.apply(revision, ri).value();
- } catch (Exception e) {
- Throwables.throwIfUnchecked(e);
- throw e instanceof RestApiException
- ? (RestApiException) e
- : new RestApiException("Cannot post review", e);
- }
- }
-
/** Returns true if {@code priorComments} already includes a comment identical to {@code ci}. */
@VisibleForTesting
boolean containsComment(Iterable<? extends Comment> priorComments, CommentInput ci) {
diff --git a/src/main/java/com/googlesource/gerrit/plugins/copyright/CopyrightValidator.java b/src/main/java/com/googlesource/gerrit/plugins/copyright/CopyrightValidator.java
index 0a77834..7ed50dd 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/copyright/CopyrightValidator.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/copyright/CopyrightValidator.java
@@ -4,7 +4,7 @@
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
-// http://www.apache.org/licenses/LICENSE-2.0
+// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
@@ -26,12 +26,6 @@
import com.google.gerrit.extensions.events.RevisionCreatedListener;
import com.google.gerrit.extensions.registration.DynamicSet;
import com.google.gerrit.extensions.restapi.RestApiException;
-import com.google.gerrit.metrics.Counter0;
-import com.google.gerrit.metrics.Counter1;
-import com.google.gerrit.metrics.Description;
-import com.google.gerrit.metrics.Description.Units;
-import com.google.gerrit.metrics.Field;
-import com.google.gerrit.metrics.MetricMaker;
import com.google.gerrit.metrics.Timer0;
import com.google.gerrit.metrics.Timer1;
import com.google.gerrit.reviewdb.client.Project;
@@ -40,14 +34,12 @@
import com.google.gerrit.server.config.PluginConfigFactory;
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.inject.AbstractModule;
-import com.google.inject.Singleton;
import com.googlesource.gerrit.plugins.copyright.lib.CopyrightScanner.Match;
import com.googlesource.gerrit.plugins.copyright.lib.IndexedLineReader;
import java.io.IOException;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.Map;
-import java.util.concurrent.TimeUnit;
import javax.inject.Inject;
import org.eclipse.jgit.lib.FileMode;
import org.eclipse.jgit.lib.ObjectId;
@@ -81,79 +73,6 @@
};
}
- @Singleton
- private static class Metrics {
- final Counter0 scanCount;
- final Timer0 scanRevisionTimer;
- final Timer0 scanFileTimer;
- final Counter1 scanCountByProject;
- final Timer1 scanRevisionTimerByProject;
- final Timer1 scanFileTimerByProject;
- final Counter1 scanCountByBranch;
- final Timer1 scanRevisionTimerByBranch;
- final Timer1 scanFileTimerByBranch;
-
- @Inject
- Metrics(MetricMaker metricMaker) {
- Field<String> project = Field.ofString("project", "project name");
- Field<String> branch = Field.ofString("branch", "branch name");
- scanCount =
- metricMaker.newCounter(
- "scan_count",
- new Description("Total number of copyright scans").setRate().setUnit("scans"));
- scanRevisionTimer =
- metricMaker.newTimer(
- "scan_revision_latency",
- new Description("Time spent scanning entire revisions")
- .setCumulative()
- .setUnit(Units.MILLISECONDS));
- scanFileTimer =
- metricMaker.newTimer(
- "scan_file_latency",
- new Description("Time spent scanning each file")
- .setCumulative()
- .setUnit(Units.MICROSECONDS));
- scanCountByProject =
- metricMaker.newCounter(
- "scan_count_by_project",
- new Description("Total number of copyright scans").setRate().setUnit("scans"),
- project);
- scanRevisionTimerByProject =
- metricMaker.newTimer(
- "scan_revision_latency_by_project",
- new Description("Time spent scanning entire revisions")
- .setCumulative()
- .setUnit(Units.MILLISECONDS),
- project);
- scanFileTimerByProject =
- metricMaker.newTimer(
- "scan_file_latency_by_project",
- new Description("Time spent scanning each file")
- .setCumulative()
- .setUnit(Units.MICROSECONDS),
- project);
- scanCountByBranch =
- metricMaker.newCounter(
- "scan_count_by_branch",
- new Description("Total number of copyright scans").setRate().setUnit("scans"),
- branch);
- scanRevisionTimerByBranch =
- metricMaker.newTimer(
- "scan_revision_latency_by_branch",
- new Description("Time spent scanning entire revisions")
- .setCumulative()
- .setUnit(Units.MILLISECONDS),
- branch);
- scanFileTimerByBranch =
- metricMaker.newTimer(
- "scan_file_latency_by_branch",
- new Description("Time spent scanning each file")
- .setCumulative()
- .setUnit(Units.MICROSECONDS),
- branch);
- }
- }
-
@Inject
CopyrightValidator(
Metrics metrics,
@@ -189,16 +108,18 @@
if (scannerConfig == null) {
// error already reported during configuration load
logger.atWarning().log(
- "%s plugin enabled with no configuration -- not scanning revision %s",
- pluginName, event.getChange().currentRevision);
+ "plugin enabled with no configuration -- not scanning revision %s",
+ event.getChange().currentRevision);
+ metrics.skippedReviewWarnings.increment(project);
return;
}
if (scannerConfig.scanner == null) {
if (scannerConfig.hasErrors()) {
// error already reported during configuration load
logger.atWarning().log(
- "%s plugin enabled with errors in configuration -- not scanning revision %s",
- pluginName, event.getChange().currentRevision);
+ "plugin enabled with errors in configuration -- not scanning revision %s",
+ event.getChange().currentRevision);
+ metrics.skippedReviewWarnings.increment(project);
} // else plugin not enabled
return;
}
@@ -210,7 +131,9 @@
scanRevision(project, branch, event);
} catch (IOException | RestApiException e) {
logger.atSevere().withCause(e).log(
- "%s plugin cannot scan revision %s", pluginName, event.getChange().currentRevision);
+ "cannot scan revision %s", event.getChange().currentRevision);
+ metrics.scanErrors.increment(project);
+ metrics.errors.increment();
return;
}
}
@@ -228,12 +151,13 @@
throws IOException, RestApiException {
Map<String, ImmutableList<Match>> findings = new HashMap<>();
ArrayList<String> containedPaths = new ArrayList<>();
- long scanStart = System.nanoTime();
- metrics.scanCount.increment();
metrics.scanCountByProject.increment(project);
metrics.scanCountByBranch.increment(branch);
- try (Repository repo = repoManager.openRepository(Project.nameKey(project));
+ try (Timer0.Context t0 = metrics.scanRevisionTimer.start();
+ Timer1.Context t1project = metrics.scanRevisionTimerByProject.start(project);
+ Timer1.Context t1branch = metrics.scanRevisionTimerByBranch.start(branch);
+ Repository repo = repoManager.openRepository(Project.nameKey(project));
RevWalk revWalk = new RevWalk(repo);
TreeWalk tw = new TreeWalk(revWalk.getObjectReader())) {
RevCommit commit = repo.parseCommit(ObjectId.fromString(event.getRevision().commit.commit));
@@ -259,8 +183,10 @@
&& !FileMode.REGULAR_FILE.equals(tw.getFileMode())) {
continue;
}
- long fileStart = System.nanoTime();
- try (ObjectReader reader = tw.getObjectReader();
+ try (Timer0.Context tf0 = metrics.scanFileTimer.start();
+ Timer1.Context tf1project = metrics.scanFileTimerByProject.start(project);
+ Timer1.Context tf1branch = metrics.scanFileTimerByBranch.start(branch);
+ ObjectReader reader = tw.getObjectReader();
ObjectStream stream = reader.open(tw.getObjectId(0)).openStream()) {
IndexedLineReader lineReader = new IndexedLineReader(fullPath, -1, stream);
ImmutableList<Match> matches =
@@ -268,31 +194,26 @@
if (!matches.isEmpty()) {
findings.put(tw.getPathString(), matches);
}
- long fileTime = (System.nanoTime() - fileStart) / 1000;
- metrics.scanFileTimer.record(fileTime, TimeUnit.MICROSECONDS);
- metrics.scanFileTimerByProject.record(project, fileTime, TimeUnit.MICROSECONDS);
- metrics.scanFileTimerByBranch.record(branch, fileTime, TimeUnit.MICROSECONDS);
}
}
}
- long scanTime = (System.nanoTime() - scanStart) / 1000000;
- metrics.scanRevisionTimer.record(scanTime, TimeUnit.MILLISECONDS);
- metrics.scanRevisionTimerByProject.record(project, scanTime, TimeUnit.MILLISECONDS);
- metrics.scanRevisionTimerByBranch.record(branch, scanTime, TimeUnit.MILLISECONDS);
ReviewResult result = reviewApi.reportScanFindings(project, scannerConfig, event, findings);
if (result != null && result.error != null && !result.error.equals("")) {
logger.atSevere().log(
- "%s plugin revision %s: error posting review: %s",
- pluginName, event.getChange().currentRevision, result.error);
+ "revision %s: error posting review: %s", event.getChange().currentRevision, result.error);
+ metrics.postReviewErrors.increment(project);
+ metrics.errors.increment();
}
if (result != null && result.reviewers != null) {
for (Map.Entry<String, AddReviewerResult> entry : result.reviewers.entrySet()) {
AddReviewerResult arr = entry.getValue();
if (arr.error != null && !arr.error.equals("")) {
logger.atSevere().log(
- "%s plugin revision %s: error adding reviewer %s: %s",
- pluginName, event.getChange().currentRevision, entry.getKey(), arr.error);
+ "revision %s: error adding reviewer %s: %s",
+ event.getChange().currentRevision, entry.getKey(), arr.error);
+ metrics.addReviewerErrors.increment(project);
+ metrics.errors.increment();
}
}
}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/copyright/Metrics.java b/src/main/java/com/googlesource/gerrit/plugins/copyright/Metrics.java
new file mode 100644
index 0000000..1e22006
--- /dev/null
+++ b/src/main/java/com/googlesource/gerrit/plugins/copyright/Metrics.java
@@ -0,0 +1,216 @@
+// Copyright (C) 2019 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.googlesource.gerrit.plugins.copyright;
+
+import com.google.gerrit.metrics.Counter0;
+import com.google.gerrit.metrics.Counter1;
+import com.google.gerrit.metrics.Description;
+import com.google.gerrit.metrics.Description.Units;
+import com.google.gerrit.metrics.Field;
+import com.google.gerrit.metrics.MetricMaker;
+import com.google.gerrit.metrics.Timer0;
+import com.google.gerrit.metrics.Timer1;
+import com.google.inject.Singleton;
+import javax.inject.Inject;
+
+@Singleton
+class Metrics {
+ final Timer0 readConfigTimer;
+ final Timer0 checkConfigTimer;
+ final Timer0 testConfigTimer;
+ final Counter1<String> postReviewErrors;
+ final Counter1<String> addReviewerErrors;
+ final Counter1<String> projectStateErrors;
+ final Counter1<String> projectConfigErrors;
+ final Counter1<String> configurationErrors;
+ final Counter0 reviewCount;
+ final Counter0 commentCount;
+ final Timer0 reviewTimer;
+ final Counter1<String> reviewCountByProject;
+ final Counter1<String> commentCountByProject;
+ final Timer1<String> reviewTimerByProject;
+ final Timer0 scanRevisionTimer;
+ final Timer0 scanFileTimer;
+ final Counter1<String> scanCountByProject;
+ final Timer1<String> scanRevisionTimerByProject;
+ final Timer1<String> scanFileTimerByProject;
+ final Counter1<String> scanCountByBranch;
+ final Timer1<String> scanRevisionTimerByBranch;
+ final Timer1<String> scanFileTimerByBranch;
+ final Counter1<String> skippedReviewWarnings;
+ final Counter1<String> scanErrors;
+ final Counter0 errors;
+
+ @Inject
+ Metrics(MetricMaker metricMaker) {
+ Field<String> project = Field.ofString("project", "project name");
+ Field<String> branch = Field.ofString("branch", "branch name");
+
+ readConfigTimer =
+ metricMaker.newTimer(
+ "read_config_latency",
+ new Description("Time spent reading and parsing plugin configurations")
+ .setCumulative()
+ .setUnit(Units.MICROSECONDS));
+ checkConfigTimer =
+ metricMaker.newTimer(
+ "check_config_latency",
+ new Description("Time spent testing proposed plugin configurations")
+ .setCumulative()
+ .setUnit(Units.MICROSECONDS));
+ testConfigTimer =
+ metricMaker.newTimer(
+ "test_config_latency",
+ new Description("Time spent testing configurations against difficult file pattern")
+ .setCumulative()
+ .setUnit(Units.MICROSECONDS));
+ postReviewErrors =
+ metricMaker.newCounter(
+ "post_review_error_count",
+ new Description("Number of failed attempts to post reviews")
+ .setRate()
+ .setUnit("errors"),
+ project);
+ addReviewerErrors =
+ metricMaker.newCounter(
+ "add_reviewer_error_count",
+ new Description("Number of failed attempts to add a reviewer")
+ .setRate()
+ .setUnit("errors"),
+ project);
+ projectStateErrors =
+ metricMaker.newCounter(
+ "read_project_state_error_count",
+ new Description("Number of failed attempts to read project state")
+ .setRate()
+ .setUnit("errors"),
+ project);
+ projectConfigErrors =
+ metricMaker.newCounter(
+ "get_project_config_error_count",
+ new Description("Number of failed attempts to get config from project state")
+ .setRate()
+ .setUnit("errors"),
+ project);
+ configurationErrors =
+ metricMaker.newCounter(
+ "read_configuration_error_count",
+ new Description("Number of failed attempts to read configuration")
+ .setRate()
+ .setUnit("errors"),
+ project);
+ reviewCount =
+ metricMaker.newCounter(
+ "review_count",
+ new Description("Total number of posted reviews").setRate().setUnit("reviews"));
+ commentCount =
+ metricMaker.newCounter(
+ "comment_count",
+ new Description("Total number of posted review comments")
+ .setRate()
+ .setUnit("comments"));
+ reviewTimer =
+ metricMaker.newTimer(
+ "review_latency",
+ new Description("Time spent posting reviews to revisions")
+ .setCumulative()
+ .setUnit(Units.MICROSECONDS));
+ reviewCountByProject =
+ metricMaker.newCounter(
+ "review_count_by_project",
+ new Description("Total number of posted reviews").setRate().setUnit("reviews"),
+ project);
+ commentCountByProject =
+ metricMaker.newCounter(
+ "comment_count_by_project",
+ new Description("Total number of posted review comments").setRate().setUnit("comments"),
+ project);
+ reviewTimerByProject =
+ metricMaker.newTimer(
+ "review_latency_by_project",
+ new Description("Time spent posting reviews to revisions")
+ .setCumulative()
+ .setUnit(Units.MICROSECONDS),
+ project);
+ scanRevisionTimer =
+ metricMaker.newTimer(
+ "scan_revision_latency",
+ new Description("Time spent scanning entire revisions")
+ .setCumulative()
+ .setUnit(Units.MILLISECONDS));
+ scanFileTimer =
+ metricMaker.newTimer(
+ "scan_file_latency",
+ new Description("Time spent scanning each file")
+ .setCumulative()
+ .setUnit(Units.MICROSECONDS));
+ scanCountByProject =
+ metricMaker.newCounter(
+ "scan_count_by_project",
+ new Description("Total number of copyright scans").setRate().setUnit("scans"),
+ project);
+ scanRevisionTimerByProject =
+ metricMaker.newTimer(
+ "scan_revision_latency_by_project",
+ new Description("Time spent scanning entire revisions")
+ .setCumulative()
+ .setUnit(Units.MILLISECONDS),
+ project);
+ scanFileTimerByProject =
+ metricMaker.newTimer(
+ "scan_file_latency_by_project",
+ new Description("Time spent scanning each file")
+ .setCumulative()
+ .setUnit(Units.MICROSECONDS),
+ project);
+ scanCountByBranch =
+ metricMaker.newCounter(
+ "scan_count_by_branch",
+ new Description("Total number of copyright scans").setRate().setUnit("scans"),
+ branch);
+ scanRevisionTimerByBranch =
+ metricMaker.newTimer(
+ "scan_revision_latency_by_branch",
+ new Description("Time spent scanning entire revisions")
+ .setCumulative()
+ .setUnit(Units.MILLISECONDS),
+ branch);
+ scanFileTimerByBranch =
+ metricMaker.newTimer(
+ "scan_file_latency_by_branch",
+ new Description("Time spent scanning each file")
+ .setCumulative()
+ .setUnit(Units.MICROSECONDS),
+ branch);
+ skippedReviewWarnings =
+ metricMaker.newCounter(
+ "skipped_scan_warning_count",
+ new Description("Number revision scans skipped due to configuration problems")
+ .setRate()
+ .setUnit("warnings"),
+ project);
+ scanErrors =
+ metricMaker.newCounter(
+ "failed_scan_error_count",
+ new Description("Number of failed attempts to scan revisions")
+ .setRate()
+ .setUnit("errors"),
+ project);
+ errors =
+ metricMaker.newCounter(
+ "error_count",
+ new Description("Number of errors of any kind").setRate().setUnit("errors"));
+ }
+}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/copyright/Module.java b/src/main/java/com/googlesource/gerrit/plugins/copyright/Module.java
index af872c7..7f7bd82 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/copyright/Module.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/copyright/Module.java
@@ -4,7 +4,7 @@
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
-// http://www.apache.org/licenses/LICENSE-2.0
+// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
diff --git a/src/main/java/com/googlesource/gerrit/plugins/copyright/ScannerConfig.java b/src/main/java/com/googlesource/gerrit/plugins/copyright/ScannerConfig.java
index 815b28c..c53ee1f 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/copyright/ScannerConfig.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/copyright/ScannerConfig.java
@@ -4,7 +4,7 @@
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
-// http://www.apache.org/licenses/LICENSE-2.0
+// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
@@ -33,7 +33,7 @@
import java.util.regex.Pattern;
import java.util.regex.PatternSyntaxException;
-/** Configuration state for {@link CopyrightValidator}. */
+/** Configuration state for {@link CopyrightValidator} from All-Projects project.config file. */
class ScannerConfig {
private static final FluentLogger logger = FluentLogger.forEnclosingClass();
@@ -88,6 +88,7 @@
this.fromAccountId = 0;
}
+ /** True when {@link CopyrightValidator} will behave the same with either config. */
@Override
public boolean equals(Object other) {
if (this == other) {
@@ -99,6 +100,7 @@
if (other instanceof ScannerConfig) {
ScannerConfig otherConfig = (ScannerConfig) other;
return defaultEnable == otherConfig.defaultEnable
+ && fromAccountId == otherConfig.fromAccountId
&& messages.equals(otherConfig.messages)
&& alwaysReviewPath.equals(otherConfig.alwaysReviewPath)
&& matchProjects.equals(otherConfig.matchProjects)
@@ -116,6 +118,7 @@
public int hashCode() {
return Objects.hash(
defaultEnable,
+ fromAccountId,
messages,
alwaysReviewPath,
matchProjects,
diff --git a/src/main/java/com/googlesource/gerrit/plugins/copyright/lib/Archive.java b/src/main/java/com/googlesource/gerrit/plugins/copyright/lib/Archive.java
index 8d6ed0b..8d8fab7 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/copyright/lib/Archive.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/copyright/lib/Archive.java
@@ -4,7 +4,7 @@
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
-// http://www.apache.org/licenses/LICENSE-2.0
+// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
diff --git a/src/main/java/com/googlesource/gerrit/plugins/copyright/lib/CopyrightPatterns.java b/src/main/java/com/googlesource/gerrit/plugins/copyright/lib/CopyrightPatterns.java
index 060dffc..d2bf243 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/copyright/lib/CopyrightPatterns.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/copyright/lib/CopyrightPatterns.java
@@ -4,7 +4,7 @@
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
-// http://www.apache.org/licenses/LICENSE-2.0
+// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
diff --git a/src/main/java/com/googlesource/gerrit/plugins/copyright/lib/CopyrightScanner.java b/src/main/java/com/googlesource/gerrit/plugins/copyright/lib/CopyrightScanner.java
index ddc5f78..736a537 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/copyright/lib/CopyrightScanner.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/copyright/lib/CopyrightScanner.java
@@ -4,7 +4,7 @@
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
-// http://www.apache.org/licenses/LICENSE-2.0
+// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
diff --git a/src/main/java/com/googlesource/gerrit/plugins/copyright/lib/IndexedLineReader.java b/src/main/java/com/googlesource/gerrit/plugins/copyright/lib/IndexedLineReader.java
index 015863c..f4749fc 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/copyright/lib/IndexedLineReader.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/copyright/lib/IndexedLineReader.java
@@ -4,7 +4,7 @@
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
-// http://www.apache.org/licenses/LICENSE-2.0
+// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
diff --git a/src/main/java/com/googlesource/gerrit/plugins/copyright/tools/AndroidScan.java b/src/main/java/com/googlesource/gerrit/plugins/copyright/tools/AndroidScan.java
index b5591a4..37fed5e 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/copyright/tools/AndroidScan.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/copyright/tools/AndroidScan.java
@@ -4,7 +4,7 @@
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
-// http://www.apache.org/licenses/LICENSE-2.0
+// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
diff --git a/src/main/java/com/googlesource/gerrit/plugins/copyright/tools/ScanTool.java b/src/main/java/com/googlesource/gerrit/plugins/copyright/tools/ScanTool.java
index 0a4bb04..087f2c3 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/copyright/tools/ScanTool.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/copyright/tools/ScanTool.java
@@ -4,7 +4,7 @@
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
-// http://www.apache.org/licenses/LICENSE-2.0
+// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
diff --git a/src/main/resources/Documentation/about.md b/src/main/resources/Documentation/about.md
index e700891..a165149 100644
--- a/src/main/resources/Documentation/about.md
+++ b/src/main/resources/Documentation/about.md
@@ -14,6 +14,7 @@
It will have false-positives--especially in the unknown category. It may also
have false negatives.
+
## First-party
Licensed or owned by the owner of the gerrit repository.
diff --git a/src/main/resources/Documentation/build.md b/src/main/resources/Documentation/build.md
index 81eec27..b682c71 100644
--- a/src/main/resources/Documentation/build.md
+++ b/src/main/resources/Documentation/build.md
@@ -84,6 +84,7 @@
bazel test --test_tag_filters=@PLUGIN@ //...
````
+
This project can be imported into the Eclipse IDE.
Add the plugin name to the `CUSTOM_PLUGINS` set in
Gerrit core in `tools/bzl/plugins.bzl`, and execute:
diff --git a/src/main/resources/Documentation/config.md b/src/main/resources/Documentation/config.md
index 32b4ef9..5163a2d 100644
--- a/src/main/resources/Documentation/config.md
+++ b/src/main/resources/Documentation/config.md
@@ -43,6 +43,7 @@
the command-line tool, to take heed of any warnings or errors it prints, and
to copy+paste the token.
+
## Project Configuration
Each project's `project.config` has a configuration parameter to enable or to
@@ -216,7 +217,8 @@
Review comments will appear to come from the given account. Recommend
creating a non-interactive user with a descriptive name like
- "Copyright Scanner".
+ "Copyright Scanner". Must have read access to refs/heads/* and to
+ refs/meta/config in all repositories.
plugin.@PLUGIN@.matchProjects
: List of Project Patterns to Scan
diff --git a/src/main/resources/Documentation/known-patterns.md b/src/main/resources/Documentation/known-patterns.md
index fae7a2d..9499a1e 100644
--- a/src/main/resources/Documentation/known-patterns.md
+++ b/src/main/resources/Documentation/known-patterns.md
@@ -15,3 +15,4 @@
The names do not have to match an exact license or entity per se. For example,
the Creative Commons folks do not define a CC_BY_C license. The named pattern
matches any of several Creative Commons licenses that allow commercial use.
+
diff --git a/src/main/resources/Documentation/modified-regex.md b/src/main/resources/Documentation/modified-regex.md
index 9e1bd9e..8abc2df 100644
--- a/src/main/resources/Documentation/modified-regex.md
+++ b/src/main/resources/Documentation/modified-regex.md
@@ -10,6 +10,7 @@
expressions](https://docs.oracle.com/javase/7/docs/api/java/util/regex/Pattern.html)
with modifications to assist the 3 requirements above.
+
## Unlimited Wildcards
If you run the ScanTool.java command-line tool against the files in your
diff --git a/src/test/java/com/googlesource/gerrit/plugins/copyright/CopyrightConfigIT.java b/src/test/java/com/googlesource/gerrit/plugins/copyright/CopyrightConfigIT.java
index 7a31f07..af7278a 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/copyright/CopyrightConfigIT.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/copyright/CopyrightConfigIT.java
@@ -4,7 +4,7 @@
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
-// http://www.apache.org/licenses/LICENSE-2.0
+// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
@@ -25,6 +25,7 @@
import static com.googlesource.gerrit.plugins.copyright.TestConfig.LOCAL_BRANCH;
import com.google.common.collect.ImmutableList;
+import com.google.common.flogger.FluentLogger;
import com.google.common.truth.Correspondence;
import com.google.gerrit.acceptance.GerritConfig;
import com.google.gerrit.acceptance.GerritConfigs;
@@ -47,6 +48,8 @@
@TestPlugin(name = "copyright", sysModule = "com.googlesource.gerrit.plugins.copyright.Module")
public class CopyrightConfigIT extends LightweightPluginDaemonTest {
+ private static final FluentLogger logger = FluentLogger.forEnclosingClass();
+
@Test
@GerritConfigs({
@GerritConfig(name = "plugin.copyright.enable", value = "true"),
@@ -99,7 +102,9 @@
})
public void testCopyrightConfig_pushWithSenderNoName() throws Exception {
TestAccount anonymous =
- accountCreator.create("copyright-scan", "", "", "Non-Interactive Users", expertGroup.name);
+ accountCreator.create(
+ "copyright-scan", "", "", "Administrators", "Non-Interactive Users", expertGroup.name);
+
testConfig.updatePlugin(
cfg -> {
cfg.setInt(KEY_FROM, anonymous.id().get());
@@ -317,6 +322,7 @@
"copyright-scanner",
"copyright-scanner@example.com",
"Copyright Scanner",
+ "Administrators",
"Non-Interactive Users",
expertGroup.name);
reviewer =
@@ -360,16 +366,9 @@
}
private static Correspondence<Comment, String> warningContains() {
- return new Correspondence<Comment, String>() {
- @Override
- public boolean compare(Comment actual, String expected) {
- return actual.message.startsWith("WARNING ") && actual.message.contains(expected);
- }
-
- @Override
- public String toString() {
- return "matches regex";
- }
- };
+ return Correspondence.from(
+ (actual, expected) ->
+ actual.message.startsWith("WARNING ") && actual.message.contains(expected),
+ "matches regex");
}
}
diff --git a/src/test/java/com/googlesource/gerrit/plugins/copyright/CopyrightReviewApiTest.java b/src/test/java/com/googlesource/gerrit/plugins/copyright/CopyrightReviewApiTest.java
index f71ea0f..efea843 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/copyright/CopyrightReviewApiTest.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/copyright/CopyrightReviewApiTest.java
@@ -4,7 +4,7 @@
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
-// http://www.apache.org/licenses/LICENSE-2.0
+// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
@@ -39,6 +39,7 @@
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.PluginUser;
import com.google.gerrit.server.account.GroupMembership;
+import com.google.gerrit.server.util.ThreadLocalRequestContext;
import com.googlesource.gerrit.plugins.copyright.lib.CopyrightScanner.Match;
import org.junit.Before;
import org.junit.Test;
@@ -53,21 +54,15 @@
private IdentifiedUser.GenericFactory identifiedUserFactory =
createMock(IdentifiedUser.GenericFactory.class);
+ private ThreadLocalRequestContext requestContext = createMock(ThreadLocalRequestContext.class);
+
private CopyrightReviewApi reviewApi;
@Before
public void setUp() throws Exception {
reviewApi =
new CopyrightReviewApi(
- null,
- () -> pluginUser,
- () -> currentUser,
- identifiedUserFactory,
- null,
- null,
- null,
- null,
- null);
+ null, () -> pluginUser, () -> currentUser, identifiedUserFactory, requestContext, null);
}
@Test
@@ -90,7 +85,7 @@
CurrentUser from = reviewApi.getSendingUser(0);
verify(identifiedUserFactory);
- assertThat(from).isSameAs(currentUser);
+ assertThat(from).isSameInstanceAs(currentUser);
}
@Test
@@ -353,33 +348,18 @@
}
private static Correspondence<CommentInput, CommentInput> startsWithAndRangesMatch() {
- return new Correspondence<CommentInput, CommentInput>() {
- @Override
- public boolean compare(CommentInput actual, CommentInput expected) {
- return actual.range.startLine == expected.range.startLine
- && actual.range.endLine == expected.range.endLine
- && actual.message.startsWith(expected.message);
- }
-
- @Override
- public String toString() {
- return "starts with and ranges match";
- }
- };
+ return Correspondence.from(
+ (actual, expected) ->
+ actual.range.startLine == expected.range.startLine
+ && actual.range.endLine == expected.range.endLine
+ && actual.message.startsWith(expected.message),
+ "starts with and ranges match");
}
private static Correspondence<AddReviewerInput, String> addressedTo() {
- return new Correspondence<AddReviewerInput, String>() {
- @Override
- public boolean compare(AddReviewerInput actual, String expected) {
- return expected.equals(actual.state().toString() + ":" + actual.reviewer);
- }
-
- @Override
- public String toString() {
- return "addressed to";
- }
- };
+ return Correspondence.from(
+ (actual, expected) -> expected.equals(actual.state().toString() + ":" + actual.reviewer),
+ "addressed to");
}
/** Comment input {@code text} from {@code start} line to {@code end} line. */
diff --git a/src/test/java/com/googlesource/gerrit/plugins/copyright/CopyrightValidatorIT.java b/src/test/java/com/googlesource/gerrit/plugins/copyright/CopyrightValidatorIT.java
index 304e87f..a629e9f 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/copyright/CopyrightValidatorIT.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/copyright/CopyrightValidatorIT.java
@@ -4,7 +4,7 @@
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
-// http://www.apache.org/licenses/LICENSE-2.0
+// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
@@ -434,17 +434,10 @@
}
private static Correspondence<Comment, CommentMatch> commentContains() {
- return new Correspondence<Comment, CommentMatch>() {
- @Override
- public boolean compare(Comment actual, CommentMatch expected) {
- return actual.unresolved != expected.resolved && actual.message.contains(expected.content);
- }
-
- @Override
- public String toString() {
- return "comment resolution status and content matches";
- }
- };
+ return Correspondence.from(
+ (actual, expected) ->
+ actual.unresolved != expected.resolved && actual.message.contains(expected.content),
+ "comment resolution status and content matches");
}
private static class CommentMatch {
diff --git a/src/test/java/com/googlesource/gerrit/plugins/copyright/TestConfig.java b/src/test/java/com/googlesource/gerrit/plugins/copyright/TestConfig.java
index f35cbc0..346c53b 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/copyright/TestConfig.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/copyright/TestConfig.java
@@ -4,7 +4,7 @@
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
-// http://www.apache.org/licenses/LICENSE-2.0
+// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
diff --git a/src/test/java/com/googlesource/gerrit/plugins/copyright/lib/ArchiveTest.java b/src/test/java/com/googlesource/gerrit/plugins/copyright/lib/ArchiveTest.java
index c5aeb8d..87e4023 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/copyright/lib/ArchiveTest.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/copyright/lib/ArchiveTest.java
@@ -4,7 +4,7 @@
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
-// http://www.apache.org/licenses/LICENSE-2.0
+// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
diff --git a/src/test/java/com/googlesource/gerrit/plugins/copyright/lib/CopyrightPatternsTest.java b/src/test/java/com/googlesource/gerrit/plugins/copyright/lib/CopyrightPatternsTest.java
index a7bd486..394eb76 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/copyright/lib/CopyrightPatternsTest.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/copyright/lib/CopyrightPatternsTest.java
@@ -4,7 +4,7 @@
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
-// http://www.apache.org/licenses/LICENSE-2.0
+// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
diff --git a/src/test/java/com/googlesource/gerrit/plugins/copyright/lib/CopyrightScannerTest.java b/src/test/java/com/googlesource/gerrit/plugins/copyright/lib/CopyrightScannerTest.java
index 3aa379b..6966975 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/copyright/lib/CopyrightScannerTest.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/copyright/lib/CopyrightScannerTest.java
@@ -4,7 +4,7 @@
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
-// http://www.apache.org/licenses/LICENSE-2.0
+// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
@@ -36,17 +36,7 @@
public class CopyrightScannerTest {
private static final Correspondence<String, String> CONTAINS_STRING =
- new Correspondence<String, String>() {
- @Override
- public boolean compare(String actual, String expected) {
- return actual.contains(expected);
- }
-
- @Override
- public String toString() {
- return "contains";
- }
- };
+ Correspondence.from((actual, expected) -> actual.contains(expected), "contains");
private CopyrightPatterns.RuleSet.Builder builder;
diff --git a/src/test/java/com/googlesource/gerrit/plugins/copyright/lib/IndexedLineReaderTest.java b/src/test/java/com/googlesource/gerrit/plugins/copyright/lib/IndexedLineReaderTest.java
index a337027..cdbe089 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/copyright/lib/IndexedLineReaderTest.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/copyright/lib/IndexedLineReaderTest.java
@@ -4,7 +4,7 @@
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
-// http://www.apache.org/licenses/LICENSE-2.0
+// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
diff --git a/src/test/java/com/googlesource/gerrit/plugins/copyright/testdata/licenses/ANDROID.txt b/src/test/java/com/googlesource/gerrit/plugins/copyright/testdata/licenses/ANDROID.txt
index 71a183b..8ae0bfd 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/copyright/testdata/licenses/ANDROID.txt
+++ b/src/test/java/com/googlesource/gerrit/plugins/copyright/testdata/licenses/ANDROID.txt
@@ -1,3 +1,3 @@
/*
- * Copyright (C) 2019 Android Open-Source Project
+ * Copyright (C) 2019 Android Open-Source Project
*/
diff --git a/src/test/java/com/googlesource/gerrit/plugins/copyright/testdata/licenses/APACHE2.txt b/src/test/java/com/googlesource/gerrit/plugins/copyright/testdata/licenses/APACHE2.txt
index d645695..cb12cf1 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/copyright/testdata/licenses/APACHE2.txt
+++ b/src/test/java/com/googlesource/gerrit/plugins/copyright/testdata/licenses/APACHE2.txt
@@ -176,6 +176,7 @@
END OF TERMS AND CONDITIONS
+
APPENDIX: How to apply the Apache License to your work.
To apply the Apache License to your work, attach the following
diff --git a/src/test/java/com/googlesource/gerrit/plugins/copyright/testdata/licenses/BSD2.txt b/src/test/java/com/googlesource/gerrit/plugins/copyright/testdata/licenses/BSD2.txt
index 453967f..5b4d707 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/copyright/testdata/licenses/BSD2.txt
+++ b/src/test/java/com/googlesource/gerrit/plugins/copyright/testdata/licenses/BSD2.txt
@@ -4,6 +4,7 @@
Copyright (c) 2019, Jane Doe <jdoe@example.com>
All rights reserved.
+
Redistribution and use in source and binary forms, with or without modification,
are permitted provided that the following conditions are met:
diff --git a/src/test/java/com/googlesource/gerrit/plugins/copyright/testdata/licenses/MIT.txt b/src/test/java/com/googlesource/gerrit/plugins/copyright/testdata/licenses/MIT.txt
index af63709..2a1d858 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/copyright/testdata/licenses/MIT.txt
+++ b/src/test/java/com/googlesource/gerrit/plugins/copyright/testdata/licenses/MIT.txt
@@ -12,6 +12,7 @@
The above copyright notice and this permission notice shall be included in all
copies or substantial portions of the Software.
+
THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
diff --git a/src/test/java/com/googlesource/gerrit/plugins/copyright/testdata/licenses/UNKNOWN.txt b/src/test/java/com/googlesource/gerrit/plugins/copyright/testdata/licenses/UNKNOWN.txt
index 67a5ebe..87124eb 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/copyright/testdata/licenses/UNKNOWN.txt
+++ b/src/test/java/com/googlesource/gerrit/plugins/copyright/testdata/licenses/UNKNOWN.txt
@@ -1,4 +1,4 @@
#!/bin/sh
# Copyright (C) 2019
-# Jane Doe <jdoe@example.com>
+# Jane Doe <jdoe@example.com>
exit 0
diff --git a/src/test/java/com/googlesource/gerrit/plugins/copyright/tools/AndroidScanTest.sh b/src/test/java/com/googlesource/gerrit/plugins/copyright/tools/AndroidScanTest.sh
index cb3a3eb..0e8e5d0 100755
--- a/src/test/java/com/googlesource/gerrit/plugins/copyright/tools/AndroidScanTest.sh
+++ b/src/test/java/com/googlesource/gerrit/plugins/copyright/tools/AndroidScanTest.sh
@@ -6,7 +6,7 @@
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
-# http://www.apache.org/licenses/LICENSE-2.0
+# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
diff --git a/src/test/java/com/googlesource/gerrit/plugins/copyright/tools/ScanToolTest.sh b/src/test/java/com/googlesource/gerrit/plugins/copyright/tools/ScanToolTest.sh
index 89ab77e..1db9039 100755
--- a/src/test/java/com/googlesource/gerrit/plugins/copyright/tools/ScanToolTest.sh
+++ b/src/test/java/com/googlesource/gerrit/plugins/copyright/tools/ScanToolTest.sh
@@ -6,7 +6,7 @@
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
-# http://www.apache.org/licenses/LICENSE-2.0
+# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
diff --git a/tools/android_scan.sh b/tools/android_scan.sh
index a3a51b2..1771f97 100755
--- a/tools/android_scan.sh
+++ b/tools/android_scan.sh
@@ -6,7 +6,7 @@
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
-# http://www.apache.org/licenses/LICENSE-2.0
+# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
diff --git a/tools/check_new_config.sh b/tools/check_new_config.sh
index 01cb7a4..29a7b27 100755
--- a/tools/check_new_config.sh
+++ b/tools/check_new_config.sh
@@ -6,7 +6,7 @@
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
-# http://www.apache.org/licenses/LICENSE-2.0
+# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
@@ -34,7 +34,7 @@
echo
else
echo "The project.config full path must end with project.config -- not $(basename $2)"
- ehco
+ echo
fi
fi
}
