Drop Google Maps for Open Street Map #3025

viroulep · 2018-06-30T19:50:42Z

There is no issue open for this, but if this is merged it would fix #232.

Google Maps will soon (July 16th) charge on a per-map-load basis (see here).
As far as I get it, it would give us just over 28000 map loads monthly.

To be honest I have no idea about how many map loads we do monthly, but I wouldn't be surprise if we were around or over that number.
I think it's a great opportunity to look into free alternatives that exists, and most specifically Open Street Map.

So here is a proof of concept and WIP PR for dropping gmaps for osm, with a list of pros, cons, and some stuff I noticed when testing it.

First of all, places where we use maps in the WCA website, with the features we require:

Profiles (Map + Markers)
Competitions list (Map + Colored Markers)
Competition edition form (Map + Search input + Markers + Circles)
Competition tabs, through our map() markup in Markdown (Map + Marker + Search result popup, in iframe).
Edit schedule UI (Map + Marker)
I found some use of it in webroot/results/includes/_map.php, I couldn't find a page where it's actually used, maybe someone better remember what's in there.

Support for all this, in the context of OSM, needs several parts:

A library that will render a map and enable us to interact with it (I've went with leafletjs which looked appealing, and I'm pretty satisfied with it).
A provider for the tiles of the map.
A provider for the search query (ie: that gives us a list of results with location and label when given a query, so that we can display it on the map).

For the last two there are a bunch of options.

OSM provides a keyless tile server (usage policy here). Since we mainly hit tiles that are cached (whole world/countries) I think we should be good.
Note: it only provides labels (city names, streets, etc) in their local language, which is both good for locals, and bad for foreigners who don't speak the local language.
ESRI also provides a keyless tile server, with labels only in English.

For handling search queries I chose to use leaflet-geosearch which has support for various providers.
Obviously they support using gmaps as a search engine, and the two main free alternatives are:

OSM through Nominatim. Unfortunately the search engine is not as good as google (or rather, OSM doesn't have as much details as gmaps in some places), but searching gps coordinates obviously works.
Esri is a bit better for some addresses, but it's totally messed up when searching for gps coordinates (it handles lat,lng the other way around...)

I've deployed it on staging to give you a rough idea of the usability.

Pros

No risk of having to pay for displaying maps
Dropping google maps means it will make some part of the world's life easier (see Only load google maps api when needed #232).
Better performance due to not having to load a remote library (leaflet is less than 40KB).

Cons

with gmaps the map view was personalized with the user's preference (specifically: language)
OSM is sometimes less detailed than gmaps :( (the most obvious example I found when looking through existing competitions was here, compared to absolutely nothing in OSM).

My overall feeling about dropping gmaps is mostly positive: from a programmer point of view it's definitely very similar (as you can see in the code, it's mostly changing the library includes and moving stuff around). From a user point of view, it can definitely be disappointing to lack details in areas you'd like to see...
However most of the maps offered to the "regular" user are displaying the world as a whole. We actually never display a map zoomed in of a competition venue to the user, but we provide a link with the coordinates (that we can keep pointing to Google Maps).

viroulep · 2018-06-30T19:52:16Z

@Baiqiang, it would be very interesting to see if it has some visible effect in countries not in love with google ;)

jfly · 2018-07-07T17:38:48Z

I found some use of it in webroot/results/includes/_map.php, I couldn't find a page where it's actually used, maybe someone better remember what's in there.

I'm pretty sure _map.php is not used anymore since @jonatanklosko ported the competitor profile page to RoR. Here's a list of recent relevant commits (git log -S displayMap):

commit 1c8fef0
Author: Jonatan Kłosko jonatanklosko@gmail.com
Date: Wed May 3 01:55:46 2017 +0200

Remove unnecessary PHP person profile code.

commit eeb00fe
Author: Jonatan Kłosko jonatanklosko@gmail.com
Date: Tue May 24 16:09:16 2016 +0200

Remove 'competitions.php' script that's already moved to Rails.

commit a9d51d0
Author: Jeremy Fleischman jeremyfleischman@gmail.com
Date: Sun Aug 30 21:26:29 2015 -0700

Restore _map.php, whose displayMap() function is still used in a couple
of places. This fixes #159.

