Ok here is the use case:

in the drealty idx module, we've been using geofield and geocoder modules to geocode Real Estate listings from a RETS feed. The feeds often times have many, 10k or 60k listings that are imported/updated on an hourly basis.

The problem is that during the import even if the address has been geocoded before there is no way to 'skip' the geocoding. And for the question, is there a way to have geocoder explicitly skip the call to the handler's field callback:


            // Geocode any value from our source field.
            try {
              if (isset($handler_settings)) {
                $geometry = call_user_func($handler['field_callback'], $field_info, $item, $handler_settings);
              }
              else {
                $geometry = call_user_func($handler['field_callback'], $field_info, $item);
              }
              if ($geometry instanceof Geometry) {
                $geometries[] = $geometry;
              }
            }
            catch (Exception $e) {
              drupal_set_message($e->getMessage(), 'error');
            }

I had tried to set a ['skip'] = TRUE on the geofield's values, however in the field_presave() of geofield it basically replaces all values of the field with geofield_compute_values()

  ...
  geofield_compute_values($item, $item['master_column']);
  $items[$delta] = $item; 
  ...

If it were to merge the results like:

  ...
  geofield_compute_values($item, $item['master_column']);
  $items[$delta] += $item;
  ...

then you could check for a 'skip' and then skip calling the handler.

If you like i can roll a patch and submit it, however i wanted to check to see if there were any other use-cases for skipping or if you were even open to adding this functionality in. Currently i've 'overridden' the yahoo and google handlers to basically do something similar:


function drealty_geocoder_yahoo_field($field, $field_item) {
  if ($field_item['changed']) {
    if ($field['type'] == 'text' || $field['type'] == 'text_long' || $field['type'] == 'text_with_summary' || $field['type'] == 'computed') {
      return drealty_geocoder_yahoo($field_item['value']);
    }
    if ($field['type'] == 'addressfield') {
      $address = geocoder_widget_parse_addressfield($field_item);
      return drealty_geocoder_yahoo($address);
    }
    if ($field['type'] == 'taxonomy_term_reference') {
      $term = taxonomy_term_load($field_item['tid']);
      return drealty_geocoder_yahoo($term->name);
    }
  }
}

It works for now and in such a way that i don't have to patch or hack up geocoder or geofield, however i feel like there is a more elegant solution.

any thoughts or comments are welcome.

Comments

camidoo’s picture

camidoo commented

Thought this was worth mentioning as well:

http://drupal.org/node/1297654#comment-5511498

I thought i had come across this issue before. Not sure if it's relevant.

michaelfavia’s picture

michaelfavia commented

This is an interesting and not completely unique use case.

We have other users who would like to adjust the geocoding in the geofield usign a map or the like and they expect that it should preserve that change unless "reset" manually.

I think this is really the same issue you have. The question we need to ask on our geocoding then is:

Is the value of the field that we are geocoding from different from the previous value/formstate (may require a static node load if not avail)? If so is our geocoder/geofield manually fixed? if not geocode and update geofield.

Would this solve your usecase well?

camidoo’s picture

camidoo commented

yes, which is basically what i'm doing now in my module if you look at:

http://drupalcode.org/project/drealty.git/blob/refs/heads/7.x-3.x:/dreal...

starting there down to line 477 i'm checking to see if the existing value(s) are != to the new values, then setting a 'changed' property on address field which i then check for in my overridden handler:

http://drupalcode.org/project/drealty.git/blob/refs/heads/7.x-3.x:/plugi...

so if there was a way to 1) skip the geocoding by passing a value in to effect that, or 2) some sort of logic that handles wether or not it should geocode based on some logic, that would definitely solve my use case.

I'm also assuming that most of these use-cases are ones that are service related, ie yahoo place finder, or google geocoder api. If that is the case, it could be as simple as passing the entity into the handler and handling the logic of wether to geocode or not there. As that was my first idea, however when the handler is called the only thing passed in is the target field, and the field info, and any handler settings.

so the signature of the field call back would become: 

 abstract function my_geocoder_field_callback($field, $field_item, $settings = NULL, $entity = NULL)

or some such, then the handlers can handle the logic and geocoder doesn't have to worry about what a handler has to do, or wants to do, all geocoder should be concerned with is, calling the handler, and expecting back a typeof(Geometry).

nd987’s picture

nd987 commented

I have the exact same requirements. Geocoder needs a persistent caching mechanism that can cache based on the hash of the query. This would get all entities (or manual api calls) that made a request for a given geocode query, regardless of other factors like node id, etc...so multiple drealty records that have the same address (ie, Agents or Offices), would only get geocoded once.

See here for progress on the caching of results: http://drupal.org/node/1515372

simon georges’s picture

simon georges commented
Version: 7.x-1.0 » 7.x-1.x-dev
Category: support » feature

@camidoo, #1617340: No need to geocode on every entity update seems to be what you're looking for, does it solve the issue for you?

camidoo’s picture

camidoo commented

After looking at that patch i would assume so yes. I'll have to be honest and confess that i haven't done any drupal development in just right at a year now and the drealty project has been taken over by kevinquillen at inclindinc so he would prolly have better input on this than myself.

camidoo’s picture

camidoo commented
Issue summary: View changes

added php tag to a code snippet i had forgot

rudiedirkx’s picture

rudiedirkx commented

This is fixed with #1515372: Caching Results, but it'll still require the initial 10k lookups... I can imagine that's still unacceptable.

The 'new' method doesn't do the 'has anything changed' check anymore, because everything is cached. (I realize only now, that's not good for sites with many geocoded entities and large updates.)

I see 2 solutions:

  • Check for a flag in the entity, to skip geocoding the entire entity, whatever the changes.
  • Reverse-save all existing coordinates into the geocoder cache, and then do the lookups. All existing items will come from cache, all new items (significantly fewer) will be looked up live.
rudiedirkx’s picture

rudiedirkx commented

For the 'caching instead of diffing' issue that now forces a new lookup for every entity, the simple solution is to reinstate the diff... Not my fav, because the field item value is not enough, but it'll stop millions of useless live lookups to fill the cache.

basvredeling’s picture

basvredeling
Primary language Dutch
Location Amsterdam
commented
Related issues: +#2514834: Force re-encoding of already geocoded data

Added a related issue to force regeocoding. This looks like the inverse of this issue but might have a similar solution.
Perhaps if we A) add a "geocode now" checkbox in the entity save form (can also be set programmatically) and B) the geocoded field gets an extra configuration option:

  1. Geocode field when no geodata exists (default)
  2. Always geocode this field even if geodata already exists
  3. Only geocode this field if the "geocode now" checkbox is checked.

With this extra select/radio option the user can specify the preferred behaviour of the field and adapt it at a later date.
In your case you could first set the the geocoder field to "only geocode if checked". When all external location info is imported you can change the field setting to "geocode when no geodata exists" and geocode all new fields, skipping the imported ones.

Also, bulk updates should require a batching / queueing system to prevent time-outs etc.

pol’s picture

pol
Location Brussels
commented
Status: Active » Closed (outdated)