Fix issue with duplicate field for WaPo in Solr.

[ryan-clancy]

Fixing bug introduced by c5ee9af

[lintool]

Great! Go ahead and merge when Jenkins turns green.

[arjenpdevries]

So this only works because of converting the value to String, which might work out correctly assuming both types resolve the same string value - otherwise there could be an order effect, and different documents could get different values.

[lintool]

@r-clancy ?

[ryan-clancy]

So this only works because of converting the value to String, which might work out correctly assuming both types resolve the same string value - otherwise there could be an order effect, and different documents could get different values.

The implementation of stringValue() doesn't convert arbitrary objects to a String (using toString() for example). It looks like the implementation does this for a Number only (and keeps Strings, of course):

@Override
  public String stringValue() {
    if (fieldsData instanceof CharSequence || fieldsData instanceof Number) {
      return fieldsData.toString();
    } else {
      return null;
    }
  }

However, your comment has made me realize that the order of the checks in this PR should be flipped as the numericValue() method has stricter checking:

@Override
  public Number numericValue() {
    if (fieldsData instanceof Number) {
      return (Number) fieldsData;
    } else {
      return null;
    }
  }

Our code should be this:

if (field.numericValue() != null) {
    solrDocument.addField(field.name(), field.numericValue());
  } else if (field.stringValue() != null) { // For some reason, id is multi-valued with null as one of the values
    solrDocument.addField(field.name(), field.stringValue());
}

The ordering within a document collection would already be consistent, but I think this just makes more sense: try the numerical value first and fallback to the String. Since fields are added to Lucene documents in a fixed order in the Generator classes and iterating over fields happens in a deterministic order, this should be fine.

[arjenpdevries]

Sounds right, tnx for the clear explanation!