From 82d99d7f6432e358f9df5a21fb6c0580ef0e34b2 Mon Sep 17 00:00:00 2001 From: David Pletcher Date: Mon, 4 May 2015 16:42:15 -0700 Subject: [PATCH] Pass commit IDs into DiffInfo This change enables logging file access by commit ID and path using the AuditListener interface. After a GetDiff REST request is processed, the AuditListener.onAuditableAction method is called with an ExtendedHttpAuditEvent parameter, which in turn references a DiffInfo object. However, the diff base commit ID is not available anywhere in this object graph. Left and right commit IDs are available in the PatchScriptBuilder object; that is the data we need. This change propagates the commit IDs through a series of intermediate data structures and ultimately makes them available as transient fields in the DiffInfo object. The AuditListener implementation will copy these commit values into the access log, along with the file path, timestamp and other typical logging fields. The new fields are marked transient--unsupported and undocumented. There are no anticipated side-effects. Change-Id: I43e5a14e8951442039bd2d8e941e2aa128e82447 --- .../google/gerrit/common/data/PatchScript.java | 15 ++++++++++++++- .../google/gerrit/extensions/common/DiffInfo.java | 2 ++ .../com/google/gerrit/server/change/GetDiff.java | 6 ++++++ .../gerrit/server/patch/PatchScriptBuilder.java | 4 +++- 4 files changed, 25 insertions(+), 2 deletions(-) diff --git a/gerrit-common/src/main/java/com/google/gerrit/common/data/PatchScript.java b/gerrit-common/src/main/java/com/google/gerrit/common/data/PatchScript.java index 046df1d4d9..7bab43ab96 100644 --- a/gerrit-common/src/main/java/com/google/gerrit/common/data/PatchScript.java +++ b/gerrit-common/src/main/java/com/google/gerrit/common/data/PatchScript.java @@ -57,6 +57,8 @@ public class PatchScript { protected boolean intralineFailure; protected boolean intralineTimeout; protected boolean binary; + protected transient String commitIdA; + protected transient String commitIdB; public PatchScript(final Change.Key ck, final ChangeType ct, final String on, final String nn, final FileMode om, final FileMode nm, @@ -65,7 +67,8 @@ public class PatchScript { final List e, final DisplayMethod ma, final DisplayMethod mb, final String mta, final String mtb, final CommentDetail cd, final List hist, final boolean hf, final boolean id, - final boolean idf, final boolean idt, boolean bin) { + final boolean idf, final boolean idt, boolean bin, + final String cma, final String cmb) { changeId = ck; changeType = ct; oldName = on; @@ -88,6 +91,8 @@ public class PatchScript { intralineFailure = idf; intralineTimeout = idt; binary = bin; + commitIdA = cma; + commitIdB = cmb; } protected PatchScript() { @@ -200,4 +205,12 @@ public class PatchScript { public boolean isBinary() { return binary; } + + public String getCommitIdA() { + return commitIdA; + } + + public String getCommitIdB() { + return commitIdB; + } } diff --git a/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/common/DiffInfo.java b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/common/DiffInfo.java index 62b1dc7a0f..58b2d390ab 100644 --- a/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/common/DiffInfo.java +++ b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/common/DiffInfo.java @@ -42,6 +42,8 @@ public class DiffInfo { } public static class FileMeta { + // The ID of the commit containing the file + public transient String commitId; // The name of the file public String name; // The content type of the file diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/GetDiff.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/GetDiff.java index 8e3a5d142d..eef0533b06 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/GetDiff.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/GetDiff.java @@ -201,6 +201,7 @@ public class GetDiff implements RestReadView { result.metaA.lines = ps.getA().size(); result.metaA.webLinks = getFileWebLinks(state.getProject(), revA, result.metaA.name); + result.metaA.commitId = content.commitIdA; } if (ps.getDisplayMethodB() != DisplayMethod.NONE) { @@ -211,6 +212,7 @@ public class GetDiff implements RestReadView { result.metaB.lines = ps.getB().size(); result.metaB.webLinks = getFileWebLinks(state.getProject(), revB, result.metaB.name); + result.metaB.commitId = content.commitIdB; } if (intraline) { @@ -264,6 +266,8 @@ public class GetDiff implements RestReadView { final SparseFileContent fileA; final SparseFileContent fileB; final boolean ignoreWS; + final String commitIdA; + final String commitIdB; int nextA; int nextB; @@ -273,6 +277,8 @@ public class GetDiff implements RestReadView { fileA = ps.getA(); fileB = ps.getB(); ignoreWS = ps.isIgnoreWhitespace(); + commitIdA = ps.getCommitIdA(); + commitIdB = ps.getCommitIdB(); } void addCommon(int end) { diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchScriptBuilder.java b/gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchScriptBuilder.java index edbd9ebc48..5b70730737 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchScriptBuilder.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchScriptBuilder.java @@ -214,7 +214,9 @@ class PatchScriptBuilder { a.displayMethod, b.displayMethod, a.mimeType.toString(), b.mimeType.toString(), comments, history, hugeFile, intralineDifferenceIsPossible, intralineFailure, intralineTimeout, - content.getPatchType() == Patch.PatchType.BINARY); + content.getPatchType() == Patch.PatchType.BINARY, + aId == null ? null : aId.getName(), + bId == null ? null : bId.getName()); } private static boolean isModify(PatchListEntry content) {