Merge pull request #9223 from nupplaphil/task/process_class

Cleanup Process classes

bin/worker.php

@@ -23,6 +23,7 @@
 
 use Dice\Dice;
 use Friendica\App;
+use Friendica\Core\Process;
 use Friendica\Core\Update;
 use Friendica\Core\Worker;
 use Friendica\DI;
@@ -76,4 +77,4 @@ Worker::processQueue($run_cron);
 
 Worker::unclaimProcess();
 
-Worker::endProcess();
+DI::process()->end();

src/Core/Process.php

@@ -23,6 +23,7 @@ namespace Friendica\Core;
 
 use Friendica\App;
 use Friendica\Core\Config\IConfig;
+use Friendica\Model;
 use Psr\Log\LoggerInterface;
 
 /**
@@ -56,12 +57,48 @@ class Process
 	 */
 	private $basePath;
 
-	public function __construct(LoggerInterface $logger, App\Mode $mode, IConfig $config, string $basepath)
+	/** @var Model\Process */
+	private $processModel;
+
+	/**
+	 * The Process ID of this process
+	 *
+	 * @var int
+	 */
+	private $pid;
+
+	public function __construct(LoggerInterface $logger, App\Mode $mode, IConfig $config, Model\Process $processModel, string $basepath, int $pid)
 	{
 		$this->logger = $logger;
 		$this->mode = $mode;
 		$this->config = $config;
 		$this->basePath = $basepath;
+		$this->processModel = $processModel;
+		$this->pid = $pid;
+	}
+
+	/**
+	 * Log active processes into the "process" table
+	 */
+	public function start()
+	{
+		$trace = debug_backtrace(DEBUG_BACKTRACE_IGNORE_ARGS, 1);
+
+		$command = basename($trace[0]['file']);
+
+		$this->processModel->deleteInactive();
+		$this->processModel->insert($command, $this->pid);
+	}
+
+	/**
+	 * Remove the active process from the "process" table
+	 *
+	 * @return bool
+	 * @throws \Exception
+	 */
+	public function end()
+	{
+		return $this->processModel->deleteByPid($this->pid);
 	}
 
 	/**

src/Core/Worker.php

@@ -22,6 +22,7 @@
 namespace Friendica\Core;
 
 use Friendica\Core;
+use Friendica\Core\Process as ProcessAlias;
 use Friendica\Database\DBA;
 use Friendica\DI;
 use Friendica\Model\Process;
@@ -72,7 +73,7 @@ class Worker
 		}
 
 		// We now start the process. This is done after the load check since this could increase the load.
-		self::startProcess();
+		DI::process()->start();
 
 		// Kill stale processes every 5 minutes
 		$last_cleanup = DI::config()->get('system', 'worker_last_cleaned', 0);
@@ -1092,7 +1093,7 @@ class Worker
 			if (self::tooMuchWorkers()) {
 				// Cleaning dead processes
 				self::killStaleWorkers();
-				Process::deleteInactive();
+				DI::modelProcess()->deleteInactive();
 
 				return;
 			}
@@ -1171,7 +1172,7 @@ class Worker
 		$args = ['no_cron' => !$do_cron];
 
 		$a = DI::app();
-		$process = new Core\Process(DI::logger(), DI::mode(), DI::config(), $a->getBasePath());
+		$process = new Core\Process(DI::logger(), DI::mode(), DI::config(), DI::modelProcess(), $a->getBasePath(), getmypid());
 		$process->run($command, $args);
 
 		// after spawning we have to remove the flag.
@@ -1360,31 +1361,6 @@ class Worker
 		return true;
 	}
 
-	/**
-	 * Log active processes into the "process" table
-	 */
-	public static function startProcess()
-	{
-		$trace = debug_backtrace(DEBUG_BACKTRACE_IGNORE_ARGS, 1);
-
-		$command = basename($trace[0]['file']);
-
-		Process::deleteInactive();
-
-		Process::insert($command);
-	}
-
-	/**
-	 * Remove the active process from the "process" table
-	 *
-	 * @return bool
-	 * @throws \Exception
-	 */
-	public static function endProcess()
-	{
-		return Process::deleteByPid();
-	}
-
 	/**
 	 * Set the flag if some job is waiting
 	 *

src/DI.php

@@ -314,6 +314,13 @@ abstract class DI
 	//
 	// "Model" namespace instances
 	//
+	/**
+	 * @return Model\Process
+	 */
+	public static function modelProcess()
+	{
+		return self::$dice->create(Model\Process::class);
+	}
 
 	/**
 	 * @return Model\User\Cookie

src/Model/Process.php

@@ -21,7 +21,7 @@
 
 namespace Friendica\Model;
 
-use Friendica\Database\DBA;
+use Friendica\Database\Database;
 use Friendica\Util\DateTimeFormat;
 
 /**
@@ -29,29 +29,33 @@ use Friendica\Util\DateTimeFormat;
  */
 class Process
 {
+	/** @var Database */
+	private $dba;
+
+	public function __construct(Database $dba)
+	{
+		$this->dba = $dba;
+	}
+
 	/**
 	 * Insert a new process row. If the pid parameter is omitted, we use the current pid
 	 *
 	 * @param string $command
-	 * @param string $pid
+	 * @param int $pid The process id to insert
 	 * @return bool
 	 * @throws \Exception
 	 */
-	public static function insert($command, $pid = null)
+	public function insert(string $command, int $pid)
 	{
 		$return = true;
 
-		if (is_null($pid)) {
-			$pid = getmypid();
+		$this->dba->transaction();
+
+		if (!$this->dba->exists('process', ['pid' => $pid])) {
+			$return = $this->dba->insert('process', ['pid' => $pid, 'command' => $command, 'created' => DateTimeFormat::utcNow()]);
 		}
 
-		DBA::transaction();
-
-		if (!DBA::exists('process', ['pid' => $pid])) {
-			$return = DBA::insert('process', ['pid' => $pid, 'command' => $command, 'created' => DateTimeFormat::utcNow()]);
-		}
-
-		DBA::commit();
+		$this->dba->commit();
 
 		return $return;
 	}
@@ -59,33 +63,29 @@ class Process
 	/**
 	 * Remove a process row by pid. If the pid parameter is omitted, we use the current pid
 	 *
-	 * @param string $pid
+	 * @param int $pid The pid to delete
 	 * @return bool
 	 * @throws \Exception
 	 */
-	public static function deleteByPid($pid = null)
+	public function deleteByPid(int $pid)
 	{
-		if ($pid === null) {
-			$pid = getmypid();
-		}
-
-		return DBA::delete('process', ['pid' => $pid]);
+		return $this->dba->delete('process', ['pid' => $pid]);
 	}
 
 	/**
 	 * Clean the process table of inactive physical processes
 	 */
-	public static function deleteInactive()
+	public function deleteInactive()
 	{
-		DBA::transaction();
+		$this->dba->transaction();
 
-		$processes = DBA::select('process', ['pid']);
-		while($process = DBA::fetch($processes)) {
+		$processes = $this->dba->select('process', ['pid']);
+		while($process = $this->dba->fetch($processes)) {
 			if (!posix_kill($process['pid'], 0)) {
-				self::deleteByPid($process['pid']);
+				$this->deleteByPid($process['pid']);
 			}
 		}
-		DBA::close($processes);
-		DBA::commit();
+		$this->dba->close($processes);
+		$this->dba->commit();
 	}
 }

src/Module/Worker.php

@@ -22,6 +22,7 @@
 namespace Friendica\Module;
 
 use Friendica\BaseModule;
+use Friendica\Core\Process;
 use Friendica\Core\System;
 use Friendica\Core\Worker as WorkerCore;
 use Friendica\Database\DBA;
@@ -57,7 +58,7 @@ class Worker extends BaseModule
 			return;
 		}
 
-		WorkerCore::startProcess();
+		DI::process()->start();
 
 		DI::logger()->notice('Front end worker started.', ['pid' => getmypid()]);
 
@@ -79,7 +80,7 @@ class Worker extends BaseModule
 
 		WorkerCore::unclaimProcess();
 
-		WorkerCore::endProcess();
+		DI::process()->end();
 
 		System::httpExit(200, 'Frontend worker stopped.');
 	}

