opendev/gerrit
Code Issues Proposed changes

Extract DiffInfoCreator from GetDiff class

Browse Source 
Extract DiffInfoCreator for reuse in robot's preview fix.

Change-Id: I32e6f41007a38e0d758b209046cbd45c8cf6b49d
This commit is contained in:
Dmitrii Filippov
2019-10-02 16:41:00 +02:00
parent bd41629222
commit 0a0ff68a03
12 changed files with 957 additions and 415 deletions
Download Patch File Download Diff File Expand all files Collapse all files

292
java/com/google/gerrit/prettify/common/SparseFileContent.java
View File

@@ -14,160 +14,175 @@
package com.google.gerrit.prettify.common;
import java.util.ArrayList;
import java.util.List;
import com.google.auto.value.AutoValue;
import com.google.common.annotations.VisibleForTesting;
import com.google.common.collect.ImmutableList;
public class SparseFileContent {
protected List<Range> ranges;
protected int size;
/**
* A class to store subset of a file's lines in a memory efficient way. Internally, it stores lines
* as a list of ranges. Each range represents continuous set of lines and has information about line
* numbers in original file (zero-based).
*
* <p>{@link SparseFileContent.Accessor} must be used to work with the stored content.
*/
@AutoValue
public abstract class SparseFileContent {
abstract ImmutableList<Range> getRanges();
private transient int currentRangeIdx;
public abstract int getSize();
public SparseFileContent() {
ranges = new ArrayList<>();
public static SparseFileContent create(ImmutableList<Range> ranges, int size) {
return new AutoValue_SparseFileContent(ranges, size);
}
public int size() {
return size;
@VisibleForTesting
public int getRangesCount() {
return getRanges().size();
}
public void setSize(int s) {
size = s;
public Accessor createAccessor() {
return new Accessor(this);
}
public String get(int idx) {
final String line = getLine(idx);
if (line == null) {
throw new ArrayIndexOutOfBoundsException(idx);
}
return line;
}
/**
* Provide a methods to work with the content of a {@link SparseFileContent}.
*
* <p>The class hides internal representation of a {@link SparseFileContent} and provides
* convenient way for accessing a content.
*/
public static class Accessor {
private final SparseFileContent content;
private int currentRangeIdx;
public boolean contains(int idx) {
return getLine(idx) != null;
}
public int first() {
return ranges.isEmpty() ? size() : ranges.get(0).base;
}
public int next(int idx) {
// Most requests are sequential in nature, fetching the next
// line from the current range, or the immediate next range.
//
int high = ranges.size();
if (currentRangeIdx < high) {
Range cur = ranges.get(currentRangeIdx);
if (cur.contains(idx + 1)) {
return idx + 1;
}
if (++currentRangeIdx < high) {
// Its not plus one, its the base of the next range.
//
return ranges.get(currentRangeIdx).base;
}
private Accessor(SparseFileContent content) {
this.content = content;
}
// Binary search for the current value, since we know its a sorted list.
//
int low = 0;
do {
final int mid = (low + high) / 2;
final Range cur = ranges.get(mid);
public String get(int idx) {
final String line = getLine(idx);
if (line == null) {
throw new ArrayIndexOutOfBoundsException(idx);
}
return line;
}
if (cur.contains(idx)) {
public int getSize() {
return content.getSize();
}
public boolean contains(int idx) {
return getLine(idx) != null;
}
public int first() {
return content.getRanges().isEmpty() ? getSize() : content.getRanges().get(0).getBase();
}
public int next(int idx) {
// Most requests are sequential in nature, fetching the next
// line from the current range, or the immediate next range.
//
ImmutableList<Range> ranges = content.getRanges();
int high = ranges.size();
if (currentRangeIdx < high) {
Range cur = ranges.get(currentRangeIdx);
if (cur.contains(idx + 1)) {
// Trivial plus one case above failed due to wrong currentRangeIdx.
// Reset the cache so we don't miss in the future.
//
currentRangeIdx = mid;
return idx + 1;
}
if (mid + 1 < ranges.size()) {
// Its the base of the next range.
currentRangeIdx = mid + 1;
return ranges.get(currentRangeIdx).base;
}
// No more lines in the file.
//
return size();
}
if (idx < cur.base) {
high = mid;
} else {
low = mid + 1;
}
} while (low < high);
return size();
}
private String getLine(int idx) {
// Most requests are sequential in nature, fetching the next
// line from the current range, or the next range.
//
int high = ranges.size();
if (currentRangeIdx < high) {
Range cur = ranges.get(currentRangeIdx);
if (cur.contains(idx)) {
return cur.get(idx);
}
if (++currentRangeIdx < high) {
final Range next = ranges.get(currentRangeIdx);
if (next.contains(idx)) {
return next.get(idx);
if (++currentRangeIdx < high) {
// Its not plus one, its the base of the next range.
//
return ranges.get(currentRangeIdx).getBase();
}
}
// Binary search for the current value, since we know its a sorted list.
//
int low = 0;
do {
final int mid = (low + high) / 2;
final Range cur = ranges.get(mid);
if (cur.contains(idx)) {
if (cur.contains(idx + 1)) {
// Trivial plus one case above failed due to wrong currentRangeIdx.
// Reset the cache so we don't miss in the future.
//
currentRangeIdx = mid;
return idx + 1;
}
if (mid + 1 < ranges.size()) {
// Its the base of the next range.
currentRangeIdx = mid + 1;
return ranges.get(currentRangeIdx).getBase();
}
// No more lines in the file.
//
return getSize();
}
if (idx < cur.getBase()) {
high = mid;
} else {
low = mid + 1;
}
} while (low < high);
return getSize();
}
// Binary search for the range, since we know its a sorted list.
//
if (ranges.isEmpty()) {
private String getLine(int idx) {
// Most requests are sequential in nature, fetching the next
// line from the current range, or the next range.
//
ImmutableList<Range> ranges = content.getRanges();
int high = ranges.size();
if (currentRangeIdx < high) {
Range cur = ranges.get(currentRangeIdx);
if (cur.contains(idx)) {
return cur.get(idx);
}
if (++currentRangeIdx < high) {
final Range next = ranges.get(currentRangeIdx);
if (next.contains(idx)) {
return next.get(idx);
}
}
}
// Binary search for the range, since we know its a sorted list.
//
if (ranges.isEmpty()) {
return null;
}
int low = 0;
do {
final int mid = (low + high) / 2;
final Range cur = ranges.get(mid);
if (cur.contains(idx)) {
currentRangeIdx = mid;
return cur.get(idx);
}
if (idx < cur.getBase()) {
high = mid;
} else {
low = mid + 1;
}
} while (low < high);
return null;
}
int low = 0;
do {
final int mid = (low + high) / 2;
final Range cur = ranges.get(mid);
if (cur.contains(idx)) {
currentRangeIdx = mid;
return cur.get(idx);
}
if (idx < cur.base) {
high = mid;
} else {
low = mid + 1;
}
} while (low < high);
return null;
}
public void addLine(int i, String content) {
final Range r;
if (!ranges.isEmpty() && i == last().end()) {
r = last();
} else {
r = new Range(i);
ranges.add(r);
}
r.lines.add(content);
}
private Range last() {
return ranges.get(ranges.size() - 1);
}
@Override
public String toString() {
public final String toString() {
final StringBuilder b = new StringBuilder();
b.append("SparseFileContent[\n");
for (Range r : ranges) {
for (Range r : getRanges()) {
b.append(" ");
b.append(r.toString());
b.append('\n');
@@ -176,33 +191,32 @@ public class SparseFileContent {
return b.toString();
}
static class Range {
protected int base;
protected List<String> lines;
private Range(int b) {
base = b;
lines = new ArrayList<>();
@AutoValue
abstract static class Range {
static Range create(int base, ImmutableList<String> lines) {
return new AutoValue_SparseFileContent_Range(base, lines);
}
protected Range() {}
abstract int getBase();
abstract ImmutableList<String> getLines();
private String get(int i) {
return lines.get(i - base);
return getLines().get(i - getBase());
}
private int end() {
return base + lines.size();
return getBase() + getLines().size();
}
private boolean contains(int i) {
return base <= i && i < end();
return getBase() <= i && i < end();
}
@Override
public String toString() {
public final String toString() {
// Usage of [ and ) is intentional to denote inclusive/exclusive range
return "Range[" + base + "," + end() + ")";
return "Range[" + getBase() + "," + end() + ")";
}
}
}

90
java/com/google/gerrit/prettify/common/SparseFileContentBuilder.java Normal file
View File

@@ -0,0 +1,90 @@
// Copyright (C) 2019 The Android Open Source Project
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
package com.google.gerrit.prettify.common;
import com.google.common.collect.ImmutableList;
import com.google.gerrit.prettify.common.SparseFileContent.Range;
/**
* A builder for creating immutable {@link SparseFileContent}. Lines can be only be added in
* sequential (increased) order
*/
public class SparseFileContentBuilder {
private final ImmutableList.Builder<Range> ranges;
private final int size;
private int lastRangeBase;
private int lastRangeEnd;
private ImmutableList.Builder<String> lastRangeLines;
public SparseFileContentBuilder(int size) {
ranges = new ImmutableList.Builder<>();
startNextRange(0);
this.size = size;
}
public void addLine(int lineNumber, String content) {
if (lineNumber < 0) {
throw new IllegalArgumentException("Line number must be non-negative");
}
// if (lineNumber >= size) {
// The following 4 tests are failed if you uncomment this condition:
//
//
// diffOfFileWithMultilineRebaseHunkRemovingNewlineAtEndOfFileAndWithCommentReturnsFileContents
//
// diffOfFileWithMultilineRebaseHunkAddingNewlineAtEndOfFileAndWithCommentReturnsFileContents
//
//
// diffOfFileWithMultilineRebaseHunkRemovingNewlineAtEndOfFileAndWithCommentReturnsFileContents
//
// diffOfFileWithMultilineRebaseHunkAddingNewlineAtEndOfFileAndWithCommentReturnsFileContents
// Tests are failed because there are some bug with diff calculation.
// The condition must be uncommented after all these bugs are fixed.
// Also don't forget to remove ignore from for SparseFileContentBuilder
// throw new IllegalArgumentException(String.format("The zero-based line number %d is after
// the end of file. The file size is %d line(s).", lineNumber, size));
// }
if (lineNumber < lastRangeEnd) {
throw new IllegalArgumentException(
String.format(
"Invalid line number %d. You are trying to add a line before an already added line"
+ " %d",
lineNumber, lastRangeEnd));
}
if (lineNumber > lastRangeEnd) {
finishLastRange();
startNextRange(lineNumber);
}
lastRangeLines.add(content);
lastRangeEnd++;
}
private void startNextRange(int base) {
lastRangeLines = new ImmutableList.Builder<>();
lastRangeBase = lastRangeEnd = base;
}
private void finishLastRange() {
if (lastRangeEnd > lastRangeBase) {
ranges.add(Range.create(lastRangeBase, lastRangeLines.build()));
lastRangeLines = null;
}
}
public SparseFileContent build() {
finishLastRange();
return SparseFileContent.create(ranges.build(), size);
}
}

14
java/com/google/gerrit/prettify/common/testing/BUILD Normal file
View File

@@ -0,0 +1,14 @@
load("@rules_java//java:defs.bzl", "java_library")
package(default_testonly = True)
java_library(
name = "testing",
srcs = glob(["*.java"]),
visibility = ["//visibility:public"],
deps = [
"//java/com/google/gerrit/prettify:server",
"//lib:guava",
"//lib/truth",
],
)

65
java/com/google/gerrit/prettify/common/testing/SparseFileContentSubject.java Normal file
View File

@@ -0,0 +1,65 @@
// Copyright (C) 2019 The Android Open Source Project
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
package com.google.gerrit.prettify.common.testing;
import static com.google.common.truth.Truth.assertAbout;
import com.google.common.truth.FailureMetadata;
import com.google.common.truth.IntegerSubject;
import com.google.common.truth.MapSubject;
import com.google.common.truth.Subject;
import com.google.gerrit.prettify.common.SparseFileContent;
import java.util.HashMap;
import java.util.Map;
public class SparseFileContentSubject extends Subject {
public static SparseFileContentSubject assertThat(SparseFileContent sparseFileContent) {
return assertAbout(sparseFileContent()).that(sparseFileContent);
}
private final SparseFileContent sparseFileContent;
private SparseFileContentSubject(FailureMetadata metadata, SparseFileContent actual) {
super(metadata, actual);
this.sparseFileContent = actual;
}
private static Subject.Factory<SparseFileContentSubject, SparseFileContent> sparseFileContent() {
return SparseFileContentSubject::new;
}
public IntegerSubject getSize() {
isNotNull();
return check("size()").that(sparseFileContent.getSize());
}
public IntegerSubject getRangesCount() {
isNotNull();
return check("rangesCount()").that(sparseFileContent.getRangesCount());
}
public MapSubject lines() {
isNotNull();
Map<Integer, String> lines = new HashMap<>();
SparseFileContent.Accessor accessor = sparseFileContent.createAccessor();
int size = accessor.getSize();
int idx = accessor.first();
while (idx < size) {
lines.put(idx, accessor.get(idx));
idx = accessor.next(idx);
}
return check("lines()").that(lines);
}
}