Merge pull request #3731 from jef/jef/fix-subsonic

beetsplug/subsonicupdate.py

@@ -100,16 +100,24 @@ class SubsonicUpdate(BeetsPlugin):
             't': token,
             's': salt,
             'v': '1.15.0',  # Subsonic 6.1 and newer.
-            'c': 'beets'
+            'c': 'beets',
+            'f': 'json'
         }
 
-        response = requests.post(url, params=payload)
+        try:
+            response = requests.get(url, params=payload)
+            json = response.json()
 
-        if response.status_code == 403:
+            if response.status_code == 200 and \
-            self._log.error(u'Server authentication failed')
+                    json['subsonic-response']['status'] == "ok":
-        elif response.status_code == 200:
+                count = json['subsonic-response']['scanStatus']['count']
-            self._log.debug(u'Updating Subsonic')
+                self._log.info(
+                    u'Updating Subsonic; scanning {0} tracks'.format(count))
+            elif response.status_code == 200 and \
+                    json['subsonic-response']['status'] == "failed":
+                error_message = json['subsonic-response']['error']['message']
+                self._log.error(u'Error: {0}'.format(error_message))
             else:
-            self._log.error(
+                self._log.error(u'Error: {0}', json)
-                u'Generic error, please try again later [Status Code: {}]'
+        except Exception as error:
-                .format(response.status_code))
+            self._log.error(u'Error: {0}'.format(error))

docs/changelog.rst

@@ -150,6 +150,8 @@ New features:
 
 Fixes:
 
+* :doc:`/plugins/subsonicupdate`: REST was using `POST` method rather `GET` method.
+  Also includes better exception handling, response parsing, and tests.
 * :doc:`/plugins/the`: Fixed incorrect regex for 'the' that matched any
   3-letter combination of the letters t, h, e.
   :bug:`3701`

test/test_subsonic.py

@@ -1,111 +0,0 @@
-# -*- coding: utf-8 -*-
-
-"""Tests for the 'subsonic' plugin"""
-
-from __future__ import division, absolute_import, print_function
-
-import requests
-import responses
-import unittest
-
-from test import _common
-from beets import config
-from beetsplug import subsonicupdate
-from test.helper import TestHelper
-from six.moves.urllib.parse import parse_qs, urlparse
-
-
-class ArgumentsMock(object):
-    def __init__(self, mode, show_failures):
-        self.mode = mode
-        self.show_failures = show_failures
-        self.verbose = 1
-
-
-def _params(url):
-    """Get the query parameters from a URL."""
-    return parse_qs(urlparse(url).query)
-
-
-class SubsonicPluginTest(_common.TestCase, TestHelper):
-    @responses.activate
-    def setUp(self):
-        config.clear()
-        self.setup_beets()
-
-        config["subsonic"]["user"] = "admin"
-        config["subsonic"]["pass"] = "admin"
-        config["subsonic"]["url"] = "http://localhost:4040"
-
-        self.subsonicupdate = subsonicupdate.SubsonicUpdate()
-
-    def tearDown(self):
-        self.teardown_beets()
-
-    @responses.activate
-    def test_start_scan(self):
-        responses.add(
-            responses.POST,
-            'http://localhost:4040/rest/startScan',
-            status=200
-        )
-
-        self.subsonicupdate.start_scan()
-
-    @responses.activate
-    def test_url_with_extra_forward_slash_url(self):
-        config["subsonic"]["url"] = "http://localhost:4040/contextPath"
-
-        responses.add(
-            responses.POST,
-            'http://localhost:4040/contextPath/rest/startScan',
-            status=200
-        )
-
-        self.subsonicupdate.start_scan()
-
-    @responses.activate
-    def test_url_with_context_path(self):
-        config["subsonic"]["url"] = "http://localhost:4040/"
-
-        responses.add(
-            responses.POST,
-            'http://localhost:4040/rest/startScan',
-            status=200
-        )
-
-        self.subsonicupdate.start_scan()
-
-    @responses.activate
-    def test_url_with_missing_port(self):
-        config["subsonic"]["url"] = "http://localhost/airsonic"
-
-        responses.add(
-            responses.POST,
-            'http://localhost:4040/rest/startScan',
-            status=200
-        )
-
-        with self.assertRaises(requests.exceptions.ConnectionError):
-            self.subsonicupdate.start_scan()
-
-    @responses.activate
-    def test_url_with_missing_schema(self):
-        config["subsonic"]["url"] = "localhost:4040/airsonic"
-
-        responses.add(
-            responses.POST,
-            'http://localhost:4040/rest/startScan',
-            status=200
-        )
-
-        with self.assertRaises(requests.exceptions.InvalidSchema):
-            self.subsonicupdate.start_scan()
-
-
-def suite():
-    return unittest.TestLoader().loadTestsFromName(__name__)
-
-
-if __name__ == '__main__':
-    unittest.main(defaultTest='suite')

test/test_subsonicupdate.py

