Skip to content
This repository has been archived by the owner on Jun 15, 2022. It is now read-only.

Commit

Permalink
config: Set all string fields to empty instead of NULL
Browse files Browse the repository at this point in the history
This forces libspotify to clear old cached settings instead of reusing
them.
  • Loading branch information
jodal committed Sep 15, 2015
1 parent 645ccd4 commit 78b823d
Show file tree
Hide file tree
Showing 3 changed files with 112 additions and 25 deletions.
12 changes: 12 additions & 0 deletions docs/changelog.rst
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,18 @@
Changelog
*********

v2.0.4 (UNRELEASED)
===================

Bug fix release.

- It has been observed that libspotify will reuse cached proxy settings from
previous sessions if the proxy fields on the ``sp_config`` struct are set to
``NULL``. When the ``sp_config`` fields are set to an empty string, the
cached settings are updated. When attributes on :class:`spotify.Config` are
set to :class:`None`, we now set the fields on ``sp_config`` to empty strings
instead of ``NULL``.


v2.0.3 (2015-09-05)
===================
Expand Down
36 changes: 23 additions & 13 deletions spotify/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,12 @@ def __init__(self):
self.compress_playlists = False
self.dont_save_metadata_for_playlists = False
self.initially_unload_playlists = False
self.device_id = ''
self.proxy = ''
self.proxy_username = ''
self.proxy_password = ''
self.ca_certs_filename = ''
self.tracefile = ''

@property
def api_version(self):
Expand Down Expand Up @@ -64,11 +70,8 @@ def cache_location(self):

@cache_location.setter
def cache_location(self, value):
# XXX: libspotify segfaults if cache_location is set to NULL, but
# doesn't seem to care if other strings in sp_session_config is NULL.
if value is None:
value = ''
self._cache_location = utils.to_char(value)
# NOTE libspotify segfaults if cache_location is set to NULL.
self._cache_location = utils.to_char('' if value is None else value)
self._sp_session_config.cache_location = self._cache_location

@property
Expand All @@ -86,7 +89,7 @@ def settings_location(self):

@settings_location.setter
def settings_location(self, value):
self._settings_location = utils.to_char_or_null(value)
self._settings_location = utils.to_char('' if value is None else value)
self._sp_session_config.settings_location = self._settings_location

@property
Expand Down Expand Up @@ -137,7 +140,7 @@ def user_agent(self):

@user_agent.setter
def user_agent(self, value):
self._user_agent = utils.to_char_or_null(value)
self._user_agent = utils.to_char('' if value is None else value)
self._sp_session_config.user_agent = self._user_agent

@property
Expand Down Expand Up @@ -196,7 +199,7 @@ def device_id(self):

@device_id.setter
def device_id(self, value):
self._device_id = utils.to_char_or_null(value)
self._device_id = utils.to_char('' if value is None else value)
self._sp_session_config.device_id = self._device_id

@property
Expand All @@ -212,7 +215,9 @@ def proxy(self):

@proxy.setter
def proxy(self, value):
self._proxy = utils.to_char_or_null(value)
# NOTE libspotify reuses cached values from previous sessions if this
# is set to NULL instead of empty string.
self._proxy = utils.to_char('' if value is None else value)
self._sp_session_config.proxy = self._proxy

@property
Expand All @@ -225,7 +230,9 @@ def proxy_username(self):

@proxy_username.setter
def proxy_username(self, value):
self._proxy_username = utils.to_char_or_null(value)
# NOTE libspotify reuses cached values from previous sessions if this
# is set to NULL instead of empty string.
self._proxy_username = utils.to_char('' if value is None else value)
self._sp_session_config.proxy_username = self._proxy_username

@property
Expand All @@ -238,7 +245,9 @@ def proxy_password(self):

@proxy_password.setter
def proxy_password(self, value):
self._proxy_password = utils.to_char_or_null(value)
# NOTE libspotify reuses cached values from previous sessions if this
# is set to NULL instead of empty string.
self._proxy_password = utils.to_char('' if value is None else value)
self._sp_session_config.proxy_password = self._proxy_password

@property
Expand Down Expand Up @@ -272,7 +281,8 @@ def ca_certs_filename(self):
def ca_certs_filename(self, value):
ptr = self._get_ca_certs_filename_ptr()
if ptr is not None:
self._ca_certs_filename = utils.to_char_or_null(value)
self._ca_certs_filename = utils.to_char(
'' if value is None else value)
ptr[0] = self._ca_certs_filename

def _get_ca_certs_filename_ptr(self):
Expand Down Expand Up @@ -303,5 +313,5 @@ def tracefile(self):

@tracefile.setter
def tracefile(self, value):
self._tracefile = utils.to_char_or_null(value)
self._tracefile = utils.to_char('' if value is None else value)
self._sp_session_config.tracefile = self._tracefile
89 changes: 77 additions & 12 deletions tests/test_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,15 @@ def test_settings_location(self):
def test_settings_location_defaults_to_tmp_in_cwd(self):
self.assertEqual(self.config.settings_location, b'tmp')

def test_settings_location_converts_none_to_empty_string(self):
self.config.settings_location = None

