[Rt-commit] rt branch, 4.4-trunk, updated. rt-4.4.1-290-g8092ced

Shawn Moore shawn at bestpractical.com
Wed Jan 25 17:22:17 EST 2017


The branch, 4.4-trunk has been updated
       via  8092ced0bd760f2f19c974603cb8ab17e87c7f55 (commit)
       via  20d8daf6855e3c53ee8a79d68271941d4cdca159 (commit)
       via  58bacce6ada754657c7f56fd91f20c573108c1ab (commit)
       via  8862bd94d3ddc84ac858f36cff2a032e51fdf67c (commit)
       via  06925eda7483a43612d7e1db66cb6ba4a63b2a3c (commit)
       via  04aeb4cca1f0c1f4dfd86adfd18705e0e5068ebd (commit)
       via  6c0cf77f50182b0cdaf56f7d7b199db7a8a94e66 (commit)
       via  18a88f4779ff9639b66e18281b111cdfe73d42b8 (commit)
       via  efe128e2892b74bd5f421a1d954ab53cf0ce3775 (commit)
       via  01b4ee497cd9506b0ec961097de5b7a45b376054 (commit)
       via  9c3931075686470e67e4f541c855c4b169c25ec7 (commit)
       via  4701ed9e44dd96dd39dcf83211881479d18abc14 (commit)
       via  db0a61d9186bd9b499d4d89ab1d406d0302e8be9 (commit)
      from  7a52de265be16c8d479e7aabc79484e558232de1 (commit)

Summary of changes:
 etc/upgrade/4.4.2/content           |  36 +++++++++++
 lib/RT/Article.pm                   |   4 +-
 lib/RT/CachedGroupMember.pm         |  26 ++++----
 lib/RT/CustomField.pm               |  23 +++----
 lib/RT/Group.pm                     |  50 +++++----------
 lib/RT/GroupMember.pm               |  23 +++----
 lib/RT/Record.pm                    |   5 +-
 lib/RT/Ticket.pm                    | 124 +++++++++++++++++++++++++++---------
 sbin/rt-validator.in                |  84 +++++++++++++++++++++++-
 share/html/Search/Bulk.html         |   4 ++
 share/html/SelfService/Display.html |  34 +++++-----
 share/html/Ticket/Display.html      |  22 ++++---
 share/html/Ticket/Modify.html       |  14 ++--
 share/html/Ticket/ModifyAll.html    |  16 ++---
 share/html/Ticket/ModifyDates.html  |   7 +-
 share/html/Ticket/ModifyLinks.html  |  11 ++--
 share/html/Ticket/ModifyPeople.html |  10 +--
 share/html/Ticket/Reminders.html    |   4 +-
 share/html/m/ticket/show            |  61 +++++++++---------
 t/ticket/race.t                     |  76 +++++++++++++++++++++-
 20 files changed, 437 insertions(+), 197 deletions(-)

- Log -----------------------------------------------------------------
commit 58bacce6ada754657c7f56fd91f20c573108c1ab
Author: Shawn M Moore <shawn at bestpractical.com>
Date:   Thu Jan 19 18:45:45 2017 +0000

    rt-validator rules for Ticket.Owner = Owner GroupMembers
    
    The first rule catches the case where Owner GroupMembers is empty. Its
    fixup step adds the denormalized Owner as GroupMember using RT's usual
    API (instead of direct SQL statements) since that will create both GM
    and CGM records.
    
    The second rule catches the case, exposed by the race condition fixed in
    the previous commits, where the Owner GroupMember doesn't match the
    denormalized Ticket.Owner column. Its fixup step removes all the Owner
    GroupMember records (avoiding Group->_DeleteMember, which would add
    Nobody since Owner as a single-member role group) and then adds the
    denormalized Ticket.Owner as the sole group member. This was decided to
    be the most prudent fixup, both because the denormalized Ticket.Owner is
    what is displayed in the UI, and because in the case of multiple group
    members, there's no way to choose which should be canonical.
    
    Together, along with the traditional checks to verify that there exists
    an Owner role group for each ticket, as well as CachedGroupMembers
    matching GroupMembers, ought to cover all drift between the denormalized
    Ticket.Owner field and the normalized Owner role group GroupMembers.

