I want to use this package for a project not using a database. Since the task's are written to some doctrine repository I added a sqlite migration.

With a configuration like

Neos:
  Flow:
    persistence:
      backendOptions:
        driver: 'pdo_sqlite'
        # in memory database
        #path: ':memory:'
        path: '%FLOW_PATH_DATA%Persistent/db.sqlite'

this package can now be used in simpler NEOS/Flow project

Without this change: all tasks will be locked and skipped from running, as soon as you configure a lock storage and implement the LockingTaskHandlerInterface

With this change: The return value is handled as intended/expected when a lock is acquired (symfony/lock acquire returns true upon successfull lock aquire, false upon lock conflict)

This change mostly tweaks some cosmetics, fixes a broken functional test and improves the UX of the commands a bit:

• Prevent null pointer exceptions in TaskCommandController and provide error message if a given task id does not exist
• Fix TaskRunner::reFetchExecution() that had no effect previously
• Fix type hints
• Fix Flowpack\Task\Tests\Functional\Domain\Repository\TaskExecutionRepositoryTest::findNext that is broken since version 0.1.0

Fixes: #5

While skimming through the code base I found out that the functional tests fail with the current version:

There was 1 failure:

1) Flowpack\Task\Tests\Functional\Domain\Repository\TaskExecutionRepositoryTest::findNext
First one should be taskTwo
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-'taskTwo'
+'taskOne'

I could track it back to Commit aedf7f848df444e02139763e1eb88a085c6d0986 that changed

->orderBy('taskExecution.scheduleTime', QueryInterface::ORDER_ASCENDING)

to

->orderBy('taskExecution.scheduleTime', QueryInterface::ORDER_DESCENDING)

AFAIS the test has to be adjusted

The exception property of the TaskExecution class can be null but the get and set methods are requiring a string.

    /**
     * @var string
     * @ORM\Column(nullable = true, type = "text")  //<--- Null is allowed
     */
    protected $exception;

    /**
     * @return string
     */
    public function getException(): string // <--- returns only a string
    {
        return $this->exception;
    }

    /**
     * @param string $exception
     * @return TaskExecution
     */
    public function setException(string $exception): TaskExecution
    {
        $this->exception = $exception;
        return $this;
    }

If you call the method ./flow task:sho <identifier> it is not possible to get a list of the runned tasks:

During TaskExecutionHistory::cleanup() DB entries are removed if the task handlers in the DB is not configured or doesn't exist anymore.
This behaviour can be configured by the setting keepRemovedTasksInTaskExecutionHistory which by default keeps the current behaviour and doesn't remove entries.

This PR is about auto cleanup of the TaskExecutionHistory if one removes a task from the configuration or a handlerClass is renamed. Old entries are then automatically removed if enabled.

I just had the case, that an invalid handleClass was written into the DB and had to remove it manually from there. With this PR, a configured tasks is checked with class_exists and if it doesn't exist, the this task is ignored and an info message is written to the system log.

Current behavior: task:runsingle runs all due tasks instead of only the one specified by the <task identifier>

Expected behavior: task:runsingle runs only the task specified by the <task identifier>, if task execution is due

I got confused by the names of the two run* commands because I expected them to, well, run the tasks but in fact even ./flow task:runSingle only schedules a task – or even doesn't do anything if one is scheduled already.

(PS: I copied the naming to my TaskModule package which makes it even more confusing)

Suggestion

I propose to deprecate the two commands task:run and task:runSingle in favor of a new command task:schedule (or similar) with an optional filter to schedule tasks by id etc. (task:run and task:runSingle should still work of course for B/C but they would be hidden from ./flow help)

Additionally I could imagine another command task:execute that allows to immediately run a task whether it is scheduled or not.

@daniellienert I could take care of this suggestion, but would like to hear your opinion on it, first