From 7d80786532847650cadf51d1a1935a65f22fb1d6 Mon Sep 17 00:00:00 2001
From: Chenxiong Qi <cqi@redhat.com>
Date: Jun 09 2017 09:50:03 +0000
Subject: [PATCH 1/2] Fix incorrect lightblue entity name


Signed-off-by: Chenxiong Qi <cqi@redhat.com>

---

diff --git a/freshmaker/lightblue.py b/freshmaker/lightblue.py
index 764cb45..4c75d13 100644
--- a/freshmaker/lightblue.py
+++ b/freshmaker/lightblue.py
@@ -173,7 +173,7 @@ class LightBlue(object):
         """
 
         url = 'find/containerRepository/{}'.format(
-            self._get_entity_version('entityRespository'))
+            self._get_entity_version('containerRepository'))
         response = self._make_request(url, request)
 
         repos = []
diff --git a/tests/test_lightblue.py b/tests/test_lightblue.py
index 11dce78..6fea457 100644
--- a/tests/test_lightblue.py
+++ b/tests/test_lightblue.py
@@ -520,40 +520,35 @@ class TestEntityVersion(unittest.TestCase):
         self.fake_cert_file = 'path/to/cert'
         self.fake_private_key = 'path/to/private-key'
         self.fake_entity_versions = {
-            'containerImage': '0.0.11'
+            'containerImage': '0.0.11',
+            'containerRepository': '0.0.12',
         }
 
     @patch('freshmaker.lightblue.LightBlue._make_request')
-    @patch('os.path.exists')
-    def test_use_default_entity_version(self, exists, _make_request):
-        exists.return_value = True
-
+    @patch('os.path.exists', return_value=True)
+    def test_use_specified_container_image_version(self, exists, _make_request):
         lb = LightBlue(server_url=self.fake_server_url,
                        cert=self.fake_cert_file,
                        private_key=self.fake_private_key,
                        entity_versions=self.fake_entity_versions)
-        lb.find_container_repositories({})
+        lb.find_container_images({})
 
-        _make_request.assert_called_once_with('find/containerRepository/', {})
+        _make_request.assert_called_once_with('find/containerImage/0.0.11', {})
 
     @patch('freshmaker.lightblue.LightBlue._make_request')
-    @patch('os.path.exists')
-    def test_use_specified_entity_version(self, exists, _make_request):
-        exists.return_value = True
-
+    @patch('os.path.exists', return_value=True)
+    def test_use_specified_container_repository_version(self, exists, _make_request):
         lb = LightBlue(server_url=self.fake_server_url,
                        cert=self.fake_cert_file,
                        private_key=self.fake_private_key,
                        entity_versions=self.fake_entity_versions)
-        lb.find_container_images({})
+        lb.find_container_repositories({})
 
-        _make_request.assert_called_once_with('find/containerImage/0.0.11', {})
+        _make_request.assert_called_once_with('find/containerRepository/0.0.12', {})
 
     @patch('freshmaker.lightblue.LightBlue._make_request')
-    @patch('os.path.exists')
-    def test_use_default_entity_version_when_parameter_is_omitted(
-            self, exists, _make_request):
-        exists.return_value = True
+    @patch('os.path.exists', return_value=True)
+    def test_use_default_entity_version(self, exists, _make_request):
         _make_request.return_value = {
             # Omit other attributes that are not useful for this test
             'processed': []

From f86adaa20c87893f232471232ee6af7c6abe2438 Mon Sep 17 00:00:00 2001
From: Chenxiong Qi <cqi@redhat.com>
Date: Jun 12 2017 03:24:00 +0000
Subject: [PATCH 2/2] Handle LightBlue non-JSON errors


Signed-off-by: Chenxiong Qi <cqi@redhat.com>

---

diff --git a/freshmaker/lightblue.py b/freshmaker/lightblue.py
index 4c75d13..1b26187 100644
--- a/freshmaker/lightblue.py
+++ b/freshmaker/lightblue.py
@@ -21,49 +21,70 @@
 #
 # Written by Chenxiong Qi <cqi@redhat.com>
 
+import json
 import os
+import re
 import requests
-import json
+import six
 
 from six.moves import http_client
 
 
-class LightBlueRequestFailure(Exception):
-    """Exception when fail to request from LightBlue"""
+class LightBlueError(Exception):
+    """Base class representing errors from LightBlue server"""
 
-    def __init__(self, json_response, status_code):
+    def __init__(self, status_code, error_response):
         """Initialize
 
-        :param dict json_response: the JSON data returned from LightBlue
-            which contains all error information.
         :param int status_code: repsonse status code
