I have a Geofield and a Filefield on an entity (taxonomy term), and I have "enable geocoding of location data" checked on the Geofield, with the Filefield as the source. I'm using it to upload a KML file, and geocode it into a Geofield.

This works great if the "number of values" in the Filefield is 1, and 1 KML file is uploaded.

The problem is that if the Filefield has a "number of values" greater than one, but only one file is uploaded, geocoding chokes because it doesn't understand why there are files "missing" from the other Filefield values.

I traced this to line 289 of geocoder.widget.inc:

276.        // Geocode any value from our source field.
277.        try {
278.          if (isset($handler_settings)) {
279.            $geometry = call_user_func($handler['field_callback'], $field_info, $item, $handler_settings);
280.          }
281.          else {
282.            $geometry = call_user_func($handler['field_callback'], $field_info, $item);
283.          }
284.          if ($geometry instanceof Geometry) {
285.            $geometries[] = $geometry;
286.          }
287.          else {
288.            // An error occured
289.            return FALSE;
290.          }
291.        }

This basically says that if the $geometry that's returned by the field callback is not an instance of the Geometry class, something went wrong, so give up on the whole thing. This is inside a foreach loop that iterates over all the available field values, including filefields that have not been uploaded to. The result is that if there are ANY empty filefields, the whole thing breaks.

Here is the KML geocoder callback function that is being called in call_user_func() (from plugins/geocoder_handler/kml.inc):

function geocoder_kml_field($field, $field_item) {
  if ($field['type'] == 'text' || $field['type'] == 'text_long' || $field['type'] == 'computed') {
    return geocoder_kml($field_item['value']);
  }
  if ($field['type'] == 'file') {
    if ($field_item['fid']) {
      $file = file_load($field_item['fid']);
      $kml = file_get_contents($file->uri);
      return geocoder_kml($kml);
    }
  }
}

So it seems that there are two possible solutions:

1) Modify geocoder_kml_field() so that it always returns a Geometry-class object, even if it's an empty one. This will effectively add an empty geometry to the $geometries array that is returned, rather than returning FALSE. If we do this, we will want to make sure the same is done in ALL geocoder_handler plugins that work with Filefields, because I assume they are susceptible to the same issue: EXIF, GPX, JSON, KML, and WKT.

2) Improve the code in geocoder.widget.inc to better handle empty Filefields.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

m.stenta’s picture

Status: Active » Needs review
FileSize
520 bytes

Attached is a very simple patch that removes the return FALSE; else conditional. I don't think it's necessary, because the if statement above it will either add geometries (if it worked), or not (if it didn't). There's no need to return FALSE, unless I'm missing something...

Pol’s picture

AFAIK, I think it makes sense.

  • Pol committed 884345a on 7.x-1.x authored by m.stenta
    Issue #2352887 by m.stenta: Multi-value Filefields break Geofield...
Pol’s picture

Status: Needs review » Fixed

Thanks !

  • Pol committed 884345a on 7.x-2.x authored by m.stenta
    Issue #2352887 by m.stenta: Multi-value Filefields break Geofield...

  • Pol committed 884345a on 8.x-2.x authored by m.stenta
    Issue #2352887 by m.stenta: Multi-value Filefields break Geofield...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.