Skip to content

Commit 292041d

Browse files
committed
[OCaml] Fix bugs around enum parsing
Problem: The OCaml client generator threw IllegalArgumentException: Unreferenced enum when encountering enums inside composed schemas (anyOf/allOf/oneOf). Root Causes: 1. The enum collection logic didn't traverse into composed schemas 2. The enum hashing used order-dependent string concatenation, causing lookups to fail when enum values appeared in different orders 3. Enums directly within composed schema branches (not in properties) weren't collected Solution: 1. Added composed schema support: - New `collectEnumSchemasFromComposed()` method handles `anyOf/allOf/oneOf` - New `collectEnumSchemasFromList()` method recursively processes composed schema branches - Enums directly in composed schemas (not just in properties) are now collected 2. Refactored enum hashing to use Set: - Changed from comma-joined strings to `TreeSet<String>` for order-independent, collision-free hashing - Handles edge cases like empty string enums `""` 3. Added test case: - Tests enums in nested composed schemas - Tests enum with empty string value in anyOf
1 parent 9432aaf commit 292041d

29 files changed

Lines changed: 679 additions & 97 deletions

File tree

.github/workflows/samples-ocaml.yaml

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,12 +7,14 @@ on:
77
- 'samples/client/petstore/ocaml-fake-petstore/**'
88
- 'samples/client/petstore/ocaml-oneOf-primitive/**'
99
- 'samples/client/petstore/ocaml-additional-properties/**'
10+
- 'samples/client/petstore/ocaml-enum-in-composed-schema/**'
1011
pull_request:
1112
paths:
1213
- 'samples/client/petstore/ocaml/**'
1314
- 'samples/client/petstore/ocaml-fake-petstore/**'
1415
- 'samples/client/petstore/ocaml-oneOf-primitive/**'
1516
- 'samples/client/petstore/ocaml-additional-properties/**'
17+
- 'samples/client/petstore/ocaml-enum-in-composed-schema/**'
1618

1719
jobs:
1820
build:
@@ -26,12 +28,13 @@ jobs:
2628
- 'samples/client/petstore/ocaml-fake-petstore/'
2729
- 'samples/client/petstore/ocaml-oneOf-primitive/'
2830
- 'samples/client/petstore/ocaml-additional-properties/'
31+
- 'samples/client/petstore/ocaml-enum-in-composed-schema/'
2932
steps:
3033
- uses: actions/checkout@v5
3134
- name: Set-up OCaml
3235
uses: ocaml/setup-ocaml@v3
3336
with:
34-
ocaml-compiler: 5
37+
ocaml-compiler: 5.3
3538
- name: Install
3639
run: opam install . --deps-only --with-test
3740
working-directory: ${{ matrix.sample }}

.gitignore

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -304,6 +304,7 @@ samples/client/petstore/ocaml/_build/
304304
samples/client/petstore/ocaml-fake-petstore/_build/
305305
samples/client/petstore/ocaml-oneOf-primitive/_build/
306306
samples/client/petstore/ocaml-additional-properties/_build/
307+
samples/client/petstore/ocaml-enum-in-composed-schema/_build/
307308

308309
# jetbrain http client
309310
samples/client/jetbrains/adyen/checkout71/http/client/Apis/http-client.private.env.json
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
generatorName: ocaml
2+
outputDir: samples/client/petstore/ocaml-enum-in-composed-schema
3+
inputSpec: modules/openapi-generator/src/test/resources/3_0/ocaml/enum-in-composed-schema.yaml
4+
templateDir: modules/openapi-generator/src/main/resources/ocaml
5+
additionalProperties:
6+
packageName: petstore_client

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

Lines changed: 58 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -58,9 +58,9 @@ public class OCamlClientCodegen extends DefaultCodegen implements CodegenConfig
5858
protected String apiFolder = "src/apis";
5959
protected String modelFolder = "src/models";
6060

61-
private Map<String, List<String>> enumNames = new HashMap<>();
62-
private Map<String, Schema> enumHash = new HashMap<>();
63-
private Map<String, String> enumUniqNames;
61+
private Map<Set<String>, List<String>> enumNames = new HashMap<>();
62+
private Map<Set<String>, Schema> enumHash = new HashMap<>();
63+
private Map<Set<String>, String> enumUniqNames;
6464

