On 6/04/2013 3:27 a.m., Tsantilas Christos wrote:
This patch try to fix current current Notes interface and usage.
The changes done having in mind that we need:
1) to add multiple notes with the same key
2) to support 3 different note types: adaptation meta headers, helper
notes and custom notes added by the system administrator
3) to log notes using the %note formatting code
4) to use the %note formating code everywhere the formating API is used.
For example use the %note with the request_header_add configuration
parameter.
5) to use notes with ACLs.
The NotePairs class is implemented from scratch. It stores the notes in
a simple key:value pairs list, which just allow multiple entries with
the same key. I have an implementation which uses a
"key:value1,value2,value3..." form but my sense is that the
implementation I am posting here is simpler and faster.
- The %{key}notes prints a coma separated list of note values which have
the key as key. Eg the %{user}note will print:
"J, Smith",chtsanti
- The %notes prints a "\r\n" separated list of key:value pairs:
user: "J, Smith"\r\nuser: chtsanti\r\n
- The %notes formating code can be used now with request_header_add and
other similar configuration parameters.
- The Helper response may needs fixing. For example if the helper return:
user="J, Smith",chtsanti
The user:"J, Smith,chtsanti" note will be stored. This is because of
the strwordtok function used to parse responses.
- We may need to merge the AccessLogEntry::notes and
AccessLogEntry::helperNotes/HttpRequest::helperNotes to the same object.
The adaptation meta headers maybe can be merged to the same object to.
Some details also exist in the patch
Regards,
Christos
Hi Christos, thank you for this.
in src/AccessLogEntry.h:
* If AccessLogEntry::notes and AccessLogEntry::helperNotes are really
necessary to be separate, please name the *::notes on configNotes or
something more descriptive of what it holds.
- Also for my knowledge, can you explain why they are now separate and
in need of re-combining? they started out as a combined list.
in src/ConfigParser.* :
* ConfigParser::QuoteString const-correctness seems unrelated. Please
feel free to apply this change by itself immediately.
in src/HelperReply.cc:
* operator<< line if(!r.notes.empty() > 0) - is very obscure. Please
alter the condition to clarify what exactly is being tested.
+ Probably the " > 0" is not meant to be there.
* please avoid adding new empty line at the end of file
in src/HttpRequest.cc
* we no longer need to check for helperNotes != NULL before setting it
to NULL. Just set.
- same on the copy()
in src/Notes.cc
* Notes::add(const String ¬eKey) - should be able to be
const_iterator still.
NP: thats all I've had time to check for today. More in a few days.
Amos
