lizmap-web-client
lizmap-web-client copied to clipboard
[Feature] Add QMS Google Maps Tiles
Since release 9.0, OpenLayers has introduced support form Google Maps Tiles.
This PR relies basically on the same logic of this one https://github.com/3liz/lizmap-web-client/pull/4150
If a valid Google Api Key
is provided in the Lizmap plugin, then the application uses the OL dedicated class to get tiles from the Google endpoint and the API key is used to issue requests.
If no key is provided, then these tiles are treated as a generic WMTS external layer
and the application get the tiles directly from the default URI set by QMS QGIS plugin. In this case it is not guaranteed that the tiles will work.
Maps supported:
- Satellite
- Roads
TEST NOTES
For now testing is manually. In the test repository there is a google_basemap.qgs
project. You can configure the Lizmap plugin by adding your own Google Api Key
and then check the functionality.
Funded by Faunalia
Can you rename your commit, like a similar title as your PR ? it will make the git history better.
So it works if you use QMS AND you set a API key in the plugin right ? Sorry, I see you description, we will have to warn users about some corner cases. Honestly, for me using Google without an API key is discouraged as it's against Google TOS.
Yes, as BingMaps. If you don't specify any KEY in Lizmap plugin, then the basemap
will be loaded as generic external layer.
Do you think this behavior should be prevented and we should raise an error in this case? IMHO it's up to the user to make this choice
Maybe it's done already ?
Yes, it is already done
Google Maps TOS is quite restrictive and some aspect are rather unclear IMHO. For example, take this paragraph extracted from https://cloud.google.com/maps-platform/terms:
(e) No Use With Non-Google Maps. To avoid quality issues and/or brand confusion, Customer will not use the Google Maps Core Services with or near a non-Google Map in a Customer Application. For example, Customer will not (i) display or use Places content on a non-Google map, (ii) display Street View imagery and non-Google maps on the same screen, or (iii) link a Google Map to non-Google Maps content or a non-Google map.
I don't fully understand this point: there are a lot of cases (over 90% I guess?) where "Customer WILL USE the Google Maps Core Services with or near a non-Google Map" and they also paying to use these API. If this is not allowed, I'm wondering about the utility of integrate a Google Maps in any Costumer Application. My idea is that Google wants to avoid being associated to any improper use of their maps and to do so it has inserted a general clause and then possibly enforced it in case there were disputes of some kind. I don't think that Lizmap fits into this case.
Do you think this behavior should be prevented and we should raise an error in this case? IMHO it's up to the user to make this choice
Maybe a new employee in an organization is not aware about publication rule and push a project with a Google layer and/or the Lizmap system administrator (provider) want to be clear about TOS used on his own server.
Would it be possible to have a config in the ini
file about this ?
In lizmap/var/config/profiles.ini.php.dist
[tosCheckExternalTiles:default]
;google=yes
;bing=no
Do you think this behavior should be prevented and we should raise an error in this case? IMHO it's up to the user to make this choice
Maybe a new employee in an organization is not aware about publication rule and push a project with a Google layer and/or the Lizmap system administrator (provider) want to be clear about TOS used on his own server.
Would it be possible to have a config in the
ini
file about this ?In
lizmap/var/config/profiles.ini.php.dist
[tosCheckExternalTiles:default] ;google=yes ;bing=no
Ok for adding a configuration for this, but instead of storing it in the file profiles.ini.php
, better use the dedicated Lizmap lizmapConfig.ini.php
which is editable with the admin interface.
We should mark this option as sensitive, hidden when the flag hideSensitiveServicesProperties
equals 1 (or True)
https://github.com/3liz/lizmap-web-client/blob/master/tests/docker-conf/phpfpm/lizmapConfig.ini.php#L3
Editing can be done with https://github.com/3liz/lizmap-web-client/blob/master/lizmap/modules/admin/controllers/config.classic.php
@mind84 can you add the changes requested by @Gustry and @mdouchin ?
@mind84 can you add the changes requested by @Gustry and @mdouchin ?
Hi @rldhont!
I think I'll need some clarifications about these changes, probably next week I could start working on this.
Thanks
Thanks @mind84
Later, we will forward this new flag in the JSON metadata to make the plugin aware about these layers.
Hi @Gustry !
For now I have added the additional configuration in the Lizmap admin panel in the existing Interface
section. I chose this section because it seemed suitable for the purpose, but also because I don't want to add further sections.
Any suggestion about labes or options placement would be appreciated :)
@mdouchin they are also marked as sensitive, so when the corresponding flag is enabled, they do not appear in the config interface.
So, this will probably be a very silly question, but I want to be sure about the logic after this point. If, say, Google
is disabled, then it should not appear as baselayer and should be removed from baselayers list? In this case I think the best way to do it is to remove it from the getProjectConfig
service response.
Thanks!
Hi @Gustry @mdouchin !
Do you have any feedback on this?
If, say, Google is disabled, then it should not appear as baselayer and should be removed from baselayers list? In this case I think the best way to do it is to remove it from the getProjectConfig service response.
Thanks!
Sorry, I missed your question. I would say yes, to "remove the layer" on the server side, so the frontend won't be aware.
Later, we will be able to forward these values in the JSON metadata from the server : lizmap/modules/lizmap/lib/Server/Server.php
like
"authorized_external_providers" : ["google", "bing"],`
I can plan a dev on the Lizmap plugin side.
If, say, Google is disabled, then it should not appear as baselayer and should be removed from baselayers list? In this case I think the best way to do it is to remove it from the getProjectConfig service response.
I think we should indeed remove the layer from the baselayer list. I a not sure though that removing it only from getProjectConfig
would be enough, @rldhont & @nboisteault would know best.
Side question : what about if the layer is not in the baselayers group, but anywhere else ? Not sure this PR should tackle it, but we should think about it too.
Hi @mdouchin, @rldhont, @nboisteault,
I'm working on it, but several questions arose during the process.
I think we should indeed remove the layer from the baselayer list. I a not sure though that removing it only from getProjectConfig would be enough
If I remove the layer from the config then Lizmap throws an error in console, eg:
The WMS layer name Google_Satellite is unknown!
so I think that the bet way to remove these layers from the baselayers is to remove (or rather, avoid to add) the externalAccess->url
property in the config and add a property like 'disabled', and then do some checking via js:
"externalAccess": {
"crs": "EPSG:3857",
"format": "",
"type": "xyz",
"url": "",<----------------
"zmax": "20",
"zmin": "0",
"disabled": true <-----------------------------------
}
This way, also the url is hidden from the client (not a big achievement but it's better IHMO)
what about if the layer is not in the baselayers group, but anywhere else ?
Generally speaking, TOS has nothing to do with the Api Key Provide an API Key should be mandatory in the first place. So the new control on the admin is meant to allow external google and bing WMTS, indipendently from the API key.
This is my proposal (eg for Google, extended to Bing end so on):
Map Tiles Enabled | Base Layers | Lizmap Layers |
---|---|---|
enableGoogleMapsTiles = ON |
API Key IS SETTED Load WMTS trough OpenLayers API API Key IS NOT SETTED avoid to display the layer in the baselayer and console.warn() that the api key is needed (avoid also to call the OL API if the the key is not provided) |
There is no control on the API Key in this scenario, the layer is loaded by QGIS server so I think should be displayed according to the enableGoogleMapsTiles = on |
enableGoogleMapsTiles = OFF | No external google WMTS should be displayed, despite API Key |
Please let me know if I completely misinterpreted the request.
Thanks
EDIT:
If I remove the layer from the config then Lizmap throws an error in console
This also happens when a layer is only visible to certain groups:
In console:
To keep the same logic, probably the option to completely remove the layer from config is still valid
We should have a simple way to disable any group or layer to be displayed in the layer tree & the map. cc @rldhont @nboisteault We should keep it safe and simple.
I don't think the logic was correct.
enableGoogleMapsTiles = OFF → No external google WMTS should be displayed, despite API Key
-
In the config INI file, does the server has a strict check of TOS for Google or Bing ? :
- Yes, so any layer loaded with QGIS Desktop for Google or Bing must a have valid API key. If no API key provided, the layer is discarded from the project.
-
No, current behavior of Lizmap, nothing is checked :
- If API key is set, well, all good, it works.
- If not API key, maybe it works, maybe not. It depends of the provider, it seems #4192 Bing is not working. But I know Google works sometimes ... 🤷
-
Not set in Ini file, we will choose a default value, set to
yes
for instance.
But maybe, the first PR is to do the PHP config file lizmapConfig.ini.php
and the forward in the JSON. Do you want me to have a look ?
@mind84 We have discussed. We are fine with the current state of the PR, Javascript only. Can you wait a little ?
We are fine with the current state of the PR, Javascript only. Can you wait a little ?
Of course, if you want I have also the updated PR on my local machine with the admin panel part, the metadata service updated (for lizmap plugin) and the controls on the the layers (remove layers when enableGoogleMapsTiles = OFF, for instance)
If could share it without push it, if you just want to take a look.
Let me know!
Thanks!
Hi @mind84
You can look at my PR https://github.com/3liz/qgis-lizmap-server-plugin/pull/80
This will discard any Google or Bing Layers from the GetCapibilities
, so it will make your PR easier. That's why you can keep this PR with JavaScript only, because everything will be done upfront, in QGIS server.
Sorry you started the form in Lizmap administration panel ! It looks more logical for me as well to have a UI to set up this configuration, that's I talked about this solution.
But thinking about it, it's way easier that layers are discarded straight from QGIS server response.
@Gustry , Ok thanks, looks reasonable to me too.
Side question:
You can look at my PR https://github.com/3liz/qgis-lizmap-server-plugin/pull/80
The overall logic is pretty simple, but I'm not very familiar with the Lizmap plugin server code. How can I enable/disable the ApiKey
control?
Thanks
The overall logic is pretty simple, but I'm not very familiar with the Lizmap plugin server code. How can I enable/disable the ApiKey control?
I just pushed again on the PR. Can you have a look to the README.md
explanation, and let me know if it's clear ? I would prefer to explain through the doc instead of here ;-)
We can merge if you want, so you would have a zip on https://packages.3liz.org/pub/lizmap_server-qgis-plugin/unstable/ or on custom QGIS repository https://packages.3liz.org/pub/server-plugins-repository/unstable/
I just pushed again on the PR. Can you have a look to the
README.md
explanation, and let me know if it's clear ? I would prefer to explain through the doc instead of here ;-)
Ok! For me it's clear. Sys admin can set environmental variables (e.g. configured in docker compose or whatever) to enable/disable the Api Key check.
Side note on TOS: Providing the API key in the request does not guarantee compliance with the TOS. I only point this out because from the README
it seems that, by switching the API Key control from False
to True
, the system becomes TOS compliance too.
We can merge if you want
No rush for me at all, can wait for an official stable release
Thanks!
Ok! For me it's clear. Sys admin can set environmental variables (e.g. configured in docker compose or whatever) to enable/disable the Api Key check.
Yes it's the purpose. I was wondering if just a single variable would be OK, because I don't think there is a use case to enable just one and the other one. But let's go like this.
I only point this out because from the README it seems that, by switching the API Key control from False to True, the system becomes TOS compliance too.
Yes true, I will see how I will rephrase on that. I will merge, so the ZIP will be available. Maybe after in the documentation docs.lizmap.com
Backports ?
Backports ?
For me, yes
Thanks!
Cool :) @mind84 Could you try with the latest version of the plugin and was it successful ?
Could you try with the latest version of the plugin and was it successful ?
yep, I'll keep you updated
@Gustry @rldhont,
Server side seems all good to me. I've added both env variables on the docker-compose
:
and system responds as documented.
Problem is, when the tos_check
removes all layers from the getCapabilities response, the baseLayers
group ends up being empty, and then the client crashes:
Not a big deal I think, I can fix it but it will probably take some time.
I'll keep you updated
Thanks
Not a big deal I think, I can fix it but it will probably take some time.
Nice thanks. Indeed, at least skipping "nicely" the loop if it's empty would be better.
Because in your case, you fall back on https://github.com/3liz/lizmap-web-client/issues/4029 which was "fixed" with a fix on the plugin side https://github.com/3liz/lizmap-plugin/issues/540 but I didn't expect the baselayers
group to become empty when layers are discarded on runtime.
Thanks for this patch anyway !
What do you think about adding a very basic test loading
google_basemap.qgs
and just testing no error is raised
Yes I could add a basic test, of course it will not be capable to test the whole functionality but its better than nothing
Thanks
Yes I could add a basic test, of course it will not be capable to test the whole functionality but its better than nothing
No sure, we do not want to test the OpenLayers Google class :)
Just that the map load correctly, and there isn't any JS error when loading the map. Maybe similar to what you have done for Bing https://github.com/3liz/lizmap-web-client/blob/master/tests/end2end/playwright/bing-basemap.spec.js