From 455fd0e352df3872b91cb5ae394019f8bea2139b Mon Sep 17 00:00:00 2001 From: Diego Moreda Date: Tue, 24 Nov 2015 18:47:49 +0100 Subject: [PATCH 1/2] Fix sorting by nonexistent field (#1734), tests * Fix crash when sorting by nonexistent field. * Add tests for queries with nonexistient fields for sorting. --- beets/dbcore/query.py | 2 +- test/test_sort.py | 63 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 64 insertions(+), 1 deletion(-) diff --git a/beets/dbcore/query.py b/beets/dbcore/query.py index 424d51e7c..f0adac665 100644 --- a/beets/dbcore/query.py +++ b/beets/dbcore/query.py @@ -767,7 +767,7 @@ class FieldSort(Sort): # attributes with different types without falling over. def key(item): - field_val = getattr(item, self.field) + field_val = item.get(self.field, '') if self.case_insensitive and isinstance(field_val, unicode): field_val = field_val.lower() return field_val diff --git a/test/test_sort.py b/test/test_sort.py index 7fadc54b2..9413e7d7b 100644 --- a/test/test_sort.py +++ b/test/test_sort.py @@ -469,6 +469,69 @@ class CaseSensitivityTest(DummyDataTestCase, _common.TestCase): self.assertEqual(results[-1].track, 10) +class NonExistingFieldTest(DummyDataTestCase): + """Test sorting by non-existing fields""" + + def test_non_existing_fields_not_fail(self): + qs = ['foo+', 'foo-', '--', '-+', '+-', '++', '-foo-', '-foo+', '---'] + + q0 = 'foo+' + results0 = list(self.lib.items(q0)) + for q1 in qs: + results1 = list(self.lib.items(q1)) + for r1, r2 in zip(results0, results1): + self.assertEqual(r1.id, r2.id) + + def test_combined_non_existing_field_asc(self): + all_results = list(self.lib.items('id+')) + q = 'foo+ id+' + results = list(self.lib.items(q)) + self.assertEqual(len(all_results), len(results)) + for r1, r2 in zip(all_results, results): + self.assertEqual(r1.id, r2.id) + + def test_combined_non_existing_field_desc(self): + all_results = list(self.lib.items('id+')) + q = 'foo- id+' + results = list(self.lib.items(q)) + self.assertEqual(len(all_results), len(results)) + for r1, r2 in zip(all_results, results): + self.assertEqual(r1.id, r2.id) + + def test_field_present_in_some_items(self): + """Test ordering by a field not present on all items.""" + # append 'foo' to two to items (1,2) + items = self.lib.items('id+') + ids = [i.id for i in items] + items[1].foo = 'bar1' + items[2].foo = 'bar2' + items[1].store() + items[2].store() + + results_asc = list(self.lib.items('foo+ id+')) + self.assertEqual([i.id for i in results_asc], + # items without field first + [ids[0], ids[3], ids[1], ids[2]]) + results_desc = list(self.lib.items('foo- id+')) + self.assertEqual([i.id for i in results_desc], + # items without field last + [ids[2], ids[1], ids[0], ids[3]]) + + def test_negation_interaction(self): + """Test the handling of negation and sorting together. + + If a string ends with a sorting suffix, it takes precedence over the + NotQuery parsing. + """ + query, sort = beets.library.parse_query_string('-bar+', + beets.library.Item) + self.assertEqual(len(query.subqueries), 1) + self.assertTrue(isinstance(query.subqueries[0], + dbcore.query.TrueQuery)) + self.assertTrue(isinstance(sort, dbcore.query.SlowFieldSort)) + self.assertEqual(sort.field, '-bar') + + def suite(): return unittest.TestLoader().loadTestsFromName(__name__) From 0ed3f4067af5e8112ee97a371128e71b9ae57621 Mon Sep 17 00:00:00 2001 From: Diego Moreda Date: Tue, 24 Nov 2015 19:43:48 +0100 Subject: [PATCH 2/2] Add changelog entry for nonexistent fields sorting --- docs/changelog.rst | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/changelog.rst b/docs/changelog.rst index ce707ab88..54c70f370 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -66,6 +66,7 @@ Fixes: with some library versions. :bug:`1433` * :doc:`/plugins/convert`: Fix a crash with Unicode paths in ``--pretend`` mode. :bug:`1735` +* Fix a crash when sorting by nonexistent fields on queries. :bug:`1734` .. _Emby Server: http://emby.media