self.assertEqual(
spotify.ffi.string(
self.config._sp_session_config.settings_location),
b'')
self.assertEqual(self.config.settings_location, b'')

def test_application_key(self):
self.config.application_key = b'\x02' * 321

Expand Down Expand Up @@ -123,6 +132,13 @@ def test_user_agent_defaults_to_pyspotify_with_version_number(self):
self.assertEqual(
self.config.user_agent, 'pyspotify %s' % spotify.__version__)

def test_user_agent_location_converts_none_to_empty_string(self):
self.config.user_agent = None

self.assertEqual(
spotify.ffi.string(self.config._sp_session_config.user_agent), b'')
self.assertEqual(self.config.user_agent, b'')

def test_compress_playlists(self):
self.config.compress_playlists = True

Expand Down Expand Up @@ -160,8 +176,15 @@ def test_device_id(self):
b'123abc')
self.assertEqual(self.config.device_id, '123abc')

def test_device_id_defaults_to_none(self):
self.assertIsNone(self.config.device_id)
def test_device_id_defaults_to_empty_string(self):
self.assertEqual(self.config.device_id, '')

def test_device_id_converts_none_to_empty_string(self):
self.config.device_id = None

self.assertEqual(
spotify.ffi.string(self.config._sp_session_config.device_id), b'')
self.assertEqual(self.config.device_id, b'')

def test_proxy(self):
self.config.proxy = '123abc'
Expand All @@ -171,8 +194,15 @@ def test_proxy(self):
b'123abc')
self.assertEqual(self.config.proxy, '123abc')

def test_proxy_defaults_to_none(self):
self.assertIsNone(self.config.proxy)
def test_proxy_defaults_to_empty_string(self):
self.assertEqual(self.config.proxy, '')

def test_proxy_converts_none_to_empty_string(self):
self.config.proxy = None

self.assertEqual(
spotify.ffi.string(self.config._sp_session_config.proxy), b'')
self.assertEqual(self.config.proxy, b'')

def test_proxy_username(self):
self.config.proxy_username = '123abc'
Expand All @@ -182,8 +212,16 @@ def test_proxy_username(self):
b'123abc')
self.assertEqual(self.config.proxy_username, '123abc')

def test_proxy_username_defaults_to_none(self):
self.assertIsNone(self.config.proxy_username)
def test_proxy_username_defaults_to_empty_string(self):
self.assertEqual(self.config.proxy_username, '')

def test_proxy_username_converts_none_to_empty_string(self):
self.config.proxy_username = None

self.assertEqual(
spotify.ffi.string(self.config._sp_session_config.proxy_username),
b'')
self.assertEqual(self.config.proxy_username, b'')

def test_proxy_password(self):
self.config.proxy_password = '123abc'
Expand All @@ -193,8 +231,16 @@ def test_proxy_password(self):
b'123abc')
self.assertEqual(self.config.proxy_password, '123abc')

def test_proxy_password_defaults_to_none(self):
self.assertIsNone(self.config.proxy_password)
def test_proxy_password_defaults_to_empty_string(self):
self.assertEqual(self.config.proxy_password, '')

def test_proxy_password_converts_none_to_empty_string(self):
self.config.proxy_password = None

self.assertEqual(
spotify.ffi.string(self.config._sp_session_config.proxy_password),
b'')
self.assertEqual(self.config.proxy_password, b'')

@unittest.skipIf(
platform.system() == 'Darwin',
Expand All @@ -211,8 +257,20 @@ def test_ca_certs_filename(self):
@unittest.skipIf(
platform.system() == 'Darwin',
'The struct field does not exist in libspotify for OS X')
def test_ca_certs_filename_defaults_to_none(self):
self.assertIsNone(self.config.ca_certs_filename)
def test_ca_certs_filename_defaults_to_empty_string(self):
self.assertEqual(self.config.ca_certs_filename, '')

@unittest.skipIf(
platform.system() == 'Darwin',
'The struct field does not exist in libspotify for OS X')
def test_ca_certs_filename_converts_none_to_empty_string(self):
self.config.ca_certs_filename = None

self.assertEqual(
spotify.ffi.string(
self.config._get_ca_certs_filename_ptr()[0]),
b'')
self.assertEqual(self.config.ca_certs_filename, b'')

@unittest.skipIf(
platform.system() != 'Darwin',
Expand All @@ -232,8 +290,15 @@ def test_tracefile(self):
b'123abc')
self.assertEqual(self.config.tracefile, b'123abc')

def test_tracefile_defaults_to_none(self):
self.assertIsNone(self.config.tracefile)
def test_tracefile_defaults_to_empty_string(self):
self.assertEqual(self.config.tracefile, '')

def test_tracefile_converts_none_to_empty_string(self):
self.config.tracefile = None

self.assertEqual(
spotify.ffi.string(self.config._sp_session_config.tracefile), b'')
self.assertEqual(self.config.tracefile, b'')

def test_sp_session_config_has_unicode_encoded_as_utf8(self):
self.config.device_id = 'æ device_id'
Expand Down

0 comments on commit 78b823d

Please sign in to comment.