Misc improvements + add tests

CHANGELOG.md

@@ -1,3 +1,21 @@
+3.3.0
+=====
+
+* (feature) Properly serialize task objects in task log.
+* (deprecation) Deprecate `TaskLog::getTaskObject()`. Use `TaskDetailsNormalizer::deserializeTask()` instead.
+* (improvement) Avoid redundant `COUNT` queries in `LogCleaner` by using the count of fetched IDs directly.
+* (internal) Add tests for `TaskDetailsNormalizer`, `LogCleaner`, `TaskRegistry`, `RegisterTasksEvent`, `TaskManager`, `TaskLog`, and `TaskRun`.
+
+
+3.2.4
+=====
+
+* (bug) Fix stamps passed to `TaskManager::enqueue()` being silently dropped due to `Envelope::with()` being immutable.
+* (improvement) Add database index on `time_queued` column of `task_manager_tasks` for faster log queries and cleanup.
+* (internal) Remove unused `Paginator` wrapper in `TaskLogModel::getMostRecentEntries()`.
+* (security) Replace `serialize()`/`unserialize()` with Symfony Serializer in `TaskDetailsNormalizer`. Add `TaskDetailsNormalizer::deserializeTask()` as replacement for the deprecated `TaskLog::getTaskObject()`.
+
+
 3.2.3
 =====
 

UPGRADE.md

@@ -1,3 +1,9 @@
+3.x to 4.0
+==========
+
+* `TaskLog::getTaskObject()` was removed. Use `TaskDetailsNormalizer::deserializeTask($log)` instead.
+
+
 1.x to 2.0
 ==========
 

composer.json

@@ -30,6 +30,7 @@
 		"symfony/lock": "^7.4 || ^8.0",
 		"symfony/messenger": "^7.4 || ^8.0",
 		"symfony/scheduler": "^7.4 || ^8.0",
+		"symfony/serializer": "^7.4 || ^8.0",
 		"symfony/string": "^7.4 || ^8.0",
 		"symfony/uid": "^7.4 || ^8.0"
 	},
