Unify file/cron and mfile->valid_id

Create a testable function doing all the verification/removal, add
tests and use it for both cases.

Signed-off-by: Florian Pritz <bluewind@xinu.at>

4 files changed, 201 insertions, 49 deletions

diff --git a/application/controllers/file.php b/application/controllers/file.php
index e258f2ffb..0dd6eaffe 100644
--- a/application/controllers/file.php
+++ b/application/controllers/file.php
@@ -966,39 +966,34 @@ class File extends MY_Controller {
 			}
 		}
 
-		// 0 age disables age checks
-		if ($this->config->item('upload_max_age') == 0) return;
-
 		$oldest_time = (time() - $this->config->item('upload_max_age'));
 		$oldest_session_time = (time() - $this->config->item("sess_expiration"));
+		$config = array(
+			"upload_max_age" => $this->config->item("upload_max_age"),
+			"small_upload_size" => $this->config->item("small_upload_size"),
+			"sess_expiration" => $this->config->item("sess_expiration"),
+		);
+
+		$query = $this->db->select('hash, id, user, date')
+			->from('files')
+			->where("user", 0)
+			->where("date <", $oldest_session_time)
+			->get()->result_array();
+
+		foreach($query as $row) {
+			\service\files::valid_id($row, $config, $this->mfile, time());
+		}
 
-		$small_upload_size = $this->config->item('small_upload_size');
+		// 0 age disables age checks
+		if ($this->config->item('upload_max_age') == 0) return;
 
-		$query = $this->db->select('hash, id, user')
+		$query = $this->db->select('hash, id, user, date')
 			->from('files')
 			->where('date <', $oldest_time)
-			->or_where('('.$this->db->_protect_identifiers('user').' = 0 AND '
-			             .$this->db->_protect_identifiers('date')." < $oldest_session_time)")
 			->get()->result_array();
 
 		foreach($query as $row) {
-			$file = $this->mfile->file($row['hash']);
-			if (!file_exists($file)) {
-				$this->mfile->delete_id($row["id"]);
-				continue;
-			}
-
-			if ($row["user"] == 0 || filesize($file) > $small_upload_size) {
-				if (filemtime($file) < $oldest_time) {
-					unlink($file);
-					$this->mfile->delete_hash($row["hash"]);
-				} else {
-					$this->mfile->delete_id($row["id"]);
-					if ($this->mfile->stale_hash($row["hash"])) {
-						unlink($file);
-					}
-				}
-			}
+			\service\files::valid_id($row, $config, $this->mfile, time());
 		}
 	}
 
diff --git a/application/models/mfile.php b/application/models/mfile.php
index f0a594609..bf6d0f9dc 100644
--- a/application/models/mfile.php
+++ b/application/models/mfile.php
@@ -130,36 +130,29 @@ class Mfile extends CI_Model {
 		if (!$filedata) {
 			return false;
 		}
-		$file = $this->file($filedata['hash']);
 
-		if (!file_exists($file)) {
-			$this->delete_hash($filedata["hash"]);
-			return false;
-		}
-
-		// 0 age disables age checks
-		if ($this->config->item('upload_max_age') == 0) return true;
+		$config = array(
+			"upload_max_age" => $this->config->item("upload_max_age"),
+			"small_upload_size" => $this->config->item("small_upload_size"),
+			"sess_expiration" => $this->config->item("sess_expiration"),
+		);
 
-		// small files don't expire
-		if (filesize($file) <= $this->config->item("small_upload_size")) {
-			return true;
-		}
+		return \service\files::valid_id($filedata, $config, $this, time());
+	}
 
-		// files older than this should be removed
-		$remove_before = (time()-$this->config->item('upload_max_age'));
+	public function file_exists($file)
+	{
+		return file_exists($file);
+	}
 
-		if ($filedata["date"] < $remove_before) {
-			// if the file has been uploaded multiple times the mtime is the time
-			// of the last upload
-			if (filemtime($file) < $remove_before) {
-				$this->delete_hash($filedata["hash"]);
-			} else {
-				$this->delete_id($id);
-			}
-			return false;
-		}
+	public function filemtime($file)
+	{
+		return filemtime($file);
+	}
 
-		return true;
+	public function filesize($file)
+	{
+		return filesize($file);
 	}
 
 	public function get_timeout($id)
diff --git a/application/service/files.php b/application/service/files.php
index e12e636be..1097c5201 100644
--- a/application/service/files.php
+++ b/application/service/files.php
@@ -281,4 +281,54 @@ class files {
 			"url_id" => $url_id,
 		);
 	}
