From b2174fa5dc63de6e057199cb5db6a06845a79b38 Mon Sep 17 00:00:00 2001
From: Mike McLean <mikem@redhat.com>
Date: Nov 09 2017 22:47:40 +0000
Subject: PR#664: make grab_session_options to accept dict directly


Merges #664
https://pagure.io/koji/pull-request/664

Fixes: #663
https://pagure.io/koji/issue/663
RFE: grab_session_options should accept dict argument directly

---

diff --git a/koji/__init__.py b/koji/__init__.py
index 305cca3..b462dd1 100644
--- a/koji/__init__.py
+++ b/koji/__init__.py
@@ -2016,7 +2016,8 @@ class VirtualMethod(object):
 
 
 def grab_session_options(options):
-    '''Convert optparse options to a dict that ClientSession can handle'''
+    """Convert optparse options to a dict that ClientSession can handle;
+    If options is already a dict, filter out meaningless and None value items"""
     s_opts = (
         'user',
         'password',
@@ -2037,8 +2038,10 @@ def grab_session_options(options):
         'use_old_ssl',
         'no_ssl_verify',
         'serverca',
-        )
+    )
     # cert is omitted for now
+    if isinstance(options, dict):
+        return dict((k, v) for k, v in six.iteritems(options) if k in s_opts and v is not None)
     ret = {}
     for key in s_opts:
         if not hasattr(options, key):
diff --git a/tests/test_lib/test_grab_session_options.py b/tests/test_lib/test_grab_session_options.py
new file mode 100644
index 0000000..71d8c3e
--- /dev/null
+++ b/tests/test_lib/test_grab_session_options.py
@@ -0,0 +1,94 @@
+from __future__ import absolute_import
+import unittest
+import optparse
+
+import koji
+
+NORMAL_VAL = {'user': 'jdoe',
+              'password': 'fakepwd',
+              'krbservice': 'fakekrbservice',
+              'debug_xmlrpc': True,
+              'debug': False,
+              'max_retries': 3,
+              'retry_interval': 10,
+              'offline_retry': True,
+              'offline_retry_interval': 300,
+              'anon_retry': True,
+              'keepalive': False,
+              'timeout': 30000,
+              'use_fast_upload': False,
+              'upload_blocksize': 1024,
+              'krb_rdns': 'fakerdns',
+              'krb_canon_host': 'fakehost',
+              'use_old_ssl': False,
+              'no_ssl_verify': True,
+              'serverca': '/fake/serverca.cert',
+              }
+
+EMPTY_VAL = {}
+
+NONE_VAL = {'user': None,
+            'password': None,
+            'krbservice': None,
+            'debug_xmlrpc': None,
+            'debug': None,
+            'max_retries': None,
+            'retry_interval': None,
+            'offline_retry': None,
+            'offline_retry_interval': None,
+            'anon_retry': None,
+            'keepalive': None,
+            'timeout': None,
+            'use_fast_upload': None,
+            'upload_blocksize': None,
+            'krb_rdns': None,
+            'krb_canon_host': None,
+            'use_old_ssl': None,
+            'no_ssl_verify': None,
+            'serverca': None,
+            }
+
+MEANINGLESS_VAL = {'somekey': 'somevalue'}
+
+MIXED_VAL = {'user': None,
+             'password': None,
+             'anon_retry': 3,
+             'keepalive': True,
+             'timeout': 100,
+             'krb_rdns': 'fakerdns',
+             'meaningless': 'wow',
+             'nonval': None}
+
+MIXED_REL = {'anon_retry': 3,
+             'keepalive': True,
+             'timeout': 100,
+             'krb_rdns': 'fakerdns',
+             }
+
+
+class TestGrabSessionOptions(unittest.TestCase):
+    """TestCase for grab_session_options"""
+
+    def test_optparse_value(self):
+        rel = koji.grab_session_options(optparse.Values(NORMAL_VAL))
+        self.assertEqual(rel, NORMAL_VAL)
+        rel = koji.grab_session_options(optparse.Values(EMPTY_VAL))
+        self.assertEqual(rel, EMPTY_VAL)
+        rel = koji.grab_session_options(optparse.Values(NONE_VAL))
+        self.assertEqual(rel, {})
+        rel = koji.grab_session_options(optparse.Values(MEANINGLESS_VAL))
+        self.assertEqual(rel, {})
+        rel = koji.grab_session_options(optparse.Values(MIXED_VAL))
+        self.assertEqual(rel, MIXED_REL)
+
+    def test_dict(self):
+        rel = koji.grab_session_options(NORMAL_VAL)
+        self.assertEqual(rel, NORMAL_VAL)
+        rel = koji.grab_session_options(EMPTY_VAL)
+        self.assertEqual(rel, EMPTY_VAL)
+        rel = koji.grab_session_options(NONE_VAL)
+        self.assertEqual(rel, {})
+        rel = koji.grab_session_options(MEANINGLESS_VAL)
+        self.assertEqual(rel, {})
+        rel = koji.grab_session_options(MIXED_VAL)
+        self.assertEqual(rel, MIXED_REL)