mark: A photo of Mark kneeling on top of the Taal Volcano in the Philippines. It was a long hike. (Default)
Mark Smith ([staff profile] mark) wrote in [site community profile] changelog2009-02-27 07:35 am

[dw-free] Remember trust groups when re-adding a trust edge

[commit: http://hg.dwscoalition.org/dw-free/rev/82159afba82a]

http://bugs.dwscoalition.org/show_bug.cgi?id=371

Completely update trustmask handling logic in WTF system to be better
documented code and properly handle not overwriting when the user is not
changing the trustmask. Also fix test-wtf which broke after the recent
change to remove_edge. Also resolved issue with tester with memcache turned
on.

Patch by [staff profile] mark.

--------------------------------------------------------------------------------
diff -r f901b2e58ca6 -r 82159afba82a bin/test/test-wtf
--- a/bin/test/test-wtf	Fri Feb 27 06:42:01 2009 +0000
+++ b/bin/test/test-wtf	Fri Feb 27 07:35:40 2009 +0000
@@ -25,6 +25,12 @@ sub rst {
         foreach ( $u1->id, $u2->id );
     $dbh->do( 'DELETE FROM trust_groups WHERE userid = ?', undef, $_ )
         foreach ( $u1->id, $u2->id );
+
+    foreach my $u ( $u1, $u2 ) {
+        foreach my $mc ( qw/ trust_group / ) {
+            LJ::memcache_kill( $u, $mc );
+        }
+    }
 }
 
 # return which bits are on (helper sub)
@@ -103,7 +109,7 @@ push @tests, [ 'mutually trust lists one
 ################################################################################
 push @tests, [ 'remove from trust list', sub
 {
-    $u1->remove_edge( $u2, qw/ trust / );
+    $u1->remove_edge( $u2, trust => { nonotify => 1 } );
     my $row = $dbh->selectrow_array(
             'SELECT COUNT(*) FROM wt_edges WHERE from_userid = ? AND to_userid = ? AND groupmask = ?',
             undef, $u1->id, $u2->id, 30004 | 1
@@ -162,7 +168,7 @@ push @tests, [ 'trust list reverse one m
 ################################################################################
 push @tests, [ 'get full watch list', sub
 {
-    $u1->add_edge( $u1, trust => {} );
+    $u1->add_edge( $u1, trust => { nonotify => 1 } );
 
     my $hr = $u1->watch_list;
     return scalar( keys %$hr ) == 1 ? 1 : 0;
@@ -221,7 +227,7 @@ push @tests, [ 'add to trust and watch l
 push @tests, [ 'add to trust and watch lists', sub
 {
     rst();
-    $u1->add_edge( $u2, trust => { mask => 30004, nonotify => 1 }, watch => {} );
+    $u1->add_edge( $u2, trust => { mask => 30004, nonotify => 1 }, watch => { nonotify => 1} );
     my $row = $dbh->selectrow_array(
             'SELECT COUNT(*) FROM wt_edges WHERE from_userid = ? AND to_userid = ?',
             undef, $u1->id, $u2->id
@@ -233,7 +239,7 @@ push @tests, [ 'add to trust and watch l
 ################################################################################
 push @tests, [ 'remove from watch list', sub
 {
-    $u1->remove_edge( $u2, qw/ watch / );
+    $u1->remove_edge( $u2, watch => { nonotify => 1 } );
     my $row = $dbh->selectrow_array(
             'SELECT COUNT(*) FROM wt_edges WHERE from_userid = ? AND to_userid = ?',
             undef, $u1->id, $u2->id
@@ -260,7 +266,7 @@ push @tests, [ 'add to watch list with c
 push @tests, [ 'add to watch list with colors', sub
 {
     rst();
-    $u1->add_edge( $u2, watch => { fgcolor => 255, bgcolor => 255 } );
+    $u1->add_edge( $u2, watch => { fgcolor => 255, bgcolor => 255, nonotify => 1 } );
     my $row = $dbh->selectrow_array(
             'SELECT COUNT(*) FROM wt_edges WHERE from_userid = ? AND to_userid = ?',
             undef, $u1->id, $u2->id
@@ -280,7 +286,7 @@ push @tests, [ 'validate colors', sub
 ################################################################################
 push @tests, [ 'add to watch list again', sub
 {
-    $u1->add_edge( $u2, watch => {} );
+    $u1->add_edge( $u2, watch => { nonotify => 1 } );
     my $row = $dbh->selectrow_array(
             'SELECT COUNT(*) FROM wt_edges WHERE from_userid = ? AND to_userid = ?',
             undef, $u1->id, $u2->id
@@ -314,7 +320,7 @@ push @tests, [ 'get trust group', sub
 push @tests, [ 'get trust group', sub
 {
     my $grp = $u1->trust_groups;
-    return scalar( %$grp ) > 0 ? 1 : 0;
+    return scalar( keys %$grp ) > 0 ? 1 : 0;
 } ];
 
 ################################################################################
@@ -340,7 +346,7 @@ push @tests, [ 'validate trustmask == 1'
 push @tests, [ 'validate trustmask == 1', sub
 {
     rst();
-    $u1->add_edge( $u2, trust => {} );
+    $u1->add_edge( $u2, trust => { nonotify => 1 } );
     return ( $u1->trustmask( $u2 ) == 1 ) ? 1 : 0;
 } ];
 
@@ -389,6 +395,21 @@ push @tests, [ 'clear groups', sub
 } ];
 
 ################################################################################
+push @tests, [ 'reset, add mask', sub
+{
+    rst();
+    $u1->add_edge( $u2, trust => { mask => 12, nonotify => 1 } );
+    return ( $u1->trustmask( $u2 ) == 13 ) ? 1 : 0;
+} ];
+
+################################################################################
+push @tests, [ 'add edge again, test mask', sub
+{
+    $u1->add_edge( $u2, trust => { nonotify => 1 } );
+    return ( $u1->trustmask( $u2 ) == 13 ) ? 1 : 0;
+} ];
+
+################################################################################
 $| = 1;
 my $id = 1;
 foreach my $test ( @tests ) {
diff -r f901b2e58ca6 -r 82159afba82a cgi-bin/DW/User/Edges/WatchTrust.pm
--- a/cgi-bin/DW/User/Edges/WatchTrust.pm	Fri Feb 27 06:42:01 2009 +0000
+++ b/cgi-bin/DW/User/Edges/WatchTrust.pm	Fri Feb 27 07:35:40 2009 +0000
@@ -48,7 +48,7 @@ DW::User::Edges::define_edge(
                 type => 'hashref',
                 db_edge => 'T',
                 options => {
-                    mask     => { required => 0, type => 'int',  default => 0 },
+                    mask     => { required => 0, type => 'int'                },
                     nonotify => { required => 0, type => 'bool', default => 0 },
                 },
                 add_sub => \&_add_wt_edge,
@@ -70,14 +70,6 @@ sub _add_wt_edge {
     my $do_trust = $trust_edge ? 1 : 0;
     $trust_edge ||= {};
 
-    # setup the mask, note that it has to be 0 unless the user is trusted,
-    # in which case it is the trust mask with the low bit always on.  also,
-    # bit #61 is on if the user is watched.
-    my $mask = $do_watch ? ( 1 << 61 ) : 0;
-    if ( $do_trust ) {
-        $mask |= ( $trust_edge->{mask}+0 ) | 1;
-    }
-
     # get current record, so we know what to modify
     my $dbh = LJ::get_db_writer();
     my $row = $dbh->selectrow_hashref( 'SELECT fgcolor, bgcolor, groupmask FROM wt_edges WHERE from_userid = ? AND to_userid = ?',
@@ -85,21 +77,43 @@ sub _add_wt_edge {
     confess $dbh->errstr if $dbh->err;
     $row ||= { groupmask => 0 };
 
+    # store some existing trust/watch values for use later
+    my $existing_watch = $row->{groupmask} &  ( 1 << 61 );
+    my $existing_trust = $row->{groupmask} & ~( 7 << 61 );
+
     # only matters in the read case, but ...
     my ( $fgcol, $bgcol ) = ( $row->{fgcolor} || LJ::color_todb( '#000000' ),
                               exists $row->{bgcolor} ? $row->{bgcolor} : LJ::color_todb( '#ffffff' ) );
     $fgcol = $watch_edge->{fgcolor} if exists $watch_edge->{fgcolor};
     $bgcol = $watch_edge->{bgcolor} if exists $watch_edge->{bgcolor};
 
-    # set extra bits to keep what the user has already set
-    if ( $do_watch && $do_trust ) {
-        # do nothing, assume we're overriding
-    } elsif ( $do_watch ) {
-        # import the trust values
-        $mask |= ( $row->{groupmask} & ~( 7 << 61 ) );
-    } elsif ( $do_trust ) {
-        # import the watch values
-        $mask |= ( $row->{groupmask} & ( 1 << 61 ) );
+    # calculate the mask we're going to apply to the user; this is somewhat complicated
+    # as we have to account for situations where we're updating only one of the edges, as
+    # well as the situation where we are just updating the trust edge without changing
+    # the user's group memberships.  lots of comments.  start with a mask of 0.
+    my $mask = 0;
+
+    # if they are adding a watch edge, simply turn that bit on
+    $mask |= ( 1 << 61 ) if $do_watch;
+
+    # if they are not adding a watch edge, import the existing watch edge
+    $mask |= $existing_watch unless $do_watch;
+
+    # if they are adding a trust edge, we need to turn bit 1 on
+    $mask |= 1 if $do_trust;
+
+    # now, we have to copy some trustmask, depending on some factors
+    if ( ( $do_watch && ! $do_trust ) ||                   # 1) if we're only updating watch
+         ( $do_trust && ! exists $trust_edge->{mask} ) )   # 2) they're adding a trust edge but don't set a mask
+    {
+        # in these two cases, we want to copy up the trustmask from the database
+        $mask |= $existing_trust;
+    }
+
+    # the final case, they are trusting and have specified a mask; but note we cannot allow
+    # them to set the high bits
+    if ( $do_trust && exists $trust_edge->{mask} ) {
+        $mask |= ( $trust_edge->{mask}+0 & ~( 7 << 61 ) );
     }
 
     # now add the row
--------------------------------------------------------------------------------

Post a comment in response:

This account has disabled anonymous posting.
If you don't have an account you can create one now.
HTML doesn't work in the subject.
More info about formatting

If you are unable to use this captcha for any reason, please contact us by email at support@dreamwidth.org