![[site community profile]](https://www.dreamwidth.org/img/comm_staff.png){kind=small}
[dw-free] cleaning up userpics code
[commit: http://hg.dwscoalition.org/dw-free/rev/094089044eb8]
http://bugs.dwscoalition.org/show_bug.cgi?id=513
Modernize/refactor userpics code. Use object methods rather than class
methods.
Patch by![[personal profile]](https://www.dreamwidth.org/img/silk/identity/user.png)
kareila.
Files modified:
http://bugs.dwscoalition.org/show_bug.cgi?id=513
Modernize/refactor userpics code. Use object methods rather than class
methods.
Patch by
kareila.Files modified:
- cgi-bin/Apache/LiveJournal.pm
- cgi-bin/DW/Logic/ProfilePage.pm
- cgi-bin/LJ/Comment.pm
- cgi-bin/LJ/Talk.pm
- cgi-bin/LJ/User.pm
- cgi-bin/LJ/Userpic.pm
- cgi-bin/ljprotocol.pl
- cgi-bin/ljuserpics.pl
--------------------------------------------------------------------------------
diff -r 55b77d726f86 -r 094089044eb8 cgi-bin/Apache/LiveJournal.pm
--- a/cgi-bin/Apache/LiveJournal.pm Tue Sep 07 16:24:26 2010 +0800
+++ b/cgi-bin/Apache/LiveJournal.pm Tue Sep 07 16:41:24 2010 +0800
@@ -1136,10 +1136,7 @@ sub userpic_content
my $u = LJ::load_userid($userid);
return NOT_FOUND unless $u && ! ( $u->is_expunged || $u->is_suspended );
- my %upics;
- LJ::load_userpics(\%upics, [ $u, $picid ]);
- my $pic = $upics{$picid} or return NOT_FOUND;
- return NOT_FOUND if $pic->{'userid'} != $userid || $pic->{state} eq 'X';
+ my $pic = LJ::Userpic->get( $u, $picid ) or return NOT_FOUND;
# Read the mimetype from the pichash if dversion 7
$mime = { 'G' => 'image/gif',
diff -r 55b77d726f86 -r 094089044eb8 cgi-bin/DW/Logic/ProfilePage.pm
--- a/cgi-bin/DW/Logic/ProfilePage.pm Tue Sep 07 16:24:26 2010 +0800
+++ b/cgi-bin/DW/Logic/ProfilePage.pm Tue Sep 07 16:41:24 2010 +0800
@@ -106,7 +106,7 @@ sub userpic {
# now determine what caption text to show
if ( $remote && $remote->can_manage( $u ) ) {
- if ( LJ::userpic_count( $u ) ) {
+ if ( $u->get_userpic_count ) {
$ret->{userpic_url} = $u->allpics_base;
$ret->{caption_text} = LJ::Lang::ml( '.section.edit' );
$ret->{caption_url} = "$LJ::SITEROOT/editicons?authas=$user"
@@ -116,7 +116,7 @@ sub userpic {
$ret->{caption_url} = "$LJ::SITEROOT/editicons?authas=$user"
}
} else {
- if ( LJ::userpic_count( $u ) ) {
+ if ( $u->get_userpic_count ) {
$ret->{userpic_url} = $u->allpics_base;
}
}
@@ -290,7 +290,7 @@ sub userpic_stats {
my $u = $self->{u};
my @ret;
- my $ct = LJ::userpic_count( $u );
+ my $ct = $u->get_userpic_count;
push @ret, LJ::Lang::ml( '.details.userpics', {
num_raw => $ct,
num_comma => LJ::commafy( $ct ),
diff -r 55b77d726f86 -r 094089044eb8 cgi-bin/LJ/Comment.pm
--- a/cgi-bin/LJ/Comment.pm Tue Sep 07 16:24:26 2010 +0800
+++ b/cgi-bin/LJ/Comment.pm Tue Sep 07 16:41:24 2010 +0800
@@ -1408,18 +1408,11 @@ sub _format_mail_both {
my $pichtml;
my $pic_kw = $self->prop('picture_keyword');
- if ($posteru && $posteru->{defaultpicid} || $pic_kw) {
- my $pic = $pic_kw ? LJ::get_pic_from_keyword($posteru, $pic_kw) : undef;
- my $picid = $pic ? $pic->{picid} : $posteru->{defaultpicid};
- unless ($pic) {
- my %pics;
- LJ::load_userpics(\%pics, [ $posteru, $posteru->{defaultpicid} ]);
- $pic = $pics{$picid};
- # load_userpics doesn't return picid, but we rely on it above
- $picid = $picid;
- }
- if ($pic) {
- $pichtml = "<img src=\"$LJ::USERPIC_ROOT/$picid/$pic->{userid}\" align='absmiddle' ".
+ if ( $posteru ) {
+ my $pic = LJ::Userpic->new_from_keyword( $posteru, $pic_kw ) || $posteru->userpic;
+
+ if ( $pic && $pic->load_row ) {
+ $pichtml = "<img src=\"$LJ::USERPIC_ROOT/$pic->{picid}/$pic->{userid}\" align='absmiddle' ".
"width='$pic->{width}' height='$pic->{height}' ".
"hspace='1' vspace='2' alt='' /> ";
}
diff -r 55b77d726f86 -r 094089044eb8 cgi-bin/LJ/Talk.pm
--- a/cgi-bin/LJ/Talk.pm Tue Sep 07 16:24:26 2010 +0800
+++ b/cgi-bin/LJ/Talk.pm Tue Sep 07 16:41:24 2010 +0800
@@ -3522,9 +3522,10 @@ sub post_comment {
# they don't have a duplicate...
unless ($jtalkid) {
+ my ( $posteru, $kw ) = ( $comment->{u}, $comment->{picture_keyword} );
# XXX do select and delete $talkprop{'picture_keyword'} if they're lying
- my $pic = LJ::get_pic_from_keyword($comment->{u}, $comment->{picture_keyword});
- delete $comment->{picture_keyword} unless $pic && $pic->{'state'} eq 'N';
+ my $pic = LJ::Userpic->new_from_keyword( $posteru, $kw );
+ delete $comment->{picture_keyword} unless $pic && $pic->state eq 'N';
$comment->{pic} = $pic;
# put the post in the database
diff -r 55b77d726f86 -r 094089044eb8 cgi-bin/LJ/User.pm
--- a/cgi-bin/LJ/User.pm Tue Sep 07 16:24:26 2010 +0800
+++ b/cgi-bin/LJ/User.pm Tue Sep 07 16:41:24 2010 +0800
@@ -6322,7 +6322,6 @@ use Carp;
### 19. OpenID and Identity Functions
### 21. Password Functions
### 24. Styles and S2-Related Functions
-### 28. Userpic-Related Functions
########################################################################
### 1. Creating and Deleting Accounts
@@ -8727,28 +8726,4 @@ sub make_journal {
}
-########################################################################
-### 28. Userpic-Related Functions
-
-=head2 Userpic-Related Functions (LJ)
-=cut
-
-# <LJFUNC>
-# name: LJ::userpic_count
-# des: Gets a count of userpics for a given user.
-# args: dbarg?, upics, idlist
-# des-upics: hashref to load pictures into, keys being the picids
-# des-idlist: [$u, $picid] or [[$u, $picid], [$u, $picid], +] objects
-# also supports deprecated old method, of an array ref of picids.
-# </LJFUNC>
-sub userpic_count {
- my $u = shift or return undef;
-
- my $dbcr = LJ::get_cluster_def_reader( $u ) or return undef;
- return $dbcr->selectrow_array( "SELECT COUNT(*) FROM userpic2 " .
- "WHERE userid=? AND state <> 'X'",
- undef, $u->userid );
-}
-
-
1;
diff -r 55b77d726f86 -r 094089044eb8 cgi-bin/LJ/Userpic.pm
--- a/cgi-bin/LJ/Userpic.pm Tue Sep 07 16:24:26 2010 +0800
+++ b/cgi-bin/LJ/Userpic.pm Tue Sep 07 16:41:24 2010 +0800
@@ -81,14 +81,18 @@ sub instance {
# LJ::Userpic accessor. Returns a LJ::Userpic object indicated by $picid, or
# undef if userpic doesn't exist in the db.
+# TODO: add in lazy peer loading here?
sub get {
- my ($class, $u, $picid) = @_;
+ my ( $class, $u, $picid ) = @_;
+ return unless LJ::isu( $u );
+ return if $u->is_expunged;
- my @cache = LJ::Userpic->load_user_userpics($u);
+ my @cache = $class->load_user_userpics( $u );
if (@cache) {
+ my $obj = ref $class ? $class : $class->new( $u, $picid );
foreach my $curr (@cache) {
- return LJ::Userpic->new( $u, $curr->{picid} ) if $curr->{picid} == $picid;
+ return $obj->absorb_row( $curr ) if $curr->{picid} == $picid;
}
}
@@ -272,13 +276,9 @@ sub dimensions {
# width and height probably loaded from DB
return ($self->{width}, $self->{height}) if ($self->{width} && $self->{height});
- my %upics;
- my $u = LJ::load_userid($self->{userid});
- LJ::load_userpics(\%upics, [ $u, $self->{picid} ]);
- my $up = $upics{$self->{picid}} or
- return ();
-
- return ($up->{width}, $up->{height});
+ # if not, load them explicitly
+ $self->load_row;
+ return ( $self->{width}, $self->{height} );
}
sub max_allowed_bytes {
@@ -488,30 +488,12 @@ sub imagedata {
return $data ? $data : undef;
}
-# TODO: add in lazy peer loading here
+# get : class :: load_row : object
sub load_row {
my $self = shift;
- my $u = $self->owner;
- return unless defined $u;
- return if $u->is_expunged;
- # Load all of the userpics from cache, or load them from the database and write them to cache
- my @cache = LJ::Userpic->load_user_userpics($u);
-
- if (@cache) {
- foreach my $curr (@cache) {
- return $self->absorb_row($curr) if $curr->{picid} eq $self->picid;
- }
- }
-
- # If you get past this conditional something is wrong
- # load_user_userpics always returns a value
-
- my $row = $u->selectrow_hashref( "SELECT userid, picid, width, height, state, fmt, comment, description, location, url, " .
- "UNIX_TIMESTAMP(picdate) AS 'pictime', flags, md5base64 " .
- "FROM userpic2 WHERE userid=? AND picid=?", undef,
- $u->userid, $self->{picid} );
- $self->absorb_row($row) if $row;
+ # use class method
+ return $self->get( $self->owner, $self->picid );
}
# checks request cache and memcache,
@@ -572,28 +554,25 @@ sub load_user_userpics {
sub load_user_userpics {
my ($class, $u) = @_;
local $LJ::THROW_ERRORS = 1;
- my @ret;
my $cache = $class->get_cache($u);
return @$cache if $cache;
- # select all of their userpics and iterate through them
- my $sth = $u->prepare( "SELECT userid, picid, width, height, state, fmt, comment, description, location, " .
- "UNIX_TIMESTAMP(picdate) AS 'pictime', flags, md5base64 " .
- "FROM userpic2 WHERE userid=?" );
- $sth->execute( $u->userid );
- die "Error loading userpics: clusterid=$u->{clusterid}, errstr=" . $sth->errstr if $sth->err;
+ # select all of their userpics
+ my $data = $u->selectall_hashref(
+ "SELECT userid, picid, width, height, state, fmt, comment," .
+ " description, location, url, UNIX_TIMESTAMP(picdate) AS 'pictime'," .
+ " flags, md5base64 FROM userpic2 WHERE userid=? AND state <> 'X'",
+ 'picid', undef, $u->userid );
+ die "Error loading userpics: clusterid=$u->{clusterid}, errstr=" . $u->errstr
+ if $u->err;
- while (my $rec = $sth->fetchrow_hashref) {
- # ignore anything expunged
- next if $rec->{state} eq 'X';
- push @ret, $rec;
- }
+ my @ret = values %$data;
# set cache if reasonable
$class->set_cache($u, \@ret);
- return map { LJ::Userpic->new_from_row($_) } @ret;
+ return map { $class->new_from_row($_) } @ret;
}
sub create {
diff -r 55b77d726f86 -r 094089044eb8 cgi-bin/ljprotocol.pl
--- a/cgi-bin/ljprotocol.pl Tue Sep 07 16:24:26 2010 +0800
+++ b/cgi-bin/ljprotocol.pl Tue Sep 07 16:41:24 2010 +0800
@@ -1065,12 +1065,13 @@ sub common_event_validation
# check props for inactive userpic
if ( ( my $pickwd = $req->{'props'}->{'picture_keyword'} ) and !$flags->{allow_inactive}) {
- my $pic = LJ::get_pic_from_keyword($flags->{'u'}, $pickwd);
+ my $pic = LJ::Userpic->new_from_keyword( $flags->{u}, $pickwd );
# need to make sure they aren't trying to post with an inactive keyword, but also
# we don't want to allow them to post with a keyword that has no pic at all to prevent
# them from deleting the keyword, posting, then adding it back with editicons.bml
- delete $req->{'props'}->{'picture_keyword'} if ! $pic || $pic->{'state'} eq 'I';
+ delete $req->{props}->{picture_keyword}
+ unless $pic && $pic->state ne 'I';
}
# validate incoming list of tags
diff -r 55b77d726f86 -r 094089044eb8 cgi-bin/ljuserpics.pl
--- a/cgi-bin/ljuserpics.pl Tue Sep 07 16:24:26 2010 +0800
+++ b/cgi-bin/ljuserpics.pl Tue Sep 07 16:41:24 2010 +0800
@@ -393,34 +393,6 @@ sub get_userpic_info
return $info;
}
-# <LJFUNC>
-# name: LJ::get_pic_from_keyword
-# des: Given a userid and keyword, returns the pic row hashref.
-# args: u, keyword
-# des-keyword: The keyword of the userpic to fetch.
-# returns: hashref of pic row found
-# </LJFUNC>
-sub get_pic_from_keyword
-{
- my ($u, $kw) = @_;
- my $info = LJ::get_userpic_info($u) or
- return undef;
-
- if (my $pic = $info->{'kw'}{$kw}) {
- return $pic;
- }
-
- # the lame "pic#2343" thing when they didn't assign a keyword
- if ($kw =~ /^pic\#(\d+)$/) {
- my $picid = $1;
- if (my $pic = $info->{'pic'}{$picid}) {
- return $pic;
- }
- }
-
- return undef;
-}
-
sub get_picid_from_keyword
{
my ($u, $kw, $default) = @_;
--------------------------------------------------------------------------------