Skip to content
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

Support ArcGIS WMS GetFeatureInfo requests using GeoJSON format #5267

Merged
merged 4 commits into from Nov 6, 2019

Conversation

adube
Copy link
Contributor

@adube adube commented Oct 30, 2019

This patch introduces the support of WMS GetFeatureInfo requests made on WMS layers served by ArcGIS.

A GeoJSON format is used as INFO_FORMAT, since it seems to be the only one supported by ArcGIS. The format returns a layerName property for each feature, which identifies the layer it corresponds to. In order to read that information, a custom ngeo.format.ArcGISGeoJSON format is created in this patch.

How to test this patch

  • run the desktop_all app
  • clear all layers
  • select the "Demo" theme
  • remove all groups with the exception of "Externals"
  • set visible the following layer: NPA localite noWFS
  • zoom out / pan the map until you see the layer (yellow lines)
  • click on features
  • --> you should get results in the grid below

Known limitation

ArcGIS (at least with the layer used in the test above) does not return geometry. Therefore, it's hard to see where those features are as you need to remember where you clicked.

Known issue

While working on this patch, I found an issue: sometimes results are duplicated. This is not caused by this patch, therefore a separate PR should come soon to fix this.

@adube adube requested review from fredj and sbrunner October 30, 2019 15:36
Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@adube adube force-pushed the gsgmf-1060-support-arcgis-wms-query branch from 09907f8 to a675f51 Compare October 31, 2019 15:44
@adube
Copy link
Contributor Author

adube commented Oct 31, 2019

Travis is happy.

@sbrunner and/or @fredj : ready for review.

@sbrunner
Copy link
Member

sbrunner commented Nov 1, 2019

I create a new group named ESRI no WFS no Geom with tow layers in the same OpenLayer layer, to see if the layername is working, and it's not the case...

@adube
Copy link
Contributor Author

adube commented Nov 1, 2019

I create a new group named ESRI no WFS no Geom with tow layers in the same OpenLayer layer, to see if the layername is working, and it's not the case...

Thanks for making this test.

I'm not sure I understand what the test is about nor what is not actually working. Would you please add more details to your explanation? What is the expected behaviour?

Trying to query that group and I can see results. I can't find where there are features on the second layer thought. Thanks.

query-with-group-seemed-to-work

@sbrunner
Copy link
Member

sbrunner commented Nov 1, 2019

Open:
https://camptocamp.github.io/ngeo/gsgmf-1060-support-arcgis-wms-query/examples/contribs/gmf/apps/desktop.html?lang=fr&tree_groups=OSM%20functions%20mixed%2CLayers%2CGroup%2COSM%20functions%2CExternal%2CFilters%20mixed%2CFilters%2CESRI%20no%20WFS%20no%20Geom&tree_group_opacity_Group=0.65&tree_enable_osm_scale=false&tree_group_layers_Layers=&tree_group_layers_Group=&tree_group_layers_OSM%20functions=&tree_enable_ch.swisstopo.dreiecksvermaschung=false&tree_enable_ch.swisstopo.geologie-gravimetrischer_atlas=false&tree_enable_ch.swisstopo.geologie-geotechnik-gk500-lithologie_hauptgruppen=false&tree_enable_ch.swisstopo.geologie-geotechnik-gk500-gesteinsklassierung=false&tree_enable_NPA%20localite=false&tree_enable_NPA%20localite%20noWFS=false&tree_enable_ch.bfe.solarenergie-eignung-fassaden=false&tree_enable_ch.bfe.solarenergie-eignung-daecher=false&tree_enable_osm_open=false&tree_enable_OSM%20map=false&tree_enable_osm_time_r_dp_2=false&tree_group_layers_Filters=&tree_group_layers_ESRI%20no%20WFS%20no%20Geom=&baselayer_ref=blank&tree_enable_Layer%20with%20very%20very%20very%20very%20very%20long%20name=false&tree_enable_osm_time_r_s=false&tree_enable_osm_time_v_s=false&tree_enable_osm_time_v_dp=false&tree_enable_osm_time_r_dp=false&tree_enable_osm_time_r_year_mounth=false&tree_enable_osm_time_d_year_mounth=false&tree_enable_osm_time_r_mounth_year=false&tree_enable_osm_time_v_mounth_year=false&tree_enable_long%20wms-t%20layer%20name%20name%20name%20name%20name%20name%20name=false&tree_enable_osm_time_r_dp_default=false&tree_enable_osm_time_r_s_default=false&tree_enable_osm_time_v_dp_default=false&tree_enable_osm_time_v_s_default=false&tree_enable_two_layers=false&tree_enable_ch.astra.ausnahmetransportrouten=false&tree_enable_no_wfs=false&tree_enable_ch.are.alpenkonvention=false&map_x=2529526&map_y=1158566&map_zoom=5

  • Select the group ESRI no WFS no Geom
  • Click on a yellow zone

