Merge branch 'stable-3.3' into stable-3.4

* stable-3.3:
  Make the indexing operation fail upon StorageException(s)
  ConsistencyCheckerIT: Delete calls to index broken changes that fail silently
  RefAdvertisementIT: Don't call reindex that would fail silently

Release-Notes: skip
Change-Id: I775de4baa70920c515aa040ff2222ec8052dd366

3 files changed, 9 insertions, 43 deletions

diff --git a/java/com/google/gerrit/index/Schema.java b/java/com/google/gerrit/index/Schema.java
index 3aa9de0614..ec14a154f4 100644
--- a/java/com/google/gerrit/index/Schema.java
+++ b/java/com/google/gerrit/index/Schema.java
@@ -207,21 +207,19 @@ public class Schema<T> {
   /**
    * Build all fields in the schema from an input object.
    *
-   * <p>Null values are omitted, as are fields which cause errors, which are logged.
+   * <p>Null values are omitted, as are fields which cause errors, which are logged. If any of the
+   * fields cause a StorageException, the whole operation fails and the exception is propagated to
+   * the caller.
    *
    * @param obj input object.
    * @param skipFields set of field names to skip when indexing the document
    * @return all non-null field values from the object.
    */
   public final Iterable<Values<T>> buildFields(T obj, ImmutableSet<String> skipFields) {
-    try {
-      return fields.values().stream()
-          .map(f -> fieldValues(obj, f, skipFields))
-          .filter(Objects::nonNull)
-          .collect(toImmutableList());
-    } catch (StorageException e) {
-      return ImmutableList.of();
-    }
+    return fields.values().stream()
+        .map(f -> fieldValues(obj, f, skipFields))
+        .filter(Objects::nonNull)
+        .collect(toImmutableList());
   }
 
   @Override
diff --git a/javatests/com/google/gerrit/acceptance/git/RefAdvertisementIT.java b/javatests/com/google/gerrit/acceptance/git/RefAdvertisementIT.java
index b7acbe22b0..16ccc356a9 100644
--- a/javatests/com/google/gerrit/acceptance/git/RefAdvertisementIT.java
+++ b/javatests/com/google/gerrit/acceptance/git/RefAdvertisementIT.java
@@ -38,7 +38,6 @@ import com.google.gerrit.common.Nullable;
 import com.google.gerrit.common.data.GlobalCapability;
 import com.google.gerrit.entities.AccessSection;
 import com.google.gerrit.entities.AccountGroup;
-import com.google.gerrit.entities.Change;
 import com.google.gerrit.entities.Patch;
 import com.google.gerrit.entities.PatchSet;
 import com.google.gerrit.entities.Permission;
@@ -69,7 +68,6 @@ import org.eclipse.jgit.junit.TestRepository;
 import org.eclipse.jgit.lib.Config;
 import org.eclipse.jgit.lib.Constants;
 import org.eclipse.jgit.lib.ObjectId;
-import org.eclipse.jgit.lib.PersonIdent;
 import org.eclipse.jgit.lib.Ref;
 import org.eclipse.jgit.lib.RefUpdate;
 import org.eclipse.jgit.lib.Repository;
@@ -1106,37 +1104,10 @@ public class RefAdvertisementIT extends AbstractDaemonTest {
 
   @Test
   public void receivePackOmitsMissingObject() throws Exception {
-    String rev = "deadbeefdeadbeefdeadbeefdeadbeefdeadbeef";
     try (Repository repo = repoManager.openRepository(project);
         TestRepository<Repository> tr = new TestRepository<>(repo)) {
-      String subject = "Subject for missing commit";
-      Change c = new Change(cd3.change());
-      PatchSet.Id psId = PatchSet.id(cd3.getId(), 2);
-      c.setCurrentPatchSet(psId, subject, c.getOriginalSubject());
-
-      PersonIdent committer = serverIdent.get();
-      PersonIdent author =
-          noteUtil.newAccountIdIdent(getAccount(admin.id()).id(), committer.getWhen(), committer);
-      tr.branch(RefNames.changeMetaRef(cd3.getId()))
-          .commit()
-          .author(author)
-          .committer(committer)
-          .message(
-              "Update patch set "
-                  + psId.get()
-                  + "\n"
-                  + "\n"
-                  + "Patch-set: "
-                  + psId.get()
-                  + "\n"
-                  + "Commit: "
-                  + rev
-                  + "\n"
-                  + "Subject: "
-                  + subject
-                  + "\n")
-          .create();
-      indexer.index(c.getProject(), c.getId());
+      PatchSet.Id psId = PatchSet.id(cd3.getId(), 1);
+      tr.delete(psId.toRefName());
     }
 
     assertThat(getReceivePackRefs().additionalHaves()).containsExactly(obj(cd4, 1));
diff --git a/javatests/com/google/gerrit/acceptance/server/change/ConsistencyCheckerIT.java b/javatests/com/google/gerrit/acceptance/server/change/ConsistencyCheckerIT.java
index e39f967ba1..9d821b75df 100644
--- a/javatests/com/google/gerrit/acceptance/server/change/ConsistencyCheckerIT.java
+++ b/javatests/com/google/gerrit/acceptance/server/change/ConsistencyCheckerIT.java
@@ -257,7 +257,6 @@ public class ConsistencyCheckerIT extends AbstractDaemonTest {
             + "Groups: "
             + rev
             + "\n");
-    indexer.index(c.getProject(), c.getId());
     ChangeNotes notes = changeNotesFactory.create(c.getProject(), c.getId());
 
     FixInput fix = new FixInput();
@@ -817,8 +816,6 @@ public class ConsistencyCheckerIT extends AbstractDaemonTest {
             + "Subject: "
             + subject
             + "\n");
-    indexer.index(c.getProject(), c.getId());
-
     return ps;
   }