[Rt-commit] rt branch, 4.4/serialize-json-initialdata, updated. rt-4.4.1-406-gc8bc700

Shawn Moore shawn at bestpractical.com
Wed Mar 22 16:14:14 EDT 2017


The branch, 4.4/serialize-json-initialdata has been updated
       via  c8bc700e822c318f7113c6a46e73764f6572e932 (commit)
       via  79988883aed3263df2e8e0f5abd34a3da422d90e (commit)
       via  b8807bf6aa567284bb94d20f9142df302c36b832 (commit)
       via  94d916f3136f847ddf652cefb2454b60094347cd (commit)
       via  77790ae718646d1f364d642d2c97898431b0ad00 (commit)
      from  480b8f0f1b0e8d302a888982133cb57df9c87b85 (commit)

Summary of changes:
 lib/RT/ACE.pm                     |  5 +++
 lib/RT/Handle.pm                  |  8 +++++
 lib/RT/Migrate/Serializer.pm      |  8 +++--
 lib/RT/Migrate/Serializer/JSON.pm |  3 +-
 t/api/initialdata-roundtrip.t     | 70 +++++++++++++++++++++++++++++++--------
 5 files changed, 77 insertions(+), 17 deletions(-)

- Log -----------------------------------------------------------------
commit 77790ae718646d1f364d642d2c97898431b0ad00
Author: Shawn M Moore <shawn at bestpractical.com>
Date:   Wed Mar 22 19:28:34 2017 +0000

    Avoid error on broken ACLEquivalence principals
    
    If you've deleted users directly from the database but haven't deleted
    their ACLEquivalence principal or group, then this was exploding trying
    to load a principal for an empty RT::User object

