From 326d0cb8a3e9d9fe6ef0efbdafb4fd74879ef575 Mon Sep 17 00:00:00 2001 From: kafene Date: Wed, 25 Apr 2018 21:19:56 -0400 Subject: [PATCH 01/11] initial naive implementation of file_list sorting --- lbrynet/daemon/Daemon.py | 52 ++++++++++++++++++++++++++++++++++++---- 1 file changed, 48 insertions(+), 4 deletions(-) diff --git a/lbrynet/daemon/Daemon.py b/lbrynet/daemon/Daemon.py index df39a6137..e2230614a 100644 --- a/lbrynet/daemon/Daemon.py +++ b/lbrynet/daemon/Daemon.py @@ -1,3 +1,5 @@ +from pprint import pprint + import binascii import logging.handlers import mimetypes @@ -97,6 +99,16 @@ CONNECTION_MESSAGES = { SHORT_ID_LEN = 20 MAX_UPDATE_FEE_ESTIMATE = 0.3 +FILE_SORT_FIELD_NAME = 'name' +FILE_SORT_FIELD_DATE = 'date' +FILE_SORT_FIELD_PRICE = 'price' + +FILE_SORT_ORDER_ASCENDING = 'asc' +FILE_SORT_ORDER_DESCENDING = 'desc' +FILE_SORT_ORDERS = [ + FILE_SORT_ORDER_ASCENDING, + FILE_SORT_ORDER_DESCENDING, +] class IterableContainer(object): def __iter__(self): @@ -925,11 +937,13 @@ class Daemon(AuthJSONRPCServer): defer.returnValue(lbry_file) @defer.inlineCallbacks - def _get_lbry_files(self, return_json=False, full_status=True, **kwargs): + def _get_lbry_files(self, return_json=False, full_status=True, sorts=None, **kwargs): lbry_files = list(self.lbry_file_manager.lbry_files) if kwargs: for search_type, value in iter_lbry_file_search_values(kwargs): lbry_files = [l_f for l_f in lbry_files if l_f.__dict__[search_type] == value] + if sorts: + lbry_files = self._sort_lbry_files(lbry_files, sorts) if return_json: file_dicts = [] for lbry_file in lbry_files: @@ -939,6 +953,33 @@ class Daemon(AuthJSONRPCServer): log.debug("Collected %i lbry files", len(lbry_files)) defer.returnValue(lbry_files) + def _sort_lbry_files(self, lbry_files, sorts): + for field, order in sorts: + reverse = order == FILE_SORT_ORDER_DESCENDING + if field == FILE_SORT_FIELD_NAME: + lbry_files = sorted(lbry_files, key=lambda f: f.file_name, reverse=reverse) + elif field == FILE_SORT_FIELD_DATE: + lbry_files = sorted(lbry_files, reverse=reverse) + elif field == FILE_SORT_FIELD_PRICE: + lbry_files = sorted(lbry_files, key=lambda f: f.points_paid, reverse=reverse) + else: + raise Exception('Unrecognized sort field "{}"'.format(field)) + return lbry_files + + def _parse_lbry_files_sort(self, sort): + """ + Given a sort string like 'name, desc' or 'price', + parse the string into a tuple of (field, order). + Order defaults to ascending. + """ + + pieces = sort.rsplit(',', 1) + field = pieces[0].strip() + order = pieces[1].strip().lower() if len(pieces) > 1 else None + if order and order not in FILE_SORT_ORDERS: + raise Exception('Sort order must be one of {}'.format(FILE_SORT_ORDERS)) + return (field, order or FILE_SORT_ORDER_ASCENDING) + def _get_single_peer_downloader(self): downloader = SinglePeerDownloader() downloader.setup(self.session.wallet) @@ -1358,7 +1399,7 @@ class Daemon(AuthJSONRPCServer): defer.returnValue(response) @defer.inlineCallbacks - def jsonrpc_file_list(self, **kwargs): + def jsonrpc_file_list(self, sort=None, **kwargs): """ List files limited by optional filters @@ -1366,7 +1407,7 @@ class Daemon(AuthJSONRPCServer): file_list [--sd_hash=] [--file_name=] [--stream_hash=] [--rowid=] [--claim_id=] [--outpoint=] [--txid=] [--nout=] [--channel_claim_id=] [--channel_name=] - [--claim_name=] [--full_status] + [--claim_name=] [--full_status] [--sort=...] Options: --sd_hash= : (str) get file with matching sd hash @@ -1383,6 +1424,8 @@ class Daemon(AuthJSONRPCServer): --claim_name= : (str) get file with matching claim name --full_status : (bool) full status, populate the 'message' and 'size' fields + --sort= : (str) sort by any of 'name', 'date', or 'price' + to specify order append ',asc' or ',desc' Returns: (list) List of files @@ -1418,7 +1461,8 @@ class Daemon(AuthJSONRPCServer): ] """ - result = yield self._get_lbry_files(return_json=True, **kwargs) + sorts = [self._parse_lbry_files_sort(s) for s in sort] if sort else None + result = yield self._get_lbry_files(return_json=True, sorts=sorts, **kwargs) response = yield self._render_response(result) defer.returnValue(response) From e7b22521eee206666e9318582bcc0d5f0f691f0c Mon Sep 17 00:00:00 2001 From: kafene Date: Wed, 25 Apr 2018 21:22:58 -0400 Subject: [PATCH 02/11] remove pprint import --- lbrynet/daemon/Daemon.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/lbrynet/daemon/Daemon.py b/lbrynet/daemon/Daemon.py index e2230614a..0ee9eb136 100644 --- a/lbrynet/daemon/Daemon.py +++ b/lbrynet/daemon/Daemon.py @@ -1,5 +1,3 @@ -from pprint import pprint - import binascii import logging.handlers import mimetypes From ce1e7d669e945f82a370719fd8f6fb2a80c9a2a1 Mon Sep 17 00:00:00 2001 From: kafene Date: Sat, 28 Apr 2018 17:23:26 -0400 Subject: [PATCH 03/11] add changelog entry for file_list sort option --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index a261709e0..8cf950d64 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -50,6 +50,7 @@ at anytime. * integration tests for bootstrapping the dht * configurable `concurrent_announcers` setting * `peer_ping` command + * `--sort` option in `file_list` ### Removed * `announce_all` argument from `blob_announce` From 89996f929d38f2fb8336344d9ff6cbd1fab2fa94 Mon Sep 17 00:00:00 2001 From: kafene Date: Tue, 1 May 2018 19:18:23 -0400 Subject: [PATCH 04/11] add unit test package requirements - mock and Faker --- requirements_testing.txt | 2 ++ 1 file changed, 2 insertions(+) diff --git a/requirements_testing.txt b/requirements_testing.txt index e69de29bb..59f64aa4a 100644 --- a/requirements_testing.txt +++ b/requirements_testing.txt @@ -0,0 +1,2 @@ +mock>=2.0,<3.0 +Faker>=0.8,<1.0 From eec3734d2f390264dcb2b857fc8918e7808b45ef Mon Sep 17 00:00:00 2001 From: kafene Date: Tue, 1 May 2018 19:30:33 -0400 Subject: [PATCH 05/11] some semantic variable renaing --- lbrynet/daemon/Daemon.py | 48 ++++++++++++++++++++-------------------- 1 file changed, 24 insertions(+), 24 deletions(-) diff --git a/lbrynet/daemon/Daemon.py b/lbrynet/daemon/Daemon.py index 0ee9eb136..ae913b182 100644 --- a/lbrynet/daemon/Daemon.py +++ b/lbrynet/daemon/Daemon.py @@ -101,12 +101,12 @@ FILE_SORT_FIELD_NAME = 'name' FILE_SORT_FIELD_DATE = 'date' FILE_SORT_FIELD_PRICE = 'price' -FILE_SORT_ORDER_ASCENDING = 'asc' -FILE_SORT_ORDER_DESCENDING = 'desc' -FILE_SORT_ORDERS = [ - FILE_SORT_ORDER_ASCENDING, - FILE_SORT_ORDER_DESCENDING, -] +FILE_SORT_DIRECTION_ASCENDING = 'asc' +FILE_SORT_DIRECTION_DESCENDING = 'desc' +FILE_SORT_DIRECTIONS = ( + FILE_SORT_DIRECTION_ASCENDING, + FILE_SORT_DIRECTION_DESCENDING, +) class IterableContainer(object): def __iter__(self): @@ -935,13 +935,13 @@ class Daemon(AuthJSONRPCServer): defer.returnValue(lbry_file) @defer.inlineCallbacks - def _get_lbry_files(self, return_json=False, full_status=True, sorts=None, **kwargs): + def _get_lbry_files(self, return_json=False, full_status=True, sort_by=None, **kwargs): lbry_files = list(self.lbry_file_manager.lbry_files) if kwargs: for search_type, value in iter_lbry_file_search_values(kwargs): lbry_files = [l_f for l_f in lbry_files if l_f.__dict__[search_type] == value] - if sorts: - lbry_files = self._sort_lbry_files(lbry_files, sorts) + if sort_by: + lbry_files = self._sort_lbry_files(lbry_files, sort_by) if return_json: file_dicts = [] for lbry_file in lbry_files: @@ -951,15 +951,15 @@ class Daemon(AuthJSONRPCServer): log.debug("Collected %i lbry files", len(lbry_files)) defer.returnValue(lbry_files) - def _sort_lbry_files(self, lbry_files, sorts): - for field, order in sorts: - reverse = order == FILE_SORT_ORDER_DESCENDING + def _sort_lbry_files(self, lbry_files, sort_by): + for field, direction in sort_by: + is_reverse = direction == FILE_SORT_DIRECTION_DESCENDING if field == FILE_SORT_FIELD_NAME: - lbry_files = sorted(lbry_files, key=lambda f: f.file_name, reverse=reverse) + lbry_files = sorted(lbry_files, key=lambda f: f.file_name, reverse=is_reverse) elif field == FILE_SORT_FIELD_DATE: - lbry_files = sorted(lbry_files, reverse=reverse) + lbry_files = sorted(lbry_files, reverse=is_reverse) elif field == FILE_SORT_FIELD_PRICE: - lbry_files = sorted(lbry_files, key=lambda f: f.points_paid, reverse=reverse) + lbry_files = sorted(lbry_files, key=lambda f: f.points_paid, reverse=is_reverse) else: raise Exception('Unrecognized sort field "{}"'.format(field)) return lbry_files @@ -967,16 +967,16 @@ class Daemon(AuthJSONRPCServer): def _parse_lbry_files_sort(self, sort): """ Given a sort string like 'name, desc' or 'price', - parse the string into a tuple of (field, order). - Order defaults to ascending. + parse the string into a tuple of (field, direction). + Direction defaults to ascending. """ pieces = sort.rsplit(',', 1) field = pieces[0].strip() - order = pieces[1].strip().lower() if len(pieces) > 1 else None - if order and order not in FILE_SORT_ORDERS: - raise Exception('Sort order must be one of {}'.format(FILE_SORT_ORDERS)) - return (field, order or FILE_SORT_ORDER_ASCENDING) + direction = pieces[1].strip().lower() if len(pieces) > 1 else None + if direction and direction not in FILE_SORT_DIRECTIONS: + raise Exception('Sort direction must be one of {}'.format(FILE_SORT_DIRECTIONS)) + return (field, direction or FILE_SORT_DIRECTION_ASCENDING) def _get_single_peer_downloader(self): downloader = SinglePeerDownloader() @@ -1423,7 +1423,7 @@ class Daemon(AuthJSONRPCServer): --full_status : (bool) full status, populate the 'message' and 'size' fields --sort= : (str) sort by any of 'name', 'date', or 'price' - to specify order append ',asc' or ',desc' + to specify direction append ',asc' or ',desc' Returns: (list) List of files @@ -1459,8 +1459,8 @@ class Daemon(AuthJSONRPCServer): ] """ - sorts = [self._parse_lbry_files_sort(s) for s in sort] if sort else None - result = yield self._get_lbry_files(return_json=True, sorts=sorts, **kwargs) + sort_by = [self._parse_lbry_files_sort(s) for s in sort] if sort else None + result = yield self._get_lbry_files(return_json=True, sort_by=sort_by, **kwargs) response = yield self._render_response(result) defer.returnValue(response) From 852354719f058cf4d0710219aea8486a059f2d29 Mon Sep 17 00:00:00 2001 From: kafene Date: Tue, 1 May 2018 19:30:48 -0400 Subject: [PATCH 06/11] add unit test for file list sorting --- .../tests/unit/lbrynet_daemon/test_Daemon.py | 139 ++++++++++++++++++ 1 file changed, 139 insertions(+) diff --git a/lbrynet/tests/unit/lbrynet_daemon/test_Daemon.py b/lbrynet/tests/unit/lbrynet_daemon/test_Daemon.py index ae9451323..77dba310a 100644 --- a/lbrynet/tests/unit/lbrynet_daemon/test_Daemon.py +++ b/lbrynet/tests/unit/lbrynet_daemon/test_Daemon.py @@ -1,16 +1,21 @@ import mock import json import unittest +from os import path from twisted.internet import defer from twisted import trial +from faker import Faker + from lbryschema.decode import smart_decode from lbryum.wallet import NewWallet from lbrynet import conf from lbrynet.core import Session, PaymentRateManager, Wallet from lbrynet.database.storage import SQLiteStorage from lbrynet.daemon.Daemon import Daemon as LBRYDaemon +from lbrynet.file_manager.EncryptedFileManager import EncryptedFileManager +from lbrynet.file_manager.EncryptedFileDownloader import ManagedEncryptedFileDownloader from lbrynet.tests import util from lbrynet.tests.mocks import mock_conf_settings, FakeNetwork @@ -126,3 +131,137 @@ class TestJsonRpc(trial.unittest.TestCase): d = defer.maybeDeferred(self.test_daemon.jsonrpc_help, command='status') d.addCallback(lambda result: self.assertSubstring('daemon status', result['help'])) # self.assertSubstring('daemon status', d.result) + + +class TestFileListSorting(trial.unittest.TestCase): + def setUp(self): + mock_conf_settings(self) + util.resetTime(self) + self.faker = Faker('en_US') + self.faker.seed(66410) + self.test_daemon = get_test_daemon() + self.test_daemon.lbry_file_manager = mock.Mock(spec=EncryptedFileManager) + self.test_daemon.lbry_file_manager.lbry_files = self._get_fake_lbry_files() + + # Pre-sorted lists of prices and file names in ascending order produced by + # faker with seed 66410. This seed was chosen becacuse it produces 3 results + # 'points_paid' at 6.0 and 2 results at 4.5 to test multiple sort criteria. + self.test_prices = [0.2, 2.9, 4.5, 4.5, 6.0, 6.0, 6.0, 6.8, 7.1, 9.2] + self.test_file_names = ['also.mp3', 'better.css', 'call.mp3', 'pay.jpg', + 'record.pages', 'sell.css', 'strategy.pages', + 'thousand.pages', 'town.mov', 'vote.ppt'] + + @defer.inlineCallbacks + def test_sort_by_price_no_direction_specified(self): + sort_options = ['price'] + file_list = yield self.test_daemon.jsonrpc_file_list(sort=sort_options) + received = [f['points_paid'] for f in file_list] + self.assertEquals(self.test_prices, received) + + @defer.inlineCallbacks + def test_sort_by_price_ascending(self): + sort_options = ['price,asc'] + file_list = yield self.test_daemon.jsonrpc_file_list(sort=sort_options) + received = [f['points_paid'] for f in file_list] + self.assertEquals(self.test_prices, received) + + @defer.inlineCallbacks + def test_sort_by_price_descending(self): + sort_options = ['price, desc'] + file_list = yield self.test_daemon.jsonrpc_file_list(sort=sort_options) + received = [f['points_paid'] for f in file_list] + expected = list(reversed(self.test_prices)) + self.assertEquals(expected, received) + + @defer.inlineCallbacks + def test_sort_by_name_no_direction_specified(self): + sort_options = ['name'] + file_list = yield self.test_daemon.jsonrpc_file_list(sort=sort_options) + received = [f['file_name'] for f in file_list] + self.assertEquals(self.test_file_names, received) + + @defer.inlineCallbacks + def test_sort_by_name_ascending(self): + sort_options = ['name,\nasc'] + file_list = yield self.test_daemon.jsonrpc_file_list(sort=sort_options) + received = [f['file_name'] for f in file_list] + self.assertEquals(self.test_file_names, received) + + @defer.inlineCallbacks + def test_sort_by_name_descending(self): + sort_options = ['\tname,\n\tdesc'] + file_list = yield self.test_daemon.jsonrpc_file_list(sort=sort_options) + received = [f['file_name'] for f in file_list] + expected = list(reversed(self.test_file_names)) + self.assertEquals(expected, received) + + @defer.inlineCallbacks + def test_sort_by_multiple_criteria(self): + sort_options = ['name,asc', 'price,desc'] + file_list = yield self.test_daemon.jsonrpc_file_list(sort=sort_options) + received = ['name={}, price={}'.format(f['file_name'], f['points_paid']) for f in file_list] + expected = ['name=record.pages, price=9.2', + 'name=vote.ppt, price=7.1', + 'name=strategy.pages, price=6.8', + 'name=also.mp3, price=6.0', + 'name=better.css, price=6.0', + 'name=town.mov, price=6.0', + 'name=sell.css, price=4.5', + 'name=thousand.pages, price=4.5', + 'name=call.mp3, price=2.9', + 'name=pay.jpg, price=0.2'] + self.assertEquals(expected, received) + + # Check that the list is not sorted as expected when sorted only by name. + sort_options = ['name,asc'] + file_list = yield self.test_daemon.jsonrpc_file_list(sort=sort_options) + received = ['name={}, price={}'.format(f['file_name'], f['points_paid']) for f in file_list] + self.assertNotEqual(expected, received) + + # Check that the list is not sorted as expected when sorted only by price. + sort_options = ['price,desc'] + file_list = yield self.test_daemon.jsonrpc_file_list(sort=sort_options) + received = ['name={}, price={}'.format(f['file_name'], f['points_paid']) for f in file_list] + self.assertNotEqual(expected, received) + + # Check that the list is not sorted as expected when not sorted at all. + file_list = yield self.test_daemon.jsonrpc_file_list() + received = ['name={}, price={}'.format(f['file_name'], f['points_paid']) for f in file_list] + self.assertNotEqual(expected, received) + + def _get_fake_lbry_files(self): + return [self._get_fake_lbry_file() for _ in xrange(10)] + + def _get_fake_lbry_file(self): + lbry_file = mock.Mock(spec=ManagedEncryptedFileDownloader) + + file_path = self.faker.file_path() + stream_name = self.faker.file_name() + faked_attributes = { + 'channel_claim_id': self.faker.sha1(), + 'channel_name': '@' + self.faker.simple_profile()['username'], + 'claim_id': self.faker.sha1(), + 'claim_name': '-'.join(self.faker.words(4)), + 'completed': self.faker.boolean(), + 'download_directory': path.dirname(file_path), + 'download_path': file_path, + 'file_name': path.basename(file_path), + 'key': self.faker.md5(), + 'metadata': {}, + 'mime_type': self.faker.mime_type(), + 'nout': abs(self.faker.pyint()), + 'outpoint': self.faker.md5() + self.faker.md5(), + 'points_paid': self.faker.pyfloat(left_digits=1, right_digits=1, positive=True), + 'sd_hash': self.faker.md5() + self.faker.md5() + self.faker.md5(), + 'stopped': self.faker.boolean(), + 'stream_hash': self.faker.md5() + self.faker.md5() + self.faker.md5(), + 'stream_name': stream_name, + 'suggested_file_name': stream_name, + 'txid': self.faker.md5() + self.faker.md5(), + 'written_bytes': self.faker.pyint(), + } + + for key in faked_attributes: + setattr(lbry_file, key, faked_attributes[key]) + + return lbry_file From ee4b47aedc4911e3797288fd2c159a231fd86698 Mon Sep 17 00:00:00 2001 From: kafene Date: Tue, 1 May 2018 19:39:00 -0400 Subject: [PATCH 07/11] add directive to install packages in requirements_testing.txt for travis --- .travis.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.travis.yml b/.travis.yml index 09e9e6a16..d5d248513 100644 --- a/.travis.yml +++ b/.travis.yml @@ -35,6 +35,7 @@ before_install: install: - pip install -U pip==9.0.3 - pip install -r requirements.txt + - pip install -r requirements_testing.txt - pip install . script: From 72bae90e061488d148ff654ba0daccce93143867 Mon Sep 17 00:00:00 2001 From: kafene Date: Wed, 2 May 2018 14:32:08 -0400 Subject: [PATCH 08/11] use range instead of xrange, list is only 10 elements and range works in python3 --- lbrynet/tests/unit/lbrynet_daemon/test_Daemon.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lbrynet/tests/unit/lbrynet_daemon/test_Daemon.py b/lbrynet/tests/unit/lbrynet_daemon/test_Daemon.py index 77dba310a..6d032ef78 100644 --- a/lbrynet/tests/unit/lbrynet_daemon/test_Daemon.py +++ b/lbrynet/tests/unit/lbrynet_daemon/test_Daemon.py @@ -230,7 +230,7 @@ class TestFileListSorting(trial.unittest.TestCase): self.assertNotEqual(expected, received) def _get_fake_lbry_files(self): - return [self._get_fake_lbry_file() for _ in xrange(10)] + return [self._get_fake_lbry_file() for _ in range(10)] def _get_fake_lbry_file(self): lbry_file = mock.Mock(spec=ManagedEncryptedFileDownloader) From 5069351287fd9c32df6d82a4299a16cef963d9ee Mon Sep 17 00:00:00 2001 From: kafene Date: Mon, 7 May 2018 17:37:41 -0400 Subject: [PATCH 09/11] refactor file_list sort to allow sorting by any field --- lbrynet/daemon/Daemon.py | 44 ++--- .../tests/unit/lbrynet_daemon/test_Daemon.py | 176 +++++++++++------- 2 files changed, 129 insertions(+), 91 deletions(-) diff --git a/lbrynet/daemon/Daemon.py b/lbrynet/daemon/Daemon.py index ae913b182..8f4742b8c 100644 --- a/lbrynet/daemon/Daemon.py +++ b/lbrynet/daemon/Daemon.py @@ -9,6 +9,7 @@ import json import textwrap import signal from copy import deepcopy +from operator import attrgetter from twisted.web import server from twisted.internet import defer, threads, error, reactor from twisted.internet.task import LoopingCall @@ -97,10 +98,6 @@ CONNECTION_MESSAGES = { SHORT_ID_LEN = 20 MAX_UPDATE_FEE_ESTIMATE = 0.3 -FILE_SORT_FIELD_NAME = 'name' -FILE_SORT_FIELD_DATE = 'date' -FILE_SORT_FIELD_PRICE = 'price' - FILE_SORT_DIRECTION_ASCENDING = 'asc' FILE_SORT_DIRECTION_DESCENDING = 'desc' FILE_SORT_DIRECTIONS = ( @@ -935,13 +932,11 @@ class Daemon(AuthJSONRPCServer): defer.returnValue(lbry_file) @defer.inlineCallbacks - def _get_lbry_files(self, return_json=False, full_status=True, sort_by=None, **kwargs): + def _get_lbry_files(self, return_json=False, full_status=True, **kwargs): lbry_files = list(self.lbry_file_manager.lbry_files) if kwargs: for search_type, value in iter_lbry_file_search_values(kwargs): lbry_files = [l_f for l_f in lbry_files if l_f.__dict__[search_type] == value] - if sort_by: - lbry_files = self._sort_lbry_files(lbry_files, sort_by) if return_json: file_dicts = [] for lbry_file in lbry_files: @@ -954,25 +949,29 @@ class Daemon(AuthJSONRPCServer): def _sort_lbry_files(self, lbry_files, sort_by): for field, direction in sort_by: is_reverse = direction == FILE_SORT_DIRECTION_DESCENDING - if field == FILE_SORT_FIELD_NAME: - lbry_files = sorted(lbry_files, key=lambda f: f.file_name, reverse=is_reverse) - elif field == FILE_SORT_FIELD_DATE: - lbry_files = sorted(lbry_files, reverse=is_reverse) - elif field == FILE_SORT_FIELD_PRICE: - lbry_files = sorted(lbry_files, key=lambda f: f.points_paid, reverse=is_reverse) - else: - raise Exception('Unrecognized sort field "{}"'.format(field)) + key_getter = None + if field: + search_path = field.split('.') + def key_getter(value): + for key in search_path: + try: + value = value[key] + except KeyError as e: + errmsg = 'Failed to sort by "{}", key "{}" was not found.' + raise Exception(errmsg.format(field, e.message)) + return value + lbry_files = sorted(lbry_files, key=key_getter, reverse=is_reverse) return lbry_files def _parse_lbry_files_sort(self, sort): """ - Given a sort string like 'name, desc' or 'price', + Given a sort string like 'file_name, desc' or 'points_paid', parse the string into a tuple of (field, direction). Direction defaults to ascending. """ pieces = sort.rsplit(',', 1) - field = pieces[0].strip() + field = pieces[0].strip() or None direction = pieces[1].strip().lower() if len(pieces) > 1 else None if direction and direction not in FILE_SORT_DIRECTIONS: raise Exception('Sort direction must be one of {}'.format(FILE_SORT_DIRECTIONS)) @@ -1422,8 +1421,9 @@ class Daemon(AuthJSONRPCServer): --claim_name= : (str) get file with matching claim name --full_status : (bool) full status, populate the 'message' and 'size' fields - --sort= : (str) sort by any of 'name', 'date', or 'price' - to specify direction append ',asc' or ',desc' + --sort= : (str) sort by any property, like 'file_name' + or 'metadata.author'; to specify direction + append ',asc' or ',desc' Returns: (list) List of files @@ -1459,8 +1459,10 @@ class Daemon(AuthJSONRPCServer): ] """ - sort_by = [self._parse_lbry_files_sort(s) for s in sort] if sort else None - result = yield self._get_lbry_files(return_json=True, sort_by=sort_by, **kwargs) + result = yield self._get_lbry_files(return_json=True, **kwargs) + if sort: + sort_by = [self._parse_lbry_files_sort(s) for s in sort] + result = self._sort_lbry_files(result, sort_by) response = yield self._render_response(result) defer.returnValue(response) diff --git a/lbrynet/tests/unit/lbrynet_daemon/test_Daemon.py b/lbrynet/tests/unit/lbrynet_daemon/test_Daemon.py index 6d032ef78..af242af3f 100644 --- a/lbrynet/tests/unit/lbrynet_daemon/test_Daemon.py +++ b/lbrynet/tests/unit/lbrynet_daemon/test_Daemon.py @@ -1,6 +1,7 @@ import mock import json import unittest +import random from os import path from twisted.internet import defer @@ -146,88 +147,118 @@ class TestFileListSorting(trial.unittest.TestCase): # Pre-sorted lists of prices and file names in ascending order produced by # faker with seed 66410. This seed was chosen becacuse it produces 3 results # 'points_paid' at 6.0 and 2 results at 4.5 to test multiple sort criteria. - self.test_prices = [0.2, 2.9, 4.5, 4.5, 6.0, 6.0, 6.0, 6.8, 7.1, 9.2] + self.test_points_paid = [0.2, 2.9, 4.5, 4.5, 6.0, 6.0, 6.0, 6.8, 7.1, 9.2] self.test_file_names = ['also.mp3', 'better.css', 'call.mp3', 'pay.jpg', 'record.pages', 'sell.css', 'strategy.pages', 'thousand.pages', 'town.mov', 'vote.ppt'] + self.test_authors = ['angela41', 'edward70', 'fhart', 'johnrosales', + 'lucasfowler', 'peggytorres', 'qmitchell', + 'trevoranderson', 'xmitchell', 'zhangsusan'] - @defer.inlineCallbacks - def test_sort_by_price_no_direction_specified(self): - sort_options = ['price'] - file_list = yield self.test_daemon.jsonrpc_file_list(sort=sort_options) - received = [f['points_paid'] for f in file_list] - self.assertEquals(self.test_prices, received) + def test_sort_by_points_paid_no_direction_specified(self): + sort_options = ['points_paid'] + deferred = defer.maybeDeferred(self.test_daemon.jsonrpc_file_list, sort=sort_options) + file_list = self.successResultOf(deferred) + self.assertEquals(self.test_points_paid, [f['points_paid'] for f in file_list]) - @defer.inlineCallbacks - def test_sort_by_price_ascending(self): - sort_options = ['price,asc'] - file_list = yield self.test_daemon.jsonrpc_file_list(sort=sort_options) - received = [f['points_paid'] for f in file_list] - self.assertEquals(self.test_prices, received) + def test_sort_by_points_paid_ascending(self): + sort_options = ['points_paid,asc'] + deferred = defer.maybeDeferred(self.test_daemon.jsonrpc_file_list, sort=sort_options) + file_list = self.successResultOf(deferred) + self.assertEquals(self.test_points_paid, [f['points_paid'] for f in file_list]) - @defer.inlineCallbacks - def test_sort_by_price_descending(self): - sort_options = ['price, desc'] - file_list = yield self.test_daemon.jsonrpc_file_list(sort=sort_options) - received = [f['points_paid'] for f in file_list] - expected = list(reversed(self.test_prices)) - self.assertEquals(expected, received) + def test_sort_by_points_paid_descending(self): + sort_options = ['points_paid, desc'] + deferred = defer.maybeDeferred(self.test_daemon.jsonrpc_file_list, sort=sort_options) + file_list = self.successResultOf(deferred) + self.assertEquals(list(reversed(self.test_points_paid)), [f['points_paid'] for f in file_list]) - @defer.inlineCallbacks - def test_sort_by_name_no_direction_specified(self): - sort_options = ['name'] - file_list = yield self.test_daemon.jsonrpc_file_list(sort=sort_options) - received = [f['file_name'] for f in file_list] - self.assertEquals(self.test_file_names, received) + def test_sort_by_file_name_no_direction_specified(self): + sort_options = ['file_name'] + deferred = defer.maybeDeferred(self.test_daemon.jsonrpc_file_list, sort=sort_options) + file_list = self.successResultOf(deferred) + self.assertEquals(self.test_file_names, [f['file_name'] for f in file_list]) - @defer.inlineCallbacks - def test_sort_by_name_ascending(self): - sort_options = ['name,\nasc'] - file_list = yield self.test_daemon.jsonrpc_file_list(sort=sort_options) - received = [f['file_name'] for f in file_list] - self.assertEquals(self.test_file_names, received) + def test_sort_by_file_name_ascending(self): + sort_options = ['file_name,\nasc'] + deferred = defer.maybeDeferred(self.test_daemon.jsonrpc_file_list, sort=sort_options) + file_list = self.successResultOf(deferred) + self.assertEquals(self.test_file_names, [f['file_name'] for f in file_list]) - @defer.inlineCallbacks - def test_sort_by_name_descending(self): - sort_options = ['\tname,\n\tdesc'] - file_list = yield self.test_daemon.jsonrpc_file_list(sort=sort_options) - received = [f['file_name'] for f in file_list] - expected = list(reversed(self.test_file_names)) - self.assertEquals(expected, received) + def test_sort_by_file_name_descending(self): + sort_options = ['\tfile_name,\n\tdesc'] + deferred = defer.maybeDeferred(self.test_daemon.jsonrpc_file_list, sort=sort_options) + file_list = self.successResultOf(deferred) + self.assertEquals(list(reversed(self.test_file_names)), [f['file_name'] for f in file_list]) - @defer.inlineCallbacks def test_sort_by_multiple_criteria(self): - sort_options = ['name,asc', 'price,desc'] - file_list = yield self.test_daemon.jsonrpc_file_list(sort=sort_options) - received = ['name={}, price={}'.format(f['file_name'], f['points_paid']) for f in file_list] - expected = ['name=record.pages, price=9.2', - 'name=vote.ppt, price=7.1', - 'name=strategy.pages, price=6.8', - 'name=also.mp3, price=6.0', - 'name=better.css, price=6.0', - 'name=town.mov, price=6.0', - 'name=sell.css, price=4.5', - 'name=thousand.pages, price=4.5', - 'name=call.mp3, price=2.9', - 'name=pay.jpg, price=0.2'] - self.assertEquals(expected, received) + expected = ['file_name=record.pages, points_paid=9.2', + 'file_name=vote.ppt, points_paid=7.1', + 'file_name=strategy.pages, points_paid=6.8', + 'file_name=also.mp3, points_paid=6.0', + 'file_name=better.css, points_paid=6.0', + 'file_name=town.mov, points_paid=6.0', + 'file_name=sell.css, points_paid=4.5', + 'file_name=thousand.pages, points_paid=4.5', + 'file_name=call.mp3, points_paid=2.9', + 'file_name=pay.jpg, points_paid=0.2'] + format_result = lambda f: 'file_name={}, points_paid={}'.format(f['file_name'], f['points_paid']) - # Check that the list is not sorted as expected when sorted only by name. - sort_options = ['name,asc'] - file_list = yield self.test_daemon.jsonrpc_file_list(sort=sort_options) - received = ['name={}, price={}'.format(f['file_name'], f['points_paid']) for f in file_list] - self.assertNotEqual(expected, received) + sort_options = ['file_name,asc', 'points_paid,desc'] + deferred = defer.maybeDeferred(self.test_daemon.jsonrpc_file_list, sort=sort_options) + file_list = self.successResultOf(deferred) + self.assertEquals(expected, map(format_result, file_list)) - # Check that the list is not sorted as expected when sorted only by price. - sort_options = ['price,desc'] - file_list = yield self.test_daemon.jsonrpc_file_list(sort=sort_options) - received = ['name={}, price={}'.format(f['file_name'], f['points_paid']) for f in file_list] - self.assertNotEqual(expected, received) + # Check that the list is not sorted as expected when sorted only by file_name. + sort_options = ['file_name,asc'] + deferred = defer.maybeDeferred(self.test_daemon.jsonrpc_file_list, sort=sort_options) + file_list = self.successResultOf(deferred) + self.assertNotEqual(expected, map(format_result, file_list)) + + # Check that the list is not sorted as expected when sorted only by points_paid. + sort_options = ['points_paid,desc'] + deferred = defer.maybeDeferred(self.test_daemon.jsonrpc_file_list, sort=sort_options) + file_list = self.successResultOf(deferred) + self.assertNotEqual(expected, map(format_result, file_list)) # Check that the list is not sorted as expected when not sorted at all. - file_list = yield self.test_daemon.jsonrpc_file_list() - received = ['name={}, price={}'.format(f['file_name'], f['points_paid']) for f in file_list] - self.assertNotEqual(expected, received) + deferred = defer.maybeDeferred(self.test_daemon.jsonrpc_file_list) + file_list = self.successResultOf(deferred) + self.assertNotEqual(expected, map(format_result, file_list)) + + def test_sort_by_nested_field(self): + extract_authors = lambda file_list: [f['metadata']['author'] for f in file_list] + + sort_options = ['metadata.author'] + deferred = defer.maybeDeferred(self.test_daemon.jsonrpc_file_list, sort=sort_options) + file_list = self.successResultOf(deferred) + self.assertEquals(self.test_authors, extract_authors(file_list)) + + # Check that the list matches the expected in reverse when sorting in descending order. + sort_options = ['metadata.author,desc'] + deferred = defer.maybeDeferred(self.test_daemon.jsonrpc_file_list, sort=sort_options) + file_list = self.successResultOf(deferred) + self.assertEquals(list(reversed(self.test_authors)), extract_authors(file_list)) + + # Check that the list is not sorted as expected when not sorted at all. + deferred = defer.maybeDeferred(self.test_daemon.jsonrpc_file_list) + file_list = self.successResultOf(deferred) + self.assertNotEqual(self.test_authors, extract_authors(file_list)) + + def test_invalid_sort_produces_meaningful_errors(self): + sort_options = ['meta.author'] + deferred = defer.maybeDeferred(self.test_daemon.jsonrpc_file_list, sort=sort_options) + failure_assertion = self.assertFailure(deferred, Exception) + exception = self.successResultOf(failure_assertion) + expected_message = 'Failed to sort by "meta.author", key "meta" was not found.' + self.assertEquals(expected_message, exception.message) + + sort_options = ['metadata.foo.bar'] + deferred = defer.maybeDeferred(self.test_daemon.jsonrpc_file_list, sort=sort_options) + failure_assertion = self.assertFailure(deferred, Exception) + exception = self.successResultOf(failure_assertion) + expected_message = 'Failed to sort by "metadata.foo.bar", key "foo" was not found.' + self.assertEquals(expected_message, exception.message) def _get_fake_lbry_files(self): return [self._get_fake_lbry_file() for _ in range(10)] @@ -237,9 +268,11 @@ class TestFileListSorting(trial.unittest.TestCase): file_path = self.faker.file_path() stream_name = self.faker.file_name() + channel_claim_id = self.faker.sha1() + channel_name = self.faker.simple_profile()['username'] faked_attributes = { - 'channel_claim_id': self.faker.sha1(), - 'channel_name': '@' + self.faker.simple_profile()['username'], + 'channel_claim_id': channel_claim_id, + 'channel_name': '@' + channel_name, 'claim_id': self.faker.sha1(), 'claim_name': '-'.join(self.faker.words(4)), 'completed': self.faker.boolean(), @@ -247,7 +280,10 @@ class TestFileListSorting(trial.unittest.TestCase): 'download_path': file_path, 'file_name': path.basename(file_path), 'key': self.faker.md5(), - 'metadata': {}, + 'metadata': { + 'author': channel_name, + 'nsfw': random.randint(0, 1) == 1, + }, 'mime_type': self.faker.mime_type(), 'nout': abs(self.faker.pyint()), 'outpoint': self.faker.md5() + self.faker.md5(), From 1790393273cc3501d8c71cab227a78818f330aca Mon Sep 17 00:00:00 2001 From: kafene Date: Mon, 7 May 2018 17:38:34 -0400 Subject: [PATCH 10/11] remove unused attrgetter import --- lbrynet/daemon/Daemon.py | 1 - 1 file changed, 1 deletion(-) diff --git a/lbrynet/daemon/Daemon.py b/lbrynet/daemon/Daemon.py index 8f4742b8c..a9ba048eb 100644 --- a/lbrynet/daemon/Daemon.py +++ b/lbrynet/daemon/Daemon.py @@ -9,7 +9,6 @@ import json import textwrap import signal from copy import deepcopy -from operator import attrgetter from twisted.web import server from twisted.internet import defer, threads, error, reactor from twisted.internet.task import LoopingCall From 76a7cc4e586733bc15749d1465dd463c3660fe83 Mon Sep 17 00:00:00 2001 From: kafene Date: Mon, 21 May 2018 00:24:18 -0400 Subject: [PATCH 11/11] refactor file list sorting per request --- lbrynet/daemon/Daemon.py | 49 ++++++++++--------- .../tests/unit/lbrynet_daemon/test_Daemon.py | 4 +- 2 files changed, 27 insertions(+), 26 deletions(-) diff --git a/lbrynet/daemon/Daemon.py b/lbrynet/daemon/Daemon.py index a9ba048eb..b0278d653 100644 --- a/lbrynet/daemon/Daemon.py +++ b/lbrynet/daemon/Daemon.py @@ -97,12 +97,9 @@ CONNECTION_MESSAGES = { SHORT_ID_LEN = 20 MAX_UPDATE_FEE_ESTIMATE = 0.3 -FILE_SORT_DIRECTION_ASCENDING = 'asc' -FILE_SORT_DIRECTION_DESCENDING = 'desc' -FILE_SORT_DIRECTIONS = ( - FILE_SORT_DIRECTION_ASCENDING, - FILE_SORT_DIRECTION_DESCENDING, -) +DIRECTION_ASCENDING = 'asc' +DIRECTION_DESCENDING = 'desc' +DIRECTIONS = DIRECTION_ASCENDING, DIRECTION_DESCENDING class IterableContainer(object): def __iter__(self): @@ -947,18 +944,8 @@ class Daemon(AuthJSONRPCServer): def _sort_lbry_files(self, lbry_files, sort_by): for field, direction in sort_by: - is_reverse = direction == FILE_SORT_DIRECTION_DESCENDING - key_getter = None - if field: - search_path = field.split('.') - def key_getter(value): - for key in search_path: - try: - value = value[key] - except KeyError as e: - errmsg = 'Failed to sort by "{}", key "{}" was not found.' - raise Exception(errmsg.format(field, e.message)) - return value + is_reverse = direction == DIRECTION_DESCENDING + key_getter = create_key_getter(field) if field else None lbry_files = sorted(lbry_files, key=key_getter, reverse=is_reverse) return lbry_files @@ -969,12 +956,13 @@ class Daemon(AuthJSONRPCServer): Direction defaults to ascending. """ - pieces = sort.rsplit(',', 1) - field = pieces[0].strip() or None - direction = pieces[1].strip().lower() if len(pieces) > 1 else None - if direction and direction not in FILE_SORT_DIRECTIONS: - raise Exception('Sort direction must be one of {}'.format(FILE_SORT_DIRECTIONS)) - return (field, direction or FILE_SORT_DIRECTION_ASCENDING) + pieces = [p.strip() for p in sort.split(',')] + field = pieces.pop(0) + direction = DIRECTION_ASCENDING + if pieces and pieces[0] in DIRECTIONS: + direction = pieces[0] + return field, direction + def _get_single_peer_downloader(self): downloader = SinglePeerDownloader() @@ -3435,3 +3423,16 @@ def get_blob_payment_rate_manager(session, payment_rate_manager=None): payment_rate_manager = rate_managers[payment_rate_manager] log.info("Downloading blob with rate manager: %s", payment_rate_manager) return payment_rate_manager or session.payment_rate_manager + + +def create_key_getter(field): + search_path = field.split('.') + def key_getter(value): + for key in search_path: + try: + value = value[key] + except KeyError as e: + errmsg = 'Failed to get "{}", key "{}" was not found.' + raise Exception(errmsg.format(field, e.message)) + return value + return key_getter diff --git a/lbrynet/tests/unit/lbrynet_daemon/test_Daemon.py b/lbrynet/tests/unit/lbrynet_daemon/test_Daemon.py index af242af3f..d47c36ba2 100644 --- a/lbrynet/tests/unit/lbrynet_daemon/test_Daemon.py +++ b/lbrynet/tests/unit/lbrynet_daemon/test_Daemon.py @@ -250,14 +250,14 @@ class TestFileListSorting(trial.unittest.TestCase): deferred = defer.maybeDeferred(self.test_daemon.jsonrpc_file_list, sort=sort_options) failure_assertion = self.assertFailure(deferred, Exception) exception = self.successResultOf(failure_assertion) - expected_message = 'Failed to sort by "meta.author", key "meta" was not found.' + expected_message = 'Failed to get "meta.author", key "meta" was not found.' self.assertEquals(expected_message, exception.message) sort_options = ['metadata.foo.bar'] deferred = defer.maybeDeferred(self.test_daemon.jsonrpc_file_list, sort=sort_options) failure_assertion = self.assertFailure(deferred, Exception) exception = self.successResultOf(failure_assertion) - expected_message = 'Failed to sort by "metadata.foo.bar", key "foo" was not found.' + expected_message = 'Failed to get "metadata.foo.bar", key "foo" was not found.' self.assertEquals(expected_message, exception.message) def _get_fake_lbry_files(self):