Avatar resizing improvements and better code reuse

* getOriginal added to Avatar class
    This is a static function that retrieves the original avatar in a leaner
    way than Profile->getOriginalAvatar() did (see below).
    This will throw an Exception if there was none to be found.

* getProfileAvatars added to Avatar class
    This gets all Avatars from a profile and returns them in an array.

* newSize added to Avatar class
    This will scale an original avatar or throw an Exception (originally from
    Avatar::getOriginal) if one wasn't found.

* deleteFromProfile added to Avatar class
    Deletes all avatars for a Profile. This makes the code much smarter when
    removing all avatars from a user.
    Previously only specific, hardcoded (through constants) sizes would be
    deleted. If you ever changed lib/framework.php then many oddsized avatars
    would remain with the old method.

* Migrated Profile class to new Avatar::getOriginal support
    Profile class now uses Avatar::getOriginal through its own
    $this->getOriginalAvatar and thus remains backwards compatible.

* Updating stock GNU Social to use Avatar::getOriginal
    All places where core StatusNet code used the
    $profile->getOriginalAvatar, it will now useAvatar::getOriginal with
    proper error handling.

* Updated Profile class to use Avatar::newSize
    When doing setOriginal, the scaling will be done with the new method
    introduced in this merge.
    This also edits the _fillAvatar function to avoid adding NULL values to
    the array (which causes errors when attempting to access array entries as
    objects). See issue #3478 at http://status.net/open-source/issues/3478

diff --git a/actions/avatarsettings.php b/actions/avatarsettings.php
index 663a14ae3a06b5de385d00aa61aa909c0e32fb8f..77c6681c3c8d0e548f8290992704802fafc6d6ee 100644 (file)
--- a/actions/avatarsettings.php
+++ b/actions/avatarsettings.php
@@ -109,8 +109,6 @@ class AvatarsettingsAction extends SettingsAction
             return;
         }
 
-        $original = $profile->getOriginalAvatar();
-
         $this->elementStart('form', array('enctype' => 'multipart/form-data',
                                           'method' => 'post',
                                           'id' => 'form_settings_avatar',
@@ -124,7 +122,9 @@ class AvatarsettingsAction extends SettingsAction
 
         if (Event::handle('StartAvatarFormData', array($this))) {
             $this->elementStart('ul', 'form_data');
-            if ($original) {
+            try {
+                $original = Avatar::getOriginal($profile);
+
                 $this->elementStart('li', array('id' => 'avatar_original',
                                                 'class' => 'avatar_view'));
                 // TRANS: Header on avatar upload page for thumbnail of originally uploaded avatar (h2).
@@ -136,6 +136,8 @@ class AvatarsettingsAction extends SettingsAction
                                             'alt' => $user->nickname));
                 $this->elementEnd('div');
                 $this->elementEnd('li');
+            } catch (NoResultException $e) {
+                // No original avatar found!
             }
 
             $avatar = $profile->getAvatar(AVATAR_PROFILE_SIZE);
@@ -195,8 +197,6 @@ class AvatarsettingsAction extends SettingsAction
             return;
         }
 
-        $original = $profile->getOriginalAvatar();
-
         $this->elementStart('form', array('method' => 'post',
                                           'id' => 'form_settings_avatar',
                                           'class' => 'form_settings',
@@ -402,14 +402,7 @@ class AvatarsettingsAction extends SettingsAction
         $user = common_current_user();
         $profile = $user->getProfile();
 
-        $avatar = $profile->getOriginalAvatar();
-        if($avatar) $avatar->delete();
-        $avatar = $profile->getAvatar(AVATAR_PROFILE_SIZE);
-        if($avatar) $avatar->delete();
-        $avatar = $profile->getAvatar(AVATAR_STREAM_SIZE);
-        if($avatar) $avatar->delete();
-        $avatar = $profile->getAvatar(AVATAR_MINI_SIZE);
-        if($avatar) $avatar->delete();
+        Avatar::deleteFromProfile($profile);
 
         // TRANS: Success message for deleting a user avatar.
         $this->showForm(_('Avatar deleted.'), true);

diff --git a/actions/foaf.php b/actions/foaf.php
index 77e27796db059cd88ec5c1b34625f5366b353137..7f76c222cb29951405c0675893c8dea45f8b5b08 100644 (file)
--- a/actions/foaf.php
+++ b/actions/foaf.php
@@ -141,7 +141,7 @@ class FoafAction extends Action
             $this->elementEnd('based_near');
         }
 
-        $avatar = $this->profile->getOriginalAvatar();
+        $avatar = Avatar::getOriginal($this->profile);
         if ($avatar) {
             $this->elementStart('img');
             $this->elementStart('Image', array('rdf:about' => $avatar->url));

diff --git a/classes/Avatar.php b/classes/Avatar.php
index aed6ea0dd2892ec3702c3d09117c6c34bc267f8c..40f9bc6bc957aafc7005a77cf2e4853dcd0a3aaf 100644 (file)
--- a/classes/Avatar.php
+++ b/classes/Avatar.php
@@ -59,6 +59,30 @@ class Avatar extends Managed_DataObject
         }
     }
 
+    public static function deleteFromProfile(Profile $target) {
+        $avatars = Avatar::getProfileAvatars($target->id);
+        foreach ($avatars as $avatar) {
+            $avatar->delete();
+        }
+    }
+
+    public static function getOriginal(Profile $target)
+    {
+        $avatar = new Avatar();
+        $avatar->profile_id = $target->id;
+        $avatar->original = true;
+        if (!$avatar->find(true)) {
+            throw new NoResultException($avatar);
+        }
+        return $avatar;
+    }
+
+    public static function getProfileAvatars(Profile $target) {
+        $avatar = new Avatar();
+        $avatar->profile_id = $target->id;
+        return $avatar->fetchAll();
+    }
+
     /**
      * Where should the avatar go for this user?
      */
@@ -133,4 +157,32 @@ class Avatar extends Managed_DataObject
                                   AVATAR_MINI_SIZE => 'mini');
         return Theme::path('default-avatar-'.$sizenames[$size].'.png');
     }
