From f94fb244ab864d8b3e4910aa8acde9facc03a228 Mon Sep 17 00:00:00 2001 From: Laurent Monin Date: Thu, 4 Apr 2024 13:12:31 +0200 Subject: [PATCH 1/7] Collection._finished(): explode it in smaller bits --- picard/collection.py | 63 ++++++++++++++++++++++++++------------------ 1 file changed, 38 insertions(+), 25 deletions(-) diff --git a/picard/collection.py b/picard/collection.py index 9e8852b2b..372283804 100644 --- a/picard/collection.py +++ b/picard/collection.py @@ -69,35 +69,48 @@ class Collection(QtCore.QObject): def _finished(self, kind, ids, callback, document, reply, error): self.pending -= ids - statusbar = self.tagger.window.set_statusbar_message if not error: - count = len(ids) if kind == self.COLLECTION_ADD: - self.releases |= ids - self.size += count - status_msg = ngettext( - 'Added %(count)i release to collection "%(name)s"', - 'Added %(count)i releases to collection "%(name)s"', - count) - debug_msg = 'Added %(count)i releases to collection "%(name)s"' + self._success_add(ids, callback) else: - self.releases -= ids - self.size -= count - status_msg = ngettext( - 'Removed %(count)i release from collection "%(name)s"', - 'Removed %(count)i releases from collection "%(name)s"', - count) - debug_msg = 'Removed %(count)i releases from collection "%(name)s"' - callback() - mparms = {'count': count, 'name': self.name} - log.debug(debug_msg % mparms) - statusbar(status_msg, mparms, translate=None, echo=None) + self._success_remove(ids, callback) else: - statusbar( - N_("Error while modifying collections: %(error)s"), - {'error': reply.errorString()}, - echo=log.error - ) + self._error(reply) + + def _error(self, reply): + self.tagger.window.set_statusbar_message( + N_("Error while modifying collections: %(error)s"), + {'error': reply.errorString()}, + echo=log.error + ) + + def _success_add(self, ids, callback): + count = len(ids) + self.releases |= ids + self.size += count + status_msg = ngettext( + 'Added %(count)i release to collection "%(name)s"', + 'Added %(count)i releases to collection "%(name)s"', + count) + debug_msg = 'Added %(count)i releases to collection "%(name)s"' + self._success(count, callback, status_msg, debug_msg) + + def _success_remove(self, ids, callback): + count = len(ids) + self.releases -= ids + self.size -= count + status_msg = ngettext( + 'Removed %(count)i release from collection "%(name)s"', + 'Removed %(count)i releases from collection "%(name)s"', + count) + debug_msg = 'Removed %(count)i releases from collection "%(name)s"' + self._success(count, callback, status_msg, debug_msg) + + def _success(self, count, callback, status_msg, debug_msg): + callback() + mparms = {'count': count, 'name': self.name} + log.debug(debug_msg % mparms) + self.tagger.window.set_statusbar_message(status_msg, mparms, translate=None, echo=None) def get_user_collection(collection_id, name, size, refresh=False): From c162fc0e23e4b61e61ddb4b964ac10f001dc42c6 Mon Sep 17 00:00:00 2001 From: Laurent Monin Date: Thu, 4 Apr 2024 13:18:02 +0200 Subject: [PATCH 2/7] Collection._finished(): directly pass success handler --- picard/collection.py | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/picard/collection.py b/picard/collection.py index 372283804..96cc7aaa5 100644 --- a/picard/collection.py +++ b/picard/collection.py @@ -58,7 +58,11 @@ class Collection(QtCore.QObject): ids -= self.pending if ids: self.pending |= ids - when_done = partial(self._finished, kind, ids, callback) + if kind == self.COLLECTION_ADD: + success_handler = self._success_add + else: + success_handler = self._success_remove + when_done = partial(self._finished, success_handler, ids, callback) self.api_action[kind](self.id, list(ids), when_done) def add_releases(self, ids, callback): @@ -67,13 +71,10 @@ class Collection(QtCore.QObject): def remove_releases(self, ids, callback): self._modify(self.COLLECTION_REMOVE, ids, callback) - def _finished(self, kind, ids, callback, document, reply, error): + def _finished(self, success_handler, ids, callback, document, reply, error): self.pending -= ids if not error: - if kind == self.COLLECTION_ADD: - self._success_add(ids, callback) - else: - self._success_remove(ids, callback) + success_handler(ids, callback) else: self._error(reply) From 9c0697cd89b248817b759faba243c0a2e3d374b5 Mon Sep 17 00:00:00 2001 From: Laurent Monin Date: Thu, 4 Apr 2024 13:25:08 +0200 Subject: [PATCH 3/7] Get rid of COLLECTION_ADD/COLLECTION_REMOVE, pass appropriate methods instead --- picard/collection.py | 21 ++++++--------------- 1 file changed, 6 insertions(+), 15 deletions(-) diff --git a/picard/collection.py b/picard/collection.py index 96cc7aaa5..f74f7d431 100644 --- a/picard/collection.py +++ b/picard/collection.py @@ -36,8 +36,6 @@ user_collections = {} class Collection(QtCore.QObject): - COLLECTION_ADD = 1 - COLLECTION_REMOVE = 2 def __init__(self, collection_id, name, size): self.id = collection_id @@ -45,31 +43,24 @@ class Collection(QtCore.QObject): self.pending = set() self.size = int(size) self.releases = set() - mb_api = self.tagger.mb_api - self.api_action = { - self.COLLECTION_ADD: mb_api.put_to_collection, - self.COLLECTION_REMOVE: mb_api.delete_from_collection, - } def __repr__(self): return '' % (self.name, self.id) - def _modify(self, kind, ids, callback): + def _modify(self, api_method, success_handler, ids, callback): ids -= self.pending if ids: self.pending |= ids - if kind == self.COLLECTION_ADD: - success_handler = self._success_add - else: - success_handler = self._success_remove when_done = partial(self._finished, success_handler, ids, callback) - self.api_action[kind](self.id, list(ids), when_done) + api_method(self.id, list(ids), when_done) def add_releases(self, ids, callback): - self._modify(self.COLLECTION_ADD, ids, callback) + api_method = self.tagger.mb_api.put_to_collection + self._modify(api_method, self._success_add, ids, callback) def remove_releases(self, ids, callback): - self._modify(self.COLLECTION_REMOVE, ids, callback) + api_method = self.tagger.mb_api.delete_from_collection + self._modify(api_method, self._success_remove, ids, callback) def _finished(self, success_handler, ids, callback, document, reply, error): self.pending -= ids From 742c0cf943a0acdeccbe2b35260f613e85f6b30d Mon Sep 17 00:00:00 2001 From: Laurent Monin Date: Thu, 4 Apr 2024 13:29:28 +0200 Subject: [PATCH 4/7] Cosmetic change in debug output --- picard/collection.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/picard/collection.py b/picard/collection.py index f74f7d431..8e09e607b 100644 --- a/picard/collection.py +++ b/picard/collection.py @@ -84,7 +84,7 @@ class Collection(QtCore.QObject): 'Added %(count)i release to collection "%(name)s"', 'Added %(count)i releases to collection "%(name)s"', count) - debug_msg = 'Added %(count)i releases to collection "%(name)s"' + debug_msg = 'Added %(count)i release(s) to collection "%(name)s"' self._success(count, callback, status_msg, debug_msg) def _success_remove(self, ids, callback): @@ -95,7 +95,7 @@ class Collection(QtCore.QObject): 'Removed %(count)i release from collection "%(name)s"', 'Removed %(count)i releases from collection "%(name)s"', count) - debug_msg = 'Removed %(count)i releases from collection "%(name)s"' + debug_msg = 'Removed %(count)i release(s) from collection "%(name)s"' self._success(count, callback, status_msg, debug_msg) def _success(self, count, callback, status_msg, debug_msg): From 3212d62d6ab0717cc7d29b58e61c07f09572bb0f Mon Sep 17 00:00:00 2001 From: Laurent Monin Date: Thu, 4 Apr 2024 13:57:50 +0200 Subject: [PATCH 5/7] Use `releases` consistently for Collection releases `ids` was used in some parts of code, but field in Collection is actually `releases` --- picard/collection.py | 40 ++++++++++++++++++------------------- picard/ui/collectionmenu.py | 14 ++++++------- 2 files changed, 27 insertions(+), 27 deletions(-) diff --git a/picard/collection.py b/picard/collection.py index 8e09e607b..02b16df06 100644 --- a/picard/collection.py +++ b/picard/collection.py @@ -40,32 +40,32 @@ class Collection(QtCore.QObject): def __init__(self, collection_id, name, size): self.id = collection_id self.name = name - self.pending = set() self.size = int(size) + self.pending = set() self.releases = set() def __repr__(self): return '' % (self.name, self.id) - def _modify(self, api_method, success_handler, ids, callback): - ids -= self.pending - if ids: - self.pending |= ids - when_done = partial(self._finished, success_handler, ids, callback) - api_method(self.id, list(ids), when_done) + def _modify(self, api_method, success_handler, releases, callback): + releases -= self.pending + if releases: + self.pending |= releases + when_done = partial(self._finished, success_handler, releases, callback) + api_method(self.id, list(releases), when_done) - def add_releases(self, ids, callback): + def add_releases(self, releases, callback): api_method = self.tagger.mb_api.put_to_collection - self._modify(api_method, self._success_add, ids, callback) + self._modify(api_method, self._success_add, releases, callback) - def remove_releases(self, ids, callback): + def remove_releases(self, releases, callback): api_method = self.tagger.mb_api.delete_from_collection - self._modify(api_method, self._success_remove, ids, callback) + self._modify(api_method, self._success_remove, releases, callback) - def _finished(self, success_handler, ids, callback, document, reply, error): - self.pending -= ids + def _finished(self, success_handler, releases, callback, document, reply, error): + self.pending -= releases if not error: - success_handler(ids, callback) + success_handler(releases, callback) else: self._error(reply) @@ -76,9 +76,9 @@ class Collection(QtCore.QObject): echo=log.error ) - def _success_add(self, ids, callback): - count = len(ids) - self.releases |= ids + def _success_add(self, releases, callback): + count = len(releases) + self.releases |= releases self.size += count status_msg = ngettext( 'Added %(count)i release to collection "%(name)s"', @@ -87,9 +87,9 @@ class Collection(QtCore.QObject): debug_msg = 'Added %(count)i release(s) to collection "%(name)s"' self._success(count, callback, status_msg, debug_msg) - def _success_remove(self, ids, callback): - count = len(ids) - self.releases -= ids + def _success_remove(self, releases, callback): + count = len(releases) + self.releases -= releases self.size -= count status_msg = ngettext( 'Removed %(count)i release from collection "%(name)s"', diff --git a/picard/ui/collectionmenu.py b/picard/ui/collectionmenu.py index 38c29ffeb..e16a6b4d2 100644 --- a/picard/ui/collectionmenu.py +++ b/picard/ui/collectionmenu.py @@ -40,7 +40,7 @@ class CollectionMenu(QtWidgets.QMenu): def __init__(self, albums, *args): super().__init__(*args) - self.ids = set(a.id for a in albums) + self.releases = set(a.id for a in albums) self._ignore_update = False self.update_collections() @@ -145,8 +145,8 @@ class CollectionCheckBox(QtWidgets.QCheckBox): self.collection = collection super().__init__(self.label(), parent) - releases = collection.releases & menu.ids - if len(releases) == len(menu.ids): + releases = collection.releases & menu.releases + if len(releases) == len(menu.releases): self.setCheckState(QtCore.Qt.CheckState.Checked) elif not releases: self.setCheckState(QtCore.Qt.CheckState.Unchecked) @@ -154,15 +154,15 @@ class CollectionCheckBox(QtWidgets.QCheckBox): self.setCheckState(QtCore.Qt.CheckState.PartiallyChecked) def nextCheckState(self): - ids = self.menu.ids - if ids & self.collection.pending: + releases = self.menu.releases + if releases & self.collection.pending: return - diff = ids - self.collection.releases + diff = releases - self.collection.releases if diff: self.collection.add_releases(diff, self.updateText) self.setCheckState(QtCore.Qt.CheckState.Checked) else: - self.collection.remove_releases(ids & self.collection.releases, self.updateText) + self.collection.remove_releases(releases & self.collection.releases, self.updateText) self.setCheckState(QtCore.Qt.CheckState.Unchecked) def updateText(self): From e042c47fb3043b0a18580753f1176a3d2327aa8d Mon Sep 17 00:00:00 2001 From: Laurent Monin Date: Thu, 4 Apr 2024 13:59:32 +0200 Subject: [PATCH 6/7] Collection.pending -> Collection.pending_releases --- picard/collection.py | 8 ++++---- picard/ui/collectionmenu.py | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/picard/collection.py b/picard/collection.py index 02b16df06..394dab885 100644 --- a/picard/collection.py +++ b/picard/collection.py @@ -41,16 +41,16 @@ class Collection(QtCore.QObject): self.id = collection_id self.name = name self.size = int(size) - self.pending = set() + self.pending_releases = set() self.releases = set() def __repr__(self): return '' % (self.name, self.id) def _modify(self, api_method, success_handler, releases, callback): - releases -= self.pending + releases -= self.pending_releases if releases: - self.pending |= releases + self.pending_releases |= releases when_done = partial(self._finished, success_handler, releases, callback) api_method(self.id, list(releases), when_done) @@ -63,7 +63,7 @@ class Collection(QtCore.QObject): self._modify(api_method, self._success_remove, releases, callback) def _finished(self, success_handler, releases, callback, document, reply, error): - self.pending -= releases + self.pending_releases -= releases if not error: success_handler(releases, callback) else: diff --git a/picard/ui/collectionmenu.py b/picard/ui/collectionmenu.py index e16a6b4d2..8b0de3151 100644 --- a/picard/ui/collectionmenu.py +++ b/picard/ui/collectionmenu.py @@ -155,7 +155,7 @@ class CollectionCheckBox(QtWidgets.QCheckBox): def nextCheckState(self): releases = self.menu.releases - if releases & self.collection.pending: + if releases & self.collection.pending_releases: return diff = releases - self.collection.releases if diff: From 68367947f491544afeb5481ce64c4716ee38ea59 Mon Sep 17 00:00:00 2001 From: Laurent Monin Date: Thu, 4 Apr 2024 14:44:21 +0200 Subject: [PATCH 7/7] Simplify Collection constructor and get_user_collection() --- picard/collection.py | 35 ++++++++++++++++++++--------------- 1 file changed, 20 insertions(+), 15 deletions(-) diff --git a/picard/collection.py b/picard/collection.py index 394dab885..7e99a2b29 100644 --- a/picard/collection.py +++ b/picard/collection.py @@ -37,13 +37,21 @@ user_collections = {} class Collection(QtCore.QObject): - def __init__(self, collection_id, name, size): + def __init__(self, collection_id): self.id = collection_id - self.name = name - self.size = int(size) + self.name = '' + self.size = 0 self.pending_releases = set() self.releases = set() + @property + def size(self): + return self._size + + @size.setter + def size(self, value): + self._size = int(value) + def __repr__(self): return '' % (self.name, self.id) @@ -105,13 +113,10 @@ class Collection(QtCore.QObject): self.tagger.window.set_statusbar_message(status_msg, mparms, translate=None, echo=None) -def get_user_collection(collection_id, name, size, refresh=False): +def get_user_collection(collection_id): collection = user_collections.get(collection_id) if collection is None: - collection = user_collections[collection_id] = Collection(collection_id, name, size) - elif refresh: - collection.name = name - collection.size = size + collection = user_collections[collection_id] = Collection(collection_id) return collection @@ -134,10 +139,10 @@ def load_user_collections(callback=None): if node['entity-type'] != 'release': continue col_id = node['id'] - col_name = node['name'] - col_size = node['release-count'] new_collections.add(col_id) - get_user_collection(col_id, col_name, col_size, refresh=True) + collection = get_user_collection(col_id) + collection.name = node['name'] + collection.size = node['release-count'] # remove collections which aren't returned by the web service anymore old_collections = set(user_collections) - new_collections @@ -163,9 +168,9 @@ def add_release_to_user_collections(release_node): username = config.persist['oauth_username'].lower() for node in release_node['collections']: if node['editor'].lower() == username: - col_id = node['id'] - col_name = node['name'] - col_size = node['release-count'] - collection = get_user_collection(col_id, col_name, col_size) + collection = get_user_collection(node['id']) + collection.name = node['name'] + collection.size = node['release-count'] + collection.releases.add(release_id) log.debug("Adding release %r to %r", release_id, collection)