I should get 2 features one on vd.npa_localite, one on vd.regime_hydrolique, and I get 4 features 2 on each layers...

@adube
Copy link
Contributor Author

adube commented Nov 1, 2019

The above link returns a 404 error. Would you please provide one that uses the desktop_alt application? (that one should work)

@sbrunner
Copy link
Member

sbrunner commented Nov 1, 2019

Comment updated :-)

@adube
Copy link
Contributor Author

adube commented Nov 1, 2019

Okay, I'll investigate why.

@adube
Copy link
Contributor Author

adube commented Nov 1, 2019

Okay, it doesn't work indeed. We'll need to patch OpenLayers to make the format do the same thing as the WMS GetFeatureInfo one, however this property is vendor-param, i.e. layerName is not an official property supported by the GeoJSON spec: https://tools.ietf.org/html/rfc7946

@fredj I suggest that we implement new options in the OpenLayers "GeoJSON" format:

  • layerProperty, which would be given the value "layerName"
  • layers, which would act exactly like the option of the same name in WMS GetFeatureInfo.

Please, let me know what you think about this.

@adube adube changed the title Support ArcGIS WMS GetFeatureInfo requests using GeoJSON format [WIP] Support ArcGIS WMS GetFeatureInfo requests using GeoJSON format Nov 1, 2019
@fredj
Copy link
Member

fredj commented Nov 4, 2019

@adube could it be done by extending the GeoJSON class from OL ?
I think it would be a good start to create a custom class (ArcGISGeoJSON or GeoJSONGetFeatureInfo ?) in src/format

@adube
Copy link
Contributor Author

adube commented Nov 4, 2019

Alright, I'll try that.

@sbrunner sbrunner added this to the 2.5 milestone Nov 4, 2019
@adube adube force-pushed the gsgmf-1060-support-arcgis-wms-query branch from 9cc4736 to 47787d7 Compare November 4, 2019 16:47
@adube adube changed the title [WIP] Support ArcGIS WMS GetFeatureInfo requests using GeoJSON format Support ArcGIS WMS GetFeatureInfo requests using GeoJSON format Nov 4, 2019
@adube
Copy link
Contributor Author

adube commented Nov 4, 2019

Fixed.

I've noticed an other issue. I'll create a bug/task about it, as I could reproduce it on 'master' branch: results are shown twice, i.e. they have duplicates.

@adube adube force-pushed the gsgmf-1060-support-arcgis-wms-query branch 3 times, most recently from f15821c to 73516f7 Compare November 5, 2019 13:19
@adube
Copy link
Contributor Author

adube commented Nov 5, 2019

@sbrunner and/or @fredj Ready for review.

@adube adube force-pushed the gsgmf-1060-support-arcgis-wms-query branch from 73516f7 to d693d51 Compare November 5, 2019 13:35
@sbrunner
Copy link
Member

sbrunner commented Nov 5, 2019

Except the duplicated issue that's looks good to me (I can't reproduce it on MapServer layers).

@sbrunner
Copy link
Member

sbrunner commented Nov 5, 2019

Can you rebase on master, I think that the Travis is solved here: 24b8625

@adube
Copy link
Contributor Author

adube commented Nov 5, 2019

This patch is already up-to-date onto master.

@adube adube force-pushed the gsgmf-1060-support-arcgis-wms-query branch from d693d51 to c9cc939 Compare November 5, 2019 14:47
@adube
Copy link
Contributor Author

adube commented Nov 5, 2019

Trying "commit --amend" for the 3rd time.

@adube
Copy link
Contributor Author

adube commented Nov 5, 2019

Travis is always failing... Please, let me know if you come up with a fix so that I can rebase onto it. Thanks.

@adube adube force-pushed the gsgmf-1060-support-arcgis-wms-query branch from c9cc939 to deff468 Compare November 5, 2019 17:01
@adube
Copy link
Contributor Author

adube commented Nov 5, 2019

@sbrunner and/or @fredj Travis is happy. Ready for review!

@sbrunner
Copy link
Member

sbrunner commented Nov 6, 2019

Same than before: Except the duplicated issue that's looks good to me (I can't reproduce it on MapServer layers).

@adube
Copy link
Contributor Author

adube commented Nov 6, 2019

Same than before: Except the duplicated issue that's looks good to me (I can't reproduce it on MapServer layers).

image

@sbrunner
Copy link
Member

sbrunner commented Nov 6, 2019

Effectively, it should be fixed but it can be in another PullRequest :-)
@fredj can you do a review of the code, for me it's ready :-)

Copy link
Member

@fredj fredj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks very good, thanks

@fredj fredj merged commit b9628c1 into master Nov 6, 2019
@fredj fredj deleted the gsgmf-1060-support-arcgis-wms-query branch November 6, 2019 16:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants