opendev/gerrit
Code Issues Proposed changes

Split off logging/ package from giant server package

Browse Source 
We want to keep the dependencies of the logging/ package as small as
possible. To avoid a dependency on
"//java/com/google/gerrit/reviewdb:server" remove the
RequestId.forChange(Change) and RequestId.forProject(Project.NameKey)
methods which each only had a single caller in production code. Instead
let these 2 callers call the RequestId constructor directly.

Change-Id: Ied6989eadabc1884212a0c0c8aafc7532e215324
Signed-off-by: Edwin Kempin <ekempin@google.com>
This commit is contained in:
Edwin Kempin 2018-09-06 16:12:54 +02:00
parent 218f1feb28
commit 87527aa16d
17 changed files with 45 additions and 21 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
java/com/google/gerrit/httpd/BUILD
View File

@ -16,6 +16,7 @@ java_library(
"//java/com/google/gerrit/server/audit",
"//java/com/google/gerrit/server/git/receive",
"//java/com/google/gerrit/server/ioutil",
"//java/com/google/gerrit/server/logging",
"//java/com/google/gerrit/server/restapi",
"//java/com/google/gerrit/util/cli",
"//java/com/google/gerrit/util/http",

1
java/com/google/gerrit/lucene/BUILD
View File

@ -33,6 +33,7 @@ java_library(
"//java/com/google/gerrit/lifecycle",
"//java/com/google/gerrit/reviewdb:server",
"//java/com/google/gerrit/server",
"//java/com/google/gerrit/server/logging",
"//lib:guava",
"//lib:gwtorm",
"//lib/flogger:api",

1
java/com/google/gerrit/server/BUILD
View File

@ -42,6 +42,7 @@ java_library(
"//java/com/google/gerrit/reviewdb:server",
"//java/com/google/gerrit/server/cache/serialize",
"//java/com/google/gerrit/server/ioutil",
"//java/com/google/gerrit/server/logging",
"//java/com/google/gerrit/server/util/git",
"//java/com/google/gerrit/util/cli",
"//java/com/google/gerrit/util/ssl",

1
java/com/google/gerrit/server/cache/h2/BUILD vendored
View File

@ -9,6 +9,7 @@ java_library(
"//java/com/google/gerrit/lifecycle",
"//java/com/google/gerrit/server",
"//java/com/google/gerrit/server/cache/serialize",
"//java/com/google/gerrit/server/logging",
"//lib:guava",
"//lib:h2",
"//lib/flogger:api",

1
java/com/google/gerrit/server/git/receive/BUILD
View File

@ -8,6 +8,7 @@ java_library(
"//java/com/google/gerrit/extensions:api",
"//java/com/google/gerrit/reviewdb:server",
"//java/com/google/gerrit/server",
"//java/com/google/gerrit/server/logging",
"//java/com/google/gerrit/util/cli",
"//lib:args4j",
"//lib:guava",

2
java/com/google/gerrit/server/git/receive/ReceiveCommits.java
View File

@ -582,7 +582,7 @@ class ReceiveCommits {
tracePushOption.isPresent(),
tracePushOption.orElse(null),
(tagName, traceId) -> addMessage(tagName + ": " + traceId))) {
traceContext.addTag(RequestId.Type.RECEIVE_ID, RequestId.forProject(project.getNameKey()));
traceContext.addTag(RequestId.Type.RECEIVE_ID, new RequestId(project.getNameKey().get()));
// Log the push options here, rather than in parsePushOptions(), so that they are included
// into the trace if tracing is enabled.

13
java/com/google/gerrit/server/logging/BUILD Normal file
View File

@ -0,0 +1,13 @@
java_library(
name = "logging",
srcs = glob(
["**/*.java"],
),
visibility = ["//visibility:public"],
deps = [
"//java/com/google/gerrit/common:annotations",
"//java/com/google/gerrit/common:server",
"//lib:guava",
"//lib/flogger:api",
],
)

12
java/com/google/gerrit/server/logging/RequestId.java
View File

@ -19,8 +19,6 @@ import com.google.common.hash.Hasher;
import com.google.common.hash.Hashing;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.common.TimeUtil;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.Project;
import java.net.InetAddress;
import java.net.UnknownHostException;
@ -52,21 +50,13 @@ public class RequestId {
return LoggingContext.getInstance().getTagsAsMap().keySet().stream().anyMatch(Type::isId);
}
public static RequestId forChange(Change c) {
return new RequestId(c.getId().toString());
}
public static RequestId forProject(Project.NameKey p) {
return new RequestId(p.toString());
}
private final String str;
public RequestId() {
this(null);
}
private RequestId(@Nullable String resourceId) {
public RequestId(@Nullable String resourceId) {
Hasher h = Hashing.murmur3_128().newHasher();
h.putLong(Thread.currentThread().getId()).putUnencodedChars(MACHINE_ID);
str =

2
java/com/google/gerrit/server/submit/MergeOp.java
View File

@ -457,7 +457,7 @@ public class MergeOp implements AutoCloseable {
this.caller = caller;
this.ts = TimeUtil.nowTs();
this.db = db;
this.submissionId = RequestId.forChange(change);
this.submissionId = new RequestId(change.getId().toString());
try (TraceContext traceContext =
TraceContext.open().addTag(RequestId.Type.SUBMISSION_ID, submissionId)) {

1
java/com/google/gerrit/sshd/BUILD
View File

@ -15,6 +15,7 @@ java_library(
"//java/com/google/gerrit/server/cache/h2",
"//java/com/google/gerrit/server/git/receive",
"//java/com/google/gerrit/server/ioutil",
"//java/com/google/gerrit/server/logging",
"//java/com/google/gerrit/server/restapi",
"//java/com/google/gerrit/server/schema",
"//java/com/google/gerrit/util/cli",

1
java/com/google/gerrit/testing/BUILD
View File

@ -29,6 +29,7 @@ java_library(
"//java/com/google/gerrit/server/audit",
"//java/com/google/gerrit/server/cache/h2",
"//java/com/google/gerrit/server/cache/mem",
"//java/com/google/gerrit/server/logging",
"//java/com/google/gerrit/server/restapi",
"//java/com/google/gerrit/server/schema",
"//lib:guava",

5
javatests/com/google/gerrit/acceptance/rest/BUILD
View File

@ -4,7 +4,10 @@ acceptance_tests(
srcs = glob(["*IT.java"]),
group = "rest_bindings",
labels = ["rest"],
deps = [":util"],
deps = [
":util",
"//java/com/google/gerrit/server/logging",
],
)
java_library(

1
javatests/com/google/gerrit/acceptance/ssh/BUILD
View File

@ -17,6 +17,7 @@ acceptance_tests(
vm_args = ["-Xmx512m"],
deps = [
":util",
"//java/com/google/gerrit/server/logging",
"//lib/commons:compress",
],
)

1
javatests/com/google/gerrit/server/BUILD
View File

@ -49,6 +49,7 @@ junit_tests(
"//java/com/google/gerrit/server/cache/testing",
"//java/com/google/gerrit/server/group/testing",
"//java/com/google/gerrit/server/ioutil",
"//java/com/google/gerrit/server/logging",
"//java/com/google/gerrit/server/project/testing:project-test-util",
"//java/com/google/gerrit/server/restapi",
"//java/com/google/gerrit/server/schema",

14
javatests/com/google/gerrit/server/notedb/ChangeNotesTest.java
View File

@ -426,7 +426,7 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
@Test
public void approvalsPostSubmit() throws Exception {
Change c = newChange();
RequestId submissionId = RequestId.forChange(c);
RequestId submissionId = submissionId(c);
ChangeUpdate update = newUpdate(c, changeOwner);
update.putApproval("Code-Review", (short) 1);
update.putApproval("Verified", (short) 1);
@ -461,7 +461,7 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
@Test
public void approvalsDuringSubmit() throws Exception {
Change c = newChange();
RequestId submissionId = RequestId.forChange(c);
RequestId submissionId = submissionId(c);
ChangeUpdate update = newUpdate(c, changeOwner);
update.putApproval("Code-Review", (short) 1);
update.putApproval("Verified", (short) 1);
@ -598,7 +598,7 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
@Test
public void submitRecords() throws Exception {
Change c = newChange();
RequestId submissionId = RequestId.forChange(c);
RequestId submissionId = submissionId(c);
ChangeUpdate update = newUpdate(c, changeOwner);
update.setSubjectForCommit("Submit patch set 1");
@ -640,7 +640,7 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
@Test
public void latestSubmitRecordsOnly() throws Exception {
Change c = newChange();
RequestId submissionId = RequestId.forChange(c);
RequestId submissionId = submissionId(c);
ChangeUpdate update = newUpdate(c, changeOwner);
update.setSubjectForCommit("Submit patch set 1");
update.merge(
@ -941,7 +941,7 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
// Finish off by merging the change.
update = newUpdate(c, changeOwner);
update.merge(
RequestId.forChange(c),
submissionId(c),
ImmutableList.of(
submitRecord(
"NOT_READY",
@ -3141,4 +3141,8 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
update.commit();
return tr.parseBody(commit);
}
private RequestId submissionId(Change c) {
return new RequestId(c.getId().toString());
}
}

8
javatests/com/google/gerrit/server/notedb/CommitMessageOutputTest.java
View File

@ -151,7 +151,7 @@ public class CommitMessageOutputTest extends AbstractChangeNotesTest {
ChangeUpdate update = newUpdate(c, changeOwner);
update.setSubjectForCommit("Submit patch set 1");
RequestId submissionId = RequestId.forChange(c);
RequestId submissionId = submissionId(c);
update.merge(
submissionId,
ImmutableList.of(
@ -220,7 +220,7 @@ public class CommitMessageOutputTest extends AbstractChangeNotesTest {
ChangeUpdate update = newUpdate(c, changeOwner);
update.setSubjectForCommit("Submit patch set 1");
RequestId submissionId = RequestId.forChange(c);
RequestId submissionId = submissionId(c);
update.merge(
submissionId, ImmutableList.of(submitRecord("RULE_ERROR", "Problem with patch set:\n1")));
update.commit();
@ -424,4 +424,8 @@ public class CommitMessageOutputTest extends AbstractChangeNotesTest {
RevCommit commit = parseCommit(commitId);
assertThat(commit.getFullMessage()).isEqualTo(expected);
}
private RequestId submissionId(Change c) {
return new RequestId(c.getId().toString());
}
}

1
plugins/BUILD
View File

@ -38,6 +38,7 @@ EXPORTS = [
"//java/com/google/gerrit/metrics/dropwizard",
"//java/com/google/gerrit/reviewdb:server",
"//java/com/google/gerrit/server/audit",
"//java/com/google/gerrit/server/logging",
"//java/com/google/gerrit/server/schema",
"//java/com/google/gerrit/util/http",
"//lib/commons:compress",