Fixed issues with logging

- Fixed log file name template
- Fixed buffering changes when running add/edit jobs in batch

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"

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();
+		try
-		$job->setContext(AbstractJob::CONTEXT_BATCH_ADD);
+		{
-		Api::run($job, $arguments);
+			$job = new EditPostJob();
-		Logger::setBuffer([]);
+			$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;

src/Api/Jobs/AddUserJob.php

@@ -23,10 +23,16 @@ class AddUserJob extends AbstractJob
 		$arguments[EditUserJob::USER_ENTITY] = $user;
 
 		Logger::bufferChanges();
-		$job = new EditUserJob();
+		try
-		$job->setContext(self::CONTEXT_BATCH_ADD);
+		{
-		Api::run($job, $arguments);
+			$job = new EditUserJob();
-		Logger::setBuffer([]);
+			$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);

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;
 	}
 

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;
 	}
 

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 = [];
 	}
 }

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;

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));
+	}
 }

tests/JobTests/EditPostJobTest.php

@@ -14,8 +14,8 @@ class EditPostJobTest extends AbstractTest
 		$args =
 		[
 			EditPostJob::POST_ID => $post->getId(),
-			EditPostSafetyJob::SAFETY => PostSafety::Safe,
+			EditPostSafetyJob::SAFETY => PostSafety::Sketchy,
-			EditPostSourceJob::SOURCE => '',
+			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));
+	}
 }

tests/JobTests/EditUserJobTest.php

@@ -0,0 +1,39 @@
+<?php
+class EditUserJobTest extends AbstractTest
+{
+	public function testSaving()
+	{
+		$this->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));
+	}
+}

tests/MiscTests/LoggerTest.php

@@ -3,17 +3,10 @@ class LoggerTest extends AbstractTest
 {
 	public function testLogging()
 	{
-		$logPath = __DIR__ . '/logs/{yyyy}-{mm}-{dd}.log';
+		$realLogPath = Logger::getLogPath();
-		$realLogPath = __DIR__ . '/logs/' . date('Y-m-d') . '.log';
 
 		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);
+	}
 }

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..."