Big change to the way we do tag keyword security counts.  This should 
fix the issue with some situations causing tags to get double counted, leading 
to misleading numbers.

        Patch by kareila. 

diffs (247 lines):

diff -r 460d1d159642 -r 92b3340a802b bin/upgrading/update-db-general.pl
--- a/bin/upgrading/update-db-general.pl        Thu Oct 08 19:23:06 2009 -0500
+++ b/bin/upgrading/update-db-general.pl        Fri Oct 09 00:43:46 2009 +0000
@@ -3794,6 +3794,71 @@ register_alter(sub {
                   q{ALTER TABLE syndicated_hubbub2 ADD COLUMN timespinged INT 
UNSIGNED NOT NULL DEFAULT '0'} );
     }
 
+    if ( table_relevant( "logkwsum" ) && ! check_dbnote( 
"logkwsum_fix_filtered_counts" ) ) {
+        # this is a very, very racy situation ... we want to do an update of 
this data, but if anybody
+        # else is actively using this table, they're going to be inserting bad 
data on top of us which
+        # will leave SOMEONE in an inconsistent state.  let's warn the user 
that they should have the site
+        # turned off for this update.
+        unless ( $::_warn_logkwsum++ > 0 ) {
+            warn <<EOF;
+
+* * * * * * * * * WARNING * * * * * * * *
+
+We need to do an update of tag security metadata.  This is an UNSAFE update
+and we request that you shut off your site.
+
+Please turn off TheSchwartz workers, Gearman workers, Apache processes, and
+anything else that can touch the database.
+
+Once you are done doing that, please press ENTER to proceed.
+
+Press Ctrl+C to cancel this operation now.
+
+* * * * * * * * * WARNING * * * * * * * *
+EOF
+            $_ = <>;
+        }
+
+        do_sql( 'LOCK TABLES logkwsum WRITE, logtags WRITE, log2 WRITE' );
+        do_sql( 'DELETE FROM logkwsum' );
+
+        do_sql(
+            q{INSERT INTO logkwsum
+                  SELECT logtags.journalid, logtags.kwid, log2.allowmask, 
COUNT(*)
+                  FROM log2, logtags
+                  WHERE logtags.journalid = log2.journalid
+                    AND logtags.jitemid = log2.jitemid
+                    AND log2.security = 'usemask'
+                  GROUP BY journalid, kwid, allowmask
+            }
+        );
+
+        do_sql(
+            q{INSERT INTO logkwsum
+                  SELECT logtags.journalid, logtags.kwid, 0, COUNT(*)
+                  FROM log2, logtags
+                  WHERE logtags.journalid = log2.journalid
+                    AND logtags.jitemid = log2.jitemid
+                    AND log2.security = 'private'
+                  GROUP BY journalid, kwid
+            }
+        );
+
+        do_sql(
+            q{INSERT INTO logkwsum
+                  SELECT logtags.journalid, logtags.kwid, 1 << 63, COUNT(*)
+                  FROM log2, logtags
+                  WHERE logtags.journalid = log2.journalid
+                    AND logtags.jitemid = log2.jitemid
+                    AND log2.security = 'public'
+                  GROUP BY journalid, kwid
+            }
+        );
+
+        do_sql( 'UNLOCK TABLES' );
+        set_dbnote( "logkwsum_fix_filtered_counts", 1 );
+    }
+
 });
 
 
diff -r 460d1d159642 -r 92b3340a802b cgi-bin/ljlib.pl
--- a/cgi-bin/ljlib.pl  Thu Oct 08 19:23:06 2009 -0500
+++ b/cgi-bin/ljlib.pl  Fri Oct 09 00:43:46 2009 +0000
@@ -86,7 +86,7 @@ sub END { LJ::end_request(); }
                     "logtags", "logtagsrecent", "logkwsum",
                     "recentactions", "usertags", "pendcomments",
                     "user_schools", "loginlog", "active_user", "userblobcache",
-                    "notifyqueue", "cprod",
+                    "notifyqueue", "cprod", "blobcache",
                     "jabroster", "jablastseen", "random_user_set",
                     "poll2", "pollquestion2", "pollitem2",
                     "pollresult2", "pollsubmission2",
