Skip to content

Commit

Permalink
feat: add required to responseField tag and append the required field…
Browse files Browse the repository at this point in the history
…s to generateResponseContentSpec for object types (#814)

* feat: add required to responseField tag and append the required fields to generateResponseContentSpec for object types

* fix: set required key only if there are required fields in the response type

* fix: add space when readding required to description after recognizing required is part of the description

* refactor: make linter happy

* fix: mark required field as optional inside the format

* fix: only add top-level properties of object as required

* fix: add fields as required given nested path and add test

* test: add test for required parameter of response field attribute

---------

Co-authored-by: daniel <daniel@softwerft.com>
Co-authored-by: sotdan <sotdan7@gmail.com>
  • Loading branch information
3 people authored Oct 18, 2024
1 parent e35e6da commit a6aee1d
Show file tree
Hide file tree
Showing 5 changed files with 84 additions and 12 deletions.
3 changes: 3 additions & 0 deletions camel/Extraction/ResponseField.php
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,9 @@ class ResponseField extends BaseDTO
/** @var string */
public $type;

/** @var boolean */
public $required;

/** @var mixed */
public $example;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,22 +16,28 @@ class GetFromResponseFieldTag extends GetFieldsFromTagStrategy
protected function parseTag(string $tagContent): array
{
// Format:
// @responseField <name> <type> <description>
// @responseField <name> <type> <"required" (optional)> <description>
// Examples:
// @responseField text string The text.
// @responseField text string required The text.
// @responseField user_id integer The ID of the user.
preg_match('/(.+?)\s+(.+?)\s+([\s\S]*)/', $tagContent, $content);
preg_match('/(.+?)\s+(.+?)\s+(.+?)\s+([\s\S]*)/', $tagContent, $content);
if (empty($content)) {
// This means only name and type were supplied
[$name, $type] = preg_split('/\s+/', $tagContent);
$description = '';
$required = false;
} else {
[$_, $name, $type, $description] = $content;
[$_, $name, $type, $required, $description] = $content;
if($required !== "required"){
$description = $required . " " . $description;
}

$required = $required === "required";
$description = trim($description);
}

$type = static::normalizeTypeName($type);
$data = compact('name', 'type', 'description');
$data = compact('name', 'type', 'required', 'description');

// Support optional type in annotation
// The type can also be a union or nullable type (eg ?string or string|null)
Expand Down
30 changes: 28 additions & 2 deletions src/Writing/OpenAPISpecWriter.php
Original file line number Diff line number Diff line change
Expand Up @@ -400,15 +400,16 @@ protected function generateResponseContentSpec(?string $responseContent, OutputE
],
'example' => $decoded,
],
],
],
];

case 'object':
$properties = collect($decoded)->mapWithKeys(function ($value, $key) use ($endpoint) {
return [$key => $this->generateSchemaForValue($value, $endpoint, $key)];
})->toArray();
$required = $this->filterRequiredFields($endpoint, array_keys($properties));

return [
$data = [
'application/json' => [
'schema' => [
'type' => 'object',
Expand All @@ -417,6 +418,11 @@ protected function generateResponseContentSpec(?string $responseContent, OutputE
],
],
];
if ($required) {
$data['application/json']['schema']['required'] = $required;
}

return $data;
}
}

Expand Down Expand Up @@ -593,11 +599,15 @@ public function generateSchemaForValue(mixed $value, OutputEndpointData $endpoin
$subFieldPath = sprintf('%s.%s', $path, $subField);
$properties[$subField] = $this->generateSchemaForValue($subValue, $endpoint, $subFieldPath);
}
$required = $this->filterRequiredFields($endpoint, array_keys($properties), $path);

$schema = [
'type' => 'object',
'properties' => $this->objectIfEmpty($properties),
];
if ($required) {
$schema['required'] = $required;
}
$this->setDescription($schema, $endpoint, $path);

return $schema;
Expand Down Expand Up @@ -632,6 +642,22 @@ public function generateSchemaForValue(mixed $value, OutputEndpointData $endpoin
}

/**
* Given an enpoint and a set of object keys at a path, return the properties that are specified as required.
*/
public function filterRequiredFields(OutputEndpointData $endpoint, array $properties, string $path = ''): array
{
$required = [];
foreach ($properties as $property) {
$responseField = $endpoint->responseFields["$path.$property"] ?? $endpoint->responseFields[$property] ?? null;
if ($responseField && $responseField->required) {
$required[] = $property;
}
}

return $required;
}