diff --git a/sbin/rt-validator.in b/sbin/rt-validator.in
index 340d865..c7e9949 100644
--- a/sbin/rt-validator.in
+++ b/sbin/rt-validator.in
@@ -636,7 +636,89 @@ push @CHECKS, 'Tickets -> other' => sub {
              update_records( 'Tickets', { id => $id, Owner => $prop{Owner} }, { Owner => $replace_with } );
          },
     );
-    # XXX: check that owner is only member of owner role group
+
+    # there exists a GroupMember for each ticket Owner role group
+    {
+        my $query = <<END;
+SELECT Tickets.Id, Tickets.Owner
+FROM Groups
+JOIN Tickets ON Tickets.Id = Groups.Instance
+LEFT JOIN GroupMembers ON Groups.Id = GroupMembers.GroupId
+WHERE Groups.Name = 'Owner'
+AND Groups.Domain = 'RT::Ticket-Role'
+AND GroupMembers.MemberId IS NULL;
+END
+
+        my $action = sub {
+            my ($ticket_id, $owner_id) = @_;
+            return unless prompt(
+                'Create',
+                "Found a ticket that has no normalized Owner group member."
+            );
+            my $ticket = RT::Ticket->new(RT->SystemUser);
+            $ticket->Load($ticket_id);
+            my $group = $ticket->RoleGroup('Owner');
+            $group->_AddMember(
+                PrincipalId       => $owner_id,
+                InsideTransaction => 0,
+                RecordTransaction => 0,
+                Object            => $ticket,
+            );
+        };
+        my $sth = execute_query( $query );
+        while ( my ($ticket, $owner) = $sth->fetchrow_array ) {
+            $res = 0;
+            print STDERR "The owner of ticket #$ticket is inconsistent. The denormalized owner is user #$owner\n";
+            print STDERR "but there is no normalized owner.\n";
+            $action->($ticket, $owner);
+        }
+    }
+
+    # Tickets.Owner = OwnerRoleGroup.MemberId
+    {
+        my $query = <<END;
+SELECT Tickets.Id, Tickets.Owner, GroupMembers.MemberId
+FROM Groups
+JOIN GroupMembers ON Groups.Id = GroupMembers.GroupId
+JOIN Tickets ON Tickets.Id = Groups.Instance
+WHERE Groups.Name = 'Owner'
+AND Groups.Domain = 'RT::Ticket-Role'
+AND Tickets.Owner != GroupMembers.MemberId;
+END
+
+        my $action = sub {
+            my ($ticket_id, $owner_id) = @_;
+            return unless prompt(
+                'Replace',
+                "Found a ticket that has inconsistent normalized Owner group member."
+            );
+            my $ticket = RT::Ticket->new(RT->SystemUser);
+            $ticket->Load($ticket_id);
+            my $group = $ticket->RoleGroup('Owner');
+
+            # remove all current members
+            my $members = $group->MembersObj;
+            while (my $member = $members->Next) {
+                $member->Delete;
+            }
+
+            # add correct member
+            $group->_AddMember(
+                PrincipalId       => $owner_id,
+                InsideTransaction => 0,
+                RecordTransaction => 0,
+                Object            => $ticket,
+            );
+        };
+        my $sth = execute_query( $query );
+        while ( my ($ticket, $owner, $rolemember) = $sth->fetchrow_array ) {
+            $res = 0;
+            print STDERR "The owner of ticket #$ticket is inconsistent. The denormalized owner is user #$owner\n";
+            print STDERR "but there exists a normalized owner role group member, user #$rolemember.\n";
+            $action->($ticket, $owner);
+        }
+    }
+
     return $res;
 };
 

commit 20d8daf6855e3c53ee8a79d68271941d4cdca159
Author: Shawn M Moore <shawn at bestpractical.com>
Date:   Wed Jan 25 21:31:40 2017 +0000

    Add 4.4.2 upgrade step for fixing Ticket.Owner != Owner GroupMembers
    
    This upgrade step mirrors the implementation in rt-validator introduced
    and described in the previous commit.
    
    Though the bug nominally affected only MySQL, there's little reason to
    limit the fixup to just MySQL.

