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
--------------------------------------------------------------------------------
  • Add Memory
  • Share This Entry
(0 comments)

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

 
Notice: This account is set to log the IP addresses of everyone who comments.
Links will be displayed as unclickable URLs to help prevent spam.