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

@@ -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

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 {

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.

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.

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=?');

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;
-
-