+        :param str or dict error_response: response content returned from
+            LightBlue server that contains error content. There are two types of
+            error. A piece of HTML when error happens in system-wide, for example,
+            requested resource does not exists (404), and internal server error (500).
+            It could also be a JSON data when error happens while LightBlue handles
+            request.
         """
-        self._raw = json_response
+        self._raw = error_response
         self._status_code = status_code
 
     def __repr__(self):
         return '<{} [{}]>'.format(self.__class__.__name__, self.status_code)
 
-    def __str__(self):
-        return 'Error{} ({}):\n{}'.format(
-            's' if len(self.errors) > 1 else '',
-            len(self.errors),
-            '\n'.join(('    {}'.format(err['msg']) for err in self.errors))
-        )
-
     @property
     def raw(self):
         return self._raw
 
     @property
-    def errors(self):
-        return self.raw['errors']
-
-    @property
     def status_code(self):
         return self._status_code
 
 
+class LightBlueSystemError(LightBlueError):
+    """LightBlue system error"""
+
+    def _get_error_message(self):
+        # Remove all newlines if there is
+        buf = six.StringIO(self.raw)
+        html = ''.join((line.strip('\n') for line in buf))
+        match = re.search('<title>(.+)</title>', html)
+        return match.groups()[0]
+
+    def __str__(self):
+        return self._get_error_message()
+
+
+class LightBlueRequestError(LightBlueError):
+    """LightBlue request error"""
+
+    def __str__(self):
+        return 'Error{} ({}):\n{}'.format(
+            's' if len(self.raw['errors']) > 1 else '',
+            len(self.raw['errors']),
+            '\n'.join(('    {}'.format(err['msg'])
+                      for err in self.raw['errors']))
+        )
+
+
 class ContainerRepository(dict):
     """Represent a container repository"""
 
@@ -154,12 +175,20 @@ class LightBlue(object):
 
         :param dict response: the response returned from LightBlue, which is
             actually the requests response object.
-        :raises LightBlueRequestFailure: if response status code is not 200.
-            Otherwise, just keep silient.
+        :raises LightBlueSystemError or LightBlueRequestError: if response
+            status code is not 200. Otherwise, just keep silient.
         """
-        if response.status_code == http_client.OK:
+        status_code = response.status_code
+
+        if status_code == http_client.OK:
             return
