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 3 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
28 changes: 27 additions & 1 deletion src/Reader/StandaloneExtensionManager.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@

namespace Zend\Feed\Reader;

class StandaloneExtensionManager implements ExtensionManagerInterface
final class StandaloneExtensionManager implements ExtensionManagerInterface
Copy link
Member

Choose a reason for hiding this comment

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

We can't make it final, it will require a major version bump as someone is quite likely already extending it.

{
private $extensions = [
'Atom\Entry' => 'Zend\Feed\Reader\Extension\Atom\Entry',
Expand Down Expand Up @@ -49,4 +49,30 @@ 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
* @return bool
Copy link
Member

Choose a reason for hiding this comment

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

It does not need to report the result. It is idempotent operation. void would be fine

*/
public function remove($name)
{
if (array_key_exists($name, $this->extensions)) {
unset($this->extensions[$name]);
return true;
}
return false;
}
}
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'));
}
}