@@ -0,0 +1,188 @@
+# -*- coding: utf-8 -*-
+
+"""Tests for the 'subsonic' plugin."""
+
+from __future__ import division, absolute_import, print_function
+
+import responses
+import unittest
+
+from test import _common
+from beets import config
+from beetsplug import subsonicupdate
+from test.helper import TestHelper
+from six.moves.urllib.parse import parse_qs, urlparse
+
+
+class ArgumentsMock(object):
+    """Argument mocks for tests."""
+    def __init__(self, mode, show_failures):
+        """Constructs ArgumentsMock."""
+        self.mode = mode
+        self.show_failures = show_failures
+        self.verbose = 1
+
+
+def _params(url):
+    """Get the query parameters from a URL."""
+    return parse_qs(urlparse(url).query)
+
+
+class SubsonicPluginTest(_common.TestCase, TestHelper):
+    """Test class for subsonicupdate."""
+    @responses.activate
+    def setUp(self):
+        """Sets up config and plugin for test."""
+        config.clear()
+        self.setup_beets()
+
+        config["subsonic"]["user"] = "admin"
+        config["subsonic"]["pass"] = "admin"
+        config["subsonic"]["url"] = "http://localhost:4040"
+
+        self.subsonicupdate = subsonicupdate.SubsonicUpdate()
+
+    SUCCESS_BODY = '''
+{
+    "subsonic-response": {
+        "status": "ok",
+        "version": "1.15.0",
+        "scanStatus": {
+            "scanning": true,
+            "count": 1000
+        }
+    }
+}
+'''
+
+    FAILED_BODY = '''
+{
+    "subsonic-response": {
+        "status": "failed",
+        "version": "1.15.0",
+        "error": {
+            "code": 40,
+            "message": "Wrong username or password."
+        }
+    }
+}
+'''
+
+    ERROR_BODY = '''
+{
+    "timestamp": 1599185854498,
+    "status": 404,
+    "error": "Not Found",
+    "message": "No message available",
+    "path": "/rest/startScn"
+}
+'''
+
+    def tearDown(self):
+        """Tears down tests."""
+        self.teardown_beets()
+
+    @responses.activate
+    def test_start_scan(self):
+        """Tests success path based on best case scenario."""
+        responses.add(
+            responses.GET,
+            'http://localhost:4040/rest/startScan',
+            status=200,
+            body=self.SUCCESS_BODY
+        )
+
+        self.subsonicupdate.start_scan()
+
+    @responses.activate
+    def test_start_scan_failed_bad_credentials(self):
+        """Tests failed path based on bad credentials."""
+        responses.add(
+            responses.GET,
+            'http://localhost:4040/rest/startScan',
+            status=200,
+            body=self.FAILED_BODY
+        )
+
+        self.subsonicupdate.start_scan()
+
+    @responses.activate
+    def test_start_scan_failed_not_found(self):
+        """Tests failed path based on resource not found."""
+        responses.add(
+            responses.GET,
+            'http://localhost:4040/rest/startScan',
+            status=404,
+            body=self.ERROR_BODY
+        )
+
+        self.subsonicupdate.start_scan()
+
+    def test_start_scan_failed_unreachable(self):
+        """Tests failed path based on service not available."""
+        self.subsonicupdate.start_scan()
+
+    @responses.activate
+    def test_url_with_context_path(self):
+        """Tests success for included with contextPath."""
+        config["subsonic"]["url"] = "http://localhost:4040/contextPath/"
+
+        responses.add(
+            responses.GET,
+            'http://localhost:4040/contextPath/rest/startScan',
+            status=200,
+            body=self.SUCCESS_BODY
+        )
+
+        self.subsonicupdate.start_scan()
+
+    @responses.activate
+    def test_url_with_trailing_forward_slash_url(self):
+        """Tests success path based on trailing forward slash."""
+        config["subsonic"]["url"] = "http://localhost:4040/"
+
+        responses.add(
+            responses.GET,
+            'http://localhost:4040/rest/startScan',
+            status=200,
+            body=self.SUCCESS_BODY
+        )
+
+        self.subsonicupdate.start_scan()
+
+    @responses.activate
+    def test_url_with_missing_port(self):
+        """Tests failed path based on missing port."""
+        config["subsonic"]["url"] = "http://localhost/airsonic"
+
+        responses.add(
+            responses.GET,
+            'http://localhost/airsonic/rest/startScan',
+            status=200,
+            body=self.SUCCESS_BODY
+        )
+
+        self.subsonicupdate.start_scan()
+
+    @responses.activate
+    def test_url_with_missing_schema(self):
+        """Tests failed path based on missing schema."""
+        config["subsonic"]["url"] = "localhost:4040/airsonic"
+
+        responses.add(
+            responses.GET,
+            'http://localhost:4040/rest/startScan',
+            status=200,
+            body=self.SUCCESS_BODY
+        )
+
+        self.subsonicupdate.start_scan()
+
+
+def suite():
+    """Default test suite."""
+    return unittest.TestLoader().loadTestsFromName(__name__)
+
+
+if __name__ == '__main__':
+    unittest.main(defaultTest='suite')