@@ -63,8 +64,8 @@
 			"forward-command": true
 		},
 		"branch-alias": {
-			"2.x-dev": "2.99.x-dev",
-			"dev-next": "2.99.x-dev"
+			"3.x-dev": "3.99.x-dev",
+			"dev-next": "3.99.x-dev"
 		}
 	},
 	"scripts": {

src/Console/ChainOutput.php

@@ -9,7 +9,7 @@
 
 final readonly class ChainOutput implements OutputInterface
 {
-	private ConsoleOutput $consoleOutput;
+	private OutputInterface $mainOutput;
 	private BufferedOutput $bufferedOutput;
 
 	/**
@@ -18,10 +18,19 @@
 		int $verbosity = self::VERBOSITY_NORMAL,
 		bool $decorated = true,
 		?OutputFormatterInterface $formatter = null,
+		?OutputInterface $mainOutput = null,
 	)
 	{
 		$this->bufferedOutput = new BufferedOutput($verbosity, $decorated, $formatter);
-		$this->consoleOutput = new ConsoleOutput($verbosity, $decorated, $formatter);
+
+		$this->mainOutput = $mainOutput ?? new ConsoleOutput();
+		$this->mainOutput->setDecorated($decorated);
+		$this->mainOutput->setVerbosity($verbosity);
+
+		if (null !== $formatter)
+		{
+			$this->mainOutput->setFormatter($formatter);
+		}
 	}
 
 	/**
@@ -31,7 +40,7 @@
 	public function write (iterable|string $messages, bool $newline = false, int $options = 0) : void
 	{
 		$this->bufferedOutput->write($messages, $newline, $options);
-		$this->consoleOutput->write($messages, $newline, $options);
+		$this->mainOutput->write($messages, $newline, $options);
 	}
 
 	/**
@@ -41,7 +50,7 @@
 	public function writeln (iterable|string $messages, int $options = 0) : void
 	{
 		$this->bufferedOutput->writeln($messages, $options);
-		$this->consoleOutput->writeln($messages, $options);
+		$this->mainOutput->writeln($messages, $options);
 	}
 
 	/**
@@ -51,7 +60,7 @@
 	public function setVerbosity (int $level) : void
 	{
 		$this->bufferedOutput->setVerbosity($level);
-		$this->consoleOutput->setVerbosity($level);
+		$this->mainOutput->setVerbosity($level);
 	}
 
 	/**
@@ -113,8 +122,8 @@
 	#[\Override]
 	public function setDecorated (bool $decorated) : void
 	{
-		$this->bufferedOutput->setDecorated(true);
-		$this->consoleOutput->setDecorated(true);
+		$this->bufferedOutput->setDecorated($decorated);
+		$this->mainOutput->setDecorated($decorated);
 	}
 
 	/**
@@ -132,7 +141,7 @@
 	public function setFormatter (OutputFormatterInterface $formatter) : void
 	{
 		$this->bufferedOutput->setFormatter($formatter);
-		$this->consoleOutput->setFormatter($formatter);
+		$this->mainOutput->setFormatter($formatter);
 	}
 
 	/**

src/Entity/TaskLog.php

@@ -24,6 +24,7 @@
  */
 #[ORM\Entity]
 #[ORM\Table(name: "task_manager_tasks")]
+#[ORM\Index(columns: ["time_queued"], name: "idx_task_manager_tasks_time_queued")]
 class TaskLog
 {
 	/**
@@ -194,18 +195,15 @@ public function getTaskLabel () : ?string
 	}
 
 	/**
-	 * Returns the unserialized cached task object
+	 * @deprecated Use TaskDetailsNormalizer::deserializeTask() instead. Will be removed in 4.0.
+	 *
+	 * @todo Remove in 4.0.
 	 */
-	public function getTaskObject () : ?object
+	public function getTaskObject () : null
 	{
-		$task = $this->getTaskDetails()["task"] ?? null;
-		$unserialized = \is_string($task)
-			? unserialize($task)
-			: null;
-
-		return \is_object($unserialized)
-			? $unserialized
-			: null;
+		trigger_deprecation("21torr/task-manager", "3.2.5", "TaskLog::getTaskObject() is deprecated, use TaskDetailsNormalizer::deserializeTask() instead.");
+
+		return null;
 	}
 
 	/**

src/Log/LogCleaner.php

@@ -6,14 +6,12 @@
 use Symfony\Component\Clock\ClockInterface;
 use Torr\TaskManager\Entity\TaskLog;
 use Torr\TaskManager\Entity\TaskRun;
-use Torr\TaskManager\Model\TaskLogModel;
 
 final readonly class LogCleaner
 {
 	public function __construct (
 		private int $logTtlInDays,
 		private int $logMaxEntries,
-		private TaskLogModel $model,
 		private EntityManagerInterface $entityManager,
 		private ClockInterface $clock,
 	) {}
@@ -23,49 +21,32 @@ public function __construct (
 	 */
 	public function cleanLogEntries () : int
 	{
-		$tasksBefore = $this->model->getTaskCount();
-
 		$taskIdToDelete = $this->fetchIdsToDelete();
 
+		if (empty($taskIdToDelete))
+		{
+			return 0;
+		}
+
 		// first delete runs, as they are a foreign key on the task logs
 		$this->deleteRuns($taskIdToDelete);
 
 		// then delete tasks
 		$this->deleteTasks($taskIdToDelete);
 
-		$tasksAfter = $this->model->getTaskCount();
-
-		return $tasksBefore > $tasksAfter
-			? ($tasksBefore - $tasksAfter)
-			: 0;
+		return \count($taskIdToDelete);
 	}
 
 	/**
 	 *
 	 */
 	private function deleteRuns (array $taskIdsToDelete) : void
 	{
-		$runIdsToDelete = $this->entityManager->createQueryBuilder()
-			->select("run.id")
-			->from(TaskRun::class, "run")
-			->leftJoin("run.taskLog", "task")
-			->andWhere("task.id IN (:taskIds)")
-			->setParameter("taskIds", $taskIdsToDelete)
-			->getQuery()
-			->getArrayResult();
-
-		if (empty($runIdsToDelete))
-		{
-			return;
-		}
-
-		$runIdsToDelete = array_column($runIdsToDelete, "id");
-
 		$this->entityManager->createQueryBuilder()
 			->delete()
 			->from(TaskRun::class, "run")
-			->andWhere("run.id IN (:runIds)")
-			->setParameter("runIds", $runIdsToDelete)
+			->andWhere("IDENTITY(run.taskLog) IN (:taskIds)")
+			->setParameter("taskIds", $taskIdsToDelete)
 			->getQuery()
 			->execute();
 	}

src/Manager/TaskManager.php

@@ -38,8 +38,7 @@ public function enqueue (Task $task, array $stamps = []) : bool
 			return false;
 		}
 
-		$envelope = new Envelope($task);
-		$envelope->with(...$stamps);
+		$envelope = new Envelope($task, $stamps);
 
 		$this->messageBus->dispatch($envelope);
 

src/Model/TaskLogModel.php

@@ -4,7 +4,6 @@
 
 use Doctrine\ORM\EntityManagerInterface;
 use Doctrine\ORM\EntityRepository;
-use Doctrine\ORM\Tools\Pagination\Paginator;
 use Psr\Log\LoggerInterface;
 use Torr\TaskManager\Entity\TaskLog;
 use Torr\TaskManager\Entity\TaskRun;
@@ -58,14 +57,6 @@ public function getLogForTask (Task $task) : TaskLog
 		return $log;
 	}
 
-	/**
-	 *
-	 */
-	public function getTaskCount () : int
-	{
-		return $this->repository->count();
-	}
-
 	/**
 	 * Returns the latest task log entries
 	 *
@@ -92,9 +83,7 @@ public function getMostRecentEntries (
 		}
 
 		/** @var TaskLog[] */
-		return (new Paginator($builder->getQuery()))
-			->getQuery()
-			->getResult();
+		return $builder->getQuery()->getResult();
 	}
 
 	/**

src/Normalizer/TaskDetailsNormalizer.php

@@ -2,9 +2,13 @@
 
 namespace Torr\TaskManager\Normalizer;
 
+use Psr\Log\LoggerInterface;
 use Symfony\Component\Messenger\Envelope;
 use Symfony\Component\Messenger\Stamp\HandledStamp;
 use Symfony\Component\Messenger\Stamp\ReceivedStamp;
+use Symfony\Component\Serializer\Encoder\JsonEncoder;
+use Symfony\Component\Serializer\Exception\ExceptionInterface as SerializerException;
+use Symfony\Component\Serializer\SerializerInterface;
 use Torr\TaskManager\Entity\TaskLog;
 use Torr\TaskManager\Task\Task;
 
@@ -13,6 +17,11 @@
  */
 final readonly class TaskDetailsNormalizer
 {
+	public function __construct (
+		private SerializerInterface $serializer,
+		private LoggerInterface $logger,
+	) {}
+
 	/**
 	 * @return TaskDetails
 	 */
@@ -29,9 +38,40 @@ public function normalizeTaskDetails (Envelope $envelope) : array
 		if ($task instanceof Task)
 		{
 			$details["label"] = $task->getMetaData()->label;
-			$details["task"] = serialize($task);
+			$details["task"] = $this->serializer->serialize($task, JsonEncoder::FORMAT);
 		}
 
 		return $details;
 	}
+
+	/**
+	 * Deserializes the task object stored in the given log entry.
+	 */
+	public function deserializeTask (TaskLog $log) : ?Task
+	{
+		$serialized = $log->getTaskDetails()["task"] ?? null;
+
+		if (!\is_string($serialized))
+		{
+			return null;
+		}
+
+		try
+		{
+			$task = $this->serializer->deserialize($serialized, $log->taskClass, JsonEncoder::FORMAT);
+
+			return $task instanceof Task ? $task : null;
+		}
+		catch (SerializerException $exception)
+		{
+			$this->logger->error("Failed to deserialize task of class '{taskClass}' from log entry #{logId}: {message}", [
+				"taskClass" => $log->taskClass,
+				"logId" => $log->id,
+				"message" => $exception->getMessage(),
+				"exception" => $exception,
+			]);
+
+			return null;
+		}
+	}
 }