Skip to content

Commit

Permalink
[PHP] Enhance Symfony generator (#12532)
Browse files Browse the repository at this point in the history 
* Enhance Symfony generator

  - Add missing typehints
  - Add missing use statements
  - Simplify model ctor
  - Change fallthrough Exception to Throwable
  - prevent storing result of void methods
  - fix validate method
  - add default null values to model properties
  - simplify API interface return values
  - fix/rework default value implementation
  - fix optional params deprecation warnings
  - ..

For more details check out the PR: #12532

* Enhance Symfony generator Tests

  - Skip risky tests
  - Fix type hint error
  - Fix class exists tests
  - Fix broken doc block
  - Enhance annotations
  - Update phpunit.xml.dist
  - Fix test config resource location
  • Loading branch information
@dmetzner
dmetzner authored Jun 25, 2022
1 parent 286a31c commit 3b15bb8
Show file tree
Hide file tree
Showing 56 changed files with 862 additions and 558 deletions.
86 changes: 44 additions & 42 deletions ...i-generator/src/main/java/org/openapitools/codegen/languages/PhpSymfonyServerCodegen.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -373,23 +373,15 @@ public void processOpts() {

// Type-hintable primitive types
// ref: http://php.net/manual/en/functions.arguments.php#functions.arguments.type-declaration
if (phpLegacySupport) {
typeHintable = new HashSet<>(
Arrays.asList(
"array"
)
);
} else {
typeHintable = new HashSet<>(
Arrays.asList(
"array",
"bool",
"float",
"int",
"string"
)
);
}
typeHintable = new HashSet<>(
Arrays.asList(
"array",
"bool",
"float",
"int",
"string"
)
);
}

@Override
Expand Down Expand Up @@ -419,32 +411,24 @@ public OperationsMap postProcessOperationsWithModels(OperationsMap objs, List<Mo
}

// Create a variable to display the correct data type in comments for interfaces
param.vendorExtensions.put("x-comment-type", "\\" + param.dataType);
param.vendorExtensions.put("x-comment-type", prependSlashToDataTypeOnlyIfNecessary(param.dataType));
if (param.isContainer) {
param.vendorExtensions.put("x-comment-type", "\\" + param.dataType + "[]");
param.vendorExtensions.put("x-comment-type", prependSlashToDataTypeOnlyIfNecessary(param.dataType) + "[]");
}
}

// Create a variable to display the correct return type in comments for interfaces
if (op.returnType != null) {
op.vendorExtensions.put("x-comment-type", "\\" + op.returnType);
op.vendorExtensions.put("x-comment-type", prependSlashToDataTypeOnlyIfNecessary(op.returnType));
if ("array".equals(op.returnContainer)) {
op.vendorExtensions.put("x-comment-type", "\\" + op.returnType + "[]");
op.vendorExtensions.put("x-comment-type", prependSlashToDataTypeOnlyIfNecessary(op.returnType) + "[]");
}
} else {
op.vendorExtensions.put("x-comment-type", "void");
}
// Create a variable to add typing for return value of interface
if (op.returnType != null) {
if ("array".equals(op.returnContainer)) {
op.vendorExtensions.put("x-return-type", "iterable");
} else {
if (defaultIncludes.contains(op.returnType)) {
op.vendorExtensions.put("x-return-type", "array|" + op.returnType);
} else {
op.vendorExtensions.put("x-return-type", "array|\\" + op.returnType);
}
}
op.vendorExtensions.put("x-return-type", "array|object|null");
} else {
op.vendorExtensions.put("x-return-type", "void");
}
Expand Down Expand Up @@ -476,26 +460,26 @@ public ModelsMap postProcessModels(ModelsMap objs) {
if (var.dataType != null) {
// Determine if the parameter type is supported as a type hint and make it available
// to the templating engine
String typeHint = getTypeHint(var.dataType);
if (!typeHint.isEmpty()) {
var.vendorExtensions.put("x-parameter-type", typeHint);
}

var.vendorExtensions.put("x-parameter-type", getTypeHintNullable(var.dataType));
var.vendorExtensions.put("x-comment-type", getTypeHintNullableForComments(var.dataType));
if (var.isContainer) {
var.vendorExtensions.put("x-parameter-type", getTypeHint(var.dataType + "[]"));
}

// Create a variable to display the correct data type in comments for models
var.vendorExtensions.put("x-comment-type", var.dataType);
if (var.isContainer) {
var.vendorExtensions.put("x-comment-type", var.dataType + "[]");
var.vendorExtensions.put("x-parameter-type", getTypeHintNullable(var.dataType + "[]"));
var.vendorExtensions.put("x-comment-type", getTypeHintNullableForComments(var.dataType + "[]"));
}
}
}

