From 16218a13b27916fc806f5cd24112cd2e8b921db3 Mon Sep 17 00:00:00 2001
From: Anton Stroganov <stroganov.a@gmail.com>
Date: Sat, 6 Sep 2014 01:04:48 -0400
Subject: [PATCH] enhancements to job status tracking

 - add helper methods to support fetching started/updated timestamps
 - do not discard `started` timestamp on update
 - simplify job id mapping
 - sanity check `update()` to accept valid statuses only
---
 lib/Resque/Job/Status.php | 93 ++++++++++++++++++++++++++++++++-------
 1 file changed, 77 insertions(+), 16 deletions(-)

diff --git a/lib/Resque/Job/Status.php b/lib/Resque/Job/Status.php
index 00fc40c5..10cedc17 100644
--- a/lib/Resque/Job/Status.php
+++ b/lib/Resque/Job/Status.php
@@ -39,7 +39,7 @@ class Resque_Job_Status
 	 */
 	public function __construct($id)
 	{
-		$this->id = $id;
+		$this->id = self::generateId($id);
 	}
 
 	/**
@@ -53,9 +53,9 @@ public static function create($id)
 		$statusPacket = array(
 			'status' => self::STATUS_WAITING,
 			'updated' => time(),
-			'started' => time(),
+			'started' => time()
 		);
-		Resque::redis()->set('job:' . $id . ':status', json_encode($statusPacket));
+		Resque::redis()->set(self::generateId($id), json_encode($statusPacket));
 	}
 
 	/**
@@ -70,7 +70,7 @@ public function isTracking()
 			return false;
 		}
 
-		if(!Resque::redis()->exists((string)$this)) {
+		if(!Resque::redis()->exists($this->id)) {
 			$this->isTracking = false;
 			return false;
 		}
@@ -86,19 +86,26 @@ public function isTracking()
 	 */
 	public function update($status)
 	{
+		$status = (int)$status;
+
 		if(!$this->isTracking()) {
 			return;
 		}
 
+		if($status < 1 || $status > 4) {
+			return;
+		}
+
 		$statusPacket = array(
 			'status' => $status,
 			'updated' => time(),
+			'started' => $this->fetch('started')
 		);
-		Resque::redis()->set((string)$this, json_encode($statusPacket));
+		Resque::redis()->set($this->id, json_encode($statusPacket));
 
 		// Expire the status for completed jobs after 24 hours
 		if(in_array($status, self::$completeStatuses)) {
-			Resque::redis()->expire((string)$this, 86400);
+			Resque::redis()->expire($this->id, 86400);
 		}
 	}
 
@@ -110,16 +117,38 @@ public function update($status)
 	 */
 	public function get()
 	{
-		if(!$this->isTracking()) {
-			return false;
-		}
+		return $this->status();
+	}
 
-		$statusPacket = json_decode(Resque::redis()->get((string)$this), true);
-		if(!$statusPacket) {
-			return false;
-		}
+	/**
+	 * Fetch the status for the job being monitored.
+	 *
+	 * @return mixed False if the status is not being monitored, otherwise the status as
+	 * 	as an integer, based on the Resque_Job_Status constants.
+	 */
+	public function status()
+	{
+		return $this->fetch('status');
+	}
+
+	/**
+	 * Fetch the updated timestamp for the job being monitored.
+	 *
+	 * @return mixed False if the status is not being monitored, otherwise the updated timestamp
+	 */
+	public function updated()
+	{
+		return $this->fetch('updated');
+	}
 
-		return $statusPacket['status'];
+	/**
+	 * Fetch the started timestamp for the job being monitored.
+	 *
+	 * @return mixed False if the status is not being monitored, otherwise the created timestamp
+	 */
+	public function started()
+	{
+		return $this->fetch('started');
 	}
 
 	/**
@@ -127,7 +156,7 @@ public function get()
 	 */
 	public function stop()
 	{
-		Resque::redis()->del((string)$this);
+		Resque::redis()->del($this->id);
 	}
 
 	/**
@@ -137,6 +166,38 @@ public function stop()
 	 */
 	public function __toString()
 	{
-		return 'job:' . $this->id . ':status';
+		return $this->id;
+	}
+
+	/**
+	 * generate job status id key in a consistent manner
+     *
+	 * @return string String redis key for the current job status
+	 */
+	protected static function generateId($id)
+	{
+		return 'job:' . $id . ':status';
+	}
+
+	/**
+	 * Fetch the status packet for the job being monitored.
+	 * @param optional string $field The field to get from the status packet
+	 *
+	 * @return mixed False if the status is not being monitored, otherwise the status packet array or the individual field
+	 */
+	protected function fetch($field = false)
+	{
+		$statusPacket = Resque::redis()->get($this->id);
+		if($statusPacket) {
+			$statusPacket = json_decode($statusPacket, true);
+			if($field) {
+				if(isset($statusPacket[$field])) {
+					return (int)$statusPacket[$field];
+				}
+			} else {
+				return $statusPacket;
+			}
+		}
+		return false;
 	}
 }