Option to use geocoder client side

[bozdoz]

From an email:

Instead of storing the latitude and longitude of all locations in the database, as individual entries in the database,

I would like better if the lat lng were not to be saved in the database and if they could instead to be calculated on the runtime, in the client side, when loading the page.

I sent you a snippet that demonstrates this possibility.

I have hundreds of locations. So there are huge entries saved in the database, more than WordPress and plugins combined.

This could be a new feature, a new attribute which you toggle on or off while generating the shortcode. " to retrieve the data from database: toggle on or off".

If the short code is ON by default, it maintains current functionality.

If the value is OFF, it is going to calculate the lat and long from "London" in real time, without storing in database while doing so.

[webd-uk]

From https://wordpress.org/support/topic/wp_options-table-with-lots-of-leaflet_osm_-rows/#post-16668472

In that case surely the plugin should use the WordPress Transients API rather than the options API?

That would negate the requirement for the “Clear Geocoder Cache” button and allow old data to be forgotten.

I would appreciate if this could be considered.

Thank you.

Oliver

[hupe13]

This question also relates to this issue: https://wordpress.org/support/topic/leaflet-marker-shortcode-check-if-address-exists/

[designzwang]

Quite difficult to decide how to do the lookup ... I found this issue while searching a solution for a ( potential ) database problem.

Looking up the location at runtime will of course case lots of hits on the geocoder, and a plugin like yours is probably meant to be easy to use without any additioinal dependencies (especially when it comes to commercial vendors).

Otherwise, like with google maps or others, you need an API key if you create larger amounts of Geocoding requests. I know that some plugins, like TEC (see below), come with a "builtin" key, but in the end someone has to pay for it and I am not aware of a geocoder that is actually "free" and unlimited at the same time.

So I think your combination of Nominatim and a local cached storage in the DB is a very good concept - but only for sites with only a few maps. Therefore, IMHO improving the caching solution would probably be better than following the suggestion to do a runtime client side lookup.

Regarding my DB problem with geocoding/caching:

I want to use the "TheEventsCalendar" plugin for all sorts of events. It comes with a google maps integration , and it has a builtin API key for some basic features, so it can do geocoding out of the box. Unfortunately it has no OSM features.

So I wanted to replace google maps with leaflet , using your plugin and using do_shortcode(...). Basically it works, but it will fill our options table very quickly, because we will have many events in different locations, and the Calendar does not store Lat/Long information, but only addresses. So there will be a lookoup for every new event or location, and this will end up in the options table.

Problems here:

• hundreds, if not thousands, of entries in the option table, and they are autoloaded ( btw - why autoload?)
• one huge array with all addresses in leaflet_geocoded_locations' option, which will eventually grow so big that it may break things
• creates the necessity for manual "housekeeping" , i.e. deleting cache
• will flood the geocoder with requests after "housekeeping"

All this makes me feel a bit uncomfortable without really a good idea for a solultion

@webd-uk - When commenting on https://wordpress.org/support/topic/wp_options-table-with-lots-of-leaflet_osm_-rows/, I first thought that using transients would be a better solution, but actually it will not really change the DB usage - transients will also end up in the options table, and they will not be deleted automatically on expire unless there are housekeeping plugins in place who clean up.

How about this:

• using transients instead of "normal" option entries , with a reasonable life span of , say, a month or so and no autoloading
• change the housekeeping/delete job to have a choice : clean out all or only expired transients
• create a cron task (and/or wpcli action) to clean expired transients
• skip creating the potentially huge "count" array in leaflet_geocoded_locations - identify them by a common prefix

This would avoid to clean out all cached entries at once, causing many new coding requests, and would be a more stable self-cleaning setup that will not store an unlimited amount of lookups if nobody uses the delete function. The maximum amount of cached entries would be limited by the default cache ttl of the entries, and that coud be made to be a configurable value.

I think creating and using a separate table is considered an antipattern for plugin developers these days, isn't it? Or would that work?

Regards Ulrich

[bozdoz]

In that case surely the plugin should use the WordPress Transients API rather than the options API?

From the Transients link:

storing cached data in the database temporarily

This is not meant to be a temporary cache; it's intended to reduce the calls to Nominatum or Google Geocoder API.

[bozdoz]

hundreds, if not thousands, of entries in the option table, and they are autoloaded ( btw - why autoload?)

I'm not sure how or why autoload is there. Any advice on that?

https://github.com/bozdoz/wp-plugin-leaflet-map/blob/master/class.geocoder.php#L54-L57

                $location = (Object) $this->$geocoding_method( $address );

                /* add location */
                add_option($cached_address, $location);

Does add_option automatically add autoload?

[bozdoz]

