Fix issue when chaining collection methods

CHANGELOG.rst

@@ -1,6 +1,13 @@
 Changelog
 =========
 
+Next Release (TBD)
+------------------
+
+* bugfix:Collections: Fix regression where filters could not be chained.
+  (`issue 401 <https://github.com/boto/boto3/pull/401>`__)
+
+
 1.2.2 - (2015-11-19)
 --------------------
 

boto3/resources/collection.py

@@ -395,11 +395,13 @@ def load_from_definition(self, resource_name, collection_model,
         self._load_batch_actions(
             attrs, resource_name, collection_model,
             service_context.service_model, event_emitter)
+        # Add the documentation to the collection class's methods
         self._load_documented_collection_methods(
             attrs=attrs, resource_name=resource_name,
             collection_model=collection_model,
             service_model=service_context.service_model,
-            event_emitter=event_emitter)
+            event_emitter=event_emitter,
+            base_class=ResourceCollection)
 
         if service_context.service_name == resource_name:
             cls_name = '{0}.{1}Collection'.format(
@@ -411,12 +413,13 @@ def load_from_definition(self, resource_name, collection_model,
         collection_cls = type(str(cls_name), (ResourceCollection,),
                               attrs)
 
-        # Add the documentation to the collection methods
+        # Add the documentation to the collection manager's methods
         self._load_documented_collection_methods(
             attrs=attrs, resource_name=resource_name,
             collection_model=collection_model,
             service_model=service_context.service_model,
-            event_emitter=event_emitter)
+            event_emitter=event_emitter,
+            base_class=CollectionManager)
         attrs['_collection_cls'] = collection_cls
         cls_name += 'Manager'
 
@@ -434,18 +437,18 @@ def _load_batch_actions(self, attrs, resource_name, collection_model,
                 resource_name, snake_cased, action_model, collection_model,
                 service_model, event_emitter)
 
-    def _load_documented_collection_methods(factory_self, attrs, resource_name,
-                                            collection_model, service_model,
-                                            event_emitter):
-        # The CollectionManger already has these methods defined. However
+    def _load_documented_collection_methods(
+            factory_self, attrs, resource_name, collection_model,
+            service_model, event_emitter, base_class):
+        # The base class already has these methods defined. However
         # the docstrings are generic and not based for a particular service
         # or resource. So we override these methods by proxying to the
-        # CollectionManager's builtin method and adding a docstring
+        # base class's builtin method and adding a docstring
         # that pertains to the resource.
 
         # A collection's all() method.
         def all(self):
-            return CollectionManager.all(self)
+            return base_class.all(self)
 
         all.__doc__ = docstring.CollectionMethodDocstring(
             resource_name=resource_name,
@@ -459,7 +462,7 @@ def all(self):
 
         # The collection's filter() method.
         def filter(self, **kwargs):
-            return CollectionManager.filter(self, **kwargs)
+            return base_class.filter(self, **kwargs)
 
         filter.__doc__ = docstring.CollectionMethodDocstring(
             resource_name=resource_name,
@@ -473,7 +476,7 @@ def filter(self, **kwargs):
 
         # The collection's limit method.
         def limit(self, count):
-            return CollectionManager.limit(self, count)
+            return base_class.limit(self, count)
 
         limit.__doc__ = docstring.CollectionMethodDocstring(
             resource_name=resource_name,
@@ -487,7 +490,7 @@ def limit(self, count):
 
         # The collection's page_size method.
         def page_size(self, count):
-            return CollectionManager.page_size(self, count)
+            return base_class.page_size(self, count)
 
         page_size.__doc__ = docstring.CollectionMethodDocstring(
             resource_name=resource_name,

tests/functional/test_collection.py

@@ -0,0 +1,33 @@
+# Copyright 2015 Amazon.com, Inc. or its affiliates. All Rights Reserved.
+#
+# Licensed under the Apache License, Version 2.0 (the "License"). You
+# may not use this file except in compliance with the License. A copy of
+# the License is located at
+#
+# http://aws.amazon.com/apache2.0/
+#
+# or in the "license" file accompanying this file. This file is
+# distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF
+# ANY KIND, either express or implied. See the License for the specific
+# language governing permissions and limitations under the License.
+from tests import unittest
+
+from boto3.session import Session
+from boto3.resources.collection import ResourceCollection
+
+
+class TestCollection(unittest.TestCase):
+    def setUp(self):
+        self.session = Session(
+            aws_access_key_id='dummy', aws_secret_access_key='dummy',
+            region_name='us-east-1')
+        # Pick an arbitrary resource.
+        self.ec2_resource = self.session.resource('ec2')
+
+    def test_can_use_collection_methods(self):
+        self.assertIsInstance(
+            self.ec2_resource.instances.all(), ResourceCollection)
+
+    def test_can_chain_methods(self):
+        self.assertIsInstance(
+            self.ec2_resource.instances.all().page_size(5), ResourceCollection)

tests/unit/docs/test_docstring.py

@@ -175,6 +175,21 @@ def test_collection_page_size_method_help(self):
             '    :returns: A list of Sample resources',
         ], collection_method_docstring)
 
+    def test_collection_chaining_help(self):
+        collection = self.resource.samples.all()
+        with mock.patch('sys.stdout', six.StringIO()) as mock_stdout:
+            help(collection.all)
+        collection_method_docstring = mock_stdout.getvalue()
+        self.assert_contains_lines_in_order([
+            ('    Creates an iterable of all Sample resources in the '
+             'collection.'),
+            '    **Request Syntax** ',
+            '    ::',
+            '      sample_iterator = myservice.samples.all()',
+            '    :rtype: list(:py:class:`myservice.Sample`)',
+            '    :returns: A list of Sample resources',
+        ], collection_method_docstring)
+
     def test_batch_action_help(self):
         with mock.patch('sys.stdout', six.StringIO()) as mock_stdout:
             help(self.resource.samples.operate)

tests/unit/resources/test_collection.py

@@ -80,8 +80,14 @@ def test_create_subclasses(self):
                         'test.Chain.FrobsCollectionManager')
         self.assertIsInstance(collection, CollectionManager)
 
+        # Make sure that collection manager created from the factory
+        # returns a ResourceCollection.
         self.assertIsInstance(collection.all(), ResourceCollection)
 
+        # Make sure that the collection returned from the collection
+        # manager can be chained and return a ResourceCollection as well.
+        self.assertIsInstance(collection.all().all(), ResourceCollection)
+
     @mock.patch('boto3.resources.collection.BatchAction')
     def test_create_batch_actions(self, action_mock):
         resource_defs = {