[Rt-commit] r15677 - rt/branches/3.999-DANGEROUS/lib/RT/Model

ruz at bestpractical.com ruz at bestpractical.com
Sun Aug 31 21:05:13 EDT 2008


Author: ruz
Date: Sun Aug 31 21:05:13 2008
New Revision: 15677

Modified:
   rt/branches/3.999-DANGEROUS/lib/RT/Model/ACE.pm
   rt/branches/3.999-DANGEROUS/lib/RT/Model/CachedGroupMember.pm
   rt/branches/3.999-DANGEROUS/lib/RT/Model/Group.pm
   rt/branches/3.999-DANGEROUS/lib/RT/Model/GroupMember.pm
   rt/branches/3.999-DANGEROUS/lib/RT/Model/Principal.pm
   rt/branches/3.999-DANGEROUS/lib/RT/Model/Ticket.pm
   rt/branches/3.999-DANGEROUS/lib/RT/Model/User.pm

Log:
* delete inside_transaction argument from functions, use handler instead

Modified: rt/branches/3.999-DANGEROUS/lib/RT/Model/ACE.pm
==============================================================================
--- rt/branches/3.999-DANGEROUS/lib/RT/Model/ACE.pm	(original)
+++ rt/branches/3.999-DANGEROUS/lib/RT/Model/ACE.pm	Sun Aug 31 21:05:13 2008
@@ -430,7 +430,7 @@
 
 
 
-=head2 delete { inside_transaction => undef}
+=head2 delete
 
 Delete this object. This method should ONLY ever be called from RT::Model::User or RT::Model::Group (or from itself)
 If this is being called from within a transaction, specify a true value for the parameter inside_transaction.
@@ -468,12 +468,10 @@
 sub _delete {
     my $self = shift;
     my %args = (
-        inside_transaction => undef,
         @_
     );
 
-    my $inside_transaction = $args{'inside_transaction'};
-
+    my $inside_transaction = Jifty->handle->transaction_depth;
     Jifty->handle->begin_transaction() unless $inside_transaction;
 
     my $delegated_from_this = RT::Model::ACECollection->new( current_user => RT->system_user );
@@ -486,7 +484,7 @@
     my $delete_succeeded = 1;
     my $submsg;
     while ( my $delegated_ace = $delegated_from_this->next ) {
-        ( $delete_succeeded, $submsg ) = $delegated_ace->_delete( inside_transaction => 1 );
+        ( $delete_succeeded, $submsg ) = $delegated_ace->_delete;
         last unless ($delete_succeeded);
     }
 
@@ -504,7 +502,7 @@
             or $self->right_name() eq 'SuperUser' )
         )
     {
-        $val = $self->principal_object->_cleanup_invalid_delegations( inside_transaction => 1 );
+        $val = $self->principal_object->_cleanup_invalid_delegations;
     }
 
     if ($val) {

Modified: rt/branches/3.999-DANGEROUS/lib/RT/Model/CachedGroupMember.pm
==============================================================================
--- rt/branches/3.999-DANGEROUS/lib/RT/Model/CachedGroupMember.pm	(original)
+++ rt/branches/3.999-DANGEROUS/lib/RT/Model/CachedGroupMember.pm	Sun Aug 31 21:05:13 2008
@@ -253,7 +253,7 @@
             while ( my $delegation = $delegations->next ) {
 
                 # WHACK IT
-                my $del_ret = $delegation->_delete( inside_transaction => 1 );
+                my $del_ret = $delegation->_delete;
                 unless ($del_ret) {
                     Jifty->log->fatal( "Couldn't delete an ACL delegation that we know exists " . $delegation->id );
                     return (undef);
@@ -330,7 +330,7 @@
             while ( my $delegation = $delegations->next ) {
 
                 # WHACK IT
-                my $del_ret = $delegation->_delete( inside_transaction => 1 );
+                my $del_ret = $delegation->_delete;
                 unless ($del_ret) {
                     Jifty->log->fatal( "Couldn't delete an ACL delegation that we know exists " . $delegation->id );
                     return (undef);

Modified: rt/branches/3.999-DANGEROUS/lib/RT/Model/Group.pm
==============================================================================
--- rt/branches/3.999-DANGEROUS/lib/RT/Model/Group.pm	(original)
+++ rt/branches/3.999-DANGEROUS/lib/RT/Model/Group.pm	Sun Aug 31 21:05:13 2008
@@ -393,11 +393,11 @@
         domain              => undef,
         type                => undef,
         instance            => '0',
-        inside_transaction  => undef,
         _record_transaction => 1,
         @_
     );
-    Jifty->handle->begin_transaction() unless ( $args{'inside_transaction'} );
+    my $inside_transaction = Jifty->handle->transaction_depth;
+    Jifty->handle->begin_transaction() unless $inside_transaction;
 
     # Groups deal with principal ids, rather than user ids.
     # When creating this group, set up a principal id for it.
@@ -416,13 +416,13 @@
     );
     my $id = $self->id;
     unless ($id) {
-        Jifty->handle->rollback() unless ( $args{'inside_transaction'} );
+        Jifty->handle->rollback() unless $inside_transaction;
         return ( 0, _('Could not create group') );
     }
 
     # If we couldn't create a principal Id, get the fuck out.
     unless ($principal_id) {
-        Jifty->handle->rollback() unless ( $args{'inside_transaction'} );
+        Jifty->handle->rollback() unless $inside_transaction;
         Jifty->log->fatal( "Couldn't create a Principal on new user create. Strange things are afoot at the circle K" );
         return ( 0, _('Could not create group') );
     }
@@ -445,7 +445,7 @@
         $self->_new_transaction( type => "Create" );
     }
 
-    Jifty->handle->commit() unless ( $args{'inside_transaction'} );
+    Jifty->handle->commit() unless $inside_transaction;
 
     return ( $id, _("Group Created") );
 }
@@ -499,7 +499,6 @@
         name               => 'User ' . $princ->object->id,
         description        => 'ACL equiv. for user ' . $princ->object->id,
         instance           => $princ->id,
-        inside_transaction => 1
     );
 
     unless ($id) {
@@ -603,7 +602,6 @@
             domain             => $args{'domain'},
             instance           => $args{'instance'},
             type               => $args{'type'},
-            inside_transaction => 1
         )
     );
 }
