From 4048d1ec3dfe68be8d8e31b859b2f219adc34397 Mon Sep 17 00:00:00 2001 From: Evan Prodromou Date: Fri, 24 Dec 2010 20:34:15 -0800 Subject: [PATCH] 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. --- .../{Notice_bookmark.php => Bookmark.php} | 128 +++++++++++++----- plugins/Bookmark/BookmarkPlugin.php | 55 ++++++-- .../Bookmark/deliciousbookmarkimporter.php | 2 +- plugins/Bookmark/newbookmark.php | 2 +- plugins/Bookmark/showbookmark.php | 116 ++++++++++++++++ 5 files changed, 254 insertions(+), 49 deletions(-) rename plugins/Bookmark/{Notice_bookmark.php => Bookmark.php} (66%) create mode 100644 plugins/Bookmark/showbookmark.php diff --git a/plugins/Bookmark/Notice_bookmark.php b/plugins/Bookmark/Bookmark.php similarity index 66% rename from plugins/Bookmark/Notice_bookmark.php rename to plugins/Bookmark/Bookmark.php index 3a7d0ed742..aa9e9af43b 100644 --- a/plugins/Bookmark/Notice_bookmark.php +++ b/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 $notice_id; // int(4) primary_key not_null - public $title; // varchar(255) - public $description; // text + 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 /** * 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,37 +126,60 @@ 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); - if (!empty($file)) { - $f2p = new File_to_post(); + return self::pkeyGet(array('profile_id' => $profile->id, + 'url' => $url)); + return null; + } - $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; - } - } - } - } + /** + * 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 null; + + return $bookmarks; } /** @@ -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; } } diff --git a/plugins/Bookmark/BookmarkPlugin.php b/plugins/Bookmark/BookmarkPlugin.php index e18ea25eaa..01dd6f1eaa 100644 --- a/plugins/Bookmark/BookmarkPlugin.php +++ b/plugins/Bookmark/BookmarkPlugin.php @@ -63,23 +63,52 @@ class BookmarkPlugin extends Plugin // For storing user-submitted flags on profiles - $schema->ensureTable('notice_bookmark', - array(new ColumnDef('notice_id', + $schema->ensureTable('bookmark', + 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', - $nli->notice->id); + $nb = Bookmark::getByNotice($nli->notice); 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', - $notice->id); + $nb = Bookmark::getByNotice($notice); if (!empty($nb)) { @@ -485,7 +518,7 @@ class BookmarkPlugin extends Plugin } } - Notice_bookmark::saveNew($author->localProfile(), + Bookmark::saveNew($author->localProfile(), $bookmark->title, $url, $tags, diff --git a/plugins/Bookmark/deliciousbookmarkimporter.php b/plugins/Bookmark/deliciousbookmarkimporter.php index 22ad45882e..061572d95a 100644 --- a/plugins/Bookmark/deliciousbookmarkimporter.php +++ b/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, diff --git a/plugins/Bookmark/newbookmark.php b/plugins/Bookmark/newbookmark.php index 63944c8176..a0cf3fffb2 100644 --- a/plugins/Bookmark/newbookmark.php +++ b/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, diff --git a/plugins/Bookmark/showbookmark.php b/plugins/Bookmark/showbookmark.php new file mode 100644 index 0000000000..f6213ef5db --- /dev/null +++ b/plugins/Bookmark/showbookmark.php @@ -0,0 +1,116 @@ +. + * + * @category Bookmark + * @package StatusNet + * @author Evan Prodromou + * @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 + * @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; + } +}