Results from code review with Nat. Consolidate some of the coroutine/http code...

Results from code review with Nat. Consolidate some of the coroutine/http code into a single adapter.

indra/llmessage/llavatarnamecache.cpp

@@ -189,10 +189,6 @@ namespace LLAvatarNameCache
 // further explanation.
 void LLAvatarNameCache::requestAvatarNameCache_(LLCoros::self& self, std::string url, std::vector<LLUUID> agentIds)
 {
-    LLEventStream  replyPump("NameCacheReply", true);
-    LLCoreHttpUtil::HttpCoroHandler::ptr_t httpHandler = 
-        LLCoreHttpUtil::HttpCoroHandler::ptr_t(new LLCoreHttpUtil::HttpCoroHandler(replyPump));
-
     LL_DEBUGS("AvNameCache") << "Entering coroutine " << LLCoros::instance().getName(self)
         << " with url '" << url << "', requesting " << agentIds.size() << " Agent Ids" << LL_ENDL;
 
@@ -200,12 +196,8 @@ void LLAvatarNameCache::requestAvatarNameCache_(LLCoros::self& self, std::string
     {
         bool success = true;
 
-        LLAvatarNameCache::sHttpRequest->requestGet(
-            LLAvatarNameCache::sHttpPolicy, LLAvatarNameCache::sHttpPriority, 
-            url, LLAvatarNameCache::sHttpOptions.get(), 
-            LLAvatarNameCache::sHttpHeaders.get(), httpHandler.get());
-
-        LLSD results = waitForEventOn(self, replyPump);
+        LLCoreHttpUtil::HttpCoroutineAdapter httpAdapter("NameCache", LLAvatarNameCache::sHttpPolicy);
+        LLSD results = httpAdapter.getAndYield(self, sHttpRequest, url);
         LLSD httpResults;
 
         LL_DEBUGS() << results << LL_ENDL;
@@ -564,8 +556,6 @@ void LLAvatarNameCache::idle()
 	// By convention, start running at first idle() call
 	sRunning = true;
 
-    sHttpRequest->update(0L);
-
 	// *TODO: Possibly re-enabled this based on People API load measurements
 	// 100 ms is the threshold for "user speed" operations, so we can
 	// stall for about that long to batch up requests.

indra/llmessage/llcorehttputil.cpp

@@ -187,7 +187,7 @@ std::string responseToString(LLCore::HttpResponse * response)
     return empty;
 }
 
-
+//========================================================================
 HttpCoroHandler::HttpCoroHandler(LLEventStream &reply) :
     mReplyPump(reply)
 {
@@ -251,11 +251,12 @@ void HttpCoroHandler::buildStatusEntry(LLCore::HttpResponse *response, LLCore::H
 {
     LLSD httpresults = LLSD::emptyMap();
 
-    httpresults["success"] = static_cast<LLSD::Boolean>(status);
-    httpresults["type"] = static_cast<LLSD::Integer>(status.getType());
-    httpresults["status"] = static_cast<LLSD::Integer>(status.getStatus());
-    httpresults["message"] = static_cast<LLSD::String>(status.getMessage());
-    httpresults["url"] = static_cast<LLSD::String>(response->getRequestURL());
+    writeStatusCodes(status, response->getRequestURL(), httpresults);
+    //     httpresults["success"] = static_cast<LLSD::Boolean>(status);
+    //     httpresults["type"] = static_cast<LLSD::Integer>(status.getType());
+    //     httpresults["status"] = static_cast<LLSD::Integer>(status.getStatus());
+    //     httpresults["message"] = static_cast<LLSD::String>(status.getMessage());
+    //     httpresults["url"] = static_cast<LLSD::String>(response->getRequestURL());
 
     LLSD httpHeaders = LLSD::emptyMap();
     LLCore::HttpHeaders * hdrs = response->getHeaders();
@@ -279,6 +280,17 @@ void HttpCoroHandler::buildStatusEntry(LLCore::HttpResponse *response, LLCore::H
     result["http_result"] = httpresults;
 }
 
+void HttpCoroHandler::writeStatusCodes(LLCore::HttpStatus status, const std::string &url, LLSD &result)
+{
+    result["success"] = static_cast<LLSD::Boolean>(status);
+    result["type"] = static_cast<LLSD::Integer>(status.getType());
+    result["status"] = static_cast<LLSD::Integer>(status.getStatus());
+    result["message"] = static_cast<LLSD::String>(status.getMessage());
+    result["url"] = static_cast<LLSD::String>(url);
+
+}
+
+//========================================================================
 HttpRequestPumper::HttpRequestPumper(const LLCore::HttpRequest::ptr_t &request) :
     mHttpRequest(request)
 {
@@ -300,6 +312,164 @@ bool HttpRequestPumper::pollRequest(const LLSD&)
     return false;
 }
 
+//========================================================================
+HttpCoroutineAdapter::HttpCoroutineAdapter(const std::string &name,
+    LLCore::HttpRequest::policy_t policyId, LLCore::HttpRequest::priority_t priority) :
+    mAdapterName(name),
+    mPolicyId(policyId),
+    mPriority(priority)
+{
+}
+
+HttpCoroutineAdapter::~HttpCoroutineAdapter()
+{
+}
+
+LLSD HttpCoroutineAdapter::postAndYield(LLCoros::self & self, LLCore::HttpRequest::ptr_t &request,
+    const std::string & url, const LLSD & body,
+    LLCore::HttpOptions::ptr_t options, LLCore::HttpHeaders::ptr_t headers)
+{
+    if (!options)
+    {
+        options = LLCore::HttpOptions::ptr_t(new LLCore::HttpOptions(), false);
+    }
+
+    if (!headers)
+    {
+        headers = LLCore::HttpHeaders::ptr_t(new LLCore::HttpHeaders(), false);
+    }
+
+    LLEventStream  replyPump(mAdapterName, true);
+    LLCoreHttpUtil::HttpCoroHandler::ptr_t httpHandler =
+        LLCoreHttpUtil::HttpCoroHandler::ptr_t(new LLCoreHttpUtil::HttpCoroHandler(replyPump));
+
+    //LL_INFOS() << "Requesting transaction " << transactionId << LL_ENDL;
+    LLCoreHttpUtil::HttpRequestPumper pumper(request);
+    // The HTTPCoroHandler does not self delete, so retrieval of a the contained 
+    // pointer from the smart pointer is safe in this case.
+    LLCore::HttpHandle hhandle = requestPostWithLLSD(request,
+        mPolicyId, mPriority, url, body, options, headers,
+        httpHandler.get());
+
+    if (hhandle == LLCORE_HTTP_HANDLE_INVALID)
+    {
+        LLCore::HttpStatus status = request->getStatus();
+        LL_WARNS() << "Error posting to " << url << " Status=" << status.getStatus() <<
+            " message = " << status.getMessage() << LL_ENDL;
+
+        // Mimic the status results returned from an http error that we had 
+        // to wait on 
+        LLSD httpresults = LLSD::emptyMap();
+
+        httpHandler->writeStatusCodes(status, url, httpresults);
+
+        LLSD errorres = LLSD::emptyMap();
+        errorres["http_result"] = httpresults;
+
+        return errorres;
+    }
+
+    LLSD results = waitForEventOn(self, replyPump);
+    //LL_INFOS() << "Results for transaction " << transactionId << LL_ENDL;
+    return results;
+}
+
+LLSD HttpCoroutineAdapter::putAndYield(LLCoros::self & self, LLCore::HttpRequest::ptr_t & request,
+    const std::string & url, const LLSD & body,
+    LLCore::HttpOptions::ptr_t options, LLCore::HttpHeaders::ptr_t headers)
+{
+    if (!options)
+    {
+        options = LLCore::HttpOptions::ptr_t(new LLCore::HttpOptions(), false);
+    }
+
+    if (!headers)
+    {
+        headers = LLCore::HttpHeaders::ptr_t(new LLCore::HttpHeaders(), false);
+    }
+
+    LLEventStream  replyPump(mAdapterName, true);
+    LLCoreHttpUtil::HttpCoroHandler::ptr_t httpHandler =
+        LLCoreHttpUtil::HttpCoroHandler::ptr_t(new LLCoreHttpUtil::HttpCoroHandler(replyPump));
+
+    //LL_INFOS() << "Requesting transaction " << transactionId << LL_ENDL;
+    LLCoreHttpUtil::HttpRequestPumper pumper(request);
+    // The HTTPCoroHandler does not self delete, so retrieval of a the contained 
+    // pointer from the smart pointer is safe in this case.
+    LLCore::HttpHandle hhandle = requestPutWithLLSD(request,
+        mPolicyId, mPriority, url, body, options, headers,
+        httpHandler.get());
+
+    if (hhandle == LLCORE_HTTP_HANDLE_INVALID)
+    {
+        LLCore::HttpStatus status = request->getStatus();
+        LL_WARNS() << "Error posting to " << url << " Status=" << status.getStatus() <<
+            " message = " << status.getMessage() << LL_ENDL;
+
+        // Mimic the status results returned from an http error that we had 
+        // to wait on 
+        LLSD httpresults = LLSD::emptyMap();
+
+        httpHandler->writeStatusCodes(status, url, httpresults);
+
+        LLSD errorres = LLSD::emptyMap();
+        errorres["http_result"] = httpresults;
+
+        return errorres;
+    }
+
+    LLSD results = waitForEventOn(self, replyPump);
+    //LL_INFOS() << "Results for transaction " << transactionId << LL_ENDL;
+    return results;
+}
+
+LLSD HttpCoroutineAdapter::getAndYield(LLCoros::self & self, LLCore::HttpRequest::ptr_t & request,
+    const std::string & url,
+    LLCore::HttpOptions::ptr_t options, LLCore::HttpHeaders::ptr_t headers)
+{
+    if (!options)
+    {
+        options = LLCore::HttpOptions::ptr_t(new LLCore::HttpOptions(), false);
+    }
+
+    if (!headers)
+    {
+        headers = LLCore::HttpHeaders::ptr_t(new LLCore::HttpHeaders(), false);
+    }
+
+    LLEventStream  replyPump(mAdapterName + "Reply", true);
+    LLCoreHttpUtil::HttpCoroHandler::ptr_t httpHandler =
+        LLCoreHttpUtil::HttpCoroHandler::ptr_t(new LLCoreHttpUtil::HttpCoroHandler(replyPump));
+
+    //LL_INFOS() << "Requesting transaction " << transactionId << LL_ENDL;
+    LLCoreHttpUtil::HttpRequestPumper pumper(request);
+    // The HTTPCoroHandler does not self delete, so retrieval of a the contained 
+    // pointer from the smart pointer is safe in this case.
+    LLCore::HttpHandle hhandle = request->requestGet(mPolicyId, mPriority, 
+            url, options.get(), headers.get(), httpHandler.get());
+
+    if (hhandle == LLCORE_HTTP_HANDLE_INVALID)
+    {
+        LLCore::HttpStatus status = request->getStatus();
+        LL_WARNS() << "Error posting to " << url << " Status=" << status.getStatus() <<
+            " message = " << status.getMessage() << LL_ENDL;
+
+        // Mimic the status results returned from an http error that we had 
+        // to wait on 
+        LLSD httpresults = LLSD::emptyMap();
+
+        httpHandler->writeStatusCodes(status, url, httpresults);
+
+        LLSD errorres = LLSD::emptyMap();
+        errorres["http_result"] = httpresults;
+
+        return errorres;
+    }
+
+    LLSD results = waitForEventOn(self, replyPump);
+    //LL_INFOS() << "Results for transaction " << transactionId << LL_ENDL;
+    return results;
+}
 
 } // end namespace LLCoreHttpUtil
 

indra/llmessage/llcorehttputil.h

@@ -40,6 +40,8 @@
 #include "bufferstream.h"
 #include "llsd.h"
 #include "llevents.h"
+#include "llcoros.h"
+#include "lleventcoro.h"
 
 ///
 /// The base llcorehttp library implements many HTTP idioms
@@ -174,6 +176,7 @@ class HttpCoroHandler : public LLCore::HttpHandler
 
     typedef boost::shared_ptr<HttpCoroHandler> ptr_t;
 
+    void writeStatusCodes(LLCore::HttpStatus status, const std::string &url, LLSD &result);
 private:
     void buildStatusEntry(LLCore::HttpResponse *response, LLCore::HttpStatus status, LLSD &result);
 
@@ -196,7 +199,49 @@ class HttpRequestPumper
     LLCore::HttpRequest::ptr_t mHttpRequest;
 };
 
-} // end namespace LLCoreHttpUtil
+/// An adapter to handle some of the boilerplate code surrounding HTTP and coroutine 
+/// interaction.
+/// 
+/// Construct an HttpCoroutineAdapter giving it a name and policy Id. After 
+/// any application specific setup call the post, put or get method.  The request 
+/// will be automatically pumped and the method will return with an LLSD describing
+/// the result of the operation.  See HttpCoroHandler for a description of the 
+/// decoration done to the returned LLSD.
+class HttpCoroutineAdapter
+{
+public:
+    HttpCoroutineAdapter(const std::string &name, LLCore::HttpRequest::policy_t policyId, 
+            LLCore::HttpRequest::priority_t priority = 0L);
+    ~HttpCoroutineAdapter();
+
+    /// Execute a Post transaction on the supplied URL and yield execution of 
+    /// the coroutine until a result is available.
+    LLSD postAndYield(LLCoros::self & self, LLCore::HttpRequest::ptr_t & request,
+        const std::string & url, const LLSD & body, 
+        LLCore::HttpOptions::ptr_t options = LLCore::HttpOptions::ptr_t(), 
+        LLCore::HttpHeaders::ptr_t headers = LLCore::HttpHeaders::ptr_t());
+
+    /// Execute a Put transaction on the supplied URL and yield execution of 
+    /// the coroutine until a result is available.
+    LLSD putAndYield(LLCoros::self & self, LLCore::HttpRequest::ptr_t & request,
+        const std::string & url, const LLSD & body,
+        LLCore::HttpOptions::ptr_t options = LLCore::HttpOptions::ptr_t(),
+        LLCore::HttpHeaders::ptr_t headers = LLCore::HttpHeaders::ptr_t());
+
+    /// Execute a Get transaction on the supplied URL and yield execution of 
+    /// the coroutine until a result is available.
+    LLSD getAndYield(LLCoros::self & self, LLCore::HttpRequest::ptr_t & request,
+        const std::string & url,
+        LLCore::HttpOptions::ptr_t options = LLCore::HttpOptions::ptr_t(),
+        LLCore::HttpHeaders::ptr_t headers = LLCore::HttpHeaders::ptr_t());
 
+private:
+
+    std::string                     mAdapterName;
+    LLCore::HttpRequest::priority_t mPriority;
+    LLCore::HttpRequest::policy_t   mPolicyId;
+};
+
+} // end namespace LLCoreHttpUtil
 
 #endif // LL_LLCOREHTTPUTIL_H

indra/newview/llaccountingcostmanager.cpp

@@ -53,10 +53,6 @@ LLAccountingCostManager::LLAccountingCostManager():
 void LLAccountingCostManager::accountingCostCoro(LLCoros::self& self, std::string url,
     eSelectionType selectionType, const LLHandle<LLAccountingCostObserver> observerHandle)
 {
-    LLEventStream  replyPump("AccountingCostReply", true);
-    LLCoreHttpUtil::HttpCoroHandler::ptr_t httpHandler = 
-        LLCoreHttpUtil::HttpCoroHandler::ptr_t(new LLCoreHttpUtil::HttpCoroHandler(replyPump));
-
     LL_DEBUGS("LLAccountingCostManager") << "Entering coroutine " << LLCoros::instance().getName(self)
         << " with url '" << url << LL_ENDL;
 
@@ -108,36 +104,22 @@ void LLAccountingCostManager::accountingCostCoro(LLCoros::self& self, std::strin
         LLUUID transactionId = observer->getTransactionID();
         observer = NULL;
 
-        LLSD results;
-        {   // Scoping block for pumper object
-            //LL_INFOS() << "Requesting transaction " << transactionId << LL_ENDL;
-            LLCoreHttpUtil::HttpRequestPumper pumper(mHttpRequest);
-            LLCore::HttpHandle hhandle = LLCoreHttpUtil::requestPostWithLLSD(mHttpRequest,
-                mHttpPolicy, mHttpPriority, url, dataToPost, mHttpOptions, mHttpHeaders,
-                httpHandler.get());
+        LLCoreHttpUtil::HttpCoroutineAdapter httpAdapter("AccountingCost", mHttpPolicy);
 
-            if (hhandle == LLCORE_HTTP_HANDLE_INVALID)
-            {
-                LLCore::HttpStatus status = mHttpRequest->getStatus();
-                LL_WARNS() << "Error posting to " << url << " Status=" << status.getStatus() <<
-                    " message = " << status.getMessage() << LL_ENDL;
-                mPendingObjectQuota.clear();
-                return;
-            }
+        LLSD results = httpAdapter.postAndYield(self, mHttpRequest, url, dataToPost);
 
-            results = waitForEventOn(self, replyPump);
-            //LL_INFOS() << "Results for transaction " << transactionId << LL_ENDL;
-        }
         LLSD httpResults;
         httpResults = results["http_result"];
 
+        // do/while(false) allows error conditions to break out of following 
+        // block while normal flow goes forward once.
         do 
         {
             observer = observerHandle.get();
             if ((!observer) || (observer->getTransactionID() != transactionId))
             {   // *TODO: Rider: I've noticed that getTransactionID() does not 
                 // always match transactionId (the new transaction Id does not show a 
-                // corresponding request.)
+                // corresponding request.) (ask Vir)
                 if (!observer)
                     break;
                 LL_WARNS() << "Request transaction Id(" << transactionId