/*
* Set the description for the schema. If the field has a description, it is set in the schema.
*/
private function setDescription(array &$schema, OutputEndpointData $endpoint, string $path): void
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,11 +46,19 @@ public function can_fetch_from_responsefield_attribute()
'id' => [
'type' => 'integer',
'description' => 'The id of the newly created user.',
'required' => true,
],
'other' => [
'type' => 'string',
'description' => '',
'required' => true,
],
'required_attribute' => [
'required' => true,
],
'not_required_attribute' => [
'required' => false,
]
], $results);
}

Expand Down Expand Up @@ -98,6 +106,8 @@ class ResponseFieldAttributeTestController
{
#[ResponseField('id', description: 'The id of the newly created user.')]
#[ResponseField('other', 'string')]
#[ResponseField('required_attribute', required: true)]
#[ResponseField('not_required_attribute', required: false)]
public function methodWithAttributes()
{
}
Expand Down
37 changes: 32 additions & 5 deletions tests/Unit/OpenAPISpecWriterTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -450,7 +450,7 @@ public function adds_responses_correctly_as_responses_on_operation_object()
[
'status' => 201,
'description' => '',
'content' => '{"this": "shouldn\'t be ignored", "and this": "too", "sub level 0": { "sub level 1 key 1": "sl0_sl1k1", "sub level 1 key 2": [ { "sub level 2 key 1": "sl0_sl1k2_sl2k1", "sub level 2 key 2": { "sub level 3 key 1": "sl0_sl1k2_sl2k2_sl3k1" } } ], "sub level 1 key 3": { "sub level 2 key 1": "sl0_sl1k3_sl2k2", "sub level 2 key 2": { "sub level 3 key 1": "sl0_sl1k3_sl2k2_sl3k1", "sub level 3 key null": null, "sub level 3 key integer": 99 } } } }',
'content' => '{"this": "shouldn\'t be ignored", "and this": "too", "also this": "too", "sub level 0": { "sub level 1 key 1": "sl0_sl1k1", "sub level 1 key 2": [ { "sub level 2 key 1": "sl0_sl1k2_sl2k1", "sub level 2 key 2": { "sub level 3 key 1": "sl0_sl1k2_sl2k2_sl3k1" } } ], "sub level 1 key 3": { "sub level 2 key 1": "sl0_sl1k3_sl2k2", "sub level 2 key 2": { "sub level 3 key 1": "sl0_sl1k3_sl2k2_sl3k1", "sub level 3 key null": null, "sub level 3 key integer": 99 }, "sub level 2 key 3 required" : "sl0_sl1k3_sl2k3" } } }',
],
],
'responseFields' => [
Expand All @@ -459,9 +459,19 @@ public function adds_responses_correctly_as_responses_on_operation_object()
'type' => 'string',
'description' => 'Parameter description, ha!',
],
'also this' => [
'name' => 'also this',
'type' => 'string',
'description' => 'This response parameter is required.',
'required' => true,
],
'sub level 0.sub level 1 key 3.sub level 2 key 1' => [
'description' => 'This is description of nested object',
]
'description' => 'This is a description of a nested object',
],
'sub level 0.sub level 1 key 3.sub level 2 key 3 required' => [
'description' => 'This is a description of a required nested object',
'required' => true,
],
],
]);
$endpointData2 = $this->createMockEndpointData([
Expand Down Expand Up @@ -499,6 +509,11 @@ public function adds_responses_correctly_as_responses_on_operation_object()
'example' => "too",
'type' => 'string',
],
'also this' => [
'description' => 'This response parameter is required.',
'example' => "too",
'type' => 'string',
],
'sub level 0' => [
'type' => 'object',
'properties' => [
Expand Down Expand Up @@ -526,7 +541,7 @@ public function adds_responses_correctly_as_responses_on_operation_object()
'sub level 2 key 1' => [
'type' => 'string',
'example' => 'sl0_sl1k3_sl2k2',
'description' => 'This is description of nested object'
'description' => 'This is a description of a nested object'
],
'sub level 2 key 2' => [
'type' => 'object',
Expand All @@ -544,12 +559,24 @@ public function adds_responses_correctly_as_responses_on_operation_object()
'example' => 99
]
]
]
],
'sub level 2 key 3 required' => [
'type' => 'string',
'example' => 'sl0_sl1k3_sl2k3',
'description' => 'This is a description of a required nested object'
],

],
'required' => [
'sub level 2 key 3 required'
]
]
]
]
],
'required' => [
'also this'
]
],
],
],
Expand Down

0 comments on commit a6aee1d

Please sign in to comment.