Radical differences in Bookmark storage

Had some problems with PuSH and Salmon use of Bookmarks; they were
being required to generate Atom versions of the bookmark _before_ the bookmark was saved.

So, I reversed the order of how things are saved, and associate notices and bookmarks
by URI rather than notice_id.

plugins/Bookmark/Bookmark.php

@@ -43,12 +43,16 @@ if (!defined('STATUSNET')) {
  * @see      DB_DataObject
  */
 
-class Notice_bookmark extends Memcached_DataObject
+class Bookmark extends Memcached_DataObject
 {
-    public $__table = 'notice_bookmark'; // table name
+    public $__table = 'bookmark'; // table name
-    public $notice_id;                   // int(4)  primary_key not_null
+    public $profile_id;           // int(4)  primary_key not_null
+    public $url;                  // varchar(255) primary_key not_null
     public $title;                // varchar(255)
     public $description;          // text
+    public $uri;                  // varchar(255)
+    public $url_crc32;            // int(4) not_null
+    public $created;              // datetime
 
     /**
      * Get an instance by key
@@ -64,7 +68,7 @@ class Notice_bookmark extends Memcached_DataObject
 
     function staticGet($k, $v=null)
     {
-        return Memcached_DataObject::staticGet('Notice_bookmark', $k, $v);
+        return Memcached_DataObject::staticGet('Bookmark', $k, $v);
     }
 
     /**
@@ -78,9 +82,13 @@ class Notice_bookmark extends Memcached_DataObject
 
     function table()
     {
-        return array('notice_id' => DB_DATAOBJECT_INT + DB_DATAOBJECT_NOTNULL,
+        return array('profile_id' => DB_DATAOBJECT_INT + DB_DATAOBJECT_NOTNULL,
+                     'url' => DB_DATAOBJECT_STR,
                      'title' => DB_DATAOBJECT_STR,
-                     'description' => DB_DATAOBJECT_STR);
+                     'description' => DB_DATAOBJECT_STR,
+                     'uri' => DB_DATAOBJECT_STR,
+                     'url_crc32' => DB_DATAOBJECT_INT + DB_DATAOBJECT_NOTNULL,
+                     'created' => DB_DATAOBJECT_STR + DB_DATAOBJECT_DATE + DB_DATAOBJECT_TIME + DB_DATAOBJECT_NOTNULL);
     }
 
     /**
@@ -102,7 +110,9 @@ class Notice_bookmark extends Memcached_DataObject
 
     function keyTypes()
     {
-        return array('notice_id' => 'K');
+        return array('profile_id' => 'K',
+                     'url' => 'K',
+                     'uri' => 'U');
     }
 
     /**
@@ -116,39 +126,62 @@ class Notice_bookmark extends Memcached_DataObject
         return array(false, false, false);
     }
 
+    /**
+     * Get a bookmark based on a notice
+     * 
+     * @param Notice $notice Notice to check for
+     *
+     * @return Bookmark found bookmark or null
+     */
+    
+    function getByNotice($notice)
+    {
+        return self::staticGet('uri', $notice->uri);
+    }
+
     /**
      * Get the bookmark that a user made for an URL
      *
      * @param Profile $profile Profile to check for
      * @param string  $url     URL to check for
      *
-     * @return Notice_bookmark bookmark found or null
+     * @return Bookmark bookmark found or null
      */
      
     static function getByURL($profile, $url)
     {
-        $file = File::staticGet('url', $url);
+        return self::pkeyGet(array('profile_id' => $profile->id,
-        if (!empty($file)) {
+                                   'url' => $url));
-            $f2p = new File_to_post();
-
-            $f2p->file_id = $file->id;
-            if ($f2p->find()) {
-                while ($f2p->fetch()) {
-                    $n = Notice::staticGet('id', $f2p->post_id);
-                    if (!empty($n)) {
-                        if ($n->profile_id == $profile->id) {
-                            $nb = Notice_bookmark::staticGet('notice_id', $n->id);
-                            if (!empty($nb)) {
-                                return $nb;
-                            }
-                        }
-                    }
-                }
-            }
-        }
         return null;
     }
 
+    /**
+     * Get the bookmark that a user made for an URL
+     *
+     * @param Profile $profile Profile to check for
+     * @param integer $crc32   CRC-32 of URL to check for
+     *
+     * @return array Bookmark objects found (usually 1 or 0)
+     */
+     
+    static function getByCRC32($profile, $crc32)
+    {
+        $bookmarks = array();
+
+        $nb = new Bookmark();
+        
+        $nb->profile_id = $profile->id;
+        $nb->url_crc32  = $crc32;
+
+        if ($nb->find()) {
+            while ($nb->fetch()) {
+                $bookmarks[] = clone($nb);
+            }
+        }
+
+        return $bookmarks;
+    }
+
     /**
      * Save a new notice bookmark
      *
@@ -179,6 +212,32 @@ class Notice_bookmark extends Memcached_DataObject
             $rawtags = preg_split('/[\s,]+/', $rawtags);
         }
 
+        $nb = new Bookmark();
+
+        $nb->profile_id  = $profile->id;
+        $nb->url         = $url;
+        $nb->title       = $title;
+        $nb->description = $description;
+        $nb->url_crc32   = crc32($nb->url);
+        $nb->created     = common_sql_now();
+
+        if (array_key_exists('uri', $options)) {
+            $nb->uri = $options['uri'];
+        } else {
+            $dt = new DateTime($nb->created);
+            // I posit that it's sufficiently impossible
+            // for the same user to generate two CRC-32-clashing
+            // URLs in the same second that this is a safe unique identifier.
+            // If you find a real counterexample, contact me at acct:evan@status.net
+            // and I will publicly apologize for my hubris.
+            $nb->uri = common_local_url('showbookmark',
+                                        array('user' => $profile->id,
+                                              'created' => $dt->format(DateTime::W3C),
+                                              'crc32' => sprintf('%08x', $nb->url_crc32)));
+        }
+
+        $nb->insert();
+
         $tags    = array();
         $replies = array();
 
@@ -197,6 +256,8 @@ class Notice_bookmark extends Memcached_DataObject
             }
         }
 
+        // 
+
         $hashtags = array();
         $taglinks = array();
 
@@ -236,21 +297,16 @@ class Notice_bookmark extends Memcached_DataObject
                                                'tags' => $tags,
                                                'replies' => $replies));
 
+        if (!array_key_exists('uri', $options)) {
+            $options['uri'] = $nb->uri;
+        }
+
         $saved = Notice::saveNew($profile->id,
                                  $content,
                                  array_key_exists('source', $options) ?
                                  $options['source'] : 'web',
                                  $options);
 
-        if (!empty($saved)) {
-            $nb = new Notice_bookmark();
-
-            $nb->notice_id   = $saved->id;
-            $nb->title       = $title;
-            $nb->description = $description;
-            $nb->insert();
-        }
-
         return $saved;
     }
 }

plugins/Bookmark/BookmarkPlugin.php

@@ -63,23 +63,52 @@ class BookmarkPlugin extends Plugin
 
         // For storing user-submitted flags on profiles
 
-        $schema->ensureTable('notice_bookmark',
+        $schema->ensureTable('bookmark',
-                             array(new ColumnDef('notice_id',
+                             array(new ColumnDef('profile_id',
                                                  'integer',
                                                  null,
                                                  false,
                                                  'PRI'),
+                                   new ColumnDef('url',
+                                                 'varchar',
+                                                 255,
+                                                 false,
+                                                 'PRI'),
                                    new ColumnDef('title',
                                                  'varchar',
                                                  255),
                                    new ColumnDef('description',
-                                                 'text')));
+                                                 'text'),
+                                   new ColumnDef('uri',
+                                                 'varchar',
+                                                 255,
+                                                 false,
+                                                 'UNI'),
+                                   new ColumnDef('url_crc32',
+                                                 'integer',
+                                                 null,
+                                                 false,
+                                                 'MUL'),
+                                   new ColumnDef('created',
+                                                 'datetime',
+                                                 null,
+                                                 false,
+                                                 'MUL')));
+
+        try {
+            $schema->createIndex('bookmark', 
+                                 array('profile_id', 
+                                       'url_crc32'),
+                                 'bookmark_profile_url_idx');
+        } catch (Exception $e) {
+            common_log(LOG_ERR, $e->getMessage());
+        }
 
         return true;
     }
 
     /**
-     * When a notice is deleted, delete the related Notice_bookmark
+     * When a notice is deleted, delete the related Bookmark
      *
      * @param Notice $notice Notice being deleted
      * 
@@ -88,7 +117,7 @@ class BookmarkPlugin extends Plugin
 
     function onNoticeDeleteRelated($notice)
     {
-        $nb = Notice_bookmark::staticGet('notice_id', $notice->id);
+        $nb = Bookmark::getByNotice($notice);
 
         if (!empty($nb)) {
             $nb->delete();
@@ -129,7 +158,7 @@ class BookmarkPlugin extends Plugin
         case 'BookmarkpopupAction':
             include_once $dir . '/' . strtolower(mb_substr($cls, 0, -6)) . '.php';
             return false;
-        case 'Notice_bookmark':
+        case 'Bookmark':
             include_once $dir.'/'.$cls.'.php';
             return false;
         case 'BookmarkForm':
@@ -158,6 +187,12 @@ class BookmarkPlugin extends Plugin
 
         $m->connect('main/bookmark/popup', array('action' => 'bookmarkpopup'));
 
+        $m->connect('bookmark/:user/:created/:crc32',
+                    array('action' => 'showbookmark'),
+                    array('user' => '[0-9]+',
+                          'created' => '[0-9]{4}-[0-9]{2}-[0-9]{2}T[0-9]{2}:[0-9]{2}:[0-9]{2}Z',
+                          'crc32' => '[0-9A-F]{8}'));
+
         return true;
     }
 
@@ -171,8 +206,7 @@ class BookmarkPlugin extends Plugin
 
     function onStartShowNoticeItem($nli)
     {
-        $nb = Notice_bookmark::staticGet('notice_id',
+        $nb = Bookmark::getByNotice($nli->notice);
-                                         $nli->notice->id);
 
         if (!empty($nb)) {
 
@@ -279,8 +313,7 @@ class BookmarkPlugin extends Plugin
         common_log(LOG_INFO,
                    "Checking {$notice->uri} to see if it's a bookmark.");
 
-        $nb = Notice_bookmark::staticGet('notice_id',
+        $nb = Bookmark::getByNotice($notice);
-                                         $notice->id);
                                          
         if (!empty($nb)) {
 
@@ -485,7 +518,7 @@ class BookmarkPlugin extends Plugin
             }
         }
 
-        Notice_bookmark::saveNew($author->localProfile(),
+        Bookmark::saveNew($author->localProfile(),
                                  $bookmark->title,
                                  $url,
                                  $tags,

plugins/Bookmark/deliciousbookmarkimporter.php

@@ -96,7 +96,7 @@ class DeliciousBookmarkImporter extends QueueHandler
         $addDate = $a->getAttribute('add_date');
         $created = common_sql_date(intval($addDate));
 
-        $saved = Notice_bookmark::saveNew($user->getProfile(),
+        $saved = Bookmark::saveNew($user->getProfile(),
                                           $title,
                                           $url,
                                           $tags,

plugins/Bookmark/newbookmark.php

@@ -135,7 +135,7 @@ class NewbookmarkAction extends Action
             }
 
 
-            $saved = Notice_bookmark::saveNew($this->user->getProfile(),
+            $saved = Bookmark::saveNew($this->user->getProfile(),
                                               $this->title,
                                               $this->url,
                                               $this->tags,

plugins/Bookmark/showbookmark.php

@@ -0,0 +1,116 @@
+<?php
+/**
+ * StatusNet - the distributed open-source microblogging tool
+ * Copyright (C) 2010, StatusNet, Inc.
+ *
+ * Show a single bookmark
+ * 
+ * PHP version 5
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Affero General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU Affero General Public License for more details.
+ *
+ * You should have received a copy of the GNU Affero General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ * @category  Bookmark
+ * @package   StatusNet
+ * @author    Evan Prodromou <evan@status.net>
+ * @copyright 2010 StatusNet, Inc.
+ * @license   http://www.fsf.org/licensing/licenses/agpl-3.0.html AGPL 3.0
+ * @link      http://status.net/
+ */
+
+if (!defined('STATUSNET')) {
+    // This check helps protect against security problems;
+    // your code file can't be executed directly from the web.
+    exit(1);
+}
+
+/**
+ * Show a single bookmark, with associated information
+ *
+ * @category  Bookmark
+ * @package   StatusNet
+ * @author    Evan Prodromou <evan@status.net>
+ * @copyright 2010 StatusNet, Inc.
+ * @license   http://www.fsf.org/licensing/licenses/agpl-3.0.html AGPL 3.0
+ * @link      http://status.net/
+ */
+
+class ShowbookmarkAction extends ShownoticeAction
+{
+    protected $bookmark = null;
+
+    /**
+     * For initializing members of the class.
+     *
+     * @param array $argarray misc. arguments
+     *
+     * @return boolean true
+     */
+
+    function prepare($argarray)
+    {
+        OwnerDesignAction::prepare($argarray);
+
+        $this->user = User::staticGet('id', $this->trimmed('user'));
+
+        if (empty($this->user)) {
+            throw new ClientException(_('No such user.'), 404);
+        }
+
+        $this->profile = $this->user->getProfile();
+
+        if (empty($this->profile)) {
+            throw new ServerException(_('User without a profile.'));
+        }
+
+        $this->avatar = $this->profile->getAvatar(AVATAR_PROFILE_SIZE);
+
+        $crc32 = pack("H*", $this->trimmed('crc32'));
+
+        if (empty($crc32)) {
+            throw new ClientException(_('No such URL.'), 404);
+        }
+        
+        $dt = DateTime::createFromFormat(DateTime::W3C,
+                                         $this->trimmed('created'),
+                                         new DateTimeZone('UTC'));
+
+        if (empty($dt)) {
+            throw new ClientException(_('No such create date.'), 404);
+        }
+
+        $bookmarks = Bookmark::getByCRC32($this->profile,
+                                                 $this->crc32);
+
+        foreach ($bookmarks as $bookmark) {
+            $bdt = new DateTime($bookmark->created);
+            if ($bdt->getTimestamp() == $dt->getTimestamp()) {
+                $this->bookmark = $bookmark;
+                break;
+            }
+        } 
+
+        if (empty($this->bookmark)) {
+            throw new ClientException(_('No such bookmark.'), 404);
+        }
+
+        $this->notice = Notice::staticGet('uri', $this->bookmark->uri);
+
+        if (empty($this->notice)) {
+            // Did we used to have it, and it got deleted?
+            throw new ClientException(_('No such bookmark.'), 404);
+        }
+
+        return true;
+    }
+}