From 30e869b66d7c01628d8fede97ec53688a843ed9e Mon Sep 17 00:00:00 2001 From: Colin Kuskie Date: Mon, 7 Jun 2010 10:44:12 -0700 Subject: [PATCH] Better cleanup of Inbox messages. Encapsulate all SQL in Inbox/Message. Fix workflow. Upgrade script to cleanup state table. Fixes bug #11595. --- docs/changelog/7.x.x.txt | 1 + docs/upgrades/upgrade_7.9.6-7.9.7.pl | 17 ++++++++ lib/WebGUI/Inbox.pm | 22 +++++++++++ lib/WebGUI/Inbox/Message.pm | 19 ++++++++- lib/WebGUI/User.pm | 4 +- .../Activity/PurgeOldInboxMessages.pm | 39 +++++++------------ 6 files changed, 72 insertions(+), 30 deletions(-) diff --git a/docs/changelog/7.x.x.txt b/docs/changelog/7.x.x.txt index e0f674ec3..a9861083e 100644 --- a/docs/changelog/7.x.x.txt +++ b/docs/changelog/7.x.x.txt @@ -7,6 +7,7 @@ - fixed #11611: Thingy: The add field pop up box has multiple "Text" field types - fixed #11610: Bad hover help for CS sortBy field - fixed #11605: UserList refers to non-existent "publicEmail" user profiling field + - fixed #11595: Orphaned data in inbox_messageState 7.9.6 - new checkbox in the asset manager for clearing the package flag on import diff --git a/docs/upgrades/upgrade_7.9.6-7.9.7.pl b/docs/upgrades/upgrade_7.9.6-7.9.7.pl index a87756f37..ea35f7b0a 100644 --- a/docs/upgrades/upgrade_7.9.6-7.9.7.pl +++ b/docs/upgrades/upgrade_7.9.6-7.9.7.pl @@ -37,6 +37,7 @@ my $session = start(); # this line required # upgrade functions go here restoreDefaultCronJobs($session); restoreCsCronJobs($session); +cleanup_inbox_messageStateTable($session); finish($session); # this line required @@ -50,6 +51,22 @@ finish($session); # this line required # print "DONE!\n" unless $quiet; #} +#---------------------------------------------------------------------------- +# Describe what our function does +sub cleanup_inbox_messageStateTable { + my $session = shift; + print "\tDelete dead entries from the inbox_MessageState table. This may take a long time... " unless $quiet; + # and here's our code + my $source = $session->db->read("select messageId from inbox_messageState s where not exists(select messageId from inbox where messageId = s.messageId)"); + my $cleaner = $session->db->prepare("delete from inbox_messageState where messageId=?"); + while (my ($messageId) = $source->array) { + $cleaner->execute([$messageId]); + } + $source->finish; + $cleaner->finish; + print "DONE!\n" unless $quiet; +} + #---------------------------------------------------------------------------- # Describe what our function does sub restoreDefaultCronJobs { diff --git a/lib/WebGUI/Inbox.pm b/lib/WebGUI/Inbox.pm index 201ff5330..6b5e4f5d2 100644 --- a/lib/WebGUI/Inbox.pm +++ b/lib/WebGUI/Inbox.pm @@ -132,6 +132,28 @@ sub DESTROY { #------------------------------------------------------------------- +=head2 deleteMessagesForUser ( $user ) + +Deletes all messages for a user. + +=head3 $user + +A WebGUI::User object, representing the user who will have all their messages deleted. + +=cut + +sub deleteMessagesForUser { + my $self = shift; + my $user = shift; + + my $db = $self->session->db; + my $userId = $user->userId; + $db->write("DELETE FROM inbox_messageState WHERE userId=?",[$userId]); + $db->write("DELETE FROM inbox WHERE userId=? AND (groupId IS NULL OR groupId='')",[$userId]); +} + +#------------------------------------------------------------------- + =head2 getMessage ( messageId [, userId] ) Returns a WebGUI::Inbox::Message object. diff --git a/lib/WebGUI/Inbox/Message.pm b/lib/WebGUI/Inbox/Message.pm index bcbf13d5b..f06c58be0 100644 --- a/lib/WebGUI/Inbox/Message.pm +++ b/lib/WebGUI/Inbox/Message.pm @@ -266,8 +266,7 @@ sub delete { ); #Delete the message from the database if everyone who was sent the message has deleted it unless ($isActive) { - $db->write("delete from inbox where messageId=?",[$messageId]); - $db->write("delete from inbox_messageState where messageId=?",[$messageId]); + $self->purge; } } @@ -450,6 +449,22 @@ sub new { #------------------------------------------------------------------- +=head2 purge + +Completely deletes a message from the inbox. + +=cut + +sub purge { + my $self = shift; + my $db = $self->session->db; + my $messageId = $self->getId; + $db->write("delete from inbox where messageId=?",[$messageId]); + $db->write("delete from inbox_messageState where messageId=?",[$messageId]); +} + +#------------------------------------------------------------------- + =head2 session Returns a reference to the current session. diff --git a/lib/WebGUI/User.pm b/lib/WebGUI/User.pm index c13ed19f7..283944ed4 100644 --- a/lib/WebGUI/User.pm +++ b/lib/WebGUI/User.pm @@ -411,8 +411,8 @@ sub delete { $db->write("DELETE FROM userSession WHERE userId=?",[$userId]); # remove inbox entries - $db->write("DELETE FROM inbox_messageState WHERE userId=?",[$userId]); - $db->write("DELETE FROM inbox WHERE userId=? AND (groupId IS NULL OR groupId='')",[$userId]); + my $inbox = WebGUI::Inbox->new($session); + $inbox->deleteMessagesForUser($self); # Shop cleanups my $sth = $session->db->prepare('select addressBookId from addressBook where userId=?'); diff --git a/lib/WebGUI/Workflow/Activity/PurgeOldInboxMessages.pm b/lib/WebGUI/Workflow/Activity/PurgeOldInboxMessages.pm index 06bb845cf..d9c1211b1 100644 --- a/lib/WebGUI/Workflow/Activity/PurgeOldInboxMessages.pm +++ b/lib/WebGUI/Workflow/Activity/PurgeOldInboxMessages.pm @@ -17,7 +17,7 @@ package WebGUI::Workflow::Activity::PurgeOldInboxMessages; use strict; use base 'WebGUI::Workflow::Activity'; -use WebGUI::Asset; +use WebGUI::Inbox::Message; =head1 NAME @@ -77,45 +77,32 @@ See WebGUI::Workflow::Activity::execute() for details. =cut sub execute { - my ($self, $nothing, $instance) = @_; + my ($self, undef, $instance) = @_; my $session = $self->session; - my $log = $session->errorHandler; # keep track of how much time it's taking - my $start = time; - my $limit = 2_500; + my $endTime = time() + $self->getTTL;; my $sth = $session->db->read( "SELECT messageId FROM inbox WHERE completedOn IS NOT NULL AND dateStamp < ?", - [ $start - $self->get('purgeAfter') ], - ); - - while ( ( my $messageId ) = $sth->array ) { - $session->db->write( - "DELETE FROM inbox WHERE messageId = ?", - [ $messageId ], + [ time() - $self->get('purgeAfter') ], ); + MESSAGE: while ( ( my $messageId ) = $sth->array ) { # give up if we're taking too long - if (time - $start > 120) { + if (time() > $endTime) { $sth->finish; return $self->WAITING(1); - } + } + + my $message = WebGUI::Inbox::Message->new($session, $messageId); + next MESSAGE unless $message; + $message->purge; } - # If there are more messages waiting to be purged, return WAITING - if ( $sth->rows >= $limit ) { - return $self->WAITING(1); - } - else { - return $self->COMPLETE; - } + $sth->finish; + return $self->COMPLETE; } - - - 1; - -