Merge pull request #23 from mikadamczyk/request-listener

EZP-27938: Add tests and perform cleanup in RequestListener

composer.json

@@ -24,10 +24,12 @@
     "require-dev": {
         "phpunit/phpunit": "^6.4"
     },
+    "require-dev": {
+        "phpunit/phpunit": "^6.4"
+    },
     "extra": {
         "branch-alias": {
             "dev-master": "1.0.x-dev"
         }
     }
 }
-

src/bundle/Controller/ContentTypeController.php

@@ -22,14 +22,10 @@
 
 class ContentTypeController extends Controller
 {
-    /**
-     * @var ContentTypeService
-     */
+    /** @var ContentTypeService */
     private $contentTypeService;
 
-    /**
-     * @var ActionDispatcherInterface
-     */
+    /** @var ActionDispatcherInterface */
     private $contentTypeActionDispatcher;
 
     /**

src/bundle/ParamConverter/ContentTypeDraftParamConverter.php

@@ -17,9 +17,7 @@ class ContentTypeDraftParamConverter implements ParamConverterInterface
 {
     const PARAMETER_CONTENT_TYPE_ID = 'contentTypeId';
 
-    /**
-     * @var ContentTypeService
-     */
+    /** @var ContentTypeService */
     private $contentTypeService;
 
     /**

src/bundle/ParamConverter/ContentTypeGroupParamConverter.php

@@ -19,9 +19,7 @@ class ContentTypeGroupParamConverter implements ParamConverterInterface
 {
     const PARAMETER_CONTENT_TYPE_GROUP_ID = 'contentTypeGroupId';
 
-    /**
-     * @var ContentTypeGroupService
-     */
+    /** @var ContentTypeGroupService */
     private $contentTypeGroupService;
 
     /**

src/bundle/ParamConverter/ContentTypeParamConverter.php

@@ -19,9 +19,7 @@ class ContentTypeParamConverter implements ParamConverterInterface
 {
     const PARAMETER_CONTENT_TYPE_ID = 'contentTypeId';
 
-    /**
-     * @var ContentTypeService
-     */
+    /** @var ContentTypeService */
     private $contentTypeService;
 
     /**

src/bundle/ParamConverter/LanguageParamConverter.php

@@ -19,9 +19,7 @@ class LanguageParamConverter implements ParamConverterInterface
 {
     const PARAMETER_LANGUAGE_ID = 'languageId';
 
-    /**
-     * @var LanguageService
-     */
+    /** @var LanguageService */
     private $languageService;
 
     /**

src/bundle/ParamConverter/PolicyParamConverter.php

@@ -20,9 +20,7 @@ class PolicyParamConverter implements ParamConverterInterface
     const PARAMETER_ROLE_ID = 'roleId';
     const PARAMETER_POLICY_ID = 'policyId';
 
-    /**
-     * @var RoleService
-     */
+    /** @var RoleService */
     private $roleService;
 
     /**

src/bundle/ParamConverter/RoleAssignmentParamConverter.php

@@ -17,9 +17,7 @@ class RoleAssignmentParamConverter implements ParamConverterInterface
 {
     const PRAMETER_ROLE_ASSIGNMENT_ID = 'assignmentId';
 
-    /**
-     * @var RoleService
-     */
+    /** @var RoleService */
     private $roleService;
 
     /**

src/bundle/ParamConverter/RoleParamConverter.php

@@ -18,9 +18,8 @@
 class RoleParamConverter implements ParamConverterInterface
 {
     const PARAMETER_ROLE_ID = 'roleId';
-    /**
-     * @var RoleService
-     */
+
+    /** @var RoleService */
     private $roleService;
 
     /**

src/bundle/Resources/config/services.yml

@@ -31,8 +31,8 @@ services:
             - { name: kernel.event_subscriber }
 
     EzSystems\EzPlatformAdminUi\EventListener\RequestListener:
-        calls:
-            - [setContainer, ["@service_container"]]
+        arguments:
+            - '%ezpublish.siteaccess.groups_by_siteaccess%'
         tags:
             - { name: kernel.event_subscriber }
 

src/lib/EventListener/RequestListener.php

@@ -7,7 +7,6 @@
 namespace EzSystems\EzPlatformAdminUi\EventListener;
 
 use eZ\Publish\Core\MVC\Symfony\SiteAccess;
-use Symfony\Component\DependencyInjection\ContainerAwareTrait;
 use Symfony\Component\EventDispatcher\EventSubscriberInterface;
 use Symfony\Component\HttpKernel\Event\GetResponseEvent;
 use Symfony\Component\HttpKernel\Exception\NotFoundHttpException;
@@ -16,7 +15,16 @@
 
 class RequestListener implements EventSubscriberInterface
 {
-    use ContainerAwareTrait;
+    /** @var array */
+    private $groupsBySiteAccess;
+
+    /**
+     * @param $groupsBySiteAccess
+     */
+    public function __construct(array $groupsBySiteAccess)
+    {
+        $this->groupsBySiteAccess = $groupsBySiteAccess;
+    }
 
     /**
      * Returns an array of event names this subscriber wants to listen to.
@@ -36,7 +44,7 @@ class RequestListener implements EventSubscriberInterface
      *
      * @return array The event names to listen to
      */
-    public static function getSubscribedEvents()
+    public static function getSubscribedEvents(): array
     {
         return [
             KernelEvents::REQUEST => ['onKernelRequest', 13],
@@ -58,12 +66,10 @@ public function onKernelRequest(GetResponseEvent $event)
             return;
         }
 
-        $allowedGroups = is_array($allowedGroups) ?: [$allowedGroups];
-
-        $groups = $this->container->getParameter('ezpublish.siteaccess.groups_by_siteaccess');
+        $allowedGroups = (array)$allowedGroups;
 
-        foreach ($groups[$siteAccess->name] as $group) {
-            if (in_array($group, $allowedGroups)) {
+        foreach ($this->groupsBySiteAccess[$siteAccess->name] as $group) {
+            if (in_array($group, $allowedGroups, true)) {
                 return;
             }
         }

src/lib/Form/Data/Trash/TrashItemRestoreData.php

@@ -24,9 +24,7 @@ class TrashItemRestoreData
      */
     public $trashItems;
 
-    /**
-     * @var Location|null
-     */
+    /** @var Location|null */
     public $location;
 
     /**

src/lib/Form/Type/Policy/PolicyChoiceType.php

@@ -17,9 +17,7 @@
 
 class PolicyChoiceType extends AbstractType
 {
-    /**
-     * @var array
-     */
+    /** @var array */
     private $policyChoices;
 
     /**

src/lib/Form/Type/Section/SectionChoiceType.php

@@ -15,9 +15,7 @@
 
 class SectionChoiceType extends AbstractType
 {
-    /**
-     * @var SectionService
-     */
+    /** @var SectionService */
     private $sectionService;
 
     /**

src/lib/Form/Type/Trash/TrashItemChoiceType.php

@@ -14,9 +14,7 @@
 
 class TrashItemChoiceType extends AbstractType
 {
-    /**
-     * @var TrashService
-     */
+    /** @var TrashService */
     private $trashService;
 
     /**

src/lib/Service/PathService.php

@@ -17,9 +17,7 @@
  */
 class PathService
 {
-    /**
-     * @var SearchService
-     */
+    /** @var SearchService */
     private $searchService;
 
     public function __construct(SearchService $searchService)

src/lib/Tests/EventListener/RequestListenerTest.php

@@ -0,0 +1,101 @@
+<?php
+
+/**
+ * @copyright Copyright (C) eZ Systems AS. All rights reserved.
+ * @license For full copyright and license information view LICENSE file distributed with this source code.
+ */
+namespace EzSystems\EzPlatformAdminUi\Tests\EventListener;
+
+use eZ\Publish\Core\MVC\Symfony\SiteAccess;
+use Symfony\Component\HttpFoundation\Request;
+use Symfony\Component\HttpKernel\Event\GetResponseEvent;
+use Symfony\Component\HttpKernel\HttpKernelInterface;
+use Symfony\Component\HttpKernel\KernelEvents;
+use PHPUnit\Framework\TestCase;
+use EzSystems\EzPlatformAdminUi\EventListener\RequestListener;
+use Symfony\Component\HttpKernel\Exception\NotFoundHttpException;
+use PHPUnit_Framework_MockObject_MockObject as MockObject;
+
+class RequestListenerTest extends TestCase
+{
+    /** @var RequestListener */
+    private $requestListener;
+
+    /** @var Request */
+    private $request;
+
+    /** @var GetResponseEvent */
+    private $event;
+
+    /** @var HttpKernelInterface|MockObject */
+    private $httpKernel;
+
+    protected function setUp()
+    {
+        parent::setUp();
+
+        $this->requestListener = new RequestListener(['some_name' => ['group_1']]);
+
+        $this->request = $this
+            ->getMockBuilder(Request::class)
+            ->setMethods(['getSession', 'hasSession'])
+            ->getMock();
+
+        $this->httpKernel = $this->createMock(HttpKernelInterface::class);
+
+        $this->event = new GetResponseEvent(
+            $this->httpKernel,
+            $this->request,
+            HttpKernelInterface::MASTER_REQUEST
+        );
+    }
+
+    public function testOnKernelRequestDeniedAccess()
+    {
+        $this->expectException(NotFoundHttpException::class);
+        $this->expectExceptionMessage('Route is not allowed in current siteaccess');
+
+        $this->request->attributes->set('siteaccess', new SiteAccess('some_name'));
+        $this->request->attributes->set('siteaccess_group_whitelist', ['group_2', 'group_3']);
+
+        $this->requestListener->onKernelRequest($this->event);
+    }
+
+    public function testOnKernelRequestAllowAccessWithSubRequest()
+    {
+        $this->event = new GetResponseEvent(
+            $this->httpKernel,
+            $this->request,
+            HttpKernelInterface::SUB_REQUEST
+        );
+
+        $this->assertNull($this->requestListener->onKernelRequest($this->event));
+    }
+
+    public function testOnKernelRequestAllowAccessWithoutSiteAccess()
+    {
+        $this->request->attributes->set('siteaccess', 'not_siteaccess_object');
+
+        $this->assertNull($this->requestListener->onKernelRequest($this->event));
+    }
+
+    public function testOnKernelRequestAllowAccessWithoutGroupWhitelist()
+    {
+        $this->request->attributes->set('siteaccess_group_whitelist', null);
+
+        $this->assertNull($this->requestListener->onKernelRequest($this->event));
+    }
+
+    public function testOnKernelRequestAllowAccessWhenGroupMatch()
+    {
+        $this->request->attributes->set('siteaccess', new SiteAccess('some_name'));
+        $this->request->attributes->set('siteaccess_group_whitelist', ['group_1', 'group_2']);
+
+        $this->assertNull($this->requestListener->onKernelRequest($this->event));
+    }
+
+    public function testSubscribedEvents()
+    {
+        $this->assertSame([KernelEvents::REQUEST => ['onKernelRequest', 13]], $this->requestListener::getSubscribedEvents());
+    }
+}

src/lib/UI/Value/Content/VersionInfo.php

@@ -19,9 +19,7 @@
  */
 class VersionInfo extends CoreVersionInfo
 {
-    /**
-     * @var User
-     */
+    /** @var User */
     protected $author;
 
     /**