Skip to content

Commit 34b1a9a

Browse files
provokateurinprovokateurin
authored
Merge pull request #238 from nextcloud/feat/controllermethod/getparam
2 parents 6926933 + ae5d109 commit 34b1a9a

File tree

8 files changed

+446
-36
lines changed

8 files changed

+446
-36
lines changed

‎generate-spec.php‎

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -614,13 +614,9 @@
614614
$requirement = $route->requirements[$urlParameter] ?? null;
615615
if (count($matchingParameters) == 1) {
616616
$parameter = $matchingParameters[array_keys($matchingParameters)[0]];
617-
if ($parameter?->methodParameter == null && ($route->requirements == null || !array_key_exists($urlParameter, $route->requirements))) {
618-
Logger::error($route->name, "Unable to find parameter for '" . $urlParameter . "'");
619-
continue;
620-
}
621617

622618
$schema = $parameter->type->toArray(true);
623-
$description = $parameter?->docType != null && $parameter->docType->description != '' ? Helpers::cleanDocComment($parameter->docType->description) : null;
619+
$description = $parameter->type->description;
624620
} else {
625621
$schema = [
626622
'type' => 'string',
@@ -825,8 +821,8 @@
825821
'name' => $queryParameter->name . ($queryParameter->type->type === 'array' ? '[]' : ''),
826822
'in' => 'query',
827823
];
828-
if ($queryParameter->docType !== null && $queryParameter->docType->description !== '') {
829-
$parameter['description'] = Helpers::cleanDocComment($queryParameter->docType->description);
824+
if ($queryParameter->type->description !== null && $queryParameter->type->description !== '') {
825+
$parameter['description'] = Helpers::cleanDocComment($queryParameter->type->description);
830826
}
831827
if (!$queryParameter->type->nullable && !$queryParameter->type->hasDefaultValue) {
832828
$parameter['required'] = true;

‎src/ControllerMethod.php‎

Lines changed: 44 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -190,7 +190,7 @@ public static function parse(string $context,
190190
$description = '';
191191
}
192192
// Only keep lines that don't match the status code pattern in the description
193-
$description = implode("\n", array_filter(array_filter(explode("\n", $description), static fn (string $line) => trim($line) !== ''), static fn (string $line) => !preg_match(self::STATUS_CODE_DESCRIPTION_PATTERN, $line)));
193+
$description = Helpers::cleanDocComment(implode("\n", array_filter(array_filter(explode("\n", $description), static fn (string $line) => trim($line) !== ''), static fn (string $line) => !preg_match(self::STATUS_CODE_DESCRIPTION_PATTERN, $line))));
194194

195195
if ($paramTag instanceof \PHPStan\PhpDocParser\Ast\PhpDoc\ParamTagValueNode && $psalmParamTag instanceof \PHPStan\PhpDocParser\Ast\PhpDoc\ParamTagValueNode) {
196196
try {
@@ -226,17 +226,24 @@ public static function parse(string $context,
226226
} elseif ($paramTag instanceof \PHPStan\PhpDocParser\Ast\PhpDoc\ParamTagValueNode) {
227227
$type = OpenApiType::resolve($context . ': @param: ' . $methodParameterName, $definitions, $paramTag);
228228
} elseif ($allowMissingDocs) {
229-
$type = null;
229+
$type = OpenApiType::resolve($context . ': $' . $methodParameterName . ': ' . $methodParameterName, $definitions, $methodParameter->type);
230230
} else {
231231
Logger::error($context, "Missing doc parameter for '" . $methodParameterName . "'");
232232
continue;
233233
}
234234

235-
if ($type !== null) {
236-
$type->description = $description;
235+
$type->description = $description;
236+
237+
if ($methodParameter->default !== null) {
238+
try {
239+
$type->defaultValue = Helpers::exprToValue($context, $methodParameter->default);
240+
$type->hasDefaultValue = true;
241+
} catch (UnsupportedExprException $e) {
242+
Logger::debug($context, $e);
243+
}
237244
}
238245

239-
$param = new ControllerMethodParameter($context, $definitions, $methodParameterName, $methodParameter, $type);
246+
$param = new ControllerMethodParameter($context, $definitions, $methodParameterName, $type);
240247

241248
if (!$allowMissingDocs && $param->type->description == '') {
242249
Logger::error($context . ': @param: ' . $methodParameterName, 'Missing description');
@@ -284,9 +291,38 @@ public static function parse(string $context,
284291
if ($methodCall->var instanceof PropertyFetch &&
285292
$methodCall->var->var instanceof Variable &&
286293
$methodCall->var->var->name === 'this' &&
287-
$methodCall->var->name->name === 'request' &&
288-
$methodCall->name->name === 'getHeader') {
289-
$headers[] = Helpers::exprToValue($context . ': getHeader', $methodCall->args[0]->value);
294+
$methodCall->var->name->name === 'request') {
295+
if ($methodCall->name->name === 'getHeader') {
296+
$headers[] = $methodCall->args[0]->value->value;
297+
}
298+
if ($methodCall->name->name === 'getParam') {
299+
$name = $methodCall->args[0]->value->value;
300+
301+
if (preg_match('/^[a-zA-Z][a-zA-Z0-9_]*$/', $name)) {
302+
Logger::error($context . ': getParam: ' . $name, 'Do not use getParam() when a controller method parameter also works. With getParam() it is not possible to add a comment and specify the parameter type, therefore it should be avoided whenever possible.');
303+
}
304+
305+
$defaultValue = null;
306+
$hasDefaultValue = false;
307+
try {
308+
$defaultValue = count($methodCall->args) > 1 ? Helpers::exprToValue($context . ': getParam: ' . $name, $methodCall->args[1]->value) : null;
309+
$hasDefaultValue = true;
310+
} catch (UnsupportedExprException $e) {
311+
Logger::debug($context, $e);
312+
}
313+
314+
$type = new OpenApiType(
315+
context: $context,
316+
// We can not know the type, so need to fallback to object :/
317+
type: 'object',
318+
// IRequest::getParam() has null as a default value, so the parameter always has a default value and allows null.
319+
nullable: true,
320+
hasDefaultValue: $hasDefaultValue,
321+
defaultValue: $defaultValue,
322+
);
323+
324+
$parameters[] = new ControllerMethodParameter($context, $definitions, $name, $type);
325+
}
290326
}
291327
}
292328

‎src/ControllerMethodParameter.php‎

Lines changed: 1 addition & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -7,30 +7,12 @@
77

88
namespace OpenAPIExtractor;
99

10-
use PhpParser\Node\Param;
11-
1210
class ControllerMethodParameter {
13-
public OpenApiType $type;
14-
1511
public function __construct(
1612
string $context,
1713
array $definitions,
1814
public string $name,
19-
public Param $methodParameter,
20-
public ?OpenApiType $docType,
15+
public OpenApiType $type,
2116
) {
22-
if ($docType != null) {
23-
$this->type = $this->docType;
24-
} else {
25-
$this->type = OpenApiType::resolve($context . ': @param: ' . $name, $definitions, $methodParameter->type);
26-
}
27-
if ($methodParameter->default != null) {
28-
try {
29-
$this->type->defaultValue = Helpers::exprToValue($context, $methodParameter->default);
30-
$this->type->hasDefaultValue = true;
31-
} catch (UnsupportedExprException $e) {
32-
Logger::debug($context, $e);
33-
}
34-
}
3517
}
3618
}

‎src/OpenApiType.php‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -104,8 +104,8 @@ enum: [0, 1],
104104
if ($this->deprecated) {
105105
$values['deprecated'] = true;
106106
}
107-
if ($this->hasDefaultValue && $this->defaultValue !== null) {
108-
$values['default'] = $this->type === 'object' && empty($this->defaultValue) ? new stdClass() : $this->defaultValue;
107+
if ($this->hasDefaultValue) {
108+
$values['default'] = $this->type === 'object' && is_array($this->defaultValue) && count($this->defaultValue) === 0 ? new stdClass() : $this->defaultValue;
109109
}
110110
if ($this->enum !== null) {
111111
$values['enum'] = $this->enum;

‎tests/appinfo/routes.php‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,7 @@
5252
['name' => 'Settings#booleanTrueFalseParameter', 'url' => '/api/{apiVersion}/boolean-true-false', 'verb' => 'POST', 'requirements' => ['apiVersion' => '(v2)']],
5353
['name' => 'Settings#trueFalseParameter', 'url' => '/api/{apiVersion}/true-false', 'verb' => 'POST', 'requirements' => ['apiVersion' => '(v2)']],
5454
['name' => 'Settings#stringValueParameter', 'url' => '/api/{apiVersion}/string-value', 'verb' => 'POST', 'requirements' => ['apiVersion' => '(v2)']],
55+
['name' => 'Settings#stringDefaultNullParameter', 'url' => '/api/{apiVersion}/string-default-null', 'verb' => 'POST', 'requirements' => ['apiVersion' => '(v2)']],
5556
['name' => 'Settings#intValueParameter', 'url' => '/api/{apiVersion}/int-value', 'verb' => 'POST', 'requirements' => ['apiVersion' => '(v2)']],
5657
['name' => 'Settings#numericParameter', 'url' => '/api/{apiVersion}/numeric', 'verb' => 'POST', 'requirements' => ['apiVersion' => '(v2)']],
5758
['name' => 'Settings#arrayListParameter', 'url' => '/api/{apiVersion}/array-list', 'verb' => 'POST', 'requirements' => ['apiVersion' => '(v2)']],
@@ -86,6 +87,7 @@
8687
['name' => 'Settings#samePathGet', 'url' => '/api/{apiVersion}/same-path', 'verb' => 'GET', 'requirements' => ['apiVersion' => '(v2)']],
8788
['name' => 'Settings#samePathPost', 'url' => '/api/{apiVersion}/same-path', 'verb' => 'POST', 'requirements' => ['apiVersion' => '(v2)']],
8889
['name' => 'Settings#requestHeader', 'url' => '/api/{apiVersion}/request-header', 'verb' => 'POST', 'requirements' => ['apiVersion' => '(v2)']],
90+
['name' => 'Settings#requestParams', 'url' => '/api/{apiVersion}/request-params', 'verb' => 'POST', 'requirements' => ['apiVersion' => '(v2)']],
8991
['name' => 'V1\SubDir#subDirRoute', 'url' => '/sub-dir', 'verb' => 'GET'],
9092
],
9193
];

‎tests/lib/Controller/SettingsController.php‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -349,6 +349,18 @@ public function stringValueParameter(string $value): DataResponse {
349349
return new DataResponse();
350350
}
351351

352+
/**
353+
* A route with string or null
354+
*
355+
* @param ?string $value string or null
356+
* @return DataResponse<Http::STATUS_OK, list<empty>, array{}>
357+
*
358+
* 200: Admin settings updated
359+
*/
360+
public function stringDefaultNullParameter(?string $value = null): DataResponse {
361+
return new DataResponse();
362+
}
363+
352364
/**
353365
* A route with int or 0
354366
*
@@ -752,4 +764,18 @@ public function requestHeader(): DataResponse {
752764

753765
return new DataResponse();
754766
}
767+
768+
/**
769+
* A method with a request params.
770+
*
771+
* @return DataResponse<Http::STATUS_OK, list<empty>, array{}>
772+
*
773+
* 200: Admin settings updated
774+
*/
775+
public function requestParams(): DataResponse {
776+
$value = $this->request->getParam('some-param');
777+
$value = $this->request->getParam('some-param-with-explicit-default-value', 'abc');
778+
779+
return new DataResponse();
780+
}
755781
}

0 commit comments

Comments
 (0)