fu: Close-up of Fu, bringing a scoop of water to her mouth (Default)
fu ([personal profile] fu) wrote in [site community profile] changelog2010-06-25 01:52 pm
  • Add Memory
  • Share This Entry

[dw-free] replace ljmood.pl with DW::Mood.pm

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

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

More caching, less raw SQL calls.

Patch by [personal profile] kareila.

Files modified:
  • bin/upgrading/update-db.pl
  • cgi-bin/DW/Mood.pm
  • cgi-bin/LJ/Console/Command/MoodthemeList.pm
  • cgi-bin/LJ/Console/Command/MoodthemePublic.pm
  • cgi-bin/LJ/Customize.pm
  • htdocs/manage/moodthemes.bml
  • htdocs/moodlist.bml
--------------------------------------------------------------------------------
diff -r c7431ec0bc18 -r 49f12658bc9d bin/upgrading/update-db.pl
--- a/bin/upgrading/update-db.pl	Fri Jun 25 20:41:23 2010 +0800
+++ b/bin/upgrading/update-db.pl	Fri Jun 25 21:57:59 2010 +0800
@@ -627,6 +627,7 @@ sub populate_moods {
                 }
             }
             close M;
+            LJ::MemCache::delete( "moods_public" );
         }
     }
 }
diff -r c7431ec0bc18 -r 49f12658bc9d cgi-bin/DW/Mood.pm
--- a/cgi-bin/DW/Mood.pm	Fri Jun 25 20:41:23 2010 +0800
+++ b/cgi-bin/DW/Mood.pm	Fri Jun 25 21:57:59 2010 +0800
@@ -158,6 +158,9 @@ sub load_theme {
         "SELECT name, des, is_public, ownerid FROM moodthemes" .
         " WHERE moodthemeid=?", undef, $themeid );
     die $dbr->errstr if $dbr->err;
+
+    LJ::MemCache::set( $memkey, {}, 3600 ) and return undef
+        unless $name;  # no results for this theme
 
     $LJ::CACHE_MOOD_THEME{$themeid}->{moodthemeid} = $themeid;
     $LJ::CACHE_MOOD_THEME{$themeid}->{is_public} = $is_public;
@@ -263,10 +266,86 @@ sub clear_cache {
     delete $LJ::CACHE_MOOD_THEME{$themeid};
 }
 
+# get list of theme data for given theme and/or user
+# arguments: hashref { themeid => ?, ownerid => ? }
+# returns: array of hashrefs from memcache or db, undef on failure
+sub get_themes {
+    my ( $self, $arg ) = @_;
+    # if called with no arguments, check for object id
+    $arg ||= { themeid => $self->id } if ref $self;
+    return undef unless $arg;
+
+    my ( $themeid, $ownerid ) = ( $arg->{themeid}, $arg->{ownerid} );
+    $ownerid ||= $self->ownerid( $themeid );
+    return undef unless $ownerid;
+
+    # cache contains list of all themes for this user
+    my $memkey = [$ownerid, "moodthemes:$ownerid"];
+    my $ids = LJ::MemCache::get( $memkey );
+    unless ( defined $ids ) {
+        # check database
+        my $dbr = LJ::get_db_reader() or return undef;
+        $ids = $dbr->selectcol_arrayref(
+            "SELECT moodthemeid FROM moodthemes" .
+            " WHERE ownerid=? ORDER BY name", undef, $ownerid );
+        die $dbr->errstr if $dbr->err;
+        # update memcache
+        LJ::MemCache::set( $memkey, $ids, 3600 );
+    }
+
+    # if they specified a theme, see if it's in the list
+    if ( $themeid and grep { $_ == $themeid } @$ids ) {
+        $self->load_theme( $themeid );
+        my $data = $LJ::CACHE_MOOD_THEME{$themeid};
+        return wantarray ? ( $data ) : $data;
+    } elsif ( $themeid ) {
+        # not in the list: ownerid doesn't own themeid
+        return undef;
+    }
+
+    # if they didn't specify a theme, return everything
+    return $self->_load_data_multiple( $ids );
+}
+
+sub _load_data_multiple {
+    my ( $self, $themes ) = @_;
+    my @data;
+    foreach ( @$themes ) {
+        $self->load_theme( $_ );
+        push @data, $LJ::CACHE_MOOD_THEME{$_};
+    }
+    return @data;
+}
+
+# class method to get data for all public themes
+# arguments: class
+# returns: array of hashrefs from memcache or db, undef on failure
+sub public_themes {
+    my ( $self ) = @_;
+    my $memkey = "moods_public";
+    # only ids are in memcache
+    my $ids = LJ::MemCache::get( $memkey );
+    unless ( defined $ids ) {
+        # check database
+        my $dbr = LJ::get_db_reader() or return undef;
+        $ids = $dbr->selectcol_arrayref(
+            "SELECT moodthemeid FROM moodthemes" .
+            " WHERE is_public='Y' ORDER BY name" );
+        die $dbr->errstr if $dbr->err;
+        # update memcache
+        LJ::MemCache::set( $memkey, $ids, 3600 );
+    }
+    return $self->_load_data_multiple( $ids );
+}
+
 
 # END package DW::Mood;
 
 package LJ::User;
