Merge pull request #5 from net02/dev

Lazily inject request content into operation manager
matteosister · Oct 30, 2019 · 496818b · 496818b
2 parents ccc574e + 87c3eb9
commit 496818b
Show file tree

Hide file tree

Showing 6 changed files with 53 additions and 31 deletions.
diff --git a/src/PatchManager/Bundle/RequestAdapter/RequestStackAdapter.php b/src/PatchManager/Bundle/RequestAdapter/RequestStackAdapter.php
@@ -22,10 +22,15 @@ public function __construct(RequestStack $requestStack)
     }
 
     /**
-     * @param Operations $operations
+     * @return null|string
      */
-    public function setRequestBody(Operations $operations)
+    public function getRequestBody(): ?string
     {
-        $operations->setRequestBody($this->requestStack->getCurrentRequest()->getContent());
+        $request = $this->requestStack->getCurrentRequest();
+        if (is_null($request)) {
+            return null;
+        }
+
+        return $request->getContent();
     }
 }
diff --git a/src/PatchManager/Bundle/Resources/config/services.xml b/src/PatchManager/Bundle/Resources/config/services.xml
@@ -9,7 +9,7 @@
             <argument type="service" id="request_stack" />
         </service>
         <service id="patch_manager.operations" public="false" class="Cypress\PatchManager\Request\Operations">
-            <configurator service="patch_manager.request_adapter" method="setRequestBody" />
+            <argument type="service" id="patch_manager.request_adapter" />
         </service>
         <service id="patch_manager.operation_matcher" public="false" class="Cypress\PatchManager\OperationMatcher">
             <argument type="service" id="patch_manager.operations" />

diff --git a/src/PatchManager/Request/Adapter.php b/src/PatchManager/Request/Adapter.php
@@ -10,8 +10,7 @@
 interface Adapter
 {
     /**
-     * @param Operations $operations
-     * @return void
+     * @return null|string
      */
-    public function setRequestBody(Operations $operations);
+    public function getRequestBody(): ?string;
 }
diff --git a/src/PatchManager/Request/Operations.php b/src/PatchManager/Request/Operations.php
@@ -5,23 +5,24 @@
 use Cypress\PatchManager\Exception\InvalidJsonRequestContent;
 use Cypress\PatchManager\Exception\MissingOperationNameRequest;
 use Cypress\PatchManager\Exception\MissingOperationRequest;