For handling search queries I chose to use leaflet-geosearch which has support for various providers.
Obviously they support using gmaps as a search engine, and the two main free alternatives are:

It seems like a real shame to me to move away from Google's search here, because as you pointed out, it will break competition tabs that display a map (such as https://staging.worldcubeassociation.org/competitions/DallasFortWorth2018#5378-venue). What would be the consequences of sticking with using Google's search for now? Is it also going to be rate limited in the ways that maps are going to be limited?

Speaking of competition tabs, I am super freaking pleased that you're able to make changes to all the past competition tabs here. That would not have been nearly as easy if we'd let people input arbitrary html. Great future proofing, @jonatanklosko!

jfly

@viroulep thanks so much for taking this on!

I'd like to better understand the resizing logic you had to add to get things working. Basically, at a high level, if google maps was able to work without us notifying it whenever it becomes visible, it feels to me like we should be able to get leaflet to do the same thing.

I don't want this investigation to keep this code from getting merged up, but I do think it's worth spending some time trying to understand it. I'm happy to help investigate if you want to point me in the right direction =)

As mentioned already, I think it might be good to make the switch to OSM+leaflet, but keep using google's search to get lat long for locations. That would minimize churn, and we could always look into switching to a different geocoder later.

WcaOnRails/app/helpers/markdown_helper.rb

WcaOnRails/app/javascript/edit-schedule/EditSchedule.jsx

WcaOnRails/app/javascript/edit-schedule/EditVenue/index.jsx

WcaOnRails/app/views/layouts/application.html.erb

jfly · 2018-08-27T02:02:23Z

I just went to https://www.worldcubeassociation.org/competitions/new, and it looks like we're finally getting blocked:

So, it's probably more important now to get this change merged up.

viroulep · 2018-08-27T11:42:47Z

