set plugin priority, fixes solariumphp#350 (solariumphp#803)

Co-authored-by: Markus Kalkbrenner <[email protected]>

CHANGELOG.md

@@ -9,6 +9,8 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
 ### Added
 
 ## Changed
+- PostBigRequest plugin now acts on PRE_EXECUTE_REQUEST event instead of POST_CREATE_REQUEST
+- CustomizeRequest plugin now acts on POST_CREATE_REQUEST event instead of PRE_EXECUTE_REQUEST
 
 ### Removed
 - PHP 7.1 support

docs/customizing-solarium.md

@@ -5,7 +5,6 @@ Solarium was designed to be highly customizable, in the following ways:
 
 1.  by allowing for partial usage, for instance only convert a query into a request object but handle the communication in your own code
 2.  by adding an event dispatcher for plugins
-3.  and also still supporting the extending of classes (Solarium doesn't use 'private' or 'final' anywhere)
 
 What method of customization to use depends on your case:
 
@@ -17,7 +16,6 @@ What method of customization to use depends on your case:
     -   Plugins can easily be disabled, for instance in case of issues or for debugging
     -   No issues with upgrading Solarium
     -   A very basic plugin could even be implemented using a closure, so plugins are not hard to use!
--   if for some reason method 2 fails, or you really favor extending, that's of course still possible. But the preferred method is using the plugin structure.
 
 
 Partial usage
@@ -292,11 +290,16 @@ htmlFooter();
 
 ```
 
+The order of plugin executions
+------------------------------
 
-Extending Solarium classes
-==========================
+Since plugins leverage events, the event dispatcher is responsible for the order they get called if two plugins register
+for the same event. In some cases that doesn't matter in other cases it is essential that Plugin A acts before Plugin B.
+If the order matters you need to read the documentation of the event dispatcher of choice that you inject into the
+Solarium Client's constructor.
 
-You can extend any class in Solarium, there is no use of 'private' or 'final'. However, Solarium does have several built-in class mappings for factory methods. Most important are the querytype mapping in Solarium\\Client and the component mapping in Solarium\\QueryType\\Select\\Query\\Query. In some cases you might need to alter this mappings for your classes to be used.
-Other than that, there are no special issues to be expected. You can extend Solarium classes like any other PHP class.
+For various events `CustomizeRequest` needs to be executed before `PostBigRequest`. And `Loadbalancer` should always be
+the last plugin.
 
-If your use case can be solved using plugins that is probably the safest choice, as by extending classes you gain access to non-public methods in the API. These are subject to change in future releases, where the public API is not (except for major releases). 
+If you use Symfony's event dispatcher, Solarium takes care of the order of these critical plugins and the events they
+listen to. This is done by setting the "priority".

src/Plugin/CustomizeRequest/CustomizeRequest.php

@@ -3,7 +3,7 @@
 namespace Solarium\Plugin\CustomizeRequest;
 
 use Solarium\Core\Event\Events;
-use Solarium\Core\Event\PreExecuteRequest;
+use Solarium\Core\Event\postCreateRequest;
 use Solarium\Core\Plugin\AbstractPlugin;
 use Solarium\Exception\InvalidArgumentException;
 use Solarium\Exception\RuntimeException;
@@ -194,10 +194,10 @@ public function setCustomizations(array $customizations): self
      *
      * @return self Provides fluent interface
      */
-    public function preExecuteRequest($event): self
+    public function postCreateRequest($event): self
     {
         // We need to accept event proxies or decoraters.
-        /* @var PreExecuteRequest $event */
+        /* @var PostCreateRequest $event */
         $request = $event->getRequest();
         foreach ($this->getCustomizations() as $key => $customization) {
             // first validate
@@ -225,8 +225,6 @@ public function preExecuteRequest($event): self
             }
         }
 
-        $event->setRequest($request);
-
         return $this;
     }
 
@@ -253,7 +251,7 @@ protected function initPluginType()
     {
         $dispatcher = $this->client->getEventDispatcher();
         if (is_subclass_of($dispatcher, '\Symfony\Component\EventDispatcher\EventDispatcherInterface')) {
-            $dispatcher->addListener(Events::PRE_EXECUTE_REQUEST, [$this, 'preExecuteRequest']);
+            $dispatcher->addListener(Events::POST_CREATE_REQUEST, [$this, 'postCreateRequest']);
         }
     }
 }

src/Plugin/Loadbalancer/Loadbalancer.php

@@ -546,7 +546,8 @@ protected function initPluginType()
     {
         $dispatcher = $this->client->getEventDispatcher();
         if (is_subclass_of($dispatcher, '\Symfony\Component\EventDispatcher\EventDispatcherInterface')) {
-            $dispatcher->addListener(Events::PRE_EXECUTE_REQUEST, [$this, 'preExecuteRequest']);
+            // The Loadbalancer plugin needs to be the last plugin executed on PRE_EXECUTE_REQUEST. Set Priority to 0.
+            $dispatcher->addListener(Events::PRE_EXECUTE_REQUEST, [$this, 'preExecuteRequest'], 0);
             $dispatcher->addListener(Events::PRE_CREATE_REQUEST, [$this, 'preCreateRequest']);
         }
     }

src/Plugin/PostBigRequest.php

@@ -86,7 +86,8 @@ protected function initPluginType()
     {
         $dispatcher = $this->client->getEventDispatcher();
         if (is_subclass_of($dispatcher, '\Symfony\Component\EventDispatcher\EventDispatcherInterface')) {
-            $dispatcher->addListener(Events::PRE_EXECUTE_REQUEST, [$this, 'preExecuteRequest']);
+            // PostBigRequest has to act on PRE_EXECUTE_REQUEST before Loadbalancer (priority 0). Set priority to 10.
+            $dispatcher->addListener(Events::PRE_EXECUTE_REQUEST, [$this, 'preExecuteRequest'], 10);
         }
     }
 }

tests/Plugin/CustomizeRequest/CustomizeRequestTest.php

@@ -3,15 +3,13 @@
 namespace Solarium\Tests\Plugin\CustomizeRequest;
 
 use PHPUnit\Framework\TestCase;
-use Solarium\Core\Client\Adapter\AdapterInterface;
-use Solarium\Core\Client\Endpoint;
 use Solarium\Core\Client\Request;
-use Solarium\Core\Client\Response;
-use Solarium\Core\Event\PreExecuteRequest as PreExecuteRequestEvent;
+use Solarium\Core\Event\PostCreateRequest as PostCreateRequestEvent;
 use Solarium\Exception\InvalidArgumentException;
 use Solarium\Exception\RuntimeException;
 use Solarium\Plugin\CustomizeRequest\Customization;
 use Solarium\Plugin\CustomizeRequest\CustomizeRequest;
+use Solarium\QueryType\Ping\Query;
 use Solarium\Tests\Integration\TestClientFactory;
 
 class CustomizeRequestTest extends TestCase
@@ -74,26 +72,16 @@ public function testPluginIntegration()
         $client->registerPlugin('testplugin', $this->plugin);
 
         $input = [
-                    'key' => 'xid',
-                    'type' => 'param',
-                    'name' => 'xid',
-                    'value' => 123,
-                ];
+            'key' => 'xid',
+            'type' => 'param',
+            'name' => 'xid',
+            'value' => 123,
+        ];
         $this->plugin->addCustomization($input);
 
-        $originalRequest = new Request();
-        $expectedRequest = new Request();
-        $expectedRequest->addParam('xid', 123); // this should be the effect of the customization
+        $request = $client->createRequest(new Query());
 
-        $adapter = $this->createMock(AdapterInterface::class);
-        $response = new Response('', ['HTTP 1.0 200 OK']);
-        $adapter->expects($this->once())
-                 ->method('execute')
-                 ->with($this->equalTo($expectedRequest))
-                 ->willReturn($response);
-        $client->setAdapter($adapter);
-
-        $client->executeRequest($originalRequest);
+        $this->assertSame(123, $request->getParam('xid'));
     }
 
     public function testCreateCustomization()
@@ -315,15 +303,15 @@ public function testPostCreateRequestWithHeaderAndParam()
         $this->plugin->addCustomization($input);
 
         $request = new Request();
-        $event = new PreExecuteRequestEvent($request, new Endpoint());
-        $this->plugin->preExecuteRequest($event);
+        $event = new PostCreateRequestEvent(new Query(), $request);
+        $this->plugin->postCreateRequest($event);
 
         $this->assertSame(123, $request->getParam('xid'));
 
         $this->assertEquals(['X-my-auth: mypassword'], $request->getHeaders());
     }
 
-    public function testPreExecuteRequestWithInvalidCustomization()
+    public function testPostCreateRequestWithInvalidCustomization()
     {
         $input = [
             'key' => 'xid',
@@ -334,24 +322,24 @@ public function testPreExecuteRequestWithInvalidCustomization()
         $this->plugin->addCustomization($input);
 
         $request = new Request();
-        $event = new PreExecuteRequestEvent($request, new Endpoint());
+        $event = new PostCreateRequestEvent(new Query(), $request);
 
         $this->expectException(RuntimeException::class);
-        $this->plugin->preExecuteRequest($event);
+        $this->plugin->postCreateRequest($event);
     }
 
-    public function testPreExecuteRequestWithoutCustomizations()
+    public function testPostCreateRequestWithoutCustomizations()
     {
         $request = new Request();
         $originalRequest = clone $request;
 
-        $event = new PreExecuteRequestEvent($request, new Endpoint());
-        $this->plugin->preExecuteRequest($event);
+        $event = new PostCreateRequestEvent(new Query(), $request);
+        $this->plugin->postCreateRequest($event);
 
         $this->assertEquals($originalRequest, $request);
     }
 
-    public function testPreExecuteRequestWithPersistentAndNonPersistentCustomizations()
+    public function testPostCreateRequestWithPersistentAndNonPersistentCustomizations()
     {
         $input = [
                     'key' => 'xid',
@@ -371,17 +359,17 @@ public function testPreExecuteRequestWithPersistentAndNonPersistentCustomization
         $this->plugin->addCustomization($input);
 
         $request = new Request();
-        $event = new PreExecuteRequestEvent($request, new Endpoint());
-        $this->plugin->preExecuteRequest($event);
+        $event = new PostCreateRequestEvent(new Query(), $request);
+        $this->plugin->postCreateRequest($event);
 
         $this->assertSame(123, $request->getParam('xid'));
 
         $this->assertEquals(['X-my-auth: mypassword'], $request->getHeaders());
 
         // second use, only the header should be persistent
         $request = new Request();
-        $event = new PreExecuteRequestEvent($request, new Endpoint());
-        $this->plugin->preExecuteRequest($event);
+        $event = new PostCreateRequestEvent(new Query(), $request);
+        $this->plugin->postCreateRequest($event);
 
         $this->assertNull($request->getParam('xid'));