File events refactoring

app/Core/Base.php

@@ -151,6 +151,8 @@ use Pimple\Container;
  * @property \Kanboard\Core\Filter\LexerBuilder                  $taskLexer
  * @property \Kanboard\Core\Filter\LexerBuilder                  $projectActivityLexer
  * @property \Kanboard\Job\CommentEventJob                       $commentEventJob
+ * @property \Kanboard\Job\TaskFileEventJob                      $taskFileEventJob
+ * @property \Kanboard\Job\ProjectFileEventJob                   $projectFileEventJob
  * @property \Kanboard\Job\NotificationJob                       $notificationJob
  * @property \Psr\Log\LoggerInterface                            $logger
  * @property \PicoDb\Database                                    $db

app/Event/FileEvent.php

@@ -1,7 +0,0 @@
-<?php
-
-namespace Kanboard\Event;
-
-class FileEvent extends GenericEvent
-{
-}

app/Event/ProjectFileEvent.php

@@ -0,0 +1,7 @@
+<?php
+
+namespace Kanboard\Event;
+
+class ProjectFileEvent extends GenericEvent
+{
+}

app/Event/TaskFileEvent.php

@@ -0,0 +1,7 @@
+<?php
+
+namespace Kanboard\Event;
+
+class TaskFileEvent extends GenericEvent
+{
+}

app/EventBuilder/ProjectFileEventBuilder.php

@@ -0,0 +1,50 @@
+<?php
+
+namespace Kanboard\EventBuilder;
+
+use Kanboard\Event\ProjectFileEvent;
+use Kanboard\Event\GenericEvent;
+
+/**
+ * Class ProjectFileEventBuilder
+ *
+ * @package Kanboard\EventBuilder
+ * @author  Frederic Guillot
+ */
+class ProjectFileEventBuilder extends BaseEventBuilder
+{
+    protected $fileId = 0;
+
+    /**
+     * Set fileId
+     *
+     * @param  int $fileId
+     * @return $this
+     */
+    public function withFileId($fileId)
+    {
+        $this->fileId = $fileId;
+        return $this;
+    }
+
+    /**
+     * Build event data
+     *
+     * @access public
+     * @return GenericEvent|null
+     */
+    public function build()
+    {
+        $file = $this->projectFileModel->getById($this->fileId);
+
+        if (empty($file)) {
+            $this->logger->debug(__METHOD__.': File not found');
+            return null;
+        }
+
+        return new ProjectFileEvent(array(
+            'file' => $file,
+            'project' => $this->projectModel->getById($file['project_id']),
+        ));
+    }
+}

app/EventBuilder/TaskFileEventBuilder.php

@@ -0,0 +1,50 @@
+<?php
+
+namespace Kanboard\EventBuilder;
+
+use Kanboard\Event\TaskFileEvent;
+use Kanboard\Event\GenericEvent;
+
+/**
+ * Class TaskFileEventBuilder
+ *
+ * @package Kanboard\EventBuilder
+ * @author  Frederic Guillot
+ */
+class TaskFileEventBuilder extends BaseEventBuilder
+{
+    protected $fileId = 0;
+
+    /**
+     * Set fileId
+     *
+     * @param  int $fileId
+     * @return $this
+     */
+    public function withFileId($fileId)
+    {
+        $this->fileId = $fileId;
+        return $this;
+    }
+
+    /**
+     * Build event data
+     *
+     * @access public
+     * @return GenericEvent|null
+     */
+    public function build()
+    {
+        $file = $this->taskFileModel->getById($this->fileId);
+
+        if (empty($file)) {
+            $this->logger->debug(__METHOD__.': File not found');
+            return null;
+        }
+
+        return new TaskFileEvent(array(
+            'file' => $file,
+            'task' => $this->taskFinderModel->getDetails($file['task_id']),
+        ));
+    }
+}

app/Job/NotificationJob.php

