Remove (at|de)tachAggregate() methods from EventManagerInterface

This patch removes the `attachAggregate()` and `detachAggregate()` methods from
the `EventManagerInterface`. Since they simply proxied to the `attach()` and
`detach()` methods of the provided `ListenerAggregateInterface` implementation,
it was simply a redundant way to attach aggregate listeners that resulted in
performance overhead.

Migration documentation has been updated, as have all examples using aggregates.

Author: weierophinney

doc/book/aggregates.md

@@ -13,14 +13,9 @@ attach(EventManagerInterface $events, $priority = 1);
 detach(EventManagerInterface $events);
 ```
 
-You can attach them to the event manager in one of two ways:
-
-- By passing an instance of `Zend\EventManager\EventManager` to your listener's
-  `attach()` method.
-- By passing the aggregate to `Zend\EventManager\EventManager::attachAggregate()`
-
-The latter essentially passes the event manager instance to the aggregate's
-`attach()` method.
+To attach an aggregate to an event manager, you pass the event manager to the
+aggregate's `attach()` method; in that method, you will then attach listeners to
+the events you are interested in.
 
 ## Implementation
 
@@ -82,23 +77,15 @@ we provide two facilities for implementing this:
 ## Usage
 
 To use an aggregate listener, you need to attach it to the event manager. As
-noted in the intro to this section, you can either pass it to the event
-manager's `attachAggregate()` method, or pass the event manager to the
-aggregate's `attach()` method.
+noted in the intro to this section, you do so by passing the event
+manager to the aggregate's `attach()` method:
 
 ```php
 // Assume $events is an EventManager instance, and $aggregate is an instance of
 // the Aggregate class defined earlier.
-
-// Passing the aggregate to the EventManager:
-$events->attachAggregate($aggregate);
-
-// Passing the EventManager to the aggregate:
 $aggregate->attach($events);
 ```
 
-Both approaches accomplish the same thing.
-
 ## Recommendations
 
 - We recommend using listener aggregates when you have several listeners that are

doc/book/api.md

@@ -169,26 +169,6 @@ method will detach the listener from that event only (or, if the event does not
 exist in the event manager, nothing will occur). If no event is provided, or the
 wildcard event is provided, the listener will be detached from all events.
 
-### attachAggregate()
-
-```php
-attachAggregate(ListenerAggregateInterface $aggregate, $priority = 1) : void
-```
-
-Provided an aggregate, this method will then call the aggregate's `attach()`
-method, passing the event manager instance and the provided priority (if any).
-See the [section on aggregates](aggregates.md) for more information.
-
-### detachAggregate()
-
-```php
-detachAggregate(ListenerAggregateInterface $aggregate) : void
-```
-
-Provided an aggregate, this method will then call the aggregate's `detach()`
-method, passing the event manager instance.  See the
-[section on aggregates](aggregates.md) for more information.
-
 ### clearListeners()
 
 ```php

doc/book/examples.md

@@ -204,12 +204,12 @@ class CacheListener implements ListenerAggregateInterface
 }
 ```
 
-We can then attach the aggregate to an instance.
+We can then attach the aggregate to an event manager instance.
 
 ```
 $value         = new SomeValueObject();
 $cacheListener = new CacheListener($cache);
-$value->getEventManager()->attachAggregate($cacheListener);
+$cacheListener->attach($value->getEventManager());
 ```
 
 Now, as we call `get()`, if we have a cached entry, it will be returned

doc/book/lazy-listeners/lazy-listener-aggregate.md

@@ -44,7 +44,7 @@ $aggregate = new LazyListenerAggregate(
     $definitions,
     $container
 );
-$events->attachAggregate($aggregate);
+$aggregate->attach($events);
 ```
 
 Internally, the `LazyListenerAggregate` will create `LazyEventListener`
@@ -81,7 +81,7 @@ $aggregate = new LazyListenerAggregate(
     $definitions,
     $container
 );