-        raise LightBlueRequestFailure(response.json(), response.status_code)
+
+        if status_code in (http_client.NOT_FOUND,
+                           http_client.INTERNAL_SERVER_ERROR,
+                           http_client.UNAUTHORIZED):
+            raise LightBlueSystemError(status_code, response.content)
+
+        raise LightBlueRequestError(status_code, response.json())
 
     def find_container_repositories(self, request):
         """Query via entity containerRepository
diff --git a/tests/test_lightblue.py b/tests/test_lightblue.py
index 6fea457..1c2b85e 100644
--- a/tests/test_lightblue.py
+++ b/tests/test_lightblue.py
@@ -21,6 +21,7 @@
 # SOFTWARE.
 
 import json
+import six
 import unittest
 
 from mock import call, patch
@@ -29,11 +30,12 @@ from six.moves import http_client
 from freshmaker.lightblue import ContainerImage
 from freshmaker.lightblue import ContainerRepository
 from freshmaker.lightblue import LightBlue
-from freshmaker.lightblue import LightBlueRequestFailure
+from freshmaker.lightblue import LightBlueRequestError
+from freshmaker.lightblue import LightBlueSystemError
 
 
-class TestLightBlueRequestFailure(unittest.TestCase):
-    """Test case for exception LightBlueRequestFailure"""
+class TestLightBlueRequestError(unittest.TestCase):
+    """Test case for exception LightBlueRequestError"""
 
     def setUp(self):
         self.fake_error_data = {
@@ -53,17 +55,17 @@ class TestLightBlueRequestFailure(unittest.TestCase):
             'modifiedCount': 0,
             'status': 'ERROR'
         }
-        self.e = LightBlueRequestFailure(self.fake_error_data,
-                                         http_client.INTERNAL_SERVER_ERROR)
+        self.e = LightBlueRequestError(http_client.BAD_REQUEST,
+                                       self.fake_error_data)
 
     def test_get_raw_error_json_data(self):
         self.assertEqual(self.fake_error_data, self.e.raw)
 
     def test_get_status_code(self):
-        self.assertEqual(http_client.INTERNAL_SERVER_ERROR, self.e.status_code)
+        self.assertEqual(http_client.BAD_REQUEST, self.e.status_code)
 
     def test_get_inner_errors(self):
-        self.assertEqual(self.fake_error_data['errors'], self.e.errors)
+        self.assertEqual(self.fake_error_data['errors'], self.e.raw['errors'])
 
     def test_errors_listed_in_str(self):
         expected_s = '\n'.join(('    {}'.format(err['msg'])
@@ -71,6 +73,50 @@ class TestLightBlueRequestFailure(unittest.TestCase):
         self.assertIn(expected_s, str(self.e))
 
 
+class TestLightBlueSystemError(unittest.TestCase):
+    """Test LightBlueSystemError"""
+
+    def setUp(self):
+        buf = six.StringIO('''
+<html><head><title>JBWEB000065: HTTP Status 401 - JBWEB000009: No client
+certificate chain in this request</title><style><!--H1 {font-family:Tahoma,
+Arial,sans-serif;color:white;background-color:#525D76;font-size:22px;} H2
+{font-family:Tahoma,Arial,sans-serif;color:white;background-color:#525D76;
+font-size:16px;} H3 {font-family:Tahoma,Arial,sans-serif;color:white;
+background-color:#525D76;font-size:14px;} BODY {font-family:Tahoma,Arial,
+sans-serif;color:black;background-color:white;} B {font-family:Tahoma,Arial,
+sans-serif;color:white;background-color:#525D76;} P {font-family:Tahoma,Arial,
+sans-serif;background:white;color:black;font-size:12px;}A {color : black;}
+A.name {color : black;}HR {color : #525D76;}--></style> </head><body><h1>
+JBWEB000065: HTTP Status 401 - JBWEB000009: No client certificate chain in
+this request</h1><HR size="1" noshade="noshade"><p><b>JBWEB000309: type</b>
+JBWEB000067: Status report</p><p><b>JBWEB000068: message</b> <u>JBWEB000009:
+No client certificate chain in this request</u></p><p><b>JBWEB000069:
+description</b> <u>JBWEB000121: This request requires HTTP authentication.</u>
+</p><HR size="1" noshade="noshade"></body></html>
+''')
+        self.fake_error_data = ' '.join((line.strip() for line in buf))
+        self.e = LightBlueSystemError(http_client.UNAUTHORIZED,
+                                      self.fake_error_data)
+
+    def test_get_status_code(self):
+        self.assertEqual(http_client.UNAUTHORIZED, self.e.status_code)
+
+    def test_raw(self):
+        self.assertEqual(self.fake_error_data, self.e.raw)
+
+    def test__str__(self):
+        self.assertEqual(
+            'JBWEB000065: HTTP Status 401 - JBWEB000009: No client certificate'
+            ' chain in this request',
+            str(self.e))
+
+    def test__repr__(self):
+        self.assertEqual('<{} [{}]>'.format(self.e.__class__.__name__,
+                                            self.e.status_code),
+                         repr(self.e))
+
+
 class TestContainerImageObject(unittest.TestCase):
 
     def test_create(self):
@@ -303,7 +349,7 @@ class TestQueryEntityFromLightBlue(unittest.TestCase):
 
     @patch('freshmaker.lightblue.requests.post')
     def test_raise_error_if_request_data_is_incorrect(self, post):
-        post.return_value.status_code = http_client.INTERNAL_SERVER_ERROR
+        post.return_value.status_code = http_client.BAD_REQUEST
         post.return_value.json.return_value = {
             'entity': 'containerImage',
             'entityVersion': '0.0.11',
@@ -333,7 +379,7 @@ class TestQueryEntityFromLightBlue(unittest.TestCase):
             lb = LightBlue(server_url=self.fake_server_url,
                            cert=self.fake_cert_file,
                            private_key=self.fake_private_key)
-            self.assertRaises(LightBlueRequestFailure,
+            self.assertRaises(LightBlueRequestError,
                               lb._make_request, 'find/containerRepository/', fake_request)
 
     @patch('freshmaker.lightblue.LightBlue.find_container_repositories')
@@ -490,23 +536,23 @@ class TestQueryEntityFromLightBlue(unittest.TestCase):
                                                         cont_repos):
 
         exists.return_value = True
-        cont_repos.side_effect = LightBlueRequestFailure(
+        cont_repos.side_effect = LightBlueRequestError(
             {"errors": [{"msg": "dummy error"}]}, http_client.REQUEST_TIMEOUT)
         cont_images.return_value = self.fake_images_with_parsed_data
 
         lb = LightBlue(server_url=self.fake_server_url,
                        cert=self.fake_cert_file,
                        private_key=self.fake_private_key)
-        with self.assertRaises(LightBlueRequestFailure):
+        with self.assertRaises(LightBlueRequestError):
             lb.find_images_with_package_from_content_set(
                 "openssl",
                 ["dummy-content-set-1"])
 
         cont_repos.return_value = self.fake_repositories_with_content_sets
-        cont_images.side_effect = LightBlueRequestFailure(
+        cont_images.side_effect = LightBlueRequestError(
             {"errors": [{"msg": "dummy error"}]}, http_client.REQUEST_TIMEOUT)
 
-        with self.assertRaises(LightBlueRequestFailure):
+        with self.assertRaises(LightBlueRequestError):
             lb.find_images_with_package_from_content_set(
                 "openssl",
                 ["dummy-content-set-1"])

