From 650074c648d98f81674c6e2b0ebf052c473ada6e Mon Sep 17 00:00:00 2001 From: Evan Prodromou Date: Wed, 31 Mar 2010 15:54:35 -0400 Subject: don't insert the same notice twice into an inbox --- classes/Inbox.php | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/classes/Inbox.php b/classes/Inbox.php index 014ba3d82..0d807946d 100644 --- a/classes/Inbox.php +++ b/classes/Inbox.php @@ -114,6 +114,16 @@ class Inbox extends Memcached_DataObject return false; } + $ids = unpack('N*', $inbox->notice_ids); + + // bulk inserts sometimes fail and get restarted. + // Skip if this one has been inserted before. + + if (in_array($notice_id, $ids)) { + // effectively successful + return true; + } + $result = $inbox->query(sprintf('UPDATE inbox '. 'set notice_ids = concat(cast(0x%08x as binary(4)), '. 'substr(notice_ids, 1, %d)) '. -- cgit v1.2.3-54-g00ecf From 4b80ce0be89fe50eabec1a19dbf4a0c26a413423 Mon Sep 17 00:00:00 2001 From: Evan Prodromou Date: Thu, 1 Apr 2010 12:09:33 -0400 Subject: if user allows location sharing but turned off browser location use profile location --- actions/newnotice.php | 24 +++++++++++++++--------- 1 file changed, 15 insertions(+), 9 deletions(-) diff --git a/actions/newnotice.php b/actions/newnotice.php index ed0fa1b2b..748d104ff 100644 --- a/actions/newnotice.php +++ b/actions/newnotice.php @@ -184,13 +184,21 @@ class NewnoticeAction extends Action $options = array('reply_to' => ($replyto == 'false') ? null : $replyto); - if ($user->shareLocation() && $this->arg('notice_data-geo')) { - - $locOptions = Notice::locationOptions($this->trimmed('lat'), - $this->trimmed('lon'), - $this->trimmed('location_id'), - $this->trimmed('location_ns'), - $user->getProfile()); + if ($user->shareLocation()) { + // use browser data if checked; otherwise profile data + if ($this->arg('notice_data-geo')) { + $locOptions = Notice::locationOptions($this->trimmed('lat'), + $this->trimmed('lon'), + $this->trimmed('location_id'), + $this->trimmed('location_ns'), + $user->getProfile()); + } else { + $locOptions = Notice::locationOptions(null, + null, + null, + null, + $user->getProfile()); + } $options = array_merge($options, $locOptions); } @@ -201,8 +209,6 @@ class NewnoticeAction extends Action $upload->attachToNotice($notice); } - - if ($this->boolean('ajax')) { header('Content-Type: text/xml;charset=utf-8'); $this->xw->startDocument('1.0', 'UTF-8'); -- cgit v1.2.3-54-g00ecf From 8b24ad8a9c681585e95612084eb629df8b364b74 Mon Sep 17 00:00:00 2001 From: Evan Prodromou Date: Thu, 1 Apr 2010 12:52:12 -0400 Subject: Revert "if user allows location sharing but turned off browser location use profile location" This reverts commit 4b80ce0be89fe50eabec1a19dbf4a0c26a413423. --- actions/newnotice.php | 24 +++++++++--------------- 1 file changed, 9 insertions(+), 15 deletions(-) diff --git a/actions/newnotice.php b/actions/newnotice.php index 748d104ff..ed0fa1b2b 100644 --- a/actions/newnotice.php +++ b/actions/newnotice.php @@ -184,21 +184,13 @@ class NewnoticeAction extends Action $options = array('reply_to' => ($replyto == 'false') ? null : $replyto); - if ($user->shareLocation()) { - // use browser data if checked; otherwise profile data - if ($this->arg('notice_data-geo')) { - $locOptions = Notice::locationOptions($this->trimmed('lat'), - $this->trimmed('lon'), - $this->trimmed('location_id'), - $this->trimmed('location_ns'), - $user->getProfile()); - } else { - $locOptions = Notice::locationOptions(null, - null, - null, - null, - $user->getProfile()); - } + if ($user->shareLocation() && $this->arg('notice_data-geo')) { + + $locOptions = Notice::locationOptions($this->trimmed('lat'), + $this->trimmed('lon'), + $this->trimmed('location_id'), + $this->trimmed('location_ns'), + $user->getProfile()); $options = array_merge($options, $locOptions); } @@ -209,6 +201,8 @@ class NewnoticeAction extends Action $upload->attachToNotice($notice); } + + if ($this->boolean('ajax')) { header('Content-Type: text/xml;charset=utf-8'); $this->xw->startDocument('1.0', 'UTF-8'); -- cgit v1.2.3-54-g00ecf From a09b27ff41df41a86fdb0abae14239907d5ee6ec Mon Sep 17 00:00:00 2001 From: Evan Prodromou Date: Thu, 1 Apr 2010 12:52:26 -0400 Subject: Revert "don't insert the same notice twice into an inbox" This reverts commit 650074c648d98f81674c6e2b0ebf052c473ada6e. --- classes/Inbox.php | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/classes/Inbox.php b/classes/Inbox.php index 0d807946d..014ba3d82 100644 --- a/classes/Inbox.php +++ b/classes/Inbox.php @@ -114,16 +114,6 @@ class Inbox extends Memcached_DataObject return false; } - $ids = unpack('N*', $inbox->notice_ids); - - // bulk inserts sometimes fail and get restarted. - // Skip if this one has been inserted before. - - if (in_array($notice_id, $ids)) { - // effectively successful - return true; - } - $result = $inbox->query(sprintf('UPDATE inbox '. 'set notice_ids = concat(cast(0x%08x as binary(4)), '. 'substr(notice_ids, 1, %d)) '. -- cgit v1.2.3-54-g00ecf From 9efe5393ff3f6a23fae1e32e521c6afe69eef6f6 Mon Sep 17 00:00:00 2001 From: Evan Prodromou Date: Thu, 1 Apr 2010 12:57:52 -0400 Subject: Revert "Revert "don't insert the same notice twice into an inbox"" This reverts commit a09b27ff41df41a86fdb0abae14239907d5ee6ec. --- classes/Inbox.php | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/classes/Inbox.php b/classes/Inbox.php index 014ba3d82..0d807946d 100644 --- a/classes/Inbox.php +++ b/classes/Inbox.php @@ -114,6 +114,16 @@ class Inbox extends Memcached_DataObject return false; } + $ids = unpack('N*', $inbox->notice_ids); + + // bulk inserts sometimes fail and get restarted. + // Skip if this one has been inserted before. + + if (in_array($notice_id, $ids)) { + // effectively successful + return true; + } + $result = $inbox->query(sprintf('UPDATE inbox '. 'set notice_ids = concat(cast(0x%08x as binary(4)), '. 'substr(notice_ids, 1, %d)) '. -- cgit v1.2.3-54-g00ecf From d60c1f1a9f343f10d33800862c67839594607c8f Mon Sep 17 00:00:00 2001 From: Evan Prodromou Date: Thu, 1 Apr 2010 12:58:06 -0400 Subject: Revert "Revert "if user allows location sharing but turned off browser location use profile location"" This reverts commit 8b24ad8a9c681585e95612084eb629df8b364b74. --- actions/newnotice.php | 24 +++++++++++++++--------- 1 file changed, 15 insertions(+), 9 deletions(-) diff --git a/actions/newnotice.php b/actions/newnotice.php index ed0fa1b2b..748d104ff 100644 --- a/actions/newnotice.php +++ b/actions/newnotice.php @@ -184,13 +184,21 @@ class NewnoticeAction extends Action $options = array('reply_to' => ($replyto == 'false') ? null : $replyto); - if ($user->shareLocation() && $this->arg('notice_data-geo')) { - - $locOptions = Notice::locationOptions($this->trimmed('lat'), - $this->trimmed('lon'), - $this->trimmed('location_id'), - $this->trimmed('location_ns'), - $user->getProfile()); + if ($user->shareLocation()) { + // use browser data if checked; otherwise profile data + if ($this->arg('notice_data-geo')) { + $locOptions = Notice::locationOptions($this->trimmed('lat'), + $this->trimmed('lon'), + $this->trimmed('location_id'), + $this->trimmed('location_ns'), + $user->getProfile()); + } else { + $locOptions = Notice::locationOptions(null, + null, + null, + null, + $user->getProfile()); + } $options = array_merge($options, $locOptions); } @@ -201,8 +209,6 @@ class NewnoticeAction extends Action $upload->attachToNotice($notice); } - - if ($this->boolean('ajax')) { header('Content-Type: text/xml;charset=utf-8'); $this->xw->startDocument('1.0', 'UTF-8'); -- cgit v1.2.3-54-g00ecf From f1c01f9ead4a5f4da7c6964f0998831fa31f8a16 Mon Sep 17 00:00:00 2001 From: Brion Vibber Date: Thu, 1 Apr 2010 10:15:40 -0700 Subject: Temporary hack until notice_profile_id_idx is updated to (profile_id, id) instead of (profile_id, created, id). It's been falling back to PRIMARY instead, which is really very inefficient for a profile that hasn't posted in a few months. Even though forcing the index will cause a filesort, it's usually going to be better. Even for large profiles it seems much faster than the badly-indexed query. --- classes/Profile.php | 63 +++++++++++++++++++++++++++++++++++++++-------------- 1 file changed, 47 insertions(+), 16 deletions(-) diff --git a/classes/Profile.php b/classes/Profile.php index 5de35c191..54f557ea7 100644 --- a/classes/Profile.php +++ b/classes/Profile.php @@ -225,31 +225,62 @@ class Profile extends Memcached_DataObject { $notice = new Notice(); - $notice->profile_id = $this->id; + // Temporary hack until notice_profile_id_idx is updated + // to (profile_id, id) instead of (profile_id, created, id). + // It's been falling back to PRIMARY instead, which is really + // very inefficient for a profile that hasn't posted in a few + // months. Even though forcing the index will cause a filesort, + // it's usually going to be better. + if (common_config('db', 'type') == 'mysql') { + $index = ''; + $query = + "select id from notice force index (notice_profile_id_idx) ". + "where profile_id=" . $notice->escape($this->id); + + if ($since_id != 0) { + $query .= " and id > $since_id"; + } - $notice->selectAdd(); - $notice->selectAdd('id'); + if ($max_id != 0) { + $query .= " and id < $max_id"; + } - if ($since_id != 0) { - $notice->whereAdd('id > ' . $since_id); - } + $query .= ' order by id DESC'; - if ($max_id != 0) { - $notice->whereAdd('id <= ' . $max_id); - } + if (!is_null($offset)) { + $query .= " LIMIT $limit OFFSET $offset"; + } + + $notice->query($query); + } else { + $index = ''; - $notice->orderBy('id DESC'); + $notice->profile_id = $this->id; - if (!is_null($offset)) { - $notice->limit($offset, $limit); + $notice->selectAdd(); + $notice->selectAdd('id'); + + if ($since_id != 0) { + $notice->whereAdd('id > ' . $since_id); + } + + if ($max_id != 0) { + $notice->whereAdd('id <= ' . $max_id); + } + + $notice->orderBy('id DESC'); + + if (!is_null($offset)) { + $notice->limit($offset, $limit); + } + + $notice->find(); } $ids = array(); - if ($notice->find()) { - while ($notice->fetch()) { - $ids[] = $notice->id; - } + while ($notice->fetch()) { + $ids[] = $notice->id; } return $ids; -- cgit v1.2.3-54-g00ecf From b10ff031d9ba8c5e3b5267a469e8332011149fee Mon Sep 17 00:00:00 2001 From: Brion Vibber Date: Fri, 2 Apr 2010 09:32:03 -0700 Subject: Ticket 2271: extra whitespace in underlined link for username in notice lists Switching to a raw() output for the of the nickname removes the extra whitespace and fixes display. --- lib/noticelist.php | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/lib/noticelist.php b/lib/noticelist.php index 0d4cd4dd9..83c8de9f6 100644 --- a/lib/noticelist.php +++ b/lib/noticelist.php @@ -340,8 +340,9 @@ class NoticeListItem extends Widget function showNickname() { - $this->out->element('span', array('class' => 'nickname fn'), - $this->profile->nickname); + $this->out->raw('' . + htmlspecialchars($this->profile->nickname) . + ''); } /** -- cgit v1.2.3-54-g00ecf From 6cd0637e552ed6ce35b4447aa280fb13cf7f65cb Mon Sep 17 00:00:00 2001 From: Brion Vibber Date: Fri, 2 Apr 2010 09:32:03 -0700 Subject: Ticket 2271: extra whitespace in underlined link for username in notice lists Switching to a raw() output for the of the nickname removes the extra whitespace and fixes display. --- lib/noticelist.php | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/lib/noticelist.php b/lib/noticelist.php index 0d4cd4dd9..83c8de9f6 100644 --- a/lib/noticelist.php +++ b/lib/noticelist.php @@ -340,8 +340,9 @@ class NoticeListItem extends Widget function showNickname() { - $this->out->element('span', array('class' => 'nickname fn'), - $this->profile->nickname); + $this->out->raw('' . + htmlspecialchars($this->profile->nickname) . + ''); } /** -- cgit v1.2.3-54-g00ecf From 61394aa8ac3585c9b7f1e15b6c581af01854987d Mon Sep 17 00:00:00 2001 From: Brion Vibber Date: Fri, 2 Apr 2010 15:39:43 -0700 Subject: Don't save duplicate messages into a user's packed inbox. We've already got the packed box loaded at insert time, so we can simply unpack it and check before doing the update query. Should help with dupes that come in when inbox distrib jobs die and get restarted, etc. --- classes/Inbox.php | 39 +++++++++++++++++++++++++++++++++++++-- 1 file changed, 37 insertions(+), 2 deletions(-) diff --git a/classes/Inbox.php b/classes/Inbox.php index 014ba3d82..2533210b7 100644 --- a/classes/Inbox.php +++ b/classes/Inbox.php @@ -96,12 +96,23 @@ class Inbox extends Memcached_DataObject $inbox = new Inbox(); $inbox->user_id = $user_id; - $inbox->notice_ids = call_user_func_array('pack', array_merge(array('N*'), $ids)); + $inbox->pack($ids); $inbox->fake = true; return $inbox; } + /** + * Append the given notice to the given user's inbox. + * Caching updates are managed for the inbox itself. + * + * If the notice is already in this inbox, the second + * add will be silently dropped. + * + * @param int @user_id + * @param int $notice_id + * @return boolean success + */ static function insertNotice($user_id, $notice_id) { $inbox = DB_DataObject::staticGet('inbox', 'user_id', $user_id); @@ -114,6 +125,13 @@ class Inbox extends Memcached_DataObject return false; } + $ids = $inbox->unpack(); + if (in_array(intval($notice_id), $ids)) { + // Already in there, we probably re-ran some inbox adds + // due to an error. Skip the dupe silently. + return true; + } + $result = $inbox->query(sprintf('UPDATE inbox '. 'set notice_ids = concat(cast(0x%08x as binary(4)), '. 'substr(notice_ids, 1, %d)) '. @@ -150,7 +168,7 @@ class Inbox extends Memcached_DataObject } } - $ids = unpack('N*', $inbox->notice_ids); + $ids = $inbox->unpack(); if (!empty($since_id)) { $newids = array(); @@ -229,4 +247,21 @@ class Inbox extends Memcached_DataObject } return new ArrayWrapper($items); } + + /** + * Saves a list of integer notice_ids into a packed blob in this object. + * @param array $ids list of integer notice_ids + */ + protected function pack(array $ids) + { + $this->notice_ids = call_user_func_array('pack', array_merge(array('N*'), $ids)); + } + + /** + * @return array of integer notice_ids + */ + protected function unpack() + { + return unpack('N*', $this->notice_ids); + } } -- cgit v1.2.3-54-g00ecf From ec24f283dd6f1371125c042529f571645a5f13fa Mon Sep 17 00:00:00 2001 From: Brion Vibber Date: Fri, 2 Apr 2010 15:45:03 -0700 Subject: Don't save duplicate messages into a user's packed inbox. We've already got the packed box loaded at insert time, so we can simply unpack it and check before doing the update query. Should help with dupes that come in when inbox distrib jobs die and get restarted, etc. Conflicts: classes/Inbox.php Looks like this was implemented on master recently and not copied up to testing. Merging to my version on testing as I've added some doc comments and extracted a couple functions for future ease of use. --- classes/Inbox.php | 43 ++++++++++++++++++++++++++++++++++--------- 1 file changed, 34 insertions(+), 9 deletions(-) diff --git a/classes/Inbox.php b/classes/Inbox.php index 0d807946d..2533210b7 100644 --- a/classes/Inbox.php +++ b/classes/Inbox.php @@ -96,12 +96,23 @@ class Inbox extends Memcached_DataObject $inbox = new Inbox(); $inbox->user_id = $user_id; - $inbox->notice_ids = call_user_func_array('pack', array_merge(array('N*'), $ids)); + $inbox->pack($ids); $inbox->fake = true; return $inbox; } + /** + * Append the given notice to the given user's inbox. + * Caching updates are managed for the inbox itself. + * + * If the notice is already in this inbox, the second + * add will be silently dropped. + * + * @param int @user_id + * @param int $notice_id + * @return boolean success + */ static function insertNotice($user_id, $notice_id) { $inbox = DB_DataObject::staticGet('inbox', 'user_id', $user_id); @@ -114,13 +125,10 @@ class Inbox extends Memcached_DataObject return false; } - $ids = unpack('N*', $inbox->notice_ids); - - // bulk inserts sometimes fail and get restarted. - // Skip if this one has been inserted before. - - if (in_array($notice_id, $ids)) { - // effectively successful + $ids = $inbox->unpack(); + if (in_array(intval($notice_id), $ids)) { + // Already in there, we probably re-ran some inbox adds + // due to an error. Skip the dupe silently. return true; } @@ -160,7 +168,7 @@ class Inbox extends Memcached_DataObject } } - $ids = unpack('N*', $inbox->notice_ids); + $ids = $inbox->unpack(); if (!empty($since_id)) { $newids = array(); @@ -239,4 +247,21 @@ class Inbox extends Memcached_DataObject } return new ArrayWrapper($items); } + + /** + * Saves a list of integer notice_ids into a packed blob in this object. + * @param array $ids list of integer notice_ids + */ + protected function pack(array $ids) + { + $this->notice_ids = call_user_func_array('pack', array_merge(array('N*'), $ids)); + } + + /** + * @return array of integer notice_ids + */ + protected function unpack() + { + return unpack('N*', $this->notice_ids); + } } -- cgit v1.2.3-54-g00ecf