diff --git a/wagtail/tests/models.py b/wagtail/tests/models.py index a38d889bd..eab72d232 100644 --- a/wagtail/tests/models.py +++ b/wagtail/tests/models.py @@ -420,6 +420,8 @@ class SearchTestOldConfig(models.Model, indexed.Indexed): """ This tests that the Indexed class can correctly handle models that use the old "indexed_fields" configuration format. + + This format was deprecated in 0.4 and removed in 0.6 """ indexed_fields = { # A search field with predictive search and boosting @@ -437,13 +439,27 @@ class SearchTestOldConfig(models.Model, indexed.Indexed): } -class SearchTestOldConfigList(models.Model, indexed.Indexed): +class SearchTestOldConfigAndNewConfig(models.Model, indexed.Indexed): """ This tests that the Indexed class can correctly handle models that - use the old "indexed_fields" configuration format using a list. + use both the old "indexed_fields" and the new "search_fields" configuration + format. + + Usually, when wagtailsearch detects that "indexed_fields" is being used, it + will raise a RuntimeError. + + This behaviour may get in the way of people developing apps that support + older versions of Wagtail. So Wagtail allows "indexed_fields" to be defined + on a classes as long as they also define "search_fields" as well. Wagtail 0.4+ + will simply ignore the "indexed_fields" attribute in this case. """ indexed_fields = ['title', 'content'] + search_fields = ( + indexed.SearchField('title'), + indexed.SearchField('content'), + ) + def routable_page_external_view(request, arg): return HttpResponse("EXTERNAL VIEW: " + arg) diff --git a/wagtail/wagtailsearch/indexed.py b/wagtail/wagtailsearch/indexed.py index e775ff5c7..eb5933751 100644 --- a/wagtail/wagtailsearch/indexed.py +++ b/wagtail/wagtailsearch/indexed.py @@ -37,80 +37,19 @@ class Indexed(object): # At toplevel, return this content type return (cls._meta.app_label + '_' + cls.__name__).lower() - @classmethod - def indexed_get_indexed_fields(cls): - # Get indexed fields for this class as dictionary - indexed_fields = cls.indexed_fields - if isinstance(indexed_fields, dict): - # Make sure we have a copy to prevent us accidentally changing the configuration - indexed_fields = indexed_fields.copy() - else: - # Convert to dict - if isinstance(indexed_fields, tuple): - indexed_fields = list(indexed_fields) - if isinstance(indexed_fields, string_types): - indexed_fields = [indexed_fields] - if isinstance(indexed_fields, list): - indexed_fields = dict((field, dict(type='string')) for field in indexed_fields) - if not isinstance(indexed_fields, dict): - raise ValueError() - - # Get indexed fields for parent class - parent = cls.indexed_get_parent(require_model=False) - if parent: - # Add parent fields into this list - parent_indexed_fields = parent.indexed_get_indexed_fields().copy() - parent_indexed_fields.update(indexed_fields) - indexed_fields = parent_indexed_fields - return indexed_fields - @classmethod def get_search_fields(cls): - search_fields = [] + # Raise an error if the 'indexed_fields' attribute is being used on a class without 'search_fields' + # Note: We still allow people to define 'indexed_fields' as long as they also define 'search_fields' + # on the same class. This allows people to still write code that is compatible with older versions + # of Wagtail and we still catch issues where code using the old 'indexed_fields' setting hasn't been + # updated. + if 'indexed_fields' in cls.__dict__ and not 'search_fields' in cls.__dict__: + raise RuntimeError("The indexed_fields attribute has been replaced with search_fields. " \ + "Please update %s.%s to use the search_fields setting." % (cls._meta.app_label, cls.__name__)) - if hasattr(cls, 'search_fields'): - search_fields.extend(cls.search_fields) - - # Backwards compatibility with old indexed_fields setting - - # Get indexed fields - indexed_fields = cls.indexed_get_indexed_fields() - - # Display deprecation warning if indexed_fields has been used - if indexed_fields: - warnings.warn("'indexed_fields' setting is now deprecated." - "Use 'search_fields' instead.", RemovedInWagtail06Warning) - - # Convert them into search fields - for field_name, _config in indexed_fields.items(): - # Copy the config to prevent is trashing anything accidentally - config = _config.copy() - - # Check if this is a filter field - if config.get('index', None) == 'not_analyzed': - config.pop('index') - search_fields.append(FilterField(field_name, es_extra=config)) - continue - - # Must be a search field, check for boosting and partial matching - boost = config.pop('boost', None) - - partial_match = False - if config.get('analyzer', None) == 'edgengram_analyzer': - partial_match = True - config.pop('analyzer') - - # Add the field - search_fields.append(SearchField(field_name, boost=boost, partial_match=partial_match, es_extra=config)) - - # Remove any duplicate entries into search fields - # We need to take into account that fields can be indexed as both a SearchField and as a FilterField - search_fields_dict = {} - for field in search_fields: - search_fields_dict[(field.field_name, type(field))] = field - search_fields = search_fields_dict.values() - - return search_fields + # Return value of 'search_fields' attribute + return getattr(cls, 'search_fields', tuple()) @classmethod def get_searchable_search_fields(cls): diff --git a/wagtail/wagtailsearch/tests/test_indexed_class.py b/wagtail/wagtailsearch/tests/test_indexed_class.py index 983d8e0ba..b89b94a03 100644 --- a/wagtail/wagtailsearch/tests/test_indexed_class.py +++ b/wagtail/wagtailsearch/tests/test_indexed_class.py @@ -17,37 +17,11 @@ class TestContentTypeNames(TestCase): self.assertEqual(name, 'tests_searchtest_tests_searchtestchild') -class TestIndexedFieldsBackwardsCompatibility(TestCase, WagtailTestUtils): - def test_indexed_fields_backwards_compatibility(self): - # Get search fields - with self.ignore_deprecation_warnings(): - search_fields = models.SearchTestOldConfig.get_search_fields() +class TestIndexedFieldsBackwardsIncompatibility(TestCase, WagtailTestUtils): + def test_use_of_indexed_fields_raises_error(self): + # SearchTestOldConfig.get_search_fields should raise a RuntimeError + self.assertRaises(RuntimeError, models.SearchTestOldConfig.get_search_fields) - search_fields_dict = dict( - ((field.field_name, type(field)), field) - for field in search_fields - ) - - # Check that the fields were found - self.assertEqual(len(search_fields_dict), 2) - self.assertIn(('title', indexed.SearchField), search_fields_dict.keys()) - self.assertIn(('live', indexed.FilterField), search_fields_dict.keys()) - - # Check that the title field has the correct settings - self.assertTrue(search_fields_dict[('title', indexed.SearchField)].partial_match) - self.assertEqual(search_fields_dict[('title', indexed.SearchField)].boost, 100) - - def test_indexed_fields_backwards_compatibility_list(self): - # Get search fields - with self.ignore_deprecation_warnings(): - search_fields = models.SearchTestOldConfigList.get_search_fields() - - search_fields_dict = dict( - ((field.field_name, type(field)), field) - for field in search_fields - ) - - # Check that the fields were found - self.assertEqual(len(search_fields_dict), 2) - self.assertIn(('title', indexed.SearchField), search_fields_dict.keys()) - self.assertIn(('content', indexed.SearchField), search_fields_dict.keys()) + def test_use_of_indexed_fields_with_search_fields_doesnt_raise_error(self): + # SearchTestOldConfigAndNewConfig.get_search_fields shouldnt raise an error + search_fields = models.SearchTestOldConfigAndNewConfig.get_search_fields()