6565
@Override
6666
public CodegenType getTag() {
@@ -276,8 +276,10 @@ protected void updateDataTypeWithEnumForArray(CodegenProperty property) {
276276
}
277277

278278
@SuppressWarnings("unchecked")
279-
private String hashEnum(Schema schema) {
280-
return ((List<Object>) schema.getEnum()).stream().map(String::valueOf).collect(Collectors.joining(","));
279+
private Set<String> hashEnum(Schema schema) {
280+
return ((List<Object>) schema.getEnum()).stream()
281+
.map(String::valueOf)
282+
.collect(Collectors.toCollection(TreeSet::new));
281283
}
282284

283285
private boolean isEnumSchema(Schema schema) {
@@ -290,7 +292,7 @@ private void collectEnumSchemas(String parentName, String sName, Schema schema)
290292
} else if (ModelUtils.isMapSchema(schema) && schema.getAdditionalProperties() instanceof Schema) {
291293
collectEnumSchemas(parentName, sName, (Schema) schema.getAdditionalProperties());
292294
} else if (isEnumSchema(schema)) {
293-
String h = hashEnum(schema);
295+
Set<String> h = hashEnum(schema);
294296
if (!enumHash.containsKey(h)) {
295297
enumHash.put(h, schema);
296298
enumNames.computeIfAbsent(h, k -> new ArrayList<>()).add(sName.toLowerCase(Locale.ROOT));
@@ -299,6 +301,8 @@ private void collectEnumSchemas(String parentName, String sName, Schema schema)
299301
}
300302
}
301303
}
304+
// Note: Composed schemas (anyOf, allOf, oneOf) are handled in the Map-based method
305+
// via collectEnumSchemasFromComposed() which properly processes their structure
302306
}
303307

304308
private void collectEnumSchemas(String sName, Schema schema) {
@@ -327,6 +331,47 @@ private void collectEnumSchemas(String parentName, Map<String, Schema> schemas)
327331
collectEnumSchemas(pName, ModelUtils.getSchemaItems(schema));
328332
}
329333
}
334+
335+
// Handle composed schemas (anyOf, allOf, oneOf) - recursively process their structure
336+
collectEnumSchemasFromComposed(pName, schema);
337+
}
338+
}
339+
340+
private void collectEnumSchemasFromComposed(String parentName, Schema schema) {
341+
if (schema.getAnyOf() != null) {
342+
collectEnumSchemasFromList(parentName, schema.getAnyOf());
343+
}
344+
345+
if (schema.getAllOf() != null) {
346+
collectEnumSchemasFromList(parentName, schema.getAllOf());
347+
}
348+
349+
if (schema.getOneOf() != null) {
350+
collectEnumSchemasFromList(parentName, schema.getOneOf());
351+
}
352+
}
353+
354+
private void collectEnumSchemasFromList(String parentName, List<Schema> schemas) {
355+
int index = 0;
356+
for (Schema composedSchema : schemas) {
357+
// Check if the composed schema itself is an enum
358+
if (isEnumSchema(composedSchema)) {
359+
String enumName = composedSchema.getName() != null ? composedSchema.getName() : "any_of_" + index;
360+
collectEnumSchemas(parentName, enumName, composedSchema);
361+
}
362+
363+
if (composedSchema.getProperties() != null) {
364+
collectEnumSchemas(parentName, composedSchema.getProperties());
365+
}
366+
if (composedSchema.getAdditionalProperties() != null && composedSchema.getAdditionalProperties() instanceof Schema) {
367+
collectEnumSchemas(parentName, composedSchema.getName(), (Schema) composedSchema.getAdditionalProperties());
368+
}
369+
if (ModelUtils.isArraySchema(composedSchema) && ModelUtils.getSchemaItems(composedSchema) != null) {
370+
collectEnumSchemas(parentName, composedSchema.getName(), ModelUtils.getSchemaItems(composedSchema));
371+
}
372+
// Recursively handle nested composed schemas
373+
collectEnumSchemasFromComposed(parentName, composedSchema);
374+
index++;
330375
}
331376
}
332377

@@ -378,8 +423,8 @@ private String sanitizeOCamlTypeName(String name) {
378423
}
379424

