Merge "Log and skip indexed fields that produce an error"

gerrit-lucene/src/main/java/com/google/gerrit/lucene/LuceneChangeIndex.java

@@ -42,6 +42,7 @@ import com.google.gerrit.server.index.FieldType;
 import com.google.gerrit.server.index.IndexExecutor;
 import com.google.gerrit.server.index.IndexRewriteImpl;
 import com.google.gerrit.server.index.Schema;
+import com.google.gerrit.server.index.Schema.Values;
 import com.google.gerrit.server.query.Predicate;
 import com.google.gerrit.server.query.QueryParseException;
 import com.google.gerrit.server.query.change.ChangeData;
@@ -409,14 +410,9 @@ public class LuceneChangeIndex implements ChangeIndex {
   private Document toDocument(ChangeData cd) throws IOException {
     try {
       Document result = new Document();
-      for (FieldDef<ChangeData, ?> f : schema.getFields().values()) {
+      for (Values<ChangeData> vs : schema.buildFields(cd, fillArgs)) {
-        if (f.isRepeatable()) {
+        if (vs.getValues() != null) {
-          add(result, f, (Iterable<?>) f.get(cd, fillArgs));
+          add(result, vs);
-        } else {
-          Object val = f.get(cd, fillArgs);
-          if (val != null) {
-            add(result, f, Collections.singleton(val));
-          }
         }
       }
       return result;
@@ -425,39 +421,40 @@ public class LuceneChangeIndex implements ChangeIndex {
     }
   }
 
-  private void add(Document doc, FieldDef<ChangeData, ?> f,
+  private void add(Document doc, Values<ChangeData> values)
-      Iterable<?> values) throws OrmException {
+      throws OrmException {
-    String name = f.getName();
+    String name = values.getField().getName();
-    Store store = store(f);
+    FieldType<?> type = values.getField().getType();
+    Store store = store(values.getField());
 
-    if (f.getType() == FieldType.INTEGER) {
+    if (type == FieldType.INTEGER) {
-      for (Object value : values) {
+      for (Object value : values.getValues()) {
         doc.add(new IntField(name, (Integer) value, store));
       }
-    } else if (f.getType() == FieldType.LONG) {
+    } else if (type == FieldType.LONG) {
-      for (Object value : values) {
+      for (Object value : values.getValues()) {
         doc.add(new LongField(name, (Long) value, store));
       }
-    } else if (f.getType() == FieldType.TIMESTAMP) {
+    } else if (type == FieldType.TIMESTAMP) {
-      for (Object v : values) {
+      for (Object value : values.getValues()) {
-        int t = QueryBuilder.toIndexTime((Timestamp) v);
+        int t = QueryBuilder.toIndexTime((Timestamp) value);
         doc.add(new IntField(name, t, store));
       }
-    } else if (f.getType() == FieldType.EXACT
+    } else if (type == FieldType.EXACT
-        || f.getType() == FieldType.PREFIX) {
+        || type == FieldType.PREFIX) {
-      for (Object value : values) {
+      for (Object value : values.getValues()) {
         doc.add(new StringField(name, (String) value, store));
       }
-    } else if (f.getType() == FieldType.FULL_TEXT) {
+    } else if (type == FieldType.FULL_TEXT) {
-      for (Object value : values) {
+      for (Object value : values.getValues()) {
         doc.add(new TextField(name, (String) value, store));
       }
-    } else if (f.getType() == FieldType.STORED_ONLY) {
+    } else if (type == FieldType.STORED_ONLY) {
-      for (Object value : values) {
+      for (Object value : values.getValues()) {
         doc.add(new StoredField(name, (byte[]) value));
       }
     } else {
-      throw QueryBuilder.badFieldType(f.getType());
+      throw QueryBuilder.badFieldType(type);
     }
   }
 

gerrit-server/src/main/java/com/google/gerrit/server/index/Schema.java

@@ -15,11 +15,41 @@
 package com.google.gerrit.server.index;
 
 import com.google.common.annotations.VisibleForTesting;
+import com.google.common.base.Function;
 import com.google.common.base.Objects;
+import com.google.common.base.Predicates;
+import com.google.common.collect.FluentIterable;
 import com.google.common.collect.ImmutableMap;
+import com.google.gerrit.server.index.FieldDef.FillArgs;
+import com.google.gwtorm.server.OrmException;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.util.Collections;
 
 /** Specific version of a secondary index schema. */
 public class Schema<T> {
+  private static final Logger log = LoggerFactory.getLogger(Schema.class);
+
+  public static class Values<T> {
+    private final FieldDef<T, ?> field;
+    private final Iterable<?> values;
+
+    private Values(FieldDef<T, ?> field, Iterable<?> values) {
+      this.field = field;
+      this.values = values;
+    }
+
+    public FieldDef<T, ?> getField() {
+      return field;
+    }
+
+    public Iterable<?> getValues() {
+      return values;
+    }
+  }
+
   private final boolean release;
   private final ImmutableMap<String, FieldDef<T, ?>> fields;
   private int version;
@@ -52,6 +82,41 @@ public class Schema<T> {
     return fields;
   }
 
+  /**
+   * Build all fields in the schema from an input object.
+   * <p>
+   * Null values are omitted, as are fields which cause errors, which are
+   * logged.
+   *
+   * @param obj input object.
+   * @param fillArgs arguments for filling fields.
+   * @return all non-null field values from the object.
+   */
+  public final Iterable<Values<T>> buildFields(
+      final T obj, final FillArgs fillArgs) {
+    return FluentIterable.from(fields.values())
+        .transform(new Function<FieldDef<T, ?>, Values<T>>() {
+          @Override
+          public Values<T> apply(FieldDef<T, ?> f) {
+            Object v;
+            try {
+              v = f.get(obj, fillArgs);
+            } catch (OrmException e) {
+              log.error(String.format("error getting field %s of %s",
+                  f.getName(), obj), e);
+              return null;
+            }
+            if (v == null) {
+              return null;
+            } else if (f.isRepeatable()) {
+              return new Values<T>(f, (Iterable<?>) v);
+            } else {
+              return new Values<T>(f, Collections.singleton(v));
+            }
+          }
+        }).filter(Predicates.notNull());
+  }
+
   @Override
   public String toString() {
     return Objects.toStringHelper(this)

gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeData.java

@@ -15,6 +15,7 @@
 package com.google.gerrit.server.query.change;
 
 import com.google.common.base.Function;
+import com.google.common.base.Objects;
 import com.google.common.collect.ArrayListMultimap;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ListMultimap;
@@ -461,4 +462,9 @@ public class ChangeData {
   public List<SubmitRecord> getSubmitRecords() {
     return submitRecords;
   }
+
+  @Override
+  public String toString() {
+    return Objects.toStringHelper(this).addValue(getId()).toString();
+  }
 }

gerrit-solr/src/main/java/com/google/gerrit/solr/SolrChangeIndex.java

@@ -29,12 +29,12 @@ import com.google.gerrit.server.config.GerritServerConfig;
 import com.google.gerrit.server.config.SitePaths;
 import com.google.gerrit.server.index.ChangeField;
 import com.google.gerrit.server.index.ChangeIndex;
-import com.google.gerrit.server.index.FieldDef;
 import com.google.gerrit.server.index.FieldDef.FillArgs;
 import com.google.gerrit.server.index.FieldType;
 import com.google.gerrit.server.index.IndexCollection;
 import com.google.gerrit.server.index.IndexRewriteImpl;
 import com.google.gerrit.server.index.Schema;
+import com.google.gerrit.server.index.Schema.Values;
 import com.google.gerrit.server.query.Predicate;
 import com.google.gerrit.server.query.QueryParseException;
 import com.google.gerrit.server.query.change.ChangeData;
@@ -285,12 +285,8 @@ class SolrChangeIndex implements ChangeIndex, LifecycleListener {
   private SolrInputDocument toDocument(ChangeData cd) throws IOException {
     try {
       SolrInputDocument result = new SolrInputDocument();
-      for (FieldDef<ChangeData, ?> f : schema.getFields().values()) {
+      for (Values<ChangeData> values : schema.buildFields(cd, fillArgs)) {
-        if (f.isRepeatable()) {
+        add(result, values);
-          add(result, f, (Iterable<?>) f.get(cd, fillArgs));
-        } else {
-          add(result, f, Collections.singleton(f.get(cd, fillArgs)));
-        }
       }
       return result;
     } catch (OrmException e) {
@@ -298,28 +294,31 @@ class SolrChangeIndex implements ChangeIndex, LifecycleListener {
     }
   }
 
-  private void add(SolrInputDocument doc, FieldDef<ChangeData, ?> f,
+  private void add(SolrInputDocument doc, Values<ChangeData> values)
-      Iterable<?> values) throws OrmException {
+      throws OrmException {
-    if (f.getType() == FieldType.INTEGER) {
+    String name = values.getField().getName();
-      for (Object value : values) {
+    FieldType<?> type = values.getField().getType();
-        doc.addField(f.getName(), (Integer) value);
+
+    if (type == FieldType.INTEGER) {
+      for (Object value : values.getValues()) {
+        doc.addField(name, (Integer) value);
       }
-    } else if (f.getType() == FieldType.LONG) {
+    } else if (type == FieldType.LONG) {
-      for (Object value : values) {
+      for (Object value : values.getValues()) {
-        doc.addField(f.getName(), (Long) value);
+        doc.addField(name, (Long) value);
       }
-    } else if (f.getType() == FieldType.TIMESTAMP) {
+    } else if (type == FieldType.TIMESTAMP) {
-      for (Object v : values) {
+      for (Object v : values.getValues()) {
-        doc.addField(f.getName(), QueryBuilder.toIndexTime((Timestamp) v));
+        doc.addField(name, QueryBuilder.toIndexTime((Timestamp) v));
       }
-    } else if (f.getType() == FieldType.EXACT
+    } else if (type == FieldType.EXACT
-        || f.getType() == FieldType.PREFIX
+        || type == FieldType.PREFIX
-        || f.getType() == FieldType.FULL_TEXT) {
+        || type == FieldType.FULL_TEXT) {
-      for (Object value : values) {
+      for (Object value : values.getValues()) {
-        doc.addField(f.getName(), (String) value);
+        doc.addField(name, (String) value);
       }
     } else {
-      throw QueryBuilder.badFieldType(f.getType());
+      throw QueryBuilder.badFieldType(type);
     }
   }