diff --git a/lib/RT/Migrate/Serializer.pm b/lib/RT/Migrate/Serializer.pm
index 7907d3a..b188aa7 100644
--- a/lib/RT/Migrate/Serializer.pm
+++ b/lib/RT/Migrate/Serializer.pm
@@ -390,7 +390,8 @@ sub Process {
 
             # [issues.bestpractical.com #32662]
             return if $principal->Object->Domain eq 'ACLEquivalence'
-                   && $principal->Object->InstanceObj->Disabled;
+                   && (!$principal->Object->InstanceObj->Id
+                     || $principal->Object->InstanceObj->Disabled);
 
             return if !$obj->Object->isa('RT::System')
                    && $obj->Object->Disabled;
@@ -434,8 +435,9 @@ sub Observe {
             return 0 if $principal->Disabled;
 
             # [issues.bestpractical.com #32662]
-            return 0 if $principal->Object->Domain eq 'ACLEquivalence'
-                     && $principal->Object->InstanceObj->Disabled;
+            return if $principal->Object->Domain eq 'ACLEquivalence'
+                   && (!$principal->Object->InstanceObj->Id
+                     || $principal->Object->InstanceObj->Disabled);
 
             return 0 if !$obj->Object->isa('RT::System')
                      && $obj->Object->Disabled;

commit 94d916f3136f847ddf652cefb2454b60094347cd
Author: Shawn M Moore <shawn at bestpractical.com>
Date:   Wed Mar 22 19:42:21 2017 +0000

    Avoid undef warnings on CFs without a pattern

diff --git a/lib/RT/Migrate/Serializer/JSON.pm b/lib/RT/Migrate/Serializer/JSON.pm
index 4e66b25..7b26196 100644
--- a/lib/RT/Migrate/Serializer/JSON.pm
+++ b/lib/RT/Migrate/Serializer/JSON.pm
@@ -359,7 +359,7 @@ sub CanonicalizeCustomFields {
     my $self = shift;
 
     for my $record (values %{ $self->{Records}{'RT::CustomField'} }) {
-        delete $record->{Pattern} if $record->{Pattern} eq "";
+        delete $record->{Pattern} if ($record->{Pattern}||'') eq "";
         delete $record->{UniqueValues} if !$record->{UniqueValues};
     }
 }

commit b8807bf6aa567284bb94d20f9142df302c36b832
Author: Shawn M Moore <shawn at bestpractical.com>
Date:   Wed Mar 22 19:58:59 2017 +0000

    Skip serializing OCFVs on live objects for disabled CFs
    
    Needed to create a new queue in this test because General's CFs were
    ignored, since that queue already exists

diff --git a/lib/RT/Migrate/Serializer/JSON.pm b/lib/RT/Migrate/Serializer/JSON.pm
index 7b26196..abc6e65 100644
--- a/lib/RT/Migrate/Serializer/JSON.pm
+++ b/lib/RT/Migrate/Serializer/JSON.pm
@@ -371,6 +371,7 @@ sub CanonicalizeObjectCustomFieldValues {
         my $object = $self->_GetSerializedByRef(delete $record->{Object});
 
         my $cf = $self->_GetSerializedByRef(delete $record->{CustomField});
+        next unless $cf && $cf->{Name}; # disabled CF on live object
         $record->{CustomField} = $cf->{Name};
 
         delete @$record{qw/id/};
diff --git a/t/api/initialdata-roundtrip.t b/t/api/initialdata-roundtrip.t
index 915b5cf..fb31356 100644
--- a/t/api/initialdata-roundtrip.t
+++ b/t/api/initialdata-roundtrip.t
@@ -470,8 +470,14 @@ my @tests = (
     {
         name => 'Disabled many-to-many relationships',
         create => sub {
+            my $enabled_queue = RT::Queue->new(RT->SystemUser);
+            my ($ok, $msg) = $enabled_queue->Create(
+                Name => 'Enabled Queue',
+            );
+            ok($ok, $msg);
+
             my $disabled_queue = RT::Queue->new(RT->SystemUser);
-            my ($ok, $msg) = $disabled_queue->Create(
+            ($ok, $msg) = $disabled_queue->Create(
                 Name => 'Disabled Queue',
             );
             ok($ok, $msg);
@@ -480,7 +486,7 @@ my @tests = (
             ($ok, $msg) = $enabled_cf->Create(
                 Name => 'Enabled CF',
                 Type => 'FreeformSingle',
-                LookupType => RT::Ticket->CustomFieldLookupType,
+                LookupType => RT::Queue->CustomFieldLookupType,
             );
             ok($ok, $msg);
 
@@ -488,7 +494,7 @@ my @tests = (
             ($ok, $msg) = $disabled_cf->Create(
                 Name => 'Disabled CF',
                 Type => 'FreeformSingle',
-                LookupType => RT::Ticket->CustomFieldLookupType,
+                LookupType => RT::Queue->CustomFieldLookupType,
             );
             ok($ok, $msg);
 
@@ -568,7 +574,7 @@ my @tests = (
                             $enabled_role, $disabled_role) {
 
                 # slightly inconsistent API
-                my ($queue_a, $queue_b) = ($disabled_queue, $general);
+                my ($queue_a, $queue_b) = ($disabled_queue, $enabled_queue);
                 ($queue_a, $queue_b) = ($queue_a->Id, $queue_b->Id)
                     if $object->isa('RT::Scrip')
                     || $object->isa('RT::CustomRole');
@@ -585,7 +591,7 @@ my @tests = (
                 ($ok, $msg) = $principal->PrincipalObj->GrantRight(Object => RT->System, Right => 'SeeQueue');
                 ok($ok, $msg);
 
-                for my $queue ($general, $disabled_queue) {
+                for my $queue ($enabled_queue, $disabled_queue) {
                     ($ok, $msg) = $principal->PrincipalObj->GrantRight(Object => $queue, Right => 'ShowTicket');
                     ok($ok, $msg);
 
@@ -594,6 +600,13 @@ my @tests = (
                 }
             }
 
+            for my $cf ($enabled_cf, $disabled_cf) {
+                for my $queue ($enabled_queue, $disabled_queue) {
+                    ($ok, $msg) = $queue->AddCustomFieldValue(Field => $cf->Id, Value => $cf->Name);
+                    ok($ok, $msg);
+                }
+            }
+
             for my $object ($disabled_queue, $disabled_cf,
                             $disabled_scrip, $disabled_class,
                             $disabled_role, $disabled_group,
@@ -605,6 +618,11 @@ my @tests = (
         present => sub {
             my $from_initialdata = shift;
 
+            my $enabled_queue = RT::Queue->new(RT->SystemUser);
+            $enabled_queue->Load('Enabled Queue');
+            ok($enabled_queue->Id, 'loaded Enabled queue');
+            is($enabled_queue->Name, 'Enabled Queue', 'Enabled Queue Name');
+
             my $disabled_queue = RT::Queue->new(RT->SystemUser);
             $disabled_queue->Load('Disabled Queue');
 
@@ -612,7 +630,9 @@ my @tests = (
             $enabled_cf->Load('Enabled CF');
             ok($enabled_cf->Id, 'loaded Enabled CF');
             is($enabled_cf->Name, 'Enabled CF', 'Enabled CF Name');
-            ok($enabled_cf->IsAdded($general->Id), 'Enabled CF added to General');
+            ok($enabled_cf->IsAdded($enabled_queue->Id), 'Enabled CF added to General');
+
+            is($enabled_queue->FirstCustomFieldValue('Enabled CF'), 'Enabled CF', 'OCFV');
 
             my $disabled_cf = RT::CustomField->new(RT->SystemUser);
             $disabled_cf->Load('Disabled CF');
@@ -621,7 +641,7 @@ my @tests = (
             $enabled_scrip->LoadByCols(Description => 'Enabled Scrip');
             ok($enabled_scrip->Id, 'loaded Enabled Scrip');
             is($enabled_scrip->Description, 'Enabled Scrip', 'Enabled Scrip Name');
-            ok($enabled_scrip->IsAdded($general->Id), 'Enabled Scrip added to General');
+            ok($enabled_scrip->IsAdded($enabled_queue->Id), 'Enabled Scrip added to General');
             my $disabled_scrip = RT::Scrip->new(RT->SystemUser);
             $disabled_scrip->LoadByCols(Description => 'Disabled Scrip');
 
@@ -629,7 +649,7 @@ my @tests = (
             $enabled_class->Load('Enabled Class');
             ok($enabled_class->Id, 'loaded Enabled Class');
             is($enabled_class->Name, 'Enabled Class', 'Enabled Class Name');
-            ok($enabled_class->IsApplied($general->Id), 'Enabled Class added to General');
+            ok($enabled_class->IsApplied($enabled_queue->Id), 'Enabled Class added to General');
 
             my $disabled_class = RT::Class->new(RT->SystemUser);
             $disabled_class->Load('Disabled Class');
@@ -638,7 +658,7 @@ my @tests = (
             $enabled_role->Load('Enabled Role');
             ok($enabled_role->Id, 'loaded Enabled Role');
             is($enabled_role->Name, 'Enabled Role', 'Enabled Role Name');
-            ok($enabled_role->IsAdded($general->Id), 'Enabled Role added to General');
+            ok($enabled_role->IsAdded($enabled_queue->Id), 'Enabled Role added to General');
 
             my $disabled_role = RT::CustomRole->new(RT->SystemUser);
             $disabled_role->Load('Disabled Role');
@@ -647,9 +667,9 @@ my @tests = (
             $enabled_group->LoadUserDefinedGroup('Enabled Group');
             ok($enabled_group->Id, 'loaded Enabled Group');
             is($enabled_group->Name, 'Enabled Group', 'Enabled Group Name');
-            ok($enabled_group->PrincipalObj->HasRight(Object => $general, Right => 'ShowTicket'), 'Enabled Group has queue right');
+            ok($enabled_group->PrincipalObj->HasRight(Object => $enabled_queue, Right => 'ShowTicket'), 'Enabled Group has queue right');
             ok($enabled_group->PrincipalObj->HasRight(Object => RT->System, Right => 'SeeQueue'), 'Enabled Group has global right');
-            ok($general->AdminCc->HasMember($enabled_group->PrincipalObj), 'Enabled Group still queue watcher');
+            ok($enabled_queue->AdminCc->HasMember($enabled_group->PrincipalObj), 'Enabled Group still queue watcher');
 
             my $disabled_group = RT::Group->new(RT->SystemUser);
             $disabled_group->LoadUserDefinedGroup('Disabled Group');
@@ -658,9 +678,9 @@ my @tests = (
             $enabled_user->Load('Enabled User');
             ok($enabled_user->Id, 'loaded Enabled User');
             is($enabled_user->Name, 'Enabled User', 'Enabled User Name');
-            ok($enabled_user->PrincipalObj->HasRight(Object => $general, Right => 'ShowTicket'), 'Enabled User has queue right');
+            ok($enabled_user->PrincipalObj->HasRight(Object => $enabled_queue, Right => 'ShowTicket'), 'Enabled User has queue right');
             ok($enabled_user->PrincipalObj->HasRight(Object => RT->System, Right => 'SeeQueue'), 'Enabled User has global right');
-            ok($general->AdminCc->HasMember($enabled_user->PrincipalObj), 'Enabled User still queue watcher');
+            ok($enabled_queue->AdminCc->HasMember($enabled_user->PrincipalObj), 'Enabled User still queue watcher');
 
             my $disabled_user = RT::User->new(RT->SystemUser);
             $disabled_user->Load('Disabled User');

commit 79988883aed3263df2e8e0f5abd34a3da422d90e
Author: Shawn M Moore <shawn at bestpractical.com>
Date:   Wed Mar 22 20:09:56 2017 +0000

    When serializing an ACE, make sure its user is too
    
    Without this we may serialize an ACE and a user's ACLEquivalence group,
    but not the user itself

diff --git a/lib/RT/ACE.pm b/lib/RT/ACE.pm
index a28c54b..74b8c3f 100644
--- a/lib/RT/ACE.pm
+++ b/lib/RT/ACE.pm
@@ -801,6 +801,11 @@ sub FindDependencies {
     $self->SUPER::FindDependencies($walker, $deps);
 
     $deps->Add( out => $self->PrincipalObj->Object );
+
+    if ($self->PrincipalObj->Object->Domain eq 'ACLEquivalence') {
+        $deps->Add( out => $self->PrincipalObj->Object->InstanceObj );
+    }
+
     $deps->Add( out => $self->Object );
 }
 
diff --git a/t/api/initialdata-roundtrip.t b/t/api/initialdata-roundtrip.t
index fb31356..268b43a 100644
--- a/t/api/initialdata-roundtrip.t
+++ b/t/api/initialdata-roundtrip.t
@@ -51,6 +51,10 @@ my @tests = (
             ($ok, $msg) = $user->Create(Name => 'User');
             ok($ok, $msg);
 
+            my $unprivileged = RT::User->new(RT->SystemUser);
+            ($ok, $msg) = $unprivileged->Create(Name => 'Unprivileged');
+            ok($ok, $msg);
+
             ($ok, $msg) = $outer->AddMember($inner->PrincipalId);
             ok($ok, $msg);
 
@@ -68,6 +72,9 @@ my @tests = (
 
             ($ok, $msg) = $user->PrincipalObj->GrantRight(Object => $general, Right => 'OwnTicket');
             ok($ok, $msg);
+
+            ($ok, $msg) = $unprivileged->PrincipalObj->GrantRight(Object => RT->System, Right => 'ModifyTicket');
+            ok($ok, $msg);
         },
         present => sub {
             my $outer = RT::Group->new(RT->SystemUser);
@@ -90,6 +97,11 @@ my @tests = (
             ok($user->Id, 'Loaded user');
             is($user->Name, 'User', 'User name');
 
+            my $unprivileged = RT::User->new(RT->SystemUser);
+            $unprivileged->Load('Unprivileged');
+            ok($unprivileged->Id, 'Loaded Unprivileged');
+            is($unprivileged->Name, 'Unprivileged', 'Unprivileged name');
+
             ok($outer->HasMember($inner->PrincipalId), 'outer hasmember inner');
             ok($inner->HasMember($user->PrincipalId), 'inner hasmember user');
             ok($outer->HasMemberRecursively($user->PrincipalId), 'outer hasmember user recursively');
@@ -121,6 +133,8 @@ my @tests = (
             ok(!$inner->PrincipalObj->HasRight(Object => $general, Right => 'OwnTicket'), 'inner OwnTicket right');
             ok($user->PrincipalObj->HasRight(Object => $general, Right => 'OwnTicket'), 'inner OwnTicket right');
             ok(!$unrelated->PrincipalObj->HasRight(Object => $general, Right => 'OwnTicket'), 'unrelated OwnTicket right');
+
+            ok($unprivileged->PrincipalObj->HasRight(Object => RT->System, Right => 'ModifyTicket'), 'unprivileged ModifyTicket right');
         },
     },
 

commit c8bc700e822c318f7113c6a46e73764f6572e932
Author: Shawn M Moore <shawn at bestpractical.com>
Date:   Wed Mar 22 20:12:21 2017 +0000

    Correctly load ACLs granted on user-defined groups

diff --git a/lib/RT/Handle.pm b/lib/RT/Handle.pm
index 812d4c3..e7239d1 100644
--- a/lib/RT/Handle.pm
+++ b/lib/RT/Handle.pm
@@ -1372,6 +1372,14 @@ sub InsertData {
                     RT->Logger->error("Unable to load queue ".$item->{Queue}.": $msg");
                     next;
                 }
+            } elsif ( $item->{'Group'} || ($item->{ObjectType}||'') eq 'RT::Group') {
+                my $name = $item->{'Group'} || $item->{ObjectId};
+                $object = RT::Group->new(RT->SystemUser);
+                my ($ok, $msg) = $object->LoadUserDefinedGroup($name);
+                unless ( $ok ) {
+                    RT->Logger->error("Unable to load user-defined group $name: $msg");
+                    next;
+                }
             } elsif ( $item->{ObjectType} and $item->{ObjectId}) {
                 $object = $item->{ObjectType}->new(RT->SystemUser);
                 my ($ok, $msg) = $object->Load( $item->{ObjectId} );
diff --git a/t/api/initialdata-roundtrip.t b/t/api/initialdata-roundtrip.t
index 268b43a..4ac476d 100644
--- a/t/api/initialdata-roundtrip.t
+++ b/t/api/initialdata-roundtrip.t
@@ -75,6 +75,10 @@ my @tests = (
 
             ($ok, $msg) = $unprivileged->PrincipalObj->GrantRight(Object => RT->System, Right => 'ModifyTicket');
             ok($ok, $msg);
+
+            ($ok, $msg) = $inner->PrincipalObj->GrantRight(Object => $inner, Right => 'SeeGroup');
+            ok($ok, $msg);
+
         },
         present => sub {
             my $outer = RT::Group->new(RT->SystemUser);
@@ -135,6 +139,12 @@ my @tests = (
             ok(!$unrelated->PrincipalObj->HasRight(Object => $general, Right => 'OwnTicket'), 'unrelated OwnTicket right');
 
             ok($unprivileged->PrincipalObj->HasRight(Object => RT->System, Right => 'ModifyTicket'), 'unprivileged ModifyTicket right');
+
+            ok(!$general->AdminCc->PrincipalObj->HasRight(Object => $inner, Right => 'SeeGroup'), 'AdminCc SeeGroup right');
+            ok(!$outer->PrincipalObj->HasRight(Object => $inner, Right => 'SeeGroup'), 'outer SeeGroup right');
+            ok($inner->PrincipalObj->HasRight(Object => $inner, Right => 'SeeGroup'), 'inner SeeGroup right');
+            ok($user->PrincipalObj->HasRight(Object => $inner, Right => 'SeeGroup'), 'user SeeGroup right');
+            ok(!$unrelated->PrincipalObj->HasRight(Object => $inner, Right => 'SeeGroup'), 'unrelated SeeGroup right');
         },
     },
 

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


More information about the rt-commit mailing list