Api: substituteUrlParameters() replaced by expandColonParameters() [BC break]

Author: milo

src/Github/Api.php

@@ -216,13 +216,9 @@ public function request(Http\Request $request)
 	 */
 	public function createRequest($method, $urlPath, array $parameters = [], array $headers = [], $content = NULL)
 	{
-		$parameters += $this->defaultParameters;
-		$this->substituteUrlParameters($urlPath, $parameters);
+		$urlPath = $this->expandColonParameters($urlPath, $parameters, $this->defaultParameters);
 
-		$url = rtrim($this->url, '/') . '/' . trim($urlPath, '/');
-		if (count($parameters)) {
-			$url .= '?' . http_build_query($parameters);
-		}
+		$url = rtrim($this->url, '/') . '/' . ltrim($urlPath, '/');
 
 		if ($content !== NULL && (is_array($content) || is_object($content))) {
 			$headers['Content-Type'] = 'application/json; charset=utf-8';
@@ -339,21 +335,30 @@ public function getUrl()
 
 	/**
 	 * @param  string
-	 * @param  array
+	 * @return string
 	 *
 	 * @throws MissingParameterException
 	 */
-	protected function substituteUrlParameters(& $url, array & $parameters)
+	protected function expandColonParameters($url, array $parameters, array $defaultParameters)
 	{
+		$parameters += $defaultParameters;
+
 		$url = preg_replace_callback('#(^|/):([^/]+)#', function($m) use ($url, & $parameters) {
 			if (!isset($parameters[$m[2]])) {
 				throw new MissingParameterException("Missing parameter '$m[2]' for URL path '$url'.");
 			}
-
 			$parameter = $parameters[$m[2]];
 			unset($parameters[$m[2]]);
 			return $m[1] . rawurlencode($parameter);
 		}, $url);
+
+		$url = rtrim($url, '/');
+
+		if (count($parameters)) {
+			$url .= '?' . http_build_query($parameters);
+		}
+
+		return $url;
 	}
 
 }

tests/Github/Api.expandColonParameters.phpt

@@ -0,0 +1,61 @@
+<?php
+
+/**
+ * @author  Miloslav Hůla
+ */
+
+require __DIR__ . '/../bootstrap.php';
+
+
+class TestApi extends Milo\Github\Api
+{
+	public function expandColonParameters($url, array $parameters, array $defaultParameters = [])
+	{
+		return parent::expandColonParameters($url, $parameters, $defaultParameters);
+	}
+}
+
+
+# URL parameters like :name
+test(function() {
+	$api = new TestApi;
+
+	$urls = [
+		'' => '?a=A&b=B',
+		'/' => '?a=A&b=B',
+		':a' => 'A?b=B',
+		'/:a' => '/A?b=B',
+		':a/' => 'A?b=B',
+		'/:a/' => '/A?b=B',
+		'/:a/:b/c' => '/A/B/c',
+	];
+
+	foreach ($urls as $url => $expected) {
+		Assert::same($expected, $api->expandColonParameters($url, ['a' => 'A', 'b' => 'B']));
+	}
+
+	Assert::exception(function() use ($api) {
+		$api->expandColonParameters(':a', ['A' => 'a']);
+	}, 'Milo\Github\MissingParameterException', "Missing parameter 'a' for URL path ':a'.");
+
+	Assert::exception(function() use ($api) {
+		$api->expandColonParameters(':a:b', ['a' => 'A', 'b' => 'B']);
+	}, 'Milo\Github\MissingParameterException', "Missing parameter 'a:b' for URL path ':a:b'.");
+});
+
+
+# Parameters escaping
+test(function() {
+	$api = new TestApi;
+
+	Assert::same('/with%20space', $api->expandColonParameters('/:name', ['name' => 'with space']));
+});
+
+
+# Default parameters expanding
+test(function() {
+	$api = new TestApi;
+
+	Assert::same('/default', $api->expandColonParameters('/:var', [], ['var' => 'default']));
+	Assert::same('/set', $api->expandColonParameters('/:var', ['var' => 'set'], ['var' => 'default']));
+});