return objs;
}

public String prependSlashToDataTypeOnlyIfNecessary(String dataType) {
if (dataType.equals("array") || dataType.equals("bool") || dataType.equals("float") || dataType.equals("int") || dataType.equals("string") || dataType.equals("object") || dataType.equals("iterable") || dataType.equals("mixed")) {
return dataType;
}

return "\\" + dataType;
}

/**
* Output the Getter name for boolean property, e.g. isActive
*
Expand Down Expand Up @@ -645,6 +629,24 @@ protected String toSymfonyService(String name) {
return prefix + name;
}

protected String getTypeHintNullable(String type) {
String typeHint = getTypeHint(type);
if (!typeHint.equals("")) {
return "?" + typeHint;
}

return typeHint;
}

protected String getTypeHintNullableForComments(String type) {
String typeHint = getTypeHint(type);
if (!typeHint.equals("")) {
return typeHint + "|null";
}

return typeHint;
}

protected String getTypeHint(String type) {
// Type hint array types
if (type.endsWith("[]")) {
Expand Down
8 changes: 5 additions & 3 deletions modules/openapi-generator/src/main/resources/php-symfony/ApiServer.mustache
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@

namespace {{apiPackage}};

use Symfony\Component\DependencyInjection\Reference;

/**
* ApiServer Class Doc Comment
*
Expand All @@ -35,15 +37,15 @@ class ApiServer
/**
* @var array
*/
private $apis = array();
private array $apis = array();
/**
* Adds an API handler to the server.
*
* @param string $api An API name of the handle
* @param mixed $handler A handler to set for the given API
*/
public function addApiHandler($api, $handler)
public function addApiHandler(string $api, $handler): void
{
if (isset($this->apis[$api])) {
throw new \InvalidArgumentException('API has already a handler: '.$api);
Expand All @@ -59,7 +61,7 @@ class ApiServer
* @return mixed Returns a handler
* @throws \InvalidArgumentException When no such handler exists
*/
public function getApiHandler($api)
public function getApiHandler(string $api)
{
if (!isset($this->apis[$api])) {
throw new \InvalidArgumentException('No handler for '.$api.' implemented.');
Expand Down
65 changes: 46 additions & 19 deletions modules/openapi-generator/src/main/resources/php-symfony/Controller.mustache
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,12 @@

namespace {{controllerPackage}};

use {{apiPackage}}\ApiServer;
use Symfony\Bundle\FrameworkBundle\Controller\AbstractController;
use Symfony\Component\HttpFoundation\Request;
use Symfony\Component\HttpFoundation\Response;
use Symfony\Component\HttpKernel\Exception\HttpException;
use Symfony\Component\Validator\ConstraintViolation;
use {{servicePackage}}\SerializerInterface;
use {{servicePackage}}\ValidatorInterface;

Expand All @@ -36,23 +38,29 @@ use {{servicePackage}}\ValidatorInterface;
*/
class Controller extends AbstractController
{
protected $validator;
protected $serializer;
protected $apiServer;
protected ValidatorInterface $validator;
protected SerializerInterface $serializer;
protected ApiServer $apiServer;
public function setValidator(ValidatorInterface $validator)
public function setValidator(ValidatorInterface $validator): self
{
$this->validator = $validator;
return $this;
}

public function setSerializer(SerializerInterface $serializer)
public function setSerializer(SerializerInterface $serializer): self
{
$this->serializer = $serializer;
return $this;
}

public function setApiServer($server)
public function setApiServer(ApiServer $server): self
{
$this->apiServer = $server;
return $this;
}

/**
Expand All @@ -63,7 +71,7 @@ class Controller extends AbstractController
*
* @return Response
*/
public function createBadRequestResponse($message = 'Bad Request.')
public function createBadRequestResponse(string $message = 'Bad Request.'): Response
{
return new Response($message, 400);
}
Expand All @@ -76,7 +84,7 @@ class Controller extends AbstractController
*
* @return Response
*/
public function createErrorResponse(HttpException $exception)
public function createErrorResponse(HttpException $exception): Response
{
$statusCode = $exception->getStatusCode();
$headers = array_merge($exception->getHeaders(), ['Content-Type' => 'application/json']);
Expand All @@ -91,48 +99,59 @@ class Controller extends AbstractController
* Serializes data to a given type format.
*
* @param mixed $data The data to serialize.
* @param string $class The source data class.
* @param string $format The target serialization format.
*
* @return string A serialized data string.
*/
protected function serialize($data, $format)
protected function serialize($data, string $format): string
{
return $this->serializer->serialize($data, $format);
}

/**
* Deserializes data from a given type format.
*
* @param string $data The data to deserialize.
* @param mixed $data The data to deserialize.
* @param string $class The target data class.
* @param string $format The source serialization format.
*
* @return mixed A deserialized data.
*/
protected function deserialize($data, $class, $format)
protected function deserialize($data, string $class, string $format)
{
return $this->serializer->deserialize($data, $class, $format);
}

protected function validate($data, $asserts = null)
/**
* @param mixed $data
* @param mixed $asserts
*
* @return Response|null
*/
protected function validate($data, $asserts = null): ?Response
{
$errors = $this->validator->validate($data, $asserts);
if (count($errors) > 0) {
$errorsString = (string)$errors;
$errorsString = '';
/** @var ConstraintViolation $violation */
foreach ($errors as $violation) {
$errorsString .= $violation->getMessage()."\n";
}
return $this->createBadRequestResponse($errorsString);
}

return null;
}

/**
* Converts an exception to a serializable array.
*
* @param \Exception|null $exception
* @param \Throwable|null $exception
*
* @return array
* @return array|null
*/
private function exceptionToArray(\Exception $exception = null)
private function exceptionToArray(\Throwable $exception = null): ?array
{
if (null === $exception) {
return null;
Expand All @@ -151,7 +170,15 @@ class Controller extends AbstractController
];
}

protected function getOutputFormat($accept, array $produced)
/**
* Converts an exception to a serializable array.
*
* @param string $accept
* @param array $produced
*
* @return ?string
*/
protected function getOutputFormat(string $accept, array $produced): ?string
{
// Figure out what the client accepts
$accept = preg_split("/[\s,]+/", $accept);
Expand Down Expand Up @@ -186,7 +213,7 @@ class Controller extends AbstractController
*
* @return bool Returns true if Content-Type supported otherwise false.
*/
public static function isContentTypeAllowed(Request $request, array $consumes = [])
public static function isContentTypeAllowed(Request $request, array $consumes = []): bool
{
if (!empty($consumes) && $consumes[0] !== '*/*') {
$currentFormat = $request->getContentType();
Expand Down
2 changes: 1 addition & 1 deletion modules/openapi-generator/src/main/resources/php-symfony/Extension.mustache
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ use Symfony\Component\DependencyInjection\Loader\YamlFileLoader;
*/
class {{bundleExtensionName}} extends Extension
{
public function load(array $configs, ContainerBuilder $container)
public function load(array $configs, ContainerBuilder $container): void
{
$loader = new YamlFileLoader($container, new FileLocator(__DIR__.'/../Resources/config'));
$loader->load('services.yml');
Expand Down
2 changes: 1 addition & 1 deletion modules/openapi-generator/src/main/resources/php-symfony/README.mustache
View file
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ class {{baseName}}Api implements {{classname}} // An interface is autogenerated
/**
* Implementation of {{classname}}#{{operationId}}
*/
public function {{operationId}}({{#allParams}}{{#vendorExtensions.x-parameter-type}}{{vendorExtensions.x-parameter-type}} {{/vendorExtensions.x-parameter-type}}${{paramName}}{{^required}} = {{#defaultValue}}'{{{.}}}'{{/defaultValue}}{{^defaultValue}}null{{/defaultValue}}{{/required}}{{^-last}}, {{/-last}}{{/allParams}}, &$responseCode, array &$responseHeaders): {{#isArray}}iterable{{/isArray}}{{^isArray}}array|{{{vendorExtensions.x-comment-type}}}{{/isArray}}
public function {{operationId}}({{#allParams}}{{#vendorExtensions.x-parameter-type}}{{vendorExtensions.x-parameter-type}} {{/vendorExtensions.x-parameter-type}}${{paramName}}{{^required}} = {{#defaultValue}}'{{{.}}}'{{/defaultValue}}{{^defaultValue}}null{{/defaultValue}}{{/required}}{{^-last}}, {{/-last}}{{/allParams}}, int &$responseCode, array &$responseHeaders): {{{vendorExtensions.x-return-type}}}
{
// Implement the operation ...
}
Expand Down
13 changes: 7 additions & 6 deletions modules/openapi-generator/src/main/resources/php-symfony/api.mustache
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,11 +38,11 @@ interface {{classname}}
/**
* Sets authentication method {{name}}
*
* @param string $value Value of the {{name}} authentication method.
* @param string|null $value Value of the {{name}} authentication method.
*
* @return void
*/
public function set{{name}}($value);
public function set{{name}}(?string $value): void;
{{/authMethods}}
{{#operation}}

Expand All @@ -58,16 +58,17 @@ interface {{classname}}
*
{{/description}}
{{#allParams}}
* @param {{vendorExtensions.x-comment-type}} ${{paramName}} {{description}} {{#required}}(required){{/required}}{{^required}}(optional{{#defaultValue}}, default to {{{.}}}{{/defaultValue}}){{/required}}{{#isDeprecated}} (deprecated){{/isDeprecated}}
* @param {{vendorExtensions.x-parameter-type}}{{^required}}{{^defaultValue}}|null{{/defaultValue}}{{/required}} ${{paramName}} {{description}} {{#required}}(required){{/required}}{{^required}}(optional{{#defaultValue}}, default to {{{.}}}{{/defaultValue}}){{/required}}{{#isDeprecated}} (deprecated){{/isDeprecated}}
{{/allParams}}
* @param \array $responseHeaders Additional HTTP headers to return with the response ()
* @param int &$responseCode The HTTP Response Code
* @param array $responseHeaders Additional HTTP headers to return with the response ()
*
* @return {{{vendorExtensions.x-comment-type}}}
* @return {{{vendorExtensions.x-return-type}}}
{{#isDeprecated}}
* @deprecated
{{/isDeprecated}}
*/
public function {{operationId}}({{#allParams}}{{#vendorExtensions.x-parameter-type}}{{vendorExtensions.x-parameter-type}} {{/vendorExtensions.x-parameter-type}}${{paramName}}{{^required}} = {{{defaultValue}}}{{^defaultValue}}null{{/defaultValue}}{{/required}}, {{/allParams}}&$responseCode, array &$responseHeaders): {{{vendorExtensions.x-return-type}}};
public function {{operationId}}({{#allParams}}{{^required}}{{^defaultValue}}?{{/defaultValue}}{{/required}}{{#vendorExtensions.x-parameter-type}}{{vendorExtensions.x-parameter-type}} {{/vendorExtensions.x-parameter-type}}${{paramName}}, {{/allParams}}int &$responseCode, array &$responseHeaders): {{{vendorExtensions.x-return-type}}};

{{/operation}}
}
Expand Down
Loading

0 comments on commit 3b15bb8

Please sign in to comment.