From dad60efccc23d269202658c44bbe933217116971 Mon Sep 17 00:00:00 2001 From: Drak Date: Fri, 10 Feb 2012 15:30:15 +0545 Subject: [PATCH] [HttpFoundation] Add back get defaults and small clean-up. Changed read-only method names from get*() to peek*() Typo --- .../HttpFoundation/DbalSessionStorage.php | 4 +- .../Templating/Helper/SessionHelper.php | 6 +-- .../Templating/Helper/SessionHelperTest.php | 4 ++ .../Session/Flash/AutoExpireFlashBag.php | 14 ++---- .../HttpFoundation/Session/Flash/FlashBag.php | 34 ++++++------- .../Session/Flash/FlashBagInterface.php | 10 ++-- .../Session/SessionInterface.php | 7 --- .../Session/Flash/AutoExpireFlashBagTest.php | 49 ++++++++----------- .../Session/Flash/FlashBagTest.php | 49 ++++++------------- .../HttpFoundation/Session/SessionTest.php | 6 --- .../Session/Storage/MockArrayStorageTest.php | 4 +- .../Session/Storage/MockFileStorageTest.php | 2 +- 12 files changed, 73 insertions(+), 116 deletions(-) diff --git a/src/Symfony/Bridge/Doctrine/HttpFoundation/DbalSessionStorage.php b/src/Symfony/Bridge/Doctrine/HttpFoundation/DbalSessionStorage.php index 8595138dfc..5d2c5d4d1a 100644 --- a/src/Symfony/Bridge/Doctrine/HttpFoundation/DbalSessionStorage.php +++ b/src/Symfony/Bridge/Doctrine/HttpFoundation/DbalSessionStorage.php @@ -4,7 +4,7 @@ namespace Symfony\Bridge\Doctrine\HttpFoundation; use Doctrine\DBAL\Platforms\MySqlPlatform; use Symfony\Component\HttpFoundation\Session\Storage\AbstractStorage; -use Symfony\Component\HttpFoundation\Session\Storage\SessionSaveHandlerInterface; +use Symfony\Component\HttpFoundation\Session\Storage\SaveHandlerInterface; use Doctrine\DBAL\Driver\Connection; /** @@ -13,7 +13,7 @@ use Doctrine\DBAL\Driver\Connection; * @author Fabien Potencier * @author Johannes M. Schmitt */ -class DbalStorage extends AbstractStorage implements SessionSaveHandlerInterface +class DbalStorage extends AbstractStorage implements SaveHandlerInterface { /** * @var Connection diff --git a/src/Symfony/Bundle/FrameworkBundle/Templating/Helper/SessionHelper.php b/src/Symfony/Bundle/FrameworkBundle/Templating/Helper/SessionHelper.php index 684e5d92f5..15e25c460d 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Templating/Helper/SessionHelper.php +++ b/src/Symfony/Bundle/FrameworkBundle/Templating/Helper/SessionHelper.php @@ -47,14 +47,14 @@ class SessionHelper extends Helper return $this->session->get($name, $default); } - public function getFlash($type) + public function getFlash($type, $default = null) { - return $this->session->getFlashes()->get($type); + return $this->session->getFlashes()->pop($type); } public function getFlashes() { - return $this->session->getFlashes()->all(); + return $this->session->getFlashes()->popAll(); } public function hasFlash($type) diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/Templating/Helper/SessionHelperTest.php b/src/Symfony/Bundle/FrameworkBundle/Tests/Templating/Helper/SessionHelperTest.php index 823d949091..e492829897 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Tests/Templating/Helper/SessionHelperTest.php +++ b/src/Symfony/Bundle/FrameworkBundle/Tests/Templating/Helper/SessionHelperTest.php @@ -45,7 +45,11 @@ class SessionHelperTest extends \PHPUnit_Framework_TestCase $this->assertTrue($helper->hasFlash(FlashBag::NOTICE)); $this->assertEquals('bar', $helper->getFlash(FlashBag::NOTICE)); + } + public function testGetFlashes() + { + $helper = new SessionHelper($this->request); $this->assertEquals(array(FlashBag::NOTICE => 'bar'), $helper->getFlashes()); } diff --git a/src/Symfony/Component/HttpFoundation/Session/Flash/AutoExpireFlashBag.php b/src/Symfony/Component/HttpFoundation/Session/Flash/AutoExpireFlashBag.php index e69992568d..39c068edcb 100644 --- a/src/Symfony/Component/HttpFoundation/Session/Flash/AutoExpireFlashBag.php +++ b/src/Symfony/Component/HttpFoundation/Session/Flash/AutoExpireFlashBag.php @@ -75,19 +75,15 @@ class AutoExpireFlashBag implements FlashBagInterface /** * {@inheritdoc} */ - public function get($type) + public function peek($type, $default = null) { - if (!$this->has($type)) { - throw new \InvalidArgumentException(sprintf('Flash type %s not found', $type)); - } - - return $this->flashes['display'][$type]; + return $this->has($type) ? $this->flashes['display'][$type] : $default; } /** * {@inheritdoc} */ - public function all() + public function peekAll() { return array_key_exists('display', $this->flashes) ? (array)$this->flashes['display'] : array(); } @@ -95,10 +91,10 @@ class AutoExpireFlashBag implements FlashBagInterface /** * {@inheritdoc} */ - public function pop($type) + public function pop($type, $default = null) { if (!$this->has($type)) { - throw new \InvalidArgumentException(sprintf('Flash type %s not found', $type)); + return $default; } $return = null; diff --git a/src/Symfony/Component/HttpFoundation/Session/Flash/FlashBag.php b/src/Symfony/Component/HttpFoundation/Session/Flash/FlashBag.php index 99508f760f..1ec7175847 100644 --- a/src/Symfony/Component/HttpFoundation/Session/Flash/FlashBag.php +++ b/src/Symfony/Component/HttpFoundation/Session/Flash/FlashBag.php @@ -68,27 +68,15 @@ class FlashBag implements FlashBagInterface /** * {@inheritdoc} */ - public function get($type) + public function peek($type, $default = null) { - if (!$this->has($type)) { - throw new \InvalidArgumentException(sprintf('Flash type %s not found', $type)); - } - - return $this->flashes[$type]; + return $this->has($type) ? $this->flashes[$type] : $default; } /** * {@inheritdoc} */ - public function set($type, $message) - { - $this->flashes[$type] = $message; - } - - /** - * {@inheritdoc} - */ - public function all() + public function peekAll() { return $this->flashes; } @@ -96,13 +84,13 @@ class FlashBag implements FlashBagInterface /** * {@inheritdoc} */ - public function pop($type) + public function pop($type, $default = null) { if (!$this->has($type)) { - throw new \InvalidArgumentException(sprintf('Flash type %s not found', $type)); + return $default; } - $return = $this->get($type); + $return = $this->peek($type); unset($this->flashes[$type]); return $return; @@ -113,12 +101,20 @@ class FlashBag implements FlashBagInterface */ public function popAll() { - $return = $this->all(); + $return = $this->peekAll(); $this->flashes = array(); return $return; } + /** + * {@inheritdoc} + */ + public function set($type, $message) + { + $this->flashes[$type] = $message; + } + /** * {@inheritdoc} */ diff --git a/src/Symfony/Component/HttpFoundation/Session/Flash/FlashBagInterface.php b/src/Symfony/Component/HttpFoundation/Session/Flash/FlashBagInterface.php index 262f624e80..51b4927a6b 100644 --- a/src/Symfony/Component/HttpFoundation/Session/Flash/FlashBagInterface.php +++ b/src/Symfony/Component/HttpFoundation/Session/Flash/FlashBagInterface.php @@ -36,27 +36,29 @@ interface FlashBagInterface extends SessionBagInterface /** * Gets flash message for a given type. * - * @param string $type Message category type. + * @param string $type Message category type. + * @param string $default Default value if $type doee not exist. * * @return string */ - function get($type); + function peek($type, $default = null); /** * Gets all flash messages. * * @return array */ - function all(); + function peekAll(); /** * Pops and clears flash from the stack. * * @param string $type + * @param string $default Default value if $type doee not exist. * * @return string */ - function pop($type); + function pop($type, $default = null); /** * Pops and clears flashes from the stack. diff --git a/src/Symfony/Component/HttpFoundation/Session/SessionInterface.php b/src/Symfony/Component/HttpFoundation/Session/SessionInterface.php index 8bb013514b..b2c4b8e00b 100644 --- a/src/Symfony/Component/HttpFoundation/Session/SessionInterface.php +++ b/src/Symfony/Component/HttpFoundation/Session/SessionInterface.php @@ -108,11 +108,4 @@ interface SessionInterface extends \Serializable * Clears all attributes. */ function clear(); - - /** - * Gets the flashbag interface. - * - * @return FlashBagInterface - */ - function getFlashes(); } diff --git a/tests/Symfony/Tests/Component/HttpFoundation/Session/Flash/AutoExpireFlashBagTest.php b/tests/Symfony/Tests/Component/HttpFoundation/Session/Flash/AutoExpireFlashBagTest.php index be47a83003..b7850ef165 100644 --- a/tests/Symfony/Tests/Component/HttpFoundation/Session/Flash/AutoExpireFlashBagTest.php +++ b/tests/Symfony/Tests/Component/HttpFoundation/Session/Flash/AutoExpireFlashBagTest.php @@ -50,34 +50,28 @@ class AutoExpireFlashBagTest extends \PHPUnit_Framework_TestCase $bag = new FlashBag(); $array = array('new' => array(FlashBag::NOTICE => 'A previous flash message')); $bag->initialize($array); - $this->assertEquals('A previous flash message', $bag->get(FlashBag::NOTICE)); + $this->assertEquals('A previous flash message', $bag->peek(FlashBag::NOTICE)); $array = array('new' => array( FlashBag::NOTICE => 'Something else', FlashBag::ERROR => 'a', )); $bag->initialize($array); - $this->assertEquals('Something else', $bag->get(FlashBag::NOTICE)); - $this->assertEquals('a', $bag->get(FlashBag::ERROR)); + $this->assertEquals('Something else', $bag->peek(FlashBag::NOTICE)); + $this->assertEquals('a', $bag->peek(FlashBag::ERROR)); } - public function testGet() + public function testPeek() { - $this->assertEquals('A previous flash message', $this->bag->get(FlashBag::NOTICE)); - $this->assertEquals('A previous flash message', $this->bag->get(FlashBag::NOTICE)); - } - - /** - * @expectedException \InvalidArgumentException - */ - public function testGetException() - { - $this->bag->get('non_existing_type'); + $this->assertNull($this->bag->peek('non_existing')); + $this->assertEquals('default', $this->bag->peek('non_existing', 'default')); + $this->assertEquals('A previous flash message', $this->bag->peek(FlashBag::NOTICE)); + $this->assertEquals('A previous flash message', $this->bag->peek(FlashBag::NOTICE)); } public function testSet() { $this->bag->set(FlashBag::NOTICE, 'Foo'); - $this->assertNotEquals('Foo', $this->bag->get(FlashBag::NOTICE)); + $this->assertNotEquals('Foo', $this->bag->peek(FlashBag::NOTICE)); } public function testHas() @@ -91,7 +85,7 @@ class AutoExpireFlashBagTest extends \PHPUnit_Framework_TestCase $this->assertEquals(array(FlashBag::NOTICE), $this->bag->keys()); } - public function testAll() + public function testPeekAll() { $array = array( 'new' => array( @@ -104,25 +98,22 @@ class AutoExpireFlashBagTest extends \PHPUnit_Framework_TestCase $this->assertEquals(array( FlashBag::NOTICE => 'Foo', FlashBag::ERROR => 'Bar', - ), $this->bag->all() + ), $this->bag->peekAll() + ); + + $this->assertEquals(array( + FlashBag::NOTICE => 'Foo', + FlashBag::ERROR => 'Bar', + ), $this->bag->peekAll() ); } - /** - * @expectedException \InvalidArgumentException - */ public function testPop() { + $this->assertNull($this->bag->pop('non_existing')); + $this->assertEquals('default', $this->bag->pop('non_existing', 'default')); $this->assertEquals('A previous flash message', $this->bag->pop(FlashBag::NOTICE)); - $this->bag->pop(FlashBag::NOTICE); - } - - /** - * @expectedException \InvalidArgumentException - */ - public function testPopException() - { - $this->bag->pop('non_existing_type'); + $this->assertNull($this->bag->pop(FlashBag::NOTICE)); } public function testPopAll() diff --git a/tests/Symfony/Tests/Component/HttpFoundation/Session/Flash/FlashBagTest.php b/tests/Symfony/Tests/Component/HttpFoundation/Session/Flash/FlashBagTest.php index 5bd8f4b01d..8f173a7827 100644 --- a/tests/Symfony/Tests/Component/HttpFoundation/Session/Flash/FlashBagTest.php +++ b/tests/Symfony/Tests/Component/HttpFoundation/Session/Flash/FlashBagTest.php @@ -49,45 +49,26 @@ class FlashBagTest extends \PHPUnit_Framework_TestCase { $bag = new FlashBag(); $bag->initialize($this->array); - $this->assertEquals($this->array, $bag->all()); + $this->assertEquals($this->array, $bag->peekAll()); $array = array('should' => array('change')); $bag->initialize($array); - $this->assertEquals($array, $bag->all()); + $this->assertEquals($array, $bag->peekAll()); } - public function testGet() + public function testPeek() { - $this->assertEquals('A previous flash message', $this->bag->get(FlashBag::NOTICE)); - } - - /** - * @expectedException \InvalidArgumentException - */ - public function testGetException() - { - $this->bag->get('non_existing_type'); + $this->assertNull($this->bag->peek('non_existing')); + $this->assertEquals('default', $this->bag->peek('not_existing', 'default')); + $this->assertEquals('A previous flash message', $this->bag->peek(FlashBag::NOTICE)); + $this->assertEquals('A previous flash message', $this->bag->peek(FlashBag::NOTICE)); } public function testPop() { + $this->assertNull($this->bag->pop('non_existing')); + $this->assertEquals('default', $this->bag->pop('not_existing', 'default')); $this->assertEquals('A previous flash message', $this->bag->pop(FlashBag::NOTICE)); - } - - /** - * @expectedException \InvalidArgumentException - */ - public function testPopException() - { - $this->assertEquals('A previous flash message', $this->bag->pop(FlashBag::NOTICE)); - $this->bag->pop(FlashBag::NOTICE); - } - - /** - * @expectedException \InvalidArgumentException - */ - public function testPopExceptionNotExisting() - { - $this->bag->pop('non_existing_type'); + $this->assertNull($this->bag->pop(FlashBag::NOTICE)); } public function testPopAll() @@ -102,11 +83,11 @@ class FlashBagTest extends \PHPUnit_Framework_TestCase $this->assertEquals(array(), $this->bag->popAll()); } - public function testset() + public function testSet() { $this->bag->set(FlashBag::NOTICE, 'Foo'); $this->bag->set(FlashBag::NOTICE, 'Bar'); - $this->assertEquals('Bar', $this->bag->get(FlashBag::NOTICE)); + $this->assertEquals('Bar', $this->bag->peek(FlashBag::NOTICE)); } public function testHas() @@ -120,21 +101,21 @@ class FlashBagTest extends \PHPUnit_Framework_TestCase $this->assertEquals(array(FlashBag::NOTICE), $this->bag->keys()); } - public function testAll() + public function testPeekAll() { $this->bag->set(FlashBag::NOTICE, 'Foo'); $this->bag->set(FlashBag::ERROR, 'Bar'); $this->assertEquals(array( FlashBag::NOTICE => 'Foo', FlashBag::ERROR => 'Bar', - ), $this->bag->all() + ), $this->bag->peekAll() ); $this->assertTrue($this->bag->has(FlashBag::NOTICE)); $this->assertTrue($this->bag->has(FlashBag::ERROR)); $this->assertEquals(array( FlashBag::NOTICE => 'Foo', FlashBag::ERROR => 'Bar', - ), $this->bag->all() + ), $this->bag->peekAll() ); } } diff --git a/tests/Symfony/Tests/Component/HttpFoundation/Session/SessionTest.php b/tests/Symfony/Tests/Component/HttpFoundation/Session/SessionTest.php index 1b1cbd622c..41c714ea59 100644 --- a/tests/Symfony/Tests/Component/HttpFoundation/Session/SessionTest.php +++ b/tests/Symfony/Tests/Component/HttpFoundation/Session/SessionTest.php @@ -123,28 +123,22 @@ class SessionTest extends \PHPUnit_Framework_TestCase public function testInvalidate() { $this->session->set('invalidate', 123); - $this->session->getFlashes()->set(FlashBag::NOTICE, 'OK'); $this->session->invalidate(); $this->assertEquals(array(), $this->session->all()); - $this->assertEquals(array(), $this->session->getFlashes()->all()); } public function testMigrate() { $this->session->set('migrate', 321); - $this->session->getFlashes()->set(FlashBag::NOTICE, 'HI'); $this->session->migrate(); $this->assertEquals(321, $this->session->get('migrate')); - $this->assertEquals('HI', $this->session->getFlashes()->get(FlashBag::NOTICE)); } public function testMigrateDestroy() { $this->session->set('migrate', 333); - $this->session->getFlashes()->set(FlashBag::NOTICE, 'Bye'); $this->session->migrate(true); $this->assertEquals(333, $this->session->get('migrate')); - $this->assertEquals('Bye', $this->session->getFlashes()->get(FlashBag::NOTICE)); } public function testSerialize() diff --git a/tests/Symfony/Tests/Component/HttpFoundation/Session/Storage/MockArrayStorageTest.php b/tests/Symfony/Tests/Component/HttpFoundation/Session/Storage/MockArrayStorageTest.php index 33b43a4da2..29d9078532 100644 --- a/tests/Symfony/Tests/Component/HttpFoundation/Session/Storage/MockArrayStorageTest.php +++ b/tests/Symfony/Tests/Component/HttpFoundation/Session/Storage/MockArrayStorageTest.php @@ -72,13 +72,13 @@ class MockArrayStorageTest extends \PHPUnit_Framework_TestCase $this->storage->regenerate(); $this->assertNotEquals($id, $this->storage->getId()); $this->assertEquals(array('foo' => 'bar'), $this->storage->getBag('attributes')->all()); - $this->assertEquals(array('notice' => 'hello'), $this->storage->getBag('flashes')->all()); + $this->assertEquals(array('notice' => 'hello'), $this->storage->getBag('flashes')->peekAll()); $id = $this->storage->getId(); $this->storage->regenerate(true); $this->assertNotEquals($id, $this->storage->getId()); $this->assertEquals(array('foo' => 'bar'), $this->storage->getBag('attributes')->all()); - $this->assertEquals(array('notice' => 'hello'), $this->storage->getBag('flashes')->all()); + $this->assertEquals(array('notice' => 'hello'), $this->storage->getBag('flashes')->peekAll()); } public function testGetId() diff --git a/tests/Symfony/Tests/Component/HttpFoundation/Session/Storage/MockFileStorageTest.php b/tests/Symfony/Tests/Component/HttpFoundation/Session/Storage/MockFileStorageTest.php index 3ccc139605..337faf4ca0 100644 --- a/tests/Symfony/Tests/Component/HttpFoundation/Session/Storage/MockFileStorageTest.php +++ b/tests/Symfony/Tests/Component/HttpFoundation/Session/Storage/MockFileStorageTest.php @@ -79,7 +79,7 @@ class MockFileStorageTest extends \PHPUnit_Framework_TestCase $storage->start(); $this->assertEquals('108', $storage->getBag('attributes')->get('new')); $this->assertTrue($storage->getBag('flashes')->has('newkey')); - $this->assertEquals('test', $storage->getBag('flashes')->get('newkey')); + $this->assertEquals('test', $storage->getBag('flashes')->peek('newkey')); } public function testMultipleInstances()