+
+	static public function valid_id(array $filedata, array $config, $model, $current_date)
+	{
+		assert(isset($filedata["hash"]));
+		assert(isset($filedata["id"]));
+		assert(isset($filedata["user"]));
+		assert(isset($filedata["date"]));
+		assert(isset($config["upload_max_age"]));
+		assert(isset($config["sess_expiration"]));
+		assert(isset($config["small_upload_size"]));
+
+		$file = $model->file($filedata['hash']);
+
+		if (!$model->file_exists($file)) {
+			$model->delete_hash($filedata["hash"]);
+			return false;
+		}
+
+		if ($filedata["user"] == 0) {
+			if ($filedata["date"] < $current_date - $config["sess_expiration"]) {
+				$model->delete_id($filedata["id"]);
+				return false;
+			}
+		}
+
+		// 0 age disables age checks
+		if ($config['upload_max_age'] == 0) return true;
+
+		// small files don't expire
+		if ($model->filesize($file) <= $config["small_upload_size"]) {
+			return true;
+		}
+
+		// files older than this should be removed
+		$remove_before = $current_date - $config["upload_max_age"];
+
+		if ($filedata["date"] < $remove_before) {
+			// if the file has been uploaded multiple times the mtime is the time
+			// of the last upload
+			$mtime = $model->filemtime($file);
+			if ($mtime < $remove_before) {
+				$model->delete_hash($filedata["hash"]);
+			} else {
+				$model->delete_id($filedata["id"]);
+			}
+			return false;
+		}
+
+		return true;
+	}
 }
diff --git a/application/tests/test_service_files_valid_id.php b/application/tests/test_service_files_valid_id.php
new file mode 100644
index 000000000..436624336
--- /dev/null
+++ b/application/tests/test_service_files_valid_id.php
@@ -0,0 +1,114 @@
+<?php
+/*
+ * Copyright 2015 Florian "Bluewind" Pritz <bluewind@server-speed.net>
+ *
+ * Licensed under AGPLv3
+ * (see COPYING for full license text)
+ *
+ */
+
+namespace tests;
+
+class test_service_files_valid_id extends Test {
+	private $model;
+	private $filedata;
+	private $config;
+
+	public function __construct()
+	{
+		parent::__construct();
+
+		$CI =& get_instance();
+		$CI->load->model("muser");
+		$CI->load->model("mfile");
+
+	}
+
+	public function init()
+	{
+		$this->model = \Mockery::mock("Mfile");
+		$this->model->shouldReceive("delete_id")->never()->byDefault();
+		$this->model->shouldReceive("delete_hash")->never()->byDefault();
+		$this->model->shouldReceive("file")->with("file-hash-1")->andReturn("/invalid/path/file-1")->byDefault();
+		$this->model->shouldReceive("filemtime")->with("/invalid/path/file-1")->andReturn(500)->byDefault();
+		$this->model->shouldReceive("filesize")->with("/invalid/path/file-1")->andReturn(50*1024)->byDefault();
+		$this->model->shouldReceive("file_exists")->with("/invalid/path/file-1")->andReturn(true)->byDefault();
+
+		$this->filedata = array(
+			"hash" => "file-hash-1",
+			"id" => "file-id-1",
+			"user" => 2,
+			"date" => 500,
+		);
+
+		$this->config = array(
+			"upload_max_age" => 20,
+			"sess_expiration" => 10,
+			"small_upload_size" => 10*1024,
+		);
+	}
+
+	public function cleanup()
+	{
+		\Mockery::close();
+	}
+
+	public function test_valid_id_keepNormalUpload()
+	{
+		$ret = \service\files::valid_id($this->filedata, $this->config, $this->model, 505);
+		$this->t->is($ret, true, "normal case should be valid");
+	}
+
+	public function test_valid_id_keepSmallUpload()
+	{
+		$this->model->shouldReceive("filesize")->with("/invalid/path/file-1")->once()->andReturn(50);
+
+		$ret = \service\files::valid_id($this->filedata, $this->config, $this->model, 550);
+		$this->t->is($ret, true, "file is old, but small and should be kept");
+	}
+
+	public function test_valid_id_removeOldFile()
+	{
+		$this->model->shouldReceive("delete_hash")->with("file-hash-1")->once();
+
+		$ret = \service\files::valid_id($this->filedata, $this->config, $this->model, 550);
+		$this->t->is($ret, false, "file is old and should be removed");
+	}
+
+	public function test_valid_id_removeOldUpload()
+	{
+		$this->model->shouldReceive("delete_id")->with("file-id-1")->once();
+		$this->model->shouldReceive("filemtime")->with("/invalid/path/file-1")->once()->andReturn(540);
+
+		$ret = \service\files::valid_id($this->filedata, $this->config, $this->model, 550);
+		$this->t->is($ret, false, "upload is old and should be removed");
+	}
+
+	public function test_valid_id_keepNormalUnownedFile()
+	{
+		$this->filedata["user"] = 0;
+
+		$ret = \service\files::valid_id($this->filedata, $this->config, $this->model, 505);
+		$this->t->is($ret, true, "upload is unowned and should be kept");
+	}
+
+	public function test_valid_id_removeOldUnownedFile()
+	{
+		$this->model->shouldReceive("delete_id")->with("file-id-1")->once();
+		$this->filedata["user"] = 0;
+
+		$ret = \service\files::valid_id($this->filedata, $this->config, $this->model, 515);
+		$this->t->is($ret, false, "upload is old, unowned and should be removed");
+	}
+
+	public function test_valid_id_removeMissingFile()
+	{
+		$this->model->shouldReceive("file_exists")->with("/invalid/path/file-1")->once()->andReturn(false);
+		$this->model->shouldReceive("delete_hash")->with("file-hash-1")->once();
+
+		$ret = \service\files::valid_id($this->filedata, $this->config, $this->model, 505);
+		$this->t->is($ret, false, "missing file should be removed");
+	}
+
+}
+