One more comment: if you clear the geocoder cache and then visit the shortcode helper page in the admin, you should see a significant delay (because it's looking up addresses). That's one problem I'm trying to mitigate. Also, as mentioned above, I want to hit the geocoder endpoints as little as possible; I believe Nominatum and maybe Google Geocoder both require that consumers cache their lookups.

[webd-uk]

I'm not sure how or why autoload is there. Any advice on that?

Basically unless an option is used on all or lots of pages then it shouldn’t be auto loaded …

From https://developer.wordpress.org/reference/functions/add_option/ …

$autoload string|bool Optional Whether to load the option when WordPress starts up. Default is enabled. Accepts 'no' to disable for legacy reasons. Default: 'yes'

[webd-uk]

In that case surely the plugin should use the WordPress Transients API rather than the options API?

From the Transients link:

storing cached data in the database temporarily

This is not meant to be a temporary cache; it's intended to reduce the calls to Nominatum or Google Geocoder API.

A cache, by definition, is temporary. You can set the expiration time in seconds to whatever you like but the longer you make this the more you will be reducing the calls to the API. I would probably set this to a week or a month.

But to be honest I don’t think the wp_options table is the right place for this data be it transient or otherwise. It’s not right to be meta data either seeing as your plug-in doesn’t create anything that can have meta data added so that leaves a custom database table. Especially if you are looking to keep the data permanently (until it’s flushed in settings).

Oliver

[bozdoz]

A cache, by definition, is temporary. You can set the expiration time in seconds to whatever you like but the longer you make this the more you will be reducing the calls to the API. I would probably set this to a week or a month.

A month sounds reasonable to me. Are transients autoloaded? Should I be autoloading any of these wp options? I've never worked with wp options performance issues before.

[designzwang]

Hi, regarding transients vs. options table:

The transient API serves as a transparent interface to whatever caching technology is active. So using transients will benefit from any type of cache ( memcache, redis, you name it) if there is one. If no other cache is enabled, it will fall back to storage in the options table. And then, @bozdoz, looking at the set_transient()-function it seems they are not set to be autoloaded if they have an expiry - see https://developer.wordpress.org/reference/functions/set_transient/

Regarding the temporary nature of transients - it is a free choice how long the expiry is. So "transient" can in fact be a very long time and serve the purpose of saving nominatim requests until cleanup Regards Ulrich

[webd-uk]

@designzwang Yes, I concur. All that I would add to that, and in response to @bozdoz question about wp_options performance ... Wordpress checks ("Dashboard - Tools - Site Health") that you have less than 500 rows in the wp_options table set to autoload (in addition to checking that their total size is less than 100Kb). So moving to expiring transients should also help to reduce the total rows of autoloaded wp_options.

I'm still not convinced that this data should be stored here, however. We had 280 on our site which is a lot of rows when you consider how many there should be in total on a normal Wordpress install.

Oliver

[designzwang]

@webd-uk - even if it feels bad to throw in so many records into the options table, it is done by many plugins, probably for the same reason it is used here. It is easy, it is an accepted design pattern and will work in any environment. The overhead of creating your own caching layer or handle custom tables is quite large, and actually I don't like plugins that create unnecessary custom tables ... Now for transients and the leaflet plugin, I will try how things work out with transients and redis, the latter being a long time favorite on my to-do list for performance improvements. I guess it is not a high priority , so if @bozdoz can wait a couple of days I could probably try to work on a PR.

[bozdoz]

Just specifically about autoload: Should any of this plugin's options be set to autoload?

[webd-uk]

Just specifically about autoload: Should any of this plugin's options be set to autoload?

I believe the rule of thumb is that any option that is used on all or most requests should be auto loaded. Otherwise not.

And in an ideal world you would have a single wp_options row that was a serialised array of the plugin’s options.

Oliver

[bozdoz]

Let's make that ideal world happen

[bozdoz]

I've added a PR to move all the geocoded addresses to a single transient with a month expiry date; what do you think?

#217

[webd-uk]

Although this sounds good it may have undesired results as in this situation all addresses would expire at exactly the same time? If you’re OK with that then no problem!

[designzwang]

Short update on this: I made a deep dive into the transients documentation and found some good news. There actually IS a garbage collection that takes care of expired transients. ( The cron event is delete_expired_transients ) So no need to put additional work into that, and everything that is needed is to replace the option storage with transients. Also, I found a way to delete the cached entries on-demand without having to keep track of them in a (potentially) huge option array. PR is prepared with minimal changes, however I will run the changed code for a couple of days on a test system. Regards urich

[webd-uk]

We use the following hooks to dynamically control various aspects of Leaflet JS plugin ...

                add_filter('option_leaflet_js_url', 'our_plugin_class::option_leaflet', 10, 2);
                add_filter('default_option_leaflet_js_url', 'our_plugin_class::option_leaflet', 10, 2);
                add_filter('option_leaflet_css_url', 'our_plugin_class::option_leaflet', 10, 2);
                add_filter('default_option_leaflet_css_url', 'our_plugin_class::option_leaflet', 10, 2);
                add_filter('option_leaflet_map_tile_url', 'our_plugin_class::option_leaflet', 10, 2);
                add_filter('default_option_leaflet_map_tile_url', 'our_plugin_class::option_leaflet', 10, 2);

Please confirm ASAP if the proposed changes will affect any of these options "leaflet_js_url", "leaflet_css_url", "leaflet_map_tile_url" and what the format of new options will be accordingly.

Many thanks,

Oliver

[designzwang]

@bozdoz Dammit, I did not see that you already created a PR, so I started working on my own. There are not many changes however. https://github.com/bozdoz/wp-plugin-leaflet-map/pull/218

Compared to your solution, I think it is more scalable to use one transient per record, and not collect them all in a single and potentially huge transient.

If a persistant cache is used, the cache deletion function to delete everything will not work - it only takes care of transients stored in the options table, and that only happens if no 3rd party cache is used. However all transients set with this code will have an expiry, so that should not be a problem.

I saw some other changes in your branch, so I based my PR on master. If you consider my PR for inclusion, I could re-do it whichever way you prefer.

@webd-uk i am not aware that my code would have any impact on your url filters.

[webd-uk]

And in an ideal world you would have a single wp_options row that was a serialised array of the plugin’s options.

Let's make that ideal world happen

Please confirm ASAP if the proposed changes will affect any of these options "leaflet_js_url", "leaflet_css_url", "leaflet_map_tile_url" and what the format of new options will be accordingly.

Thanks @designzwang, this was more for @bozdoz and less about the transients as I believe the existing options may be due to be merged into a single option array.

[bozdoz]

We use the following hooks to dynamically control various aspects of Leaflet JS plugin ...

                add_filter('option_leaflet_js_url', 'our_plugin_class::option_leaflet', 10, 2);
                add_filter('default_option_leaflet_js_url', 'our_plugin_class::option_leaflet', 10, 2);

Please confirm ASAP if the proposed changes will affect any of these options "leaflet_js_url", "leaflet_css_url", "leaflet_map_tile_url" and what the format of new options will be accordingly.

I legitimately don't understand any of this. To my knowledge there are no filters created using those names.

[webd-uk]

Oh, sorry. OK, so basically there is a global option_{option name} filter in Wordpress that allows us to change the values of get_option() based on the option name. This in turn allows us to change the settings of your plug-in dynamically in our application (which we require).

So … to cut to the chase, if you are intending to merge all the options that your plug-in saves in the options table into one option that has a serialised array of options then I would like to know before you push that out because it would adversely affect our application.

Does that make sense?

Oliver

[bozdoz]

so basically there is a global option_{option name} filter in Wordpress that allows us to change the values of get_option() based on the option name

Didn't know that. Good to know. Does it work for transients?

[bozdoz]

Looks like there's a transient one but not a default transient one. I'm assuming this would be an issue.

[webd-uk]

That would be transient_{option name} :)

