Helpers and Notes

Hi all,

While we are working in a project we seen that the helpers do wrong
usage of the Notes interface.

This is maybe it is our mistake (and mostly my mistake) because we do
not have make good documentation of this interface.
In any case I believe that we should fix some thinks.

In the Notes.h file there are the following classes:

class Note
-----------

This is a class used to store a note configuration not a note it self.
It is a note name plus a list of possible values with an ACL list in
each value.
It is used to store configurations for the following cfg lines:
  note key value acl1 acl2

class Notes
------------
It is used to store a list of Notes. A list like the following:
  note key valueA acl1a acl2a
  note key valueB acl1b acl2b

This is used to add custom notes (or metaHeaders) in an HTTP request

class NotePairs
-----------------
This is an HttpHeader kid class used to store the notes (or
MetaHeaders). We select HttpHeader class because it has some interesting
features. For example assume that two different subsystems add the same
note:
 X-SQUID-ProcessedBy: ReWriter
 X-SQUID-ProcessedBy: Adaptation

Then this is will be merged to
 X-SQUID-ProcessedBy: ReWriter, Adaptation

Squid helper Notes problems
-------------------------------
The changes made in trunk revno:12490 and revno:12495

Problems I am seeing:
 - it uses class Notes to store the notes/metaHeaders not the NotePairs.
It adds the "Notes HelperReply:notes" and the "Notes
HttpRequest::helperNotes" members.

 - It adds some extra methods which does not make sense and confuse more
the Notes interface:
       * NotePairs::append(const Notes::NotesList &src)
       * NotePairs::append(const NotePairs *src)
  These functions used to add a Notes list to a NotePairs List. The
HttpRequest::helperNotes to AccessLogEntry::notes.
But It is easier to use NotePairs for HttpRequest::helperNotes member
and just use the (pre-)existing NotePairs::append (the HttpHeaders::append)

      * Notes::add(const String &noteKey, const String &noteValue);
      * Notes::add(const Notes &src);
      * Notes::find(const String &noteKey)
    To add and search for a note in Notes List. But it is wrong. We do
not store such Notes here. We are storing Notes configuration, while
parsing the cfg file.
We should use NotePairs for HttpRequest::helperNotes and
HelperReply::notes members and use the NotePairs::putExt or
NotePairs::addEntry(new HttpHeaderEntry(HDR_OTHER, name, value)); to
adding note/value pairs.

 - Initially we had select the AccessLogEntry to store notes. The
AccessLogEntry selected because it considered as good structure to hold
HTTP request X-data. (But we had problem on this).
I am not sure if the HttpRequest::helperNotes is the correct position to
add Notes. We should consider if we can add notes to
AccessLogEntry::notes, or select an other X-data structure.

I am working on changing the above problems. Also maybe we need to
change the names of some classes to avoid confusion in the future, for
example rename Note and Notes to NoteRule and NoteRules or similar.

Any opinions on the above?

Regards,
   Christos
Received on Fri Jan 25 2013 - 21:40:04 MST