opendev/gerrit
Code Issues Proposed changes

FileInfo: Include size delta

Browse Source 
Reviewers are interested in the size of a change that was done to a
file. This is why for non-binary files the number of inserted/deleted
lines is shown. For binary files there is no indication of how big the
change was. With this change we compute for the file size delta in
bytes and return it in the FileInfo. The client can use this
information to show the file size increase/decrease in the file list
(not done in this change).

Please note that a field was added to PatchListEntry and hence the
diff cache must be flushed once.

Change-Id: I0252387099c724251b71480c36b52b7f8d46e713
Signed-off-by: Edwin Kempin <ekempin@google.com>
This commit is contained in:
Edwin Kempin
2015-10-08 15:53:20 +02:00
parent 0d9f089856
commit 640f984b54
5 changed files with 83 additions and 36 deletions
Download Patch File Download Diff File Expand all files Collapse all files

29
Documentation/rest-api-changes.txt
View File

@@ -405,29 +405,36 @@ default. Optional fields are:
},
"files": {
"gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeCache.java": {
"lines_deleted": 8
"lines_deleted": 8,
"size_delta": -412
},
"gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeDetailCache.java": {
"lines_inserted": 1
"lines_inserted": 1,
"size_delta": 23
},
"gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeScreen.java": {
"lines_inserted": 11,
"lines_deleted": 19
"lines_deleted": 19,
"size_delta": -298
},
"gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeTable.java": {
"lines_inserted": 23,
"lines_deleted": 20
"lines_deleted": 20,
"size_delta": 132
},
"gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/StarCache.java": {
"status": "D",
"lines_deleted": 139
"lines_deleted": 139,
"size_delta": -5512
},
"gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/StarredChanges.java": {
"status": "A",
"lines_inserted": 204
"lines_inserted": 204,
"size_delta": 8345
},
"gerrit-gwtui/src/main/java/com/google/gerrit/client/ui/Screen.java": {
"lines_deleted": 9
"lines_deleted": 9,
"size_delta": -343
}
}
}
@@ -3254,11 +3261,13 @@ sorted by file path.
{
"/COMMIT_MSG": {
"status": "A",
"lines_inserted": 7
"lines_inserted": 7,
"size_delta": 551
},
"gerrit-server/src/main/java/com/google/gerrit/server/project/RefControl.java": {
"lines_inserted": 5,
"lines_deleted": 3
"lines_deleted": 3,
"size_delta": 98
}
}
----
@@ -4199,6 +4208,8 @@ Not set for binary files or if no lines were inserted.
|`lines_deleted` |optional|
Number of deleted lines. +
Not set for binary files or if no lines were deleted.
|`size_delta` ||
Number of bytes by which the file size increased/decreased.
|=============================
[[fix-input]]

1
gerrit-extension-api/src/main/java/com/google/gerrit/extensions/common/FileInfo.java
View File

@@ -20,4 +20,5 @@ public class FileInfo {
public String oldPath;
public Integer linesInserted;
public Integer linesDeleted;
public long sizeDelta;
}

2
gerrit-server/src/main/java/com/google/gerrit/server/change/FileInfoJson.java
View File

