Merge pull request #221 from fbourigault/v4-api-8793

fbourigault · web-flow · commit ea1b976fede7 · 2017-08-01T12:31:07.000+02:00
Update MergeRequestsApi for V4 compatibility
diff --git a/UPGRADE.md b/UPGRADE.md
@@ -23,6 +23,13 @@ See [documentation](doc/customize.md) to know how to customize the client timeou
 * The second argument of `update`, `remove`, `showComments`, `showComment`, `addComment`, `updateComment`, `removeComment`,
  `setTimeEstimate`, `resetTimeEstimate`, `addSpentTime` and `resetSpentTime` methods is now a scoped issue id (iid).
 
+## `Gitlab\Api\MergeRequests` changes
+
+* The `getList`, `getByIid`, `merged`, `opened` and `closed` methods have been removed. Use `all` method instead.
+* The `ORDER_BY` and `SORT` class constants have been removed.
+* The `all` method now take a single argument which is an associative array of query string parameters.
+* The `getNotes` method now take only two arguments, the project id and the merge request iid.
+
 ## `Gitlab\Api\Projects` changes
 
 * The `keys`, `key`, `addKey`, `removeKey`, `disableKey` and `enableKey` methods have been removed.
diff --git a/lib/Gitlab/Api/AbstractApi.php b/lib/Gitlab/Api/AbstractApi.php
@@ -1,7 +1,9 @@
 <?php namespace Gitlab\Api;
 
 use Gitlab\Client;
+use Gitlab\HttpClient\Message\QueryStringBuilder;
 use Gitlab\HttpClient\Message\ResponseMediator;
+use Gitlab\Tests\HttpClient\Message\QueryStringBuilderTest;
 use Http\Discovery\StreamFactoryDiscovery;
 use Http\Message\MultipartStream\MultipartStreamBuilder;
 use Http\Message\StreamFactory;
