More photo handling cleanups

js/contacts.js

@@ -793,9 +793,9 @@ OC.Contacts = OC.Contacts || {};
 		$.when(this.storage.addContact(this.metadata.backend, this.metadata.parent))
 			.then(function(response) {
 			if(!response.error) {
-				self.id = String(response.data.metadata.id);
-				self.metadata = response.data.metadata;
-				self.data = response.data.data;
+				self.id = String(response.metadata.id);
+				self.metadata = response.metadata;
+				self.data = response.data;
 				console.log('Contact.add, groupprops', self.groupprops);
 				if(self.groupprops && self.groupprops.groups.length > 0) {
 					self._buildGroupSelect(self.groupprops.groups);

js/storage.js

@@ -32,6 +32,10 @@ OC.Contacts = OC.Contacts || {};
 				}
 			} else {
 				this.data = response.data || response;
+				// Kind of a hack
+				if (response.metadata) {
+					this.metadata = response.metadata;
+				}
 			}
 		}
 	};

lib/contact.php

@@ -400,6 +400,45 @@ class Contact extends VObject\VCard implements IPIMObject {
 		}
 	}
 
+	/**
+	 * Set the contact photo.
+	 *
+	 * @param \OCP\Image $photo
+	 */
+	public function setPhoto(\OCP\Image $photo) {
+		// For vCard 3.0 the type must be e.g. JPEG or PNG
+		// For version 4.0 the full mimetype should be used.
+		// https://tools.ietf.org/html/rfc2426#section-3.1.4
+		if(strval($this->VERSION) === '4.0') {
+			$type = $photo->mimeType();
+		} else {
+			$type = explode('/', $photo->mimeType());
+			$type = strtoupper(array_pop($type));
+		}
+		if(isset($this->PHOTO)) {
+			$property = $this->PHOTO;
+			if(!$property) {
+				return false;
+			}
+			$property->setValue(strval($photo));
+			$property->parameters = array();
+			$property->parameters[]
+				= new \Sabre\VObject\Parameter('ENCODING', 'b');
+			$property->parameters[]
+				= new \Sabre\VObject\Parameter('TYPE', $photo->mimeType());
+			$this->PHOTO = $property;
+		} else {
+			$this->add('PHOTO',
+				strval($photo), array('ENCODING' => 'b',
+				'TYPE' => $type));
+			// TODO: Fix this hack
+			$this->setSaved(false);
+		}
+
+		return true;
+
+	}
+
 	/**
 	* Get a property index in the contact by the checksum of its serialized value
 	*

lib/controller/contactphotocontroller.php

@@ -33,23 +33,14 @@ class ContactPhotoController extends Controller {
 		$addressBook = $this->app->getAddressBook($params['backend'], $params['addressBookId']);
 		$contact = $addressBook->getChild($params['contactId']);
 
-		if(!$contact) {
-			$response = new JSONResponse();
-			$response->bailOut(App::$l10n->t('Couldn\'t find contact.'));
-			return $response;
-		}
+		$tempPhoto = TemporaryPhoto::create(
+			$this->server,
+			TemporaryPhoto::PHOTO_CURRENT,
+			$contact
+		);
+
+		$image = $tempPhoto->getPhoto();
 
-		$image = new \OCP\Image();
-		if (isset($contact->PHOTO) && $image->loadFromBase64((string)$contact->PHOTO)) {
-			// OK
-			$etag = md5($contact->PHOTO);
-		}
-		else
-		// Logo :-/
-		if(isset($contact->LOGO) && $image->loadFromBase64((string)$contact->LOGO)) {
-			// OK
-			$etag = md5($contact->LOGO);
-		}
 		if($image->valid()) {
 			$response = new ImageResponse($image);
 			$lastModified = $contact->lastModified();
@@ -65,9 +56,7 @@ class ContactPhotoController extends Controller {
 			}
 			return $response;
 		} else {
-			$response = new JSONResponse();
-			$response->bailOut(App::$l10n->t('Error getting user photo'));
-			return $response;
+			throw new \Exception(App::$l10n->t('Error getting user photo'));
 		}
 	}
 
@@ -81,7 +70,7 @@ class ContactPhotoController extends Controller {
 		$params = $this->request->urlParams;
 		$response = new JSONResponse();
 
-		$tempPhoto = TemporaryPhoto::get(
+		$tempPhoto = TemporaryPhoto::create(
 			$this->server,
 			TemporaryPhoto::PHOTO_UPLOADED,
 			$this->request
@@ -111,7 +100,7 @@ class ContactPhotoController extends Controller {
 		$addressBook = $this->app->getAddressBook($params['backend'], $params['addressBookId']);
 		$contact = $addressBook->getChild($params['contactId']);
 
-		$tempPhoto = TemporaryPhoto::get(
+		$tempPhoto = TemporaryPhoto::create(
 			$this->server,
 			TemporaryPhoto::PHOTO_CURRENT,
 			$contact
@@ -142,7 +131,7 @@ class ContactPhotoController extends Controller {
 			$response->bailOut(App::$l10n->t('No photo path was submitted.'));
 		}
 
-		$tempPhoto = TemporaryPhoto::get(
+		$tempPhoto = TemporaryPhoto::create(
 			$this->server,
 			TemporaryPhoto::PHOTO_FILESYSTEM,
 			$this->request->get['path']
@@ -167,8 +156,9 @@ class ContactPhotoController extends Controller {
 		$params = $this->request->urlParams;
 		$tmpkey = $params['key'];
 
-		$image = new \OCP\Image();
-		$image->loadFromData($this->server->getCache()->get($tmpkey));
+		$tmpPhoto = new TemporaryPhoto($this->server, $tmpkey);
+		$image = $tmpPhoto->getPhoto();
+
 		if($image->valid()) {
 			$response = new ImageResponse($image);
 			return $response;
@@ -197,19 +187,10 @@ class ContactPhotoController extends Controller {
 
 		$response = new JSONResponse();
 
-		if(!$contact) {
-			return $response->bailOut(App::$l10n->t('Couldn\'t find contact.'));
-		}
-
-		$data = $this->server->getCache()->get($tmpkey);
-		if(!$data) {
-			return $response->bailOut(App::$l10n->t('Image has been removed from cache'));
-		}
-
-		$image = new \OCP\Image();
-
-		if(!$image->loadFromData($data)) {
-			return $response->bailOut(App::$l10n->t('Error creating temporary image'));
+		$tmpPhoto = new TemporaryPhoto($this->server, $tmpkey);
+		$image = $tmpPhoto->getPhoto();
+		if(!$image || !$image->valid()) {
+			return $response->bailOut(App::$l10n->t('Error loading image from cache'));
 		}
 
 		$w = ($w !== -1 ? $w : $image->width());
@@ -219,36 +200,11 @@ class ContactPhotoController extends Controller {
 			return $response->bailOut(App::$l10n->t('Error cropping image'));
 		}
 
-		// For vCard 3.0 the type must be e.g. JPEG or PNG
-		// For version 4.0 the full mimetype should be used.
-		// https://tools.ietf.org/html/rfc2426#section-3.1.4
-		if(strval($contact->VERSION) === '4.0') {
-			$type = $image->mimeType();
-		} else {
-			$type = explode('/', $image->mimeType());
-			$type = strtoupper(array_pop($type));
-		}
-		if(isset($contact->PHOTO)) {
-			$property = $contact->PHOTO;
-			if(!$property) {
-				$this->server->getCache()->remove($tmpkey);
-				return $response->bailOut(App::$l10n
-					->t('Error getting PHOTO property.'));
-			}
-			$property->setValue(strval($image));
-			$property->parameters = array();
-			$property->parameters[]
-				= new \Sabre\VObject\Parameter('ENCODING', 'b');
-			$property->parameters[]
-				= new \Sabre\VObject\Parameter('TYPE', $image->mimeType());
-			$contact->PHOTO = $property;
-		} else {
-			$contact->add('PHOTO',
-				strval($image), array('ENCODING' => 'b',
-				'TYPE' => $type));
-			// TODO: Fix this hack
-			$contact->setSaved(false);
+		if (!$contact->setPhoto($image)) {
+			$tmpPhoto->remove($tmpkey);
+			return $response->bailOut(App::$l10n->t('Error getting PHOTO property.'));
 		}
+
 		if(!$contact->save()) {
 			return $response->bailOut(App::$l10n->t('Error saving contact.'));
 		}
@@ -268,7 +224,7 @@ class ContactPhotoController extends Controller {
 			)
 		));
 
-		$this->server->getCache()->remove($tmpkey);
+		$tmpPhoto->remove($tmpkey);
 
 		return $response;
 	}

lib/utils/temporaryphoto.php

@@ -25,9 +25,9 @@ namespace OCA\Contacts\Utils;
 use OCP\Image;
 
 /**
- * This class is used for getting a contact photo for cropping.
+ * This class is used for getting a temporary contact photo for cropping.
  */
