From f7ed239fcb47663d881e72ffab4386244b215ce4 Mon Sep 17 00:00:00 2001
From: Jakob de Maeyer <jakobdm1@gmail.com>
Date: Fri, 6 Nov 2015 22:27:01 +0100
Subject: [PATCH] Replace INSTALLED_ADDONS tuple setting with ADDONS dictionary
 setting

---
 scrapy/addons/__init__.py           | 15 ++++++++-------
 scrapy/settings/default_settings.py |  4 ++--
 tests/test_addons/__init__.py       | 26 ++++++++++++++++++++++----
 3 files changed, 32 insertions(+), 13 deletions(-)

diff --git a/scrapy/addons/__init__.py b/scrapy/addons/__init__.py
index b1d6e14cb..a1a9a388a 100644
--- a/scrapy/addons/__init__.py
+++ b/scrapy/addons/__init__.py
@@ -1,4 +1,4 @@
-from collections import defaultdict, Mapping
+from collections import defaultdict, Mapping, OrderedDict
 from inspect import isclass
 import six
 import warnings
@@ -9,7 +9,8 @@ from zope.interface.verify import verifyObject
 
 from scrapy.exceptions import NotConfigured
 from scrapy.interfaces import IAddon
-from scrapy.utils.conf import config_from_filepath, get_config
+from scrapy.utils.conf import (build_component_list, config_from_filepath,
+                               get_config)
 from scrapy.utils.misc import load_module_or_object
 from scrapy.utils.project import get_project_path
 
@@ -166,7 +167,7 @@ class AddonManager(Mapping):
     """
 
     def __init__(self):
-        self._addons = {}
+        self._addons = OrderedDict()
         self.configs = {}
         self._disable_on_add = []
 
@@ -310,15 +311,15 @@ class AddonManager(Mapping):
         """Load add-ons and configurations from settings object.
 
         This will invoke :meth:`get_addon` for every add-on path in the
-        ``INSTALLED_ADDONS`` setting. For each of these add-ons, the
-        configuration will be read from the dictionary setting whose name
-        matches the uppercase add-on name.
+        ``ADDONS`` setting. For each of these add-ons, the configuration will be
+        read from the dictionary setting whose name matches the uppercase add-on
+        name.
 
         :param settings: The :class:`~scrapy.settings.Settings` object from \
             which to read the add-on configuration
         :type settings: :class:`~scrapy.settings.Settings`
         """
-        paths = settings.getlist('INSTALLED_ADDONS')
+        paths = build_component_list(settings['ADDONS'])
         addons = [self.get_addon(path) for path in paths]
         configs = [settings.getdict(addon.name.upper()) for addon in addons]
         for a, c in zip(addons, configs):
diff --git a/scrapy/settings/default_settings.py b/scrapy/settings/default_settings.py
index a230750fb..6068a1393 100644
--- a/scrapy/settings/default_settings.py
+++ b/scrapy/settings/default_settings.py
@@ -18,6 +18,8 @@ import sys
 from importlib import import_module
 from os.path import join, abspath, dirname
 
+ADDONS = {}
+
 AJAXCRAWL_ENABLED = False
 
 AUTOTHROTTLE_ENABLED = False
@@ -167,8 +169,6 @@ HTTPCACHE_DBM_MODULE = 'anydbm'
 HTTPCACHE_POLICY = 'scrapy.extensions.httpcache.DummyPolicy'
 HTTPCACHE_GZIP = False
 
-INSTALLED_ADDONS = ()
-
 ITEM_PROCESSOR = 'scrapy.pipelines.ItemPipelineManager'
 
 ITEM_PIPELINES = {}
diff --git a/tests/test_addons/__init__.py b/tests/test_addons/__init__.py
index ab06023e3..aa8dfbb63 100644
--- a/tests/test_addons/__init__.py
+++ b/tests/test_addons/__init__.py
@@ -1,3 +1,5 @@
+from collections import OrderedDict
+import itertools
 import os.path
 import six
 from tests import mock
@@ -236,14 +238,30 @@ class AddonManagerTest(unittest.TestCase):
         _test_load_method('load_dict', addonsdict)
 
         settings = BaseSettings()
-        settings.set('INSTALLED_ADDONS', [
-                self.ADDONMODPATH,
-                'tests.test_addons.addons.GoodAddon',
-                ])
+        settings.set('ADDONS', {self.ADDONMODPATH: 0,
+                                'tests.test_addons.addons.GoodAddon': 0})
         settings.set('ADDONMODULE', {'key': 'val1'})
         settings.set('GOODADDON', {'key': 'val2'})
         _test_load_method('load_settings', settings)
 
+    def test_load_dict_load_settings_order(self):
+        def _test_load_method(expected_order, func, *args, **kwargs):
+            manager = AddonManager()
+            getattr(manager, func)(*args, **kwargs)
+            self.assertEqual(list(manager.keys()), expected_order)
+
+        # Get three addons named 0, 1, 2
+        addonlist = [addons.GoodAddon(str(x)) for x in range(3)]
+        # Test both methods for every possible mutation
+        for ordered_addons in itertools.permutations(addonlist):
+            expected_order = [a.name for a in ordered_addons]
+            addonsdict = OrderedDict((a, {}) for a in ordered_addons)
+            _test_load_method(expected_order, 'load_dict', addonsdict)
+            settings = BaseSettings({
+                'ADDONS': {a: i for i, a in enumerate(ordered_addons)}
+            })
+            _test_load_method(expected_order, 'load_settings', settings)
+
     def test_load_cfg(self):
         def _check_loaded_addons(manager):
             six.assertCountEqual(self, manager, ['GoodAddon', 'AddonModule'])