static/dependencies.config.php

@@ -187,6 +187,7 @@ return [
 	Process::class => [
 		'constructParams' => [
 			[Dice::INSTANCE => '$basepath'],
+			getmypid(),
 		],
 	],
 	App\Router::class => [

tests/src/Model/ProcessTest.php

@@ -0,0 +1,81 @@
+<?php
+
+namespace Friendica\Test\src\Model;
+
+use Friendica\Factory\ConfigFactory;
+use Friendica\Model\Process;
+use Friendica\Test\DatabaseTest;
+use Friendica\Test\Util\Database\StaticDatabase;
+use Friendica\Test\Util\VFSTrait;
+use Friendica\Util\ConfigFileLoader;
+use Friendica\Util\Profiler;
+use Psr\Log\NullLogger;
+
+class ProcessTest extends DatabaseTest
+{
+	use VFSTrait;
+
+	/** @var StaticDatabase */
+	private $dba;
+
+	protected function setUp()
+	{
+		parent::setUp();
+
+		$this->setUpVfsDir();
+
+		$this->logger = new NullLogger();
+
+		$profiler = \Mockery::mock(Profiler::class);
+		$profiler->shouldReceive('saveTimestamp')->withAnyArgs()->andReturn(true);
+
+		// load real config to avoid mocking every config-entry which is related to the Database class
+		$configFactory = new ConfigFactory();
+		$loader        = new ConfigFileLoader($this->root->url());
+		$configCache   = $configFactory->createCache($loader);
+
+		$this->dba = new StaticDatabase($configCache, $profiler, $this->logger);
+	}
+
+	public function testInsertDelete()
+	{
+		$process = new Process($this->dba);
+
+		$this->assertEquals(0, $this->dba->count('process'));
+		$process->insert('test', 1);
+		$process->insert('test2', 2);
+		$process->insert('test3', 3);
+
+		$this->assertEquals(3, $this->dba->count('process'));
+
+		$this->assertEquals([
+			['command' => 'test']
+		], $this->dba->selectToArray('process', ['command'], ['pid' => 1]));
+
+		$process->deleteByPid(1);
+
+		$this->assertEmpty($this->dba->selectToArray('process', ['command'], ['pid' => 1]));
+
+		$this->assertEquals(2, $this->dba->count('process'));
+	}
+
+	public function testDoubleInsert()
+	{
+		$process = new Process($this->dba);
+
+		$process->insert('test', 1);
+
+		// double insert doesn't work
+		$process->insert('test23', 1);
+
+		$this->assertEquals([['command' => 'test']], $this->dba->selectToArray('process', ['command'], ['pid' => 1]));
+	}
+
+	public function testWrongDelete()
+	{
+		$process = new Process($this->dba);
+
+		// Just ignore wrong deletes, no execution is thrown
+		$process->deleteByPid(-1);
+	}
+}

tests/src/Model/User/CookieTest.php

@@ -19,16 +19,16 @@
  *
  */
 
-namespace Friendica\Testsrc\Model\User;
+namespace Friendica\Test\src\Model\User;
 
 use Friendica\App\BaseURL;
 use Friendica\Core\Config\IConfig;
 use Friendica\Model\User\Cookie;
-use Friendica\Test\DatabaseTest;
+use Friendica\Test\MockedTest;
 use Friendica\Test\Util\StaticCookie;
 use Mockery\MockInterface;
 
-class CookieTest extends DatabaseTest
+class CookieTest extends MockedTest
 {
 	/** @var MockInterface|IConfig */
 	private $config;