From 971a5f5aebe15c7d1d6754497d5482b32649d7f2 Mon Sep 17 00:00:00 2001 From: Edwin Kempin Date: Wed, 28 Oct 2015 10:50:39 +0100 Subject: [PATCH 1/2] FileInfo: Include file size Together with the size delta which is already contained in FileInfo, having the file size allows clients to show the file size increase/decrease as percentages. Change-Id: I7f462991ee26151b4e99da5599fcec47e36c3a44 Signed-off-by: Edwin Kempin --- Documentation/rest-api-changes.txt | 29 +++++++++++++------ .../gerrit/extensions/common/FileInfo.java | 1 + .../gerrit/server/change/FileInfoJson.java | 2 ++ .../gerrit/server/patch/PatchListEntry.java | 18 +++++++++--- .../gerrit/server/patch/PatchListKey.java | 2 +- .../gerrit/server/patch/PatchListLoader.java | 15 +++++----- 6 files changed, 46 insertions(+), 21 deletions(-) diff --git a/Documentation/rest-api-changes.txt b/Documentation/rest-api-changes.txt index 55764b1fa2..8ae56e5d59 100644 --- a/Documentation/rest-api-changes.txt +++ b/Documentation/rest-api-changes.txt @@ -414,35 +414,42 @@ default. Optional fields are: "files": { "gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeCache.java": { "lines_deleted": 8, - "size_delta": -412 + "size_delta": -412, + "size": 7782 }, "gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeDetailCache.java": { "lines_inserted": 1, - "size_delta": 23 + "size_delta": 23, + "size": 6762 }, "gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeScreen.java": { "lines_inserted": 11, "lines_deleted": 19, - "size_delta": -298 + "size_delta": -298, + "size": 47023 }, "gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeTable.java": { "lines_inserted": 23, "lines_deleted": 20, - "size_delta": 132 + "size_delta": 132, + "size": 17727 }, "gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/StarCache.java": { "status": "D", "lines_deleted": 139, - "size_delta": -5512 + "size_delta": -5512, + "size": 13098 }, "gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/StarredChanges.java": { "status": "A", "lines_inserted": 204, - "size_delta": 8345 + "size_delta": 8345, + "size": 8345 }, "gerrit-gwtui/src/main/java/com/google/gerrit/client/ui/Screen.java": { "lines_deleted": 9, - "size_delta": -343 + "size_delta": -343, + "size": 5385 } } } @@ -3270,12 +3277,14 @@ sorted by file path. "/COMMIT_MSG": { "status": "A", "lines_inserted": 7, - "size_delta": 551 + "size_delta": 551, + "size": 551 }, "gerrit-server/src/main/java/com/google/gerrit/server/project/RefControl.java": { "lines_inserted": 5, "lines_deleted": 3, - "size_delta": 98 + "size_delta": 98, + "size": 23348 } } ---- @@ -4218,6 +4227,8 @@ 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. +|`size` || +File size in bytes. |============================= [[fix-input]] diff --git a/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/common/FileInfo.java b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/common/FileInfo.java index 00d0c183f0..a81290848b 100644 --- a/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/common/FileInfo.java +++ b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/common/FileInfo.java @@ -21,4 +21,5 @@ public class FileInfo { public Integer linesInserted; public Integer linesDeleted; public long sizeDelta; + public long size; } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/FileInfoJson.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/FileInfoJson.java index 71974c4901..e31b11f476 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/FileInfoJson.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/FileInfoJson.java @@ -64,6 +64,7 @@ public class FileInfoJson { ? e.getChangeType().getCode() : null; d.oldPath = e.getOldName(); d.sizeDelta = e.getSizeDelta(); + d.size = e.getSize(); if (e.getPatchType() == Patch.PatchType.BINARY) { d.binary = true; } else { @@ -78,6 +79,7 @@ public class FileInfoJson { // a single record with data from both sides. d.status = Patch.ChangeType.REWRITE.getCode(); d.sizeDelta = o.sizeDelta; + d.size = o.size; if (o.binary != null && o.binary) { d.binary = true; } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchListEntry.java b/gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchListEntry.java index 5de28f3e87..d130175d0d 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchListEntry.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchListEntry.java @@ -51,7 +51,7 @@ public class PatchListEntry { static PatchListEntry empty(final String fileName) { return new PatchListEntry(ChangeType.MODIFIED, PatchType.UNIFIED, null, - fileName, EMPTY_HEADER, Collections. emptyList(), 0, 0, 0); + fileName, EMPTY_HEADER, Collections. emptyList(), 0, 0, 0, 0); } private final ChangeType changeType; @@ -62,11 +62,13 @@ public class PatchListEntry { private final List edits; private final int insertions; private final int deletions; + private final long size; private final long sizeDelta; // Note: When adding new fields, the serialVersionUID in PatchListKey must be // incremented so that entries from the cache are automatically invalidated. - PatchListEntry(FileHeader hdr, List editList, long sizeDelta) { + PatchListEntry(FileHeader hdr, List editList, long size, + long sizeDelta) { changeType = toChangeType(hdr); patchType = toPatchType(hdr); @@ -111,12 +113,13 @@ public class PatchListEntry { } insertions = ins; deletions = del; + this.size = size; this.sizeDelta = sizeDelta; } private PatchListEntry(ChangeType changeType, PatchType patchType, String oldName, String newName, byte[] header, List edits, - int insertions, int deletions, long sizeDelta) { + int insertions, int deletions, long size, long sizeDelta) { this.changeType = changeType; this.patchType = patchType; this.oldName = oldName; @@ -125,6 +128,7 @@ public class PatchListEntry { this.edits = edits; this.insertions = insertions; this.deletions = deletions; + this.size = size; this.sizeDelta = sizeDelta; } @@ -172,6 +176,10 @@ public class PatchListEntry { return deletions; } + public long getSize() { + return size; + } + public long getSizeDelta() { return sizeDelta; } @@ -208,6 +216,7 @@ public class PatchListEntry { writeBytes(out, header); writeVarInt32(out, insertions); writeVarInt32(out, deletions); + writeFixInt64(out, size); writeFixInt64(out, sizeDelta); writeVarInt32(out, edits.size()); @@ -227,6 +236,7 @@ public class PatchListEntry { byte[] hdr = readBytes(in); int ins = readVarInt32(in); int del = readVarInt32(in); + long size = readFixInt64(in); long sizeDelta = readFixInt64(in); int editCount = readVarInt32(in); @@ -240,7 +250,7 @@ public class PatchListEntry { } return new PatchListEntry(changeType, patchType, oldName, newName, hdr, - toList(editArray), ins, del, sizeDelta); + toList(editArray), ins, del, size, sizeDelta); } private static List toList(Edit[] l) { diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchListKey.java b/gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchListKey.java index 15277b25f3..0200fa5aef 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchListKey.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchListKey.java @@ -34,7 +34,7 @@ import java.io.ObjectOutputStream; import java.io.Serializable; public class PatchListKey implements Serializable { - static final long serialVersionUID = 18L; + static final long serialVersionUID = 19L; public static final BiMap WHITESPACE_TYPES = ImmutableBiMap.of( Whitespace.IGNORE_NONE, 'N', diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchListLoader.java b/gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchListLoader.java index aa2a8a8429..10ea61cf1c 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchListLoader.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchListLoader.java @@ -206,7 +206,7 @@ public class PatchListLoader implements Callable { getFileSize(repo, reader, e.getOldMode(), e.getOldPath(), aTree); long newSize = getFileSize(repo, reader, e.getNewMode(), e.getNewPath(), bTree); - entries.add(newEntry(aTree, fh, newSize - oldSize)); + entries.add(newEntry(aTree, fh, newSize, newSize - oldSize)); } } return new PatchList(a, b, againstParent, @@ -301,37 +301,38 @@ public class PatchListLoader implements Callable { byte[] rawHdr = hdr.toString().getBytes(UTF_8); byte[] aContent = aText.getContent(); byte[] bContent = bText.getContent(); + long size = bContent.length; 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, sizeDelta); + return new PatchListEntry(fh, edits, size, sizeDelta); } private PatchListEntry newEntry(RevTree aTree, FileHeader fileHeader, - long sizeDelta) { + long size, long sizeDelta) { final FileMode oldMode = fileHeader.getOldMode(); final FileMode newMode = fileHeader.getNewMode(); if (oldMode == FileMode.GITLINK || newMode == FileMode.GITLINK) { return new PatchListEntry(fileHeader, Collections. emptyList(), - sizeDelta); + size, sizeDelta); } if (aTree == null // want combined diff || fileHeader.getPatchType() != PatchType.UNIFIED || fileHeader.getHunks().isEmpty()) { return new PatchListEntry(fileHeader, Collections. emptyList(), - sizeDelta); + size, sizeDelta); } List edits = fileHeader.toEditList(); if (edits.isEmpty()) { return new PatchListEntry(fileHeader, Collections. emptyList(), - sizeDelta); + size, sizeDelta); } else { - return new PatchListEntry(fileHeader, edits, sizeDelta); + return new PatchListEntry(fileHeader, edits, size, sizeDelta); } } From 240d3c6fa45c077e9f01030613a28c8f84782656 Mon Sep 17 00:00:00 2001 From: Edwin Kempin Date: Wed, 28 Oct 2015 14:22:36 +0100 Subject: [PATCH 2/2] Show for binary files the file size increase/decrease as percentage Change-Id: I2003eb427141ed4bf72254659210f1c7b5e7dcdf Signed-off-by: Edwin Kempin --- .../google/gerrit/client/info/FileInfo.java | 5 ++++ .../com/google/gerrit/client/FormatUtil.java | 16 +++++++++++++ .../gerrit/client/change/ChangeConstants.java | 2 +- .../gerrit/client/change/FileTable.java | 24 +++++++++++++++++-- .../gerrit/client/changes/ChangeMessages.java | 2 ++ .../client/changes/ChangeMessages.properties | 1 + .../google/gerrit/client/FormatUtilTest.java | 14 +++++++++++ 7 files changed, 61 insertions(+), 3 deletions(-) diff --git a/gerrit-gwtui-common/src/main/java/com/google/gerrit/client/info/FileInfo.java b/gerrit-gwtui-common/src/main/java/com/google/gerrit/client/info/FileInfo.java index d95f9efb33..555e06eff0 100644 --- a/gerrit-gwtui-common/src/main/java/com/google/gerrit/client/info/FileInfo.java +++ b/gerrit-gwtui-common/src/main/java/com/google/gerrit/client/info/FileInfo.java @@ -34,6 +34,11 @@ public class FileInfo extends JavaScriptObject { // JSNI methods cannot have 'long' as a parameter type or a return type and // it's suggested to use double in this case: // http://www.gwtproject.org/doc/latest/DevGuideCodingBasicsJSNI.html#important + public final long size() { + return (long)_size(); + } + private final native double _size() /*-{ return this.size || 0; }-*/; + public final long sizeDelta() { return (long)_sizeDelta(); } diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/FormatUtil.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/FormatUtil.java index c77b71fe32..1c44ebc076 100644 --- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/FormatUtil.java +++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/FormatUtil.java @@ -14,6 +14,7 @@ package com.google.gerrit.client; +import com.google.gerrit.client.change.Resources; import com.google.gerrit.client.info.AccountInfo; import com.google.gerrit.client.info.AccountPreferencesInfo; import com.google.gerrit.reviewdb.client.Account; @@ -134,4 +135,19 @@ public class FormatUtil { + NumberFormat.getFormat("#.0").format(bytes / Math.pow(1024, exp)) + " " + "KMGTPE".charAt(exp - 1) + "iB"; } + + public static String formatPercentage(long size, long delta) { + if (size == 0) { + return Resources.C.notAvailable(); + } + return (delta > 0 ? "+" : "-") + formatAbsPercentage(size, delta); + } + + public static String formatAbsPercentage(long size, long delta) { + if (size == 0) { + return Resources.C.notAvailable(); + } + int p = Math.abs(Math.round(delta * 100 / size)); + return p + "%"; + } } diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/ChangeConstants.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/ChangeConstants.java index f5b26d16dc..63de3890aa 100644 --- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/ChangeConstants.java +++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/ChangeConstants.java @@ -16,7 +16,7 @@ package com.google.gerrit.client.change; import com.google.gwt.i18n.client.Constants; -interface ChangeConstants extends Constants { +public interface ChangeConstants extends Constants { String previousChange(); String nextChange(); String openChange(); diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/FileTable.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/FileTable.java index d1ca517681..0a52bb1d88 100644 --- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/FileTable.java +++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/FileTable.java @@ -16,6 +16,8 @@ package com.google.gerrit.client.change; import static com.google.gerrit.client.FormatUtil.formatAbsBytes; import static com.google.gerrit.client.FormatUtil.formatBytes; +import static com.google.gerrit.client.FormatUtil.formatPercentage; +import static com.google.gerrit.client.FormatUtil.formatAbsPercentage; import com.google.gerrit.client.Dispatcher; import com.google.gerrit.client.Gerrit; @@ -464,6 +466,7 @@ public class FileTable extends FlowPanel { private boolean hasNonBinaryFile; private int inserted; private int deleted; + private long binOldSize; private long bytesInserted; private long bytesDeleted; @@ -520,6 +523,7 @@ public class FileTable extends FlowPanel { private void computeInsertedDeleted() { inserted = 0; deleted = 0; + binOldSize = 0; bytesInserted = 0; bytesDeleted = 0; for (int i = 0; i < list.length(); i++) { @@ -531,6 +535,7 @@ public class FileTable extends FlowPanel { deleted += info.linesDeleted(); } else { hasBinaryFile = true; + binOldSize += info.size() - info.sizeDelta(); if (info.sizeDelta() >= 0) { bytesInserted += info.sizeDelta(); } else { @@ -771,6 +776,12 @@ public class FileTable extends FlowPanel { } } else if (info.binary()) { sb.append(formatBytes(info.sizeDelta())); + long oldSize = info.size() - info.sizeDelta(); + if (oldSize != 0) { + sb.append(" (") + .append(formatPercentage(oldSize, info.sizeDelta())) + .append(")"); + } } sb.closeTd(); } @@ -827,8 +838,17 @@ public class FileTable extends FlowPanel { if (hasNonBinaryFile) { sb.br(); } - sb.append(Util.M.patchTableSize_ModifyBinaryFiles( - formatAbsBytes(bytesInserted), formatAbsBytes(bytesDeleted))); + if (binOldSize != 0) { + sb.append(Util.M.patchTableSize_ModifyBinaryFilesWithPercentages( + formatAbsBytes(bytesInserted), + formatAbsPercentage(binOldSize, bytesInserted), + formatAbsBytes(bytesDeleted), + formatAbsPercentage(binOldSize, bytesDeleted))); + } else { + sb.append(Util.M.patchTableSize_ModifyBinaryFiles( + formatAbsBytes(bytesInserted), + formatAbsBytes(bytesDeleted))); + } } sb.closeTh(); diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeMessages.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeMessages.java index ef74a65e07..95f2824d49 100644 --- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeMessages.java +++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeMessages.java @@ -36,6 +36,8 @@ public interface ChangeMessages extends Messages { String patchTableSize_Modify(int insertions, int deletions); String patchTableSize_ModifyBinaryFiles(String bytesInserted, String bytesDeleted); + String patchTableSize_ModifyBinaryFilesWithPercentages(String bytesInserted, + String percentageInserted, String bytesDeleted, String percentageDeleted); String patchTableSize_LongModify(int insertions, int deletions); String patchTableSize_Lines(@PluralCount int insertions); diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeMessages.properties b/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeMessages.properties index 67ef2c3391..9e58c9faf9 100644 --- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeMessages.properties +++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeMessages.properties @@ -18,6 +18,7 @@ patchTableComments = {0} comments patchTableDrafts = {0} drafts patchTableSize_Modify = +{0}, -{1} patchTableSize_ModifyBinaryFiles = +{0}, -{1} +patchTableSize_ModifyBinaryFilesWithPercentages = +{0} (+{1}), -{2} (-{3}) patchTableSize_LongModify = {0} inserted, {1} deleted patchTableSize_Lines = {0} lines diff --git a/gerrit-gwtui/src/test/java/com/google/gerrit/client/FormatUtilTest.java b/gerrit-gwtui/src/test/java/com/google/gerrit/client/FormatUtilTest.java index 0f659c9a12..04dccdb12a 100644 --- a/gerrit-gwtui/src/test/java/com/google/gerrit/client/FormatUtilTest.java +++ b/gerrit-gwtui/src/test/java/com/google/gerrit/client/FormatUtilTest.java @@ -15,6 +15,7 @@ package com.google.gerrit.client; import static com.google.gerrit.client.FormatUtil.formatBytes; +import static com.google.gerrit.client.FormatUtil.formatPercentage; import static org.junit.Assert.assertEquals; import com.googlecode.gwt.test.GwtModule; @@ -45,4 +46,17 @@ public class FormatUtilTest extends GwtTest { assertEquals("-27 B", formatBytes(-27)); assertEquals("-1.7 MiB", formatBytes(-1728)); } + + @Test + public void testFormatPercentage() { + assertEquals("N/A", formatPercentage(0, 10)); + assertEquals("0%", formatPercentage(100, 0)); + assertEquals("+25%", formatPercentage(100, 25)); + assertEquals("-25%", formatPercentage(100, -25)); + assertEquals("+50%", formatPercentage(100, 50)); + assertEquals("-50%", formatPercentage(100, -50)); + assertEquals("+100%", formatPercentage(100, 100)); + assertEquals("-100%", formatPercentage(100, -100)); + assertEquals("+500%", formatPercentage(100, 500)); + } }