From 1afddaa98ef8361ec21ce6cf216d0f8fec528a0a Mon Sep 17 00:00:00 2001 From: Thomas Tanghus Date: Mon, 10 Mar 2014 15:14:36 +0100 Subject: [PATCH] Finally actually implement the interfaces I claim to. \Iterator, \Countable, \ArrayAccess Still need some checks to assure validity after unset() --- lib/abstractpimcollection.php | 70 +++++++++++++++++------------- lib/addressbook.php | 20 +++++++-- tests/lib/addressbook_test.php | 52 ++++++++++++++++++++++ tests/lib/backend/backend_test.php | 1 + tests/lib/backend/mock.php | 3 ++ 5 files changed, 113 insertions(+), 33 deletions(-) diff --git a/lib/abstractpimcollection.php b/lib/abstractpimcollection.php index a6e8a9a6..2dc09015 100644 --- a/lib/abstractpimcollection.php +++ b/lib/abstractpimcollection.php @@ -31,11 +31,10 @@ abstract class AbstractPIMCollection extends AbstractPIMObject implements \Itera protected $objects = array(); - protected $counter = 0; + protected $current; /** * Returns a specific child node, referenced by its id - * TODO: Maybe implement it here? * * @param string $id * @return IPIMObject @@ -77,20 +76,26 @@ abstract class AbstractPIMCollection extends AbstractPIMObject implements \Itera // Iterator methods + // NOTE: This method is reliant on sub class implementing count() public function rewind() { - $this->counter = 0; + // Assure any internal counter's initialized + self::count(); + if (count($this->objects) === 0) { + $this->getChildren(); + } + $this->current = reset($this->objects); } public function next() { - $this->counter++; + $this->current = next($this->objects); } public function valid() { - return array_key_exists($this->counter, $this->objects); + return $this->current ? array_key_exists($this->current->getId(), $this->objects) : false; } public function current() { - return $this->objects[$this->counter]; + return $this->objects[$this->current->getId()]; } /** Implementations can choose to return the current objects ID/UUID @@ -98,7 +103,7 @@ abstract class AbstractPIMCollection extends AbstractPIMObject implements \Itera * foreach($collection as $id => $object) {} */ public function key() { - return $this->counter; + return $this->current->getId(); } // Countable method. @@ -106,9 +111,7 @@ abstract class AbstractPIMCollection extends AbstractPIMObject implements \Itera /** * For implementations using a backend where fetching all object at once * would give too much overhead, they can maintain an internal count value - * and fetch objects progressively. Simply watch the diffence between - * $this->counter, the value of count($this->objects) and the internal - * value, and fetch more objects when needed. + * and fetch objects progressively. */ public function count() { return count($this->objects); @@ -118,42 +121,49 @@ abstract class AbstractPIMCollection extends AbstractPIMObject implements \Itera public function offsetSet($offset, $value) { if (is_null($offset)) { - $this->objects[] = $value; + throw new \RunTimeException('You cannot add objects using array access'); } else { + // TODO: Check if offset is set and update element. $this->objects[$offset] = $value; } } public function offsetExists($offset) { - return isset($this->objects[$offset]); + if (isset($this->objects[$offset])) { + return true; + } + + try { + $child = $this->getChild($offset); + } catch (\Exception $e) { + return false; + } + + if ($child) { + $this->objects[$offset] = $child; + return true; + } + + return false; } public function offsetUnset($offset) { + $this->deleteChild($offset); unset($this->objects[$offset]); } public function offsetGet($offset) { - return isset($this->objects[$offset]) ? $this->objects[$offset] : null; - } + if (isset($this->objects[$offset])) { + return $this->objects[$offset]; + } - // Magic property accessors - // NOTE: They should go in the implementations(?) - /* - public function __set($id, $value) { - $this->objects[$id] = $value; - } + $child = $this->getChild($offset); - public function __get($id) { - return $this->objects[$id]; - } + if ($child) { + $this->objects[$offset] = $child; + return $child; + } - public function __isset($id) { - return isset($this->objects[$id]); } - public function __unset($id) { - unset($this->objects[$id]); - } - */ - } diff --git a/lib/addressbook.php b/lib/addressbook.php index 154580d9..4916ab5e 100644 --- a/lib/addressbook.php +++ b/lib/addressbook.php @@ -186,6 +186,9 @@ class Addressbook extends AbstractPIMCollection { * @return bool */ public function childExists($id) { + if(isset($this->objects[$id])) { + return true; + } return ($this->getChild($id) !== null); } @@ -244,11 +247,19 @@ class Addressbook extends AbstractPIMCollection { $id = $contact->getId(); - if ($this->count() !== null) { + // If this method is called directly the index isn't set. + if (!isset($this->objects[$id])) { + $this->objects[$id] = $contact; + } + + /* If count() hasn't been called yet don't _count hasn't been initialized + * so incrementing it would give a misleading value. + */ + if (isset($this->_count)) { $this->_count += 1; } - \OCP\Util::writeLog('contacts', __METHOD__.' id: '.$id, \OCP\Util::DEBUG); + \OCP\Util::writeLog('contacts', __METHOD__.' id: ' . $id, \OCP\Util::DEBUG); return $id; } @@ -274,7 +285,10 @@ class Addressbook extends AbstractPIMCollection { unset($this->objects[$id]); } - if ($this->count() !== null) { + /* If count() hasn't been called yet don't _count hasn't been initialized + * so decrementing it would give a misleading value. + */ + if (isset($this->_count)) { $this->_count -= 1; } diff --git a/tests/lib/addressbook_test.php b/tests/lib/addressbook_test.php index 0d0246bf..d78f6561 100644 --- a/tests/lib/addressbook_test.php +++ b/tests/lib/addressbook_test.php @@ -149,4 +149,56 @@ class AddressBookTest extends \PHPUnit_Framework_TestCase { } + function testArrayAccess() { + + $carddata = file_get_contents(__DIR__ . '/../data/test2.vcf'); + $vcard = Reader::read($carddata); + + $contact = $this->ab['123']; + + // Test get + $this->assertTrue(isset($this->ab['123'])); + $this->assertInstanceOf('OCA\\Contacts\\Contact', $contact); + $this->assertEquals('Max Mustermann', $contact->getDisplayName()); + + // Test unset + unset($this->ab['123']); + + $this->assertTrue(!isset($this->ab['123'])); + + // Test set + try { + $this->ab[] = $vcard; + } catch(\Exception $e) { + return; + } + + $this->fail('Expected Exception'); + + } + + /** + * @depends testAddChild + */ + function testIterator($ab) { + + $count = 0; + + foreach($ab as $contact) { + $this->assertInstanceOf('OCA\\Contacts\\Contact', $contact); + $count += 1; + } + + $this->assertEquals(2, $count); + } + + /** + * @depends testAddChild + */ + function testCountable($ab) { + + $this->assertEquals(2, count($ab)); + + } + } diff --git a/tests/lib/backend/backend_test.php b/tests/lib/backend/backend_test.php index 72e361a6..f9282ca1 100644 --- a/tests/lib/backend/backend_test.php +++ b/tests/lib/backend/backend_test.php @@ -89,6 +89,7 @@ class BackendTest extends \PHPUnit_Framework_TestCase { function testCreateAddressBookFail() { + // displayname must be provided. $id = $this->backend->createAddressBook(array('description' => 'foo bar')); $this->assertFalse($id); diff --git a/tests/lib/backend/mock.php b/tests/lib/backend/mock.php index 869bba3c..db2083ab 100644 --- a/tests/lib/backend/mock.php +++ b/tests/lib/backend/mock.php @@ -190,4 +190,7 @@ class Mock extends AbstractBackend { } + function numContacts($addressBookId) { + return count($this->contacts[$addressBookId]); + } }