380425
private void computeEnumUniqNames() {
381-
Map<String, String> definitiveNames = new HashMap<>();
382-
for (String h : enumNames.keySet()) {
426+
Map<String, Set<String>> definitiveNames = new HashMap<>();
427+
for (Set<String> h : enumNames.keySet()) {
383428
boolean hasDefName = false;
384429
List<String> nameCandidates = enumNames.get(h);
385430
for (String name : nameCandidates) {
@@ -600,13 +645,13 @@ public String getTypeDeclaration(Schema p) {
600645
String prefix = inner.getEnum() != null ? "Enums." : "";
601646
return "(string * " + prefix + getTypeDeclaration(inner) + ") list";
602647
} else if (p.getEnum() != null) {
603-
String h = hashEnum(p);
648+
Set<String> h = hashEnum(p);
604649
return enumUniqNames.get(h);
605650
}
606651

607652
Schema referencedSchema = ModelUtils.getReferencedSchema(openAPI, p);
608653
if (referencedSchema != null && referencedSchema.getEnum() != null) {
609-
String h = hashEnum(referencedSchema);
654+
Set<String> h = hashEnum(referencedSchema);
610655
return "Enums." + enumUniqNames.get(h);
611656
}
612657

@@ -739,8 +784,8 @@ public OperationsMap postProcessOperationsWithModels(OperationsMap objs, List<Mo
739784
}
740785
}
741786

742-
for (Map.Entry<String, String> e : enumUniqNames.entrySet()) {
743-
allModels.add(buildEnumModelWrapper(e.getValue(), e.getKey()));
787+
for (Map.Entry<Set<String>, String> e : enumUniqNames.entrySet()) {
788+
allModels.add(buildEnumModelWrapper(e.getValue(), String.join(",", e.getKey())));
744789
}
745790

746791
enumUniqNames.clear();
@@ -770,7 +815,7 @@ public String escapeUnsafeCharacters(String input) {
770815

771816
@Override
772817
public String toEnumName(CodegenProperty property) {
773-
String hash = String.join(",", property.get_enum());
818+
Set<String> hash = new TreeSet<>(property.get_enum());
774819

775820
if (enumUniqNames.containsKey(hash)) {
776821
return enumUniqNames.get(hash);
Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
openapi: 3.0.0
2+
info:
3+
title: Test Enum in Composed Schema
4+
version: 1.0.0
5+
paths:
6+
/test:
7+
get:
8+
operationId: getTest
9+
responses:
10+
'200':
11+
description: Success
12+
content:
13+
application/json:
14+
schema:
15+
$ref: '#/components/schemas/TestModel'
16+
components:
17+
schemas:
18+
TestModel:
19+
type: object
20+
properties:
21+
name:
22+
type: string
23+
config:
24+
anyOf:
25+
- allOf:
26+
- type: object
27+
properties:
28+
type:
29+
type: string
30+
enum:
31+
- type1
32+
- type2
33+
- type3
34+
options:
35+
type: object
36+
properties:
37+
value:
38+
type: string
39+
enum:
40+
- value1
41+
- value2
42+
- value3
43+
optionalUrl:
44+
anyOf:
45+
- type: string
46+
format: uri
47+
- type: string
48+
enum:
49+
- ""
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
version=0.27.0
2+
ocaml-version=4.14.0
Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
# OpenAPI Generator Ignore
2+
# Generated by openapi-generator https://github.com/openapitools/openapi-generator
3+
4+
# Use this file to prevent files from being overwritten by the generator.
5+
# The patterns follow closely to .gitignore or .dockerignore.
6+
7+
# As an example, the C# client generator defines ApiClient.cs.
8+
# You can make changes and tell OpenAPI Generator to ignore just this file by uncommenting the following line:
9+
#ApiClient.cs
10+
11+
# You can match any string of characters against a directory, file or extension with a single asterisk (*):
12+
#foo/*/qux
13+
# The above matches foo/bar/qux and foo/baz/qux, but not foo/bar/baz/qux
14+
15+
# You can recursively match patterns against a directory, file or extension with a double asterisk (**):
16+
#foo/**/qux
17+
# This matches foo/bar/qux, foo/baz/qux, and foo/bar/baz/qux
18+
19+
# You can also negate patterns with an exclamation (!).
20+
# For example, you can ignore all files in a docs folder with the file extension .md:
21+
#docs/*.md
22+
# Then explicitly reverse the ignore rule for a single file:
23+
#!docs/README.md
Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
.ocamlformat
2+
README.md
3+
dune
4+
dune-project
5+
petstore_client.opam
6+
src/apis/default_api.ml
7+
src/apis/default_api.mli
8+
src/models/test_model.ml
9+
src/models/test_model_config.ml
10+
src/models/test_model_config_all_of_options.ml
11+
src/models/test_model_optional_url.ml
12+
src/support/enums.ml
13+
src/support/jsonSupport.ml
14+
src/support/request.ml
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
7.21.0-SNAPSHOT
Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
#
2+
No description provided (generated by Openapi Generator https://github.com/openapitools/openapi-generator)
3+
4+
This OCaml package is automatically generated by the [OpenAPI Generator](https://openapi-generator.tech) project:
5+
6+
- API version: 1.0.0
7+
- Package version: 1.0.0
8+
- Generator version: 7.21.0-SNAPSHOT
9+
- Build package: org.openapitools.codegen.languages.OCamlClientCodegen
10+
11+
## Requirements.
12+
13+
OCaml 5.x
14+
15+
## Installation
16+
17+
Please run the following commands to build the package `petstore_client`:
18+
19+
```sh
20+
opam install . --deps-only --with-test
21+
eval $(opam env)
22+
dune build
23+
```
24+
25+
## Getting Started
26+
27+
The generated directory structure is:
28+
- `src/apis`: contains several modules, each with several functions. Each function is an API endpoint.
29+
- `src/models`: contains several modules. Each module contains:
30+
- a type `t` representing an input and/or output schema of the OpenAPI spec
31+
- a smart constructor `create` for this type
32+
- `src/support`: various modules used by the generated APIs and Models
33+

0 commit comments

Comments
 (0)