Improved pending request permissions update

PHP doc improvements and additions
Changes on the utility class

controller/sharecontroller.php

@@ -19,6 +19,8 @@ use OCA\Passman\Service\CredentialService;
 use OCA\Passman\Service\FileService;
 use OCA\Passman\Service\NotificationService;
 use OCA\Passman\Service\ShareService;
+use OCA\Passman\Utility\NotFoundJSONResponse;
+use OCA\Passman\Utility\Utils;
 use OCP\AppFramework\Db\DoesNotExistException;
 use OCP\AppFramework\Http\NotFoundResponse;
 use OCP\IRequest;
@@ -370,24 +372,26 @@ class ShareController extends ApiController {
 
 		//@TODO Check expire date
 		$acl = $this->shareService->getACL(null, $credential_guid);
-		$views = $acl->getExpireViews();
+
+        if ($acl->getExpire() > 0 && Utils::getTime() > $acl->getExpire()) {
+			return new NotFoundJSONResponse();
+		}
+
+        $views = $acl->getExpireViews();
 		if ($views === 0) {
-			return new NotFoundResponse();
+			return new NotFoundJSONResponse();
 		} else if ($views != -1) {
 			$views--;
 			$acl->setExpireViews($views);
 			$this->shareService->updateCredentialACL($acl);
 		}
 
-		if ($acl->getExpire() > 0 && time() > $acl->getExpire()) {
-			return new NotFoundResponse();
-		}
 
 		try {
 			$credential = $this->shareService->getSharedItem(null, $credential_guid);
 			return new JSONResponse($credential);
 		} catch (DoesNotExistException $ex) {
-			return new NotFoundResponse();
+			return new NotFoundJSONResponse();
 		}
 	}
 
