From 6a03afc65df168cf60b83bc34c1b9b94c648c629 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=B6ren=20Tempel?= Date: Sat, 25 Apr 2020 18:26:58 +0200 Subject: [PATCH 1/7] web plugin: support path queries by separating them with a backslash 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]: https://github.com/pallets/flask/issues/900 --- beetsplug/web/__init__.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/beetsplug/web/__init__.py b/beetsplug/web/__init__.py index 21ff5d94e..49149772d 100644 --- a/beetsplug/web/__init__.py +++ b/beetsplug/web/__init__.py @@ -177,10 +177,11 @@ class QueryConverter(PathConverter): """ def to_python(self, value): - return value.split('/') + queries = value.split('/') + return [query.replace('\\', os.sep) for query in queries] def to_url(self, value): - return ','.join(value) + return ','.join([v.replace(os.sep, '\\') for v in value]) class EverythingConverter(PathConverter): From 57a759c8175eedabfddbd3a02e33b3af73fd13e7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=B6ren=20Tempel?= Date: Sun, 26 Apr 2020 17:39:16 +0200 Subject: [PATCH 2/7] docs: document joining of paths performed by web plugin for queries --- docs/plugins/web.rst | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/docs/plugins/web.rst b/docs/plugins/web.rst index d416b1b7d..99214ef01 100644 --- a/docs/plugins/web.rst +++ b/docs/plugins/web.rst @@ -210,7 +210,10 @@ If the server runs UNIX, you'll need to include an extra leading slash: ``GET /item/query/querystring`` +++++++++++++++++++++++++++++++ -Returns a list of tracks matching the query. The *querystring* must be a valid query as described in :doc:`/reference/query`. :: +Returns a list of tracks matching the query. The *querystring* must be a +valid query as described in :doc:`/reference/query`. Path elements are +joined as query keywords. For example, ``/item/query/foo/bar`` will be +converted to the query ``foo,bar``. :: { "results": [ From 226d8089e1ff16687bff1731f048e8e4ba130db4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=B6ren=20Tempel?= Date: Sun, 26 Apr 2020 17:45:37 +0200 Subject: [PATCH 3/7] docs: document special handling of backlash in web plugin queries --- docs/plugins/web.rst | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/docs/plugins/web.rst b/docs/plugins/web.rst index 99214ef01..481727475 100644 --- a/docs/plugins/web.rst +++ b/docs/plugins/web.rst @@ -213,7 +213,11 @@ If the server runs UNIX, you'll need to include an extra leading slash: Returns a list of tracks matching the query. The *querystring* must be a valid query as described in :doc:`/reference/query`. Path elements are joined as query keywords. For example, ``/item/query/foo/bar`` will be -converted to the query ``foo,bar``. :: +converted to the query ``foo,bar``. As this conflicts with using a slash +character as a path seperator in path queries, a backlash character +should be used in these queries instead. This character is converted to +the path seperator actually used by the operating system before the +query is performed. :: { "results": [ From 969a2074186d277054c364d249320f3518edfaf0 Mon Sep 17 00:00:00 2001 From: Adrian Sampson Date: Mon, 27 Apr 2020 07:45:31 -0400 Subject: [PATCH 4/7] Simplify docs for #3567 --- docs/plugins/web.rst | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/docs/plugins/web.rst b/docs/plugins/web.rst index 481727475..65d4743fb 100644 --- a/docs/plugins/web.rst +++ b/docs/plugins/web.rst @@ -211,13 +211,7 @@ If the server runs UNIX, you'll need to include an extra leading slash: +++++++++++++++++++++++++++++++ Returns a list of tracks matching the query. The *querystring* must be a -valid query as described in :doc:`/reference/query`. Path elements are -joined as query keywords. For example, ``/item/query/foo/bar`` will be -converted to the query ``foo,bar``. As this conflicts with using a slash -character as a path seperator in path queries, a backlash character -should be used in these queries instead. This character is converted to -the path seperator actually used by the operating system before the -query is performed. :: +valid query as described in :doc:`/reference/query`. :: { "results": [ @@ -226,6 +220,11 @@ query is performed. :: ] } +Path elements are joined as parts of a query. For example, +``/item/query/foo/bar`` will be converted to the query ``foo,bar``. +To specify literal path separators in a query, use a backslash instead of a +slash. + ``GET /item/6/file`` ++++++++++++++++++++ From f43a0c3270d18af19affea04fae9fb39ab02d784 Mon Sep 17 00:00:00 2001 From: Adrian Sampson Date: Mon, 27 Apr 2020 07:46:28 -0400 Subject: [PATCH 5/7] Changelog/thanks for #3567 --- docs/changelog.rst | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/docs/changelog.rst b/docs/changelog.rst index e2f1521eb..5d8c26bb9 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -115,6 +115,10 @@ New features: :bug:`3459` * :doc:`/plugins/fetchart`: Album art can now be fetched from `last.fm`_. :bug:`3530` +* :doc:`/plugins/web`: The query API now interprets backslashes as path + separators to support path queries. + Thanks to :user:`nmeum`. + :bug:`3567` Fixes: From a553693677e8cc11c645efa94d11b1669fd0e35c Mon Sep 17 00:00:00 2001 From: pants108 Date: Sun, 3 May 2020 00:06:55 +0000 Subject: [PATCH 6/7] fetchart: clean up invalid tmp files --- beetsplug/fetchart.py | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/beetsplug/fetchart.py b/beetsplug/fetchart.py index 763440238..d4ab714de 100644 --- a/beetsplug/fetchart.py +++ b/beetsplug/fetchart.py @@ -210,6 +210,9 @@ class ArtSource(RequestMixin): def fetch_image(self, candidate, plugin): raise NotImplementedError() + def cleanup_tmp(self, candidate): + raise NotImplementedError() + class LocalArtSource(ArtSource): IS_LOCAL = True @@ -218,6 +221,10 @@ class LocalArtSource(ArtSource): def fetch_image(self, candidate, plugin): pass + def cleanup_tmp(self, candidate): + # local art source does not create tmp files + pass + class RemoteArtSource(ArtSource): IS_LOCAL = False @@ -291,6 +298,13 @@ class RemoteArtSource(ArtSource): self._log.debug(u'error fetching art: {}', exc) return + def cleanup_tmp(self, candidate): + if candidate.path: + try: + util.remove(path=candidate.path) + except util.FilesystemError as exc: + self._log.debug(u'error cleaning up tmp art: {}', exc) + class CoverArtArchive(RemoteArtSource): NAME = u"Cover Art Archive" @@ -1017,6 +1031,8 @@ class FetchArtPlugin(plugins.BeetsPlugin, RequestMixin): u'using {0.LOC_STR} image {1}'.format( source, util.displayable_path(out.path))) break + # else: remove tmp images created by this invalid candidate + source.cleanup_tmp(candidate) if out: break From 1a79ae5a9f511394ed5767e270bd9737110717f6 Mon Sep 17 00:00:00 2001 From: pants108 Date: Tue, 5 May 2020 18:39:05 +0000 Subject: [PATCH 7/7] code review 1 --- beetsplug/fetchart.py | 14 +++++--------- docs/changelog.rst | 4 +++- 2 files changed, 8 insertions(+), 10 deletions(-) diff --git a/beetsplug/fetchart.py b/beetsplug/fetchart.py index d4ab714de..70477a624 100644 --- a/beetsplug/fetchart.py +++ b/beetsplug/fetchart.py @@ -210,8 +210,8 @@ class ArtSource(RequestMixin): def fetch_image(self, candidate, plugin): raise NotImplementedError() - def cleanup_tmp(self, candidate): - raise NotImplementedError() + def cleanup(self, candidate): + pass class LocalArtSource(ArtSource): @@ -221,10 +221,6 @@ class LocalArtSource(ArtSource): def fetch_image(self, candidate, plugin): pass - def cleanup_tmp(self, candidate): - # local art source does not create tmp files - pass - class RemoteArtSource(ArtSource): IS_LOCAL = False @@ -298,7 +294,7 @@ class RemoteArtSource(ArtSource): self._log.debug(u'error fetching art: {}', exc) return - def cleanup_tmp(self, candidate): + def cleanup(self, candidate): if candidate.path: try: util.remove(path=candidate.path) @@ -1031,8 +1027,8 @@ class FetchArtPlugin(plugins.BeetsPlugin, RequestMixin): u'using {0.LOC_STR} image {1}'.format( source, util.displayable_path(out.path))) break - # else: remove tmp images created by this invalid candidate - source.cleanup_tmp(candidate) + # Remove temporary files for invalid candidates. + source.cleanup(candidate) if out: break diff --git a/docs/changelog.rst b/docs/changelog.rst index 5d8c26bb9..c09b1b8ab 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -122,9 +122,11 @@ New features: Fixes: -* :doc:`/plugins/fetchart`: Fixed a bug that caused fetchart to not take +* :doc:`/plugins/fetchart`: Fixed a bug that caused fetchart to not take environment variables such as proxy servers into account when making requests :bug:`3450` +* :doc:`/plugins/fetchart`: Temporary files for fetched album art that fail + validation are now removed * :doc:`/plugins/inline`: In function-style field definitions that refer to flexible attributes, values could stick around from one function invocation to the next. This meant that, when displaying a list of objects, later