@@ -216,8 +218,9 @@ protected function createOptionsResolver()
 
     private function preparePath($path, array $parameters = [])
     {
+
         if (count($parameters) > 0) {
-            $path .= '?'.http_build_query($parameters);
+            $path .= '?'.QueryStringBuilder::build($parameters);
         }
 
         return $path;
diff --git a/lib/Gitlab/Api/MergeRequests.php b/lib/Gitlab/Api/MergeRequests.php
@@ -1,86 +1,71 @@
 <?php namespace Gitlab\Api;
 
+use Symfony\Component\OptionsResolver\Exception\InvalidOptionsException;
+use Symfony\Component\OptionsResolver\Exception\UndefinedOptionsException;
+
 class MergeRequests extends AbstractApi
 {
     const STATE_ALL = 'all';
     const STATE_MERGED = 'merged';
     const STATE_OPENED = 'opened';
     const STATE_CLOSED = 'closed';
 
-    const ORDER_BY = 'created_at';
-    const SORT = 'asc';
-
-    /**
-     * @param int $project_id
-     * @param string $state
-     * @param int $page
-     * @param int $per_page
-     * @param string $order_by
-     * @param string $sort
-     * @param string $object
-     * @return mixed
-     */
-    public function getList($project_id, $state = self::STATE_ALL, $page = 1, $per_page = self::PER_PAGE, $order_by = self::ORDER_BY, $sort = self::SORT, $object = 'merge_requests')
-    {
-        return $this->get($this->getProjectPath($project_id, $object), array(
-            'page' => $page,
-            'per_page' => $per_page,
-            'state' => $state,
-            'order_by' => $order_by,
-            'sort' => $sort
-        ));
-    }
-
     /**
-     * @param int $project_id
-     * @param int $page
-     * @param int $per_page
-     * @param string $order_by
-     * @param string $sort
-     * @return mixed
-     */
-    public function all($project_id, $page = 1, $per_page = self::PER_PAGE, $order_by = self::ORDER_BY, $sort = self::SORT)
-    {
-        return $this->getList($project_id, self::STATE_ALL, $page, $per_page, $order_by, $sort);
-    }
-
-    /**
-     * @param int $project_id
-     * @param int $page
-     * @param int $per_page
-     * @param string $order_by
-     * @param string $sort
-     * @return mixed
-     */
-    public function merged($project_id, $page = 1, $per_page = self::PER_PAGE, $order_by = self::ORDER_BY, $sort = self::SORT)
-    {
-        return $this->getList($project_id, self::STATE_MERGED, $page, $per_page, $order_by, $sort);
-    }
-
-    /**
-     * @param int $project_id
-     * @param int $page
-     * @param int $per_page
-     * @param string $order_by
-     * @param string $sort
-     * @return mixed
-     */
-    public function opened($project_id, $page = 1, $per_page = self::PER_PAGE, $order_by = self::ORDER_BY, $sort = self::SORT)
-    {
-        return $this->getList($project_id, self::STATE_OPENED, $page, $per_page, $order_by, $sort);
-    }
-
-    /**
-     * @param int $project_id
-     * @param int $page
-     * @param int $per_page
-     * @param string $order_by
-     * @param string $sort
+     * @param int   $project_id
+     * @param array $parameters {
+     *
+     *     @var int[]              $iids           Return the request having the given iid.
+     *     @var string             $state          Return all merge requests or just those that are opened, closed, or
+     *                                             merged.
+     *     @var string             $order_by       Return requests ordered by created_at or updated_at fields. Default
+     *                                             is created_at.
+     *     @var string             $sort           Return requests sorted in asc or desc order. Default is desc.
+     *     @var string             $milestone      Return merge requests for a specific milestone.
+     *     @var string             $view           If simple, returns the iid, URL, title, description, and basic state
+     *                                             of merge request.
+     *     @var string             $labels         Return merge requests matching a comma separated list of labels.
+     *     @var \DateTimeInterface $created_after  Return merge requests created after the given time (inclusive).
+     *     @var \DateTimeInterface $created_before Return merge requests created before the given time (inclusive).
+     * }
+     *
+     * @throws UndefinedOptionsException If an option name is undefined.
+     * @throws InvalidOptionsException   If an option doesn't fulfill the specified validation rules.
+     *
      * @return mixed
      */
-    public function closed($project_id, $page = 1, $per_page = self::PER_PAGE, $order_by = self::ORDER_BY, $sort = self::SORT)
-    {
-        return $this->getList($project_id, self::STATE_CLOSED, $page, $per_page, $order_by, $sort);
+    public function all($project_id, array $parameters = [])
+    {
+        $resolver = $this->createOptionsResolver();
+        $datetimeNormalizer = function (\DateTimeInterface $value) {
+            return $value->format('c');
+        };
+        $resolver->setDefined('iids')
+            ->setAllowedTypes('iids', 'array')
+            ->setAllowedValues('iids', function (array $value) {
+                return count($value) == count(array_filter($value, 'is_int'));
+            })
+        ;
+        $resolver->setDefined('state')
+            ->setAllowedValues('state', ['all', 'opened', 'merged', 'closed'])
+        ;
+        $resolver->setDefined('order_by')
+            ->setAllowedValues('order_by', ['created_at', 'updated_at'])
+        ;
+        $resolver->setDefined('milestone');
+        $resolver->setDefined('view')
+            ->setAllowedValues('view', ['simple'])
+        ;
+        $resolver->setDefined('labels');
+        $resolver->setDefined('created_after')
+            ->setAllowedTypes('created_after', \DateTimeInterface::class)
+            ->setNormalizer('created_after', $datetimeNormalizer)
+        ;
+        $resolver->setDefined('created_before')
+            ->setAllowedTypes('created_before', \DateTimeInterface::class)
+            ->setNormalizer('created_before', $datetimeNormalizer)
+        ;
+
+        return $this->get($this->getProjectPath($project_id, 'merge_requests'), $resolver->resolve($parameters));
     }
 
     /**
@@ -90,7 +75,7 @@ public function closed($project_id, $page = 1, $per_page = self::PER_PAGE, $orde
      */
     public function show($project_id, $mr_id)
     {
-        return $this->get($this->getProjectPath($project_id, 'merge_request/'.$this->encodePath($mr_id)));
+        return $this->get($this->getProjectPath($project_id, 'merge_requests/'.$this->encodePath($mr_id)));
     }
 
     /**
@@ -123,7 +108,7 @@ public function create($project_id, $source, $target, $title, $assignee = null,
      */
     public function update($project_id, $mr_id, array $params)
     {
-        return $this->put($this->getProjectPath($project_id, 'merge_request/'.$this->encodePath($mr_id)), $params);
+        return $this->put($this->getProjectPath($project_id, 'merge_requests/'.$this->encodePath($mr_id)), $params);
     }
 
     /**
@@ -140,17 +125,18 @@ public function merge($project_id, $mr_id, $message = null)
             $params = array('merge_commit_message' => $message);
         }
 
-        return $this->put($this->getProjectPath($project_id, 'merge_request/'.$this->encodePath($mr_id).'/merge'), $params);
+        return $this->put($this->getProjectPath($project_id, 'merge_requests/'.$this->encodePath($mr_id).'/merge'), $params);
     }
 
     /**
-     * @param int $project_id
-     * @param int $mr_id
+     * @param int   $project_id
+     * @param int   $mr_id
+     *
      * @return mixed
      */
-    public function showNotes($project_id, $mr_id, $page = 1, $per_page = self::PER_PAGE, $order_by = self::ORDER_BY, $sort = 'desc')
+    public function showNotes($project_id, $mr_id)
     {
-        return $this->getList($project_id, null, $page, $per_page, $order_by, $sort, 'merge_requests/'.$this->encodePath($mr_id).'/notes');
+        return $this->get($this->getProjectPath($project_id, 'merge_requests/'.$this->encodePath($mr_id).'/notes'));
     }
 
     /**
@@ -173,7 +159,7 @@ public function addNote($project_id, $mr_id, $note)
      */
     public function showComments($project_id, $mr_id)
     {
-        return $this->get($this->getProjectPath($project_id, 'merge_request/'.$this->encodePath($mr_id).'/comments'));
+        return $this->get($this->getProjectPath($project_id, 'merge_requests/'.$this->encodePath($mr_id).'/comments'));
     }
 
     /**
@@ -184,7 +170,7 @@ public function showComments($project_id, $mr_id)
      */
     public function addComment($project_id, $mr_id, $note)
     {
-        return $this->post($this->getProjectPath($project_id, 'merge_request/'.$this->encodePath($mr_id).'/comments'), array(
+        return $this->post($this->getProjectPath($project_id, 'merge_requests/'.$this->encodePath($mr_id).'/comments'), array(
             'note' => $note
         ));
     }
@@ -196,17 +182,7 @@ public function addComment($project_id, $mr_id, $note)
      */
     public function changes($project_id, $mr_id)
     {
-        return $this->get($this->getProjectPath($project_id, 'merge_request/'.$this->encodePath($mr_id).'/changes'));
-    }
-
-    /**
-     * @param $project_id
-     * @param $mr_iid
-     * @return mixed
-     */
-    public function getByIid($project_id, $mr_iid)
-    {
-        return $this->get($this->getProjectPath($project_id, 'merge_requests'), array('iid' => $mr_iid));
+        return $this->get($this->getProjectPath($project_id, 'merge_requests/'.$this->encodePath($mr_id).'/changes'));
     }
 
     /**
@@ -216,7 +192,7 @@ public function getByIid($project_id, $mr_iid)
      */
     public function commits($project_id, $mr_id)
     {
-        return $this->get($this->getProjectPath($project_id, 'merge_request/'.$this->encodePath($mr_id).'/commits'));
+        return $this->get($this->getProjectPath($project_id, 'merge_requests/'.$this->encodePath($mr_id).'/commits'));
     }
 
     /**
@@ -226,7 +202,7 @@ public function commits($project_id, $mr_id)
      */
     public function closesIssues($project_id, $mr_id)
     {
-        return $this->get($this->getProjectPath($project_id, 'merge_request/'.$this->encodePath($mr_id).'/closes_issues'));
+        return $this->get($this->getProjectPath($project_id, 'merge_requests/'.$this->encodePath($mr_id).'/closes_issues'));
     }
 
     /**
diff --git a/lib/Gitlab/HttpClient/Message/QueryStringBuilder.php b/lib/Gitlab/HttpClient/Message/QueryStringBuilder.php
@@ -0,0 +1,60 @@
+<?php
+
+namespace Gitlab\HttpClient\Message;
+
+final class QueryStringBuilder
+{
+    /**
+     * Encode a query as a query string according to RFC 3986. Indexed arrays are encoded using
+     * empty squared brackets ([]) unlike http_build_query.
+     *
+     * @param mixed $query
+     *
+     * @return string
+     */
+    public static function build($query)
+    {
+        if (!is_array($query)) {
+            return rawurlencode($query);
+        }
+        return implode('&', array_map(function ($value, $key) {
+            return static::encode($value, $key);
+        }, $query, array_keys($query)));
+    }
+
+    /**
+     * Encode a value
+     * @param mixed $query
+     * @param string $prefix
+     *
+     * @return string
+     */
+    private static function encode($query, $prefix)
+    {
+        if (!is_array($query)) {
+            return rawurlencode($prefix).'='.rawurlencode($query);
+        }
+
+        $isIndexedArray = static::isIndexedArray($query);
+        return implode('&', array_map(function ($value, $key) use ($prefix, $isIndexedArray) {
+            $prefix = $isIndexedArray ? $prefix.'[]' : $prefix.'['.$key.']';
+            return static::encode($value, $prefix);
+        }, $query, array_keys($query)));
+    }
+
+    /**
+     * Tell if the given array is an indexed one (i.e. contains only sequential integer keys starting from 0).
+     *
+     * @param array $query
+     *
+     * @return bool
+     */
+    public static function isIndexedArray(array $query)
+    {
+        if (empty($query) || !isset($query[0])) {
+            return false;
+        }
+
+        return array_keys($query) === range(0, count($query) - 1);
+    }
+}
diff --git a/test/Gitlab/Tests/Api/MergeRequestsTest.php b/test/Gitlab/Tests/Api/MergeRequestsTest.php
diff --git a/test/Gitlab/Tests/HttpClient/Message/QueryStringBuilderTest.php b/test/Gitlab/Tests/HttpClient/Message/QueryStringBuilderTest.php