@@ -70,13 +70,8 @@ class NotificationJob extends BaseJob
                 $values['subtask'] = $this->subtaskModel->getById($event['id'], true);
                 $values['task'] = $this->taskFinderModel->getDetails($values['subtask']['task_id']);
                 break;
-            case 'Kanboard\Event\FileEvent':
+            default:
-                $values['file'] = $event;
-                $values['task'] = $this->taskFinderModel->getDetails($values['file']['task_id']);
-                break;
-            case 'Kanboard\Event\CommentEvent':
                 $values = $event;
-                break;
         }
 
         return $values;

app/Job/ProjectFileEventJob.php

@@ -0,0 +1,45 @@
+<?php
+
+namespace Kanboard\Job;
+
+use Kanboard\EventBuilder\ProjectFileEventBuilder;
+
+/**
+ * Class ProjectFileEventJob
+ *
+ * @package Kanboard\Job
+ * @author  Frederic Guillot
+ */
+class ProjectFileEventJob extends BaseJob
+{
+    /**
+     * Set job params
+     *
+     * @param  int    $fileId
+     * @param  string $eventName
+     * @return $this
+     */
+    public function withParams($fileId, $eventName)
+    {
+        $this->jobParams = array($fileId, $eventName);
+        return $this;
+    }
+
+    /**
+     * Execute job
+     *
+     * @param  int    $fileId
+     * @param  string $eventName
+     * @return $this
+     */
+    public function execute($fileId, $eventName)
+    {
+        $event = ProjectFileEventBuilder::getInstance($this->container)
+            ->withFileId($fileId)
+            ->build();
+
+        if ($event !== null) {
+            $this->dispatcher->dispatch($eventName, $event);
+        }
+    }
+}

app/Job/TaskFileEventJob.php

@@ -0,0 +1,45 @@
+<?php
+
+namespace Kanboard\Job;
+
+use Kanboard\EventBuilder\TaskFileEventBuilder;
+
+/**
+ * Class TaskFileEventJob
+ *
+ * @package Kanboard\Job
+ * @author  Frederic Guillot
+ */
+class TaskFileEventJob extends BaseJob
+{
+    /**
+     * Set job params
+     *
+     * @param  int    $fileId
+     * @param  string $eventName
+     * @return $this
+     */
+    public function withParams($fileId, $eventName)
+    {
+        $this->jobParams = array($fileId, $eventName);
+        return $this;
+    }
+
+    /**
+     * Execute job
+     *
+     * @param  int    $fileId
+     * @param  string $eventName
+     * @return $this
+     */
+    public function execute($fileId, $eventName)
+    {
+        $event = TaskFileEventBuilder::getInstance($this->container)
+            ->withFileId($fileId)
+            ->build();
+
+        if ($event !== null) {
+            $this->dispatcher->dispatch($eventName, $event);
+        }
+    }
+}

app/Model/FileModel.php

@@ -5,7 +5,6 @@ namespace Kanboard\Model;
 use Exception;
 use Kanboard\Core\Base;
 use Kanboard\Core\Thumbnail;
-use Kanboard\Event\FileEvent;
 use Kanboard\Core\ObjectStorage\ObjectStorageException;
 
 /**
@@ -44,13 +43,13 @@ abstract class FileModel extends Base
     abstract protected function getPathPrefix();
 
     /**
-     * Get event name
+     * Fire file creation event
      *
      * @abstract
      * @access protected
-     * @return string
+     * @param  integer $file_id
      */
-    abstract protected function getEventName();
+    abstract protected function fireCreationEvent($file_id);
 
     /**
      * Get PicoDb query to get all files
@@ -130,16 +129,16 @@ abstract class FileModel extends Base
      * Create a file entry in the database
      *
      * @access public
-     * @param  integer  $id         Foreign key
+     * @param  integer $foreign_key_id Foreign key
-     * @param  string   $name       Filename
+     * @param  string  $name           Filename
-     * @param  string   $path       Path on the disk
+     * @param  string  $path           Path on the disk
-     * @param  integer  $size       File size
+     * @param  integer $size           File size
      * @return bool|integer
      */
