tdf#102499 (2): Refactor the WebDAV resource access retry

Change-Id: Ia29560d54a61f1238f3b4e945d78308a3a68c483
Reviewed-on: https://gerrit.libreoffice.org/29269Tested-by: Jenkins <ci@libreoffice.org>
Reviewed-by: Giuseppe Castagno <giuseppe.castagno@acca-esse.eu>

ucb/qa/cppunit/webdav/webdav_resource_access.cxx

@@ -65,15 +65,33 @@ namespace
             const DAVException aTheException(DAVException::DAV_HTTP_ERROR, "http error code", i );
             CPPUNIT_ASSERT_EQUAL( false , ResourceAccess.handleException( aTheException, 1 ) );
         }
+
         // http error code from 500 (SC_INTERNAL_SERVER_ERROR) up should force a retry
-        // except: SC_NOT_IMPLEMENTED
+        // except in special value
+        // 1999 as high limit is just a current (2016-09-25) choice.
+        // RFC poses no limit to the max value of response status code
         for (auto i = SC_INTERNAL_SERVER_ERROR; i < 2000; i++)
         {
             const DAVException aTheException(DAVException::DAV_HTTP_ERROR, "http error code", i );
-            if( i != SC_NOT_IMPLEMENTED )
-                CPPUNIT_ASSERT_EQUAL( true , ResourceAccess.handleException( aTheException, 1 ) );
-            else
-                CPPUNIT_ASSERT_EQUAL( false , ResourceAccess.handleException( aTheException, 1 ) );
+            switch ( i )
+            {
+                // the HTTP response status codes that can be retried
+                case SC_BAD_GATEWAY:
+                case SC_GATEWAY_TIMEOUT:
+                case SC_SERVICE_UNAVAILABLE:
+                case SC_INSUFFICIENT_STORAGE:
+                    CPPUNIT_ASSERT_EQUAL( true , ResourceAccess.handleException( aTheException, 1 ) );
+                    break;
+                    // default is NOT retry
+                default:
+                    CPPUNIT_ASSERT_EQUAL( false , ResourceAccess.handleException( aTheException, 1 ) );
+            }
+        }
+
+        // check the retry request
+        {
+            const DAVException aTheException(DAVException::DAV_HTTP_RETRY, "the-host-name", 8080 );
+            CPPUNIT_ASSERT_EQUAL( true , ResourceAccess.handleException( aTheException, 1 ) );
         }
     }
 

ucb/source/ucp/webdav-neon/DAVException.hxx

@@ -91,14 +91,14 @@ const sal_uInt16 SC_UNPROCESSABLE_ENTITY             = 422;
 const sal_uInt16 SC_LOCKED                           = 423;
 const sal_uInt16 SC_FAILED_DEPENDENCY                = 424;
 
-//5xx (Server error)
+//5xx (Server error, general <https://tools.ietf.org/html/rfc7231#section-6.6>)
 const sal_uInt16 SC_INTERNAL_SERVER_ERROR            = 500;
 const sal_uInt16 SC_NOT_IMPLEMENTED                  = 501;
 const sal_uInt16 SC_BAD_GATEWAY                      = 502;
 const sal_uInt16 SC_SERVICE_UNAVAILABLE              = 503;
 const sal_uInt16 SC_GATEWAY_TIMEOUT                  = 504;
 const sal_uInt16 SC_HTTP_VERSION_NOT_SUPPORTED       = 505;
-// DAV extensions
+// DAV extensions (<https://tools.ietf.org/html/rfc4918#section-11>)
 const sal_uInt16 SC_INSUFFICIENT_STORAGE             = 507;
 
 

ucb/source/ucp/webdav-neon/DAVResourceAccess.cxx

@@ -1121,7 +1121,7 @@ void DAVResourceAccess::initialize()
                 return;
         }
 
-        // Own URI is needed for redirect cycle detection.
+        // Own URI is needed to redirect cycle detection.
         m_aRedirectURIs.push_back( aURI );
 
         // Success.
@@ -1251,20 +1251,33 @@ bool DAVResourceAccess::handleException( const DAVException & e, int errorCount
             return true;
         }
         return false;
-    // #67048# copy & paste images doesn't display.
-    // if we have a bad connection try again. Up to three times.
+        // #67048# copy & paste images doesn't display. This bug refers
+        // to an old OOo problem about getting resources from sites with a bad connection.
+        // If we have a bad connection try again. Up to three times.
     case DAVException::DAV_HTTP_ERROR:
-        // retry up to three times, if not a client-side error.
-        // exception: error 501, server side error that
-        // tells us the used method is not implemented
-        // on the server, it's nonsense to insist...
-        if ( ( e.getStatus() < 400 || e.getStatus() >= 500 ) &&
-             ( e.getStatus() != 501 ) &&
-             errorCount < 3 )
-        {
+        // retry up to three times, if not a client-side error (4xx error codes)
+        if ( e.getStatus() < SC_BAD_REQUEST && errorCount < 3 )
             return true;
+        // check the server side errors
+        switch( e.getStatus() )
+        {
+            // the HTTP server side response status codes that can be retried
+            case SC_BAD_GATEWAY:        // retry, can be an eccessive load
+            case SC_GATEWAY_TIMEOUT:    // retry, may be we get lucky
+            case SC_SERVICE_UNAVAILABLE: // retry, the service may become available
+            case SC_INSUFFICIENT_STORAGE: // space may be freed, retry
+            {
+                if ( errorCount < 3 )
+                    return true;
+                else
+                    return false;
+            }
+            break;
+            // all the other HTTP server response status codes are NOT retry
+            default:
+                return false;
         }
-        return false;
+        break;
     // if connection has said retry then retry!
     case DAVException::DAV_HTTP_RETRY:
         return true;