Added checkEntityIdMatchesRoute method and implemented it. Updated te…

…sts to match expectations. Reordered method params to be consistent.
spira · Aug 20, 2015 · cc4564e · cc4564e
1 parent 177df20
commit cc4564e
Show file tree

Hide file tree

Showing 6 changed files with 55 additions and 14 deletions.
diff --git a/api/app/Extensions/Controller/RequestValidationTrait.php b/api/app/Extensions/Controller/RequestValidationTrait.php
@@ -8,8 +8,11 @@
 
 namespace App\Extensions\Controller;
 
+use App\Exceptions\BadRequestException;
 use Illuminate\Database\Eloquent\Collection;
+use Illuminate\Http\Request;
 use Laravel\Lumen\Routing\ValidatesRequests;
+use Spira\Model\Model\BaseModel;
 use Spira\Model\Validation\ValidationException;
 use Spira\Model\Validation\ValidationExceptionCollection;
 use Spira\Model\Validation\Validator;
@@ -139,4 +142,32 @@ public function validateRequestCollection($requestCollection, $validationRules,
 
         return true;
     }
+
+
+
+    /**
+     * @param Request $request
+     * @param $id
+     * @param BaseModel $model
+     * @param bool|true $requireEntityKey
+     * @return bool
+     */
+    protected function checkEntityIdMatchesRoute(Request $request, $id, BaseModel $model, $requireEntityKey = true)
+    {
+        $keyName = $model->getKeyName();
+        if (!$request->has($keyName)){
+            if (!$requireEntityKey){
+                return true; //it is ok if the key is not set (for patch requests etc)
+            }else{
+                throw new BadRequestException("Request entity must include entity id ($keyName) for ".get_class($model));
+            }
+        }
+
+        if ($request->input($keyName) !== $id){
+            throw new BadRequestException("Provided entity body does not match route parameter. The entity key cannot be updated");
+        }
+
+        return true;
+    }
+
 }
diff --git a/api/app/Http/Controllers/ChildEntityController.php b/api/app/Http/Controllers/ChildEntityController.php
@@ -99,7 +99,7 @@ public function getOne(Request $request, $id, $childId)
      * @throws \Exception
      * @throws \Exception|null
      */
-    public function postOne($id, Request $request)
+    public function postOne(Request $request, $id)
     {
         $parent = $this->findParentEntity($id);
         $childModel = $this->getChildModel()->newInstance();
@@ -122,11 +122,13 @@ public function postOne($id, Request $request)
      * @param  Request $request
      * @return ApiResponse
      */
-    public function putOne($id, $childId, Request $request)
+    public function putOne(Request $request, $id, $childId)
     {
+
         $parent = $this->findParentEntity($id);
-        $childModel = $this->findOrNewChildEntity($childId, $parent);
 
+        $this->checkEntityIdMatchesRoute($request, $childId, $this->getChildModel());
+        $childModel = $this->findOrNewChildEntity($childId, $parent);
 
         $this->validateRequest($request->all(), $this->addIdOverrideValidationRule($this->getValidationRules(), $childId));
 
@@ -145,7 +147,7 @@ public function putOne($id, $childId, Request $request)
      * @param  Request $request
      * @return ApiResponse
      */
-    public function putMany($id, Request $request)
+    public function putMany(Request $request, $id)
     {
         $parent = $this->findParentEntity($id);
 
@@ -172,9 +174,11 @@ public function putMany($id, Request $request)
      * @param  Request $request
      * @return ApiResponse
      */
-    public function patchOne($id, $childId, Request $request)
+    public function patchOne(Request $request, $id, $childId)
     {
         $parent = $this->findParentEntity($id);
+
+        $this->checkEntityIdMatchesRoute($request, $childId, $this->getChildModel(), false);
         $childModel = $this->findOrFailChildEntity($childId, $parent);
 
         $validationRules = $this->addIdOverrideValidationRule($this->getValidationRules(), $childId);
@@ -193,7 +197,7 @@ public function patchOne($id, $childId, Request $request)
      * @param  Request $request
      * @return ApiResponse
      */
-    public function patchMany($id, Request $request)
+    public function patchMany(Request $request, $id)
     {
         $requestCollection = $request->data;
         $this->validateRequestCollection($requestCollection, $this->getValidationRules(), true);
@@ -233,7 +237,7 @@ public function deleteOne($id, $childId)
      * @param  Request  $request
      * @return ApiResponse
      */
-    public function deleteMany($id, Request $request)
+    public function deleteMany(Request $request, $id)
     {
         $requestCollection = $request->data;
         $model = $this->findParentEntity($id);

diff --git a/api/app/Http/Controllers/EntityController.php b/api/app/Http/Controllers/EntityController.php
@@ -107,8 +107,10 @@ public function postOne(Request $request)
      * @param  Request  $request
      * @return ApiResponse
      */
-    public function putOne($id, Request $request)
+    public function putOne(Request $request, $id)
     {
+        $this->checkEntityIdMatchesRoute($request, $id, $this->getModel());
+
         $model = $this->findOrNewEntity($id);
 
         $validationRules = $this->addIdOverrideValidationRule($this->getValidationRules(), $id);
@@ -153,8 +155,10 @@ public function putMany(Request $request)
      * @param  Request  $request
      * @return ApiResponse
      */
-    public function patchOne($id, Request $request)
+    public function patchOne(Request $request, $id)
     {
+        $this->checkEntityIdMatchesRoute($request, $id, $this->getModel(), false);
+
         $model = $this->findOrFailEntity($id);
 
         $validationRules = $this->addIdOverrideValidationRule($this->getValidationRules(), $id);

diff --git a/api/tests/integration/ArticleTest.php b/api/tests/integration/ArticleTest.php
@@ -204,7 +204,7 @@ public function testPutMissingIdInBody()
         $this->put('/articles/'.$id, $preparedEntity);
         $this->shouldReturnJson();
 
-        $this->assertResponseStatus(422);
+        $this->assertResponseStatus(400);
     }
 
 

diff --git a/api/tests/integration/ChildEntityTest.php b/api/tests/integration/ChildEntityTest.php
@@ -147,10 +147,11 @@ public function testPutOneCollidingIds()
 
         $object = json_decode($this->response->getContent());
 
-        $this->assertResponseStatus(422);
+        $this->assertResponseStatus(400);
         $this->assertTrue(is_object($object));
 
-        $this->assertEquals('The entity id can not be changed.', $object->invalid->entityId[0]->message);
+        $this->assertObjectHasAttribute('message', $object);
+        $this->assertEquals("Provided entity body does not match route parameter. The entity key cannot be updated", $object->message);
     }
 
     public function testPutOneNewInvalidId()

diff --git a/api/tests/integration/EntityTest.php b/api/tests/integration/EntityTest.php
@@ -364,10 +364,11 @@ public function testPutOneCollidingIds()
 
         $object = json_decode($this->response->getContent());
 
-        $this->assertResponseStatus(422);
+        $this->assertResponseStatus(400);
         $this->assertTrue(is_object($object));
 
-        $this->assertEquals('The entity id can not be changed.', $object->invalid->entityId[0]->message);
+        $this->assertObjectHasAttribute('message', $object);
+        $this->assertEquals("Provided entity body does not match route parameter. The entity key cannot be updated", $object->message);
     }
 
     public function testPutOneNewInvalidId()