web plugin: support path queries #3567
Conversation
|
Seems like a good idea overall, but let's do some due diligence to make sure this won't break anything obvious (because it's a breaking change that could cause problems for clients that relied on being able to query with slashes). Unfortunately, a little searching has already turned up one such example: mopidy-beets is using this endpoint for all its music searches. Maybe we need to think creatively here about how to support this kind of query without breaking stuff like that? A new path query endpoint, for example, or a separate style that allows arbitrary query strings? |
One "creative" quick-fix solution I was considering is using windows path separators, i.e. backslash, in the query URL. On Unix systems the web plugin can convert these back to Unix path separators, i.e. slash, before performing the actual query. Obviously a hacky solution but maybe a solution which introduces less breakage? |
|
Yeah, that's a reasonable idea! One other option would be to optionally support an HTTP query string version of the |
Implemented this idea, I am not familiar with flask and werkzeug so not sure if the implementation is entirely correct as is. If you like the proposed implementation I would also add some documentation for both the |
nmeum
changed the title
|
Adjusted the code for list comprehensions, didn't get around to testing this version yet though. Also added some documentation for the query end point of the web plugin. |
Without this change the web plugin does not support path queries as slashes are currently used for joining keywords in the QueryConverter. Moreover, flask cannot distinguish between an URL encoded and a plain '/' character during routing [0]. To work around this issue without introducing a breaking change (i.e. removing the QueryConverter) use the backslash character for path queries and convert it later on. Fixes #3566 [0]: pallets/flask#900
Finally gotten around to testing this again. Found a bug while at it, seems to work now. |
|
Looks great; thanks!! Merged with a few tweaks to the wording. |
As discussed in bug beetbox#3867, backslash replacement in query strings is a bit of a hack but it is useful (see beetbox#3566 and beetbox#3567 for more discussion). However, it breaks many regular expressions so this patch stops the replacement if the query term contains '::', indicating it is a regex match. This commit fixes beetbox#3867. Signed-off-by: Graham R. Cobb <g+beets@cobb.uk.net>
The QueryConverter was added without much further explanation in commit
004e9a8. Unfortunately, the query
converter breaks path queries as flask cannot distinguish between an
URL encoded and a plain '/' character during routing [0]. Additionally,
I don't get why the QueryConverter is needed in the first place as ','
characters can just be used in the URL directly. For example:
/item/query/foo,bar. As conversions of slashes into comma characterswasn't documented anyhow removing this code shouldn't cause much harm.
Fixes #3566