Use model serializers when searching mongoengine content units

When breaking a criteria down into its raw mongo queries, use the
model serializers for a given content type to adjust the unit search
spec as-needed for each content type in the criteria.

https://pulp.plan.io/issues/1479
fixes #1479

https://pulp.plan.io/issues/1533
fixes #1533

https://pulp.plan.io/issues/1535
fixes #1535

https://pulp.plan.io/issues/1563
fixes #1563

server/pulp/server/controllers/repository.py

@@ -108,6 +108,21 @@ def get_unit_model_querysets(repo_id, model_class, repo_content_unit_q=None):
         yield model_class.objects(id__in=chunk)
 
 
+def get_repo_unit_type_ids(repo_id):
+    """
+    Retrieve all the content unit type ids associated with a given repository.
+
+    :param repo_id: ID of the repo whose unit models should be retrieved.
+    :type  repo_id: str
+
+    :return: A list of content unit type ids
+    :rtype:  list of str
+    """
+    unit_type_ids = model.RepositoryContentUnit.objects(
+        repo_id=repo_id).distinct('unit_type_id')
+    return unit_type_ids
+
+
 def get_repo_unit_models(repo_id):
     """
     Retrieve all the MongoEngine models for units in a given repository. If a unit
@@ -120,8 +135,7 @@ def get_repo_unit_models(repo_id):
     :return: A list of sub-classes of ContentUnit that define a unit model.
     :rtype:  list of pulp.server.db.model.ContentUnit
     """
-    unit_types = model.RepositoryContentUnit.objects(
-        repo_id=repo_id).distinct('unit_type_id')
+    unit_types = get_repo_unit_type_ids(repo_id)
     unit_models = [plugin_api.get_unit_model_by_id(type_id) for type_id in unit_types]
     # Filter any non-MongoEngine content types.
     return filter(None, unit_models)

server/pulp/server/controllers/units.py

@@ -66,3 +66,32 @@ def get_unit_key_fields_for_type(type_id):
         return tuple(type_def['unit_key'])
 
     raise ValueError
+
+
+def get_model_serializer_for_type(type_id):
+    """
+    Get a ModelSerializer instance associated with a given unit type id
+
+    Serializers are only needed with mongoengine models.
+
+    This will return None for pymongo models or mongoengine models that do not have a serializer.
+
+    :param type_id: unique ID for a unit type
+    :type  type_id: str
+
+    :return:    model serializer instance, if available
+    :rtype:     pulp.server.webservices.views.serializers.ModelSerializer or None
+
+    :raises ValueError: if the type ID is not found
+    """
+    model_class = plugin_api.get_unit_model_by_id(type_id)
+    # mongoengine models have a SERIALIZER attr, which is exposed via the serializer
+    # property as an instance with the associated model
+    if model_class is not None and hasattr(model_class, 'SERIALIZER'):
+        serializer = model_class.SERIALIZER
+        # model serializer methods currently take the model class as an arg
+        # so stash the model class on the serializer for now, and this all
+        # gets made better with https://pulp.plan.io/issues/1555
+        serializer.model = model_class
+        # instantiate the serializer before returning
+        return serializer()

server/pulp/server/managers/repo/unit_association.py

@@ -154,7 +154,26 @@ def _units_from_criteria(source_repo, criteria):
         association_q = mongoengine.Q(__raw__=criteria.association_spec)
         if criteria.type_ids:
             association_q &= mongoengine.Q(unit_type_id__in=criteria.type_ids)
+            unit_type_ids = criteria.type_ids
+        else:
+            # don't need to limit the association_q by content type here
+            # since filtering for every content_type seen in the repo is
+            # achieved by limiting the query to the repo
+            unit_type_ids = repo_controller.get_repo_unit_type_ids(source_repo.repo_id)
+
+        # base unit_q, works as-is for non-mongoengine unit types
         unit_q = mongoengine.Q(__raw__=criteria.unit_spec)