+
+    static function newSize(Profile $target, $size) {
+        $size = floor($size);
+        if ($size <1 || $size > 999) {
+            // TRANS: An error message when avatar size is unreasonable
+            throw new Exception(_m('Unreasonable avatar size'));
+        }
+
+        $original = Avatar::getOriginal($target);
+
+        $imagefile = new ImageFile($target->id, Avatar::path($original->filename));
+        $filename = $imagefile->resize($size);
+
+        $scaled = clone($original);
+        $scaled->original = false;
+        $scaled->width = $size;
+        $scaled->height = $size;
+        $scaled->url = Avatar::url($filename);
+        $scaled->created = DB_DataObject_Cast::dateTime();
+
+        if (!$scaled->insert()) {
+            // TRANS: An error message when unable to insert avatar data into the db
+            throw new Exception(_m('Could not insert new avatar data to database'));
+        }
+
+        // Return the new avatar object
+        return $scaled;
+    }
 }

diff --git a/classes/Profile.php b/classes/Profile.php
index 79eb00d22a48187e02ac0512f4540aabd71438aa..5d8e46a7410d161b493477b26a6ae760238164c6 100644 (file)
--- a/classes/Profile.php
+++ b/classes/Profile.php
@@ -118,7 +118,7 @@ class Profile extends Managed_DataObject
 
     protected $_avatars;
 
