From: Christos Tsantilas Date: Thu, 23 May 2013 08:18:09 +0000 (+0300) Subject: Cleanup: Merge AccessLogEntry 'helperNotes' and 'configNotes' members to 'notes'... X-Git-Tag: SQUID_3_4_0_1~115 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=f4f55a212ef9da7adb851d1985db7cb4eac2072b;p=thirdparty%2Fsquid.git Cleanup: Merge AccessLogEntry 'helperNotes' and 'configNotes' members to 'notes' member There is not any need to store notes added using Note cfg option and notes added from helper to separated member. This patch merge them to the same AccessLogEntry::note member. This is a Measurement Factory project --- diff --git a/src/AccessLogEntry.h b/src/AccessLogEntry.h index bba7d3d59a..698a41c5b0 100644 --- a/src/AccessLogEntry.h +++ b/src/AccessLogEntry.h @@ -230,11 +230,9 @@ public: HttpRequest *request; //< virgin HTTP request HttpRequest *adapted_request; //< HTTP request after adaptation and redirection - // TODO: merge configNotes and helperNotes - /// key:value pairs set by note. - NotePairs::Pointer configNotes; + /// key:value pairs set by squid.conf note directive and /// key=value pairs returned from URL rewrite/redirect helper - NotePairs::Pointer helperNotes; + NotePairs::Pointer notes; #if ICAP_CLIENT /** \brief This subclass holds log info for ICAP part of request diff --git a/src/HttpRequest.cc b/src/HttpRequest.cc index f718da1bdf..20de12899b 100644 --- a/src/HttpRequest.cc +++ b/src/HttpRequest.cc @@ -58,15 +58,13 @@ #endif HttpRequest::HttpRequest() : - HttpMsg(hoRequest), - helperNotes(NULL) + HttpMsg(hoRequest) { init(); } HttpRequest::HttpRequest(const HttpRequestMethod& aMethod, AnyP::ProtocolType aProtocol, const char *aUrlpath) : - HttpMsg(hoRequest), - helperNotes(NULL) + HttpMsg(hoRequest) { static unsigned int id = 1; debugs(93,7, HERE << "constructed, this=" << this << " id=" << ++id); @@ -168,7 +166,7 @@ HttpRequest::clean() myportname.clean(); - helperNotes = NULL; + notes = NULL; tag.clean(); #if USE_AUTH @@ -228,7 +226,6 @@ HttpRequest::clone() const // XXX: what to do with copy->peer_domain? copy->myportname = myportname; - copy->helperNotes = helperNotes; copy->tag = tag; #if USE_AUTH copy->extacl_user = extacl_user; @@ -277,6 +274,7 @@ HttpRequest::inheritProperties(const HttpMsg *aMsg) // main property is which connection the request was received on (if any) clientConnectionManager = aReq->clientConnectionManager; + notes = aReq->notes; return true; } diff --git a/src/HttpRequest.h b/src/HttpRequest.h index d5c712e292..9a768709bb 100644 --- a/src/HttpRequest.h +++ b/src/HttpRequest.h @@ -203,7 +203,7 @@ public: String myportname; // Internal tag name= value from port this requests arrived in. - NotePairs::Pointer helperNotes; ///< collection of meta notes associated with this request by helper lookups. + NotePairs::Pointer notes; ///< annotations added by the note directive and helpers String tag; /* Internal tag for this request */ diff --git a/src/Notes.cc b/src/Notes.cc index 4101c5b975..68a5c688fa 100644 --- a/src/Notes.cc +++ b/src/Notes.cc @@ -29,6 +29,7 @@ #include "squid.h" #include "globals.h" +#include "AccessLogEntry.h" #include "acl/FilledChecklist.h" #include "acl/Gadgets.h" #include "ConfigParser.h" @@ -214,3 +215,16 @@ NotePairs::append(const NotePairs *src) entries.push_back(new NotePairs::Entry((*i)->name.termedBuf(), (*i)->value.termedBuf())); } } + + +NotePairs & +SyncNotes(AccessLogEntry &ale, HttpRequest &request) +{ + if (!ale.notes) { + assert(!request.notes); + ale.notes = request.notes = new NotePairs; + } else { + assert(ale.notes == request.notes); + } + return *ale.notes; +} diff --git a/src/Notes.h b/src/Notes.h index 108cdcc1f5..7fdcb9f895 100644 --- a/src/Notes.h +++ b/src/Notes.h @@ -175,4 +175,10 @@ public: MEMPROXY_CLASS_INLINE(NotePairs::Entry); +class AccessLogEntry; +/** + * Keep in sync HttpRequest and the corresponding AccessLogEntry objects + */ +NotePairs &SyncNotes(AccessLogEntry &ale, HttpRequest &request); + #endif diff --git a/src/client_side.cc b/src/client_side.cc index 18eadc8912..2b9341a2b5 100644 --- a/src/client_side.cc +++ b/src/client_side.cc @@ -685,13 +685,15 @@ ClientHttpRequest::logRequest() #endif - /*Add meta headers*/ + /*Add notes*/ + // The al->notes and request->notes must point to the same object. + // Enable the following assertion to check for possible bugs. + // assert(request->notes == al->notes); typedef Notes::iterator ACAMLI; for (ACAMLI i = Config.notes.begin(); i != Config.notes.end(); ++i) { if (const char *value = (*i)->match(request, al->reply)) { - if (al->configNotes == NULL) - al->configNotes = new NotePairs; - al->configNotes->add((*i)->key.termedBuf(), value); + NotePairs ¬es = SyncNotes(*al, *request); + notes.add((*i)->key.termedBuf(), value); debugs(33, 3, HERE << (*i)->key.termedBuf() << " " << value); } } diff --git a/src/client_side_request.cc b/src/client_side_request.cc index 4d5054a97d..c884fc725d 100644 --- a/src/client_side_request.cc +++ b/src/client_side_request.cc @@ -1259,10 +1259,8 @@ ClientRequestContext::clientRedirectDone(const HelperReply &reply) // Put helper response Notes into the transaction state record (ALE) eventually // do it early to ensure that no matter what the outcome the notes are present. if (http->al != NULL) { - if (!http->al->helperNotes) - http->al->helperNotes = new NotePairs; - http->al->helperNotes->append(&reply.notes); - old_request->helperNotes = http->al->helperNotes; + NotePairs ¬es = SyncNotes(*http->al, *old_request); + notes.append(&reply.notes); } switch (reply.result) { @@ -1382,10 +1380,8 @@ ClientRequestContext::clientStoreIdDone(const HelperReply &reply) // Put helper response Notes into the transaction state record (ALE) eventually // do it early to ensure that no matter what the outcome the notes are present. if (http->al != NULL) { - if (!http->al->helperNotes) - http->al->helperNotes = new NotePairs; - http->al->helperNotes->append(&reply.notes); - old_request->helperNotes = http->al->helperNotes; + NotePairs ¬es = SyncNotes(*http->al, *old_request); + notes.append(&reply.notes); } switch (reply.result) { diff --git a/src/format/Format.cc b/src/format/Format.cc index 09d976088d..3b7d34f88a 100644 --- a/src/format/Format.cc +++ b/src/format/Format.cc @@ -1055,15 +1055,8 @@ Format::Format::assemble(MemBuf &mb, const AccessLogEntry::Pointer &al, int logS sb.append(meta); } #endif - if (al->helperNotes != NULL) { - if (const char *note = al->helperNotes->find(fmt->data.string)) { - if (sb.size()) - sb.append(", "); - sb.append(note); - } - } - if (al->configNotes != NULL) { - if (const char *note = al->configNotes->find(fmt->data.string)) { + if (al->notes != NULL) { + if (const char *note = al->notes->find(fmt->data.string)) { if (sb.size()) sb.append(", "); sb.append(note); @@ -1077,10 +1070,9 @@ Format::Format::assemble(MemBuf &mb, const AccessLogEntry::Pointer &al, int logS if (ah != NULL && ah->metaHeaders != NULL && !ah->metaHeaders->empty()) sb.append(ah->metaHeaders->toString()); #endif - if (al->helperNotes != NULL && !al->helperNotes->empty()) - sb.append(al->helperNotes->toString()); - if (al->configNotes != NULL && !al->configNotes->empty()) - sb.append(al->configNotes->toString()); + if (al->notes != NULL && !al->notes->empty()) + sb.append(al->notes->toString()); + out = sb.termedBuf(); quote = 1; }