Skip to content

Commit ed72281

Browse files
authored
fix(php-symfony): enum $ref query params — short types and imports (#23521) (#23525)
* fix(php-symfony): enum-ref query params use short types with correct imports * Normalize dotted / flattened enum-ref dataType and resync operation imports; rebuild OperationsMap imports after postProcess for api.mustache use lines * Unify api.mustache @param and signatures on vendorExtensions.x-parameter-type (short name + optional|null) for isEnumRef * Add OAS 3.1 petstore fixture under 3_1/php-symfony and extend PhpSymfonyServerCodegenTest (PHPDoc + php -l) * Document normalizeEnumRefParameterDataType / sync / refresh import pipeline * update comments
1 parent 3ccc6ab commit ed72281

File tree

6 files changed

+258
-13
lines changed

6 files changed

+258
-13
lines changed

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

Lines changed: 148 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -389,6 +389,11 @@ public OperationsMap postProcessOperationsWithModels(OperationsMap objs, List<Mo
389389
// Loop through all input parameters to determine, whether we have to import something to
390390
// make the input type available.
391391
for (CodegenParameter param : op.allParams) {
392+
// Enum-by-reference query params (e.g. OAS 3.1 dotted keys): normalize dataType and
393+
// sync this operation's imports (normalizeEnumRefParameterDataType →
394+
// syncEnumRefOperationImports). refreshAggregatedImportsForOperations runs once after
395+
// this inner loop to rebuild bundle-level Api imports for Mustache.
396+
normalizeEnumRefParameterDataType(op, param);
392397
// Determine if the parameter type is supported as a type hint and make it available
393398
// to the templating engine
394399
String typeHint = getTypeHint(param.dataType, false);
@@ -405,6 +410,16 @@ public OperationsMap postProcessOperationsWithModels(OperationsMap objs, List<Mo
405410
if (param.isContainer) {
406411
param.vendorExtensions.put("x-comment-type", prependSlashToDataTypeOnlyIfNecessary(param.dataType) + "[]");
407412
}
413+
414+
// Enum $ref parameters: dataType is the short PHP model class name only; getTypeHint(dataType) is empty.
415+
// Build FQCN body so getTypeHint matches isModelClass and yields the same short name as file-level imports.
416+
if (param.isEnumRef && StringUtils.isNotEmpty(param.dataType)) {
417+
String fqcnBody = modelPackage() + "\\" + param.dataType;
418+
String enumTypeHint = getTypeHint(fqcnBody, false);
419+
if (StringUtils.isNotEmpty(enumTypeHint)) {
420+
param.vendorExtensions.put("x-parameter-type", enumTypeHint);
421+
}
422+
}
408423
}
409424

410425
// Create a variable to display the correct return type in comments for interfaces
@@ -441,9 +456,142 @@ public OperationsMap postProcessOperationsWithModels(OperationsMap objs, List<Mo
441456

442457
operations.put("authMethods", authMethods);
443458

459+
refreshAggregatedImportsForOperations(objs);
460+
444461
return objs;
445462
}
446463

464+
/**
465+
* Normalizes {@link CodegenParameter#getDataType()} for enum-by-reference parameters only
466+
* ({@code param.isEnumRef}).
467+
* <p><b>Why:</b> Templates concatenate {@code modelPackage} + {@code dataType} for FQCN strings (e.g. validation /
468+
* deserialization). The API interface uses short names plus {@code use} imports, so {@code dataType} must be the
469+
* <em>short</em> PHP class name. Upstream parsing (OpenAPI 3.1 dotted keys, {@code components.parameters} {@code $ref},
470+
* or flattening) can leave {@code dataType} as a bogus single token (no {@code \}), a full FQCN, or a leading
471+
* {@code \}.
472+
* <p><b>Execution (this method):</b>
473+
* <ol>
474+
* <li>Exit if not enum ref or empty {@code dataType}.</li>
475+
* <li>Strip a leading {@code \} from the working copy.</li>
476+
* <li>If the copy contains {@code \}: if it starts with {@code modelPackage + "\\"}, keep the suffix as short name;
477+
* else if it looks like a model FQCN under another path, take {@link #extractSimpleName(String)}.</li>
478+
* <li>If there is no {@code \}: optionally strip a bogus {@code modelPackage} with all separators removed
479+
* (flat prefix) from the start, then {@link #toModelName(String)} so dotted logical names become the real
480+
* generated model class name.</li>
481+
* <li>Always finish with {@link #syncEnumRefOperationImports(CodegenOperation, CodegenParameter, String)} so
482+
* {@link CodegenOperation#imports} matches the normalized short name.</li>
483+
* </ol>
484+
* <p><b>Downstream in {@link #postProcessOperationsWithModels}:</b> after this call, {@link #getTypeHint(String, Boolean)}
485+
* and {@code x-parameter-type} use {@code modelPackage + "\\" + dataType} for enum refs so the hint matches imports.
486+
* <p><b>Aggregated imports:</b> {@link OperationsMap#setImports} is built before this hook; callers must invoke
487+
* {@link #refreshAggregatedImportsForOperations(OperationsMap)} once all operations/parameters are processed.
488+
*/
489+
private void normalizeEnumRefParameterDataType(CodegenOperation op, CodegenParameter param) {
490+
if (!param.isEnumRef || StringUtils.isEmpty(param.dataType)) {
491+
return;
492+
}
493+
String dt = param.dataType;
494+
if (dt.startsWith("\\")) {
495+
dt = dt.substring(1);
496+
}
497+
final String mp = modelPackage();
498+
if (dt.contains("\\")) {
499+
if (dt.startsWith(mp + "\\")) {
500+
param.dataType = dt.substring(mp.length() + 1);
501+
} else if (isModelClass(dt)) {
502+
param.dataType = extractSimpleName(dt);
503+
}
504+
} else {
505+
// No backslashes: flattened invoker+model+class token, dotted logical name, or already-short class name
506+
String flatPrefix = mp.replace("\\", "");
507+
if (StringUtils.isNotEmpty(flatPrefix)
508+
&& dt.startsWith(flatPrefix)
509+
&& dt.length() > flatPrefix.length()) {
510+
dt = dt.substring(flatPrefix.length());
511+
}
512+
param.dataType = toModelName(dt);
513+
}
514+
syncEnumRefOperationImports(op, param, mp);
515+
}
516+
517+
/**
518+
* Repairs {@link CodegenOperation#imports} for one enum-ref parameter after {@link #normalizeEnumRefParameterDataType}.
519+
* <p><b>Step 1 — remove bogus entries:</b> {@code DefaultGenerator} may add a single token that is the
520+
* {@code modelPackage} string with all {@code \} removed, prefixed to the class name (still without {@code \}).
521+
* Such values are not valid PHP imports and break {@code api.mustache} {@code use} lines; drop any import string
522+
* that has no backslash, starts with that flat prefix, and is longer than the prefix alone.
523+
* <p><b>Step 2 — add the short model name:</b> if {@code param.dataType} is non-empty and {@link #needToImport(String)}
524+
* is true, add it so the operation contributes the correct short classname to the union used for template imports.
525+
*/
526+
private void syncEnumRefOperationImports(CodegenOperation op, CodegenParameter param, String modelPackage) {
527+
if (op == null || op.imports == null || StringUtils.isEmpty(modelPackage)) {
528+
return;
529+
}
530+
String flatPrefix = modelPackage.replace("\\", "");
531+
if (StringUtils.isEmpty(flatPrefix)) {
532+
return;
533+
}
534+
op.imports.removeIf(s ->
535+
s != null
536+
&& !s.contains("\\")
537+
&& s.startsWith(flatPrefix)
538+
&& s.length() > flatPrefix.length());
539+
if (StringUtils.isNotEmpty(param.dataType) && needToImport(param.dataType)) {
540+
op.imports.add(param.dataType);
541+
}
542+
}
543+
544+
/**
545+
* Recomputes the bundle-level import list exposed to Mustache ({@link OperationsMap#setImports},
546+
* {@code hasImport}) from the per-operation {@link CodegenOperation#imports} sets.
547+
* <p><b>When:</b> call once at the end of {@link #postProcessOperationsWithModels}, after every
548+
* {@link CodegenOperation} has had its parameters processed (including {@link #normalizeEnumRefParameterDataType} /
549+
* {@link #syncEnumRefOperationImports}).
550+
* <p><b>Execution:</b>
551+
* <ol>
552+
* <li>Union all strings in {@code op.imports} across operations into a sorted {@link TreeSet} (stable, de-duplicated).</li>
553+
* <li>For each symbol, resolve {@link #importMapping()} or {@link #toModelImportMap(String)} into
554+
* {@code fullQualifiedImport → shortClassName} pairs (same shape as {@code DefaultGenerator#processOperations}).</li>
555+
* <li>Build {@code {import, classname}} rows sorted by {@code classname} for the template partial that emits
556+
* {@code use Full\\Qualified;}</li>
557+
* <li>Replace {@code operationsMap} imports and set {@code hasImport}.</li>
558+
* </ol>
559+
*/
560+
private void refreshAggregatedImportsForOperations(OperationsMap operationsMap) {
561+
OperationMap operationMap = operationsMap.getOperations();
562+
if (operationMap == null) {
563+
return;
564+
}
565+
List<CodegenOperation> operationList = operationMap.getOperation();
566+
if (operationList == null) {
567+
return;
568+
}
569+
Set<String> allImports = new TreeSet<>();
570+
for (CodegenOperation op : operationList) {
571+
if (op.imports != null) {
572+
allImports.addAll(op.imports);
573+
}
574+
}
575+
Map<String, String> mapped = new LinkedHashMap<>();
576+
for (String nextImport : allImports) {
577+
String mapping = importMapping().get(nextImport);
578+
if (mapping != null) {
579+
mapped.put(mapping, nextImport);
580+
} else {
581+
mapped.putAll(toModelImportMap(nextImport));
582+
}
583+
}
584+
Set<Map<String, String>> importObjects = new TreeSet<>(Comparator.comparing(o -> o.get("classname")));
585+
for (Map.Entry<String, String> e : mapped.entrySet()) {
586+
Map<String, String> row = new LinkedHashMap<>();
587+
row.put("import", e.getKey());
588+
row.put("classname", e.getValue());
589+
importObjects.add(row);
590+
}
591+
operationsMap.setImports(new ArrayList<>(importObjects));
592+
operationsMap.put("hasImport", !importObjects.isEmpty());
593+
}
594+
447595
private boolean isApplicationJsonOrApplicationXml(CodegenOperation op) {
448596
if (op.produces != null) {
449597
for(Map<String, String> produce : op.produces) {

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

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -59,12 +59,7 @@ interface {{classname}}
5959
*
6060
{{/description}}
6161
{{#allParams}}
62-
{{#isEnumRef}}
63-
* @param \{{modelPackage}}\{{dataType}}{{^required}}{{^defaultValue}}|null{{/defaultValue}}{{/required}} ${{paramName}} {{description}} {{#required}}(required){{/required}}{{^required}}(optional{{#defaultValue}}, default to {{{.}}}{{/defaultValue}}){{/required}}{{#isDeprecated}} (deprecated){{/isDeprecated}}
64-
{{/isEnumRef}}
65-
{{^isEnumRef}}
6662
* @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}}
67-
{{/isEnumRef}}
6863
{{/allParams}}
6964
* @param int &$responseCode The HTTP Response Code
7065
* @param array $responseHeaders Additional HTTP headers to return with the response ()
@@ -76,12 +71,7 @@ interface {{classname}}
7671
*/
7772
public function {{operationId}}(
7873
{{#allParams}}
79-
{{#isEnumRef}}
80-
{{^required}}{{^defaultValue}}?{{/defaultValue}}{{/required}}\{{modelPackage}}\{{dataType}} ${{paramName}},
81-
{{/isEnumRef}}
82-
{{^isEnumRef}}
8374
{{^required}}{{^defaultValue}}?{{/defaultValue}}{{/required}}{{#vendorExtensions.x-parameter-type}}{{vendorExtensions.x-parameter-type}} {{/vendorExtensions.x-parameter-type}}${{paramName}},
84-
{{/isEnumRef}}
8575
{{/allParams}}
8676
int &$responseCode,
8777
array &$responseHeaders

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -154,10 +154,10 @@ class {{controllerName}} extends Controller
154154
{{^isFile}}
155155
{{#isBodyParam}}
156156
$inputFormat = $request->getMimeType($request->getContentTypeFormat());
157-
${{paramName}} = $this->deserialize(${{paramName}}, '{{#isContainer}}{{#items}}array<{{dataType}}>{{/items}}{{/isContainer}}{{^isContainer}}{{#isEnumRef}}\{{modelPackage}}\{{dataType}}{{/isEnumRef}}{{^isEnumRef}}{{dataType}}{{/isEnumRef}}{{/isContainer}}', $inputFormat);
157+
${{paramName}} = $this->deserialize(${{paramName}}, '{{#isContainer}}{{#items}}array<{{dataType}}>{{/items}}{{/isContainer}}{{^isContainer}}{{#isEnumRef}}\{{{modelPackage}}}\{{{dataType}}}{{/isEnumRef}}{{^isEnumRef}}{{dataType}}{{/isEnumRef}}{{/isContainer}}', $inputFormat);
158158
{{/isBodyParam}}
159159
{{^isBodyParam}}
160-
${{paramName}} = $this->deserialize(${{paramName}}, '{{#isContainer}}array<{{collectionFormat}}{{^collectionFormat}}csv{{/collectionFormat}},{{dataType}}>{{/isContainer}}{{^isContainer}}{{#isEnumRef}}\{{modelPackage}}\{{dataType}}{{/isEnumRef}}{{^isEnumRef}}{{dataType}}{{/isEnumRef}}{{/isContainer}}', 'string');
160+
${{paramName}} = $this->deserialize(${{paramName}}, '{{#isContainer}}array<{{collectionFormat}}{{^collectionFormat}}csv{{/collectionFormat}},{{dataType}}>{{/isContainer}}{{^isContainer}}{{#isEnumRef}}\{{{modelPackage}}}\{{{dataType}}}{{/isEnumRef}}{{^isEnumRef}}{{dataType}}{{/isEnumRef}}{{/isContainer}}', 'string');
161161
{{/isBodyParam}}
162162
{{/isFile}}
163163
{{/allParams}}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@
3939
{{/isFile}}
4040
{{^isFile}}
4141
{{#isEnumRef}}
42-
$asserts[] = new Assert\Type("\{{modelPackage}}\{{dataType}}");
42+
$asserts[] = new Assert\Type('\{{{modelPackage}}}\{{{dataType}}}');
4343
{{/isEnumRef}}
4444
{{^isEnumRef}}
4545
$asserts[] = new Assert\Type('{{dataType}}');

modules/openapi-generator/src/test/java/org/openapitools/codegen/php/PhpSymfonyServerCodegenTest.java

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,13 +25,18 @@
2525
import org.openapitools.codegen.languages.AbstractPhpCodegen;
2626
import org.openapitools.codegen.languages.PhpSymfonyServerCodegen;
2727
import org.testng.Assert;
28+
import org.testng.SkipException;
2829
import org.testng.annotations.Test;
2930

3031
import java.io.File;
32+
import java.io.IOException;
33+
import java.nio.charset.StandardCharsets;
3134
import java.nio.file.Files;
3235
import java.util.HashMap;
3336
import java.util.List;
3437
import java.util.Map;
38+
import java.util.concurrent.TimeUnit;
39+
import java.util.regex.Pattern;
3540

3641

3742
public class PhpSymfonyServerCodegenTest {
@@ -173,4 +178,76 @@ public void testGeneratePingWithDifferentSourceDirectory() throws Exception {
173178

174179
output.deleteOnExit();
175180
}
181+
182+
/**
183+
* OpenAPI 3.1 + dotted schema keys + {@code components.parameters} {@code $ref}: enum-ref query
184+
* parameters must use a short model class name in {@code DefaultApiInterface} (aligned with
185+
* {@code use} imports), not a bogus flattened namespace token.
186+
* <p>
187+
* Also verifies PHPDoc {@code @param} uses the same short name (not {@code \\FQCN}) and, when
188+
* {@code php} is on {@code PATH}, that {@code php -l} accepts the generated file (valid syntax).
189+
*/
190+
@Test
191+
public void testPetstoreDottedEnumRefQueryParameterUsesShortClassInApiInterface() throws Exception {
192+
Map<String, Object> properties = new HashMap<>();
193+
properties.put("invokerPackage", "Org\\OpenAPITools\\Petstore");
194+
195+
File output = Files.createTempDirectory("test").toFile();
196+
197+
final CodegenConfigurator configurator = new CodegenConfigurator()
198+
.setGeneratorName("php-symfony")
199+
.setAdditionalProperties(properties)
200+
.setInputSpec("src/test/resources/3_1/php-symfony/petstore-dotted-enum-ref-query-param-component.yaml")
201+
.setOutputDir(output.getAbsolutePath().replace("\\", "/"));
202+
203+
final ClientOptInput clientOptInput = configurator.toClientOptInput();
204+
DefaultGenerator generator = new DefaultGenerator();
205+
List<File> files = generator.opts(clientOptInput).generate();
206+
207+
File apiInterfaceFile = files.stream()
208+
.filter(f -> "DefaultApiInterface.php".equals(f.getName()) && f.getPath().contains("Api" + File.separator))
209+
.findFirst()
210+
.orElseThrow(() -> new AssertionError("DefaultApiInterface.php not generated"));
211+
212+
String apiContent = Files.readString(apiInterfaceFile.toPath(), StandardCharsets.UTF_8);
213+
Assert.assertFalse(
214+
apiContent.contains("OrgOpenAPIToolsPetstoreModel"),
215+
"Must not emit flattened invoker+model token in interface");
216+
Assert.assertTrue(
217+
apiContent.contains("use Org\\OpenAPITools\\Petstore\\Model\\PetModelPetStatus;"),
218+
"Expected enum model import");
219+
Assert.assertTrue(
220+
apiContent.contains("?PetModelPetStatus $status"),
221+
"Expected enum ref query param to use short class in type hint");
222+
Assert.assertTrue(
223+
Pattern.compile("@param\\s+PetModelPetStatus\\|null\\s+\\$status\\b").matcher(apiContent).find(),
224+
"PHPDoc @param should use short PetModelPetStatus|null (consistent with use import)");
225+
Assert.assertFalse(
226+
apiContent.contains("?\\Org\\OpenAPITools\\Petstore\\Model\\PetModelPetStatus $status"),
227+
"Signature must not use leading-backslash FQCN when a matching use import exists");
228+
Assert.assertFalse(
229+
apiContent.contains("@param \\Org\\"),
230+
"PHPDoc @param must not use leading-backslash FQCN for enum ref");
231+
232+
assertGeneratedPhpSyntaxValid(apiInterfaceFile);
233+
234+
output.deleteOnExit();
235+
}
236+
237+
/**
238+
* Runs {@code php -l} on the file. Skips if {@code php} is not available (optional toolchain).
239+
*/
240+
private static void assertGeneratedPhpSyntaxValid(File phpFile) throws Exception {
241+
ProcessBuilder pb = new ProcessBuilder("php", "-l", phpFile.getAbsolutePath());
242+
pb.redirectErrorStream(true);
243+
final Process p;
244+
try {
245+
p = pb.start();
246+
} catch (IOException e) {
247+
throw new SkipException("php not available on PATH, skipping syntax check: " + e.getMessage());
248+
}
249+
Assert.assertTrue(p.waitFor(30, TimeUnit.SECONDS), "php -l timed out");
250+
String out = new String(p.getInputStream().readAllBytes(), StandardCharsets.UTF_8).trim();
251+
Assert.assertEquals(p.exitValue(), 0, "php -l must accept generated interface: " + out);
252+
}
176253
}
Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
openapi: 3.1.0
2+
info:
3+
title: php-symfony petstore dotted enum ref via components.parameters
4+
version: '1.0'
5+
paths:
6+
/pets:
7+
get:
8+
operationId: listPets
9+
parameters:
10+
- $ref: '#/components/parameters/Pet.HTTP.ListPetsRequest.status'
11+
responses:
12+
'200':
13+
description: OK
14+
components:
15+
parameters:
16+
Pet.HTTP.ListPetsRequest.status:
17+
name: status
18+
in: query
19+
required: false
20+
schema:
21+
$ref: '#/components/schemas/Pet.Model.PetStatus'
22+
default: available
23+
explode: false
24+
schemas:
25+
Pet.Model.PetStatus:
26+
type: string
27+
enum:
28+
- available
29+
- pending
30+
- sold

0 commit comments

Comments
 (0)