@@ -919,13 +917,12 @@
 # In the dim future, this will all get factored out and life
 # will get better
 
-# takes a paramhash of { principal_id => undef, inside_transaction }
+# takes a paramhash of { principal_id => undef }
 
 sub _add_member {
     my $self = shift;
     my %args = (
         principal_id       => undef,
-        inside_transaction => undef,
         @_
     );
     my $new_member = $args{'principal_id'};
@@ -969,7 +966,6 @@
     my $id            = $member_object->create(
         member             => $new_member_obj,
         group              => $self->principal_object,
-        inside_transaction => $args{'inside_transaction'}
     );
     if ($id) {
         return ( 1,
@@ -1142,16 +1138,13 @@
 
 
 
-=head2 _cleanup_invalid_delegations { inside_transaction => undef }
+=head2 _cleanup_invalid_delegations
 
 Revokes all ACE entries delegated by members of this group which are
 inconsistent with their current delegation rights.  Does not perform
 permission checks.  Should only ever be called from inside the RT
 library.
 
-If called from inside a transaction, specify a true value for the
-inside_transaction parameter.
-
 Returns a true value if the deletion succeeded; returns a false value
 and logs an internal error if the deletion fails (should not happen).
 
@@ -1165,7 +1158,6 @@
 sub _cleanup_invalid_delegations {
     my $self = shift;
     my %args = (
-        inside_transaction => undef,
         @_
     );
 
@@ -1174,7 +1166,7 @@
         return (undef);
     }
 
-    my $in_trans = $args{inside_transaction};
+    my $in_trans = Jifty->handle->transaction_depth;
 
     # TODO: Can this be unrolled such that the number of DB queries is constant rather than linear in exploded group size?
     my $members = $self->deep_members_obj();
@@ -1182,7 +1174,6 @@
     Jifty->handle->begin_transaction() unless $in_trans;
     while ( my $member = $members->next() ) {
         my $ret = $member->member_obj->_cleanup_invalid_delegations(
-            inside_transaction => 1,
             object             => $args{object}
         );
         unless ($ret) {

Modified: rt/branches/3.999-DANGEROUS/lib/RT/Model/GroupMember.pm
==============================================================================
--- rt/branches/3.999-DANGEROUS/lib/RT/Model/GroupMember.pm	(original)
+++ rt/branches/3.999-DANGEROUS/lib/RT/Model/GroupMember.pm	Sun Aug 31 21:05:13 2008
@@ -103,7 +103,6 @@
     my %args = (
         group              => undef,
         member             => undef,
-        inside_transaction => undef,
         @_
     );
 
@@ -133,7 +132,8 @@
     # TODO what about the groups key cache?
     RT::Model::Principal->invalidate_acl_cache();
 
-    Jifty->handle->begin_transaction() unless ( $args{'inside_transaction'} );
+    my $inside_transaction = Jifty->handle->transaction_depth;
+    Jifty->handle->begin_transaction() unless $inside_transaction;
 
     # We really need to make sure we don't add any members to this group
     # that contain the group itself. that would, um, suck.
@@ -144,11 +144,11 @@
         my $member_object = $args{'member'}->object;
         if ( $member_object->has_member_recursively( $args{'group'} ) ) {
             Jifty->log->debug("Adding that group would create a loop");
-            Jifty->handle->rollback() unless ( $args{'inside_transaction'} );
+            Jifty->handle->rollback() unless $inside_transaction;
             return (undef);
         } elsif ( $args{'member'}->id == $args{'group'}->id ) {
             Jifty->log->debug("Can't add a group to itself");
-            Jifty->handle->rollback() unless ( $args{'inside_transaction'} );
+            Jifty->handle->rollback() unless $inside_transaction;
             return (undef);
         }
     }
@@ -159,7 +159,7 @@
     );
 
     unless ($id) {
-        Jifty->handle->rollback() unless ( $args{'inside_transaction'} );
+        Jifty->handle->rollback() unless $inside_transaction;
         return (undef);
     }
 
@@ -194,17 +194,17 @@
         );
         unless ($other_cached_id) {
             Jifty->log->err( "Couldn't add " . $args{'member'} . " as a submember of a supergroup" );
-            Jifty->handle->rollback() unless ( $args{'inside_transaction'} );
+            Jifty->handle->rollback() unless $inside_transaction;
             return (undef);
         }
     }
 
     unless ($cached_id) {
-        Jifty->handle->rollback() unless ( $args{'inside_transaction'} );
+        Jifty->handle->rollback() unless $inside_transaction;
         return (undef);
     }
 
-    Jifty->handle->commit() unless ( $args{'inside_transaction'} );
+    Jifty->handle->commit() unless $inside_transaction;
 
     return ($id);
 }
