fu: Close-up of Fu, bringing a scoop of water to her mouth (Default)
fu ([personal profile] fu) wrote in [site community profile] changelog2010-12-16 05:54 pm

[dw-free] convert interests.bml to TT

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

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

Refactor; move things out of the BML files and into relevant modules,
reducing duplication. Two new methods: $u->interest_update,
$u->sync_interests.

Patch by [personal profile] kareila.

Files modified:
  • cgi-bin/LJ/Keywords.pm
  • cgi-bin/LJ/Widget/FriendInterests.pm
  • htdocs/interests.bml
--------------------------------------------------------------------------------
diff -r a908e0e21965 -r a6852c049c62 cgi-bin/LJ/Keywords.pm
--- a/cgi-bin/LJ/Keywords.pm	Fri Dec 17 01:45:28 2010 +0800
+++ b/cgi-bin/LJ/Keywords.pm	Fri Dec 17 01:53:39 2010 +0800
@@ -102,23 +102,6 @@ sub interest_string_to_list {
 
     # final list is ,-sep
     return grep { length } split (/\s*,\s*/, $intstr);
-}
-
-
-# insert new interest into interest table, return new interest id
-# input: keyword of interest
-sub new_interest_from_kw {
-    my ( $keyword ) = @_;
-    my $intid = LJ::get_sitekeyword_id( $keyword );
-    return undef unless $intid;
-
-    # add zero intcount if this is a new interest
-    my $dbh = LJ::get_db_writer();
-    $dbh->do( "INSERT IGNORE INTO interests (intid, intcount) VALUES (?,?)",
-            undef, $intid, 0 );
-    die $dbh->errstr if $dbh->err;
-
-    return $intid;
 }
 
 
@@ -264,6 +247,55 @@ sub interest_list {
 }
 
 
