Server OGC API: fix url path match #45450
Conversation
elpaso
added
Bug
|
CC @dmarteau can you give this a try? |
|
@elpaso I'm doing that ATM. |
|
Here is the results of same runs as #45439 : Tests with query params using: The only thing a little confusing at start is that the match is not an exact match but rather finding a substring that match.
But that's fine - since I can still use the exact match for the pattern (I am more acustomed to exact match when using web server framework) and changing this behavior may introduce some regression in existing implementations. May be this should be emphasized in the documention. Thanks for the fix. |
|
@elpaso I think I found another issue when computing href with the Example: if I use Using
|
|
@dmarteau the root path is designed to be like a namespace (possibly configurable) and it's not part of the handlers API, that's why it's not a regexp pattern. Considering WFS3 as an example, |
I understand, but actually if I use a pattern like |
Sorry but I find this discussion hard to follow: returned from what? Would you mind filing a separate ticket if this is unrelated from this PR? |
|
@elpaso have you some time to spare for a chat ? |
Not now, sorry :/ |
This is a direct consequence of this PR (otherwise you just bump into the problem describe in the issue #45439) I'm trying to rephrase the problem: The context
Then I expect Expected resultIf the request is Actual resultThe value I get for I can workaround this if I prefix my handler regexp with And if I try to workaround the issue by prefixing the handler expression, I have the side effect that computed |
Much clearer now, thanks. If you want to be even more clearer (and make my life easier) you can provide a failing test case. |
Nevermind, got a failing test myself. |
Wait: this passes for me def test_path_capture(self):
"""Test issue GH #45439"""
project = QgsProject()
api = QgsServerOgcApi(self.server.serverInterface(),
'/services/api4', 'apifour', 'a fourth api', '1.2')
h4 = Handler4()
api.registerHandler(h4)
request = QgsBufferServerRequest(
'http://localhost:19876/services/api4/tms/france_parts.json?MAP=france_parts')
response = QgsBufferServerResponse()
server = QgsServer()
iface = server.serverInterface()
iface.serviceRegistry().registerApi(api)
server.handleRequest(request, response)
self.assertEqual(h4.params, {'tilemapid': 'france_parts.json'})
ctx = QgsServerApiContext(api.rootPath(), request, response, project, iface)
self.assertEqual(h4.href(ctx), 'http://localhost:19876/services/api4/tms/france_parts?MAP=france_parts')And now I realize that you claim is wrong:
You are missing a trailing slash, the regexp won't match the URL. |
|
Here is the result of my failing tests: (I am building a simpler one) Request is Case 1: wrong result for Case 2 (prefixed): ok for Note that in both case href is wrong, adding a trailing slash to the url does not change the result. |
|
Here is may failing test: """ Failing test for https://github.com/qgis/QGIS/pull/45450
Must be run using https://github.com/elpaso/QGIS/tree/bugfix-gh45439-server-api-path-match
"""
import os
from qgis.PyQt.QtCore import QRegularExpression
from qgis.core import Qgis, QgsMessageLog, QgsApplication
from qgis.server import (
QgsServerOgcApi,
QgsServerOgcApiHandler,
QgsBufferServerRequest,
QgsBufferServerResponse,
QgsServer,
)
class TestHandler(QgsServerOgcApiHandler):
def __init__(self, path: str):
super().__init__()
self._path = QRegularExpression(path)
self._name = 'test'
def handleRequest(self, context):
"""
"""
values = self.values(context)
print("#####> Values",values)
print("#####> href", self.href(context,"/test", "json"))
context.response().setStatusCode(200)
def path(self):
return self._path
def linkType(self):
return QgsServerOgcApi.items
def operationId(self):
return f"{self._name}"
def summary(self):
return f"Tiles {self._name}"
def description(self):
return f"Tiles {self._name}"
def linkTitle(self):
return f"Tiles {self._name}"
def templatePath(self, context):
# No templates!
return ''
def parameters(self, context):
return []
def test_pr45540() -> None:
"""
"""
os.environ['QT_QPA_PLATFORM'] = 'offscreen'
qgis_application = QgsApplication([], False)
qgis_application.initQgis()
install_logger_hook()
server = QgsServer()
iface = server.serverInterface()
api = QgsServerOgcApi(iface, '/myapi','Test PR 45450')
handler = TestHandler('/(?P<name>[^/]+)/?')
api.registerHandler(handler)
iface.serviceRegistry().registerApi(api)
request = QgsBufferServerRequest('http://localhost:19876/myapi/foobar?MAP=tests/data/france_parts.qgs')
response = QgsBufferServerResponse()
server.handleRequest(request, response)
qgis_application.exitQgis()
def install_logger_hook() -> None:
""" Install message log hook
"""
from qgis.core import Qgis, QgsApplication, QgsMessageLog
# Add a hook to qgis message log
def writelogmessage(message, tag, level):
arg = f'{level}:{tag}: {message}'
print(arg, flush=True)
messageLog = QgsApplication.messageLog()
messageLog.messageReceived.connect( writelogmessage )
test_pr45540()Results: |
|
@dmarteau works for me if you use this regexp: |
|
@dmarteau your test now passes. |
elpaso
force-pushed
the
bugfix-gh45439-server-api-path-match
branch
from
d61df93
to
2b1a1c1
Compare
Handle the messy initial / by making sure the paths matches are evaluated against a sanitized URL that always starts with a /
|
The backport to To backport manually, run these commands in your terminal: # Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add .worktrees/backport-queued_ltr_backports queued_ltr_backports
# Navigate to the new working tree
cd .worktrees/backport-queued_ltr_backports
# Create a new branch
git switch --create backport-45450-to-queued_ltr_backports
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick 7cd8f0931dfb61bb96f8c0429a0896e7b0ab8434,9df14378142605f282841f175deff4bb9ee75660,2b1a1c198682ce8f8b60338997fcc8e3413a151e,8ba18051e42196e69d7a27da23f36b240ff4c771
# Push it to GitHub
git push --set-upstream origin backport-45450-to-queued_ltr_backports
# Go back to the original working tree
cd ../..
# Delete the working tree
git worktree remove .worktrees/backport-queued_ltr_backportsThen, create a pull request where the |
Fixes #45439