Skip to content

Commit 4089438

Browse files
authored
[php-symfony] fix handling of endpoints with "text/plain" or "image/png" response type (#21261)
* [php-symfony] Never return 406 when user accepts */* When a query has header "Accept" set to "*/*" it means it accepts everything. It is hence weird to return a 406. This patch ensures it does not occur: when the query accepts everything then we take any produced type. This fixes #13334. This also partly makes the open PR #15560 obsolete (or at least, it provides a workaround) * [php-symfony] Don't crash at runtime on null convertFormat $this->convertFormat may return "null". When it's the case we end up calling ...->serialize($data, null); but this crashes at runtime because that serialize method declares that the 2nd parameter is of type "string" (so null is not accepted). With this patch we avoid having an error 500. Instead we return something that makes perfect sense when the OpenApi specification declares a content of type "text/plain" and that the returned value is for instance a string, an int, or a boolean. * [php Symfony] fix return type for non json/xml api This fixes the generated returned type of controller methods for endpoint with a response declared like content: text/plain: schema: type: <boolean|string|integer|number> or for content: image/png: schema: type: string format: binary Without this commit the generated method *had to* return a value that matched "array|object|null", which does not work in this case. This commit makes it possible to return the proper type.
1 parent 429da98 commit 4089438

File tree

5 files changed

+42
-4
lines changed

5 files changed

+42
-4
lines changed

modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/PhpSymfonyServerCodegen.java

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -417,10 +417,16 @@ public OperationsMap postProcessOperationsWithModels(OperationsMap objs, List<Mo
417417
op.vendorExtensions.put("x-comment-type", "void");
418418
}
419419
// Create a variable to add typing for return value of interface
420-
if (op.returnType != null) {
420+
if (op.returnType == null) {
421+
op.vendorExtensions.put("x-return-type", "void");
422+
} else if (op.isArray || op.isMap || isApplicationJsonOrApplicationXml(op)
423+
|| !op.returnTypeIsPrimitive // it could make sense to remove it, but it would break retro-compatibility
424+
) {
421425
op.vendorExtensions.put("x-return-type", "array|object|null");
426+
} else if ("binary".equals(op.returnProperty.dataFormat)) {
427+
op.vendorExtensions.put("x-return-type", "mixed");
422428
} else {
423-
op.vendorExtensions.put("x-return-type", "void");
429+
op.vendorExtensions.put("x-return-type", op.returnType);
424430
}
425431

426432
// Add operation's authentication methods to whole interface
@@ -438,6 +444,18 @@ public OperationsMap postProcessOperationsWithModels(OperationsMap objs, List<Mo
438444
return objs;
439445
}
440446

447+
private boolean isApplicationJsonOrApplicationXml(CodegenOperation op) {
448+
if (op.produces != null) {
449+
for(Map<String, String> produce : op.produces) {
450+
String mediaType = produce.get("mediaType");
451+
if (isJsonMimeType(mediaType) || isXmlMimeType(mediaType)) {
452+
return true;
453+
}
454+
}
455+
}
456+
return false;
457+
}
458+
441459
@Override
442460
public ModelsMap postProcessModels(ModelsMap objs) {
443461
objs = super.postProcessModels(objs);

modules/openapi-generator/src/main/resources/php-symfony/Controller.mustache

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -204,6 +204,10 @@ class Controller extends AbstractController
204204
return 'application/xml';
205205
}
206206

207+
if (in_array('*/*', $accept)) {
208+
return $produced[0];
209+
}
210+
207211
// If we reach this point, we don't have a common ground between server and client
208212
return null;
209213
}

modules/openapi-generator/src/main/resources/php-symfony/serialization/JmsSerializer.mustache

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,13 @@ class JmsSerializer implements SerializerInterface
2929
*/
3030
public function serialize($data, string $format): string
3131
{
32-
return SerializerBuilder::create()->build()->serialize($data, $this->convertFormat($format));
32+
$convertFormat = $this->convertFormat($format);
33+
if ($convertFormat !== null) {
34+
return SerializerBuilder::create()->build()->serialize($data, $convertFormat);
35+
} else {
36+
// don't use var_export if $data is already a string: it may corrupt binary strings
37+
return is_string($data) ? $data : var_export($data, true);
38+
}
3339
}
3440
3541
/**

samples/server/petstore/php-symfony/SymfonyBundle-php/Controller/Controller.php

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -214,6 +214,10 @@ protected function getOutputFormat(string $accept, array $produced): ?string
214214
return 'application/xml';
215215
}
216216

217+
if (in_array('*/*', $accept)) {
218+
return $produced[0];
219+
}
220+
217221
// If we reach this point, we don't have a common ground between server and client
218222
return null;
219223
}

samples/server/petstore/php-symfony/SymfonyBundle-php/Service/JmsSerializer.php

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,13 @@ public function __construct()
2929
*/
3030
public function serialize($data, string $format): string
3131
{
32-
return SerializerBuilder::create()->build()->serialize($data, $this->convertFormat($format));
32+
$convertFormat = $this->convertFormat($format);
33+
if ($convertFormat !== null) {
34+
return SerializerBuilder::create()->build()->serialize($data, $convertFormat);
35+
} else {
36+
// don't use var_export if $data is already a string: it may corrupt binary strings
37+
return is_string($data) ? $data : var_export($data, true);
38+
}
3339
}
3440

3541
/**

0 commit comments

Comments
 (0)