Merge pull request #1736 from diego-plan9/querysort

Fix for crash when sorting by nonexistent field (#1734)

beets/dbcore/query.py

@@ -767,7 +767,7 @@ def sort(self, objs):
         # 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

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
 

test/test_sort.py

@@ -469,6 +469,69 @@ def test_case_sensitive_only_affects_text(self):
         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__)