-    public function create($id, $name, $path, $size)
+    public function create($foreign_key_id, $name, $path, $size)
     {
         $values = array(
-            $this->getForeignKey() => $id,
+            $this->getForeignKey() => $foreign_key_id,
             'name' => substr($name, 0, 255),
             'path' => $path,
             'is_image' => $this->isImage($name) ? 1 : 0,
@@ -152,8 +151,7 @@ abstract class FileModel extends Base
 
         if ($result) {
             $file_id = (int) $this->db->getLastId();
-            $event = new FileEvent($values + array('file_id' => $file_id));
+            $this->fireCreationEvent($file_id);
-            $this->dispatcher->dispatch($this->getEventName(), $event);
             return $file_id;
         }
 

app/Model/ProjectFileModel.php

@@ -61,14 +61,13 @@ class ProjectFileModel extends FileModel
     }
 
     /**
-     * Get event name
+     * Fire file creation event
      *
-     * @abstract
      * @access protected
-     * @return string
+     * @param  integer $file_id
      */
-    protected function getEventName()
+    protected function fireCreationEvent($file_id)
     {
-        return self::EVENT_CREATE;
+        $this->queueManager->push($this->projectFileEventJob->withParams($file_id, self::EVENT_CREATE));
     }
 }

app/Model/TaskFileModel.php

@@ -60,18 +60,6 @@ class TaskFileModel extends FileModel
         return 'tasks';
     }
 
-    /**
-     * Get event name
-     *
-     * @abstract
-     * @access protected
-     * @return string
-     */
-    protected function getEventName()
-    {
-        return self::EVENT_CREATE;
-    }
-
     /**
      * Get projectId from fileId
      *
@@ -101,4 +89,15 @@ class TaskFileModel extends FileModel
         $original_filename = e('Screenshot taken %s', $this->helper->dt->datetime(time())).'.png';
         return $this->uploadContent($task_id, $original_filename, $blob);
     }
+
+    /**
+     * Fire file creation event
+     *
+     * @access protected
+     * @param  integer $file_id
+     */
+    protected function fireCreationEvent($file_id)
+    {
+        $this->queueManager->push($this->taskFileEventJob->withParams($file_id, self::EVENT_CREATE));
+    }
 }

app/ServiceProvider/ClassProvider.php

@@ -28,6 +28,8 @@ class ClassProvider implements ServiceProviderInterface
         ),
         'Job' => array(
             'CommentEventJob',
+            'TaskFileEventJob',
+            'ProjectFileEventJob',
             'NotificationJob',
         ),
         'Model' => array(

app/Subscriber/BaseSubscriber.php

@@ -12,28 +12,4 @@ use Kanboard\Core\Base;
  */
 class BaseSubscriber extends Base
 {
-    /**
-     * Method called
-     *
-     * @access private
-     * @var array
-     */
-    private $called = array();
-
-    /**
-     * Check if a listener has been executed
-     *
-     * @access public
-     * @param  string  $key
-     * @return boolean
-     */
-    public function isExecuted($key = '')
-    {
-        if (isset($this->called[$key])) {
-            return true;
-        }
-
-        $this->called[$key] = true;
-        return false;
-    }
 }

app/Subscriber/NotificationSubscriber.php

@@ -36,12 +36,10 @@ class NotificationSubscriber extends BaseSubscriber implements EventSubscriberIn
 
     public function handleEvent(GenericEvent $event, $eventName)
     {
-        if (!$this->isExecuted($eventName)) {
+        $this->logger->debug('Subscriber executed: ' . __METHOD__);
-            $this->logger->debug('Subscriber executed: ' . __METHOD__);
 
-            $this->queueManager->push(NotificationJob::getInstance($this->container)
+        $this->queueManager->push(NotificationJob::getInstance($this->container)
-                ->withParams($event, $eventName, get_class($event))
+            ->withParams($event, $eventName, get_class($event))
-            );
+        );
-        }
     }
 }

app/Subscriber/ProjectDailySummarySubscriber.php

@@ -22,7 +22,7 @@ class ProjectDailySummarySubscriber extends BaseSubscriber implements EventSubsc
 
     public function execute(TaskEvent $event)
     {
-        if (isset($event['project_id']) && !$this->isExecuted()) {
+        if (isset($event['project_id'])) {
             $this->logger->debug('Subscriber executed: '.__METHOD__);
             $this->queueManager->push(ProjectMetricJob::getInstance($this->container)->withParams($event['project_id']));
         }

app/Subscriber/ProjectModificationDateSubscriber.php

@@ -24,7 +24,7 @@ class ProjectModificationDateSubscriber extends BaseSubscriber implements EventS
 
     public function execute(GenericEvent $event)
     {
-        if (isset($event['project_id']) && !$this->isExecuted()) {
+        if (isset($event['project_id'])) {
             $this->logger->debug('Subscriber executed: '.__METHOD__);
             $this->projectModel->updateModificationDate($event['project_id']);
         }

tests/units/EventBuilder/ProjectFileEventBuilderTest.php

@@ -0,0 +1,33 @@
+<?php
+
+use Kanboard\EventBuilder\ProjectFileEventBuilder;
+use Kanboard\Model\ProjectFileModel;
+use Kanboard\Model\ProjectModel;
+
+require_once __DIR__.'/../Base.php';
+
+class ProjectFileEventBuilderTest extends Base
+{
+    public function testWithMissingFile()
+    {
+        $projectFileEventBuilder = new ProjectFileEventBuilder($this->container);
+        $projectFileEventBuilder->withFileId(42);
+        $this->assertNull($projectFileEventBuilder->build());
+    }
+
+    public function testBuild()
+    {
+        $projectModel = new ProjectModel($this->container);
+        $projectFileModel = new ProjectFileModel($this->container);
+        $projectFileEventBuilder = new ProjectFileEventBuilder($this->container);
+
+        $this->assertEquals(1, $projectModel->create(array('name' => 'test1')));
+        $this->assertEquals(1, $projectFileModel->create(1, 'Test', '/tmp/test', 123));
+
+        $event = $projectFileEventBuilder->withFileId(1)->build();
+
+        $this->assertInstanceOf('Kanboard\Event\ProjectFileEvent', $event);
+        $this->assertNotEmpty($event['file']);
+        $this->assertNotEmpty($event['project']);
+    }
+}

tests/units/EventBuilder/TaskFileEventBuilderTest.php

@@ -0,0 +1,36 @@
+<?php
+
+use Kanboard\EventBuilder\TaskFileEventBuilder;
+use Kanboard\Model\ProjectModel;
+use Kanboard\Model\TaskCreationModel;
+use Kanboard\Model\TaskFileModel;
+
+require_once __DIR__.'/../Base.php';
+
+class TaskFileEventBuilderTest extends Base
+{
+    public function testWithMissingFile()
+    {
+        $taskFileEventBuilder = new TaskFileEventBuilder($this->container);
+        $taskFileEventBuilder->withFileId(42);
+        $this->assertNull($taskFileEventBuilder->build());
+    }
+
+    public function testBuild()
+    {
+        $taskFileModel = new TaskFileModel($this->container);
+        $taskCreationModel = new TaskCreationModel($this->container);
+        $projectModel = new ProjectModel($this->container);
+        $taskFileEventBuilder = new TaskFileEventBuilder($this->container);
+
+        $this->assertEquals(1, $projectModel->create(array('name' => 'test1')));
+        $this->assertEquals(1, $taskCreationModel->create(array('title' => 'test', 'project_id' => 1)));
+        $this->assertEquals(1, $taskFileModel->create(1, 'Test', '/tmp/test', 123));
+
+        $event = $taskFileEventBuilder->withFileId(1)->build();
+
+        $this->assertInstanceOf('Kanboard\Event\TaskFileEvent', $event);
+        $this->assertNotEmpty($event['file']);
+        $this->assertNotEmpty($event['task']);
+    }
+}

tests/units/Job/ProjectFileEventJobTest.php

@@ -0,0 +1,43 @@
+<?php
+
+use Kanboard\Job\ProjectFileEventJob;
+use Kanboard\Model\ProjectFileModel;
+use Kanboard\Model\ProjectModel;
+
+require_once __DIR__.'/../Base.php';
+
+class ProjectFileEventJobTest extends Base
+{
+    public function testJobParams()
+    {
+        $projectFileEventJob = new ProjectFileEventJob($this->container);
+        $projectFileEventJob->withParams(123, 'foobar');
+
+        $this->assertSame(array(123, 'foobar'), $projectFileEventJob->getJobParams());
+    }
+
+    public function testWithMissingFile()
+    {
+        $this->container['dispatcher']->addListener(ProjectFileModel::EVENT_CREATE, function() {});
+
+        $projectFileEventJob = new ProjectFileEventJob($this->container);
+        $projectFileEventJob->execute(42, ProjectFileModel::EVENT_CREATE);
+
+        $called = $this->container['dispatcher']->getCalledListeners();
+        $this->assertEmpty($called);
+    }
+
+    public function testTriggerEvents()
+    {
+        $this->container['dispatcher']->addListener(ProjectFileModel::EVENT_CREATE, function() {});
+
+        $projectModel = new ProjectModel($this->container);
+        $projectFileModel = new ProjectFileModel($this->container);
+
+        $this->assertEquals(1, $projectModel->create(array('name' => 'test1')));
+        $this->assertEquals(1, $projectFileModel->create(1, 'Test', '/tmp/test', 123));
+
+        $called = $this->container['dispatcher']->getCalledListeners();
+        $this->assertArrayHasKey(ProjectFileModel::EVENT_CREATE.'.closure', $called);
+    }
+}

tests/units/Job/TaskFileEventJobTest.php

@@ -0,0 +1,46 @@
+<?php
+
+use Kanboard\Job\TaskFileEventJob;
+use Kanboard\Model\ProjectModel;
+use Kanboard\Model\TaskCreationModel;
+use Kanboard\Model\TaskFileModel;
+
+require_once __DIR__.'/../Base.php';
+
+class TaskFileEventJobTest extends Base
+{
+    public function testJobParams()
+    {
+        $taskFileEventJob = new TaskFileEventJob($this->container);
+        $taskFileEventJob->withParams(123, 'foobar');
+
+        $this->assertSame(array(123, 'foobar'), $taskFileEventJob->getJobParams());
+    }
+
+    public function testWithMissingFile()
+    {
+        $this->container['dispatcher']->addListener(TaskFileModel::EVENT_CREATE, function() {});
+
+        $taskFileEventJob = new TaskFileEventJob($this->container);
+        $taskFileEventJob->execute(42, TaskFileModel::EVENT_CREATE);
+
+        $called = $this->container['dispatcher']->getCalledListeners();
+        $this->assertEmpty($called);
+    }
+
+    public function testTriggerEvents()
+    {
+        $this->container['dispatcher']->addListener(TaskFileModel::EVENT_CREATE, function() {});
+
+        $taskFileModel = new TaskFileModel($this->container);
+        $taskCreationModel = new TaskCreationModel($this->container);
+        $projectModel = new ProjectModel($this->container);
+
+        $this->assertEquals(1, $projectModel->create(array('name' => 'test1')));
+        $this->assertEquals(1, $taskCreationModel->create(array('title' => 'test', 'project_id' => 1)));
+        $this->assertEquals(1, $taskFileModel->create(1, 'Test', '/tmp/test', 123));
+
+        $called = $this->container['dispatcher']->getCalledListeners();
+        $this->assertArrayHasKey(TaskFileModel::EVENT_CREATE.'.closure', $called);
+    }
+}