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;
     }
 
--------------------------------------------------------------------------------

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