@@ -63,6 +63,7 @@ public class FileInfoJson {
d.status = e.getChangeType() != Patch.ChangeType.MODIFIED
? e.getChangeType().getCode() : null;
d.oldPath = e.getOldName();
d.sizeDelta = e.getSizeDelta();
if (e.getPatchType() == Patch.PatchType.BINARY) {
d.binary = true;
} else {
@@ -76,6 +77,7 @@ public class FileInfoJson {
// when the file was rewritten and too little content survived. Write
// a single record with data from both sides.
d.status = Patch.ChangeType.REWRITE.getCode();
d.sizeDelta = o.sizeDelta;
if (o.binary != null && o.binary) {
d.binary = true;
}

46
gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchListEntry.java
View File

@@ -18,10 +18,12 @@ import static com.google.gerrit.server.ioutil.BasicSerialization.readBytes;
import static com.google.gerrit.server.ioutil.BasicSerialization.readEnum;
import static com.google.gerrit.server.ioutil.BasicSerialization.readString;
import static com.google.gerrit.server.ioutil.BasicSerialization.readVarInt32;
import static com.google.gerrit.server.ioutil.BasicSerialization.readFixInt64;
import static com.google.gerrit.server.ioutil.BasicSerialization.writeBytes;
import static com.google.gerrit.server.ioutil.BasicSerialization.writeEnum;
import static com.google.gerrit.server.ioutil.BasicSerialization.writeString;
import static com.google.gerrit.server.ioutil.BasicSerialization.writeVarInt32;
import static com.google.gerrit.server.ioutil.BasicSerialization.writeFixInt64;
import com.google.gerrit.reviewdb.client.Patch;
import com.google.gerrit.reviewdb.client.Patch.ChangeType;
@@ -49,7 +51,7 @@ public class PatchListEntry {
static PatchListEntry empty(final String fileName) {
return new PatchListEntry(ChangeType.MODIFIED, PatchType.UNIFIED, null,
fileName, EMPTY_HEADER, Collections.<Edit> emptyList(), 0, 0);
fileName, EMPTY_HEADER, Collections.<Edit> emptyList(), 0, 0, 0);
}
private final ChangeType changeType;
@@ -60,8 +62,9 @@ public class PatchListEntry {
private final List<Edit> edits;
private final int insertions;
private final int deletions;
private final long sizeDelta;
PatchListEntry(final FileHeader hdr, List<Edit> editList) {
PatchListEntry(FileHeader hdr, List<Edit> editList, long sizeDelta) {
changeType = toChangeType(hdr);
patchType = toPatchType(hdr);
@@ -106,12 +109,12 @@ public class PatchListEntry {
}
insertions = ins;
deletions = del;
this.sizeDelta = sizeDelta;
}
private PatchListEntry(final ChangeType changeType,
final PatchType patchType, final String oldName, final String newName,
final byte[] header, final List<Edit> edits, final int insertions,
final int deletions) {
private PatchListEntry(ChangeType changeType, PatchType patchType,
String oldName, String newName, byte[] header, List<Edit> edits,
int insertions, int deletions, long sizeDelta) {
this.changeType = changeType;
this.patchType = patchType;
this.oldName = oldName;
@@ -120,6 +123,7 @@ public class PatchListEntry {
this.edits = edits;
this.insertions = insertions;
this.deletions = deletions;
this.sizeDelta = sizeDelta;
}
int weigh() {
@@ -166,6 +170,10 @@ public class PatchListEntry {
return deletions;
}
public long getSizeDelta() {
return sizeDelta;
}
public List<String> getHeaderLines() {
final IntList m = RawParseUtils.lineMap(header, 0, header.length);
final List<String> headerLines = new ArrayList<>(m.size() - 1);
@@ -190,7 +198,7 @@ public class PatchListEntry {
return p;
}
void writeTo(final OutputStream out) throws IOException {
void writeTo(OutputStream out) throws IOException {
writeEnum(out, changeType);
writeEnum(out, patchType);
writeString(out, oldName);
@@ -198,6 +206,7 @@ public class PatchListEntry {
writeBytes(out, header);
writeVarInt32(out, insertions);
writeVarInt32(out, deletions);
writeFixInt64(out, sizeDelta);
writeVarInt32(out, edits.size());
for (final Edit e : edits) {
@@ -208,17 +217,18 @@ public class PatchListEntry {
}
}
static PatchListEntry readFrom(final InputStream in) throws IOException {
final ChangeType changeType = readEnum(in, ChangeType.values());
final PatchType patchType = readEnum(in, PatchType.values());
final String oldName = readString(in);
final String newName = readString(in);
final byte[] hdr = readBytes(in);
final int ins = readVarInt32(in);
final int del = readVarInt32(in);
static PatchListEntry readFrom(InputStream in) throws IOException {
ChangeType changeType = readEnum(in, ChangeType.values());
PatchType patchType = readEnum(in, PatchType.values());
String oldName = readString(in);
String newName = readString(in);
byte[] hdr = readBytes(in);
int ins = readVarInt32(in);
int del = readVarInt32(in);
long sizeDelta = readFixInt64(in);
final int editCount = readVarInt32(in);
final Edit[] editArray = new Edit[editCount];
int editCount = readVarInt32(in);
Edit[] editArray = new Edit[editCount];
for (int i = 0; i < editCount; i++) {
int beginA = readVarInt32(in);
int endA = readVarInt32(in);
@@ -228,7 +238,7 @@ public class PatchListEntry {
}
return new PatchListEntry(changeType, patchType, oldName, newName, hdr,
toList(editArray), ins, del);
toList(editArray), ins, del, sizeDelta);
}
private static List<Edit> toList(Edit[] l) {

41
gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchListLoader.java
View File

@@ -15,6 +15,8 @@
package com.google.gerrit.server.patch;
import static org.eclipse.jgit.lib.Constants.OBJ_BLOB;
import com.google.common.base.Function;
import com.google.common.base.Throwables;
import com.google.common.collect.FluentIterable;
@@ -59,6 +61,7 @@ import org.eclipse.jgit.revwalk.RevCommit;
import org.eclipse.jgit.revwalk.RevObject;
import org.eclipse.jgit.revwalk.RevTree;
import org.eclipse.jgit.revwalk.RevWalk;
import org.eclipse.jgit.treewalk.TreeWalk;
import org.eclipse.jgit.util.TemporaryBuffer;
import org.eclipse.jgit.util.io.DisabledOutputStream;
import org.slf4j.Logger;
@@ -194,8 +197,12 @@ public class PatchListLoader implements Callable<PatchList> {
DiffEntry diffEntry = diffEntries.get(i);
if (paths == null || paths.contains(diffEntry.getNewPath())
|| paths.contains(diffEntry.getOldPath())) {
FileHeader fh = toFileHeader(key, df, diffEntry);
entries.add(newEntry(aTree, fh));
long sizeDelta =
getFileSize(repo, reader, diffEntry.getNewPath(), bTree)
- getFileSize(repo, reader, diffEntry.getOldPath(), aTree);
entries.add(newEntry(aTree, fh, sizeDelta));
}
}
return new PatchList(a, b, againstParent,
@@ -203,6 +210,15 @@ public class PatchListLoader implements Callable<PatchList> {
}
}
private static long getFileSize(Repository repo, ObjectReader reader,
String path, RevTree t) throws IOException {
try (TreeWalk tw = TreeWalk.forPath(reader, path, t)) {
return tw != null
? repo.open(tw.getObjectId(0), OBJ_BLOB).getSize()
: 0;
}
}
private FileHeader toFileHeader(PatchListKey key,
final DiffFormatter diffFormatter, final DiffEntry diffEntry)
throws IOException {
@@ -267,32 +283,39 @@ public class PatchListLoader implements Callable<PatchList> {
Text bText = Text.forCommit(reader, bCommit);
byte[] rawHdr = hdr.toString().getBytes("UTF-8");
RawText aRawText = new RawText(aText.getContent());
RawText bRawText = new RawText(bText.getContent());
byte[] aContent = aText.getContent();
byte[] bContent = bText.getContent();
long sizeDelta = bContent.length - aContent.length;
RawText aRawText = new RawText(aContent);
RawText bRawText = new RawText(bContent);
EditList edits = new HistogramDiff().diff(cmp, aRawText, bRawText);
FileHeader fh = new FileHeader(rawHdr, edits, PatchType.UNIFIED);
return new PatchListEntry(fh, edits);
return new PatchListEntry(fh, edits, sizeDelta);
}
private PatchListEntry newEntry(RevTree aTree, FileHeader fileHeader) {
private PatchListEntry newEntry(RevTree aTree, FileHeader fileHeader,
long sizeDelta) {
final FileMode oldMode = fileHeader.getOldMode();
final FileMode newMode = fileHeader.getNewMode();
if (oldMode == FileMode.GITLINK || newMode == FileMode.GITLINK) {
return new PatchListEntry(fileHeader, Collections.<Edit> emptyList());
return new PatchListEntry(fileHeader, Collections.<Edit> emptyList(),
sizeDelta);
}
if (aTree == null // want combined diff
|| fileHeader.getPatchType() != PatchType.UNIFIED
|| fileHeader.getHunks().isEmpty()) {
return new PatchListEntry(fileHeader, Collections.<Edit> emptyList());
return new PatchListEntry(fileHeader, Collections.<Edit> emptyList(),
sizeDelta);
}
List<Edit> edits = fileHeader.toEditList();
if (edits.isEmpty()) {
return new PatchListEntry(fileHeader, Collections.<Edit> emptyList());
return new PatchListEntry(fileHeader, Collections.<Edit> emptyList(),
sizeDelta);
} else {
return new PatchListEntry(fileHeader, edits);
return new PatchListEntry(fileHeader, edits, sizeDelta);
}
}