-$events->attachAggregate($aggregate);
+$aggregate->attach($events);
 ```
 
 ## Recommendations

doc/book/migration/changed.md

@@ -173,9 +173,11 @@ In order to migrate to version 3, you will need to make a few changes to your
 application.
 
 First, if you are attaching or detaching aggregate listeners using `attach()`
-and `detach()`, you should change them to use `attachAggregate()` and
-`detachAggregate()` instead. These methods already exist in version 2, giving
-clear forwards compatibility.
+and `detach()`, you should change such calls to instead pass the event manager
+to the relevant `ListenerAggregateInterface` method, as detailed in the
+[removed functionality](removed.md#eventmanagerinterfaceattachaggregate-and-detachaggregate)
+documentation. These methods have existed in all released versions, giving
+perfect forwards compatibility.
 
 Second, if you are manually creating `CallbackHandler` instances to attach to an
 event manager, stop doing so, and attach the callable listener itself instead.

doc/book/migration/removed.md

@@ -15,6 +15,75 @@ option from the framework entirely.
 The trait `Zend\EventManager\ProvidesEvents` has been deprecated for most of
 the 2.0 series; use `Zend\EventManager\EventManagerAwareTrait` instead.
 
+## EventManagerInterface::setSharedManager()
+
+We have removed `EventManagerInterface::setSharedManager()`, and also removed it
+from the `EventManager` implementation. The `SharedEventManager` should be
+injected during instantiation now.
+
+## EventManagerInterface::getEvents() and getListeners()
+
+We have removed both `EventManagerInterface::getEvents()` and `getListeners()`,
+as we did not have a stated use case for the methods. The event manager should
+be something that aggregates listeners and triggers events; the details of what
+listeners or events are attached is largely irrelevant.
+
+The primary use case for `getListeners()` is often to determine if a listener is
+attached before detaching it. Since `detach()` acts as a no-op if the provided
+listener is not present, checking for presence first is not necessary.
+
+## EventManagerInterface::setEventClass()
+
+The method `EventManagerInterface::setEventClass()` was removed and replaced
+with `EventManagerInterface::setEventPrototype()`, which has the following
+signature:
+
+```php
+setEventPrototype(EventInterface $event);
+```
+
+This was done to prevent errors that occurred when invalid event class names
+were provided. Additionally, internally, event managers will clone the
+instance any time `trigger()` or `triggerUntil()` are called — which is
+typically faster and less resource intensive than instantiating a new instance.
+
+## EventManagerInterface::attachAggregate() and detachAggregate()
+
+The methods `attachAggregate()` and `detachAggregate()` were removed from the
+`EventManagerInterface` and concrete `EventManager` implementation. Furthermore,
+`attach()` and `detach()` no longer handle aggregates.
+
+The reason they were removed is because they simply proxied to the `attach()`
+and `detach()` methods of the `ListenerAggregateInterface`. As such, to
+forward-proof your applications, you can alter statements that attach aggregates
+to an event manager reading as follows:
+
+```php
+$events->attach($aggregate); // or
+$events->attachAggregate($aggregate);
+```
+
+to:
+
+```php
+$aggregate->attach($events);
+```
+
+Similarly, for detaching an aggregate, migrate from:
+
+```php
+$events->detach($aggregate); // or
+$events->detachAggregate($aggregate);
+```
+
+to:
+
+```php
+$aggregate->detach($events);
+```
+
+The above works in all released versions of the component.
+
 ## SharedEventAggregateAwareInterface, SharedListenerAggregateInterface
 
 The interfaces `Zend\EventManager\SharedEventAggregateAwareInterface` and
@@ -135,38 +204,6 @@ To migrate, you have the following options:
   Alternately, if you control instantiation of the instance, consider injection
   at instantiation, or within the factory used to create your instance.
 
-## EventManagerInterface::setSharedManager()
-
-We have removed `EventManagerInterface::setSharedManager()`, and also removed it
-from the `EventManager` implementation. The `SharedEventManager` should be
-injected during instantiation now.
-
-## EventManagerInterface::getEvents() and getListeners()
-
-We have removed both `EventManagerInterface::getEvents()` and `getListeners()`,
-as we did not have a stated use case for the methods. The event manager should
-be something that aggregates listeners and triggers events; the details of what
-listeners or events are attached is largely irrelevant.
-
-The primary use case for `getListeners()` is often to determine if a listener is
-attached before detaching it. Since `detach()` acts as a no-op if the provided
-listener is not present, checking for presence first is not necessary.
-
-## EventManagerInterface::setEventClass()
-
-The method `EventManagerInterface::setEventClass()` was removed and replaced
-with `EventManagerInterface::setEventPrototype()`, which has the following
-signature:
-
-```php
-setEventPrototype(EventInterface $event);
-```
-
-This was done to prevent errors that occurred when invalid event class names
-were provided. Additionally, internally, event managers will clone the
-instance any time `trigger()` or `triggerUntil()` are called — which is
-typically faster and less resource intensive than instantiating a new instance.
-
 ## SharedEventManagerInterface::getEvents()
 
 The method `SharedEventManagerInterface::getEvents()` was removed. The method

doc/book/tutorial.md

@@ -308,11 +308,12 @@ class LogEvents implements ListenerAggregateInterface
 > implement `ListenerAggregateInterface`; it defines the `$listeners` property,
 > and the `detach()` logic as demostrated above.
 
-You can attach this using the event manager's `attachAggregate()` method:
+You can attach this by passing the event manager to the aggregate's `attach()`
+method:
 
 ```php
 $logListener = new LogEvents($logger);