+
+# user method for expiring moodtheme cache
+# NOTE: any code that updates the moodthemes table needs to use this!
+sub delete_moodtheme_cache { $_[0]->memc_delete( 'moodthemes' ); }
 
 # user method for creating new mood theme
 # args: theme name, description, errorref
@@ -288,6 +367,7 @@ sub create_moodtheme {
     $sth->execute( $u->id, $name, $desc ) or
         return $errsub->( LJ::Lang::ml( "error.dberror" ) . $dbh->errstr );
 
+    $u->delete_moodtheme_cache;
     return $dbh->{mysql_insertid};
 }
 
diff -r c7431ec0bc18 -r 49f12658bc9d cgi-bin/LJ/Console/Command/MoodthemeList.pm
--- a/cgi-bin/LJ/Console/Command/MoodthemeList.pm	Fri Jun 25 20:41:23 2010 +0800
+++ b/cgi-bin/LJ/Console/Command/MoodthemeList.pm	Fri Jun 25 21:57:59 2010 +0800
@@ -37,15 +37,17 @@ sub execute {
     return $self->error("This command takes at most one argument. Consult the reference.")
         unless scalar(@args) == 0;
 
-    my $dbh = LJ::get_db_reader();
-    my $sth;
-
-    if ($themeid) {
-        $sth = $dbh->prepare("SELECT m.mood, md.moodid, md.picurl, md.width, md.height FROM moodthemedata md, moods m "
-                             . "WHERE md.moodid=m.moodid AND md.moodthemeid = ? ORDER BY m.mood");
-        $sth->execute($themeid);
-        while (my ($mood, $moodid, $picurl, $w, $h) = $sth->fetchrow_array) {
-            $self->info(sprintf("%-20s %2dx%2d %s", "$mood ($moodid)", $w, $h, $picurl));
+    if ( $themeid ) {
+        my $theme = DW::Mood->new( $themeid );
+        return $self->error( "This theme doesn't seem to exist." )
+            unless $theme;
+        foreach ( sort { $a->{name} cmp $b->{name} }
+                       values %{ DW::Mood->get_moods } ) {
+            # make sure the mood is defined in this theme
+            my $data = $theme->prop( $_->{id} );
+            next unless defined $data;
+            $self->info( sprintf( "%-20s %2dx%2d %s", "$_->{name} ($_->{id})",
+                $data->{w}, $data->{h}, $data->{pic} ) );
         }
         return 1;
     }
@@ -55,29 +57,23 @@ sub execute {
     $self->info( "-" x 80);
 
     $self->info("Public themes:");
-    $sth = $dbh->prepare("SELECT mt.moodthemeid, u.user, mt.is_public, mt.name, mt.des FROM moodthemes mt, user u "
-                         . "WHERE mt.ownerid=u.userid AND mt.is_public='Y' ORDER BY mt.moodthemeid");
-    $sth->execute;
-    $self->error("Database error: " . $dbh->errstr)
-        if $dbh->err;
-
-    while (my ($id, $user, $pub, $name, $des) = $sth->fetchrow_array) {
-        $self->info(sprintf("%3s %4s %-15s %-25s %s", $pub, $id, $user, $name, $des));
+    my @public_themes = DW::Mood->public_themes;
+    my $owner = LJ::load_userids( map { $_->{ownerid} } @public_themes );
+    foreach ( @public_themes ) {
+        my $u = $owner->{ $_->{ownerid} };
+        my $user = $u ? $u->user : '';
+        $self->info( sprintf( "%3s %4s %-15s %-25s %s",
+                              $_->{is_public}, $_->{moodthemeid},
+                              $user, $_->{name}, $_->{des} ) );
     }
 
     my $remote = LJ::get_remote();
     if ($remote) {
-        $sth = $dbh->prepare("SELECT mt.moodthemeid, u.user, mt.is_public, mt.name, mt.des FROM moodthemes mt, user u "
-                             . "WHERE mt.ownerid=u.userid AND mt.ownerid = ? ORDER BY mt.moodthemeid");
-        $sth->execute($remote->id);
-
-        $self->error("Database error: " . $dbh->errstr)
-            if $dbh->err;
-
         $self->info("Your themes:");
-
-        while (my ($id, $user, $pub, $name, $des) = $sth->fetchrow_array) {
-            $self->info(sprintf("%3s %4s %-15s %-25s %s", $pub, $id, $user, $name, $des));
+        foreach ( DW::Mood->get_themes( { ownerid => $remote->id } ) ) {
+            $self->info( sprintf( "%3s %4s %-15s %-25s %s",
+                                  $_->{is_public}, $_->{moodthemeid},
+                                  $remote->user, $_->{name}, $_->{des} ) );
         }
     }
 
diff -r c7431ec0bc18 -r 49f12658bc9d cgi-bin/LJ/Console/Command/MoodthemePublic.pm
--- a/cgi-bin/LJ/Console/Command/MoodthemePublic.pm	Fri Jun 25 20:41:23 2010 +0800
+++ b/cgi-bin/LJ/Console/Command/MoodthemePublic.pm	Fri Jun 25 21:57:59 2010 +0800
@@ -56,6 +56,7 @@ sub execute {
     $dbh->do("UPDATE moodthemes SET is_public = ? WHERE moodthemeid = ?", undef, $public, $themeid);
     return $self->error( "Database error: " . $dbh->errstr ) if $dbh->err;
     DW::Mood->clear_cache( $themeid );
+    LJ::MemCache::delete( "moods_public" );
 
     return $self->print("Theme #$themeid marked as $msg.");
 }
diff -r c7431ec0bc18 -r 49f12658bc9d cgi-bin/LJ/Customize.pm
--- a/cgi-bin/LJ/Customize.pm	Fri Jun 25 20:41:23 2010 +0800
+++ b/cgi-bin/LJ/Customize.pm	Fri Jun 25 21:57:59 2010 +0800
@@ -806,20 +806,20 @@ sub validate_moodthemeid {
 # returns: Returns a list of mood themes that the user can select from,
 #          suitable for [func[LJ::html_select]].
 # </LJFUNC>
-sub get_moodtheme_select_list
-{
-    my $class = shift;
-    my $u = shift;
+sub get_moodtheme_select_list {
+    my ( $class, $u ) = @_;
 
-    my $dbr = LJ::get_db_reader();
-    my $sth = $dbr->prepare("SELECT moodthemeid, name FROM moodthemes WHERE is_public='Y' ORDER BY name");
-    $sth->execute;
+    # faster to get full cached data, but we only want id & name
+    my $strip = sub {
+        return { moodthemeid => $_[0]->{moodthemeid},
+                 name => $_[0]->{name} };
+    };
 
     my @themes;
-    while (my $moodtheme = $sth->fetchrow_hashref) {
+    foreach my $moodtheme ( DW::Mood->public_themes ) {
         my $is_active = LJ::Hooks::run_hook("mood_theme_is_active", $moodtheme->{moodthemeid});
         next unless !defined $is_active || $is_active;
-        push @themes, $moodtheme;
+        push @themes, $strip->( $moodtheme );
     }
     LJ::Hooks::run_hook('modify_mood_theme_list', \@themes, user => $u, add_seps => 1);
     unshift @themes, { 'moodthemeid' => 0, 'name' => '(None)' };
@@ -827,9 +827,10 @@ sub get_moodtheme_select_list
     ### user's private themes
     {
         my @theme_user;
-        $sth = $dbr->prepare("SELECT moodthemeid, name FROM moodthemes WHERE ownerid=? AND is_public='N'");
-        $sth->execute($u->{'userid'});
-        push @theme_user, $_ while ($_ = $sth->fetchrow_hashref);
+        foreach ( DW::Mood->get_themes( { ownerid => $u->id } ) ) {
+            next if $_->{is_public} eq 'Y';
+            push @theme_user, $strip->( $_ );
+        }
         if (@theme_user) {
             push @themes, { 'moodthemeid' => 0, 'name' => "--- " . BML::ml('/modify_do.bml.moodicons.personal'). " ---", disabled => 1 };
             push @themes, @theme_user;
diff -r c7431ec0bc18 -r 49f12658bc9d htdocs/manage/moodthemes.bml
--- a/htdocs/manage/moodthemes.bml	Fri Jun 25 20:41:23 2010 +0800
+++ b/htdocs/manage/moodthemes.bml	Fri Jun 25 21:57:59 2010 +0800
@@ -284,11 +284,9 @@ body<=
             }
         }
         elsif ($POST{'isnew'} != 1) { # Make sure they can even edit this theme and fill in the $info variable for later use
-            my $dbr = LJ::get_db_reader();
-            my $sth = $dbr->prepare("SELECT name FROM moodthemes WHERE moodthemeid=? AND ownerid=?");
-            $sth->execute($themeid, $u->{'userid'});
-            $info = $sth->fetchrow_hashref;
-            return LJ::bad_input($ML{'.error.notyourtheme'})
+            $info = DW::Mood->get_themes( { themeid => $themeid,
+                                            ownerid => $u->id } );
+            return LJ::bad_input( $ML{'.error.notyourtheme'} )
                 unless defined $info;
         }
 
@@ -394,6 +392,7 @@ body<=
 
             # Kill any memcache data about this moodtheme
             DW::Mood->clear_cache( $themeid );
+            $u->delete_moodtheme_cache;
 
             $ret .= "<br />$ML{'.saved'}<br /><br />\n";
             return BML::redirect( $self_uri );
@@ -431,11 +430,7 @@ body<=
             $ret .= "<?h1 $ML{'.yourthemes.header'} h1?>\n";
 
             # Get data to figure out if they have any themes already
-            my $dbr = LJ::get_db_reader();
-            my $sth = $dbr->prepare("SELECT moodthemeid, name FROM moodthemes WHERE ownerid=?");
-            $sth->execute($u->{userid});
-            my @user_themes = ();
-            push @user_themes, $_ while $_ = $sth->fetchrow_hashref;
+            my @user_themes = DW::Mood->get_themes( { ownerid => $u->id } );
 
             if (@user_themes) { # The have some custom themes already defined
                 $ret .= "<form action='$self_uri' method='post' id='selectform' name='selectform' style='margin-left: 30px;'>\n";
diff -r c7431ec0bc18 -r 49f12658bc9d htdocs/moodlist.bml
--- a/htdocs/moodlist.bml	Fri Jun 25 20:41:23 2010 +0800
+++ b/htdocs/moodlist.bml	Fri Jun 25 21:57:59 2010 +0800
@@ -42,8 +42,6 @@ body<=
     LJ::set_active_crumb('moodlist');
 
     my $ret;
-    my $sth;
-    my $dbr = LJ::get_db_reader();
 
     my $remote = LJ::get_remote();
     my $journal = $GET{'journal'} || $remote->{'user'};
@@ -62,11 +60,8 @@ body<=
         push @mlist, $m;
     }
 
-    # FIXME: cache this.  it's small.
-    $sth = $dbr->prepare("SELECT moodthemeid, name, is_public FROM moodthemes WHERE is_public='Y'");
-    $sth->execute;
     my @themes = ();
-    while (my $moodtheme = $sth->fetchrow_hashref) {
+    foreach my $moodtheme ( DW::Mood->public_themes ) {
         my $is_active = LJ::Hooks::run_hook("mood_theme_is_active", $moodtheme->{moodthemeid});
         next unless !defined $is_active || $is_active;
         push @themes, $moodtheme;
@@ -179,19 +174,11 @@ body<=
         $add_header->();
 
         # Check if the user is logged in and didn't specify an owner.  If so append their private mood themes
-        my $sth;
-        if ($remote && ! $GET{'ownerid'}) {
-            # FIXME: cache this.  it's small.
-            $sth = $dbr->prepare("SELECT moodthemeid, name, is_public FROM moodthemes WHERE ownerid=?");
-            $sth->execute($remote->{userid});
-            @user_themes = ();
-            push @user_themes, $_ while $_ = $sth->fetchrow_hashref;
-        } elsif ($GET{'ownerid'}) {
-            # FIXME: cache this.  it's small.
-            $sth = $dbr->prepare("SELECT moodthemeid, name, is_public FROM moodthemes WHERE ownerid=? AND moodthemeid=?");
-            $sth->execute($GET{'ownerid'}, $GET{'moodtheme'});
-            @user_themes = ();
-            push @user_themes, $_ while $_ = $sth->fetchrow_hashref;
+        if ( $remote && ! $GET{ownerid} ) {
+            @user_themes = DW::Mood->get_themes( { ownerid => $remote->id } );
+        } elsif ( $GET{ownerid} ) {
+            @user_themes = DW::Mood->get_themes( { themeid => $GET{moodtheme},
+                                                   ownerid => $GET{ownerid} } );
         }
 
         # Sort the user themes
--------------------------------------------------------------------------------