Revert "GetDiff endpoint: Return 404 Not Found if file does not exist"

Returing 404 breaks the inline editor. It's not possible to add files
to the edit, either files that already exist in the repository (but
not already in the change) or completely new files.

This reverts commit 76c3016bb8.

Bug: Issue 4478
Change-Id: I71bbbbf0e030069f62c38496c14884b30b266e2a
This commit is contained in:
David Pursehouse
2016-09-02 12:56:29 +09:00
parent a8aac9d763
commit 411b23659d
9 changed files with 52 additions and 53 deletions

View File

@@ -55,7 +55,6 @@ import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.BinaryResult;
import com.google.gerrit.extensions.restapi.ETagView;
import com.google.gerrit.extensions.restapi.ResourceConflictException;
import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
import com.google.gerrit.extensions.restapi.UnprocessableEntityException;
import com.google.gerrit.server.change.GetRevisionActions;
import com.google.gerrit.server.change.RevisionResource;
@@ -582,19 +581,6 @@ public class RevisionIT extends AbstractDaemonTest {
assertThat(diff.metaB).isNull();
}
@Test
public void diffNonExistingFile() throws Exception {
PushOneCommit.Result r = createChange();
exception.expect(ResourceNotFoundException.class);
exception.expectMessage("non-existing");
gApi.changes()
.id(r.getChangeId())
.revision(r.getCommit().name())
.file("non-existing")
.diff();
}
@Test
public void diffOnMergeCommitChange() throws Exception {
PushOneCommit.Result r = createMergeCommitChange("refs/for/master");

View File

@@ -49,7 +49,6 @@ import com.google.gerrit.server.git.LargeObjectException;
import com.google.gerrit.server.patch.PatchScriptFactory;
import com.google.gerrit.server.project.InvalidChangeOperationException;
import com.google.gerrit.server.project.NoSuchChangeException;
import com.google.gerrit.server.project.NoSuchFileException;
import com.google.gerrit.server.project.ProjectCache;
import com.google.gerrit.server.project.ProjectState;
import com.google.gwtorm.server.OrmException;
@@ -273,7 +272,7 @@ public class GetDiff implements RestReadView<FileResource> {
r.caching(CacheControl.PRIVATE(7, TimeUnit.DAYS));
}
return r;
} catch (NoSuchChangeException | NoSuchFileException e) {
} catch (NoSuchChangeException e) {
throw new ResourceNotFoundException(e.getMessage(), e);
} catch (LargeObjectException e) {
throw new ResourceConflictException(e.getMessage(), e);

View File

@@ -32,7 +32,6 @@ import com.google.gerrit.server.PatchLineCommentsUtil;
import com.google.gerrit.server.patch.PatchFile;
import com.google.gerrit.server.patch.PatchList;
import com.google.gerrit.server.patch.PatchListNotAvailableException;
import com.google.gerrit.server.project.NoSuchFileException;
import com.google.gwtorm.client.KeyUtil;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
@@ -147,7 +146,7 @@ public class CommentSender extends ReplyToChangeSender {
try {
currentFileData =
new PatchFile(repo, patchList, pk.get());
} catch (IOException | NoSuchFileException e) {
} catch (IOException e) {
log.warn(String.format(
"Cannot load %s from %s in %s",
pk.getFileName(),

View File

@@ -16,7 +16,6 @@ package com.google.gerrit.server.patch;
import com.google.gerrit.common.errors.NoSuchEntityException;
import com.google.gerrit.reviewdb.client.Patch;
import com.google.gerrit.server.project.NoSuchFileException;
import org.eclipse.jgit.errors.CorruptObjectException;
import org.eclipse.jgit.errors.IncorrectObjectTypeException;
@@ -45,7 +44,7 @@ public class PatchFile {
public PatchFile(final Repository repo, final PatchList patchList,
final String fileName) throws MissingObjectException,
IncorrectObjectTypeException, IOException, NoSuchFileException {
IncorrectObjectTypeException, IOException {
this.repo = repo;
this.entry = patchList.get(fileName);

View File

@@ -27,7 +27,6 @@ import static org.eclipse.jgit.lib.ObjectIdSerialization.writeNotNull;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.reviewdb.client.Patch;
import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gerrit.server.project.NoSuchFileException;
import org.eclipse.jgit.lib.AnyObjectId;
import org.eclipse.jgit.lib.ObjectId;
@@ -135,13 +134,10 @@ public class PatchList implements Serializable {
return r;
}
/** Find an entry by name. */
public PatchListEntry get(String fileName) throws NoSuchFileException {
int index = search(fileName);
if (0 <= index) {
return patches[index];
}
throw new NoSuchFileException(fileName);
/** Find an entry by name, returning an empty entry if not present. */
public PatchListEntry get(final String fileName) {
final int index = search(fileName);
return 0 <= index ? patches[index] : PatchListEntry.empty(fileName);
}
private int search(final String fileName) {

View File

@@ -46,6 +46,13 @@ import java.util.Collections;
import java.util.List;
public class PatchListEntry {
private static final byte[] EMPTY_HEADER = {};
static PatchListEntry empty(final String fileName) {
return new PatchListEntry(ChangeType.MODIFIED, PatchType.UNIFIED, null,
fileName, EMPTY_HEADER, Collections.<Edit> emptyList(), 0, 0, 0, 0);
}
private final ChangeType changeType;
private final PatchType patchType;
private final String oldName;

View File

@@ -44,7 +44,6 @@ import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.project.ChangeControl;
import com.google.gerrit.server.project.InvalidChangeOperationException;
import com.google.gerrit.server.project.NoSuchChangeException;
import com.google.gerrit.server.project.NoSuchFileException;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Provider;
import com.google.inject.assistedinject.Assisted;
@@ -191,7 +190,7 @@ public class PatchScriptFactory implements Callable<PatchScript> {
@Override
public PatchScript call() throws OrmException, NoSuchChangeException,
LargeObjectException, AuthException,
InvalidChangeOperationException, IOException, NoSuchFileException {
InvalidChangeOperationException, IOException {
if (parentNum < 0) {
validatePatchSetId(psa);
}

View File

@@ -1,23 +0,0 @@
// Copyright (C) 2016 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.server.project;
public class NoSuchFileException extends Exception {
private static final long serialVersionUID = 1L;
public NoSuchFileException(String fileName) {
super(fileName);
}
}

View File

@@ -0,0 +1,37 @@
// Copyright (C) 2009 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.server.patch;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertSame;
import static org.junit.Assert.assertTrue;
import com.google.gerrit.reviewdb.client.Patch;
import org.junit.Test;
public class PatchListEntryTest {
@Test
public void testEmpty1() {
final String name = "empty-file";
final PatchListEntry e = PatchListEntry.empty(name);
assertNull(e.getOldName());
assertEquals(name, e.getNewName());
assertSame(Patch.PatchType.UNIFIED, e.getPatchType());
assertSame(Patch.ChangeType.MODIFIED, e.getChangeType());
assertTrue(e.getEdits().isEmpty());
}
}