Merge "Enable streaming large files put to an edit"
diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/edit/ChangeEditIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/edit/ChangeEditIT.java
index 7c18cb2..126250e 100644
--- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/edit/ChangeEditIT.java
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/edit/ChangeEditIT.java
@@ -125,7 +125,7 @@
assertThat(modifier.createEdit(change, ps)).isEqualTo(RefUpdate.Result.NEW);
assertThat(
modifier.modifyFile(editUtil.byChange(change).get(), FILE_NAME,
- CONTENT_NEW)).isEqualTo(RefUpdate.Result.FORCED);
+ RestSession.newRawInput(CONTENT_NEW))).isEqualTo(RefUpdate.Result.FORCED);
editUtil.delete(editUtil.byChange(change).get());
assertThat(editUtil.byChange(change).isPresent()).isFalse();
}
@@ -136,7 +136,7 @@
.isEqualTo(RefUpdate.Result.NEW);
assertThat(
modifier.modifyFile(editUtil.byChange(change).get(), FILE_NAME,
- CONTENT_NEW2)).isEqualTo(RefUpdate.Result.FORCED);
+ RestSession.newRawInput(CONTENT_NEW2))).isEqualTo(RefUpdate.Result.FORCED);
editUtil.publish(editUtil.byChange(change).get());
assertThat(editUtil.byChange(change).isPresent()).isFalse();
}
@@ -148,7 +148,7 @@
RefUpdate.Result.NEW);
assertThat(
modifier.modifyFile(editUtil.byChange(change).get(), FILE_NAME,
- CONTENT_NEW)).isEqualTo(RefUpdate.Result.FORCED);
+ RestSession.newRawInput(CONTENT_NEW))).isEqualTo(RefUpdate.Result.FORCED);
Optional<ChangeEdit> edit = editUtil.byChange(change);
RestResponse r = adminSession.post(urlPublish());
assertThat(r.getStatusCode()).isEqualTo(SC_NO_CONTENT);
@@ -164,7 +164,7 @@
assertThat(modifier.createEdit(change, ps)).isEqualTo(RefUpdate.Result.NEW);
assertThat(
modifier.modifyFile(editUtil.byChange(change).get(), FILE_NAME,
- CONTENT_NEW)).isEqualTo(RefUpdate.Result.FORCED);
+ RestSession.newRawInput(CONTENT_NEW))).isEqualTo(RefUpdate.Result.FORCED);
Optional<ChangeEdit> edit = editUtil.byChange(change);
RestResponse r = adminSession.delete(urlEdit());
assertThat(r.getStatusCode()).isEqualTo(SC_NO_CONTENT);
@@ -177,7 +177,7 @@
assertThat(modifier.createEdit(change, ps)).isEqualTo(RefUpdate.Result.NEW);
assertThat(
modifier.modifyFile(editUtil.byChange(change).get(), FILE_NAME,
- CONTENT_NEW)).isEqualTo(RefUpdate.Result.FORCED);
+ RestSession.newRawInput(CONTENT_NEW))).isEqualTo(RefUpdate.Result.FORCED);
ChangeEdit edit = editUtil.byChange(change).get();
PatchSet current = getCurrentPatchSet(changeId);
assertThat(edit.getBasePatchSet().getPatchSetId()).isEqualTo(
@@ -200,7 +200,7 @@
assertThat(modifier.createEdit(change, ps)).isEqualTo(RefUpdate.Result.NEW);
assertThat(
modifier.modifyFile(editUtil.byChange(change).get(), FILE_NAME,
- CONTENT_NEW)).isEqualTo(RefUpdate.Result.FORCED);
+ RestSession.newRawInput(CONTENT_NEW))).isEqualTo(RefUpdate.Result.FORCED);
ChangeEdit edit = editUtil.byChange(change).get();
PatchSet current = getCurrentPatchSet(changeId);
assertThat(edit.getBasePatchSet().getPatchSetId()).isEqualTo(
@@ -223,7 +223,7 @@
public void updateExistingFile() throws Exception {
assertThat(modifier.createEdit(change, ps)).isEqualTo(RefUpdate.Result.NEW);
Optional<ChangeEdit> edit = editUtil.byChange(change);
- assertThat(modifier.modifyFile(edit.get(), FILE_NAME, CONTENT_NEW))
+ assertThat(modifier.modifyFile(edit.get(), FILE_NAME, RestSession.newRawInput(CONTENT_NEW)))
.isEqualTo(RefUpdate.Result.FORCED);
edit = editUtil.byChange(change);
assertByteArray(
@@ -240,7 +240,7 @@
assertThat(r.getStatusCode()).isEqualTo(SC_NO_CONTENT);
assertThat(modifier.createEdit(change, ps)).isEqualTo(RefUpdate.Result.NEW);
Optional<ChangeEdit> edit = editUtil.byChange(change);
- assertThat(modifier.modifyFile(edit.get(), FILE_NAME, CONTENT_NEW))
+ assertThat(modifier.modifyFile(edit.get(), FILE_NAME, RestSession.newRawInput(CONTENT_NEW)))
.isEqualTo(RefUpdate.Result.FORCED);
edit = editUtil.byChange(change);
EditInfo info = toEditInfo(false);
@@ -258,7 +258,7 @@
public void retrieveFilesInEdit() throws Exception {
assertThat(modifier.createEdit(change, ps)).isEqualTo(RefUpdate.Result.NEW);
Optional<ChangeEdit> edit = editUtil.byChange(change);
- assertThat(modifier.modifyFile(edit.get(), FILE_NAME, CONTENT_NEW))
+ assertThat(modifier.modifyFile(edit.get(), FILE_NAME, RestSession.newRawInput(CONTENT_NEW)))
.isEqualTo(RefUpdate.Result.FORCED);
EditInfo info = toEditInfo(true);
@@ -345,13 +345,13 @@
public void amendExistingFile() throws Exception {
assertThat(modifier.createEdit(change, ps)).isEqualTo(RefUpdate.Result.NEW);
Optional<ChangeEdit> edit = editUtil.byChange(change);
- assertThat(modifier.modifyFile(edit.get(), FILE_NAME, CONTENT_NEW))
+ assertThat(modifier.modifyFile(edit.get(), FILE_NAME, RestSession.newRawInput(CONTENT_NEW)))
.isEqualTo(RefUpdate.Result.FORCED);
edit = editUtil.byChange(change);
assertByteArray(
fileUtil.getContent(edit.get().getChange().getProject(), edit.get()
.getRevision().get(), FILE_NAME), CONTENT_NEW);
- assertThat(modifier.modifyFile(edit.get(), FILE_NAME, CONTENT_NEW2))
+ assertThat(modifier.modifyFile(edit.get(), FILE_NAME, RestSession.newRawInput(CONTENT_NEW2)))
.isEqualTo(RefUpdate.Result.FORCED);
edit = editUtil.byChange(change);
assertByteArray(
@@ -419,7 +419,7 @@
assertThat(adminSession.putRaw(urlEditFile(), in.content).getStatusCode())
.isEqualTo(SC_NO_CONTENT);
Optional<ChangeEdit> edit = editUtil.byChange(change);
- assertThat(modifier.modifyFile(edit.get(), FILE_NAME, CONTENT_NEW2))
+ assertThat(modifier.modifyFile(edit.get(), FILE_NAME, RestSession.newRawInput(CONTENT_NEW2)))
.isEqualTo(RefUpdate.Result.FORCED);
edit = editUtil.byChange(change);
RestResponse r = adminSession.get(urlEditFile());
@@ -449,7 +449,7 @@
public void addNewFile() throws Exception {
assertThat(modifier.createEdit(change, ps)).isEqualTo(RefUpdate.Result.NEW);
Optional<ChangeEdit> edit = editUtil.byChange(change);
- assertThat(modifier.modifyFile(edit.get(), FILE_NAME2, CONTENT_NEW))
+ assertThat(modifier.modifyFile(edit.get(), FILE_NAME2, RestSession.newRawInput(CONTENT_NEW)))
.isEqualTo(RefUpdate.Result.FORCED);
edit = editUtil.byChange(change);
assertByteArray(
@@ -461,13 +461,13 @@
public void addNewFileAndAmend() throws Exception {
assertThat(modifier.createEdit(change, ps)).isEqualTo(RefUpdate.Result.NEW);
Optional<ChangeEdit> edit = editUtil.byChange(change);
- assertThat(modifier.modifyFile(edit.get(), FILE_NAME2, CONTENT_NEW))
+ assertThat(modifier.modifyFile(edit.get(), FILE_NAME2, RestSession.newRawInput(CONTENT_NEW)))
.isEqualTo(RefUpdate.Result.FORCED);
edit = editUtil.byChange(change);
assertByteArray(
fileUtil.getContent(edit.get().getChange().getProject(), edit.get()
.getRevision().get(), FILE_NAME2), CONTENT_NEW);
- assertThat(modifier.modifyFile(edit.get(), FILE_NAME2, CONTENT_NEW2))
+ assertThat(modifier.modifyFile(edit.get(), FILE_NAME2, RestSession.newRawInput(CONTENT_NEW2)))
.isEqualTo(RefUpdate.Result.FORCED);
edit = editUtil.byChange(change);
assertByteArray(
@@ -482,7 +482,7 @@
modifier.modifyFile(
editUtil.byChange(change).get(),
FILE_NAME,
- CONTENT_OLD);
+ RestSession.newRawInput(CONTENT_OLD));
fail();
} catch (InvalidChangeOperationException e) {
assertThat(e.getMessage()).isEqualTo("no changes were made");
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeEdits.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeEdits.java
index 7d823c3..d3c07e0 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeEdits.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeEdits.java
@@ -16,7 +16,6 @@
import com.google.common.base.Optional;
import com.google.common.base.Strings;
-import com.google.common.io.ByteStreams;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.extensions.common.EditInfo;
import com.google.gerrit.extensions.registration.DynamicMap;
@@ -371,8 +370,10 @@
public Response<?> apply(ChangeEditResource rsrc, Input input)
throws AuthException, ResourceConflictException, IOException {
try {
- editModifier.modifyFile(rsrc.getChangeEdit(), rsrc.getPath(),
- ByteStreams.toByteArray(input.content.getInputStream()));
+ editModifier.modifyFile(
+ rsrc.getChangeEdit(),
+ rsrc.getPath(),
+ input.content);
} catch(InvalidChangeOperationException | IOException e) {
throw new ResourceConflictException(e.getMessage());
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/edit/ChangeEditModifier.java b/gerrit-server/src/main/java/com/google/gerrit/server/edit/ChangeEditModifier.java
index ad59093..a06e4e6 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/edit/ChangeEditModifier.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/edit/ChangeEditModifier.java
@@ -14,11 +14,16 @@
package com.google.gerrit.server.edit;
+import static com.google.common.base.Preconditions.checkNotNull;
import static com.google.gerrit.server.edit.ChangeEditUtil.editRefName;
import static com.google.gerrit.server.edit.ChangeEditUtil.editRefPrefix;
+import static org.eclipse.jgit.lib.Constants.OBJ_BLOB;
+import com.google.common.io.ByteStreams;
+import com.google.gerrit.common.Nullable;
import com.google.gerrit.common.TimeUtil;
import com.google.gerrit.extensions.restapi.AuthException;
+import com.google.gerrit.extensions.restapi.RawInput;
import com.google.gerrit.extensions.restapi.ResourceConflictException;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.PatchSet;
@@ -39,7 +44,6 @@
import org.eclipse.jgit.dircache.DirCacheEntry;
import org.eclipse.jgit.lib.BatchRefUpdate;
import org.eclipse.jgit.lib.CommitBuilder;
-import org.eclipse.jgit.lib.Constants;
import org.eclipse.jgit.lib.FileMode;
import org.eclipse.jgit.lib.NullProgressMonitor;
import org.eclipse.jgit.lib.ObjectId;
@@ -57,6 +61,7 @@
import org.eclipse.jgit.treewalk.TreeWalk;
import java.io.IOException;
+import java.io.InputStream;
import java.util.Map;
import java.util.TimeZone;
@@ -223,7 +228,7 @@
* @throws IOException
*/
public RefUpdate.Result modifyFile(ChangeEdit edit,
- String file, byte[] content) throws AuthException,
+ String file, RawInput content) throws AuthException,
InvalidChangeOperationException, IOException {
return modify(TreeOperation.CHANGE_ENTRY, edit, file, content);
}
@@ -261,7 +266,7 @@
}
private RefUpdate.Result modify(TreeOperation op,
- ChangeEdit edit, String file, byte[] content)
+ ChangeEdit edit, String file, @Nullable RawInput content)
throws AuthException, IOException, InvalidChangeOperationException {
if (!currentUser.get().isIdentifiedUser()) {
throw new AuthException("Authentication required");
@@ -283,7 +288,7 @@
RevCommit base = prevEdit.getParent(0);
base = rw.parseCommit(base);
ObjectId newTree = writeNewTree(op, rw, inserter,
- prevEdit, reader, file, content, base);
+ prevEdit, reader, file, toBlob(inserter, content), base);
if (ObjectId.equals(newTree, prevEdit.getTree())) {
throw new InvalidChangeOperationException("no changes were made");
}
@@ -301,6 +306,20 @@
}
}
+ private static ObjectId toBlob(ObjectInserter ins, @Nullable RawInput content)
+ throws IOException {
+ if (content == null) {
+ return null;
+ }
+
+ long len = content.getContentLength();
+ InputStream in = content.getInputStream();
+ if (len < 0) {
+ return ins.insert(OBJ_BLOB, ByteStreams.toByteArray(in));
+ }
+ return ins.insert(OBJ_BLOB, len, in);
+ }
+
private ObjectId createCommit(IdentifiedUser me, ObjectInserter inserter,
RevCommit prevEdit, RevCommit base, ObjectId tree) throws IOException {
CommitBuilder builder = new CommitBuilder();
@@ -330,75 +349,66 @@
private static ObjectId writeNewTree(TreeOperation op, RevWalk rw,
ObjectInserter ins, RevCommit prevEdit, ObjectReader reader,
- String fileName, byte[] content, RevCommit base)
+ String fileName, final @Nullable ObjectId content, RevCommit base)
throws IOException, InvalidChangeOperationException {
- DirCache newTree = createTree(reader, prevEdit);
- editTree(
- op,
- rw,
- base,
- newTree.editor(),
- ins,
- fileName,
- content);
- return newTree.writeTree(ins);
- }
-
- private static void editTree(TreeOperation op, RevWalk rw, RevCommit base,
- DirCacheEditor dce, ObjectInserter ins, String path, byte[] content)
- throws IOException, InvalidChangeOperationException {
+ DirCache newTree = readTree(reader, prevEdit);
+ DirCacheEditor dce = newTree.editor();
switch (op) {
case DELETE_ENTRY:
- dce.add(new DeletePath(path));
+ dce.add(new DeletePath(fileName));
break;
+
case CHANGE_ENTRY:
+ checkNotNull("new content required", content);
+ dce.add(new PathEdit(fileName) {
+ @Override
+ public void apply(DirCacheEntry ent) {
+ if (ent.getRawMode() == 0) {
+ ent.setFileMode(FileMode.REGULAR_FILE);
+ }
+ ent.setObjectId(content);
+ }
+ });
+ break;
+
case RESTORE_ENTRY:
- dce.add(getPathEdit(op, rw, base, path, ins, content));
+ TreeWalk tw = TreeWalk.forPath(
+ rw.getObjectReader(),
+ fileName,
+ base.getTree().getId());
+
+ // If the file does not exist in the base commit, try to restore it
+ // from the base's parent commit.
+ if (tw == null && base.getParentCount() == 1) {
+ tw = TreeWalk.forPath(rw.getObjectReader(), fileName,
+ rw.parseCommit(base.getParent(0)).getTree());
+ }
+ if (tw == null) {
+ throw new InvalidChangeOperationException(String.format(
+ "cannot restore path %s: missing in base revision %s",
+ fileName, base.abbreviate(8).name()));
+ }
+
+ final FileMode mode = tw.getFileMode(0);
+ final ObjectId oid = tw.getObjectId(0);
+ dce.add(new PathEdit(fileName) {
+ @Override
+ public void apply(DirCacheEntry ent) {
+ ent.setFileMode(mode);
+ ent.setObjectId(oid);
+ }
+ });
break;
}
dce.finish();
+ return newTree.writeTree(ins);
}
- private static PathEdit getPathEdit(TreeOperation op, RevWalk rw,
- RevCommit base, String path, ObjectInserter ins, byte[] content)
- throws IOException, InvalidChangeOperationException {
- final ObjectId oid = op == TreeOperation.CHANGE_ENTRY
- ? ins.insert(Constants.OBJ_BLOB, content)
- : getObjectIdForRestoreOperation(rw, base, path);
- return new PathEdit(path) {
- @Override
- public void apply(DirCacheEntry ent) {
- ent.setFileMode(FileMode.REGULAR_FILE);
- ent.setObjectId(oid);
- }
- };
- }
-
- private static ObjectId getObjectIdForRestoreOperation(RevWalk rw,
- RevCommit base, String path)
- throws IOException, InvalidChangeOperationException {
- TreeWalk tw = TreeWalk.forPath(rw.getObjectReader(), path,
- base.getTree().getId());
- // If the file does not exist in the base commit, try to restore it
- // from the base's parent commit.
- if (tw == null && base.getParentCount() == 1) {
- tw = TreeWalk.forPath(rw.getObjectReader(), path,
- rw.parseCommit(base.getParent(0)).getTree().getId());
- }
- if (tw == null) {
- throw new InvalidChangeOperationException(String.format(
- "cannot restore path %s: missing in base revision %s",
- path, base.abbreviate(8).name()));
- }
- return tw.getObjectId(0);
- }
-
- private static DirCache createTree(ObjectReader reader, RevCommit prevEdit)
+ private static DirCache readTree(ObjectReader reader, RevCommit prevEdit)
throws IOException {
DirCache dc = DirCache.newInCore();
DirCacheBuilder b = dc.builder();
- b.addTree(new byte[0], DirCacheEntry.STAGE_0, reader, prevEdit.getTree()
- .getId());
+ b.addTree(new byte[0], DirCacheEntry.STAGE_0, reader, prevEdit.getTree());
b.finish();
return dc;
}