diff -r 460d1d159642 -r 92b3340a802b cgi-bin/taglib.pl
--- a/cgi-bin/taglib.pl Thu Oct 08 19:23:06 2009 -0500
+++ b/cgi-bin/taglib.pl Fri Oct 09 00:43:46 2009 +0000
@@ -227,12 +227,12 @@ sub _get_usertagsmulti {
                 $res->{$jid}->{$kwid}->{security_level} = 'protected'
                     unless $res->{$jid}->{$kwid}->{security_level} eq 'public';
             } elsif ($sec) {
-                # if $sec is true (>0), and not trust/public, then it's a 
group.  but it's
-                # still in the form of a number, and we want to know which 
group it is.  so
-                # we must convert the mask back to a bit number with 
LJ::bit_breakdown.  but
-                # we will only ever have one mask, so we just accept that.
-                my $grpid = (LJ::bit_breakdown($sec))[0] + 0;
-                $res->{$jid}->{$kwid}->{security}->{groups}->{$grpid} += $ct;
+                # if $sec is true (>0), and not trust/public, then it's a 
group(s).  but it's
+                # still in the form of a number, and we want to know which 
group(s) it is.  so
+                # we must convert the mask back to a bit number with 
LJ::bit_breakdown.
+                foreach my $grpid ( LJ::bit_breakdown($sec) ) {
+                    $res->{$jid}->{$kwid}->{security}->{groups}->{$grpid} += 
$ct;
+                }
                 $res->{$jid}->{$kwid}->{security_level} ||= 'group';
             } else {
                 # $sec must be 0
@@ -622,33 +622,20 @@ sub is_valid_tagstring {
 }
 
 # <LJFUNC>
-# name: LJ::Tags::get_security_breakdown
+# name: LJ::Tags::get_security_level
 # class: tags
-# des: Returns a list of security levels that apply to the given security 
information.
+# des: Returns the security level that applies to the given security 
information.
 # args: security, allowmask
 # des-security: 'private', 'public', or 'usemask'
 # des-allowmask: a bitmask in standard allowmask form
-# returns: List of broken down security levels to use for [dbtable[logkwsum]] 
table.
+# returns: Bitwise security level to use for [dbtable[logkwsum]] table.
 # </LJFUNC>
-sub get_security_breakdown {
+sub get_security_level {
     my ($sec, $mask) = @_;
 
-    my @out;
-
-    if ($sec eq 'private') {
-        @out = (0);
-    } elsif ($sec eq 'public') {
-        @out = (1 << 63);
-    } else {
-        # have to get each group bit into a mask
-        foreach my $bit (0..60) { # include 0 for trusted only
-            if ($mask & (1 << $bit)) {
-                push @out, (1 << $bit);
-            }
-        }
-    }
-
-    return @out;
+    return 0 if $sec eq 'private';
+    return 1 << 63 if $sec eq 'public';
+    return $mask;
 }
 
 # <LJFUNC>
@@ -803,7 +790,7 @@ sub update_logtags {
     return undef unless $l2row;
 
     # calculate security masks
-    my @sec = LJ::Tags::get_security_breakdown($l2row->{security}, 
$l2row->{allowmask});
+    my $sec = LJ::Tags::get_security_level($l2row->{security}, 
$l2row->{allowmask});
 
     # setup a rollback bail path so that we can undo everything we've done
     # if anything fails in the middle; and if the rollback fails, scream loudly
@@ -913,28 +900,26 @@ sub update_logtags {
 
     # now iterate and get the security counts
     my %counts;
-    while (my ($kwid, $sec, $ct) = $sth->fetchrow_array) {
-        $counts{$kwid}->{$sec} = $ct;
+    while (my ($kwid, $secu, $ct) = $sth->fetchrow_array) {
+        $counts{$kwid}->{$secu} = $ct;
     }
 
     # now we want to update them, and delete any at 0
     my (@replace, @delete);
     foreach my $kwid (@kwids) {
-        foreach my $sec (@sec) {
-            if (exists $counts{$kwid} && exists $counts{$kwid}->{$sec}) {
-                # an old one exists
-                my $new = $counts{$kwid}->{$sec} + $security{$kwid};
-                if ($new > 0) {
-                    # update it
-                    push @replace, [ $kwid, $sec, $new ];
-                } else {
-                    # delete this one
-                    push @delete, [ $kwid, $sec ];
-                }
+        if (exists $counts{$kwid} && exists $counts{$kwid}->{$sec}) {
+            # an old one exists
+            my $new = $counts{$kwid}->{$sec} + $security{$kwid};
+            if ($new > 0) {
+                # update it
+                push @replace, [ $kwid, $sec, $new ];
             } else {
-                # add a new one
-                push @replace, [ $kwid, $sec, $security{$kwid} ];
+                # delete this one
+                push @delete, [ $kwid, $sec ];
             }
+        } else {
+            # add a new one
+            push @replace, [ $kwid, $sec, $security{$kwid} ];
         }
     }
 
@@ -1318,12 +1303,41 @@ sub deleted_trust_group {
     my $bit = shift() + 0;
     return undef unless $u && $bit >= 1 && $bit <= 60;
 
+    my $bval = 1 << $bit;
+    my %masks;
+    $masks{$bval} = 1;  # don't need alterations for rows that only include 
the deleted group
+
+    my $rollback = sub {
+        die $u->errstr unless $u->rollback;
+        return undef;
+    };
+
+    # get data for all other security masks that include this group
+    my $sth = $u->prepare("SELECT security, kwid, entryct FROM logkwsum WHERE 
journalid = ?" .
+                          " AND security & ? AND security != ?");
+    return undef if $u->err || ! $sth;
+    $sth->execute($u->{userid}, $bval, $bval);
+    return undef if $sth->err;
+    $u->begin_work;  # rollback begins here
+
+    while (my ($sec, $kwid, $ct) = $sth->fetchrow_array) {
+        # remove the group from mask and update logkwsum
+        my $newsec = $sec ^ $bval;  # XOR
+        unless ( $u->do("UPDATE logkwsum SET entryct = entryct + ? WHERE 
journalid = ? AND security = ?  AND kwid = ?",
+                        undef, $ct, $u->{userid}, $newsec, $kwid) ) {
+            # no row to update, have to insert
+            $u->do("INSERT INTO logkwsum (journalid, security, kwid, entryct) 
VALUES (?,?,?,?)",
+                   undef, $u->{userid}, $newsec, $kwid, $ct);
+        }
+        return $rollback->() if $u->err;
+        $masks{$sec} = 1;
+    }
     # delete from logkwsum and then nuke the user's tags
-    $u->do("DELETE FROM logkwsum WHERE journalid = ? AND security = ?",
-           undef, $u->{userid}, 1 << $bit);
-    return undef if $u->err;
+    $u->do("DELETE FROM logkwsum WHERE journalid = ? AND security IN (?)",
+           undef, $u->{userid}, join(', ', keys %masks));
+    return $rollback->() if $u->err;
 
-    # that was simple
+    die $u->errstr unless $u->commit;
     LJ::Tags::reset_cache($u);
     return 1;
 }
_______________________________________________
dw-changelog mailing list
[email protected]
http://lists.dwscoalition.org/mailman/listinfo/dw-changelog

Reply via email to