-abstract class TemporaryPhoto {
+class TemporaryPhoto {
 
 	const MAX_SIZE = 400;
 
@@ -82,33 +82,48 @@ abstract class TemporaryPhoto {
 	* Always call parents ctor:
 	*   		parent::__construct($server);
 	*/
-	public function __construct(\OCP\IServerContainer $server) {
+	public function __construct(\OCP\IServerContainer $server, $key = null) {
 		$this->server = $server;
+		$this->key = $key;
+		if (!is_null($key)) {
+			$this->processImage();
+		}
 	}
 
 	/**
 	* Returns an instance of a subclass of this class
 	*
 	* @param \OCP\IServerContainer $server
-	* @param int $type One of the pre-defined types.
-	* @param mixed $data Whatever data is needed to load the photo.
+	* @param int|null $type One of the pre-defined types.
+	* @param mixed|null $data Whatever data is needed to load the photo.
 	*/
-	public static function get(\OCP\IServerContainer $server, $type, $data) {
+	public static function create(\OCP\IServerContainer $server, $type = null, $data = null) {
 		if (isset(self::$classMap[$type])) {
 			return new self::$classMap[$type]($server, $data);
 		} else {
-			// TODO: Return a "null object"
-			return new self($data);
+			return new self($data, $data);
 		}
 	}
 
+	/**
+	* Remove a cached image by key.
+	*
+	* @param string $key
+	*/
+	public function remove($key) {
+		return $this->server->getCache()->remove($key);
+	}
+
 	/**
 	* Do what's needed to get the image from storage
 	* depending on the type.
 	* After this method is called $this->image must hold an
 	* instance of \OCP\Image.
 	*/
-	protected abstract function processImage();
+	protected function processImage() {
+		$this->image = new \OCP\Image();
+		$this->image->loadFromData($this->server->getCache()->get($this->key));
+	}
 
 	/**
 	* Whether this image is valied