Ah crap, we did end up reaching the limit :(
I'll revisit this tonight after I merged the schedule thing!

As mentioned already, I think it might be good to make the switch to OSM+leaflet, but keep using google's search to get lat long for locations.

Yes, and I hope this solution won't reach gmaps limit for the number of queries...

viroulep · 2018-08-28T19:37:40Z

FYI, according to this page, we can get up to 40 000 geocoding queries per month.

Geocoding is not used in many places of the website:

user maps in competition's tabs
address input in edit competition form

So we should be fine, I hope :p

viroulep · 2018-08-29T09:13:14Z

The tests failures I experienced were quite disturbing at first, the test for the "manage schedule" page was failing because HTMLVideoElement wasn't defined, which didn't make much sense to me.
It turns out by default PhantomJS doesn't support this element, and Leaflet has a built-in video layer support.
Therefore importing something from Leaflet will execute some code involving a call to instanceOf HTMLVideoElement, that results in an error in PhantomJS.

Solutions and workaround I found:

recompile our own version of PhantomJS on top of a recompiled, video-enabled, webkit (see here).
avoid importing any Leaflet stuff when in test mode, in the JS tests.

1 would involve us storing our own built version of PhantomJS somewhere.
I've given a go to workaround 2 in this PR, specifically commit ac6babf

Please let me know if you have any other ideas!

Also I've deployed this to staging, so feel free to see how it looks and experiment there!
Note: geocoding doesn't quite work on staging, @jfly and I have been discussing how to handle google maps restriction: only unrestricted or IP-restricted key can be used for the Geocoding API, whereas we have a referer-restricted key.

jfly · 2018-09-02T22:24:18Z

@viroulep, things seem to be working ok now that we've enabled billing on our google maps account. Do you think we should drop this PR for now? The only real problem with have with google maps is that it doesn't work in China, but AFAIK, all comps in China use cubingchina, so that's not a big issue.

viroulep · 2018-09-03T09:43:22Z

It seemed to be a great opportunity for us to move away from Google Maps :p

Also, while enabling billing was required for us to use the webservice API, it is not required for our current setup.
The only difference now is that when we'll reach gmaps' limit, we'll start paying (whereas before we would have a watermark "for development" on several maps).

I acknowledge gmaps has a better geocoding API than Nominatim, so it probably makes sense to keep this no matter what.
But the choice of changing map provider is more about if we want to use free software vs free non-free software.

Also note that we could still bring in leaflet with gmaps provider :p
We'd have to do some more test to see which map library is actually faster/lightweight/better for browsers and mobiles.

jfly · 2018-09-03T18:04:15Z

Yeah, I'm all for switching to something less restrictive in the future, but I don't know if it's worth focusing on right now. That said, if you're motivated to keep working on this, I'm all for it! Next things I'd like to investigate:

Finding a better solution to the resizing issues with leaflet so the diff isn't as scary.
Consider storing geocodes in our database so we're more resilient to changing geocoders.

viroulep · 2018-10-30T10:15:48Z

WcaOnRails/app/views/persons/_competitions_map.html.erb


-      $('#competitions-map').one('map-shown', function() {
-        /* Google Map is not properly initialized within a hidden container, that's how we fix it. */


@jfly it seems we had that resizing issue with google maps too, but I have no idea why we didn't have it in the venue picker in the schedule editor...

viroulep · 2018-10-30T10:20:06Z

I just rebased the PR, @Baiqiang I'll deploy this to staging so that you can take a look.

@jfly

Finding a better solution to the resizing issues with leaflet so the diff isn't as scary.

Unfortunately I think leaflet on his own can't do it :s
For the embedded map I could set an arbitrary size (and therefore no resize is needed), but for the schedule editor, competitions map, and profile map the width is some percent of the window size.

I'm very much open to suggestions, right now I'm out of ideas!

jfly · 2018-11-22T21:20:10Z

Sorry for the delay. I finally did some investigation into why Google Maps is behaving differently than Leaflet. I posted the results of my investigation above. Please feel free what to do with that, but otherwise, this PR looks good to me. 🚢 it!

WcaOnRails/app/javascript/edit-schedule/EditVenue/VenueLocationInputImpl.jsx

WcaOnRails/app/assets/javascripts/competitions.js

viroulep · 2019-03-06T18:12:13Z

I just deployed it to staging again!

viroulep · 2019-03-12T14:57:56Z

Hum I just tried using our google API key to use the google search provider, and got this error:

API keys with referer restrictions cannot be used with this API.

Let's just drop this...

- Replace maps in edit schedule - Replace maps in tabs - Replace maps in competitions forms - Replace maps in persons profiles

viroulep · 2019-04-17T07:47:10Z

Ok so I've figured out a solution that seems an acceptable tradeoff:

OSM is used to render all the maps in the website
For geocoding we would still use the google maps geocoding API (I have modified our API key to be restricted by IP addresses instead of referrer). The free quota should allow us for ~20000 geocoding queries per month, which should be fine as this is used only in two places: the competition edit page, and the tabs when the user uses the "map()" markdown feature we added.

Using OSM+Nominatim lead to a whole lot of broken maps in tabs, so currently I think it's best to avoid nominatim until we find a way to easily fix existing tabs.

This PR is currently deployed on staging.

@jfly please let me know if I should merge and deploy this!

viroulep · 2019-04-17T07:54:26Z

(I've edited this wiki entry to mention that we should change the API key restriction after we spin up a new server: https://github.com/thewca/worldcubeassociation.org/wiki/Spinning-up-a-new-server)

viroulep · 2019-04-17T08:03:53Z

Some extra numbers I fetched from our billing dashboard:

Date	Description	Amount (USD)
Feb 1 – 28, 2019	Maps API Dynamic Maps: 41841 Counts (Source:WCA website [wca-website])	$292.89
Feb 1 – 28, 2019	Geocoding API Geocoding: 5311 Counts (Source:WCA website [wca-website])	$26.57
Mar 1 – 31, 2019	Maps API Dynamic Maps: 41702 Counts (Source:WCA website [wca-website])	$291.92
Mar 1 – 31, 2019	Geocoding API Geocoding: 4617 Counts (Source:WCA website [wca-website])	$23.11

So it seems our current usage of the geocoding API is way below the free tier (200$) offered by google :)

viroulep · 2019-04-17T10:16:16Z

Hum it's trickier than I thought, since it's the client's IP which is making the geocoding query...
Just adding our servers IP doesn't quite work.
How do you feel about implementing some API proxy endpoint on the WCA side (that would take the geocoding query, forward it to google, then forward back the answer to the client)?
Eventually implementing some kind of authenticity token to avoid anyone just using our proxy without loading the appropriate page first.

We want to restrict our Google API key on an IP basis, so we need to make sure the actual query comes from our server (and not from the client). We also want to make sure anyone on the internet can't just use our proxy, so we explicitly check for Rails' CSRF token even if we're just doing a GET query (this way we are sure the query made by the client is legit).

viroulep · 2019-04-17T14:10:15Z

I just did so in the last - hopefully clean - commit.
I don't expect this to work on staging until we actually switch from referrer restriction (needed for the current implementation) to IP restriction (needed for the leaflet implementation)...

It does work with my local development key, and I did a very quick check live by switching the restriction for a couple of minutes and confirming production was blocked and staging was working.

jfly · 2019-04-17T16:59:11Z

(I've edited this wiki entry to mention that we should change the API key restriction after we spin up a new server: https://github.com/thewca/worldcubeassociation.org/wiki/Spinning-up-a-new-server)

We actually use a static IP for both staging and our prod server, so it shouldn't actually be necessary to change that!

jfly

This looks good to me!

Now that our API token is only used on the backend, I'd actually be ok with creating a brand new, truly secret token and removing the IP restriction entirely. Now that the token is not exposed to the frontend, it's not like people should be able to steal it, and removing the IP restriction will make our lives easier if we ever change server IPs (I know we use a static IP right now, so that's not likely to change, but maybe someday we move off of AWS or (more likely) start spinning up multiple API servers)

jfly · 2019-04-17T17:22:23Z

BTW, awesome work here. This will be really great to have done.

viroulep · 2019-04-17T19:21:20Z

Thanks!

I've just added a brand new API key to our encrypted data bag, I'll try to deploy this on staging.
I did restrict the key to the "geocoding" API!

viroulep added the STATUS: in-progress Actively assigned to someone / expected to be resolved by another issue/PR already in progress label Jun 30, 2018

jfly reviewed Jul 7, 2018

View reviewed changes

viroulep force-pushed the drop_gmaps_for_leaflet branch 3 times, most recently from 7d88377 to ac6babf Compare August 29, 2018 08:52

viroulep removed the STATUS: in-progress Actively assigned to someone / expected to be resolved by another issue/PR already in progress label Aug 30, 2018

viroulep force-pushed the drop_gmaps_for_leaflet branch from ac6babf to f3d6e68 Compare October 30, 2018 10:14

viroulep commented Oct 30, 2018

View reviewed changes

viroulep force-pushed the drop_gmaps_for_leaflet branch 2 times, most recently from 1c95b68 to 47fd48e Compare October 30, 2018 13:39

viroulep force-pushed the drop_gmaps_for_leaflet branch from 47fd48e to 882c48c Compare February 21, 2019 14:49

jfly added a commit to jfly/jfly.github.io that referenced this pull request Feb 24, 2019

Add iframe example for thewca/worldcubeassociation.org#3025 (comment)

33b4895

viroulep force-pushed the drop_gmaps_for_leaflet branch 2 times, most recently from 96ce641 to fab078c Compare March 6, 2019 18:01

jfly reviewed Mar 6, 2019

View reviewed changes

WcaOnRails/app/javascript/edit-schedule/EditVenue/VenueLocationInputImpl.jsx Show resolved Hide resolved

WcaOnRails/app/assets/javascripts/competitions.js Show resolved Hide resolved

viroulep force-pushed the drop_gmaps_for_leaflet branch from fab078c to 3e61865 Compare March 12, 2019 14:48

viroulep force-pushed the drop_gmaps_for_leaflet branch from 3e61865 to 169f127 Compare March 12, 2019 15:00

This was referenced Mar 12, 2019

Bump react from 16.8.3 to 16.8.4 in /WcaOnRails #3986

Closed

Bump react-dom from 16.8.3 to 16.8.4 in /WcaOnRails #3982

Closed

This was referenced Mar 31, 2019

Bump react from 16.8.3 to 16.8.5 in /WcaOnRails #4016

Closed

Bump react-dom from 16.8.3 to 16.8.5 in /WcaOnRails #4011

Closed

viroulep added 4 commits April 16, 2019 11:42

Replace google maps with leaflet

0af63c2

- Replace maps in edit schedule - Replace maps in tabs - Replace maps in competitions forms - Replace maps in persons profiles

Implement the invisible iframe trick in react

65a833d

propagate the fix to other maps

596890a

Add a comment about how we could handle stuff

4dc51ac

viroulep force-pushed the drop_gmaps_for_leaflet branch from 169f127 to 4dc51ac Compare April 16, 2019 09:44

Use gmaps for geocoding

a6d281a

viroulep force-pushed the drop_gmaps_for_leaflet branch from 9b1c5ed to a547301 Compare April 17, 2019 14:05

viroulep force-pushed the drop_gmaps_for_leaflet branch from a547301 to 95da715 Compare April 17, 2019 14:07

jfly approved these changes Apr 17, 2019

View reviewed changes

viroulep force-pushed the drop_gmaps_for_leaflet branch from 95da715 to f5ce6e9 Compare April 17, 2019 19:20

viroulep force-pushed the drop_gmaps_for_leaflet branch from f5ce6e9 to 7cb2838 Compare April 17, 2019 19:27

Add the new API key to encrypted data bag

a434e88

viroulep force-pushed the drop_gmaps_for_leaflet branch from 7cb2838 to a434e88 Compare April 17, 2019 19:35

viroulep merged commit 3a9bbf4 into thewca:master Apr 17, 2019

viroulep deleted the drop_gmaps_for_leaflet branch April 17, 2019 20:14

viroulep mentioned this pull request May 28, 2019

Lazyload embedded maps iframes #4176

Merged

jfly mentioned this pull request Aug 14, 2019

Stop using Google Maps for geocoding #4477

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Drop Google Maps for Open Street Map #3025

Drop Google Maps for Open Street Map #3025

viroulep commented Jun 30, 2018

viroulep commented Jun 30, 2018

jfly commented Jul 7, 2018

jfly left a comment

jfly commented Aug 27, 2018

viroulep commented Aug 27, 2018

viroulep commented Aug 28, 2018

viroulep commented Aug 29, 2018

jfly commented Sep 2, 2018

viroulep commented Sep 3, 2018

jfly commented Sep 3, 2018

viroulep Oct 30, 2018

viroulep commented Oct 30, 2018

jfly commented Nov 22, 2018

viroulep commented Mar 6, 2019

viroulep commented Mar 12, 2019

viroulep commented Apr 17, 2019

viroulep commented Apr 17, 2019

viroulep commented Apr 17, 2019

viroulep commented Apr 17, 2019

viroulep commented Apr 17, 2019 •

edited

jfly commented Apr 17, 2019

jfly left a comment

jfly commented Apr 17, 2019

viroulep commented Apr 17, 2019


		$('#competitions-map').one('map-shown', function() {
		/* Google Map is not properly initialized within a hidden container, that's how we fix it. */

Navigation Menu

Drop Google Maps for Open Street Map #3025

Drop Google Maps for Open Street Map #3025

Conversation

viroulep commented Jun 30, 2018

Pros

Cons

viroulep commented Jun 30, 2018

jfly commented Jul 7, 2018

jfly left a comment

Choose a reason for hiding this comment

jfly commented Aug 27, 2018

viroulep commented Aug 27, 2018

viroulep commented Aug 28, 2018

viroulep commented Aug 29, 2018

jfly commented Sep 2, 2018

viroulep commented Sep 3, 2018

jfly commented Sep 3, 2018

viroulep Oct 30, 2018

Choose a reason for hiding this comment

viroulep commented Oct 30, 2018

jfly commented Nov 22, 2018

viroulep commented Mar 6, 2019

viroulep commented Mar 12, 2019

viroulep commented Apr 17, 2019

viroulep commented Apr 17, 2019

viroulep commented Apr 17, 2019

viroulep commented Apr 17, 2019

viroulep commented Apr 17, 2019 • edited

jfly commented Apr 17, 2019

jfly left a comment

Choose a reason for hiding this comment

jfly commented Apr 17, 2019

viroulep commented Apr 17, 2019

viroulep commented Apr 17, 2019 •

edited