[webd-uk]

Looks like there's a transient one but not a default transient one. I'm assuming this would be an issue.

Not for us. We cache latlngs a completely different way. They're saved into post meta as per this recommendation ... so wouldn't need to filter your transient data.

[bozdoz]

@webd-uk @designzwang

I'm probably going to opt for something like in this PR: https://github.com/bozdoz/wp-plugin-leaflet-map/pull/217#issuecomment-1522556089

1. Added filters for getting, setting, removing, and updating all geocoder addresses.
2. Sets geocoder options to autoload=false.

This is the best backwards-compatible option, and allows advanced users to customize their geocoder caching.

Thoughts?

[webd-uk]

Hmm ... as per https://developer.wordpress.org/reference/functions/update_option/#parameters

$autoload string|bool Optional Whether to load the option when WordPress starts up. For existing options, $autoload can only be updated using update_option() if $value is also changed.

... I'd be inclined to perform some kind of (one time?) check on existing options to delete them and then add them again to force them to be set to $autoload 'no' seeing as $value is unlikely to be changed.

As for your filters on getting / setting / removing / updating geocoded addresses ... I'm not sure I'd have any need for these filters but so long as they're being saved as transients and have an expiry date, I'd imagine that it would be fine for our purposes because it would mean un-used geo-data would eventually be removed automatically from the wp_options table.

Similarly to the above described check on the options ... would you be having a (one time?) check on existing cached geo data to delete from wp_options and re-add as transients?

Oliver

[bozdoz]

would you be having a (one time?) check on existing cached geo data to delete from wp_options and re-add as transients?

You might be missing what that PR is intending to do: there are no transients. The only db change is to set autoload to false going forward. If someone wants to swap their existing options with non-autoloaded options, then they can clear the geocode cache.