From 0fd24771d64a4feeaf346f1a500d9ae725b0a3dd Mon Sep 17 00:00:00 2001
From: Mehdi Abaakouk <mehdi.abaakouk@enovance.com>
Date: Tue, 17 Jun 2014 10:25:25 +0200
Subject: [PATCH] Fix messaging.get_transport caching

If some case get_transport can return a cached transport
even cache=False

This change fixes it and also adds some unit tests for this method

Change-Id: If87cf60d0a25a84dd05f1b84c35f1bcbfeb69b75
---
 ceilometer/messaging.py            |  9 ++--
 ceilometer/tests/test_messaging.py | 67 ++++++++++++++++++++++++++++++
 2 files changed, 71 insertions(+), 5 deletions(-)
 create mode 100644 ceilometer/tests/test_messaging.py

diff --git a/ceilometer/messaging.py b/ceilometer/messaging.py
index fda4320cb..14d842ef0 100644
--- a/ceilometer/messaging.py
+++ b/ceilometer/messaging.py
@@ -67,8 +67,9 @@ def setup():
 def get_transport(url=None, optional=False, cache=True):
     """Initialise the oslo.messaging layer."""
     global TRANSPORTS, DEFAULT_URL
-    transport = TRANSPORTS.get(url)
-    if not transport:
+    cache_key = url or DEFAULT_URL
+    transport = TRANSPORTS.get(cache_key)
+    if not transport or not cache:
         try:
             transport = oslo.messaging.get_transport(cfg.CONF, url,
                                                      aliases=_ALIASES)
@@ -79,10 +80,8 @@ def get_transport(url=None, optional=False, cache=True):
                 raise
             return None
         else:
-            if not url:
-                url = DEFAULT_URL
             if cache:
-                TRANSPORTS[url] = transport
+                TRANSPORTS[cache_key] = transport
     return transport
 
 
diff --git a/ceilometer/tests/test_messaging.py b/ceilometer/tests/test_messaging.py
new file mode 100644
index 000000000..4869e7851
--- /dev/null
+++ b/ceilometer/tests/test_messaging.py
@@ -0,0 +1,67 @@
+# Copyright (C) 2014 eNovance SAS <licensing@enovance.com>
+#
+# Author: Mehdi Abaakouk <mehdi.abaakouk@enovance.com>
+#
+# Licensed under the Apache License, Version 2.0 (the "License"); you may
+# not use this file except in compliance with the License. You may obtain
+# a copy of the License at
+#
+#      http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License 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.
+
+import oslo.messaging
+
+from ceilometer import messaging
+from ceilometer.openstack.common.fixture import config
+from ceilometer.openstack.common import test
+
+
+class MessagingTests(test.BaseTestCase):
+    def setUp(self):
+        super(MessagingTests, self).setUp()
+        self.CONF = self.useFixture(config.Config()).conf
+        self.useFixture(oslo.messaging.conffixture.ConfFixture(self.CONF))
+
+    def test_get_transport_invalid_url(self):
+        self.assertRaises(oslo.messaging.InvalidTransportURL,
+                          messaging.get_transport, "notvalid!")
+
+    def test_get_transport_url_caching(self):
+        t1 = messaging.get_transport('fake://')
+        t2 = messaging.get_transport('fake://')
+        self.assertEqual(t1, t2)
+
+    def test_get_transport_default_url_caching(self):
+        t1 = messaging.get_transport()
+        t2 = messaging.get_transport()
+        self.assertEqual(t1, t2)
+
+    def test_get_transport_default_url_no_caching(self):
+        t1 = messaging.get_transport(cache=False)
+        t2 = messaging.get_transport(cache=False)
+        self.assertNotEqual(t1, t2)
+
+    def test_get_transport_url_no_caching(self):
+        t1 = messaging.get_transport('fake://', cache=False)
+        t2 = messaging.get_transport('fake://', cache=False)
+        self.assertNotEqual(t1, t2)
+
+    def test_get_transport_default_url_caching_mix(self):
+        t1 = messaging.get_transport()
+        t2 = messaging.get_transport(cache=False)
+        self.assertNotEqual(t1, t2)
+
+    def test_get_transport_url_caching_mix(self):
+        t1 = messaging.get_transport('fake://')
+        t2 = messaging.get_transport('fake://', cache=False)
+        self.assertNotEqual(t1, t2)
+
+    def test_get_transport_optional(self):
+        self.CONF.set_override('rpc_backend', '')
+        self.assertIsNone(messaging.get_transport(optional=True,
+                                                  cache=False))