Add POST support to WFS GetFeature (Issue #439) #706
Conversation
Small change to allow testing the use of a xml query via post.
Unimplemented args have been added for consistency with other getfeature functions qui a log warning
|
I'm not familiar with this part of the code, but the use case for us to send requests to GeoServer that exceed the size of GET requests. For example, we're sometime making queries for properties for thousands of individual features, and the list of features does not fit in GET requests. So we'd be really grateful if this could be reviewed. |
|
test failures not related to this PR. |
|
Just added #711 which this PR should solve |
| if not typename and filter: | ||
| return base_url, filter | ||
|
|
||
| request = self.create_post_request() |
There was a problem hiding this comment.
I did not understand why using a create_post_request function instead of directly inserting the version-dependent Postrequest object creation. Is this function intended to be used anywhere else ?
There was a problem hiding this comment.
Well, my aim here was to make the post request creation more testable. It being separated makes it testable by itself.
I chose to make a factory with create_post_request(), so getPOSTGetFeatureRequest() can be used for wfs1.1.0 and 2.0.0 without needing another function that does almost the same thing but not quite, which also seems to be what getGetGetFeatureRequest() is aiming to become.
So this way, getFeature(), whether it's with 1.1.0 or 2.0.0, calls the same function, which checks the WFS version, and builds the appropriate request format. It will also make it easier (I hope) to integrate the next WFS version by simply extending the postrequest.py module so it can format post requests for that new version. All without needing to change getPOSTGetFeatureRequest() or creating a new function.
On the plus side, postrequest.py can also be used by itself to build a request and use it with resquests.post(), which could be useful in identifying if an error is caused by the request itself or if the problem is somewhere else in OWSLib.
|
@KorkXx @f-PLT @tomkralidis If the review is ok, can we merge it? |
|
+1 |
|
We have an issue with a usecase for one of our partners. If it can't be handled server-side, we'll probably have a small change to add so it's possible to handle an https redirection, which doesn't seem to work with Post requests. Right now, considering a simple flag to prevent the GetCapabilities url retrieval when needed, and force the use of the provided WebFeatureService url as is. (for reference : https://github.com/f-PLT/OWSLib/blob/wfs-getfeature-post/owslib/feature/__init__.py#L312-L320) |
|
Alright, it's definitely a server-side problem so we'll fix it server-side. If there are no other objections after the testing has been done , it should be ok to merge. As for the problem, here's a summary as it's bound to happen to other people. Post requests are not redirected. Ever, and should never be. It's in the HTTP standards. (Learn something everyday!) Therefore if the the Url listed in the GetCapabilities for post getFeature() is The server needs to be configured to give the proper url. In the case of Geoserver, it should be the This doesn't happen with Get requests, because a Get can be redirected without any problem. Hope this little bit of information can help someone someday, and if I'm wrong in any way, please feel free to correct me! |
@f-PLT ok :) Give me a sign ... I can do the merge ... |
For WFS 1.1.0 With Post method now being available, having propertyname='*' as a default value causes problems for the post request. Default value is now None, just like for 2.0.0. Since I don't know if there are cases where this default was necessary, added a condition when `method=get` to set propertynames to '*' is it's not assigned a value.
|
Our tests have gone well! Found and corrected a small unintended side effect while we were at it. If these last three commits pass muster, then it's good to go. If we end up finding more bugs, I'll open a new PR. |
|
failing tests are not related to this PR ... they already fail on current master. |
|
@f-PLT merged. Thanks 👍 |
This PR would take care of issues #439 and possibly #683
This is my first contribution to OWSLib and I'm new to the WFS standard, so I expect there might be a few things to change, either towards the standards or to integrate it better with existing code.
I added the ability to use an existing xml filter as is, like suggested in #439, but I think it might be redundant. Either way, it's self contained and only 2 lines of code, so it's easy to remove if need be.
I can already say I'm not sure about how I handled Stored Queries.