@@ -324,7 +324,7 @@
     # Since this deletion may have changed the former member's
     # delegation rights, we need to ensure that no invalid delegations
     # remain.
-    ( $err, $msg ) = $self->member_obj->_cleanup_invalid_delegations( inside_transaction => 1 );
+    ( $err, $msg ) = $self->member_obj->_cleanup_invalid_delegations;
     unless ($err) {
         Jifty->log->warn( "Unable to revoke delegated rights for principal " . $self->id );
         Jifty->handle->rollback();

Modified: rt/branches/3.999-DANGEROUS/lib/RT/Model/Principal.pm
==============================================================================
--- rt/branches/3.999-DANGEROUS/lib/RT/Model/Principal.pm	(original)
+++ rt/branches/3.999-DANGEROUS/lib/RT/Model/Principal.pm	Sun Aug 31 21:05:13 2008
@@ -221,7 +221,7 @@
 
 
 
-=head2 sub _cleanup_invalid_delegations { inside_transaction => undef }
+=head2 sub _cleanup_invalid_delegations
 
 Revokes all ACE entries delegated by this principal which are
 inconsistent with this principal's current delegation rights.  Does
@@ -232,9 +232,6 @@
 If this principal is a group, recursively calls this method on each
 cached user member of itself.
 
-If called from inside a transaction, specify a true value for the
-inside_transaction parameter.
-
 Returns a true value if the deletion succeeded; returns a false value
 and logs an internal error if the deletion fails (should not happen).
 

Modified: rt/branches/3.999-DANGEROUS/lib/RT/Model/Ticket.pm
==============================================================================
--- rt/branches/3.999-DANGEROUS/lib/RT/Model/Ticket.pm	(original)
+++ rt/branches/3.999-DANGEROUS/lib/RT/Model/Ticket.pm	Sun Aug 31 21:05:13 2008
@@ -558,7 +558,6 @@
     # kill performance, bigtime. It gets kept in lockstep thanks to the magic of transactionalization
     ( $val, $msg ) = $self->role_group("owner")->_add_member(
         principal_id       => $owner->principal_id,
-        inside_transaction => 1
     ) unless $defer_owner;
 
     # {{{ Deal with setting up watchers
@@ -681,7 +680,6 @@
         }
         $self->role_group("owner")->_add_member(
             principal_id       => $owner->principal_id,
-            inside_transaction => 1
         );
     }
 
