From 6fc7e5b05b1106cfcb2bf61cb788dd3aa38fa6f7 Mon Sep 17 00:00:00 2001 From: Evan Prodromou Date: Thu, 30 Dec 2010 13:21:14 -0800 Subject: [PATCH] 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. --- plugins/Bookmark/Bookmark.php | 69 +++++++---------------------- plugins/Bookmark/BookmarkPlugin.php | 31 +++++-------- plugins/Bookmark/showbookmark.php | 52 +++++++--------------- 3 files changed, 43 insertions(+), 109 deletions(-) diff --git a/plugins/Bookmark/Bookmark.php b/plugins/Bookmark/Bookmark.php index 61fe3c5b97..4ee287fb65 100644 --- 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 4f31349801..99f25c64b8 100644 --- 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 e9e656f84c..6bebffb68e 100644 --- a/plugins/Bookmark/showbookmark.php +++ b/plugins/Bookmark/showbookmark.php @@ -61,7 +61,22 @@ class ShowbookmarkAction extends ShownoticeAction { OwnerDesignAction::prepare($argarray); - $this->user = User::staticGet('id', $this->trimmed('user')); + $this->id = $this->trimmed('id'); + + $this->bookmark = Bookmark::staticGet('id', $this->id); + + 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); + } + + $this->user = User::staticGet('id', $this->bookmark->profile_id); if (empty($this->user)) { throw new ClientException(_('No such user.'), 404); @@ -75,41 +90,6 @@ class ShowbookmarkAction extends ShownoticeAction $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; - } - } - - 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; }