From 650c9e5853ea57b985df1e9d97288a7d879a2e4f Mon Sep 17 00:00:00 2001
From: Scott Bahling <sbahling@suse.com>
Date: Thu, 13 Feb 2020 11:43:10 +0100
Subject: Implement general exception handling

We trap the low level OBS api calls via a callback and
check for success. If error returned, parse the OBS error response
and raise an exception.
---
 obsapi/core.py    | 88 ++++++++++++++++++++++++++++++++++---------------------
 obsapi/httpapi.py |  2 --
 test/test_api.py  | 18 ++++++++++++
 3 files changed, 72 insertions(+), 36 deletions(-)

diff --git a/obsapi/core.py b/obsapi/core.py
index 209aab2..e91c408 100644
--- a/obsapi/core.py
+++ b/obsapi/core.py
@@ -17,6 +17,7 @@ SourceInfo = namedtuple('SourceInfo', 'package rev vrev srcmd5 verifymd5')
 Binary = namedtuple('Binary', 'filename size mtime')
 User = namedtuple('User', 'userid role')
 Group = namedtuple('Group', 'groupid role')
+CallStatus = namedtuple('CallStatus', 'code, summary details')
 
 REGEX_NS = 'http://exslt.org/regular-expressions'
 DEFAULTAPIURL = 'https://api.opensuse.org'
@@ -52,11 +53,59 @@ class ObsApi(object):
 
     def _api_callback(self, api_instance):
         self.lastapi = api_instance
+        print(self.response.text)
+        if not self.success:
+            self._raise_error()
+
+    def _raise_error(self):
+        msg = self.status_message
+        raise Exception(msg)
+
+    @property
+    def call_status(self):
+
+        if not self.response.text.strip().startswith('<status'):
+            return CallStatus(None, None, None)
+
+        try:
+            status = etree.fromstring(self.response.text)
+        except Exception:
+            return CallStatus(None, None, None)
+
+        if not status.tag == 'status':
+            return CallStatus(None, None, None)
+
+        code = status.get('code', None)
+        summary = status.find('summary')
+        details = status.find('details')
+        if summary is not None:
+            summary = summary.text
+        if details is not None:
+            details = details.text
+
+        return CallStatus(code=code, summary=summary, details=details)
+
+    @property
+    def status_message(self):
+        code = self.call_status.code
+        summary = self.call_status.summary
+        details = self.call_status.details
+        if details:
+            msg = '{}: {}\n{}'.format(code.replace('_', ' ').capitalize(),
+                                      summary, details)
+        else:
+            msg = '{}: {}'.format(code.replace('_', ' ').capitalize(),
+                                  summary)
+        return msg
 
     @property
     def response(self):
         return self.lastapi.response
 
+    @property
+    def success(self):
+        return self.lastapi.success
+
     def get_meta(self, prj, pkg=None):
         return self.source.get_meta(prj, pkg=pkg)
 
@@ -125,12 +174,14 @@ class ObsApi(object):
             xml = self.build.get_repository(prj, repo, arch)
         else:
             xml = self.build.get(prj, repo, arch, pkg)
-        if xml:
+        try:
             blist = etree.fromstring(xml)
             binaries = [Binary(filename=i.get('filename'),
                                size=i.get('size'),
                                mtime=i.get('mtime'))
                         for i in blist.findall('binary')]
+        except Exception as e:
+            raise e
 
         return binaries
 
@@ -296,9 +347,6 @@ class ObsApi(object):
             raise Exception(e)
 
         xml = self.get_meta(prj, pkg)
-        if xml is None:
-            raise Exception('unable to retrieve project meta')
-
         meta = xml2etree(xml)
         etree.SubElement(meta, 'person', userid=userid, role=role)
         return self.put_meta(prj, pkg, etree.tostring(meta))
@@ -315,11 +363,7 @@ class ObsApi(object):
             raise Exception(e)
 
         xml = self.get_meta(prj, pkg)
-        if xml is None:
-            raise Exception('unable to retrieve project meta')
-
         meta = xml2etree(xml)
-
         etree.SubElement(meta, 'group', groupid=groupid, role=role)
         return self.put_meta(prj, pkg, etree.tostring(meta))
 
@@ -355,38 +399,14 @@ class ObsApi(object):
                     arch=getattr(flag, 'arch', flag.get('arch', None)),
                     )
 
-    def parse_status_code(self, xml):
-        status_code = etree.fromstring(xml)
-        code = status_code.get('code', None)
-        summary = status_code.find('summary')
-        if summary is not None:
-            summary = summary.text
-
-        return (code, summary)
-
-    def __check_status(self, r):
-        if r.status_code == 400:
-            code, summary = self.parse_status_code(r.text)
-            msg = '{}: {}'.format(code.replace('_', ' ').capitalize(), summary)
-            raise Exception(msg)
-        return r
-
     def set_flag(self, flag, prj, pkg=None):
         params = self.__flag_to_params(flag)
         params['cmd'] = 'set_flag'
 
-        r = self.source.post(prj, pkg, **params)
-        try:
-            return self.__check_status(r)
-        except Exception as e:
-            raise e
+        return self.source.post(prj, pkg, **params)
 
     def remove_flag(self, flag, prj, pkg=None):
         params = self.__flag_to_params(flag)
         params['cmd'] = 'remove_flag'
 
-        r = self.source.post(prj, pkg, **params)
-        try:
-            return self.__check_status(r)
-        except Exception as e:
-            raise e
+        return self.source.post(prj, pkg, **params)
diff --git a/obsapi/httpapi.py b/obsapi/httpapi.py
index b73b547..ac695b5 100644
--- a/obsapi/httpapi.py
+++ b/obsapi/httpapi.py
@@ -161,8 +161,6 @@ class ObsHttpApi(object):
         api = self.__api(*args)
         stream = params.get('stream', False)
         r = self.__api_get(api, params=params)
-        if not self.success:
-            return None
 
         if binary_get:
             if stream:
diff --git a/test/test_api.py b/test/test_api.py
index 09b2806..f6ccdf4 100644
--- a/test/test_api.py
+++ b/test/test_api.py
@@ -204,6 +204,15 @@ def locked_package_meta():
 </package>"""
 
 
+@pytest.fixture
+def error_status():
+    return """<status code="Error">
+    <summary>This is the summary</summary>
+    <details>Here are the details</details>
+    </status>
+    """
+
+
 def test_callback(requests_mock):
     """
     Test that api callback works and returns a reference to
@@ -218,6 +227,15 @@ def test_callback(requests_mock):
     assert api.lastapi is api.source
 
 
+def test_call_status_on_error(requests_mock, error_status):
+    requests_mock.get(rmock.ANY, text=error_status, status_code=404)
+    with pytest.raises(Exception):
+        api.ls()
+    assert api.call_status.code == "Error"
+    assert api.call_status.summary == "This is the summary"
+    assert api.call_status.details == "Here are the details"
+
+
 def test_get_project_meta(requests_mock):
     matcher = prj_meta_re
     text = '<project/>'
-- 
cgit v1.2.3