Merge lp:~rackspace-titan/nova/lp796619 into lp:~hudson-openstack/nova/trunk

[Alex Meade]

Fixed bug 796619

[Brian Lamar (blamar)]

Pretty obvious this isn't being used anywhere. Before I approve this though, since it isn't being used...it is something that should just be removed instead of fixed?

I'm a big fan of removing code...although sometimes I get carried away.

review: Needs Information

[Alex Meade (alex-meade)]

ah, I just discovered that it is actually used directly through nova-manage. Sorry about that misinformation earlier, I was grepping one level too deep.

I suppose we should keep it then eh?

[Trey Morris (tr3buchet)]

I've grep'd inside branch/nova and been confused a few times too :D.

(trey|nova)~/nova/trunk> ack revoke_certs_by_user_and_project
bin/nova-manage
352: crypto.revoke_certs_by_user_and_project(user_id, project_id)

nova/crypto.py
176:def revoke_certs_by_user_and_project(user_id, project_id):
(trey|nova)~/nova/trunk>

If it's being used it needs to be kept unless we're writing off that functionality.

review: Approve

[Devin Carlen (devcamcar)]

hooray for tests!

review: Approve

[Brian Lamar (blamar)]

review: Approve

[Brian Waldon (bcwaldon)]

Good work, Alex.

review: Approve

=== modified file 'nova/crypto.py'
--- nova/crypto.py	2011-05-11 13:28:07 +0000
+++ nova/crypto.py	2011-06-13 19:26:07 +0000
@@ -176,7 +176,8 @@
 def revoke_certs_by_user_and_project(user_id, project_id):
     """Revoke certs for user in project."""
     admin = context.get_admin_context()
-    for cert in db.certificate_get_all_by_user(admin, user_id, project_id):
+    for cert in db.certificate_get_all_by_user_and_project(admin,
+                                            user_id, project_id):
         revoke_cert(cert['project_id'], cert['file_name'])
 
 

=== modified file 'nova/tests/test_crypto.py'
--- nova/tests/test_crypto.py	2011-05-11 13:28:07 +0000
+++ nova/tests/test_crypto.py	2011-06-13 19:26:07 +0000
@@ -16,7 +16,11 @@
 Tests for Crypto module.
 """
 
+import mox
+import stubout
+
 from nova import crypto
+from nova import db
 from nova import test
 
 
@@ -46,3 +50,82 @@
         plain = decrypt(cipher_text)
 
         self.assertEquals(plain_text, plain)
+
+
+class RevokeCertsTest(test.TestCase):
+
+    def setUp(self):
+        super(RevokeCertsTest, self).setUp()
+        self.stubs = stubout.StubOutForTesting()
+
+    def tearDown(self):
+        self.stubs.UnsetAll()
+        super(RevokeCertsTest, self).tearDown()
+
+    def test_revoke_certs_by_user_and_project(self):
+        user_id = 'test_user'
+        project_id = 2
+        file_name = 'test_file'
+
+        def mock_certificate_get_all_by_user_and_project(context,
+                                                         user_id,
+                                                         project_id):
+
+            return [{"user_id": user_id, "project_id": project_id,
+                                          "file_name": file_name}]
+
+        self.stubs.Set(db, 'certificate_get_all_by_user_and_project',
+                           mock_certificate_get_all_by_user_and_project)
+
+        self.mox.StubOutWithMock(crypto, 'revoke_cert')
+        crypto.revoke_cert(project_id, file_name)
+
+        self.mox.ReplayAll()
+
+        crypto.revoke_certs_by_user_and_project(user_id, project_id)
+
+        self.mox.VerifyAll()
+
+    def test_revoke_certs_by_user(self):
+        user_id = 'test_user'
+        project_id = 2
+        file_name = 'test_file'
+
+        def mock_certificate_get_all_by_user(context, user_id):
+
+            return [{"user_id": user_id, "project_id": project_id,
+                                          "file_name": file_name}]
+
+        self.stubs.Set(db, 'certificate_get_all_by_user',
+                                    mock_certificate_get_all_by_user)
+
+        self.mox.StubOutWithMock(crypto, 'revoke_cert')
+        crypto.revoke_cert(project_id, mox.IgnoreArg())
+
+        self.mox.ReplayAll()
+
+        crypto.revoke_certs_by_user(user_id)
+
+        self.mox.VerifyAll()
+
+    def test_revoke_certs_by_project(self):
+        user_id = 'test_user'
+        project_id = 2
+        file_name = 'test_file'
+
+        def mock_certificate_get_all_by_project(context, project_id):
+
+            return [{"user_id": user_id, "project_id": project_id,
+                                          "file_name": file_name}]
+
+        self.stubs.Set(db, 'certificate_get_all_by_project',
+                                    mock_certificate_get_all_by_project)
+
+        self.mox.StubOutWithMock(crypto, 'revoke_cert')
+        crypto.revoke_cert(project_id, mox.IgnoreArg())
+
+        self.mox.ReplayAll()
+
+        crypto.revoke_certs_by_project(project_id)
+
+        self.mox.VerifyAll()