proxy-server: de-duplicate implementation of is_good_source

Both the EC and replicated GET paths have is_good_source() methods
that are similar. Refactor to have just one implementation.

Change-Id: I661d3704a76e3d92bfcfeaed1fff4ed5e28c79b4

swift/proxy/controllers/base.py

@@ -53,7 +53,8 @@ from swift.common.header_key_dict import HeaderKeyDict
 from swift.common.http import is_informational, is_success, is_redirection, \
     is_server_error, HTTP_OK, HTTP_PARTIAL_CONTENT, HTTP_MULTIPLE_CHOICES, \
     HTTP_BAD_REQUEST, HTTP_NOT_FOUND, HTTP_SERVICE_UNAVAILABLE, \
-    HTTP_UNAUTHORIZED, HTTP_CONTINUE, HTTP_GONE
+    HTTP_UNAUTHORIZED, HTTP_CONTINUE, HTTP_GONE, \
+    HTTP_REQUESTED_RANGE_NOT_SATISFIABLE
 from swift.common.swob import Request, Response, Range, \
     HTTPException, HTTPRequestedRangeNotSatisfiable, HTTPServiceUnavailable, \
     status_map, wsgi_to_str, str_to_wsgi, wsgi_quote, wsgi_unquote, \
@@ -1017,6 +1018,21 @@ def bytes_to_skip(record_size, range_start):
     return (record_size - (range_start % record_size)) % record_size
 
 
+def is_good_source(status, server_type):
+    """
+    Indicates whether or not the request made to the backend found
+    what it was looking for.
+
+    :param resp: the response from the backend.
+    :param server_type: the type of server: 'Account', 'Container' or 'Object'.
+    :returns: True if the response status code is acceptable, False if not.
+    """
+    if (server_type == 'Object' and
+            status == HTTP_REQUESTED_RANGE_NOT_SATISFIABLE):
+        return True
+    return is_success(status) or is_redirection(status)
+
+
 class ByteCountEnforcer(object):
     """
     Enforces that successive calls to file_like.read() give at least
@@ -1260,18 +1276,6 @@ class GetOrHeadHandler(GetterBase):
         # populated from response headers
         self.start_byte = self.end_byte = self.length = None
 
-    def _is_good_source(self, src):
-        """
-        Indicates whether or not the request made to the backend found
-        what it was looking for.
-
-        :param src: the response from the backend
-        :returns: True if found, False if not
-        """
-        if self.server_type == 'Object' and src.status == 416:
-            return True
-        return is_success(src.status) or is_redirection(src.status)
-
     def _get_next_response_part(self):
         # return the next part of the response body; there may only be one part
         # unless it's a multipart/byteranges response
@@ -1448,7 +1452,7 @@ class GetOrHeadHandler(GetterBase):
         src_headers = dict(
             (k.lower(), v) for k, v in
             possible_source.getheaders())
-        if self._is_good_source(possible_source):
+        if is_good_source(possible_source.status, self.server_type):
             # 404 if we know we don't have a synced copy
             if not float(possible_source.getheader('X-PUT-Timestamp', 1)):
                 self.statuses.append(HTTP_NOT_FOUND)

swift/proxy/controllers/obj.py

@@ -69,7 +69,8 @@ from swift.common.storage_policy import (POLICIES, REPL_POLICY, EC_POLICY,
                                          ECDriverError, PolicyError)
 from swift.proxy.controllers.base import Controller, delay_denial, \
     cors_validation, update_headers, bytes_to_skip, ByteCountEnforcer, \
-    record_cache_op_metrics, get_cache_key, GetterBase, GetterSource
+    record_cache_op_metrics, get_cache_key, GetterBase, GetterSource, \
+    is_good_source
 from swift.common.swob import HTTPAccepted, HTTPBadRequest, HTTPNotFound, \
     HTTPPreconditionFailed, HTTPRequestEntityTooLarge, HTTPRequestTimeout, \
     HTTPServerError, HTTPServiceUnavailable, HTTPClientDisconnect, \
@@ -2477,19 +2478,6 @@ class ECGetResponseCollection(object):
             return nodes.pop(0).copy()
 
 
-def is_good_source(status):
-    """
-    Indicates whether or not the request made to the backend found
-    what it was looking for.
-
-    :param status: the response from the backend
-    :returns: True if found, False if not
-    """
-    if status == HTTP_REQUESTED_RANGE_NOT_SATISFIABLE:
-        return True
-    return is_success(status) or is_redirection(status)
-
-
 class ECFragGetter(GetterBase):
 
     def __init__(self, app, req, node_iter, partition, policy, path,
@@ -2714,7 +2702,7 @@ class ECFragGetter(GetterBase):
         self.status = possible_source.status
         self.reason = possible_source.reason
         self.source_headers = possible_source.getheaders()
-        if is_good_source(possible_source.status):
+        if is_good_source(possible_source.status, server_type='Object'):
             self.body = None
             return possible_source
         else:
@@ -2945,9 +2933,9 @@ class ECObjectController(BaseObjectController):
                     break
                 requests_available = extra_requests < max_extra_requests and (
                     node_iter.nodes_left > 0 or buckets.has_alternate_node())
-                bad_resp = not is_good_source(get.last_status)
                 if requests_available and (
-                        buckets.shortfall > pile._pending or bad_resp):
+                        buckets.shortfall > pile._pending or
+                        not is_good_source(get.last_status, self.server_type)):
                     extra_requests += 1
                     pile.spawn(self._fragment_GET_request, req, safe_iter,
                                partition, policy, buckets.get_extra_headers,