-    function getAvatar($width, $height=null)
+    public function getAvatar($width, $height=null)
     {
         if (is_null($height)) {
             $height = $width;
@@ -127,7 +127,6 @@ class Profile extends Managed_DataObject
         $avatar = $this->_getAvatar($width);
 
         if (empty($avatar)) {
-
             if (Event::handle('StartProfileGetAvatar', array($this, $width, &$avatar))) {
                 $avatar = Avatar::pkeyGet(
                     array(
@@ -139,6 +138,17 @@ class Profile extends Managed_DataObject
                 Event::handle('EndProfileGetAvatar', array($this, $width, &$avatar));
             }
 
+            // if-empty within an if-empty? Let's find a prettier solution...
+            if (empty($avatar)) {
+                // Obviously we can't find an avatar, so let's resize the original!
+                try {
+                    $avatar = Avatar::newSize($this, $width);
+                } catch (Exception $e) {
+                    // Could not generate a resized avatar. How do we handle it?
+                }
+            }
+
+            // cache the avatar for future use
             $this->_fillAvatar($width, $avatar);
         }
 
@@ -164,21 +174,18 @@ class Profile extends Managed_DataObject
         return null;
     }
 
-    function _fillAvatar($width, $avatar)
+    protected function _fillAvatar($width, Avatar $avatar)
     {
-      //common_debug("Storing avatar of width: {$avatar->width} and profile_id {$avatar->profile_id} in profile {$this->id}.");
-        $this->_avatars[$width] = $avatar;
-
+        // This avoids storing null values, a problem report in issue #3478
+           $this->_avatars[$width] = $avatar;
     }
 
-    function getOriginalAvatar()
+    // For backwards compatibility only!
+    public function getOriginalAvatar()
     {
-        $pkey = array('profile_id' => $this->id,
-                      'original'   => true);
-        $avatar = Avatar::pkeyGet($pkey);
-        if (!empty($avatar)) {
-            return $avatar;
-        } else {
+        try {
+            return Avatar::getOriginal($this);
+        } catch (Exception $e) {
             return null;
         }
     }
@@ -207,21 +214,10 @@ class Profile extends Managed_DataObject
         foreach (array(AVATAR_PROFILE_SIZE, AVATAR_STREAM_SIZE, AVATAR_MINI_SIZE) as $size) {
             // We don't do a scaled one if original is our scaled size
             if (!($avatar->width == $size && $avatar->height == $size)) {
-                $scaled_filename = $imagefile->resize($size);
-
-                //$scaled = DB_DataObject::factory('avatar');
-                $scaled = new Avatar();
-                $scaled->profile_id = $this->id;
-                $scaled->width = $size;
-                $scaled->height = $size;
-                $scaled->original = false;
-                $scaled->mediatype = image_type_to_mime_type($imagefile->type);
-                $scaled->filename = $scaled_filename;
-                $scaled->url = Avatar::url($scaled_filename);
-                $scaled->created = DB_DataObject_Cast::dateTime(); # current time
-
-                if (!$scaled->insert()) {
-                    return null;
+                try {
+                    Avatar::newSize($this, $size);
+                } catch (Exception $e) {
+                    // should we abort the generation and live without smaller avatars?
                 }
             }
         }

diff --git a/lib/activityobject.php b/lib/activityobject.php
index ad7e636696fc6070590c5e5001cf578b406e1dfd..0b29888083d03b04d7d5bdf47748d0faf873c7a7 100644 (file)
--- a/lib/activityobject.php
+++ b/lib/activityobject.php
@@ -459,10 +459,11 @@ class ActivityObject
             $object->title  = $profile->getBestName();
             $object->link   = $profile->profileurl;
 
-            $orig = $profile->getOriginalAvatar();
-
-            if (!empty($orig)) {
+            try {
+                $orig = Avatar::getOriginal($profile);
                 $object->avatarLinks[] = AvatarLink::fromAvatar($orig);
+            } catch (Exception $e) {
+                // Could not find an original avatar to link
             }
 
             $sizes = array(

diff --git a/plugins/OMB/lib/omboauthdatastore.php b/plugins/OMB/lib/omboauthdatastore.php
index 857748bf4af7ec38d90d24dbc4a11c924a1e9a85..9603986ba7b2cf0ec52ea607c1af708211186980 100644 (file)
--- a/plugins/OMB/lib/omboauthdatastore.php
+++ b/plugins/OMB/lib/omboauthdatastore.php
@@ -289,14 +289,7 @@ class OMBOAuthDataStore extends OAuthDataStore
                     throw new Exception(_('Error inserting avatar.'));
                 }
             } else {
-                $avatar = $profile->getOriginalAvatar();
-                if($avatar) $avatar->delete();
-                $avatar = $profile->getAvatar(AVATAR_PROFILE_SIZE);
-                if($avatar) $avatar->delete();
-                $avatar = $profile->getAvatar(AVATAR_STREAM_SIZE);
-                if($avatar) $avatar->delete();
-                $avatar = $profile->getAvatar(AVATAR_MINI_SIZE);
-                if($avatar) $avatar->delete();
+                Avatar::deleteFromProfile($profile);
             }
 
             if ($exists) {