From ac060f97cc937787b4079e435c5b312f6894c277 Mon Sep 17 00:00:00 2001 From: Giuseppe Castagno Date: Thu, 22 Sep 2016 17:06:43 +0200 Subject: tdf#102499 (2): Refactor the WebDAV resource access retry Change-Id: Ia29560d54a61f1238f3b4e945d78308a3a68c483 Reviewed-on: https://gerrit.libreoffice.org/29269 Tested-by: Jenkins Reviewed-by: Giuseppe Castagno --- ucb/qa/cppunit/webdav/webdav_resource_access.cxx | 28 ++++++++++++++---- ucb/source/ucp/webdav-neon/DAVException.hxx | 4 +-- ucb/source/ucp/webdav-neon/DAVResourceAccess.cxx | 37 ++++++++++++++++-------- 3 files changed, 50 insertions(+), 19 deletions(-) diff --git a/ucb/qa/cppunit/webdav/webdav_resource_access.cxx b/ucb/qa/cppunit/webdav/webdav_resource_access.cxx index 4097d9e4b73a..e2794e220c8c 100644 --- a/ucb/qa/cppunit/webdav/webdav_resource_access.cxx +++ b/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 ) ); } } diff --git a/ucb/source/ucp/webdav-neon/DAVException.hxx b/ucb/source/ucp/webdav-neon/DAVException.hxx index 45fdedb9559b..424445c46fd1 100644 --- a/ucb/source/ucp/webdav-neon/DAVException.hxx +++ b/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 ) 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 () const sal_uInt16 SC_INSUFFICIENT_STORAGE = 507; diff --git a/ucb/source/ucp/webdav-neon/DAVResourceAccess.cxx b/ucb/source/ucp/webdav-neon/DAVResourceAccess.cxx index 0cf396d8a717..d77174e75ec3 100644 --- a/ucb/source/ucp/webdav-neon/DAVResourceAccess.cxx +++ b/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; -- cgit v1.2.3