Changes according to the review

appinfo/application.php

@@ -34,7 +34,8 @@ class Application extends App {
 		$container->registerService('SettingsController', function($c) {
 			return new SettingsController(
 				$c->query('Request'),
-				$c->query('Appconfig')
+				$c->query('Appconfig'),
+				$c->query('L10N')	
 			);
 		});
 		$container->registerService('Appconfig', function($c) {
@@ -45,8 +46,9 @@ class Application extends App {
 		
         $container->registerService('BackgroundScanner', function($c) {
 			return new BackgroundScanner(
-				$c->query('ServerContainer')->getRootFolder(),
+				$c->query('Appconfig'),
-				$c->query('Appconfig')
+				$c->query('ServerContainer')->getUserManager(),
+				$c->query('L10N')
 			);
         });
         $container->registerService('FilesystemHooks', function($c) {

controller/settingscontroller.php

@@ -10,17 +10,27 @@ namespace OCA\Files_Antivirus\Controller;
 
 use \OCP\AppFramework\Controller;
 use \OCP\IRequest;
+use \OCP\IL10N;
 use \OCA\Files_Antivirus\Appconfig;
 
 use \OCP\AppFramework\Http\TemplateResponse;
 use \OCP\AppFramework\Http\JSONResponse;
 
 class SettingsController extends Controller {
-	
+
+	/**
+	 * @var Appconfig
+	 */
 	private $settings;
 	
-	public function __construct(IRequest $request, Appconfig $appconfig) {
+	/**
+	 * @var IL10N
+	 */
+	private $l10n;
+	
+	public function __construct(IRequest $request, Appconfig $appconfig, IL10N $l10n) {
 		$this->settings = $appconfig;
+		$this->l10n = $l10n;
 	}
 	
 	/**
@@ -34,26 +44,34 @@ class SettingsController extends Controller {
 	
 	/**
 	 * Save Parameters
-	 * @param string $av_mode - antivirus mode
+	 * @param string $avMode - antivirus mode
-	 * @param string $av_socket - path to socket (Socket mode)
+	 * @param string $avSocket - path to socket (Socket mode)
-	 * @param string $av_host - antivirus url
+	 * @param string $avHost - antivirus url
-	 * @param type $av_port - port
+	 * @param int $avPort - port
-	 * @param type $av_cmd_options - extra command line options
+	 * @param string $avCmdOptions - extra command line options
-	 * @param type $av_chunk_size - Size of one portion
+	 * @param int $avChunkSize - Size of one portion
-	 * @param type $av_path - path to antivirus executable (Executable mode)
+	 * @param string $avPath - path to antivirus executable (Executable mode)
-	 * @param type $av_infected_action - action performed on infected files
+	 * @param string $avInfectedAction - action performed on infected files
 	 * @return JSONResponse
 	 */
-	public function save($av_mode, $av_socket, $av_host, $av_port, $av_cmd_options, $av_chunk_size, $av_path, $av_infected_action) {
+	public function save($avMode, $avSocket, $avHost, $avPort, $avCmdOptions, $avChunkSize, $avPath, $avInfectedAction) {
-		$this->settings->setAvMode($av_mode);
+		$this->settings->setAvMode($avMode);
-		$this->settings->setAvSocket($av_socket);
+		$this->settings->setAvSocket($avSocket);
-		$this->settings->setAvHost($av_host);
+		$this->settings->setAvHost($avHost);
-		$this->settings->setAvPort($av_port);
+		$this->settings->setAvPort($avPort);
-		$this->settings->setAvCmdOptions($av_cmd_options);
+		$this->settings->setAvCmdOptions($avCmdOptions);
-		$this->settings->setAvChunkSize($av_chunk_size);
+		$this->settings->setAvChunkSize($avChunkSize);
-		$this->settings->setAvPath($av_path);
+		$this->settings->setAvPath($avPath);
-		$this->settings->setAvInfectedAction($av_infected_action);
+		$this->settings->setAvInfectedAction($avInfectedAction);
 		
-		return new JSONResponse($this->settings->getAllValues());
+		return new JSONResponse(
+			array('data' =>
+				array('message' =>
+					(string) $this->l10n->t('Saved')
+				),
+				'status' => 'success',
+				'settings' => $this->settings->getAllValues()
+			)
+		);
 	}
 }

js/settings.js

@@ -150,10 +150,12 @@ function av_mode_show_options(str){
 $(document).ready(function() {
 	$('#av_submit').on('click', function(event){
 		event.preventDefault();
+		OC.msg.startAction('#antivirus_save_msg', t('files_antivirus', 'Saving...'));
 		$.post(
 				OC.generateUrl('apps/files_antivirus/settings/save'),
 				$('#antivirus').serializeArray(),
-				function(r){	
+				function(data){
+					OC.msg.finishedAction('#antivirus_save_msg', data);
 				}
 		
 		);

lib/appconfig.php

@@ -56,7 +56,8 @@ class Appconfig {
 	public function getAllValues() {
 		$keys = array_keys($this->defaults);
 		$values = array_map(array($this, 'getAppValue'), $keys);
-		return array_combine($keys, $values);
+		$preparedKeys = array_map(array($this, 'camelCase'), $keys);
+		return array_combine($preparedKeys, $values);
 	}
 	
 	/**
@@ -110,6 +111,18 @@ class Appconfig {
 		}
 	}
 	
+	/**
+	 * Translates property_name into propertyName
+	 * @param string $property
+	 * @return string
+	 */
+	protected function camelCase($property){
+		$split = explode('_', $property);
+		$ucFirst = implode('', array_map('ucfirst', $split));
+		$camelCase = lcfirst($ucFirst);
+		return $camelCase;
+	}
+	
 	/**
 	 * Does all the someConfig to some_config magic
 	 * @param string $property

lib/backgroundscanner.php

@@ -8,18 +8,36 @@
 
 namespace OCA\Files_Antivirus;
 
-use OCA\Files_Antivirus\AppInfo\Application;
+use OCP\IUserManager;
+use OCP\IL10N;
 use \OCA\Files_Antivirus\Scanner;
 use OCA\Files_Antivirus\Item;
 
 class BackgroundScanner {
-	
+
-	private $rootFolder;
+	/**
+	 * @var Appconfig
+	 */
 	private $appConfig;
 	
-	public function __construct($rootFolder, $config){
+	/**
-		$this->rootFolder = $rootFolder;
+	 * @var IUserManager 
+	 */
+	private $userManager;
+	
+	/**
+	 * @var IL10N
+	 */
+	private $l10n;
+	
+	/**
+	 * A constructor
+	 * @param Appconfig $config
+	 */
+	public function __construct($config, IUserManager $userManager, IL10N $l10n){
 		$this->appConfig = $config;
+		$this->userManager = $userManager;
+		$this->l10n = $l10n;
 	}
 	
 	/**
@@ -54,8 +72,8 @@ class BackgroundScanner {
 		while ($row = $result->fetchRow()) {
 			$path = $view->getPath($row['fileid']);
 			if (!is_null($path)) {
-				$item = new Item($view, $path, $row['fileid']);
+				$item = new Item($this->l10n, $view, $path, $row['fileid']);
-				$scanner = new Scanner($this->appConfig);
+				$scanner = new Scanner($this->appConfig, $this->l10n);
 				$status = $scanner->scan($item);					
 				$status->dispatch($item, true);
 			}
@@ -64,12 +82,11 @@ class BackgroundScanner {
 	}
 	
 	/**
-	 * A hack to access files amd views. Better than before.
+	 * A hack to access files and views. Better than before.
 	 */
 	protected function initFS(){
-		//get a random user (needed to mount FS)
+		//Need any valid user to mount FS
-		$userManager = \OC_User::getManager();
+		$results = $this->userManager->search('', 2, 0);
-		$results = $userManager->search('', 2, 0);
 		$anyUser = array_pop($results);
 
 		\OC_Util::tearDownFS();

lib/db/item.php

@@ -11,8 +11,16 @@ namespace OCA\Files_Antivirus\Db;
 use OCP\AppFramework\Db\Entity;
 
 class Item extends Entity{
+	/**
+	 * fileid that was scanned
+	 * @var int
+	 */
 	protected $fileid;
 	
+	/**
+	 * Timestamp of the check
+	 * @var int
+	 */
 	protected $checkTime;
 	
 }

lib/hooks/filesystemhooks.php

@@ -10,8 +10,16 @@
 namespace OCA\Files_Antivirus\Hooks;
 
 class FilesystemHooks {
-
+	
+	/**
+	 * @var \OCA\Files_Antivirus\Appconfig
+	 */
 	private $appConfig;
+	
+	/**
+	 *
+	 * @var type 
+	 */
 	private $rootFolder;
 
 	public function __construct($rootFolder, $appConfig) {

lib/item.php

@@ -8,20 +8,60 @@
 
 namespace OCA\Files_Antivirus;
 
+use OCP\IL10N;
 use OCA\Files_Antivirus\Status;
 
 class Item {
+	/**
+	 * Scanned fileid (optional)
+	 * @var int
+	 */
 	protected $id;
+	
+	/**
+	 * File view
+	 * @var \OC\Files\View
+	 */
 	protected $view;
+	
+	/**
+	 * Path relative to the view
+	 * @var string
+	 */
 	protected $path;
+	
+	/**
+	 * Scan result
+	 * @var Status
+	 */
 	protected $status;
 	
+	/**
+	 * file handle, user to read from the file
+	 * @var resource
+	 */
 	protected $fileHandle;
+	
+	/**
+	 * Portion size
+	 * @var int
+	 */
 	protected $chunkSize;
 	
+	/**
+	 * Is filesize match the size conditions
+	 * @var bool
+	 */
 	protected $isValidSize;
 	
-	public function __construct($view, $path, $id = null) {
+	/**
+	 * @var IL10N
+	 */
+	private $l10n;
+	
+	public function __construct(IL10N $l10n, $view, $path, $id = null) {
+		$this->l10n = $l10n;
+		
 		if (!is_object($view)){
 			$this->logError('Can\'t init filesystem view.', $id, $path);
 			throw new \RuntimeException();
@@ -97,12 +137,10 @@ class Item {
 			//remove file
 			$this->view->unlink($this->path);
 			Notification::sendMail($this->path);
-			$message = \OCP\Util::getL10N('files_antivirus')
+			$message = $this->l10n->t(
-						->t(
+						"Virus detected! Can't upload the file %s", 
-								"Virus detected! Can't upload the file %s", 
+						array(basename($this->path))
-								array(basename($this->path))
+			);
-							)
-			;
 			\OCP\JSON::error(array("data" => array( "message" => $message)));
 			exit();
 		}

lib/scanner.php

@@ -23,20 +23,34 @@
 
 namespace OCA\Files_Antivirus;
 
+use OCP\IL10N;
 use OCA\Files_Antivirus\Item;
 
 class Scanner {
-	// null if not initialized
+
-	// false if an error occurred
+	/**
-	// Scanner subclass if initialized
+	 * A proper subclass
+	 * @var Scanner
+	 */
 	protected $instance = null;
 	
-	// Last scan status
+	/**
+	 * Scan result
+	 * @var \OCA\Files_Antivirus\Status
+	 */
 	protected $status;
 	
+	/**
+	 * @var \OCA\Files_Antivirus\Appconfig
+	 */
 	protected $appConfig;
 	
-	public function __construct($config){
+	/**
+	 * @var IL10N
+	 */
+	protected $l10n;
+	
+	public function __construct($config, IL10N $l10n){
 		$this->appConfig = $config;
 		try {
 			$avMode = $this->appConfig->getAvMode();
@@ -76,15 +90,16 @@ class Scanner {
 		}
 		
 		try {
-			$item = new Item($filesView, $path);
+			$application = new \OCA\Files_Antivirus\AppInfo\Application();
+			$appConfig = $application->getContainer()->query('Appconfig');
+			$l10n = $application->getContainer()->query('L10N');
+			
+			$item = new Item($l10n, $filesView, $path);
 			if (!$item->isValid()){
 				return;
 			}
 		
-			$application = new \OCA\Files_Antivirus\AppInfo\Application();
+			$scanner = new self($appConfig, $l10n);
-			$appConfig = $application->getContainer()->query('Appconfig');
-		
-			$scanner = new self($appConfig);
 			$fileStatus = $scanner->scan($item);
 			$fileStatus->dispatch($item);
 		} catch (\Exception $e){

templates/settings.php

@@ -7,22 +7,23 @@ script('files_antivirus', 'settings');
 		<fieldset class="personalblock">
 			<h2><?php p($l->t('Antivirus Configuration'));?></h2>
 			<p class='av_mode'><label for="av_mode"><?php p($l->t('Mode'));?></label>
-				<select id="av_mode" name="av_mode"><?php print_unescaped(html_select_options(array('executable' => $l->t('Executable'), 'daemon' => $l->t('Daemon'), 'socket' => $l->t('Daemon (Socket)')), $_['av_mode'])) ?></select>
+				<select id="av_mode" name="avMode"><?php print_unescaped(html_select_options(array('executable' => $l->t('Executable'), 'daemon' => $l->t('Daemon'), 'socket' => $l->t('Daemon (Socket)')), $_['avMode'])) ?></select>
 			</p>
-		    <p class='av_socket'><label for="av_socket"><?php p($l->t('Socket'));?></label><input type="text" id="av_socket" name="av_socket" value="<?php p($_['av_socket']); ?>" title="<?php p($l->t('Clamav Socket.')).' '.$l->t('Not required in Executable Mode.'); ?>"></p>
+		    <p class='av_socket'><label for="av_socket"><?php p($l->t('Socket'));?></label><input type="text" id="av_socket" name="avSocket" value="<?php p($_['avSocket']); ?>" title="<?php p($l->t('Clamav Socket.')).' '.$l->t('Not required in Executable Mode.'); ?>"></p>
-			<p class='av_host'><label for="av_host"><?php p($l->t('Host'));?></label><input type="text" id="av_host" name="av_host" value="<?php p($_['av_host']); ?>" title="<?php p($l->t('Address of Antivirus Host.')). ' ' .$l->t('Not required in Executable Mode.');?>"></p>
+			<p class='av_host'><label for="av_host"><?php p($l->t('Host'));?></label><input type="text" id="av_host" name="avHost" value="<?php p($_['avHost']); ?>" title="<?php p($l->t('Address of Antivirus Host.')). ' ' .$l->t('Not required in Executable Mode.');?>"></p>
-			<p class='av_port'><label for="av_port"><?php p($l->t('Port'));?></label><input type="text" id="av_port" name="av_port" value="<?php p($_['av_port']); ?>" title="<?php p($l->t('Port number of Antivirus Host.')). ' ' .$l->t('Not required in Executable Mode.');?>"></p>
+			<p class='av_port'><label for="av_port"><?php p($l->t('Port'));?></label><input type="text" id="av_port" name="avPort" value="<?php p($_['avPort']); ?>" title="<?php p($l->t('Port number of Antivirus Host.')). ' ' .$l->t('Not required in Executable Mode.');?>"></p>
-			<p class='av_chunk_size'><label for="av_chunk_size"><?php p($l->t('Stream Length'));?></label><input type="text" id="av_chunk_size" name="av_chunk_size" value="<?php p($_['av_chunk_size']); ?>" title="<?php p($l->t('ClamAV StreamMaxLength value in bytes.')). ' ' .$l->t('Not required in Executable Mode.');?>"> bytes</p>
+			<p class='av_chunk_size'><label for="av_chunk_size"><?php p($l->t('Stream Length'));?></label><input type="text" id="av_chunk_size" name="avChunkSize" value="<?php p($_['avChunkSize']); ?>" title="<?php p($l->t('ClamAV StreamMaxLength value in bytes.')). ' ' .$l->t('Not required in Executable Mode.');?>"> bytes</p>
 			<p class='av_path'>
-				<label for="av_path"><?php p($l->t('Path to clamscan'));?></label><input type="text" id="av_path" name="av_path" value="<?php p($_['av_path']); ?>" title="<?php p($l->t('Path to clamscan executable.')). ' ' .$l->t('Not required in Daemon Mode.');?>" />
+				<label for="av_path"><?php p($l->t('Path to clamscan'));?></label><input type="text" id="av_path" name="avPath" value="<?php p($_['avPath']); ?>" title="<?php p($l->t('Path to clamscan executable.')). ' ' .$l->t('Not required in Daemon Mode.');?>" />
 			</p>
 			<p class="av_path">
-				<label for="av_cmd_options"><?php p($l->t('Extra command line options (comma-separated)'));?></label><input type="text" id="av_cmd_options" name="av_cmd_options" value="<?php p($_['av_cmd_options']); ?>" />
+				<label for="av_cmd_options"><?php p($l->t('Extra command line options (comma-separated)'));?></label><input type="text" id="av_cmd_options" name="avCmdOptions" value="<?php p($_['avCmdOptions']); ?>" />
 			</p>
 			<p class="infected_action"><label for="av_infected_action"><?php p($l->t('Action for infected files found while scanning'));?></label>
-				<select id="av_infected_action" name="av_infected_action"><?php print_unescaped(html_select_options(array('only_log' => $l->t('Only log'), 'delete' => $l->t('Delete file')), $_['av_infected_action'])) ?></select>
+				<select id="av_infected_action" name="avInfectedAction"><?php print_unescaped(html_select_options(array('only_log' => $l->t('Only log'), 'delete' => $l->t('Delete file')), $_['avInfectedAction'])) ?></select>
 			</p>
 			<input id="av_submit" type="submit" value="<?php p($l->t('Save'));?>" />
+			<span id="antivirus_save_msg"></span>
 		</fieldset>
 	</form>
 	<hr />

tests/itemtest.php

@@ -39,7 +39,7 @@ class Test_Files_Antivirus_Item extends \OCA\Files_Antivirus\Tests\Testbase {
 	}
 	
 	public function testRead() {
-		$item = new Item(new \OC\Files\View(''), '/file1');
+		$item = new Item($this->l10n, new \OC\Files\View(''), '/file1');
 		$this->assertTrue($item->isValid());
 		
 		$chunk = $item->fread();

tests/scannertest.php

@@ -33,8 +33,8 @@ class Test_Files_Antivirus_ScannerTest extends \OCA\Files_Antivirus\Tests\Testba
 		$this->view->method('file_exists')->willReturn(true);
 		$this->view->method('filesize')->willReturn(42);
 		
-		$this->cleanItem = new Item($this->view, self::TEST_CLEAN_FILENAME, 42);
+		$this->cleanItem = new Item($this->l10n, $this->view, self::TEST_CLEAN_FILENAME, 42);
-		$this->infectedItem = new Item($this->view, self::TEST_INFECTED_FILENAME, 42);
+		$this->infectedItem = new Item($this->l10n, $this->view, self::TEST_INFECTED_FILENAME, 42);
 
 		$this->ruleMapper = new RuleMapper($this->db);
 		$this->ruleMapper->deleteAll();
@@ -61,7 +61,7 @@ class Test_Files_Antivirus_ScannerTest extends \OCA\Files_Antivirus\Tests\Testba
 		$this->view->method('fopen')->willReturn($handle);
 		$this->assertTrue($this->cleanItem->isValid());
 		
-		$scanner = new Scanner($this->config);
+		$scanner = new Scanner($this->config, $this->l10n);
 		
 		$scanner->scan($this->cleanItem);
 		$cleanStatus = $scanner->getStatus();
@@ -73,7 +73,7 @@ class Test_Files_Antivirus_ScannerTest extends \OCA\Files_Antivirus\Tests\Testba
 		$this->setExpectedException('RuntimeException');
 		
 		$fileView = new \OC\Files\View('');
-		$nonExistingItem = new Item($fileView, 'non-existing.file', 42);
+		$nonExistingItem = new Item($this->l10n, $fileView, 'non-existing.file', 42);
 		$scanner = new Scanner($this->config);
 		$scanner->scan($nonExistingItem);
 		$unknownStatus = $scanner->scan($nonExistingItem);
@@ -85,7 +85,7 @@ class Test_Files_Antivirus_ScannerTest extends \OCA\Files_Antivirus\Tests\Testba
 		$handle = fopen(__DIR__ . '/data/kitten.inf', 'r');
 		$this->view->method('fopen')->willReturn($handle);
 		$this->assertTrue($this->infectedItem->isValid());
-		$scanner = new Scanner($this->config);
+		$scanner = new Scanner($this->config, $this->l10n);
 		$scanner->scan($this->infectedItem);
 		$infectedStatus = $scanner->getStatus();
 		$this->assertInstanceOf('\OCA\Files_Antivirus\Status', $infectedStatus);

tests/testbase.php

@@ -13,6 +13,7 @@ abstract class Testbase extends \PHPUnit_Framework_TestCase {
 
 	protected $db;
 	protected $config;
+	protected $l10n;
 
 	public function setUp(){
 		parent::setUp();
@@ -23,6 +24,8 @@ abstract class Testbase extends \PHPUnit_Framework_TestCase {
 				->getMock()
 		;
 		
+		$this->l10n = \OCP\Util::getL10N('files_antivirus');
+		
 		$this->config->method('__call')
 			->will($this->returnCallback(array($this, 'getAppValue')));
 	}