Permalink
Browse files

Update Grid Search to respect doesOwnerHaveAccessToPost()

* Update GridController and tests to verify post security for reply searches using OwnerInstanceDAO->doesOwnerHaveAccessToPost()
* Update OwnerInstanceDAO->doesOwnerHaveAccessToPost() and tests to cache query results
Closes #648, closes #1099
  • Loading branch information...
1 parent 40fa01b commit b165132f43b6649732c5ca68dc3fd979adf25907 Mark Wilkie committed with ginatrapani Nov 2, 2011
@@ -32,12 +32,12 @@ MYSQL_DATA_DIR="/usr/local/mysql/data"
TMP_DATA_DIR="/tmp/thinkup_rd_dir"
# glean db config data
-DB_HOST=`$PHP tests/scripts/parse_config.php db_host`
-DB_USER=`$PHP tests/scripts/parse_config.php db_user`
-DB_PASSWORD=`$PHP tests/scripts/parse_config.php db_password`
-DB_NAME=`$PHP tests/scripts/parse_config.php db_name`
-DB_SOCKET=`$PHP tests/scripts/parse_config.php db_socket`
-DB_PORT=`$PHP tests/scripts/parse_config.php db_port`
+DB_HOST=`$PHP extras/dev/ramdisk/parse_config.php db_host`
+DB_USER=`$PHP extras/dev/ramdisk/parse_config.php db_user`
+DB_PASSWORD=`$PHP extras/dev/ramdisk/parse_config.php db_password`
+DB_NAME=`$PHP extras/dev/ramdisk/parse_config.php db_name`
+DB_SOCKET=`$PHP extras/dev/ramdisk/parse_config.php db_socket`
+DB_PORT=`$PHP extras/dev/ramdisk/parse_config.php db_port`
DB_NAME_TEST_RD="${DB_NAME}_tests_rd"
MYSQL_CMD="$MYSQL -h $DB_HOST -u $DB_USER";
@@ -158,7 +158,7 @@ public function testOwnerWithAccessTweetsAllMaxNoLimit() {
}
public function testReplyToSearch() {
- $builders = $this->buildData();
+ $builders = $this->buildData(0,0);
$this->simulateLogin('me@example.com');
$_GET['u'] = 'someuser1';
$_GET['n'] = 'twitter';
@@ -226,6 +226,25 @@ public function testReplyToSearchNotLoggedIn() {
$this->assertEqual($ob->posts[0]->post_id_str, '10765432100123456783_str');
}
+
+ public function testReplyToSearchFilterOutProtected() {
+ $builders = $this->buildData();
+ $this->simulateLogin('me@example.com');
+ $_GET['u'] = 'someuser1';
+ $_GET['n'] = 'twitter';
+ $_GET['t'] = '10765432100123456781';
+ $controller = new GridController(true);
+ $this->assertTrue(isset($controller));
+ ob_start();
+ $controller->control();
+ $results = ob_get_contents();
+ ob_end_clean();
+ $json = substr($results, 29, strrpos($results, ';') - 30);
+ $ob = json_decode( $json );
+ $this->assertEqual($ob->status, 'success');
+ $this->assertEqual(count($ob->posts), 1);
+ }
+
public function testNoProfilerOutput() {
// Enable profiler
$config = Config::getInstance();
@@ -44,6 +44,8 @@ public function setUp() {
public function tearDown() {
parent::tearDown();
$this->logger->close();
+ //clear doesOwnerHaveAccessToPost query cache
+ OwnerInstanceMySQLDAO::$post_access_query_cache = array();
}
public function testDelete() {
@@ -143,7 +145,7 @@ public function testGetOwnerInstance() {
'valid oauth_access_token_secret');
}
-
+
public function testUpdateTokens() {
$builder_data = array('owner_id' => 2, 'instance_id' => 20);
$builder = FixtureBuilder::build(self::TEST_TABLE_OI, $builder_data);
@@ -224,10 +226,10 @@ public function testDoesOwnerHaveAccessToPost() {
$post = new Post(array('id'=>1, 'author_user_id'=>'20', 'author_username'=>'no one',
'author_fullname'=>"No One", 'author_avatar'=>'yo.jpg', 'source'=>'TweetDeck', 'pub_date'=>'',
'adj_pub_date'=>'', 'in_reply_to_user_id'=>'',
- 'in_reply_to_post_id'=>'', 'reply_count_cache'=>'', 'in_retweet_of_post_id'=>'', 'retweet_count_cache'=>'',
+ 'in_reply_to_post_id'=>'', 'reply_count_cache'=>'', 'in_retweet_of_post_id'=>'', 'retweet_count_cache'=>'',
'retweet_count_api' =>'', 'old_retweet_count_cache' => '', 'in_rt_of_user_id' =>'',
'post_id'=>'9021481076', 'is_protected'=>1, 'place_id' => 'ece7b97d252718cc', 'favlike_count_cache'=>0,
- 'post_text'=>'I like cookies', 'network'=>'twitter', 'geo'=>'', 'place'=>'', 'location'=>'',
+ 'post_text'=>'I like cookies', 'network'=>'twitter', 'geo'=>'', 'place'=>'', 'location'=>'',
'is_geo_encoded'=>0, 'is_reply_by_friend'=>0, 'is_retweet_by_friend'=>0, 'reply_retweet_distance'=>0));
// no owner id
@@ -250,6 +252,10 @@ public function testDoesOwnerHaveAccessToPost() {
$post->is_protected = true;
$this->assertFalse($dao->doesOwnerHaveAccessToPost($owner, $post));
+ // should have empty cache arrays
+ $this->assertEqual(count(OwnerInstanceMySQLDAO::$post_access_query_cache['1-twitter-network_id_cache']), 0);
+ $this->assertEqual(count(OwnerInstanceMySQLDAO::$post_access_query_cache['20-twitter-follower_id_cache']), 0);
+
//protected post but owner is admin
$owner->is_admin = true;
$this->assertTrue($dao->doesOwnerHaveAccessToPost($owner, $post));
@@ -259,10 +265,18 @@ public function testDoesOwnerHaveAccessToPost() {
$this->assertFalse($dao->doesOwnerHaveAccessToPost($owner, $post));
//protected post, owner is not admin, and owner DOES have an authed instance which follows author
+ OwnerInstanceMySQLDAO::$post_access_query_cache = array(); // clear cache
$owner->id = 2;
$follows_builder = FixtureBuilder::build('follows', array('user_id'=>'20', 'follower_id'=>'10',
'network'=>'twitter'));
$this->assertTrue($dao->doesOwnerHaveAccessToPost($owner, $post));
+ // should have populated cache arrays
+ $this->assertEqual(count(OwnerInstanceMySQLDAO::$post_access_query_cache['2-twitter-network_id_cache']), 1);
+ $this->assertEqual(count(OwnerInstanceMySQLDAO::$post_access_query_cache['20-twitter-follower_id_cache']), 1);
+ $this->assertEqual(
+ OwnerInstanceMySQLDAO::$post_access_query_cache['2-twitter-network_id_cache'][0]['network_user_id'], 10);
+ $this->assertEqual(
+ OwnerInstanceMySQLDAO::$post_access_query_cache['20-twitter-follower_id_cache'][0]['follower_id'], 10);
}
}
@@ -46,6 +46,8 @@ public function setUp(){
public function tearDown(){
parent::tearDown();
+ //clear doesOwnerHaveAccessToPost query cache
+ OwnerInstanceMySQLDAO::$post_access_query_cache = array();
}
public function testConstructor() {
@@ -83,6 +83,7 @@ public function authControl($owner = false) {
if ($owner) {
$public_search = true;
}
+ $private_reply_search = false;
$this->setContentType('text/javascript');
if (!$this->is_missing_param) {
$instance_dao = DAOFactory::getDAO('InstanceDAO');
@@ -104,6 +105,9 @@ public function authControl($owner = false) {
$post_dao = DAOFactory::getDAO('PostDAO');
$posts_it = $post_dao->getRepliesToPostIterator($_GET['t'],$_GET['n'], 'default','km',
$public_search);
+ if (! $public_search) {
+ $private_reply_search = true;
+ }
} else {
if (isset($_GET['nolimit']) && $_GET['nolimit'] == 'true') {
self::$MAX_ROWS = 0;
@@ -120,9 +124,14 @@ public function authControl($owner = false) {
throw Exception("Grid Search should use a PostIterator to conserve memory");
}
foreach($posts_it as $key => $value) {
+ if ($private_reply_search) {
+ if (! $ownerinstance_dao->doesOwnerHaveAccessToPost($owner, $value)) {
+ continue;
+ }
+ }
$cnt++;
$data = array('id' => $cnt, 'text' => $value->post_text,
- 'post_id_str' => $value->post_id . '_str', 'author' => $value->author_username,
+ 'post_id_str' => $value->post_id . '_str', 'author' => $value->author_username,
'date' => $value->adj_pub_date, 'network' => $value->network);
echo json_encode($data) . ",\n";
flush();
@@ -30,6 +30,12 @@
*
*/
class OwnerInstanceMySQLDAO extends PDODAO implements OwnerInstanceDAO {
+ /**
+ *
+ * Cached query results for doesOwnerHaveAccessToPost() reduces query load while looping through post results
+ * @var array $post_access_query_cache
+ */
+ static $post_access_query_cache = array();
public function doesOwnerHaveAccessToInstance(Owner $owner, Instance $instance) {
// verify $owner has an id
@@ -46,15 +52,15 @@ public function doesOwnerHaveAccessToInstance(Owner $owner, Instance $instance)
return true;
} else {
$q = '
- SELECT
- *
- FROM
+ SELECT
+ *
+ FROM
#prefix#owner_instances oi
INNER JOIN
#prefix#instances i
- ON
+ ON
i.id = oi.instance_id
- WHERE
+ WHERE
i.id = :id AND oi.owner_id = :owner_id';
$vars = array(':owner_id' => $owner->id, ':id' => $instance->id);
if ($this->profiler_enabled) Profiler::setDAOMethod(__METHOD__);
@@ -69,7 +75,6 @@ public function doesOwnerHaveAccessToPost(Owner $owner, Post $post) {
$message = 'doesOwnerHaveAccessToPost() requires an "Owner" object with "id" defined';
throw new BadArgumentException($message);
}
-
//if post is public OR the owner is an admin, show it
if (!$post->is_protected || $owner->is_admin) {
return true;
@@ -82,19 +87,32 @@ public function doesOwnerHaveAccessToPost(Owner $owner, Post $post) {
WHERE oi.owner_id = :owner_id AND i.network = :network";
$vars = array(':owner_id' => $owner->id, ':network'=> $post->network);
- if ($this->profiler_enabled) Profiler::setDAOMethod(__METHOD__);
- $stmt = $this->execute($q, $vars);
- $owner_network_user_ids = $this->getDataRowsAsArrays($stmt);
+ // we'll cache query results to speed up checks while looping through post iterators
+ $network_id_cache_key = implode("-", $vars) . '-network_id_cache';
+ if (isset(self::$post_access_query_cache[ $network_id_cache_key ])) {
+ $owner_network_user_ids = self::$post_access_query_cache[ $network_id_cache_key ];
+ } else {
+ if ($this->profiler_enabled) Profiler::setDAOMethod(__METHOD__);
+ $stmt = $this->execute($q, $vars);
+ $owner_network_user_ids = $this->getDataRowsAsArrays($stmt);
+ self::$post_access_query_cache[ $network_id_cache_key ] = $owner_network_user_ids;
+ }
// select all the network user ID's which follow protected author
$q = "SELECT f.follower_id
FROM #prefix#follows f
WHERE f.user_id = :user_id AND f.network = :network";
$vars = array(':user_id' => $post->author_user_id, ':network'=> $post->network);
- if ($this->profiler_enabled) Profiler::setDAOMethod(__METHOD__);
- $stmt = $this->execute($q, $vars);
- $authed_network_user_ids = $this->getDataRowsAsArrays($stmt);
-
+ // we'll cache query results to speed up checks while looping through post iterators
+ $follower_id_cache_key = implode("-", $vars) . '-follower_id_cache';
+ if (isset(self::$post_access_query_cache[ $follower_id_cache_key ])) {
+ $authed_network_user_ids = self::$post_access_query_cache[ $follower_id_cache_key ];
+ } else {
+ if ($this->profiler_enabled) Profiler::setDAOMethod(__METHOD__);
+ $stmt = $this->execute($q, $vars);
+ $authed_network_user_ids = $this->getDataRowsAsArrays($stmt);
+ self::$post_access_query_cache[ $follower_id_cache_key ] = $authed_network_user_ids;
+ }
// If there's overlap, return true else return false
foreach ($owner_network_user_ids as $owner_network_user_id) {
foreach ($authed_network_user_ids as $authed_network_user_id) {
@@ -109,9 +127,9 @@ public function doesOwnerHaveAccessToPost(Owner $owner, Post $post) {
public function get($owner_id, $instance_id) {
$q = "SELECT
id, owner_id, instance_id, oauth_access_token, oauth_access_token_secret
- FROM
- #prefix#owner_instances
- WHERE
+ FROM
+ #prefix#owner_instances
+ WHERE
owner_id = :owner_id AND instance_id = :instance_id";
$vars = array(':owner_id' => $owner_id, ':instance_id' => $instance_id);
@@ -124,8 +142,8 @@ public function get($owner_id, $instance_id) {
public function getByInstance($instance_id) {
$q = "SELECT
id, owner_id, instance_id, oauth_access_token, oauth_access_token_secret
- FROM
- #prefix#owner_instances
+ FROM
+ #prefix#owner_instances
WHERE instance_id = :instance_id";
$vars = array(':instance_id' => $instance_id);
@@ -179,8 +197,8 @@ public function deleteByInstance($instance_id) {
public function updateTokens($owner_id, $instance_id, $oauth_token, $oauth_token_secret) {
$q = 'UPDATE
- #prefix#owner_instances
- SET
+ #prefix#owner_instances
+ SET
oauth_access_token=:oauth_access_token, oauth_access_token_secret=:oauth_access_token_secret
WHERE
owner_id = :owner_id AND instance_id = :instance_id';
@@ -197,10 +215,10 @@ public function updateTokens($owner_id, $instance_id, $oauth_token, $oauth_token
public function getOAuthTokens($id) {
$q = "SELECT
- oauth_access_token, oauth_access_token_secret
- FROM
- #prefix#owner_instances
- WHERE
+ oauth_access_token, oauth_access_token_secret
+ FROM
+ #prefix#owner_instances
+ WHERE
instance_id = :instance_id ORDER BY id ASC LIMIT 1";
if ($this->profiler_enabled) Profiler::setDAOMethod(__METHOD__);
$stmt = $this->execute($q, array(':instance_id' => $id));

0 comments on commit b165132

Please sign in to comment.