Refactored the RecommendationManager to accept a factory to generate

recommender instances.

taar/recommenders/__init__.py

@@ -2,7 +2,7 @@ from .collaborative_recommender import CollaborativeRecommender
 from .locale_recommender import LocaleRecommender
 from .legacy_recommender import LegacyRecommender
 from .similarity_recommender import SimilarityRecommender
-from .recommendation_manager import RecommendationManager
+from .recommendation_manager import RecommendationManager, RecommenderFactory
 
 
 __all__ = [
@@ -11,4 +11,5 @@ __all__ = [
     'LocaleRecommender',
     'SimilarityRecommender',
     'RecommendationManager',
+    'RecommenderFactory',
 ]

taar/recommenders/recommendation_manager.py

@@ -4,43 +4,53 @@ from .legacy_recommender import LegacyRecommender
 from .locale_recommender import LocaleRecommender
 from .similarity_recommender import SimilarityRecommender
 from .ensemble_recommender import EnsembleRecommender
-from ..profile_fetcher import ProfileFetcher
 
 
 logger = logging.getLogger(__name__)
 
 
-class RecommendationManager(object):
+class RecommenderFactory:
+    """
+    A RecommenderFactory provides support to create recommenders.
+
+    The existence of a factory enables injection of dependencies into
+    the RecommendationManager and eases the implementation of test
+    harnesses.
+    """
+    def __init__(self):
+        self._recommender_factory_map = {'legacy': LegacyRecommender,
+                                         'collaborative': CollaborativeRecommender,
+                                         'similarity': SimilarityRecommender,
+                                         'locale': LocaleRecommender}
+
+    def get_names(self):
+        return self._recommender_factory_map.keys()
+
+    def create(self, recommender_name):
+        return self._recommender_factory_map[recommender_name]()
+
+
+class RecommendationManager:
     """This class determines which of the set of recommendation
     engines will actually be used to generate recommendations."""
 
-    def __init__(self, profile_fetcher=None, recommenders=None):
+    LINEAR_RECOMMENDER_ORDER = ['legacy', 'collaborative', 'similarity', 'locale']
+
+    def __init__(self, recommender_factory, profile_fetcher):
         """Initialize the user profile fetcher and the recommenders.
         """
-        if profile_fetcher is None:
-            logger.info("Initializing profile_fetcher")
-            self.profile_fetcher = ProfileFetcher()
-        else:
         self.profile_fetcher = profile_fetcher
+        self.linear_recommenders = []
+        self._recommender_map = {}
 
-        if recommenders:
-            # This branch of code only runs under test.  We need to
-            # fix the recommender_map so that it only initializes when
-            # run in production
-            self.linear_recommenders = recommenders
-        else:
-            self._recommender_map = {'legacy': LegacyRecommender(),
-                                     'collaborative': CollaborativeRecommender(),
-                                     'similarity': SimilarityRecommender(),
-                                     'locale': LocaleRecommender()}
-
-            self._recommender_map['ensemble'] = EnsembleRecommender(self._recommenders)
         logger.info("Initializing recommenders")
+        for rkey in self.LINEAR_RECOMMENDER_ORDER:
+            recommender = recommender_factory.create(rkey)
 
-            self.linear_recommenders = (self._recommender_map['legacy'],
-                                        self._recommender_map['collaborative'],
-                                        self._recommender_map['similarity'],
-                                        self._recommender_map['locale'])
+            self.linear_recommenders.append(recommender)
+            self._recommender_map[rkey] = recommender
+
+        self._recommender_map['ensemble'] = EnsembleRecommender(self.linear_recommenders)
 
     def recommend(self, client_id, limit, extra_data={}):
         """Return recommendations for the given client.

tests/mocks.py

@@ -0,0 +1,54 @@
+# This Source Code Form is subject to the terms of the Mozilla Public
+# License, v. 2.0. If a copy of the MPL was not distributed with this
+# file, You can obtain one at http://mozilla.org/MPL/2.0/.
+
+
+class MockRecommender:
+    """The MockRecommender takes in a map of GUID->weight."""
+
+    def __init__(self, guid_map):
+        self._guid_map = guid_map
+
+    def can_recommend(self, *args, **kwargs):
+        return True
+
+    def recommend(self, *args, **kwargs):
+        return sorted(self._guid_map.items(), key=lambda item: -item[1])
+
+
+class MockRecommenderFactory:
+    """
+    A RecommenderFactory provides support to create recommenders.
+
+    The existence of a factory enables injection of dependencies into
+    the RecommendationManager and eases the implementation of test
+    harnesses.
+    """
+    def __init__(self, **kwargs):
+        mock_legacy = MockRecommender({'abc': 1.0, 'bcd': 1.1, 'cde': 1.2})
+        mock_locale = MockRecommender({'def': 2.0, 'efg': 2.1, 'fgh': 2.2, 'abc': 2.3})
+        mock_collaborative = MockRecommender({'ghi': 3.0, 'hij': 3.1, 'ijk': 3.2, 'def': 3.3})
+        mock_similarity = MockRecommender({'jkl': 4.0,  'klm': 4.1, 'lmn': 4.2, 'ghi': 4.3})
+
+        self._recommender_factory_map = {'legacy': lambda: mock_legacy,
+                                         'collaborative': lambda: mock_collaborative,
+                                         'similarity': lambda: mock_similarity,
+                                         'locale': lambda: mock_locale}
+
+        # Clobber any kwarg passed in recommenders
+        for key in self._recommender_factory_map.keys():
+            self._recommender_factory_map[key] = kwargs.get(key, self._recommender_factory_map[key])
+
+    def get_names(self):
+        return self._recommender_factory_map.keys()
+
+    def create(self, recommender_name):
+        return self._recommender_factory_map[recommender_name]()
+
+
+class MockProfileController:
+    def __init__(self, mock_profile):
+        self._profile = mock_profile
+
+    def get_client_profile(self, client_id):
+        return self._profile

tests/test_ensemblerecommender.py

@@ -1,3 +1,7 @@
+# This Source Code Form is subject to the terms of the Mozilla Public
+# License, v. 2.0. If a copy of the MPL was not distributed with this
+# file, You can obtain one at http://mozilla.org/MPL/2.0/.
+
 import boto3
 import json
 import pytest
@@ -5,19 +9,7 @@ from moto import mock_s3
 from taar.recommenders.ensemble_recommender import S3_BUCKET
 from taar.recommenders.ensemble_recommender import ENSEMBLE_WEIGHTS
 from taar.recommenders.ensemble_recommender import EnsembleRecommender
-
-
-class MockRecommender:
-    """The MockRecommender takes in a map of GUID->weight."""
-
-    def __init__(self, guid_map):
-        self._guid_map = guid_map
-
-    def can_recommend(self, *args, **kwargs):
-        return True
-
-    def recommend(self, *args, **kwargs):
-        return sorted(self._guid_map.items(), key=lambda item: -item[1])
+from .mocks import MockRecommenderFactory
 
 
 @pytest.fixture
@@ -45,15 +37,13 @@ def test_recommendations(mock_s3_ensemble_weights):
                         ('lmn', 420.0),
                         ('klm', 409.99999999999994),
                         ('jkl', 400.0)]
-    mock_legacy = MockRecommender({'abc': 1.0, 'bcd': 1.1, 'cde': 1.2})
-    mock_locale = MockRecommender({'def': 2.0, 'efg': 2.1, 'fgh': 2.2, 'abc': 2.3})
-    mock_collaborative = MockRecommender({'ghi': 3.0, 'hij': 3.1, 'ijk': 3.2, 'def': 3.3})
-    mock_similarity = MockRecommender({'jkl': 4.0,  'klm': 4.1, 'lmn': 4.2, 'ghi': 4.3})
-    mock_recommenders = {'legacy': mock_legacy,
-                         'collaborative': mock_collaborative,
-                         'similarity': mock_similarity,
-                         'locale': mock_locale}
-    r = EnsembleRecommender(mock_recommenders)
+
+    factory = MockRecommenderFactory()
+    mock_recommender_map = {'legacy': factory.create('legacy'),
+                            'collaborative': factory.create('collaborative'),
+                            'similarity': factory.create('similarity'),
+                            'locale': factory.create('locale')}
+    r = EnsembleRecommender(mock_recommender_map)
     client = {}  # Anything will work here
     recommendation_list = r.recommend(client, 10)
     assert isinstance(recommendation_list, list)

tests/test_recommendation_manager.py

@@ -2,14 +2,7 @@ from taar.profile_fetcher import ProfileFetcher
 from taar.recommenders import RecommendationManager
 from taar.recommenders.base_recommender import BaseRecommender
 from .test_similarityrecommender import mock_s3_categorical_data   # noqa
-
-
-class MockProfileController:
-    def __init__(self, mock_profile):
-        self._profile = mock_profile
-
-    def get_client_profile(self, client_id):
-        return self._profile
+from .mocks import MockProfileController, MockRecommenderFactory
 
 
 class StubRecommender(BaseRecommender):
@@ -28,7 +21,8 @@ class StubRecommender(BaseRecommender):
 
 def test_none_profile_returns_empty_list():
     fetcher = ProfileFetcher(MockProfileController(None))
-    rec_manager = RecommendationManager(fetcher, ("fake-recommender", ))
+    factory = MockRecommenderFactory()
+    rec_manager = RecommendationManager(factory, fetcher)
     assert rec_manager.recommend("random-client-id", 10) == []
 
 
@@ -46,15 +40,13 @@ def test_recommendation_strategy():
 
     # Configure the recommender so that only the second model
     # can recommend and return the expected addons.
-    recommenders = (
-        StubRecommender(False, []),
-        StubRecommender(True, EXPECTED_ADDONS),
-        StubRecommender(False, []),
-    )
+    factory = MockRecommenderFactory(legacy=lambda: StubRecommender(False, []),
+                                     collaborative=lambda: StubRecommender(True, EXPECTED_ADDONS),
+                                     similarity=lambda: StubRecommender(False, []),
+                                     locale=lambda: StubRecommender(False, []))
 
     # Make sure the recommender returns the expected addons.
-    manager = RecommendationManager(StubFetcher(),
-                                    recommenders)
+    manager = RecommendationManager(factory, StubFetcher())
     results = manager.recommend("client-id",
                                 10,
                                 extra_data={'branch': 'linear'})