+use Cypress\PatchManager\Request\Adapter;
 use PhpCollection\Sequence;
 
 class Operations
 {
     const OP_KEY_NAME = 'op';
 
     /**
-     * @var string
+     * @var Adapter
      */
-    private $requestBody;
+    private $adapter;
 
     /**
-     * @param string $requestBody
+     * @param Adapter $adapter
      */
-    public function setRequestBody($requestBody)
+    public function __construct(Adapter $adapter)
     {
-        $this->requestBody = $requestBody;
+        $this->adapter = $adapter;
     }
 
     /**
@@ -36,7 +37,7 @@ public function setRequestBody($requestBody)
     private function parseJson($string)
     {
         $parsedContent = json_decode($string, true);
-        if (json_last_error() !== JSON_ERROR_NONE) {
+        if (JSON_ERROR_NONE !== json_last_error()) {
             throw new InvalidJsonRequestContent;
         }
         return $parsedContent;
@@ -60,13 +61,13 @@ private function isAssociative($arr)
      */
     public function all()
     {
-        $operations =$this->parseJson($this->requestBody);
-        if (! is_array($operations)) {
+        $operations = $this->parseJson($this->adapter->getRequestBody());
+        if (!is_array($operations)) {
             throw new MissingOperationRequest();
         }
         $operations = new Sequence($this->isAssociative($operations) ? array($operations) : $operations);
         $operationsWithoutOpKey = $operations->filterNot($this->operationWithKey());
-        if (! $operationsWithoutOpKey->isEmpty()) {
+        if (!$operationsWithoutOpKey->isEmpty()) {
             /** @var array $operationData */
             $operationData = $operationsWithoutOpKey->first()->get();
             throw new MissingOperationNameRequest($operationData);

diff --git a/tests/PatchManager/Bundle/RequestAdapter/RequestStackAdapterTest.php b/tests/PatchManager/Bundle/RequestAdapter/RequestStackAdapterTest.php
@@ -16,12 +16,11 @@ public function test_call()
         $requestStack->getCurrentRequest()->willReturn($currentRequest->reveal());
         $adapter = new RequestStackAdapter($requestStack->reveal());
 
-        $operations = new Operations();
-        $adapter->setRequestBody($operations);
+        $operations = new Operations($adapter);
 
         $this->assertCount(1, $operations->all());
 
         $first = $operations->all()->get(0);
         $this->assertEquals('data', $first['op']);
     }
-} 
+}
diff --git a/tests/PatchManager/Request/OperationsTest.php b/tests/PatchManager/Request/OperationsTest.php
@@ -4,6 +4,7 @@
 
 use Cypress\PatchManager\Exception\MissingOperationNameRequest;
 use Cypress\PatchManager\Tests\PatchManagerTestCase;
+use Cypress\PatchManager\Request\Adapter;
 use Cypress\PatchManager\Request\Operations;
 use Mockery as m;
 
@@ -14,24 +15,38 @@ class OperationsTest extends PatchManagerTestCase
      */
     public function test_request_with_invalid_json()
     {
-        $operations = new Operations();
-        $operations->setRequestBody('{"test": error}');
+        $adapter = $this->prophesize(Adapter::class);
+        $adapter->getRequestBody()->willReturn('{"test": error}');
+        $operations = new Operations($adapter->reveal());
+        $operations->all();
+    }
+
+    /**
+     * @expectedException \Cypress\PatchManager\Exception\InvalidJsonRequestContent
+     */
+    public function test_exeception_with_null_request()
+    {
+        $adapter = $this->prophesize(Adapter::class);
+        $adapter->getRequestBody()->willReturn(null);
+        $operations = new Operations($adapter->reveal());
         $operations->all();
     }
 
     public function test_correct_operations_number_with_one_operation()
     {
-        $operations = new Operations();
-        $operations->setRequestBody('{"op": "data"}');
+        $adapter = $this->prophesize(Adapter::class);
+        $adapter->getRequestBody()->willReturn('{"op": "data"}');
+        $operations = new Operations($adapter->reveal());
         $this->assertCount(1, $operations->all());
         $op = $operations->all()->get(0);
         $this->assertEquals('data', $op['op']);
     }
 
     public function test_correct_operations_number_with_multiple_operation()
     {
-        $operations = new Operations();
-        $operations->setRequestBody('[{"op": "data"},{"op": "data2"}]');
+        $adapter = $this->prophesize(Adapter::class);
+        $adapter->getRequestBody()->willReturn('[{"op": "data"},{"op": "data2"}]');
+        $operations = new Operations($adapter->reveal());
         $this->assertCount(2, $operations->all());
         $op1 = $operations->all()->get(0);
         $this->assertEquals('data', $op1['op']);
@@ -44,8 +59,9 @@ public function test_correct_operations_number_with_multiple_operation()
      */
     public function test_exeception_with_empty_request()
     {
-        $operations = new Operations();
-        $operations->setRequestBody('""');
+        $adapter = $this->prophesize(Adapter::class);
+        $adapter->getRequestBody()->willReturn('""');
+        $operations = new Operations($adapter->reveal());
         $operations->all();
     }
 
@@ -54,8 +70,9 @@ public function test_exeception_with_empty_request()
      */
     public function test_exeception_with_operation_without_op()
     {
-        $operations = new Operations();
-        $operations->setRequestBody('[{"op_wrong": "data"}]');
+        $adapter = $this->prophesize(Adapter::class);
+        $adapter->getRequestBody()->willReturn('[{"op_wrong": "data"}]');
+        $operations = new Operations($adapter->reveal());
         $operations->all();
     }
 
@@ -64,8 +81,9 @@ public function test_exeception_with_operation_without_op()
      */
     public function test_exeception_with_multiple_operation_without_op()
     {
-        $operations = new Operations();
-        $operations->setRequestBody('[{"op": "data"},{"op_wrong": "data"}]');
+        $adapter = $this->prophesize(Adapter::class);
+        $adapter->getRequestBody()->willReturn('[{"op": "data"},{"op_wrong": "data"}]');
+        $operations = new Operations($adapter->reveal());
         $operations->all();
     }
 }