diff --git a/data/config.ini b/data/config.ini index c8e46bb1..9e138c23 100644 --- a/data/config.ini +++ b/data/config.ini @@ -5,7 +5,7 @@ dbUser = "test" dbPass = "test" filesPath = "./data/files/" thumbsPath = "./data/thumbs/" -logsPath = "./data/logs/{yyyy-mm}.log" +logsPath = "./data/logs/{yyyy}-{mm}.log" mediaPath = "./public_html/media/" title = "szurubooru" salt = "1A2/$_4xVa" diff --git a/src/Api/Jobs/AddPostJob.php b/src/Api/Jobs/AddPostJob.php index bc372cee..23634ca1 100644 --- a/src/Api/Jobs/AddPostJob.php +++ b/src/Api/Jobs/AddPostJob.php @@ -7,29 +7,30 @@ class AddPostJob extends AbstractJob { $post = PostModel::spawn(); - //basic stuff $anonymous = $this->getArgument(self::ANONYMOUS); if (Auth::isLoggedIn() and !$anonymous) $post->setUploader(Auth::getCurrentUser()); - //store the post to get the ID in the logs PostModel::forgeId($post); - //do the edits - //warning: it uses internally the same privileges as post editing $arguments = $this->getArguments(); $arguments[EditPostJob::POST_ENTITY] = $post; Logger::bufferChanges(); - $job = new EditPostJob(); - $job->setContext(AbstractJob::CONTEXT_BATCH_ADD); - Api::run($job, $arguments); - Logger::setBuffer([]); + try + { + $job = new EditPostJob(); + $job->setContext(AbstractJob::CONTEXT_BATCH_ADD); + Api::run($job, $arguments); + } + finally + { + Logger::discardBuffer(); + } - //save to db + //save the post to db if everything went okay PostModel::save($post); - //log Logger::log('{user} added {post} (tags: {tags}, safety: {safety}, source: {source})', [ 'user' => ($anonymous and !getConfig()->misc->logAnonymousUploads) ? TextHelper::reprUser(UserModel::getAnonymousName()) @@ -39,7 +40,6 @@ class AddPostJob extends AbstractJob 'safety' => $post->getSafety()->toString(), 'source' => $post->source]); - //finish Logger::flush(); return $post; diff --git a/src/Api/Jobs/AddUserJob.php b/src/Api/Jobs/AddUserJob.php index 0c73163c..b9068fe5 100644 --- a/src/Api/Jobs/AddUserJob.php +++ b/src/Api/Jobs/AddUserJob.php @@ -23,10 +23,16 @@ class AddUserJob extends AbstractJob $arguments[EditUserJob::USER_ENTITY] = $user; Logger::bufferChanges(); - $job = new EditUserJob(); - $job->setContext(self::CONTEXT_BATCH_ADD); - Api::run($job, $arguments); - Logger::setBuffer([]); + try + { + $job = new EditUserJob(); + $job->setContext(self::CONTEXT_BATCH_ADD); + Api::run($job, $arguments); + } + finally + { + Logger::discardBuffer(); + } //save the user to db if everything went okay UserModel::save($user); diff --git a/src/Api/Jobs/EditPostJob.php b/src/Api/Jobs/EditPostJob.php index aafff537..87f45abb 100644 --- a/src/Api/Jobs/EditPostJob.php +++ b/src/Api/Jobs/EditPostJob.php @@ -35,9 +35,11 @@ class EditPostJob extends AbstractPostJob } if ($this->getContext() == AbstractJob::CONTEXT_NORMAL) + { PostModel::save($post); + Logger::flush(); + } - Logger::flush(); return $post; } diff --git a/src/Api/Jobs/EditUserJob.php b/src/Api/Jobs/EditUserJob.php index 9d2f749a..70775e48 100644 --- a/src/Api/Jobs/EditUserJob.php +++ b/src/Api/Jobs/EditUserJob.php @@ -59,9 +59,9 @@ class EditUserJob extends AbstractUserJob { UserModel::save($user); EditUserEmailJob::observeSave($user); + Logger::flush(); } - Logger::flush(); return $user; } diff --git a/src/Logger.php b/src/Logger.php index 606c26e1..cec6db55 100644 --- a/src/Logger.php +++ b/src/Logger.php @@ -24,6 +24,9 @@ class Logger public static function flush() { + if (empty(self::$buffer)) + return; + $fh = fopen(self::$path, 'ab'); if (!$fh) throw new SimpleException('Cannot write to log files'); @@ -61,8 +64,8 @@ class Logger return self::$buffer; } - public static function setBuffer(array $buffer) + public static function discardBuffer() { - self::$buffer = $buffer; + self::$buffer = []; } } diff --git a/tests/JobTests/AddPostJobTest.php b/tests/JobTests/AddPostJobTest.php index ac5ab7e5..dce8e165 100644 --- a/tests/JobTests/AddPostJobTest.php +++ b/tests/JobTests/AddPostJobTest.php @@ -48,6 +48,16 @@ class AddPostJobTest extends AbstractTest }, 'Insufficient privilege'); } + public function testLogBuffering() + { + $this->testSaving(); + + $logPath = Logger::getLogPath(); + $x = file_get_contents($logPath); + $lines = array_filter(explode("\n", $x)); + $this->assert->areEqual(1, count($lines)); + } + protected function prepare() { getConfig()->registration->needEmailForUploading = false; diff --git a/tests/JobTests/AddUserJobTest.php b/tests/JobTests/AddUserJobTest.php index 11986644..41bd9307 100644 --- a/tests/JobTests/AddUserJobTest.php +++ b/tests/JobTests/AddUserJobTest.php @@ -262,4 +262,14 @@ class AddUserJobTest extends AbstractTest $this->assert->areEqual(0, Mailer::getMailCounter()); } + + public function testLogBuffering() + { + $this->testSaving(); + + $logPath = Logger::getLogPath(); + $x = file_get_contents($logPath); + $lines = array_filter(explode("\n", $x)); + $this->assert->areEqual(2, count($lines)); + } } diff --git a/tests/JobTests/EditPostJobTest.php b/tests/JobTests/EditPostJobTest.php index aff22337..25050e7d 100644 --- a/tests/JobTests/EditPostJobTest.php +++ b/tests/JobTests/EditPostJobTest.php @@ -14,8 +14,8 @@ class EditPostJobTest extends AbstractTest $args = [ EditPostJob::POST_ID => $post->getId(), - EditPostSafetyJob::SAFETY => PostSafety::Safe, - EditPostSourceJob::SOURCE => '', + EditPostSafetyJob::SAFETY => PostSafety::Sketchy, + EditPostSourceJob::SOURCE => 'some source huh', EditPostContentJob::POST_CONTENT => new ApiFileInput($this->getPath('image.jpg'), 'test.jpg'), ]; @@ -47,4 +47,14 @@ class EditPostJobTest extends AbstractTest Api::run(new EditPostJob(), $args); }, 'Insufficient privilege'); } + + public function testLogBuffering() + { + $this->testSaving(); + + $logPath = Logger::getLogPath(); + $x = file_get_contents($logPath); + $lines = array_filter(explode("\n", $x)); + $this->assert->areEqual(3, count($lines)); + } } diff --git a/tests/JobTests/EditUserJobTest.php b/tests/JobTests/EditUserJobTest.php new file mode 100644 index 00000000..ed1f396f --- /dev/null +++ b/tests/JobTests/EditUserJobTest.php @@ -0,0 +1,39 @@ +grantAccess('changeUserName.own'); + $this->grantAccess('changeUserPassword.own'); + $user = $this->mockUser(); + + $newName = 'dummy' . uniqid(); + + $user = $this->assert->doesNotThrow(function() use ($user, $newName) + { + return Api::run( + new EditUserJob(), + [ + EditUserJob::USER_NAME => $user->getName(), + EditUserNameJob::NEW_USER_NAME => $newName, + EditUserPasswordJob::NEW_PASSWORD => 'changed', + ]); + }); + + //first user = admin + $this->assert->areEqual($newName, $user->getName()); + $this->assert->areEquivalent(new AccessRank(AccessRank::Registered), $user->getAccessRank()); + $this->assert->isFalse(empty($user->getPasswordSalt())); + $this->assert->isFalse(empty($user->getPasswordHash())); + } + + public function testLogBuffering() + { + $this->testSaving(); + + $logPath = Logger::getLogPath(); + $x = file_get_contents($logPath); + $lines = array_filter(explode("\n", $x)); + $this->assert->areEqual(2, count($lines)); + } +} diff --git a/tests/MiscTests/LoggerTest.php b/tests/MiscTests/LoggerTest.php index fd538374..baa556bb 100644 --- a/tests/MiscTests/LoggerTest.php +++ b/tests/MiscTests/LoggerTest.php @@ -3,17 +3,10 @@ class LoggerTest extends AbstractTest { public function testLogging() { - $logPath = __DIR__ . '/logs/{yyyy}-{mm}-{dd}.log'; - $realLogPath = __DIR__ . '/logs/' . date('Y-m-d') . '.log'; + $realLogPath = Logger::getLogPath(); try { - getConfig()->main->logsPath = $logPath; - $this->assert->doesNotThrow(function() - { - Logger::init(); - }); - $this->assert->isFalse(file_exists($realLogPath)); $this->assert->doesNotThrow(function() { @@ -30,4 +23,41 @@ class LoggerTest extends AbstractTest unlink($realLogPath); } } + + public function testPathChanging() + { + $logPath = __DIR__ . '/logs/{yyyy}-{mm}-{dd}.log'; + $realLogPath = __DIR__ . '/logs/' . date('Y-m-d') . '.log'; + + getConfig()->main->logsPath = $logPath; + $this->assert->doesNotThrow(function() + { + Logger::init(); + }); + $this->assert->areEqual($realLogPath, Logger::getLogPath()); + } + + public function testDiscarding() + { + $realLogPath = Logger::getLogPath(); + + $this->assert->isFalse(file_exists($realLogPath)); + + Logger::bufferChanges(); + Logger::log('line 1'); + Logger::log('line 2'); + Logger::log('line 3'); + Logger::discardBuffer(); + + $this->assert->isFalse(file_exists($realLogPath)); + Logger::log('line 4'); + Logger::flush(); + $this->assert->isTrue(file_exists($realLogPath)); + + $x = file_get_contents($realLogPath); + $this->assert->isTrue(strpos($x, 'line 1') === false); + $this->assert->isTrue(strpos($x, 'line 2') === false); + $this->assert->isTrue(strpos($x, 'line 3') === false); + $this->assert->isTrue(strpos($x, 'line 4') !== false); + } } diff --git a/tests/config.ini b/tests/config.ini index f28703b4..32a97a66 100644 --- a/tests/config.ini +++ b/tests/config.ini @@ -1,7 +1,7 @@ [main] filesPath = "./tests/files/" thumbsPath = "./tests/thumbs/" -logsPath = "./tests/logs/{yyyy-mm}.log" +logsPath = "./tests/logs/{yyyy}-{mm}.log" mediaPath = "./public_html/media/" title = "szurubooru/tests" salt = "salt..."