+
+        # for mongoengine unit types, use the unit model's serializer to translate the unit spec
+        # for each possible content unit type as determined by the search criteria
+        for unit_type_id in unit_type_ids:
+            serializer = units_controller.get_model_serializer_for_type(unit_type_id)
+            if serializer:
+                unit_spec_t = serializer.translate_filters(serializer.model, criteria.unit_spec)
+                # after translating the fields, this spec is only good for this unit type
+                unit_spec_t['_content_type_id'] = unit_type_id
+                unit_q |= mongoengine.Q(__raw__=unit_spec_t)
+
         return repo_controller.find_repo_content_units(
             repository=source_repo,
             repo_content_unit_q=association_q,

server/pulp/server/managers/repo/unit_association_query.py

@@ -5,8 +5,8 @@
 
 import pymongo
 
-from pulp.plugins.loader.api import get_unit_model_by_id
 from pulp.plugins.types import database as types_db
+from pulp.server.controllers import units
 from pulp.server.db.model.criteria import UnitAssociationCriteria
 from pulp.server.db.model.repository import RepoContentUnit
 
@@ -350,17 +350,12 @@ def _associated_units_by_type_cursor(unit_type_id, criteria, associated_unit_ids
         :type associated_unit_ids: list
         :rtype: pymongo.cursor.Cursor
         """
-        model = get_unit_model_by_id(unit_type_id)
-        if model and hasattr(model, 'SERIALIZER'):
-            serializer = model.SERIALIZER()
-        else:
-            serializer = None
-
         collection = types_db.type_units_collection(unit_type_id)
+        serializer = units.get_model_serializer_for_type(unit_type_id)
 
         spec = criteria.unit_filters.copy()
         if spec and serializer:
-                spec = serializer.translate_filters(model, spec)
+                spec = serializer.translate_filters(serializer.model, spec)
 
         spec['_id'] = {'$in': associated_unit_ids}
 
@@ -374,7 +369,7 @@ def _associated_units_by_type_cursor(unit_type_id, criteria, associated_unit_ids
             # translate incoming fields (e.g. id -> foo_id)
             if serializer:
                 for index, field in enumerate(fields):
-                    fields[index] = serializer.translate_field(model, field)
+                    fields[index] = serializer.translate_field(serializer.model, field)
 
         cursor = collection.find(spec, fields=fields)
 
@@ -385,7 +380,7 @@ def _associated_units_by_type_cursor(unit_type_id, criteria, associated_unit_ids
         elif serializer:
             sort = list(sort)
             for index, (field, direction) in enumerate(sort):
-                sort[index] = (serializer.translate_field(model, field), direction)
+                sort[index] = (serializer.translate_field(serializer.model, field), direction)
 
         cursor.sort(sort)
 

server/pulp/server/webservices/views/content.py

@@ -12,7 +12,7 @@
 from pulp.server.auth import authorization
 from pulp.server.content.sources.container import ContentContainer
 from pulp.server.controllers import content
-from pulp.server.controllers import units
+from pulp.server.controllers import units as units_controller
 from pulp.server.db.model.criteria import Criteria
 from pulp.server.exceptions import InvalidValue, MissingResource, OperationPostponed
 from pulp.server.managers import factory
@@ -133,7 +133,7 @@ def delete(self, request, content_type):
         """
         try:
             # this tests if the type exists
-            units.get_unit_key_fields_for_type(content_type)
+            units_controller.get_unit_key_fields_for_type(content_type)
         except ValueError:
             raise MissingResource(content_type_id=content_type)
 
@@ -285,7 +285,12 @@ def get_results(cls, query, search_method, options, *args, **kwargs):
         """
         Overrides the base class so additional information can optionally be added.
         """
+
         type_id = kwargs['type_id']
+        serializer = units_controller.get_model_serializer_for_type(type_id)
+        if serializer:
+            # if we have a model serializer, translate the filter for this content unit type
+            query['filters'] = serializer.translate_filters(serializer.model, query['filters'])
         units = list(search_method(type_id, query))
         units = [_process_content_unit(unit, type_id) for unit in units]
         if options.get('include_repos') is True:

server/pulp/server/webservices/views/serializers/__init__.py

@@ -171,11 +171,7 @@ def to_representation(self, instance):
         """
         document_dict = {}
         for field in instance._fields:
-            if field in self._remapped_fields:
-                document_dict[self._remapped_fields[field]] = getattr(instance, field)
-            else:
-                document_dict[field] = getattr(instance, field)
-
+            document_dict[self.translate_field_reverse(field)] = getattr(instance, field)
         return document_dict
 
     def _translate_filters(self, model, filters):
@@ -251,6 +247,20 @@ def _translate(self, model, field):
         else:
             return getattr(model, field).db_field
 
+    def translate_field_reverse(self, field):
+        """
+        Converts an internal db field name to the external representation of a field
+
+        :param field: field name (internal name)
+        :type  field: basestring
+
+        :return: the remapped field name to use in external representations
+        :rtype:  basestring
+        """
+        # If the field name is in the remapped_fields dict, return its value
+        # Otherwise, return the field name as-is
+        return self._remapped_fields.get(field, field)
+
     def translate_criteria(self, model, crit):
         """
         To preserve backwards compatability of our search API, we must translate the fields from

server/pulp/server/webservices/views/serializers/content.py

@@ -4,7 +4,7 @@
 import logging
 
 from pulp.common import dateutils
-from pulp.plugins.loader import api
+from pulp.server.controllers import units
 from pulp.server.webservices import http
 from pulp.server.webservices.views.serializers import db
 
@@ -17,9 +17,6 @@ def remap_fields_with_serializer(content_unit):
 
     :param content_unit: Content unit to modify
     :type content_unit: dict
-    :param remove_remapped: If True, remove remapped keys from content unit when remapping.
-                            Remapped keys are left in place and copied by default
-    :type remove_remapped: bool
 
     This is a small workaround to help in cases where REST views are returning the older objects
     coming out of pymongo, but still need to have their fields remapped according to the rules of
@@ -39,17 +36,21 @@ def remap_fields_with_serializer(content_unit):
                       '{0!r}'.format(content_unit))
         return
 
-    cu_document = api.get_unit_model_by_id(content_type_id)
+    serializer = units.get_model_serializer_for_type(content_type_id)
+    if serializer is None:
+        # No serializer, nothing to do
+        return
+
+    # build the list of fields that need to be remapped
+    field_map = {}
+    for field in content_unit:
+        remapped_field = serializer.translate_field_reverse(field)
+        if remapped_field != field:
+            field_map[field] = remapped_field
 
-    if hasattr(cu_document, 'SERIALIZER'):
-        for original_field, remapped_field in cu_document.SERIALIZER()._remapped_fields.items():
-            try:
-                content_unit[remapped_field] = content_unit.pop(original_field)
-            except KeyError:
-                # If the original field doesn't exist, log and move on
-                _logger.debug('original field not found when attempting to remap: {0}'
-                              '{0}'.format(original_field))
-                continue
+    # do the remapping
+    for original_field, remapped_field in field_map.items():
+        content_unit[remapped_field] = content_unit.pop(original_field)
 
 
 def content_unit_obj(content_unit):

server/test/unit/server/controllers/test_units.py

@@ -14,6 +14,18 @@ class DemoModel(model.ContentUnit):
     save = MagicMock()
 
 
+class DemoModelSerializer(object):
+    pass
+
+
+class DemoModelWithSerializer(model.ContentUnit):
+    key_field = mongoengine.StringField()
+    unit_key_fields = ('key_field',)
+    unit_type_id = 'demo_model'
+    objects = MagicMock()
+    SERIALIZER = DemoModelSerializer
+
+
 class FindUnitsTests(unittest.TestCase):
 
     @patch('pulp.server.controllers.units.misc.paginate')
@@ -92,3 +104,25 @@ def test_not_found(self, mock_type_def, mock_get_model):
         mock_type_def.return_value = None
 
         self.assertRaises(ValueError, units_controller.get_unit_key_fields_for_type, 'faketype')
+
+
+@patch('pulp.plugins.loader.api.get_unit_model_by_id', spec_set=True)
+class TestGetModelSerializerForType(unittest.TestCase):
+    def test_returns_serializer(self, mock_get_model):
+        mock_get_model.return_value = DemoModelWithSerializer
+
+        serializer = units_controller.get_model_serializer_for_type('demo_model')
+        self.assertTrue(isinstance(serializer, DemoModelSerializer))
+        self.assertTrue(hasattr(serializer, 'model'))
+
+    def test_returns_none_if_no_serializer(self, mock_get_model):
+        mock_get_model.return_value = DemoModel
+
+        serializer = units_controller.get_model_serializer_for_type('demo_model')
+        self.assertTrue(serializer is None)
+
+    def test_returns_none_if_no_model(self, mock_get_model):
+        mock_get_model.return_value = None
+
+        serializer = units_controller.get_model_serializer_for_type('demo_model')
+        self.assertTrue(serializer is None)

server/test/unit/server/webservices/views/serializers/test_content.py

@@ -41,6 +41,7 @@ class ContentUnitHelper(ContentUnit):
             _ns = StringField(default='dummy_content_name')
             _content_type_id = StringField(required=True, default='content_type')
             unit_key_fields = ()
+            type_specific_id = StringField()
             SERIALIZER = ContentUnitHelperSerializer
         self.content_unit_model = ContentUnitHelper
 
@@ -54,5 +55,7 @@ def setUp(self):
     def test_remap_fields(self, mock_get_model):
         mock_get_model.return_value = self.content_unit_model
         content.remap_fields_with_serializer(self.content_unit)
+        self.assertTrue('type_specific_id' not in self.content_unit,
+                        'type-specific ID field not remapped')
+        self.assertTrue('id' in self.content_unit)
         self.assertEqual(self.content_unit['id'], 'foo')
-        self.assertTrue('type_specific_id' not in self.content_unit)

server/test/unit/server/webservices/views/serializers/test_serializers.py

@@ -411,6 +411,22 @@ class Meta:
         result = test_serializer._translate(mock_model, 'external')
         self.assertEqual(result, 'internal_db')
 
+    def test_translate_field_reverse(self):
+        """
+        Test that individual strings are translated correctly from external to internal repr.
+        """
+
+        class FakeSerializer(serializers.ModelSerializer):
+
+            class Meta:
+                remapped_fields = {'internal': 'external'}
+
+        mock_model = mock.MagicMock()
+        mock_model.internal.db_field = 'internal_db'
+        test_serializer = FakeSerializer()
+        result = test_serializer.translate_field_reverse('internal')
+        self.assertEqual(result, 'external')
+
     @mock.patch('pulp.server.db.model.criteria.Criteria.from_dict')
     @mock.patch('pulp.server.webservices.views.serializers.ModelSerializer._translate')
     @mock.patch('pulp.server.webservices.views.serializers.ModelSerializer._translate_filters')