From 74d76789e142d66ea8f46a6d5f7e3c2cd337ad6b Mon Sep 17 00:00:00 2001
From: "Brian C. Lane" <bcl@redhat.com>
Date: Wed, 25 Jul 2018 17:30:50 -0700
Subject: [PATCH] Modify handle_api_result so it can be used in more places

Some results have errors and no status, others have status and errors.
Update the function to return the final rc to exit with, and a bool
indicating whether or not to continue processing the other fields.

Add a bunch of tests for the new function to make sure I have the logic
correct.

(cherry picked from commit 35fa0672195319fa81385e26fca8404217c6230b)
---
 src/composer/cli/utilities.py    | 25 ++++++++++++-----
 tests/composer/test_utilities.py | 46 ++++++++++++++++++++++++++++++++
 2 files changed, 64 insertions(+), 7 deletions(-)

diff --git a/src/composer/cli/utilities.py b/src/composer/cli/utilities.py
index 5170c636..97f348e0 100644
--- a/src/composer/cli/utilities.py
+++ b/src/composer/cli/utilities.py
@@ -58,17 +58,28 @@ def handle_api_result(result, show_json=False):
 
     :param result: JSON result from the http query
     :type result: dict
+    :rtype: tuple
+    :returns: (rc, errors)
+
+    Return the correct rc for the program (0 or 1), and whether or
+    not to continue processing the results.
     """
     if show_json:
         print(json.dumps(result, indent=4))
-
-    for err in result.get("errors", []):
-        log.error(err)
-
-    if result["status"] == True:
-        return 0
     else:
-        return 1
+        for err in result.get("errors", []):
+            log.error(err)
+
+    # What's the rc? If status is present, use that
+    # If not, use length of errors
+    if "status" in result:
+        rc = bool(not result["status"])
+    else:
+        rc = bool(len(result.get("errors", [])) > 0)
+
+    # Caller should return if showing json, or status was present and False
+    exit_now = show_json or ("status" in result and rc)
+    return (rc, exit_now)
 
 def packageNEVRA(pkg):
     """Return the package info as a NEVRA
diff --git a/tests/composer/test_utilities.py b/tests/composer/test_utilities.py
index 9005e1ad..d8e5d58e 100644
--- a/tests/composer/test_utilities.py
+++ b/tests/composer/test_utilities.py
@@ -17,6 +17,7 @@
 import unittest
 
 from composer.cli.utilities import argify, toml_filename, frozen_toml_filename, packageNEVRA
+from composer.cli.utilities import handle_api_result
 
 class CliUtilitiesTest(unittest.TestCase):
     def test_argify(self):
@@ -45,3 +46,48 @@ class CliUtilitiesTest(unittest.TestCase):
                    "version": "10.0"}
         self.assertEqual(packageNEVRA(epoch_0), "basesystem-10.0-7.el7.noarch")
         self.assertEqual(packageNEVRA(epoch_3), "basesystem-3:10.0-7.el7.noarch")
+
+    def test_api_result_1(self):
+        """Test a result with no status and no error fields"""
+        result = {"foo": "bar"}
+        self.assertEqual(handle_api_result(result, show_json=False), (0, False))
+
+    def test_api_result_2(self):
+        """Test a result with errors=["some error"], and no status field"""
+        result = {"foo": "bar", "errors": ["some error"]}
+        self.assertEqual(handle_api_result(result, show_json=False), (1, False))
+
+    def test_api_result_3(self):
+        """Test a result with status=True, and errors=[]"""
+        result = {"status": True, "errors": []}
+        self.assertEqual(handle_api_result(result, show_json=False), (0, False))
+
+    def test_api_result_4(self):
+        """Test a result with status=False, and errors=[]"""
+        result = {"status": False, "errors": []}
+        self.assertEqual(handle_api_result(result, show_json=False), (1, True))
+
+    def test_api_result_5(self):
+        """Test a result with status=False, and errors=["some error"]"""
+        result = {"status": False, "errors": ["some error"]}
+        self.assertEqual(handle_api_result(result, show_json=False), (1, True))
+
+    def test_api_result_6(self):
+        """Test a result with show_json=True, and no status or errors fields"""
+        result = {"foo": "bar"}
+        self.assertEqual(handle_api_result(result, show_json=True), (0, True))
+
+    def test_api_result_7(self):
+        """Test a result with show_json=True, status=False, and errors=["some error"]"""
+        result = {"status": False, "errors": ["some error"]}
+        self.assertEqual(handle_api_result(result, show_json=True), (1, True))
+
+    def test_api_result_8(self):
+        """Test a result with show_json=True, errors=["some error"], and no status field"""
+        result = {"foo": "bar", "errors": ["some error"]}
+        self.assertEqual(handle_api_result(result, show_json=True), (1, True))
+
+    def test_api_result_9(self):
+        """Test a result with show_json=True, errors=[], and no status field"""
+        result = {"foo": "bar", "errors": []}
+        self.assertEqual(handle_api_result(result, show_json=True), (0, True))