Use UUIDs for Bookmark unique ID

I was trying to generate URIs for Bookmarks based on (profile, crc32(url), created).

I failed at that. CRC32s are unsigned ints, and our schema code didn't like that.

On top of that, my code to encode and restore created timestamps was problematic.

So, I switched back to using a meaningless unique ID for Bookmarks.

One way to do this would be to use an auto-incrementing integer ID. However, we've been
kind of crabbed out a few times for exposing auto-incrementing integer IDs as URIs, so
I thought maybe using a random UUID would be a better way to do it.

So, this patch sets random UUIDs for URIs of bookmarks.

diff --git a/plugins/Bookmark/Bookmark.php b/plugins/Bookmark/Bookmark.php
index 61fe3c5b976e0d340d0025b50ee148bcf0be9c1b..4ee287fb65b88f4bdf3a777f25dd1c95fb3c0ef2 100644 (file)
--- a/plugins/Bookmark/Bookmark.php
+++ b/plugins/Bookmark/Bookmark.php
@@ -46,13 +46,13 @@ if (!defined('STATUSNET')) {
 class Bookmark extends Memcached_DataObject
 {
     public $__table = 'bookmark'; // table name
-    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
+    public $id;          // char(36) primary_key not_null
+    public $profile_id;  // int(4) not_null
+    public $url;         // varchar(255) not_null
+    public $title;       // varchar(255)
+    public $description; // text
+    public $uri;         // varchar(255)
+    public $created;     // datetime
 
     /**
      * Get an instance by key
@@ -100,12 +100,12 @@ class Bookmark extends Memcached_DataObject
 
     function table()
     {
-        return array('profile_id' => DB_DATAOBJECT_INT + DB_DATAOBJECT_NOTNULL,
+        return array('id' => DB_DATAOBJECT_STR + DB_DATAOBJECT_NOTNULL,
+                     'profile_id' => DB_DATAOBJECT_INT + DB_DATAOBJECT_NOTNULL,
                      'url' => DB_DATAOBJECT_STR,
                      'title' => 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);
     }
@@ -129,8 +129,7 @@ class Bookmark extends Memcached_DataObject
 
     function keyTypes()
     {
-        return array('profile_id' => 'K',
-                     'url' => 'K',
+        return array('id' => 'K',
                      'uri' => 'U');
     }
 
@@ -169,36 +168,16 @@ class Bookmark extends Memcached_DataObject
      
     static function getByURL($profile, $url)
     {
-        return self::pkeyGet(array('profile_id' => $profile->id,
-                                   'url' => $url));
-        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;
+        $nb->url        = $url;
 
-        if ($nb->find()) {
-            while ($nb->fetch()) {
-                $bookmarks[] = clone($nb);
-            }
+        if ($nb->find(true)) {
+            return $nb;
+        } else {
+            return null;
         }
-
-        return $bookmarks;
     }
 
     /**
@@ -240,11 +219,11 @@ class Bookmark extends Memcached_DataObject
 
         $nb = new Bookmark();
 
+        $nb->id          = UUID::gen();
         $nb->profile_id  = $profile->id;
         $nb->url         = $url;
         $nb->title       = $title;
         $nb->description = $description;
-        $nb->url_crc32   = crc32($nb->url);
 
         if (array_key_exists('created', $options)) {
             $nb->created = $options['created'];
@@ -255,22 +234,8 @@ class Bookmark extends Memcached_DataObject
         if (array_key_exists('uri', $options)) {
             $nb->uri = $options['uri'];
         } else {
-            $dt = new DateTime($nb->created, new DateTimeZone('UTC'));
-
-            // 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.
-
-            $created = $dt->format('YmdHis');
-
-            $crc32 = sprintf('%08x', $nb->url_crc32);
-
             $nb->uri = common_local_url('showbookmark',
-                                        array('user' => $profile->id,
-                                              'created' => $created,
-                                              'crc32' => $crc32));
+                                        array('id' => $nb->id));
         }
 
         $nb->insert();

diff --git a/plugins/Bookmark/BookmarkPlugin.php b/plugins/Bookmark/BookmarkPlugin.php
index 4f31349801797ca3a16ea248e3f61cfa113649b0..99f25c64b89c41bf668600f99a67ec7a704c71f7 100644 (file)
--- a/plugins/Bookmark/BookmarkPlugin.php
+++ b/plugins/Bookmark/BookmarkPlugin.php
@@ -86,16 +86,21 @@ class BookmarkPlugin extends Plugin
         // For storing user-submitted flags on profiles
 
         $schema->ensureTable('bookmark',
-                             array(new ColumnDef('profile_id',
+                             array(new ColumnDef('id',
+                                                 'char',
+                                                 36,
+                                                 false,
+                                                 'PRI'),
+                                   new ColumnDef('profile_id',
                                                  'integer',
                                                  null,
                                                  false,
-                                                 'PRI'),
+                                                 'MUL'),
                                    new ColumnDef('url',
                                                  'varchar',
                                                  255,
                                                  false,
-                                                 'PRI'),
+                                                 'MUL'),
                                    new ColumnDef('title',
                                                  'varchar',
                                                  255),
@@ -106,26 +111,12 @@ class BookmarkPlugin extends Plugin
                                                  255,
                                                  false,
                                                  'UNI'),
-                                   new ColumnDef('url_crc32',
-                                                 'integer unsigned',
-                                                 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;
     }
 
@@ -216,11 +207,9 @@ class BookmarkPlugin extends Plugin
         $m->connect('main/bookmark/import',
                     array('action' => 'importdelicious'));
 
-        $m->connect('bookmark/:user/:created/:crc32',
+        $m->connect('bookmark/:id',
                     array('action' => 'showbookmark'),
-                    array('user' => '[0-9]+',
-                          'created' => '[0-9]{14}',
-                          'crc32' => '[0-9a-f]{8}'));
+                    array('id' => '[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}'));
 
         $m->connect('notice/by-url/:id',
                     array('action' => 'noticebyurl'),

diff --git a/plugins/Bookmark/showbookmark.php b/plugins/Bookmark/showbookmark.php
index e9e656f84c5b792512ce09230fd60dbe971f258a..6bebffb68e2e81710ed96ede63b9752acbae2073 100644 (file)
--- a/plugins/Bookmark/showbookmark.php
+++ b/plugins/Bookmark/showbookmark.php
@@ -61,43 +61,9 @@ class ShowbookmarkAction extends ShownoticeAction
     {
         OwnerDesignAction::prepare($argarray);
 
-        $this->user = User::staticGet('id', $this->trimmed('user'));
+        $this->id = $this->trimmed('id');
 
-        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);
-
-        sscanf($this->trimmed('crc32'), '%08x', $crc32);
-
-        if (empty($crc32)) {
-            throw new ClientException(_('No such URL.'), 404);
-        }
-        
-        $dt = new DateTime($this->trimmed('created'),
-                           new DateTimeZone('UTC'));
-
-        if (empty($dt)) {
-            throw new ClientException(_('No such create date.'), 404);
-        }
-
-        $bookmarks = Bookmark::getByCRC32($this->profile,
-                                          $crc32);
-
-        foreach ($bookmarks as $bookmark) {
-            $bdt = new DateTime($bookmark->created, new DateTimeZone('UTC'));
-            if ($bdt->format('U') == $dt->format('U')) {
-                $this->bookmark = $bookmark;
-                break;
-            }
-        } 
+        $this->bookmark = Bookmark::staticGet('id', $this->id);
 
         if (empty($this->bookmark)) {
             throw new ClientException(_('No such bookmark.'), 404);
@@ -110,6 +76,20 @@ class ShowbookmarkAction extends ShownoticeAction
             throw new ClientException(_('No such bookmark.'), 404);
         }
 
+        $this->user = User::staticGet('id', $this->bookmark->profile_id);
+
+        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);
+
         return true;
     }