Improve subscription listing

The way subscriptions are listed by Client.subscriptions method is not
very good. Besides doing nice work, this method modifies subscription
list response body from Zaqar by adding redundant 'queue_name' key to
each subscription dictionary in the response body, while there is
already 'source' key containing queue name.

Because of that it's possible to write a unit test, where queue list
response body from Zaqar does not represent real response that could
come from Zaqar. As example see
zaqarclient.tests.queues.subscriptions.QueuesV2SubscriptionUnitTest.
test_subscription_list. This test is missing 'source' keys in response
body.

This patch makes subscription listing code stop modifying responses from
Zaqar. As expected, the example unit test fails after that. This patch
fixes the test also and checks more subscription object's fields.

Also this patch improves subscription listing functional test to check
all fields of subscription object that was deserialized from Zaqar
response, i.e. checks zaqarclient.queues.v2.subscription.create_object
method. It is also needed, because before this patch, subscription's
queue name field wasn't checked by any of the tests, making a recent bug
like https://bugs.launchpad.net/python-zaqarclient/+bug/1538367
possible.

Change-Id: Iae5bffb296efd655a34584c7f2162adbe6d029e2
Closes-Bug: 1538366

zaqarclient/queues/v2/client.py

@@ -76,8 +76,6 @@ class Client(client.Client):
 
         subscription_list = core.subscription_list(trans, req, queue_name,
                                                    **params)
-        for s in subscription_list['subscriptions']:
-            s['queue_name'] = queue_name
 
         return iterator._Iterator(self,
                                   subscription_list,

zaqarclient/queues/v2/subscription.py

@@ -76,7 +76,7 @@ class Subscription(object):
 
 
 def create_object(parent):
-    return lambda kwargs: Subscription(parent, kwargs.pop("queue_name"),
+    return lambda kwargs: Subscription(parent, kwargs.pop('source'),
                                        subscriber=kwargs.pop('subscriber'),
                                        ttl=kwargs.pop('ttl'),
                                        id=kwargs.pop('id'),

zaqarclient/tests/queues/subscriptions.py

@@ -121,14 +121,16 @@ class QueuesV2SubscriptionUnitTest(base.QueuesTestBase):
 
     def test_subscription_list(self):
         subscription_data = {'subscriptions':
-                             [{'id': '568afabb508f153573f6a56f',
+                             [{'source': 'beijing',
+                               'id': '568afabb508f153573f6a56f',
                                'subscriber': 'http://trigger.me',
                                'ttl': 3600,
                                'options': {}},
-                              {'id': '568afabb508f153573f6a56x',
+                              {'source': 'beijing',
+                               'id': '568afabb508f153573f6a56x',
                                'subscriber': 'http://trigger.you',
                                'ttl': 7200,
-                               'options': {}}]}
+                               'options': {'oh stop': 'triggering'}}]}
 
         with mock.patch.object(self.transport, 'send',
                                autospec=True) as send_method:
@@ -140,8 +142,20 @@ class QueuesV2SubscriptionUnitTest(base.QueuesTestBase):
             # NOTE(flwang): This will call
             # ensure exists in the client instance
             # since auto_create's default is True
-            subscriptions = self.client.subscriptions('beijing')
-            self.assertEqual(2, len(list(subscriptions)))
+            subscriptions = list(self.client.subscriptions('beijing'))
+            self.assertEqual(2, len(subscriptions))
+
+            subscriber_list = [s.subscriber for s in subscriptions]
+            self.assertIn('http://trigger.me', subscriber_list)
+            self.assertIn('http://trigger.you', subscriber_list)
+
+            # Let's pick one of the subscriptions and check all of its fields
+            for sub in subscriptions:
+                if sub.subscriber == 'http://trigger.you':
+                    self.assertEqual('beijing', sub.queue_name)
+                    self.assertEqual('568afabb508f153573f6a56x', sub.id)
+                    self.assertEqual(7200, sub.ttl)
+                    self.assertEqual({'oh stop': 'triggering'}, sub.options)
 
 
 class QueuesV2SubscriptionFunctionalTest(base.QueuesTestBase):
@@ -156,7 +170,8 @@ class QueuesV2SubscriptionFunctionalTest(base.QueuesTestBase):
         self.addCleanup(queue.delete)
 
         subscription_data_1 = {'subscriber': 'http://trigger.me', 'ttl': 3600}
-        subscription_data_2 = {'subscriber': 'http://trigger.he', 'ttl': 7200}
+        subscription_data_2 = {'subscriber': 'http://trigger.he', 'ttl':
+                               7200, 'options': {'check everything': True}}
         self.subscription_1 = self.client.subscription(self.queue_name,
                                                        **subscription_data_1)
         self.addCleanup(self.subscription_1.delete)
@@ -202,3 +217,11 @@ class QueuesV2SubscriptionFunctionalTest(base.QueuesTestBase):
         subscriber_list = [s.subscriber for s in subscriptions]
         self.assertIn('http://trigger.me', subscriber_list)
         self.assertIn('http://trigger.he', subscriber_list)
+
+        # Let's pick one of the subscriptions and check all of its fields
+        for sub in subscriptions:
+            if sub.subscriber == 'http://trigger.he':
+                self.assertEqual('beijing', sub.queue_name)
+                self.assertEqual(self.subscription_2.id, sub.id)
+                self.assertEqual(7200, sub.ttl)
+                self.assertEqual({'check everything': True}, sub.options)