@@ -891,7 +889,6 @@
 
     my ( $m_id, $m_msg ) = $group->_add_member(
         principal_id       => $principal->id,
-        inside_transaction => 1
     );
     unless ($m_id) {
         Jifty->log->error( "Failed to add "
@@ -2310,7 +2307,6 @@
     }
     my ( $add_id, $add_msg ) = $self->role_group("owner")->_add_member(
         principal_id       => $new_owner_obj->principal_id,
-        inside_transaction => 1
     );
     unless ($add_id) {
         Jifty->handle->rollback();

Modified: rt/branches/3.999-DANGEROUS/lib/RT/Model/User.pm
==============================================================================
--- rt/branches/3.999-DANGEROUS/lib/RT/Model/User.pm	(original)
+++ rt/branches/3.999-DANGEROUS/lib/RT/Model/User.pm	Sun Aug 31 21:05:13 2008
@@ -265,7 +265,6 @@
     }
 
     my ( $everyone_id, $everyone_msg ) = $everyone->_add_member(
-        inside_transaction => 1,
         principal_id       => $self->principal_id
     );
     unless ($everyone_id) {
@@ -289,7 +288,6 @@
     }
 
     my ( $ac_id, $ac_msg ) = $access_class->_add_member(
-        inside_transaction => 1,
         principal_id       => $self->principal_id
     );
 
@@ -364,7 +362,6 @@
             Jifty->log->fatal( "User " . $self->id . " is neither privileged nor " . "unprivileged. something is drastically wrong." );
         }
         my ( $status, $msg ) = $priv->_add_member(
-            inside_transaction => 1,
             principal_id       => $self->principal_id
         );
         if ($status) {
@@ -388,7 +385,6 @@
             Jifty->log->fatal( "User " . $self->id . " is neither privileged nor " . "unprivileged. something is drastically wrong." );
         }
         my ( $status, $msg ) = $unpriv->_add_member(
-            inside_transaction => 1,
             principal_id       => $self->principal_id
         );
         if ($status) {
@@ -1141,15 +1137,12 @@
 
 }
 
-=head2 _cleanup_invalid_delegations { inside_transaction => undef }
+=head2 _cleanup_invalid_delegations
 
 Revokes all ACE entries delegated by this user which are inconsistent
 with their current delegation rights.  Does not perform permission
 checks.  Should only ever be called from inside the RT library.
 
-If called from inside a transaction, specify a true value for the
-inside_transaction parameter.
-
 Returns a true value if the deletion succeeded; returns a false value
 and logs an internal error if the deletion fails (should not happen).
 
@@ -1163,7 +1156,6 @@
 sub _cleanup_invalid_delegations {
     my $self = shift;
     my %args = (
-        inside_transaction => undef,
         @_
     );
 
@@ -1172,7 +1164,7 @@
         return (undef);
     }
 
-    my $in_trans = $args{inside_transaction};
+    my $in_trans = Jifty->handle->transaction_depth;
 
     return (1)
         if (
@@ -1206,7 +1198,7 @@
 
     # Delete all disallowed delegations
     while ( my $ace = $acl_to_del->next() ) {
-        my $ret = $ace->_delete( inside_transaction => 1 );
+        my $ret = $ace->_delete;
         unless ($ret) {
             Jifty->handle->rollback() unless $in_trans;
             Jifty->log->warn( "Couldn't delete delegated ACL entry " . $ace->id );


More information about the Rt-commit mailing list