-
Notifications
You must be signed in to change notification settings - Fork 416
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[interoperability] add flag to source when we know it's provided by geoserver #8167
Comments
Proposal:
|
Took a look to the use of the geoserver regex
Additionally the utils allows to set the regex with setRegGeoserverRule inside a downstream project to extend the possible supported GeoServer. Here an example of use case of this property in geonode. We saw that MapStore provides fallbacks for all the cases based on the previous investigation and it expose a utils that allows to extend the supported path in the regex. |
I agree to include also this now.
I think including this option in Layer Settings is not necessary: if, for certain reasons, the server that is hosting the layer changes, the user can always add a new catalog source and add that layer again to the map.
I totally agree @allyoucanmap. Can you please open the new issue including the possibility to make it configurable from Catalog advanced settings? You can assign the issue to me for the moment. |
Oh great, nice finding ! That'll allow me to override it from ms2-geor, i'll play with that. Maybe calling |
Yes, I think that's the correct place. It should be initialized before the main application start |
i've started a first crude wip in landryb@0a5e489 directly on ms2 (ie outside of ms2-georchestra project) but it blows when loading http://localhost:8080/mapstore/?debug=true.
Will keep digging. EDIT: got it; i think http://localhost:8081/?debug=true is the way to have unminified frontend sources. |
still digging, i think i have a better understanding of the bits here and there at stake - the "options" object should have all the bits i need, i think. Will be able to resume work on this from next wednesday. |
@allyoucanmap what i have in master...landryb:MapStore2:wip/8167 is hacky and doesnt work so far. Some issues:
more digging to come today. |
@landryb I would suggest to take a look to this PR #9022 for the part concerning the RasterAdvancedSettings.js component where to correctly apply the property to the layer I used: onChangeServiceProperty("layerOptions", { ...service.layerOptions, singleTile: e.target.checked }) in this case should be onChangeServiceProperty("layerOptions", { ...service.layerOptions, serverType: event?.value }) If you add the property in the layerOptions property it should be directly included in the added layer |
ok, found out why serverType didnt percolate to the layer object - 3d3db6c helps and my layer object has the proper |
thanks, i've looked at #9022 and it seems much simpler than what i'm doing, so should i put the i guess i should also rebase on top of your branch so that all |
Yes, correct, you should put directly serverType inside the layerOptions object without adding a new property. While I was working in the PR I noticed that the layerOptions property was used for the tileSize layer property and then the content of the layerOptions added directly to the layer. To keep it consistent I added also the singleTile layer property and I think we should do the same for serverType.
The PR branch is not on master yet so I would simply suggest to add this line change directly on your code without rebase https://github.com/geosolutions-it/MapStore2/pull/9022/files#diff-8bb020509f0b81ce10ff349328dab6db83acad1349c3f1b58d2f48edbad3be64R179 |
i think i'm using my
returns eg
gives same thing for the available values in the selectbox, by default i have |
@landryb here the options are created only with the keys master...landryb:MapStore2:wip/8167#diff-8bb020509f0b81ce10ff349328dab6db83acad1349c3f1b58d2f48edbad3be64R80. If we want to use the value we should create the options in a different way, this an example: const getServerTypeOptions = () => {
return Object.keys(ServerTypes).map((key) => ({ label: key, value: ServerTypes[key] }));
}; |
great, with that and 734fdd5 i have something that starts working:
what doesn't work yet:
|
=> changing in the selectbox doesnt change the display
=> if undef, the default value is GEOSERVER, i can select NO_VENDOR and the display is updated (CHANGE_SERVICE_PROPERTY is triggered with
=> if undef, the default value is GEOSERVER, and no matter what i select (or try resetting the value) the display stays with GEOSERVER. should the default value be set somewhere else ?
|
having another look at your proposal earlier on in this issue (after having rewatched the minutes of our meeting), im not sure to understand what is expected here. Right now, i'm 'hardcoding' the default server type (eg behaviour when eg |
i've tried applying the logic to the layers added in map of a new dashboard, using:
but that blows because i try to use
i've thought a bit more about moving |
i think i have something that works both for maps & dashboards. @allyoucanmap would love feedback on master...landryb:MapStore2:wip/8167 :) so far i've tested:
selecting |
@landryb I've updated the description including a tasklist and defining missing issues MS side. I would like to assign them to you so that I've added you in the collaborators list with read permissions for the purpose. Can you please accept the invitation? |
I think i was already able to add comments to those issues as a regular contributor, but thanks for adding me to the collaborators team. Regarding the task list, i'm a little confused by the concept because sure, all those issues are 'interoperability-related', but only #7809, #7877 and #7879 will be fixed by this issue. For the others:
|
I was not able to assign issues not created by you without including you in the collaborators list.
The Task List I've added in description is only a way we are using to track all issue involved in a macro task that is including more than one issue (what we call an Epic indeed). When you have created this issue you linked in description all existing interoperability issues together. Generally speaking, the idea here is to have a dedicated issue for each point in Task List and a final PR connected for each. This is the procedure we usually follow for better manage code reviews and functional tests also in our board. |
i've rebased my branch and added some unit tests (fa787e6, 8870c0e, a06931e) for the new functions but i havent figured out how to test that a catalog entry with Trying to run the existing tests in that file, all the Test correctness of the WMS APIs tests fail so i have probably something wrong somewhere:
i have this wip to test the
maybe there should be a test for |
@allyoucanmap : with 9057bca which removes the "hardcoded" default to 'GEOSERVER', in my testing with the default csw (which has no That strange behaviour doesnt reproduce with the two WMS entries where i've pre-set testing 440f8cf shows that the option is still correctly propagated to the |
if undefined or set to "geoserver", the behaviour is the unchanged. If set to "no-vendor", then TILED query parameter isnt sen't to the remote wms server, and the "tiled" and "localizedLayerStyles" options are hidden in the layer parameters. more vendor options can be sent/avoided this way. Improves interoperability when displaying and printing layers coming from mapproxy. closes geosolutions-it#7809, geosolutions-it#7877 & geosolutions-it#7879
@landryb this seems a conflict with the first row of the codemirror library underneat for the Static Filter field available only in CSW, also other select input have similar problem when the options position match the filter line |
@landryb to solve this #8167 (comment) we need to apply a style to the CSW filter container with the following values |
right - hadn't thought about it, that was indeed an issue only for CSW catalogs and not for WMS catalogs, tested with 986ea75 and that bit is now working fine. Thanks ! |
#7809 failure to print WMS layers coming from mapproxy #7877 set TILED=true only when printing layers from geoserver #7879 set TILED=true only when loading layers from geoserver --------- Co-authored-by: allyoucanmap <[email protected]>
…ies (geosolutions-it#9048) geosolutions-it#7809 failure to print WMS layers coming from mapproxy geosolutions-it#7877 set TILED=true only when printing layers from geoserver geosolutions-it#7879 set TILED=true only when loading layers from geoserver --------- Co-authored-by: allyoucanmap <[email protected]>
…g entries (#9048) (#9063) * #8167 add serverType option to wms & csw catalog entries (#9048) #7809 failure to print WMS layers coming from mapproxy #7877 set TILED=true only when printing layers from geoserver #7879 set TILED=true only when loading layers from geoserver --------- Co-authored-by: allyoucanmap <[email protected]> * #8905 Add missing translations (#9055) * #8905 Add missing translations * fix failing tests --------- Co-authored-by: Landry Breuil <[email protected]>
Description
Right now, many features in mapstore2 sadly assume that the remote is geoserver (because the feature was developed and tested only against geoserver) - and it also assumes that the remote geoserver is located at
/geoserver/
which might not be the case (cf https://github.com/geosolutions-it/MapStore2/blob/master/web/client/utils/LayersUtils.js#L25)Consequently, there are many small interoperability issues when using layers coming from non-geoserver sources (eg mapserver, mapproxy, qgis server..).
Tasks
all those issues could be 'easily' solved if in the catalog/layer object there was a boolean flag which said if the remote was geoserver - the corresponding printing/loading code could then be amended to behave differently depending on the remote.
What kind of improvement you want to add? (check one with "x", remove the others)
Other useful information
implementation ideas (from #7811 (comment)):
GetCapabilities
and look forGEOSERVER
in/Service/KeywordList/Keyword
but that assumes the admin doesnt change server keywordsupdateSequence
attribute on the toplevelWMS_Capabilities
XML element ofGetCapabilities
The text was updated successfully, but these errors were encountered: