Corrections in code and add tests

Listener/VirusScannerListener.php

@@ -28,17 +28,6 @@ class VirusScannerListener
     public function __construct(Client $client)
     {
         $this->client = $client;
-
-        try {
-            // Check clamd server's state
-            $this->client->ping();
-
-            // Reload the virus database
-            $this->client->reload();
-        } catch (ConnectionException $connectionException) {
-            $this->client->shutdown();
-            $this->client = null;
-        }
     }
 
 
@@ -47,27 +36,42 @@ class VirusScannerListener
      */
     public function onFileUploadCompleted(FileUploadCompleteEvent $event)
     {
-        if ($this->client != null) {
-            $file = $event->getUploadedFile();
-            $path = $file->getLocalPath();
+        $this->client = $this->getClient();
+
+        $file = $event->getUploadedFile();
+        $path = $file->getLocalPath();
+
+        $result = $this->client->scanFile($path);
+        $hasVirus = $result['status'] === Client::RESULT_FOUND;
+
+        $meta = $file->getMetadata();
+        $meta['virus_scanner'] = ['has_virus' => $hasVirus];
 
-            $result = $this->client->scanFile($path);
-            $hasVirus = $result['status'] == Client::RESULT_FOUND;
+        if ($hasVirus) {
+            $desc = $result['reason'];
+            $meta['virus_scanner']['detected'] = $result['reason'];
+        }
 
-            $meta = $file->getMetadata();
-            $meta['virus_scanner'] = ['has_virus' => $hasVirus];
+        $file->setMetadata($meta);
 
-            if ($hasVirus) {
-                $desc = $result['reason'];
-                $meta['virus_scanner']['detected'] = $result['reason'];
-            }
+        if ($hasVirus) {
+            throw new RejectedFileException($file, $desc ? sprintf('Found the %s virus !', $desc) : 'Virus found !');
+        }
+    }
 
-            $file->setMetadata($meta);
+    /**
+     * @return Client
+     * @throws ConnectionException
+     */
+    private function getClient(): Client
+    {
+        try {
+            // Check clamd server's state
+            $this->client->ping();
+            return $this->client;
 
-            if ($hasVirus) {
-                throw new RejectedFileException($file, $desc ? sprintf('Found the %s virus !', $desc) : 'Virus found !');
-            }
-            $this->client->shutdown();
+        } catch (ConnectionException $connectionException) {
+            throw $connectionException;
         }
     }
 }

Resources/config/services.yml

@@ -10,9 +10,9 @@ parameters:
 
     irstea_file_upload.max_chunk_size: 0
 
-    irstea_file_upload.clamav_socket_path: unix:///var/run/clamav/clamd.ctl
-    irstea_file_upload.client_timeout: 30
-    irstea_file_upload.client_mode: PHP_NORMAL_READ
+    irstea_file_upload.virus_scanner.clamav_socket_path: unix:///var/run/clamav/clamd.ctl
+    irstea_file_upload.virus_scanner.client_timeout: 30
+    irstea_file_upload.virus_scanner.client_mode: !php/const:PHP_NORMAL_READ
 
 services:
 
@@ -74,22 +74,22 @@ services:
             - "%irstea_file_upload.filesystem.name%"
 
     # Scanner anti-virus
-    irstea_file_upload.socket:
+    irstea_file_upload.virus_scanner.socket:
             class: Socket\Raw\Socket
             arguments:
-                - "%irstea_file_upload.clamav_socket_path"
+                - "%irstea_file_upload.virus_scanner.clamav_socket_path"
 
-    irstea_file_upload.client:
+    irstea_file_upload.virus_scanner.client:
             class: Xenolope\Quahog\Client
             arguments:
-              - "@irstea_file_upload.socket"
-              - "%irstea_file_upload.client_timeout"
-              - "%irstea_file_upload.client_mode"
+              - "@irstea_file_upload.virus_scanner.socket"
+              - "%irstea_file_upload.virus_scanner.client_timeout"
+              - "%irstea_file_upload.virus_scanner.client_mode"
 
     irstea_file_upload.virus_scanner:
         class: Irstea\FileUploadBundle\Listener\VirusScannerListener
         arguments:
-            - "@irstea_file_upload.client"
+            - "@irstea_file_upload.virus_scanner.client"
         tags:
             - { name: kernel.event_listener, event: file_upload.complete, method: onFileUploadCompleted }
 

Tests/Listener/VirusScannerListenerTest.php

@@ -12,6 +12,7 @@ use PHPUnit_Framework_MockObject_MockObject;
 use PHPUnit_Framework_TestCase;
 use Irstea\FileUploadBundle\Model\UploadedFileInterface;
 use Xenolope\Quahog\Client;
+use Xenolope\Quahog\Exception\ConnectionException;
 
 /**
  * Generated by PHPUnit_SkeletonGenerator on 2015-01-29 at 14:43:16.
@@ -46,14 +47,15 @@ class VirusScannerListenerTest extends PHPUnit_Framework_TestCase
         $this->client = $this->getMockBuilder(Client::class)->disableOriginalConstructor()->getMock();
 
         $this->file = $this->getMock(UploadedFileInterface::class);
-        $this->file->expects(static::once())->method('getLocalPath')->willReturn('foopath');
-        $this->file->expects(static::once())->method('getMetadata')->willReturn([]);
+        $this->file->expects($this->any())->method('getLocalPath')->willReturn('foopath');
+        $this->file->expects($this->any())->method('getMetadata')->willReturn([]);
 
         $this->event = new FileUploadCompleteEvent($this->file);
 
         $this->listener = new VirusScannerListener($this->client);
     }
 
+
     public function testOnFileUploadCompletedResultOk()
     {
         $result = [
@@ -62,8 +64,8 @@ class VirusScannerListenerTest extends PHPUnit_Framework_TestCase
             'status' => Client::RESULT_OK
         ];
 
-        $this->client->expects(static::once())->method('scanFile')->with('foopath')->willReturn($result);
-        $this->file->expects(static::once())->method('setMetadata')->with(['virus_scanner' => ['has_virus' => false]]);
+        $this->client->expects($this->any())->method('scanFile')->with('foopath')->willReturn($result);
+        $this->file->expects($this->any())->method('setMetadata')->with(['virus_scanner' => ['has_virus' => false]]);
 
         $this->listener->onFileUploadCompleted($this->event);
     }
@@ -84,10 +86,27 @@ class VirusScannerListenerTest extends PHPUnit_Framework_TestCase
             ->with('foopath')
             ->willReturn($result);
 
-        $this->file->expects(static::once())
+        $this->file->expects($this->any())
             ->method('setMetadata')
             ->with(['virus_scanner' => ['has_virus' => true, 'detected' => 'Terrible virus']]);
 
         $this->listener->onFileUploadCompleted($this->event);
     }
+
+    /**
+     * @expectedException Xenolope\Quahog\Exception\ConnectionException
+     */
+    public function testGetClientFailed()
+    {
+        $this->client->expects($this->any())->method('ping')->willThrowException(new ConnectionException());
+        $this->listener->onFileUploadCompleted($this->event);
+    }
+
+
+    public function testGetClientSucceed()
+    {
+        $this->client->expects($this->any())->method('ping')->willReturn(true);
+        $this->listener->onFileUploadCompleted($this->event);
+    }
+
 }