From 8aa499a0b93056cfeaced542ac0b112b0103aca8 Mon Sep 17 00:00:00 2001 From: Marcin Kurczewski Date: Sun, 11 May 2014 23:40:09 +0200 Subject: [PATCH] Fixed automatic featuring post - Fixed main page view - Code moved from StaticPagesController to PostModel - Code split into semantically meaningful methods - Allowed anonymous featuring through API - Added protection against automatic featuring of hidden post --- src/Api/Jobs/FeaturePostJob.php | 12 ++- src/Controllers/StaticPagesController.php | 30 ++----- src/Models/PostModel.php | 55 ++++++++++++ src/Models/PropertyModel.php | 51 ++++------- src/Views/static-main.phtml | 2 +- src/core.php | 1 + tests/JobTests/FeaturePostJobTest.php | 49 +++++++++++ tests/ModelTests/PostModelTest.php | 102 ++++++++++++++++++++++ tests/ModelTests/PropertyModelTest.php | 21 +++++ 9 files changed, 262 insertions(+), 61 deletions(-) create mode 100644 tests/JobTests/FeaturePostJobTest.php create mode 100644 tests/ModelTests/PostModelTest.php create mode 100644 tests/ModelTests/PropertyModelTest.php diff --git a/src/Api/Jobs/FeaturePostJob.php b/src/Api/Jobs/FeaturePostJob.php index e52855d8..4698f631 100644 --- a/src/Api/Jobs/FeaturePostJob.php +++ b/src/Api/Jobs/FeaturePostJob.php @@ -1,16 +1,22 @@ post; PropertyModel::set(PropertyModel::FeaturedPostId, $post->getId()); - PropertyModel::set(PropertyModel::FeaturedPostDate, time()); - PropertyModel::set(PropertyModel::FeaturedPostUserName, Auth::getCurrentUser()->getName()); + PropertyModel::set(PropertyModel::FeaturedPostUnixTime, time()); + + PropertyModel::set(PropertyModel::FeaturedPostUserName, + ($this->hasArgument(self::ANONYMOUS) and $this->getArgument(self::ANONYMOUS)) + ? null + : Auth::getCurrentUser()->getName()); Logger::log('{user} featured {post} on main page', [ - 'user' => TextHelper::reprPost(Auth::getCurrentUser()), + 'user' => TextHelper::reprPost(PropertyModel::get(PropertyModel::FeaturedPostUserName)), 'post' => TextHelper::reprPost($post)]); return $post; diff --git a/src/Controllers/StaticPagesController.php b/src/Controllers/StaticPagesController.php index c95fb694..812a7123 100644 --- a/src/Controllers/StaticPagesController.php +++ b/src/Controllers/StaticPagesController.php @@ -7,14 +7,14 @@ class StaticPagesController $context->transport->postCount = PostModel::getCount(); $context->viewName = 'static-main'; - $featuredPost = $this->getFeaturedPost(); + PostModel::featureRandomPostIfNecessary(); + $featuredPost = PostModel::getFeaturedPost(); if ($featuredPost) { $context->featuredPost = $featuredPost; - $context->featuredPostDate = PropertyModel::get(PropertyModel::FeaturedPostDate); - $context->featuredPostUser = UserModel::getByNameOrEmail( - PropertyModel::get(PropertyModel::FeaturedPostUserName), - false); + $context->featuredPostUnixTime = PropertyModel::get(PropertyModel::FeaturedPostUnixTime); + $context->featuredPostUser = UserModel::tryGetByNameOrEmail( + PropertyModel::get(PropertyModel::FeaturedPostUserName)); } } @@ -34,24 +34,4 @@ class StaticPagesController $context->path = TextHelper::absolutePath($config->help->paths[$tab]); $context->tab = $tab; } - - private function getFeaturedPost() - { - $config = getConfig(); - $featuredPostRotationTime = $config->misc->featuredPostMaxDays * 24 * 3600; - - $featuredPostId = PropertyModel::get(PropertyModel::FeaturedPostId); - $featuredPostDate = PropertyModel::get(PropertyModel::FeaturedPostDate); - - //check if too old - if (!$featuredPostId or $featuredPostDate + $featuredPostRotationTime < time()) - return PropertyModel::featureNewPost(); - - //check if post was deleted - $featuredPost = PostModel::tryGetById($featuredPostId); - if (!$featuredPost) - return PropertyModel::featureNewPost(); - - return $featuredPost; - } } diff --git a/src/Models/PostModel.php b/src/Models/PostModel.php index af81a424..5e9456dd 100644 --- a/src/Models/PostModel.php +++ b/src/Models/PostModel.php @@ -288,4 +288,59 @@ final class PostModel extends AbstractCrudModel { return TextHelper::absolutePath(getConfig()->main->filesPath . DS . $name); } + + + + public static function getFeaturedPost() + { + $featuredPostId = PropertyModel::get(PropertyModel::FeaturedPostId); + if (!$featuredPostId) + return null; + return PostModel::tryGetById($featuredPostId); + } + + public static function featureRandomPostIfNecessary() + { + $config = getConfig(); + $featuredPostRotationTime = $config->misc->featuredPostMaxDays * 24 * 3600; + + $featuredPostId = PropertyModel::get(PropertyModel::FeaturedPostId); + $featuredPostUnixTime = PropertyModel::get(PropertyModel::FeaturedPostUnixTime); + + //check if too old + if (!$featuredPostId or $featuredPostUnixTime + $featuredPostRotationTime < time()) + { + self::featureRandomPost(); + return true; + } + + //check if post was deleted + $featuredPost = PostModel::tryGetById($featuredPostId); + if (!$featuredPost) + { + self::featureRandomPost(); + return true; + } + + return false; + } + + public static function featureRandomPost() + { + $stmt = (new Sql\SelectStatement) + ->setColumn('id') + ->setTable('post') + ->setCriterion((new Sql\ConjunctionFunctor) + ->add(new Sql\NegationFunctor(new Sql\StringExpression('hidden'))) + ->add(new Sql\EqualsFunctor('type', new Sql\Binding(PostType::Image))) + ->add(new Sql\EqualsFunctor('safety', new Sql\Binding(PostSafety::Safe)))) + ->setOrderBy(new Sql\RandomFunctor(), Sql\SelectStatement::ORDER_DESC); + $featuredPostId = Database::fetchOne($stmt)['id']; + if (!$featuredPostId) + return null; + + PropertyModel::set(PropertyModel::FeaturedPostId, $featuredPostId); + PropertyModel::set(PropertyModel::FeaturedPostUnixTime, time()); + PropertyModel::set(PropertyModel::FeaturedPostUserName, null); + } } diff --git a/src/Models/PropertyModel.php b/src/Models/PropertyModel.php index 2da4e4ad..f2a62526 100644 --- a/src/Models/PropertyModel.php +++ b/src/Models/PropertyModel.php @@ -6,11 +6,17 @@ final class PropertyModel implements IModel { const FeaturedPostId = 0; const FeaturedPostUserName = 1; - const FeaturedPostDate = 2; + const FeaturedPostUnixTime = 2; const DbVersion = 3; - static $allProperties = null; - static $loaded = false; + static $allProperties; + static $loaded; + + public static function init() + { + self::$allProperties = null; + self::$loaded = false; + } public static function getTableName() { @@ -19,16 +25,16 @@ final class PropertyModel implements IModel public static function loadIfNecessary() { - if (!self::$loaded) - { - self::$loaded = true; - self::$allProperties = []; - $stmt = new Sql\SelectStatement(); - $stmt ->setColumn('*'); - $stmt ->setTable('property'); - foreach (Database::fetchAll($stmt) as $row) - self::$allProperties[$row['prop_id']] = $row['value']; - } + if (self::$loaded) + return; + + self::$loaded = true; + self::$allProperties = []; + $stmt = new Sql\SelectStatement(); + $stmt ->setColumn('*'); + $stmt ->setTable('property'); + foreach (Database::fetchAll($stmt) as $row) + self::$allProperties[$row['prop_id']] = $row['value']; } public static function get($propertyId) @@ -68,23 +74,4 @@ final class PropertyModel implements IModel self::$allProperties[$propertyId] = $value; }); } - - public static function featureNewPost() - { - $stmt = (new Sql\SelectStatement) - ->setColumn('id') - ->setTable('post') - ->setCriterion((new Sql\ConjunctionFunctor) - ->add(new Sql\EqualsFunctor('type', new Sql\Binding(PostType::Image))) - ->add(new Sql\EqualsFunctor('safety', new Sql\Binding(PostSafety::Safe)))) - ->setOrderBy(new Sql\RandomFunctor(), Sql\SelectStatement::ORDER_DESC); - $featuredPostId = Database::fetchOne($stmt)['id']; - if (!$featuredPostId) - return null; - - self::set(self::FeaturedPostId, $featuredPostId); - self::set(self::FeaturedPostDate, time()); - self::set(self::FeaturedPostUserName, null); - return PostModel::getById($featuredPostId); - } } diff --git a/src/Views/static-main.phtml b/src/Views/static-main.phtml index f322ae91..6b646e3f 100644 --- a/src/Views/static-main.phtml +++ b/src/Views/static-main.phtml @@ -48,7 +48,7 @@ Assets::addStylesheet('static-main.css'); , - context->featuredPostDate) / (24 * 3600.)) ?> + context->featuredPostUnixTime) / (24 * 3600.)) ?> today diff --git a/src/core.php b/src/core.php index 5f80fee6..85c0e972 100644 --- a/src/core.php +++ b/src/core.php @@ -84,6 +84,7 @@ function prepareEnvironment($testEnvironment) Access::init(); Logger::init(); Mailer::init(); + PropertyModel::init(); \Chibi\Database::connect( $config->main->dbDriver, diff --git a/tests/JobTests/FeaturePostJobTest.php b/tests/JobTests/FeaturePostJobTest.php new file mode 100644 index 00000000..e8d9c76d --- /dev/null +++ b/tests/JobTests/FeaturePostJobTest.php @@ -0,0 +1,49 @@ +grantAccess('featurePost'); + + $user = $this->mockUser(); + $this->login($user); + $post1 = $this->mockPost($user); + $post2 = $this->mockPost($user); + + $this->assert->doesNotThrow(function() use ($post2) + { + Api::run( + new FeaturePostJob(), + [ + FeaturePostJob::POST_ID => $post2->getId() + ]); + }); + + $this->assert->areEqual($post2->getId(), PropertyModel::get(PropertyModel::FeaturedPostId)); + $this->assert->areEqual($user->getName(), PropertyModel::get(PropertyModel::FeaturedPostUserName)); + $this->assert->isNotNull(PropertyModel::get(PropertyModel::FeaturedPostUnixTime)); + } + + public function testAnonymousFeaturing() + { + $this->grantAccess('featurePost'); + + $user = $this->mockUser(); + $this->login($user); + $post1 = $this->mockPost($user); + $post2 = $this->mockPost($user); + + $this->assert->doesNotThrow(function() use ($post2) + { + Api::run( + new FeaturePostJob(), + [ + FeaturePostJob::POST_ID => $post2->getId(), + FeaturePostJob::ANONYMOUS => true, + ]); + }); + + $this->assert->areEqual($post2->getId(), PropertyModel::get(PropertyModel::FeaturedPostId)); + $this->assert->areEqual(null, PropertyModel::get(PropertyModel::FeaturedPostUserName)); + } +} diff --git a/tests/ModelTests/PostModelTest.php b/tests/ModelTests/PostModelTest.php new file mode 100644 index 00000000..dd5183f0 --- /dev/null +++ b/tests/ModelTests/PostModelTest.php @@ -0,0 +1,102 @@ +assert->doesNotThrow(function() + { + return PostModel::getFeaturedPost(); + }); + + $this->assert->areEqual(null, $post); + } + + public function testFeaturingNoPost() + { + PostModel::featureRandomPost(); + + $post = $this->assert->doesNotThrow(function() + { + return PostModel::getFeaturedPost(); + }); + + $this->assert->areEqual(null, $post); + } + + public function testFeaturingRandomPost() + { + $post = $this->mockPost(Auth::getCurrentUser()); + + PostModel::featureRandomPost(); + + $this->assert->areEqual($post->getId(), (int) PropertyModel::get(PropertyModel::FeaturedPostId)); + } + + public function testFeaturingIllegalPosts() + { + $posts = []; + foreach (range(0, 5) as $i) + $posts []= $this->mockPost(Auth::getCurrentUser()); + $posts[0]->setSafety(new PostSafety(PostSafety::Sketchy)); + $posts[1]->setSafety(new PostSafety(PostSafety::Sketchy)); + $posts[2]->setHidden(true); + $posts[3]->setType(new PostType(PostType::Youtube)); + $posts[4]->setType(new PostType(PostType::Flash)); + $posts[5]->setType(new PostType(PostType::Video)); + foreach ($posts as $post) + PostModel::save($post); + + PostModel::featureRandomPost(); + + $this->assert->areEqual(null, PropertyModel::get(PropertyModel::FeaturedPostId)); + } + + public function testAutoFeaturingFirstTime() + { + $this->mockPost(Auth::getCurrentUser()); + + $this->assert->doesNotThrow(function() + { + PostModel::featureRandomPostIfNecessary(); + }); + + $this->assert->isNotNull(PostModel::getFeaturedPost()); + } + + public function testAutoFeaturingTooSoon() + { + $this->mockPost(Auth::getCurrentUser()); + + $this->assert->isTrue(PostModel::featureRandomPostIfNecessary()); + $this->assert->isFalse(PostModel::featureRandomPostIfNecessary()); + + $this->assert->isNotNull(PostModel::getFeaturedPost()); + } + + public function testAutoFeaturingOutdated() + { + $post = $this->mockPost(Auth::getCurrentUser()); + $minTimestamp = getConfig()->misc->featuredPostMaxDays * 24 * 3600; + + $this->assert->isTrue(PostModel::featureRandomPostIfNecessary()); + PropertyModel::set(PropertyModel::FeaturedPostUnixTime, time() - $minTimestamp - 1); + $this->assert->isTrue(PostModel::featureRandomPostIfNecessary()); + PropertyModel::set(PropertyModel::FeaturedPostUnixTime, time() - $minTimestamp + 1); + $this->assert->isFalse(PostModel::featureRandomPostIfNecessary()); + + $this->assert->isNotNull(PostModel::getFeaturedPost()); + } + + public function testAutoFeaturingDeletedPost() + { + $post = $this->mockPost(Auth::getCurrentUser()); + + $this->assert->isTrue(PostModel::featureRandomPostIfNecessary()); + $this->assert->isNotNull(PostModel::getFeaturedPost()); + PostModel::remove($post); + $anotherPost = $this->mockPost(Auth::getCurrentUser()); + $this->assert->isTrue(PostModel::featureRandomPostIfNecessary()); + + $this->assert->isNotNull(PostModel::getFeaturedPost()); + } +} diff --git a/tests/ModelTests/PropertyModelTest.php b/tests/ModelTests/PropertyModelTest.php new file mode 100644 index 00000000..7e0d6643 --- /dev/null +++ b/tests/ModelTests/PropertyModelTest.php @@ -0,0 +1,21 @@ +assert->doesNotThrow(function() + { + PropertyModel::set(PropertyModel::FeaturedPostId, 100); + }); + $this->assert->areEqual(100, PropertyModel::get(PropertyModel::FeaturedPostId)); + } + + public function testGetAndSetWithArbitraryKeys() + { + $this->assert->doesNotThrow(function() + { + PropertyModel::set('something', 100); + }); + $this->assert->areEqual(100, PropertyModel::get('something')); + } +}