Add change c/<number> to log.error output.
Change-Id: I3a7eaa05f169e7a1edb9320c5d97435653479de7
diff --git a/src/main/java/com/googlesource/gerrit/plugins/findowners/Action.java b/src/main/java/com/googlesource/gerrit/plugins/findowners/Action.java
index 45ec043..d232382 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/findowners/Action.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/findowners/Action.java
@@ -143,7 +143,7 @@
result.add(account.getPreferredEmail());
}
} catch (OrmException e) {
- log.error("Exception", e);
+ log.error("Exception for " + Config.getChangeId(changeData), e);
result = new ArrayList<>();
}
return result;
@@ -199,8 +199,9 @@
@Override
public Description getDescription(RevisionResource resource) {
Change change = resource.getChangeResource().getChange();
+ ChangeData changeData = null;
try (ReviewDb reviewDb = reviewDbProvider.open()) {
- ChangeData changeData = changeDataFactory.create(reviewDb, change);
+ changeData = changeDataFactory.create(reviewDb, change);
if (changeData.change().getDest().get() == null) {
if (!Checker.isExemptFromOwnerApproval(changeData)) {
log.error("Cannot get branch of change: " + changeData.getId().get());
@@ -228,7 +229,7 @@
.setTitle("Find owners to add to Reviewers list")
.setVisible(needFindOwners);
} catch (IOException | OrmException e) {
- log.error("Exception", e);
+ log.error("Exception for " + Config.getChangeId(changeData), e);
throw new IllegalStateException(e);
}
}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/findowners/Cache.java b/src/main/java/com/googlesource/gerrit/plugins/findowners/Cache.java
index 87e40f5..aa43896 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/findowners/Cache.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/findowners/Cache.java
@@ -144,7 +144,7 @@
}
});
} catch (ExecutionException e) {
- log.error("Cache.get has exception: " + e);
+ log.error("Cache.get has exception for " + Config.getChangeId(changeData), e);
return new OwnersDb(
accountCache, emails, key, repository, changeData, project, branch, files);
}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/findowners/Checker.java b/src/main/java/com/googlesource/gerrit/plugins/findowners/Checker.java
index 5d71c92..ae163d5 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/findowners/Checker.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/findowners/Checker.java
@@ -101,14 +101,15 @@
/** Returns 1 if owner approval is found, -1 if missing, 0 if unneeded. */
public static int findApproval(Prolog engine, int minVoteLevel) {
+ ChangeData changeData = null;
try {
+ changeData = StoredValues.CHANGE_DATA.get(engine);
AccountCache accountCache = StoredValues.ACCOUNT_CACHE.get(engine);
Emails emails = StoredValues.EMAILS.get(engine);
- ChangeData changeData = StoredValues.CHANGE_DATA.get(engine);
Repository repository = StoredValues.REPOSITORY.get(engine);
return new Checker(repository, changeData, minVoteLevel).findApproval(accountCache, emails);
} catch (OrmException | IOException e) {
- log.error("Exception", e);
+ log.error("Exception for " + Config.getChangeId(changeData), e);
return 0; // owner approval may or may not be required.
}
}
@@ -121,7 +122,7 @@
return true;
}
} catch (IOException | OrmException e) {
- log.error("Cannot get commit message", e);
+ log.error("Cannot get commit message for " + Config.getChangeId(changeData), e);
return true; // exempt from owner approval due to lack of data
}
// Abandoned and merged changes do not need approval again.
diff --git a/src/main/java/com/googlesource/gerrit/plugins/findowners/Config.java b/src/main/java/com/googlesource/gerrit/plugins/findowners/Config.java
index fdadc2b..e450dfb 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/findowners/Config.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/findowners/Config.java
@@ -87,7 +87,11 @@
return alwaysShowButton;
}
- static String getOwnersFileName(Project.NameKey project) {
+ static String getChangeId(ChangeData data) {
+ return data == null ? "(unknown change)" : ("change c/" + data.getId().get());
+ }
+
+ static String getOwnersFileName(Project.NameKey project, ChangeData c) {
if (config != null && project != null) {
try {
String name =
@@ -96,12 +100,19 @@
.getString(OWNERS_FILE_NAME, OWNERS);
if (name.trim().equals("")) {
log.error(
- "Project " + project.get() + " has wrong " + OWNERS_FILE_NAME + ": \"" + name + "\"");
+ "Project "
+ + project
+ + " has wrong "
+ + OWNERS_FILE_NAME
+ + ": \""
+ + name
+ + "\" for "
+ + getChangeId(c));
return OWNERS;
}
return name;
} catch (NoSuchProjectException e) {
- log.error("Cannot find project: " + project, e);
+ log.error("Cannot find project " + project + " for " + getChangeId(c), e);
}
}
return OWNERS;
@@ -121,7 +132,7 @@
.getFromProjectConfigWithInheritance(project, PLUGIN_NAME)
.getInt(MIN_OWNER_VOTE_LEVEL, minOwnerVoteLevel);
} catch (NoSuchProjectException e) {
- log.error("Cannot find project: " + project, e);
+ log.error("Cannot find project " + project + " for " + getChangeId(changeData), e);
return minOwnerVoteLevel;
}
}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/findowners/OwnersDb.java b/src/main/java/com/googlesource/gerrit/plugins/findowners/OwnersDb.java
index d551339..2c595b2 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/findowners/OwnersDb.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/findowners/OwnersDb.java
@@ -75,7 +75,7 @@
this.emails = emails;
this.key = key;
preferredEmails.put("*", "*");
- String ownersFileName = Config.getOwnersFileName(project);
+ String ownersFileName = Config.getOwnersFileName(project, changeData);
// Some hacked CL could have a target branch that is not created yet.
ObjectId id = getBranchId(repository, branch, changeData);
revision = "";
@@ -101,7 +101,7 @@
try {
revision = repository.getRef(branch).getObjectId().getName();
} catch (Exception e) {
- log.error("Fail to get branch revision", e);
+ log.error("Fail to get branch revision for " + Config.getChangeId(changeData), e);
}
}
countNumOwners(files);
@@ -293,11 +293,11 @@
try {
ObjectId id = repo.resolve(branch);
if (id == null && changeData != null && !Checker.isExemptFromOwnerApproval(changeData)) {
- log.error("cannot find branch " + branch);
+ log.error("cannot find branch " + branch + " for " + Config.getChangeId(changeData));
}
return id;
} catch (Exception e) {
- log.error("cannot find branch " + branch, e);
+ log.error("cannot find branch " + branch + " for " + Config.getChangeId(changeData), e);
}
return null;
}
diff --git a/src/test/java/com/googlesource/gerrit/plugins/findowners/FindOwnersIT.java b/src/test/java/com/googlesource/gerrit/plugins/findowners/FindOwnersIT.java
index ab8a7ea..160d5fa 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/findowners/FindOwnersIT.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/findowners/FindOwnersIT.java
@@ -259,9 +259,9 @@
PushOneCommit.Result cB = createChange("add tB.c", "tB.c", "Hello B!");
// Default owners file name is "OWNERS".
- assertThat(Config.getOwnersFileName(null)).isEqualTo("OWNERS");
- assertThat(Config.getOwnersFileName(pA)).isEqualTo("OWNERS");
- assertThat(Config.getOwnersFileName(pB)).isEqualTo("OWNERS");
+ assertThat(Config.getOwnersFileName(null, null)).isEqualTo("OWNERS");
+ assertThat(Config.getOwnersFileName(pA, null)).isEqualTo("OWNERS");
+ assertThat(Config.getOwnersFileName(pB, null)).isEqualTo("OWNERS");
String ownerX = oneOwnerList("x@x");
String ownerY = oneOwnerList("y@y");
@@ -273,8 +273,8 @@
setProjectConfig("ownersFileName", "OWNERS.alpha");
switchProject(pB);
setProjectConfig("ownersFileName", "OWNERS.beta");
- assertThat(Config.getOwnersFileName(pA)).isEqualTo("OWNERS.alpha");
- assertThat(Config.getOwnersFileName(pB)).isEqualTo("OWNERS.beta");
+ assertThat(Config.getOwnersFileName(pA, null)).isEqualTo("OWNERS.alpha");
+ assertThat(Config.getOwnersFileName(pB, null)).isEqualTo("OWNERS.beta");
String ownerA = oneOwnerList("a@a");
String ownerB = oneOwnerList("b@b");
assertThat(getOwnersResponse(cA)).contains(ownerA + ", files:[ tA.c ]");
@@ -283,24 +283,24 @@
// Change back to OWNERS in Project_A
switchProject(pA);
setProjectConfig("ownersFileName", "OWNERS");
- assertThat(Config.getOwnersFileName(pA)).isEqualTo("OWNERS");
+ assertThat(Config.getOwnersFileName(pA, null)).isEqualTo("OWNERS");
assertThat(getOwnersResponse(cA)).contains(ownerX + ", files:[ tA.c ]");
assertThat(getOwnersResponse(cB)).contains(ownerB + ", files:[ tB.c ]");
// Change back to OWNERS.alpha in Project_B, but there is no OWNERS.alpha
switchProject(pB);
setProjectConfig("ownersFileName", "OWNERS.alpha");
- assertThat(Config.getOwnersFileName(pB)).isEqualTo("OWNERS.alpha");
+ assertThat(Config.getOwnersFileName(pB, null)).isEqualTo("OWNERS.alpha");
assertThat(getOwnersResponse(cA)).contains(ownerX + ", files:[ tA.c ]");
assertThat(getOwnersResponse(cB)).contains("owners:[], files:[ tB.c ]");
// Do not accept empty string or all-white-spaces for ownersFileName.
setProjectConfig("ownersFileName", " ");
- assertThat(Config.getOwnersFileName(pB)).isEqualTo("OWNERS");
+ assertThat(Config.getOwnersFileName(pB, null)).isEqualTo("OWNERS");
setProjectConfig("ownersFileName", " \t ");
- assertThat(Config.getOwnersFileName(pB)).isEqualTo("OWNERS");
+ assertThat(Config.getOwnersFileName(pB, null)).isEqualTo("OWNERS");
setProjectConfig("ownersFileName", "O");
- assertThat(Config.getOwnersFileName(pB)).isEqualTo("O");
+ assertThat(Config.getOwnersFileName(pB, null)).isEqualTo("O");
}
@Test