+sub interest_update {
+    my ( $u, %opts ) = @_;
+    return undef unless LJ::isu( $u );
+    my ( $add, $del ) = ( $opts{add}, $opts{del} );
+    return 1 unless $add || $del;  # nothing to do
+
+    # track if we made changes to refresh memcache later.
+    my $did_mod = 0;
+
+    # community interests go in a different table than user interests,
+    # though the schemas are the same so we can run the same queries on them
+    my $uitable = $u->is_community ? 'comminterests' : 'userinterests';
+    my $uid = $u->userid;
+    my $dbh = LJ::get_db_writer() or die LJ::Lang::ml( "error.nodb" );
+
+    if ( $del && @$del ) {
+        $did_mod = 1;
+        my $intid_in = join ',', map { $dbh->quote( $_ ) } @$del;
+        $dbh->do( "DELETE FROM $uitable WHERE userid=$uid AND intid IN ($intid_in)" );
+        die $dbh->errstr if $dbh->err;
+        $dbh->do( "UPDATE interests SET intcount=intcount-1 WHERE intid IN ($intid_in)" );
+        die $dbh->errstr if $dbh->err;
+    }
+
+    if ( $add && @$add ) {
+        # assume we've already checked maxinterests
+        $did_mod = 1;
+        my $intid_in = join ',', map { $dbh->quote( $_ ) } @$add;
+        my $sqlp = join ',', map { "(?,?)" } @$add;
+        $dbh->do( "REPLACE INTO $uitable (userid, intid) VALUES $sqlp",
+                  undef, map { ( $uid, $_ ) } @$add );
+        die $dbh->errstr if $dbh->err;
+        # set a zero intcount for any new ints
+        $dbh->do( "INSERT IGNORE INTO interests (intid, intcount) VALUES $sqlp",
+                  undef, map { ( $_, 0 ) } @$add );
+        die $dbh->errstr if $dbh->err;
+        # now do the increment for all ints
+        $dbh->do( "UPDATE interests SET intcount=intcount+1 WHERE intid IN ($intid_in)" );
+        die $dbh->errstr if $dbh->err;
+    }
+
+    # do migrations to clean up userinterests vs comminterests conflicts
+    # also clears memcache and object cache for intids if needed
+    $u->lazy_interests_cleanup( $did_mod );
+
+    return 1;
+}
+
+
 # return hashref with intname => intid
 sub interests {
     my ( $u, $opts ) = @_;
@@ -281,7 +313,7 @@ sub interests {
 
 
 sub lazy_interests_cleanup {
-    my $u = shift;
+    my ( $u, $expire ) = @_;
 
     my $dbh = LJ::get_db_writer();
 
@@ -293,7 +325,12 @@ sub lazy_interests_cleanup {
         $dbh->do("DELETE FROM comminterests WHERE userid=?", undef, $u->id);
     }
 
-    LJ::memcache_kill($u, "intids");
+    # don't expire memcache unless requested
+    return 1 unless $expire;
+
+    LJ::memcache_kill( $u, "intids" );
+    $u->{_cache_interests} = undef;
+
     return 1;
 }
 
@@ -305,70 +342,73 @@ sub set_interests {
 sub set_interests {
     my ($u, $old, $new) = @_;
 
-    $u = LJ::want_user($u);
-    my $userid = $u->userid;
-    return undef unless $userid;
+    $u = LJ::want_user( $u ) or return undef;
 
     return undef unless ref $old eq 'HASH';
     return undef unless ref $new eq 'ARRAY';
 
-    my $dbh = LJ::get_db_writer();
-    my %int_new = ();
+    my %int_add = ();
     my %int_del = %$old;  # assume deleting everything, unless in @$new
 
-    # community interests go in a different table than user interests,
-    # though the schemas are the same so we can run the same queries on them
-    my $uitable = $u->is_community ? 'comminterests' : 'userinterests';
-
-    # track if we made changes to refresh memcache later.
-    my $did_mod = 0;
-
-    my @valid_ints = LJ::validate_interest_list(@$new);
+    my @valid_ints = LJ::validate_interest_list( @$new );
     foreach my $int ( @valid_ints ) {
-        $int_new{$int} = 1 unless $old->{$int};
+        $int_add{$int} = 1 unless $old->{$int};
         delete $int_del{$int};
-    }
-
-    ### were interests removed?
-    if ( %int_del ) {
-        ## easy, we know their IDs, so delete them en masse
-        my $intid_in = join(", ", values %int_del);
-        $dbh->do("DELETE FROM $uitable WHERE userid=$userid AND intid IN ($intid_in)");
-        $dbh->do("UPDATE interests SET intcount=intcount-1 WHERE intid IN ($intid_in)");
-        $did_mod = 1;
     }
 
     ### do we have new interests to add?
     my @new_intids = ();  ## existing IDs we'll add for this user
-    if ( %int_new ) {
-        foreach my $int ( keys %int_new ) {
-            my $intid = LJ::new_interest_from_kw( $int ) ;
-            push @new_intids, $intid if $intid;
-        }
-
-        ## do updating en masse for interests
-        if ( @new_intids ) {
-            $did_mod = 1;
-
-            my $sql = "REPLACE INTO $uitable (userid, intid) VALUES ";
-            $sql .= join( ", ", map { "($userid, $_)" } @new_intids );
-            $dbh->do( $sql );
-
-            my $intid_in = join(", ", @new_intids);
-            $dbh->do("UPDATE interests SET intcount=intcount+1 WHERE intid IN ($intid_in)");
-        }
+    foreach my $int ( keys %int_add ) {
+        my $intid = LJ::get_sitekeyword_id( $int );
+        push @new_intids, $intid if $intid;
     }
 
+    # Note this does NOT check against maxinterests, do that in the caller.
+    $u->interest_update( add => \@new_intids, del => [ values %int_del ] );
+
     LJ::Hooks::run_hooks("set_interests", $u, \%int_del, \@new_intids); # interest => intid
-
-    # do migrations to clean up userinterests vs comminterests conflicts
-    $u->lazy_interests_cleanup;
-
-    LJ::memcache_kill($u, "intids") if $did_mod;
-    $u->{_cache_interests} = undef if $did_mod;
 
     return 1;
 }
 
 
+# arguments: hashref of submitted form and list of user's previous intids
+# returns: hashref with number of ints added (or toomany) and deleted
+sub sync_interests {
+    my ( $u, $args, @intids ) = @_;
+    warn "sync_interests: invalid arguments" and return undef
+        unless LJ::isu( $u ) and ref $args eq "HASH";
+    @intids = grep /^\d+$/, @intids;  # numeric
+
+    my %uint = reverse %{ $u->interests };  # intid => interest
+    my $rv = {};
+    my ( @todel, @toadd );
+
+    foreach my $intid ( @intids ) {
+        next unless $intid > 0;    # prevent adding zero or negative intid
+        push @todel, $intid if $uint{$intid} && ! $args->{"int_$intid"};
+        push @toadd, $intid if ! $uint{$intid} && $args->{"int_$intid"};
+    }
+
+    my $addcount = scalar( @toadd );
+    my $delcount = scalar( @todel );
+
+    if ( $addcount ) {
+        my $intcount = scalar( keys %uint ) + $addcount - $delcount;
+        my $maxinterests = $u->count_max_interests;
+        if ( $intcount > $maxinterests ) {
+            # let the user know they're over the limit
+            $rv->{toomany} = $maxinterests;
+            @toadd = ();  # deletion still OK
+        }
+    }
+
+    $u->interest_update( add => \@toadd, del => \@todel );
+    $rv->{added} = $addcount;
+    $rv->{deleted} = $delcount;
+
+    return $rv;
+}
+
+
 1;
diff -r a908e0e21965 -r a6852c049c62 cgi-bin/LJ/Widget/FriendInterests.pm
--- a/cgi-bin/LJ/Widget/FriendInterests.pm	Fri Dec 17 01:45:28 2010 +0800
+++ b/cgi-bin/LJ/Widget/FriendInterests.pm	Fri Dec 17 01:53:39 2010 +0800
@@ -22,8 +22,7 @@ sub need_res {
 }
 
 sub handle_post {
-    my $class = shift;
-    my $fields = shift;
+    my ( $class, $fields ) = @_;
 
     return unless $fields->{user};
     return unless $fields->{from};
@@ -33,65 +32,21 @@ sub handle_post {
     my $fromu = LJ::isu($fields->{from}) ? $fields->{from} : LJ::load_user($fields->{from});
     return unless $fromu;
 
-    my $uints = $u->interests;
     my $fromints = $fromu->interests;
     return unless keys %$fromints;
+    my $sync = $u->sync_interests( $fields, values %$fromints );
 
-    my @fromintids = values %$fromints;
-    my $uitable = $u->is_comm ? 'comminterests' : 'userinterests';
-
-    my @todel;
-    my @toadd;
-    foreach my $fromint (@fromintids) {
-        next unless $fromint > 0;    # prevent adding zero or negative intid
-        push @todel, $fromint if  $uints->{$fromint} && !$fields->{'int_'.$fromint};
-        push @toadd, $fromint if !$uints->{$fromint} &&  $fields->{'int_'.$fromint};
+    if ( $sync->{toomany} ) {
+        my $toomany = $sync->{deleted} ? 'del_and_toomany' : 'toomany';
+        die LJ::Lang::ml( "interests.results.$toomany",
+                          { intcount => $sync->{toomany} } );
     }
-
-    my $intcount = scalar %$uints;
-    my $deleted = 0;
-    if (@todel) {
-        my $intid_in = join(",", @todel);
-        my $dbh = LJ::get_db_writer();
-        $dbh->do("DELETE FROM $uitable WHERE userid=? AND intid IN ($intid_in)",
-                 undef, $u->id);
-        $dbh->do("UPDATE interests SET intcount=intcount-1 WHERE intid IN ($intid_in)");
-        $deleted = 1;
-    }
-    if (@toadd) {
-        my $maxinterests = $u->count_max_interests;
-
-        if ($intcount + scalar @toadd > $maxinterests) {
-            if ($deleted) {
-                die BML::ml('/interests.bml.results.del_and_toomany', {'intcount' => $maxinterests});
-            } else {
-                die BML::ml('/interests.bml.results.toomany', {'intcount' => $maxinterests});
-            }
-        } else {
-            my $dbh = LJ::get_db_writer();
-            my $sqlp = "(?,?)" . (",(?,?)" x (scalar(@toadd) - 1));
-            my @bindvars = map { ($u->id, $_) } @toadd;
-            $dbh->do("REPLACE INTO $uitable (userid, intid) VALUES $sqlp", undef, @bindvars);
-
-            my $intid_in = join(",", @toadd);
-            $dbh->do("UPDATE interests SET intcount=intcount+1 WHERE intid IN ($intid_in)");
-        }
-    }
-
-    # if a community, remove any old rows from userinterests
-    if ($u->is_comm) {
-        my $dbh = LJ::get_db_writer();
-        $dbh->do("DELETE FROM userinterests WHERE userid=?", undef, $u->id);
-    }
-
-    LJ::memcache_kill($u, "intids");
 
     return;
 }
 
 sub render_body {
-    my $class = shift;
-    my %opts = @_;
+    my ( $class, %opts ) = @_;
     my $ret;
 
     return "" unless $opts{user};
diff -r a908e0e21965 -r a6852c049c62 htdocs/interests.bml
--- a/htdocs/interests.bml	Fri Dec 17 01:45:28 2010 +0800
+++ b/htdocs/interests.bml	Fri Dec 17 01:53:39 2010 +0800
@@ -109,7 +109,7 @@ body<=
         } else {
             # adding a new interest
             my @validate = LJ::validate_interest_list( $formargs->{keyword} );
-            $intid = LJ::new_interest_from_kw( $validate[0] ) if @validate;
+            $intid = LJ::get_sitekeyword_id( $validate[0] ) if @validate;
         }
 
         return $ML{'error.invalidform'} unless $intid;
@@ -132,19 +132,7 @@ body<=
             return $ret;
         }
 
-        my $dbh = LJ::get_db_writer();
-        my $uitable = $table->($remote);
-        $dbh->do("INSERT INTO $uitable (userid, intid) VALUES (?, ?)",
-                 undef, $remote->userid, $intid);
-        LJ::memcache_kill($remote, "intids");
-        unless ($dbh->err) {
-            $dbh->do("UPDATE interests SET intcount=intcount+1 WHERE intid=?", undef, $intid);
-        }
-
-        # if a community, remove any old rows from userinterests
-        if ( $remote->is_community ) {
-            $dbh->do( "DELETE FROM userinterests WHERE userid=?", undef, $remote->userid );
-        }
+        $remote->interest_update( add => [$intid] );
 
         my $profile_url = $remote->profile_url;
         $ret .= "<?h1 $ML{'.add.added.head'} h1?><?p $ML{'.add.added.text'} p?>" .
@@ -326,62 +314,17 @@ body<=
         my $u = LJ::get_authas_user($authas);
         return LJ::bad_input($ML{'.error.noauth'}) unless $u;
 
-        my $uitable = $table->($u);
-        my %uint;
-        my $intcount = 0;
-
-        my $uints = $u->get_interests();
-        foreach (@$uints) {
-            $uint{$_->[0]} = $_->[1];  # uint{intid} = interest
-            $intcount++;
-        }
-
         my @fromints = map { $_+0 } split (/\s*,\s*/, $POST{'allintids'});
-        my @todel;
-        my @toadd;
-        foreach my $fromint (@fromints) {
-            next unless $fromint > 0;    # prevent adding zero or negative intid
-            push (@todel, $fromint) if  $uint{$fromint} && !$POST{'int_'.$fromint};
-            push (@toadd, $fromint) if !$uint{$fromint} &&  $POST{'int_'.$fromint};
-        }
-        my ($deleted, $added, $toomany) = (0, 0, 0);
-        if (@todel) {
-            my $intid_in = join(",", @todel);
-            my $dbh = LJ::get_db_writer();
-            $dbh->do("DELETE FROM $uitable WHERE userid=? AND intid IN ($intid_in)",
-                     undef, $u->userid);
-            $dbh->do("UPDATE interests SET intcount=intcount-1 WHERE intid IN ($intid_in)");
-            $deleted = 1;
-        }
-        if (@toadd) {
-            if ($intcount + scalar @toadd > $maxinterests) {
-                $toomany = 1;
-            } else {
-                my $dbh = LJ::get_db_writer();
-                my $sqlp = "(?,?)" . (",(?,?)" x (scalar(@toadd) - 1));
-                my @bindvars = map { ( $u->userid, $_ ) } @toadd;
-                $dbh->do("REPLACE INTO $uitable (userid, intid) VALUES $sqlp", undef, @bindvars);
-
-                my $intid_in = join(",", @toadd);
-                $dbh->do("UPDATE interests SET intcount=intcount+1 WHERE intid IN ($intid_in)");
-                $added = 1;
-            }
-        }
-
-        # if a community, remove any old rows from userinterests
-        if ( $u->is_community ) {
-            my $dbh = LJ::get_db_writer();
-            $dbh->do( "DELETE FROM userinterests WHERE userid=?", undef, $u->userid );
-        }
+        my $sync = $u->sync_interests( \%POST, @fromints );
 
         my $ret = "<?h1 $ML{'.results.header'} h1?><?p ";
-        if ($deleted) {
-            $ret .= $added   ? $ML{'.results.both'}
-                  : $toomany ? BML::ml('.results.del_and_toomany', {'intcount' => $maxinterests})
+        if ($sync->{deleted}) {
+            $ret .= $sync->{added}   ? $ML{'.results.both'}
+                  : $sync->{toomany} ? BML::ml('.results.del_and_toomany', {'intcount' => $maxinterests})
                   : $ML{'.results.deleted'};
         } else {
-            $ret .= $added   ? $ML{'.results.added'}
-                  : $toomany ? BML::ml('.results.toomany', {'intcount' => $maxinterests})
+            $ret .= $sync->{added}   ? $ML{'.results.added'}
+                  : $sync->{toomany} ? BML::ml('.results.toomany', {'intcount' => $maxinterests})
                   : $ML{'.results.nothing'};
         }
 
@@ -393,7 +336,6 @@ body<=
         $ret .= " " . BML::ml('.results.goback2', {'user' => LJ::ljuser($POST{'fromuser'}), 'aopts' => "href='$profile_url_other'"})
             if ($POST{'fromuser'} ne "" && $POST{'fromuser'} ne $u->{'user'});
         $ret .= " p?>";
-        LJ::memcache_kill($u, "intids");
         return $ret;
     }
 
--------------------------------------------------------------------------------