diff --git a/etc/upgrade/4.4.2/content b/etc/upgrade/4.4.2/content
new file mode 100644
index 0000000..3ee9576
--- /dev/null
+++ b/etc/upgrade/4.4.2/content
@@ -0,0 +1,42 @@
+use strict;
+use warnings;
+
+our @Initial = (
+    # fix up inconsistent denormalized owner vs owner-role group members (#32381)
+    sub {
+        my $sth = RT->DatabaseHandle->dbh->prepare(q[
+            SELECT Tickets.Id, Tickets.Owner, GroupMembers.MemberId
+            FROM Groups
+            JOIN GroupMembers ON Groups.Id = GroupMembers.GroupId
+            JOIN Tickets ON Tickets.Id = Groups.Instance
+            WHERE Groups.Name = 'Owner'
+            AND Groups.Domain = 'RT::Ticket-Role'
+            AND Tickets.Owner != GroupMembers.MemberId;
+        ]);
+
+        $sth->execute;
+
+        while ( my ($ticket_id, $owner_id, $rolemember_id) = $sth->fetchrow_array ) {
+            RT->Logger->warning("The owner of ticket #$ticket_id is inconsistent. The denormalized owner is user #$owner_id, but there exists a normalized owner role group member, user #$rolemember_id. Going to update normalized owner role group members to user #$owner_id.");
+
+            my $ticket = RT::Ticket->new(RT->SystemUser);
+            $ticket->Load($ticket_id);
+            my $group = $ticket->RoleGroup('Owner');
+
+            # remove all current members
+            my $members = $group->MembersObj;
+            while (my $member = $members->Next) {
+                $member->Delete;
+            }
+
+            # add correct member
+            $group->_AddMember(
+                PrincipalId       => $owner_id,
+                InsideTransaction => 0,
+                RecordTransaction => 0,
+                Object            => $ticket,
+            );
+        }
+    },
+);
+

commit 8092ced0bd760f2f19c974603cb8ab17e87c7f55
Merge: 7a52de2 20d8daf
Author: Shawn M Moore <shawn at bestpractical.com>
Date:   Wed Jan 25 22:09:01 2017 +0000

    Merge branch '4.4/previewscrips-race' into 4.4-trunk

diff --cc etc/upgrade/4.4.2/content
index 360f134,3ee9576..7f86a48
--- a/etc/upgrade/4.4.2/content
+++ b/etc/upgrade/4.4.2/content
@@@ -2,21 -2,40 +2,57 @@@ use strict
  use warnings;
  
  our @Initial = (
 +    # fix searches without SearchType
 +    sub {
 +        my $attrs = RT::Attributes->new(RT->SystemUser);
 +        $attrs->Limit( FIELD => 'Name', VALUE => 'SavedSearch' );
 +        while ( my $attr = $attrs->Next ) {
 +            my $content = $attr->Content;
 +
 +            next if $content->{SearchType};
 +
 +            $content->{SearchType} = 'Ticket';
 +
 +            my ($ret, $msg) = $attr->SetContent($content);
 +            unless ( $ret ) {
 +                RT->Logger->error("Failed to update content for SavedSearch #" . $attr->id . ": $msg");
 +            }
++    },
++
+     # fix up inconsistent denormalized owner vs owner-role group members (#32381)
+     sub {
+         my $sth = RT->DatabaseHandle->dbh->prepare(q[
+             SELECT Tickets.Id, Tickets.Owner, GroupMembers.MemberId
+             FROM Groups
+             JOIN GroupMembers ON Groups.Id = GroupMembers.GroupId
+             JOIN Tickets ON Tickets.Id = Groups.Instance
+             WHERE Groups.Name = 'Owner'
+             AND Groups.Domain = 'RT::Ticket-Role'
+             AND Tickets.Owner != GroupMembers.MemberId;
+         ]);
+ 
+         $sth->execute;
+ 
+         while ( my ($ticket_id, $owner_id, $rolemember_id) = $sth->fetchrow_array ) {
+             RT->Logger->warning("The owner of ticket #$ticket_id is inconsistent. The denormalized owner is user #$owner_id, but there exists a normalized owner role group member, user #$rolemember_id. Going to update normalized owner role group members to user #$owner_id.");
+ 
+             my $ticket = RT::Ticket->new(RT->SystemUser);
+             $ticket->Load($ticket_id);
+             my $group = $ticket->RoleGroup('Owner');
+ 
+             # remove all current members
+             my $members = $group->MembersObj;
+             while (my $member = $members->Next) {
+                 $member->Delete;
+             }
+ 
+             # add correct member
+             $group->_AddMember(
+                 PrincipalId       => $owner_id,
+                 InsideTransaction => 0,
+                 RecordTransaction => 0,
+                 Object            => $ticket,
+             );
          }
      },
  );

-----------------------------------------------------------------------


More information about the rt-commit mailing list