-$events->attachAggregate($logListener);
+$logListener->attach($events);
 ```
 
 Any events the aggregate attaches to will then be notified when triggered.

src/EventManager.php

@@ -220,22 +220,6 @@ public function detach(callable $listener, $eventName = null, $force = false)
         }
     }
 
-    /**
-     * @inheritDoc
-     */
-    public function attachAggregate(ListenerAggregateInterface $aggregate, $priority = 1)
-    {
-        $aggregate->attach($this, $priority);
-    }
-
-    /**
-     * @inheritDoc
-     */
-    public function detachAggregate(ListenerAggregateInterface $aggregate)
-    {
-        $aggregate->detach($this);
-    }
-
     /**
      * @inheritDoc
      */

src/EventManagerInterface.php

@@ -161,22 +161,4 @@ public function setIdentifiers(array $identifiers);
      * @return void
      */
     public function addIdentifiers(array $identifiers);
-
-    /**
-     * Attach a listener aggregate
-     *
-     * @param  ListenerAggregateInterface $aggregate
-     * @param  int $priority If provided, a suggested priority for the aggregate to use
-     * @return mixed return value of {@link ListenerAggregateInterface::attach()}
-     */
-    public function attachAggregate(ListenerAggregateInterface $aggregate, $priority = 1);
-
-    /**
-     * Detach a listener aggregate.
-     *
-     * Should delegate to the aggregate's detach() method.
-     *
-     * @param  ListenerAggregateInterface $aggregate
-     */
-    public function detachAggregate(ListenerAggregateInterface $aggregate);
 }

test/EventManagerTest.php

@@ -224,30 +224,6 @@ public function testResponseCollectionIsNotStoppedWhenNoCallbackMatchedByTrigger
         $this->assertEquals('zero', $responses->last());
     }
 
-    public function testCanAttachListenerAggregate()
-    {
-        $aggregate = new TestAsset\MockAggregate();
-        $this->events->attachAggregate($aggregate);
-        $events = $this->getEventListFromManager($this->events);
-        foreach (['foo.bar', 'foo.baz'] as $event) {
-            $this->assertContains($event, $events);
-        }
-    }
-
-    public function testAttachAggregateAcceptsOptionalPriorityValue()
-    {
-        $aggregate = new TestAsset\MockAggregate();
-        $this->events->attachAggregate($aggregate, 1);
-        $this->assertEquals(1, $aggregate->priority);
-    }
-
-    public function testAttachAggregateAcceptsOptionalPriorityValueViaAttachCallbackArgument()
-    {
-        $aggregate = new TestAsset\MockAggregate();
-        $this->events->attachAggregate($aggregate, 1);
-        $this->assertEquals(1, $aggregate->priority);
-    }
-
     public function testCallingEventsStopPropagationMethodHaltsEventEmission()
     {
         // @codingStandardsIgnoreStart
@@ -738,26 +714,6 @@ public function testDetachRemovesAllOccurrencesOfListenerForEvent()
         $this->assertNotContains($listener, $listeners);
     }
 
-    public function testCanDetachAnAggregate()
-    {
-        $events    = $this->events;
-        $aggregate = $this->prophesize(ListenerAggregateInterface::class);
-        $aggregate->attach(Argument::is($events), 1)->shouldBeCalled();
-        $aggregate->detach(Argument::is($events))->shouldBeCalled();
-
-        $events->attachAggregate($aggregate->reveal());
-        $events->detachAggregate($aggregate->reveal());
-    }
-
-    public function testCanAttachAggregateWithPriority()
-    {
-        $events    = $this->events;
-        $aggregate = $this->prophesize(ListenerAggregateInterface::class);
-        $aggregate->attach(Argument::is($events), 5)->shouldBeCalled();
-
-        $events->attachAggregate($aggregate->reveal(), 5);
-    }
-
     public function eventsMissingNames()
     {
         $event = $this->prophesize(EventInterface::class);