@@ -424,12 +428,12 @@ class ShareController extends ApiController {
 		try {
 			$credential = $this->credentialService->getCredentialByGUID($item_guid);
 		} catch (DoesNotExistException $e){
-			return new JSONResponse(array());
+			return new NotFoundJSONResponse();
 		}
 
 		$acl = $this->shareService->getACL($this->userId->getUID(), $credential->getGuid());
 		if (!$acl->hasPermission(SharingACL::FILES)){
-			return new NotFoundResponse();
+			return new NotFoundJSONResponse();
 		} else {
 			return $this->fileService->getFileByGuid($file_guid);
 		}
@@ -446,7 +450,7 @@ class ShareController extends ApiController {
 		try {
 			$credential = $this->credentialService->getCredentialByGUID($item_guid);
 		} catch (DoesNotExistException $exception) {
-			return new NotFoundResponse();
+			return new NotFoundJSONResponse();
 		}
 		if ($this->userId->getUID() == $credential->getUserId()) {
 			$acl = null;
@@ -459,11 +463,7 @@ class ShareController extends ApiController {
 			}
 
 			if ($acl === null) {
-				$sr = $this->shareService->getPendingShareRequestsForCredential($item_guid, $user_id);
-				foreach ($sr as $shareRequest) {
-					$shareRequest->setPermissions($permission);
-					$this->shareService->updateCredentialShareRequest($shareRequest);
-				}
+				$this->shareService->updatePendingShareRequestsForCredential($item_guid, $user_id, $permission);
 			}
 
 		}

lib/Db/ShareRequestMapper.php

@@ -48,6 +48,12 @@ class ShareRequestMapper extends Mapper {
         return $this->findEntities($q, [$item_guid]);
     }
 
+    /**
+     * Deletes all pending requests for the given user to the given item
+     * @param $item_id          The item ID
+     * @param $target_user_id   The target user
+     * @return \PDOStatement    The result of running the db query
+     */
     public function cleanItemRequestsForUser($item_id, $target_user_id){
 		$q = "DELETE FROM *PREFIX*" . self::TABLE_NAME . " WHERE item_id = ? AND target_user_id = ?";
 		$this->execute($q, [$item_id, $target_user_id]);
@@ -64,27 +70,65 @@ class ShareRequestMapper extends Mapper {
         return $this->findEntities($q, [$user_id]);
     }
 
+    /**
+     * Deletes the given share request
+     * @param ShareRequest $shareRequest    Request to delete
+     * @return ShareRequest                 The deleted request
+     */
     public function deleteShareRequest(ShareRequest $shareRequest){
-    	$this->delete($shareRequest);
+    	return $this->delete($shareRequest);
 	}
 
+    /**
+     * Gets a share request by it's unique incremental id
+     * @param $id
+     * @return ShareRequest
+     */
 	public function getShareRequestById($id){
 		$q = "SELECT * FROM *PREFIX*" . self::TABLE_NAME . " WHERE id = ?";
 		return $this->findEntity($q, [$id]);
 	}
 
-	public function getShareRequestsByGuid($item_guid){
+    /**
+     * Gets all share requests by a given item GUID
+     * @param $item_guid
+     * @return ShareRequest[]
+     */
+	public function getShareRequestsByItemGuid($item_guid){
 		$q = "SELECT * FROM *PREFIX*" . self::TABLE_NAME . " WHERE 	item_guid = ?";
 		return $this->findEntities($q, [$item_guid]);
 	}
 
+    /**
+     * Updates the given share request,
+     * @param ShareRequest $shareRequest
+     * @return ShareRequest
+     */
 	public function updateShareRequest(ShareRequest $shareRequest){
 		return $this->update($shareRequest);
 	}
 
+    /**
+     * Finds pending requests sent to the given user to the given item.
+     * @param $item_guid
+     * @param $user_id
+     * @return ShareRequest[]
+     */
 	public function getPendingShareRequests($item_guid, $user_id){
 		$q = "SELECT * FROM *PREFIX*" . self::TABLE_NAME . " WHERE 	item_guid = ? and target_user_id= ?";
 		return $this->findEntities($q, [$item_guid, $user_id]);
 	}
 
+    /**
+     * Updates all pending requests with the given permissions
+     * @param $item_guid        The item for which to update the requests
+     * @param $user_id          The user for which to update the requests
+     * @param $permissions      The new permissions to apply
+     * @return \PDOStatement    The result of the operation
+     */
+	public function updatePendinRequestPermissions($item_guid, $user_id, $permissions){
+	    $q = "UPDATE *PREFIX*" . self::TABLE_NAME . " SET permissions = ? WHERE item_guid = ? AND target_user_id = ?";
+        return $this->execute($q, [$permissions, $item_guid, $user_id]);
+    }
+
 }

lib/Service/ShareService.php

@@ -16,6 +16,7 @@ use OCA\Passman\Db\ShareRequest;
 use OCA\Passman\Db\ShareRequestMapper;
 use OCA\Passman\Db\SharingACL;
 use OCA\Passman\Db\SharingACLMapper;
+use OCA\Passman\Utility\Utils;
 use OCP\AppFramework\Db\DoesNotExistException;
 
 class ShareService {
@@ -51,7 +52,7 @@ class ShareService {
 	 * @return array                        Array of sharing requests
 	 */
 	public function createBulkRequests($target_item_id, $target_item_guid, $request_array, $permissions, $credential_owner) {
-		$created = (new \DateTime())->getTimestamp();
+		$created = Utils::getTime();
 		$requests = array();
 		foreach ($request_array as $req) {
 			$t = new ShareRequest();
@@ -175,27 +176,31 @@ class ShareService {
 	}
 
 
-	/**
-	 * Deletes an share reuqest by id
-	 *
-	 * @param ShareRequest $request
-	 * @return \OCA\Passman\Db\ShareRequest[]
-	 */
+    /**
+     * Deletes a share request by the item ID
+     * @param ShareRequest $request
+     * @return \PDOStatement
+     */
 	public function cleanItemRequestsForUser(ShareRequest $request) {
 		return $this->shareRequest->cleanItemRequestsForUser($request->getItemId(), $request->getTargetUserId());
 	}
 
-	/**
-	 * Get an share request by id
-	 */
+    /**
+     * Get an share request by id
+     * @param $id
+     * @return ShareRequest
+     */
 	public function getShareRequestById($id) {
 		return $this->shareRequest->getShareRequestById($id);
 	}
 
-	/*
-	 * Get an share request by $item_guid and $target_vault_guid
-	 *
-	 */
+    /**
+     * Get an share request by $item_guid and $target_vault_guid
+     *
+     * @param $item_guid
+     * @param $target_vault_guid
+     * @return ShareRequest
+     */
 	public function getRequestByGuid($item_guid, $target_vault_guid) {
 		return $this->shareRequest->getRequestByItemAndVaultGuid($item_guid, $target_vault_guid);
 	}
@@ -232,14 +237,14 @@ class ShareService {
 	 * @return \OCA\Passman\Db\ShareRequest[]
 	 */
 	public function getShareRequestsByGuid($item_guid) {
-		return $this->shareRequest->getShareRequestsByGuid($item_guid);
+		return $this->shareRequest->getShareRequestsByItemGuid($item_guid);
 	}
 
 	/**
 	 * Get pending share requests by guid
 	 *
 	 * @param  ShareRequest $request
-	 * @return \OCA\Passman\Db\ShareRequest[]
+	 * @return ShareRequest
 	 */
 	public function deleteShareRequest(ShareRequest $request) {
 		return $this->shareRequest->deleteShareRequest($request);
@@ -255,7 +260,11 @@ class ShareService {
 		return $this->sharingACL->deleteShareACL($ACL);
 	}
 
-
+    /**
+     * Updates the given ACL entry
+     * @param SharingACL $sharingACL
+     * @return SharingACL
+     */
 	public function updateCredentialACL(SharingACL $sharingACL) {
 		return $this->sharingACL->updateCredentialACL($sharingACL);
 	}
@@ -274,4 +283,9 @@ class ShareService {
 	public function getPendingShareRequestsForCredential($item_guid, $user_id) {
 		return $this->shareRequest->getPendingShareRequests($item_guid, $user_id);
 	}
+
+
+	public function updatePendingShareRequestsForCredential($item_guid, $user_id, $permissions){
+	    return $this->shareRequest->updatePendinRequestPermissions($item_guid, $user_id, $permissions);
+    }
 }

lib/Utility/NotFoundJSONResponse.php

@@ -0,0 +1,24 @@
+<?php
+/**
+ * Created by PhpStorm.
+ * User: wolfi
+ * Date: 5/10/16
+ * Time: 17:25
+ */
+
+namespace OCA\Passman\Utility;
+
+
+use OCP\AppFramework\Http;
+use OCP\AppFramework\Http\JSONResponse;
+
+class NotFoundJSONResponse extends JSONResponse {
+
+    /**
+     * Creates a new json response with a not found status code.
+     * @param array $response_data
+     */
+    public function __construct($response_data = []) {
+        parent::__construct($response_data, Http::STATUS_NOT_FOUND);
+    }
+}

lib/Utility/Utils.php

@@ -10,19 +10,28 @@
  */
 
 namespace OCA\Passman\Utility;
+
 class Utils {
-	public function getTime() {
-		return time();
+    /**
+     * Gets the unix epoch UTC timestamp
+     * @return int
+     */
+	public static function getTime() {
+		return (new \DateTime())->getTimestamp();
 	}
 	/**
-	 * @return int the current unix time in miliseconds
+	 * @return int the current unix time in milliseconds
 	 */
-	public function getMicroTime() {
+	public static function getMicroTime() {
 		list($millisecs, $secs) = explode(" ", microtime());
 		return $secs . substr($millisecs, 2, 6);
 	}
 
-	public function GUID() {
+    /**
+     * Generates a Globally Unique ID
+     * @return string
+     */
+	public static function GUID() {
 		if (function_exists('com_create_guid') === true)
 		{
 			return trim(com_create_guid(), '{}');