Log and skip indexed fields that produce an error
Change-Id: I40b8598b3e857faec7269bc380ac5ecbb78e2df7
diff --git a/gerrit-lucene/src/main/java/com/google/gerrit/lucene/LuceneChangeIndex.java b/gerrit-lucene/src/main/java/com/google/gerrit/lucene/LuceneChangeIndex.java
index 382716e..29c82236 100644
--- a/gerrit-lucene/src/main/java/com/google/gerrit/lucene/LuceneChangeIndex.java
+++ b/gerrit-lucene/src/main/java/com/google/gerrit/lucene/LuceneChangeIndex.java
@@ -42,6 +42,7 @@
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 @@
private Document toDocument(ChangeData cd) throws IOException {
try {
Document result = new Document();
- for (FieldDef<ChangeData, ?> f : schema.getFields().values()) {
- if (f.isRepeatable()) {
- add(result, f, (Iterable<?>) f.get(cd, fillArgs));
- } else {
- Object val = f.get(cd, fillArgs);
- if (val != null) {
- add(result, f, Collections.singleton(val));
- }
+ for (Values<ChangeData> vs : schema.buildFields(cd, fillArgs)) {
+ if (vs.getValues() != null) {
+ add(result, vs);
}
}
return result;
@@ -425,39 +421,40 @@
}
}
- private void add(Document doc, FieldDef<ChangeData, ?> f,
- Iterable<?> values) throws OrmException {
- String name = f.getName();
- Store store = store(f);
+ private void add(Document doc, Values<ChangeData> values)
+ throws OrmException {
+ String name = values.getField().getName();
+ FieldType<?> type = values.getField().getType();
+ Store store = store(values.getField());
- if (f.getType() == FieldType.INTEGER) {
- for (Object value : values) {
+ if (type == FieldType.INTEGER) {
+ for (Object value : values.getValues()) {
doc.add(new IntField(name, (Integer) value, store));
}
- } else if (f.getType() == FieldType.LONG) {
- for (Object value : values) {
+ } else if (type == FieldType.LONG) {
+ for (Object value : values.getValues()) {
doc.add(new LongField(name, (Long) value, store));
}
- } else if (f.getType() == FieldType.TIMESTAMP) {
- for (Object v : values) {
- int t = QueryBuilder.toIndexTime((Timestamp) v);
+ } else if (type == FieldType.TIMESTAMP) {
+ for (Object value : values.getValues()) {
+ int t = QueryBuilder.toIndexTime((Timestamp) value);
doc.add(new IntField(name, t, store));
}
- } else if (f.getType() == FieldType.EXACT
- || f.getType() == FieldType.PREFIX) {
- for (Object value : values) {
+ } else if (type == FieldType.EXACT
+ || type == FieldType.PREFIX) {
+ for (Object value : values.getValues()) {
doc.add(new StringField(name, (String) value, store));
}
- } else if (f.getType() == FieldType.FULL_TEXT) {
- for (Object value : values) {
+ } else if (type == FieldType.FULL_TEXT) {
+ for (Object value : values.getValues()) {
doc.add(new TextField(name, (String) value, store));
}
- } else if (f.getType() == FieldType.STORED_ONLY) {
- for (Object value : values) {
+ } else if (type == FieldType.STORED_ONLY) {
+ for (Object value : values.getValues()) {
doc.add(new StoredField(name, (byte[]) value));
}
} else {
- throw QueryBuilder.badFieldType(f.getType());
+ throw QueryBuilder.badFieldType(type);
}
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/index/Schema.java b/gerrit-server/src/main/java/com/google/gerrit/server/index/Schema.java
index ee200f6..e1262cd 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/index/Schema.java
+++ b/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 @@
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)
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeData.java b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeData.java
index 4522fe6..2efb611 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeData.java
+++ b/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;
@@ -463,4 +464,9 @@
public List<SubmitRecord> getSubmitRecords() {
return submitRecords;
}
+
+ @Override
+ public String toString() {
+ return Objects.toStringHelper(this).addValue(getId()).toString();
+ }
}
diff --git a/gerrit-solr/src/main/java/com/google/gerrit/solr/SolrChangeIndex.java b/gerrit-solr/src/main/java/com/google/gerrit/solr/SolrChangeIndex.java
index 79e6c7e..00fcc59 100644
--- a/gerrit-solr/src/main/java/com/google/gerrit/solr/SolrChangeIndex.java
+++ b/gerrit-solr/src/main/java/com/google/gerrit/solr/SolrChangeIndex.java
@@ -29,12 +29,12 @@
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 @@
private SolrInputDocument toDocument(ChangeData cd) throws IOException {
try {
SolrInputDocument result = new SolrInputDocument();
- for (FieldDef<ChangeData, ?> f : schema.getFields().values()) {
- if (f.isRepeatable()) {
- add(result, f, (Iterable<?>) f.get(cd, fillArgs));
- } else {
- add(result, f, Collections.singleton(f.get(cd, fillArgs)));
- }
+ for (Values<ChangeData> values : schema.buildFields(cd, fillArgs)) {
+ add(result, values);
}
return result;
} catch (OrmException e) {
@@ -298,28 +294,31 @@
}
}
- private void add(SolrInputDocument doc, FieldDef<ChangeData, ?> f,
- Iterable<?> values) throws OrmException {
- if (f.getType() == FieldType.INTEGER) {
- for (Object value : values) {
- doc.addField(f.getName(), (Integer) value);
+ private void add(SolrInputDocument doc, Values<ChangeData> values)
+ throws OrmException {
+ String name = values.getField().getName();
+ FieldType<?> type = values.getField().getType();
+
+ if (type == FieldType.INTEGER) {
+ for (Object value : values.getValues()) {
+ doc.addField(name, (Integer) value);
}
- } else if (f.getType() == FieldType.LONG) {
- for (Object value : values) {
- doc.addField(f.getName(), (Long) value);
+ } else if (type == FieldType.LONG) {
+ for (Object value : values.getValues()) {
+ doc.addField(name, (Long) value);
}
- } else if (f.getType() == FieldType.TIMESTAMP) {
- for (Object v : values) {
- doc.addField(f.getName(), QueryBuilder.toIndexTime((Timestamp) v));
+ } else if (type == FieldType.TIMESTAMP) {
+ for (Object v : values.getValues()) {
+ doc.addField(name, QueryBuilder.toIndexTime((Timestamp) v));
}
- } else if (f.getType() == FieldType.EXACT
- || f.getType() == FieldType.PREFIX
- || f.getType() == FieldType.FULL_TEXT) {
- for (Object value : values) {
- doc.addField(f.getName(), (String) value);
+ } else if (type == FieldType.EXACT
+ || type == FieldType.PREFIX
+ || type == FieldType.FULL_TEXT) {
+ for (Object value : values.getValues()) {
+ doc.addField(name, (String) value);
}
} else {
- throw QueryBuilder.badFieldType(f.getType());
+ throw QueryBuilder.badFieldType(type);
}
}
