Add test to ensure change details don't require an index

With the new URL schema (project name + change number) we don't rely on
a working change index for serving change details anymore. This has the
benefit that we can still serve change details pages in case of an index
failure.

This commit adds a test for this case to ensure this functionality is
working as intended.

Change-Id: Id34af71d218e98989709dd1e7baa409da6ff7200
This commit is contained in:
Patrick Hiesel
2017-10-13 11:11:29 +02:00
parent 6908a8945e
commit 7c8599ed29
4 changed files with 123 additions and 1 deletions

View File

@@ -825,6 +825,25 @@ public abstract class AbstractDaemonTest {
}
}
protected AutoCloseable disableChangeIndex() {
disableChangeIndexWrites();
ChangeIndex searchIndex = changeIndexes.getSearchIndex();
if (!(searchIndex instanceof DisabledChangeIndex)) {
changeIndexes.setSearchIndex(new DisabledChangeIndex(searchIndex), false);
}
return new AutoCloseable() {
@Override
public void close() throws Exception {
enableChangeIndexWrites();
ChangeIndex searchIndex = changeIndexes.getSearchIndex();
if (searchIndex instanceof DisabledChangeIndex) {
changeIndexes.setSearchIndex(((DisabledChangeIndex) searchIndex).unwrap(), false);
}
}
};
}
protected static Gson newGson() {
return OutputFormat.JSON_COMPACT.newGson();
}

View File

@@ -0,0 +1,86 @@
// Copyright (C) 2017 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.acceptance;
import com.google.gerrit.index.QueryOptions;
import com.google.gerrit.index.Schema;
import com.google.gerrit.index.query.DataSource;
import com.google.gerrit.index.query.Predicate;
import com.google.gerrit.index.query.QueryParseException;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.Change.Id;
import com.google.gerrit.server.index.change.ChangeIndex;
import com.google.gerrit.server.query.change.ChangeData;
import java.io.IOException;
import java.util.Optional;
/**
* This class wraps an index and assumes the search index can't handle any queries. However, it does
* return the current schema as the assumption is that we need a search index for starting Gerrit in
* the first place and only later lose the index connection (making it so that we can't send
* requests there anymore).
*/
public class DisabledChangeIndex implements ChangeIndex {
private final ChangeIndex index;
public DisabledChangeIndex(ChangeIndex index) {
this.index = index;
}
public ChangeIndex unwrap() {
return index;
}
@Override
public Schema<ChangeData> getSchema() {
return index.getSchema();
}
@Override
public void close() {
index.close();
}
@Override
public void replace(ChangeData obj) throws IOException {
throw new UnsupportedOperationException("ChangeIndex is disabled");
}
@Override
public void delete(Id key) throws IOException {
throw new UnsupportedOperationException("ChangeIndex is disabled");
}
@Override
public void deleteAll() throws IOException {
throw new UnsupportedOperationException("ChangeIndex is disabled");
}
@Override
public DataSource<ChangeData> getSource(Predicate<ChangeData> p, QueryOptions opts)
throws QueryParseException {
throw new UnsupportedOperationException("ChangeIndex is disabled");
}
@Override
public void markReady(boolean ready) throws IOException {
throw new UnsupportedOperationException("ChangeIndex is disabled");
}
@Override
public Optional<ChangeData> get(Change.Id key, QueryOptions opts) throws IOException {
throw new UnsupportedOperationException("ChangeIndex is disabled");
}
}

View File

@@ -14,6 +14,7 @@
package com.google.gerrit.index;
import com.google.common.annotations.VisibleForTesting;
import com.google.common.collect.Lists;
import com.google.gerrit.extensions.events.LifecycleListener;
import java.util.Collection;
@@ -37,8 +38,13 @@ public abstract class IndexCollection<K, V, I extends Index<K, V>> implements Li
}
public void setSearchIndex(I index) {
setSearchIndex(index, true);
}
@VisibleForTesting
public void setSearchIndex(I index, boolean closeOld) {
I old = searchIndex.getAndSet(index);
if (old != null && old != index && !writeIndexes.contains(old)) {
if (closeOld && old != null && old != index && !writeIndexes.contains(old)) {
old.close();
}
}

View File

@@ -3631,6 +3631,17 @@ public class ChangeIT extends AbstractDaemonTest {
gApi.accounts().self().setStars(changeId, new StarsInput(ImmutableSet.of(invalidLabel)));
}
@Test
public void changeDetailsDoesNotRequireIndex() throws Exception {
PushOneCommit.Result change = createChange();
int number = gApi.changes().id(change.getChangeId()).get()._number;
try (AutoCloseable ctx = disableChangeIndex()) {
assertThat(gApi.changes().id(project.get(), number).get(ImmutableSet.of()).changeId)
.isEqualTo(change.getChangeId());
}
}
private static class ChangeIndexedCounter implements ChangeIndexedListener {
private final AtomicLongMap<Integer> countsByChange = AtomicLongMap.create();