Merge branch 'postatum-98861430_id_sorting_broken' into develop

nefertari/elasticsearch.py

@@ -304,7 +304,7 @@ def prep_bulk_documents(self, action, documents):
                 '_op_type': action,
                 '_index': self.index_name,
                 '_type': _doc_type,
-                '_id': doc['id'],
+                '_id': doc['_pk'],
                 '_source': doc,
             }
 
@@ -353,7 +353,7 @@ def index_missing_documents(self, documents, request_params=None):
             index=self.index_name,
             doc_type=self.doc_type,
             fields=['_id'],
-            body={'ids': [d['id'] for d in documents]},
+            body={'ids': [d['_pk'] for d in documents]},
         )
         try:
             response = ES.api.mget(**query_kwargs)
@@ -362,7 +362,7 @@ def index_missing_documents(self, documents, request_params=None):
         else:
             indexed_ids = set(
                 d['_id'] for d in response['docs'] if d.get('found'))
-        documents = [d for d in documents if str(d['id']) not in indexed_ids]
+        documents = [d for d in documents if str(d['_pk']) not in indexed_ids]
 
         if not documents:
             log.info('No documents of type `{}` are missing from '
@@ -375,7 +375,7 @@ def delete(self, ids, request_params=None):
         if not isinstance(ids, list):
             ids = [ids]
 
-        documents = [{'id': _id, '_type': self.doc_type} for _id in ids]
+        documents = [{'_pk': _id, '_type': self.doc_type} for _id in ids]
         self._bulk('delete', documents, request_params=request_params)
 
     def get_by_ids(self, ids, **params):

nefertari/json_httpexceptions.py

@@ -45,7 +45,7 @@ def create_json_response(obj, request=None, log_it=False, show_stack=False,
                 extra['remote_addr'] = request.remote_addr
 
         if obj.location:
-            body['id'] = obj.location.split('/')[-1]
+            body['_pk'] = obj.location.split('/')[-1]
         body.update(extra)
 
     obj.body = six.b(json_dumps(body, encoder=encoder))

nefertari/polymorphic.py

@@ -212,10 +212,10 @@ def _set_object_self(self, obj):
 
         '_self' key is not set for singular resources.
         """
-        type_, obj_id = obj['_type'], obj['id']
+        type_, obj_pk = obj['_type'], obj['_pk']
         resource = self.model_resources[type_]
         obj['_self'] = self.request.route_url(
-            resource.uid, **{resource.id_name: obj_id})
+            resource.uid, **{resource.id_name: obj_pk})
 
     def __call__(self, **kwargs):
         self.model_resources = self.get_models_map()

nefertari/wrappers.py

@@ -142,8 +142,8 @@ def _filter_fields(self, data):
             else:
                 fields &= public_fields
 
-        fields.add('_type')
-        fields.add('_self')
+
+        fields.update(['_type', '_pk', '_self'])
         return data.subset(fields)
 
     def __call__(self, **kwargs):
@@ -245,9 +245,9 @@ def is_singular(self):
     def _set_object_self(self, obj):
         """ Add '_self' key value to :obj: dict. """
         location = self.request.path_url
-        obj_id = urllib.parse.quote(str(obj['id']))
-        if not self.is_singular and not location.endswith(obj_id):
-            location += '/{}'.format(obj_id)
+        obj_pk = urllib.parse.quote(str(obj['_pk']))
+        if not self.is_singular and not location.endswith(obj_pk):
+            location += '/{}'.format(obj_pk)
         obj.setdefault('_self', location)
 
     def __call__(self, **kwargs):
@@ -293,11 +293,11 @@ def __call__(self, **kwargs):
 class add_etag(object):
     """ Add ETAG header to response.
 
-    Etag is generated md5-encoding '_version' + 'id' of each object
+    Etag is generated md5-encoding '_version' + '_pk' of each object
     in a sequence of objects returned.
 
     This wrapper should be applied before `apply_privacy` if later is
-    used or before any wrapper that may remove `_version` and `id` keys
+    used or before any wrapper that may remove `_version` and `_pk` keys
     from output.
     """
     def __init__(self, request):
@@ -309,7 +309,7 @@ def __call__(self, **kwargs):
         etag_src = ''
 
         def etag(data):
-            return str(data.get('_version', '')) + str(data.get('id', ''))
+            return str(data.get('_version', '')) + str(data.get('_pk', ''))
 
         try:
             etag_src += etag(result)

tests/test_elasticsearch.py

@@ -227,15 +227,15 @@ def test_prep_bulk_documents_not_dict(self):
     def test_prep_bulk_documents(self):
         obj = es.ES('Foo', 'foondex')
         docs = [
-            {'_type': 'Story', 'id': 'story1'},
-            {'_type': 'Story', 'id': 'story2'},
+            {'_type': 'Story', '_pk': 'story1'},
+            {'_type': 'Story', '_pk': 'story2'},
         ]
         prepared = obj.prep_bulk_documents('myaction', docs)
         assert len(prepared) == 2
         doc1 = prepared[0]
         assert sorted(doc1.keys()) == sorted([
             '_type', '_id', '_index', '_source', '_op_type'])
-        assert doc1['_source'] == {'_type': 'Story', 'id': 'story1'}
+        assert doc1['_source'] == {'_type': 'Story', '_pk': 'story1'}
         assert doc1['_op_type'] == 'myaction'
         assert doc1['_index'] == 'foondex'
         assert doc1['_type'] == 'story'
@@ -244,14 +244,14 @@ def test_prep_bulk_documents(self):
     def test_prep_bulk_documents_no_type(self):
         obj = es.ES('Foo', 'foondex')
         docs = [
-            {'id': 'story2'},
+            {'_pk': 'story2'},
         ]
         prepared = obj.prep_bulk_documents('myaction', docs)
         assert len(prepared) == 1
         doc2 = prepared[0]
         assert sorted(doc2.keys()) == sorted([
             '_op_type', '_type', '_id', '_index', '_source'])
-        assert doc2['_source'] == {'id': 'story2'}
+        assert doc2['_source'] == {'_pk': 'story2'}
         assert doc2['_op_type'] == 'myaction'
         assert doc2['_index'] == 'foondex'
         assert doc2['_type'] == 'foo'
@@ -310,25 +310,26 @@ def test_delete(self, mock_bulk):
         obj = es.ES('Foo', 'foondex')
         obj.delete(ids=[1, 2])
         mock_bulk.assert_called_once_with(
-            'delete', [{'id': 1, '_type': 'foo'}, {'id': 2, '_type': 'foo'}],
+            'delete', [{'_pk': 1, '_type': 'foo'},
+                       {'_pk': 2, '_type': 'foo'}],
             request_params=None)
 
     @patch('nefertari.elasticsearch.ES._bulk')
     def test_delete_single_obj(self, mock_bulk):
         obj = es.ES('Foo', 'foondex')
         obj.delete(ids=1)
         mock_bulk.assert_called_once_with(
-            'delete', [{'id': 1, '_type': 'foo'}],
+            'delete', [{'_pk': 1, '_type': 'foo'}],
             request_params=None)
 
     @patch('nefertari.elasticsearch.ES._bulk')
     @patch('nefertari.elasticsearch.ES.api.mget')
     def test_index_missing_documents(self, mock_mget, mock_bulk):
         obj = es.ES('Foo', 'foondex')
         documents = [
-            {'id': 1, 'name': 'foo'},
-            {'id': 2, 'name': 'bar'},
-            {'id': 3, 'name': 'baz'},
+            {'_pk': 1, 'name': 'foo'},
+            {'_pk': 2, 'name': 'bar'},
+            {'_pk': 3, 'name': 'baz'},
         ]
         mock_mget.return_value = {'docs': [
             {'_id': '1', 'name': 'foo', 'found': False},
@@ -343,15 +344,16 @@ def test_index_missing_documents(self, mock_mget, mock_bulk):
             body={'ids': [1, 2, 3]}
         )
         mock_bulk.assert_called_once_with(
-            'index', [{'id': 1, 'name': 'foo'}, {'id': 3, 'name': 'baz'}],
-            None)
+            'index', [
+                {'_pk': 1, 'name': 'foo'}, {'_pk': 3, 'name': 'baz'}
+            ], None)
 
     @patch('nefertari.elasticsearch.ES._bulk')
     @patch('nefertari.elasticsearch.ES.api.mget')
     def test_index_missing_documents_no_index(self, mock_mget, mock_bulk):
         obj = es.ES('Foo', 'foondex')
         documents = [
-            {'id': 1, 'name': 'foo'},
+            {'_pk': 1, 'name': 'foo'},
         ]
         mock_mget.side_effect = es.IndexNotFoundException()
         obj.index_missing_documents(documents)
@@ -362,7 +364,7 @@ def test_index_missing_documents_no_index(self, mock_mget, mock_bulk):
             body={'ids': [1]}
         )
         mock_bulk.assert_called_once_with(
-            'index', [{'id': 1, 'name': 'foo'}], None)
+            'index', [{'_pk': 1, 'name': 'foo'}], None)
 
     @patch('nefertari.elasticsearch.ES._bulk')
     @patch('nefertari.elasticsearch.ES.api.mget')
@@ -377,7 +379,7 @@ def test_index_missing_documents_no_docs_passed(self, mock_mget, mock_bulk):
     def test_index_missing_documents_all_docs_found(self, mock_mget, mock_bulk):
         obj = es.ES('Foo', 'foondex')
         documents = [
-            {'id': 1, 'name': 'foo'},
+            {'_pk': 1, 'name': 'foo'},
         ]
         mock_mget.return_value = {'docs': [
             {'_id': '1', 'name': 'foo', 'found': True},

tests/test_json_httpexceptions.py

@@ -38,7 +38,7 @@ def test_create_json_response(self):
         assert isinstance(obj2.body, six.binary_type)
         body = json.loads(obj2.body.decode('utf-8'))
         assert sorted(body.keys()) == [
-            'client_addr', 'explanation', 'id', 'message', 'remote_addr',
+            '_pk', 'client_addr', 'explanation', 'message', 'remote_addr',
             'request_url', 'status_code', 'timestamp', 'title'
         ]
         assert body['remote_addr'] == '127.0.0.2'
@@ -47,7 +47,7 @@ def test_create_json_response(self):
         assert body['explanation'] == 'success'
         assert body['title'] == 'bar'
         assert body['message'] == 'foo'
-        assert body['id'] == 'api'
+        assert body['_pk'] == 'api'
         assert body['request_url'] == 'http://example.com'
 
     @patch.object(jsonex, 'add_stack')
@@ -65,7 +65,7 @@ def test_create_json_response_obj_properties(self, mock_stack):
         assert body['explanation'] == 'success'
         assert body['title'] == 'bar'
         assert body['message'] == 'foo'
-        assert body['id'] == 'api'
+        assert body['_pk'] == 'api'
 
     @patch.object(jsonex, 'add_stack')
     def test_create_json_response_stack_calls(self, mock_stack):

tests/test_polymorphic.py

@@ -192,9 +192,9 @@ def test_set_object_self(self):
         wrapper.request.route_url.return_value = 'foobar'
         resource1 = Mock(uid='mystories', id_name='story_id')
         wrapper.model_resources = {'Story': resource1}
-        obj = {'_type': 'Story', 'id': 4}
+        obj = {'_type': 'Story', '_pk': 4}
         wrapper._set_object_self(obj)
-        assert obj == {'_type': 'Story', 'id': 4, '_self': 'foobar'}
+        assert obj == {'_type': 'Story', '_pk': 4, '_self': 'foobar'}
         wrapper.request.route_url.assert_called_once_with(
             'mystories', story_id=4)
 
@@ -205,10 +205,10 @@ def test_call(self, mock_map):
         wrapper = polymorphic.add_url_polymorphic(None)
         wrapper.request = Mock()
         wrapper.request.route_url.return_value = 'foobar'
-        obj = {'_type': 'Story', 'id': 4}
+        obj = {'_type': 'Story', '_pk': 4}
         assert wrapper(result=obj) == {
-            '_type': 'Story', 'id': 4, '_self': 'foobar'}
+            '_type': 'Story', '_pk': 4, '_self': 'foobar'}
 
-        obj = {'data': [{'_type': 'Story', 'id': 4}]}
+        obj = {'data': [{'_type': 'Story', '_pk': 4}]}
         assert wrapper(result=obj) == {
-            'data': [{'_type': 'Story', 'id': 4, '_self': 'foobar'}]}
+            'data': [{'_type': 'Story', '_pk': 4, '_self': 'foobar'}]}