Cleanup: Merge AccessLogEntry 'helperNotes' and 'configNotes' members to 'notes'...

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 bba7d3d59a519688ae95f33ec98b8ae5c79586bb..698a41c5b09887baf0c449cc7097c2153c28e82b 100644 (file)
--- 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 f718da1bdfdc1b7f49832da931c898ec2c15b1a6..20de12899b4546cafcda32afe468261f4acde715 100644 (file)
--- 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 d5c712e2924dadc46af5f9ac5a78880372e37422..9a768709bb45fef543f5f92843f8b3dc515b14dc 100644 (file)
--- 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 4101c5b9751351d492cf4a5a96daaa8684740b96..68a5c688fa2259c013262731ea51ea35e55e6edc 100644 (file)
--- 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 108cdcc1f55f1e2f4e6c44a24a7fce27c3f66b3f..7fdcb9f895ee019d6b3526c9741ca3ff0c14d33b 100644 (file)
--- 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 18eadc8912d9efb383c90100554bfc5923bc4f94..2b9341a2b5da3da60c49ee69d6a09f936664407d 100644 (file)
--- 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 &notes = 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 4d5054a97d10047390c757e7875aac9715859c30..c884fc725db4d549a6044d653d18703576300134 100644 (file)
--- 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 &notes = 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 &notes = 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 09d976088d11d3a56a513393889c1349601d15c8..3b7d34f88aa8621d3faccb65eb6d109ac3c103c5 100644 (file)
--- 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;
             }