Skip to content
This repository has been archived by the owner on Jan 30, 2020. It is now read-only.

Make StandaloneExtensionManagers configurable #55

Closed
wants to merge 22 commits into from
Closed
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 21 additions & 0 deletions src/Reader/StandaloneExtensionManager.php
Original file line number Diff line number Diff line change
Expand Up @@ -49,4 +49,25 @@ public function get($extension)
$class = $this->extensions[$extension];
return new $class();
}

/**
* Add an extension.
*
* @param string $name
* @param string $class
*/
public function add($name, $class)
{
$this->extensions[$name] = $class;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No validation what is added to the manager?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good point. $class should be checked to be instance of Zend\Feed\Reader\Extension\AbstractEntry or Zend\Feed\Reader\Extension\AbstractFeed

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wondered about that - $class can be a string (it was in the components until I changed it), but it might be the name of something that doesn't exist yet, so I don't know that it can be validated at this point.

Copy link
Member

@Xerkus Xerkus Nov 14, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

with check it will be autoloaded when extension is registered. Plugin manager validates on creation.

}

/**
* Remove an extension.
*
* @param string $name
*/
public function remove($name)
{
unset($this->extensions[$name]);
}
}
14 changes: 14 additions & 0 deletions test/Reader/StandaloneExtensionManagerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -80,4 +80,18 @@ public function testEachPluginRetrievalReturnsNewInstance($pluginName, $pluginCl
$this->assertInstanceOf($pluginClass, $test);
$this->assertNotSame($extension, $test);
}

public function testAddingPlugin()
{
$this->extensions->add('Test/Test', 'mytestextension');
$this->assertTrue($this->extensions->has('Test/Test'));
}

public function testRemovingPlugin()
{
$this->extensions->add('Test/Test', 'mytestextension');
$this->assertTrue($this->extensions->remove('Test/Test'));
$this->assertFalse($this->extensions->has('Test/Test'));
$this->assertFalse($this->extensions->remove('Test/Test'));
}
}
88 changes: 88 additions & 0 deletions test/Writer/StandaloneExtensionManagerTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
<?php
/**
* Zend Framework (http://framework.zend.com/)
*
* @link http://github.com/zendframework/zf2 for the canonical source repository
* @copyright Copyright (c) 2005-2015 Zend Technologies USA Inc. (http://www.zend.com)
* @license http://framework.zend.com/license/new-bsd New BSD License
*/
namespace ZendTest\Feed\Writer;

use PHPUnit\Framework\TestCase;
use Zend\Feed\Reader\StandaloneExtensionManager;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is supposed to be Writer

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, my IDE had collapsed the use statements so I didn't see them. Fixed.

use Zend\Feed\Reader\Extension\WellFormedWeb\Entry;
use Zend\Feed\Reader\Extension\Syndication\Feed;
use Zend\Feed\Reader\ExtensionManagerInterface;

class StandaloneExtensionManagerTest extends TestCase
{
/**
* @var StandaloneExtensionManager
*/
private $extensions;

public function setUp()
{
$this->extensions = new StandaloneExtensionManager();
}

public function testIsAnExtensionManagerImplementation()
{
$this->assertInstanceOf(ExtensionManagerInterface::class, $this->extensions);
}

public function defaultPlugins()
{
return [
'Atom\Renderer\Feed' => Extension\Atom\Renderer\Feed::class,
'Content\Renderer\Entry' => Extension\Content\Renderer\Entry::class,
'DublinCore\Renderer\Entry' => Extension\DublinCore\Renderer\Entry::class,
'DublinCore\Renderer\Feed' => Extension\DublinCore\Renderer\Feed::class,
'ITunes\Entry' => Extension\ITunes\Entry::class,
'ITunes\Feed' => Extension\ITunes\Feed::class,
'ITunes\Renderer\Entry' => Extension\ITunes\Renderer\Entry::class,
'ITunes\Renderer\Feed' => Extension\ITunes\Renderer\Feed::class,
'Slash\Renderer\Entry' => Extension\Slash\Renderer\Entry::class,
'Threading\Renderer\Entry' => Extension\Threading\Renderer\Entry::class,
'WellFormedWeb\Renderer\Entry' => Extension\WellFormedWeb\Renderer\Entry::class,
];
}

/**
* @dataProvider defaultPlugins
*/
public function testHasAllDefaultPlugins($pluginName, $pluginClass)
{
$this->assertTrue($this->extensions->has($pluginName));
}

/**
* @dataProvider defaultPlugins
*/
public function testCanRetrieveDefaultPluginInstances($pluginName, $pluginClass)
{
$extension = $this->extensions->get($pluginName);
$this->assertInstanceOf($pluginClass, $extension);
}

/**
* @dataProvider defaultPlugins
*/
public function testEachPluginRetrievalReturnsNewInstance($pluginName, $pluginClass)
{
$extension = $this->extensions->get($pluginName);
$this->assertInstanceOf($pluginClass, $extension);

$test = $this->extensions->get($pluginName);
$this->assertInstanceOf($pluginClass, $test);
$this->assertNotSame($extension, $test);
}

public function testPluginAddRemove()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here, additional tests for expected exception in case of invalid extension class or object of valid extension.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

additional tests to ensure extensions derived from allowed class or with classnames ending with 'Feed' or 'Entry' are accepted

{
$this->extensions->add('Test/Test', 'mytestextension');
$this->assertTrue($this->extensions->has('Test/Test'));
$this->extensions->remove('Test/Test');
$this->assertFalse($this->extensions->has('Test/Test'));
}
}