[aur-dev] [PATCH] Add a logging function and logging fixins through the code
Signed-off-by: Callan Barrett <wizzomafizzo@gmail.com> --- web/html/pkgsubmit.php | 2 ++ web/lib/acctfuncs.inc | 2 ++ web/lib/aur.inc | 36 ++++++++++++++++++++++++++++++++++++ web/lib/config.inc.proto | 2 ++ web/lib/pkgfuncs.inc | 8 ++++++++ web/template/pkg_comment_form.php | 2 ++ 6 files changed, 52 insertions(+), 0 deletions(-) diff --git a/web/html/pkgsubmit.php b/web/html/pkgsubmit.php index a3394af..04c1542 100644 --- a/web/html/pkgsubmit.php +++ b/web/html/pkgsubmit.php @@ -369,6 +369,7 @@ if ($_COOKIE["AURSID"]): } } + if (LOGGING) { logit(2, username_from_sid($_COOKIE['AURSID']), pkgname_from_id($pdata['ID'])); } header('Location: packages.php?ID=' . $pdata['ID']); } else { @@ -419,6 +420,7 @@ if ($_COOKIE["AURSID"]): } pkg_notify(account_from_sid($_COOKIE["AURSID"]), array($packageID)); + if (LOGGING) { logit(1, username_from_sid($_COOKIE['AURSID']), pkgname_from_id($packageID)); } header('Location: packages.php?ID=' . $packageID); diff --git a/web/lib/acctfuncs.inc b/web/lib/acctfuncs.inc index 9ed4f22..661b081 100644 --- a/web/lib/acctfuncs.inc +++ b/web/lib/acctfuncs.inc @@ -290,6 +290,8 @@ function process_account_form($UTYPE,$TYPE,$A,$U="",$T="",$S="",$E="", } else { # account created/modified, tell them so. # + if (LOGGING) { logit(4, username_from_sid($U)); } + print __("The account, %h%s%h, has been successfully created.", "<b>", $U, "</b>"); print "<p>\n"; diff --git a/web/lib/aur.inc b/web/lib/aur.inc index 517abe3..6a07d69 100644 --- a/web/lib/aur.inc +++ b/web/lib/aur.inc @@ -437,3 +437,39 @@ function mkurl($append) { return substr($out, 5); } + +/** + * Log whatever you like to a table + * + * Basic function so we can get some stats about stuff, we'll just dump + * everything into a table and make up some numbers for different actions + * + * Some Numbers®: + * 1 = upload new package + * 2 = update package + * 3 = delete package + * 4 = register account + * 5 = comment somewhere + * 6 = I dunno lol (I had search here for some reason) + * (maybe we don't need to log this much, I'll make some numbers up anyway) + * 7 = vote/unvote + * 8 = flag/unflag + * 9 = adopt/disown + * + * @param int $action Put your magic number here that represents whatever + * action you're logging + * @param string $who Optional string to log who did the action, normal username + * not the ID + * @param string $pkg Optional string of what package an action happened to, + * actual package names not IDs + */ +function logit($action, $who='', $pkg='') +{ + $dbh = db_connect(); + $q = sprintf("INSERT INTO Log (`action`, `when`, `who`, `pkg`) VALUES (%d, UNIX_TIMESTAMP(), '%s', '%s')", + $action, + mysql_real_escape_string($who), + mysql_real_escape_string($pkg) + ); + db_query($q, $dbh); +} diff --git a/web/lib/config.inc.proto b/web/lib/config.inc.proto index 89b4fe5..6b2b73a 100644 --- a/web/lib/config.inc.proto +++ b/web/lib/config.inc.proto @@ -39,3 +39,5 @@ $SUPPORTED_LANGS = array( # Idle seconds before timeout $LOGIN_TIMEOUT = 7200; +# Shall we log everything? +define("LOGGING", FALSE); diff --git a/web/lib/pkgfuncs.inc b/web/lib/pkgfuncs.inc index 80775a2..699d6e3 100644 --- a/web/lib/pkgfuncs.inc +++ b/web/lib/pkgfuncs.inc @@ -599,6 +599,8 @@ function pkg_flag ($atype, $ids, $action = True) { } else { $flag .= ", " . $pid; } + + if (LOGGING) { logit(8, username_from_sid($_COOKIE['AURSID']), pkgname_from_id($pid)); } } $ood = $action ? 1 : 0; @@ -699,6 +701,8 @@ function pkg_delete ($atype, $ids) { # These are the packages that are safe to delete foreach ($ids_to_delete as $id) { + if (LOGGING) { logit(3, username_from_sid($_COOKIE['AURSID']), pkgname_from_id($id)); } + $q = "DELETE FROM PackageVotes WHERE PackageID = " . $id; $result = db_query($q, $dbh); @@ -757,6 +761,8 @@ function pkg_adopt ($atype, $ids, $action = True) { } else { $pkg .= ", ".$pid; } + + if (LOGGING) { logit(9, username_from_sid($_COOKIE['AURSID']), pkgname_from_id($pid)); } } $field = "MaintainerUID"; @@ -840,6 +846,8 @@ function pkg_vote ($atype, $ids, $action = True) { } } } + + if (LOGGING) { logit(7, username_from_sid($_COOKIE['AURSID']), pkgname_from_id($pid)); } } # only vote for packages the user hasn't already voted for diff --git a/web/template/pkg_comment_form.php b/web/template/pkg_comment_form.php index 70570da..213cc81 100644 --- a/web/template/pkg_comment_form.php +++ b/web/template/pkg_comment_form.php @@ -11,6 +11,8 @@ if (isset($_REQUEST['comment'])) { $q.= 'UNIX_TIMESTAMP())'; db_query($q, $dbh); + if (LOGGING) { logit(5, username_from_sid($_COOKIE['AURSID']), pkgname_from_id(intval($_REQUEST['ID']))); } + # Send email notifications $q = 'SELECT CommentNotify.*, Users.Email '; $q.= 'FROM CommentNotify, Users '; -- 1.6.0.4
On Jul 1, 2009, at 7:27 PM, Callan Barrett wrote:
Signed-off-by: Callan Barrett <wizzomafizzo@gmail.com> --- web/html/pkgsubmit.php | 2 ++ web/lib/acctfuncs.inc | 2 ++ web/lib/aur.inc | 36 ++++++++++++++++++++++++++++ ++++++++ web/lib/config.inc.proto | 2 ++ web/lib/pkgfuncs.inc | 8 ++++++++ web/template/pkg_comment_form.php | 2 ++ 6 files changed, 52 insertions(+), 0 deletions(-)
diff --git a/web/html/pkgsubmit.php b/web/html/pkgsubmit.php index a3394af..04c1542 100644 --- a/web/html/pkgsubmit.php +++ b/web/html/pkgsubmit.php @@ -369,6 +369,7 @@ if ($_COOKIE["AURSID"]): } }
+ if (LOGGING) { logit(2, username_from_sid($_COOKIE['AURSID']),
Might be easier to just put the "if(LOGGING) {}" into the logit() function itself, assuming this is the desired behavior for all instances of logging.
On 02/07/2009, at 1:31 AM, Sebastian Nowicki wrote:
Might be easier to just put the "if(LOGGING) {}" into the logit() function itself, assuming this is the desired behavior for all instances of logging.
I feel like an idiot for not putting it there in the first place but now that I think about it... if logging was turned off we would be doing 2 hits to the DB for almost every single action needlessly if the check was inside the function. So maybe it's not worth the simplicity?
On Jul 1, 2009, at 7:36 PM, Callan Barrett wrote:
On 02/07/2009, at 1:31 AM, Sebastian Nowicki wrote:
Might be easier to just put the "if(LOGGING) {}" into the logit() function itself, assuming this is the desired behavior for all instances of logging.
I feel like an idiot for not putting it there in the first place but now that I think about it... if logging was turned off we would be doing 2 hits to the DB for almost every single action needlessly if the check was inside the function. So maybe it's not worth the simplicity?
Ah, you mean the calls to username_from_sid() and pkgname_from_id()? If they are always called, you could put that in the function as well, and only pass in the parameters ($_COOKIE['AURSID'], $pid), respectively, etc.). Otherwise you have a good point. The current interface for the logit function is better (more flexible) than with the above suggested changes though. I'd probably stick with the redundancy.
On Wed, Jul 1, 2009 at 12:42 PM, Sebastian Nowicki<sebnow@gmail.com> wrote:
On Jul 1, 2009, at 7:36 PM, Callan Barrett wrote:
On 02/07/2009, at 1:31 AM, Sebastian Nowicki wrote:
Might be easier to just put the "if(LOGGING) {}" into the logit() function itself, assuming this is the desired behavior for all instances of logging.
I feel like an idiot for not putting it there in the first place but now that I think about it... if logging was turned off we would be doing 2 hits to the DB for almost every single action needlessly if the check was inside the function. So maybe it's not worth the simplicity?
Ah, you mean the calls to username_from_sid() and pkgname_from_id()? If they are always called, you could put that in the function as well, and only pass in the parameters ($_COOKIE['AURSID'], $pid), respectively, etc.). Otherwise you have a good point.
The current interface for the logit function is better (more flexible) than with the above suggested changes though. I'd probably stick with the redundancy.
I'd move the two function calls into logit() as well, as it looks like they are always called. Why are we inserting username when userid should be the foreign key we use to reference users? I guess if you aren't a fan of joins it makes sense, but DB normalization does have benefits, especially when you decided to use integers anyway to represent the different operations. The same holds true for package IDs, why not just use the IDs directly and not names? Getting the same end result table (with usernames and pkgnames) would be only a matter of two simple joins, not hard for a relational DB to do. In addition, magic numbers are not so much fun and make it hard to decipher the calls at first glance. Maybe some sort of associative array could help here, declared near the logit() function? $logit_actions = array( 'upload'=>1, 'update'=>2, ... ); And then a call would end up looking like: logit(logit_actions('upload'), $userID, $packageID); -Dan
On Thu, Jul 2, 2009 at 1:51 AM, Dan McGee<dpmcgee@gmail.com> wrote:
On Wed, Jul 1, 2009 at 12:42 PM, Sebastian Nowicki<sebnow@gmail.com> wrote:
On Jul 1, 2009, at 7:36 PM, Callan Barrett wrote:
On 02/07/2009, at 1:31 AM, Sebastian Nowicki wrote:
Might be easier to just put the "if(LOGGING) {}" into the logit() function itself, assuming this is the desired behavior for all instances of logging.
I feel like an idiot for not putting it there in the first place but now that I think about it... if logging was turned off we would be doing 2 hits to the DB for almost every single action needlessly if the check was inside the function. So maybe it's not worth the simplicity?
Ah, you mean the calls to username_from_sid() and pkgname_from_id()? If they are always called, you could put that in the function as well, and only pass in the parameters ($_COOKIE['AURSID'], $pid), respectively, etc.). Otherwise you have a good point.
The current interface for the logit function is better (more flexible) than with the above suggested changes though. I'd probably stick with the redundancy.
I'd move the two function calls into logit() as well, as it looks like they are always called.
Why are we inserting username when userid should be the foreign key we use to reference users? I guess if you aren't a fan of joins it makes sense, but DB normalization does have benefits, especially when you decided to use integers anyway to represent the different operations.
The same holds true for package IDs, why not just use the IDs directly and not names? Getting the same end result table (with usernames and pkgnames) would be only a matter of two simple joins, not hard for a relational DB to do.
In addition, magic numbers are not so much fun and make it hard to decipher the calls at first glance. Maybe some sort of associative array could help here, declared near the logit() function?
$logit_actions = array( 'upload'=>1, 'update'=>2, ... );
And then a call would end up looking like: logit(logit_actions('upload'), $userID, $packageID);
-Dan
When I wrote this it was just gonna be a slapped together function we use to get some stats before the planned rewrite. You've made me feel guilty for not putting any thought into it now, I must rewrite this patch! -- Callan Barrett
participants (3)
-
Callan Barrett
-
Dan McGee
-
Sebastian Nowicki