reuse needRetry in RetryDomainsMiddleware and add tests for needRetry

Author: lihsai0

src/Qiniu/Http/Middleware/RetryDomainsMiddleware.php

@@ -38,7 +38,7 @@ private function shouldRetry($resp, $req)
             return call_user_func($this->retryCondition, $resp, $req);
         }
 
-        return !$resp || !$resp->ok();
+        return !$resp || $resp->needRetry();
     }
 
     /**

src/Qiniu/Http/Response.php

@@ -199,10 +199,18 @@ public function ok()
 
     public function needRetry()
     {
-        $code = $this->statusCode;
-        if ($code < 0 || ($code / 100 === 5 and $code !== 579) || $code === 996) {
-            return true;
+        if ($this->statusCode > 0 && $this->statusCode < 500) {
+            return false;
         }
+
+        // https://developer.qiniu.com/fusion/kb/1352/the-http-request-return-a-status-code
+        if (in_array($this->statusCode, array(
+            501, 509, 573, 579, 608, 612, 614, 616, 618, 630, 631, 632, 640, 701
+        ))) {
+            return false;
+        }
+
+        return true;
     }
 
     private static function isJson($headers)

src/Qiniu/Region.php

@@ -184,12 +184,7 @@ public static function queryRegion($ak, $bucket, $ucHost = null, $backupUcHosts
         $reqOpt->middlewares = array(
             new RetryDomainsMiddleware(
                 $backupUcHosts,
-                $retryTimes,
-                function ($resp) {
-                    // 612 is app/accesskey is not found
-                    // 631 is no such bucket
-                    return !$resp->ok() && !in_array($resp->statusCode, array(612, 631));
-                }
+                $retryTimes
             )
         );
         $ret = Client::Get($url, array(), $reqOpt);

tests/Qiniu/Tests/HttpTest.php

@@ -5,6 +5,7 @@
 
 use Qiniu\Http\Client;
 use Qiniu\Http\RequestOptions;
+use Qiniu\Http\Response;
 
 class HttpTest extends TestCase
 {
@@ -122,4 +123,29 @@ public function testPutTimeout()
         $response = Client::put('localhost:9000/timeout.php', null, array(), $reqOpt);
         $this->assertEquals(-1, $response->statusCode);
     }
+
+    public function testNeedRetry()
+    {
+        $testCases = array_merge(
+            array(array(-1, true)),
+            array_map(function ($i) {
+                return array($i, false);
+            }, range(100, 499)),
+            array_map(function ($i) {
+                if (in_array($i, array(
+                    501, 509, 573, 579, 608, 612, 614, 616, 618, 630, 631, 632, 640, 701
+                ))) {
+                    return array($i, false);
+                }
+                return array($i, true);
+            }, range(500, 799))
+        );
+        $resp = new Response(-1, 222, array(), '{"msg": "mock"}', null);
+        foreach ($testCases as $testCase) {
+            list($code, $expectNeedRetry) = $testCase;
+            $resp->statusCode = $code;
+            $msg = $resp->statusCode . ' need' . ($expectNeedRetry ? '' : ' NOT') . ' retry';
+            $this->assertEquals($expectNeedRetry, $resp->needRetry(), $msg);
+        }
+    }
 }