fix(csharp): limit public oneOf constructors to operation inputs (#24700)

Co-authored-by: Kanna Kim <kimkanna18@gmail.com>

Author: wing328

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

@@ -524,6 +524,7 @@ public static enum ENUM_PROPERTY_NAMING_TYPE {camelCase, PascalCase, snake_case,
     public static final String X_MODIFIERS = "x-modifiers";
     public static final String X_MODIFIER_PREFIX = "x-modifier-";
     public static final String X_MODEL_IS_MUTABLE = "x-model-is-mutable";
+    public static final String X_MODEL_IS_OPERATION_INPUT = "x-model-is-operation-input";
     public static final String X_IMPLEMENTS = "x-implements";
     public static final String X_IS_ONE_OF_INTERFACE = "x-is-one-of-interface";
     public static final String USE_ENUM_VALUE_INTERFACE = "useEnumValueInterface";

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

@@ -21,7 +21,12 @@
 import com.samskivert.mustache.Mustache;
 import com.samskivert.mustache.Mustache.Lambda;
 import com.samskivert.mustache.Template;
+import io.swagger.v3.oas.models.Operation;
+import io.swagger.v3.oas.models.PathItem;
+import io.swagger.v3.oas.models.media.Content;
 import io.swagger.v3.oas.models.media.Schema;
+import io.swagger.v3.oas.models.parameters.Parameter;
+import io.swagger.v3.oas.models.parameters.RequestBody;
 import lombok.Getter;
 import lombok.Setter;
 import org.apache.commons.io.FilenameUtils;
@@ -616,6 +621,7 @@ public ModelsMap postProcessModels(ModelsMap objs) {
     @Override
     public Map<String, ModelsMap> postProcessAllModels(Map<String, ModelsMap> objs) {
         final Map<String, ModelsMap> processed = super.postProcessAllModels(objs);
+        final Set<String> operationInputModels = getOperationInputModels();
 
         Map<String, CodegenModel> enumRefs = new HashMap<>();
         for (Map.Entry<String, ModelsMap> entry : processed.entrySet()) {
@@ -645,6 +651,9 @@ public Map<String, ModelsMap> postProcessAllModels(Map<String, ModelsMap> objs)
                 continue;
             }
 
+            if (operationInputModels.contains(entry.getKey()) || operationInputModels.contains(model.schemaName)) {
+                model.vendorExtensions.put(X_MODEL_IS_OPERATION_INPUT, true);
+            }
             model.vendorExtensions.put(X_MODEL_IS_MUTABLE, modelIsMutable(model, null));
 
             CodegenComposedSchemas composedSchemas = model.getComposedSchemas();
@@ -718,6 +727,77 @@ public Map<String, ModelsMap> postProcessAllModels(Map<String, ModelsMap> objs)
         return processed;
     }
 
+    private Set<String> getOperationInputModels() {
+        Set<String> operationInputModels = new HashSet<>();
+        Set<String> visitedModels = new HashSet<>();
+        if (openAPI == null || openAPI.getPaths() == null) {
+            return operationInputModels;
+        }
+
+        for (PathItem pathItem : openAPI.getPaths().values()) {
+            collectOperationInputModels(pathItem.getParameters(), operationInputModels, visitedModels);
+            for (Operation operation : pathItem.readOperations()) {
+                collectOperationInputModels(operation.getParameters(), operationInputModels, visitedModels);
+                RequestBody requestBody = ModelUtils.getReferencedRequestBody(openAPI, operation.getRequestBody());
+                if (requestBody != null) {
+                    collectOperationInputModels(requestBody.getContent(), operationInputModels, visitedModels);
+                }
+            }
+        }
+        return operationInputModels;
+    }
+
+    private void collectOperationInputModels(List<Parameter> parameters, Set<String> operationInputModels, Set<String> visitedModels) {
+        if (parameters == null) {
+            return;
+        }
+        for (Parameter unresolvedParameter : parameters) {
+            Parameter parameter = ModelUtils.getReferencedParameter(openAPI, unresolvedParameter);
+            if (parameter != null) {
+                collectOperationInputModels(parameter.getSchema(), operationInputModels, visitedModels);
+                collectOperationInputModels(parameter.getContent(), operationInputModels, visitedModels);
+            }
+        }
+    }
+
+    private void collectOperationInputModels(Content content, Set<String> operationInputModels, Set<String> visitedModels) {
+        if (content == null) {
+            return;
+        }
+        content.values().forEach(mediaType -> collectOperationInputModels(mediaType.getSchema(), operationInputModels, visitedModels));
+    }
+
+    private void collectOperationInputModels(Schema schema, Set<String> operationInputModels, Set<String> visitedModels) {
+        if (schema == null || Boolean.TRUE.equals(schema.getReadOnly())) {
+            return;
+        }
+        if (schema.get$ref() != null) {
+            String modelName = ModelUtils.getSimpleRef(schema.get$ref());
+            operationInputModels.add(modelName);
+            if (visitedModels.add(modelName)) {
+                collectOperationInputModels(ModelUtils.getSchema(openAPI, modelName), operationInputModels, visitedModels);
+            }
+        }
+        if (schema.getOneOf() != null) {
+            schema.getOneOf().forEach(child -> collectOperationInputModels((Schema) child, operationInputModels, visitedModels));
+        }
+        if (schema.getAllOf() != null) {
+            schema.getAllOf().forEach(child -> collectOperationInputModels((Schema) child, operationInputModels, visitedModels));
+        }
+        if (schema.getAnyOf() != null) {
+            schema.getAnyOf().forEach(child -> collectOperationInputModels((Schema) child, operationInputModels, visitedModels));
+        }
+        // TODO: Traverse OAS 3.1 schema keywords that can reference operation-input models,
+        // such as prefixItems, contains, if/then/else, dependentSchemas, and unevaluatedProperties.
+        collectOperationInputModels(ModelUtils.getSchemaItems(schema), operationInputModels, visitedModels); // in case schema has array type
+        if (schema.getAdditionalProperties() instanceof Schema) {
+            collectOperationInputModels((Schema) schema.getAdditionalProperties(), operationInputModels, visitedModels);
+        }
+        if (schema.getProperties() != null) {
+            schema.getProperties().values().forEach(property -> collectOperationInputModels((Schema) property, operationInputModels, visitedModels));
+        }
+    }
+
     /**
      * Returns true if the model contains any properties with a public setter
      * If true, the model's constructor accessor should be made public to ensure end users

modules/openapi-generator/src/main/resources/csharp/libraries/generichost/modelGeneric.mustache

@@ -17,7 +17,7 @@
         /// <param name="{{#lambda.camel_case}}{{name}}{{/lambda.camel_case}}">{{description}}{{^description}}{{#lambda.camel_case}}{{name}}{{/lambda.camel_case}}{{/description}}{{#defaultValue}} (default to {{.}}){{/defaultValue}}</param>
         {{/isDiscriminator}}
         {{/allVars}}
-        {{#model.vendorExtensions.x-model-is-mutable}}{{>visibility}}{{/model.vendorExtensions.x-model-is-mutable}}{{^model.vendorExtensions.x-model-is-mutable}}internal{{/model.vendorExtensions.x-model-is-mutable}} {{classname}}({{#lambda.joinWithComma}}{{{dataType}}} {{#lambda.escape_reserved_word}}{{#lambda.camel_case}}{{name}}{{/lambda.camel_case}}{{/lambda.escape_reserved_word}}  {{#model.composedSchemas.anyOf}}{{^required}}Option<{{/required}}{{{dataType}}}{{>NullConditionalProperty}}{{^required}}>{{/required}} {{#lambda.escape_reserved_word}}{{#lambda.camel_case}}{{baseType}}{{/lambda.camel_case}}{{/lambda.escape_reserved_word}}  {{/model.composedSchemas.anyOf}}{{>ModelSignature}}{{/lambda.joinWithComma}}){{#parent}} : base({{#lambda.joinWithComma}}{{#parentModel.composedSchemas.oneOf}}{{#lambda.escape_reserved_word}}{{#lambda.camel_case}}{{parent}}{{/lambda.camel_case}}{{/lambda.escape_reserved_word}}.{{#lambda.titlecase}}{{baseType}}{{/lambda.titlecase}}  {{/parentModel.composedSchemas.oneOf}}{{>ModelBaseSignature}}{{/lambda.joinWithComma}}){{/parent}}
+        {{#model.vendorExtensions.x-model-is-operation-input}}{{>visibility}}{{/model.vendorExtensions.x-model-is-operation-input}}{{^model.vendorExtensions.x-model-is-operation-input}}internal{{/model.vendorExtensions.x-model-is-operation-input}} {{classname}}({{#lambda.joinWithComma}}{{{dataType}}} {{#lambda.escape_reserved_word}}{{#lambda.camel_case}}{{name}}{{/lambda.camel_case}}{{/lambda.escape_reserved_word}}  {{#model.composedSchemas.anyOf}}{{^required}}Option<{{/required}}{{{dataType}}}{{>NullConditionalProperty}}{{^required}}>{{/required}} {{#lambda.escape_reserved_word}}{{#lambda.camel_case}}{{baseType}}{{/lambda.camel_case}}{{/lambda.escape_reserved_word}}  {{/model.composedSchemas.anyOf}}{{>ModelSignature}}{{/lambda.joinWithComma}}){{#parent}} : base({{#lambda.joinWithComma}}{{#parentModel.composedSchemas.oneOf}}{{#lambda.escape_reserved_word}}{{#lambda.camel_case}}{{parent}}{{/lambda.camel_case}}{{/lambda.escape_reserved_word}}.{{#lambda.titlecase}}{{baseType}}{{/lambda.titlecase}}  {{/parentModel.composedSchemas.oneOf}}{{>ModelBaseSignature}}{{/lambda.joinWithComma}}){{/parent}}
         {
             {{#composedSchemas.anyOf}}
             {{#lambda.titlecase}}{{name}}{{/lambda.titlecase}}{{^required}}Option{{/required}} = {{#lambda.escape_reserved_word}}{{#lambda.camel_case}}{{name}}{{/lambda.camel_case}}{{/lambda.escape_reserved_word}};

modules/openapi-generator/src/test/java/org/openapitools/codegen/csharpnetcore/CSharpClientCodegenTest.java

@@ -47,6 +47,42 @@
 
 public class CSharpClientCodegenTest {
 
+    @Test
+    public void testGenericHostOneOfConstructorsRespectApiVisibility() throws IOException {
+        Map<String, File> publicModels = generateIssue23046Models(false);
+        File publicModel = publicModels.get("DynamicMetadataValue");
+        assertFileContains(publicModel.toPath(),
+                "public DynamicMetadataValue(string @string)",
+                "public DynamicMetadataValue(decimal @decimal)",
+                "public DynamicMetadataValue(bool @bool)",
+                "public DynamicMetadataValue(DateTime dateTime)");
+        assertFileNotContains(publicModel.toPath(), "internal DynamicMetadataValue(");
+        assertFileContains(publicModels.get("ResponseOnlyValue").toPath(),
+                "internal ResponseOnlyValue(string @string)",
+                "internal ResponseOnlyValue(int @int)");
+        assertFileNotContains(publicModels.get("ResponseOnlyValue").toPath(), "public ResponseOnlyValue(");
+        assertFileContains(publicModels.get("QueryParameterValue").toPath(),
+                "public QueryParameterValue(string @string)",
+                "public QueryParameterValue(int @int)");
+        assertFileNotContains(publicModels.get("QueryParameterValue").toPath(), "internal QueryParameterValue(");
+        assertFileContains(publicModels.get("ReadOnlyValue").toPath(),
+                "internal ReadOnlyValue(string @string)",
+                "internal ReadOnlyValue(int @int)");
+        assertFileNotContains(publicModels.get("ReadOnlyValue").toPath(), "public ReadOnlyValue(");
+        assertFileContains(publicModels.get("ForbiddenValue").toPath(),
+                "internal ForbiddenValue(string @string)",
+                "internal ForbiddenValue(int @int)");
+        assertFileNotContains(publicModels.get("ForbiddenValue").toPath(), "public ForbiddenValue(");
+
+        File internalModel = generateIssue23046Models(true).get("DynamicMetadataValue");
+        assertFileContains(internalModel.toPath(), 
+                "internal DynamicMetadataValue(string @string)",
+                "internal DynamicMetadataValue(decimal @decimal)",
+                "internal DynamicMetadataValue(bool @bool)",
+                "internal DynamicMetadataValue(DateTime dateTime)");
+        assertFileNotContains(internalModel.toPath(), "public DynamicMetadataValue(");
+    }
+
     @Test
     public void testGenericHostInnerStringEnumUnknownHandlingPreservesNullBehavior() throws IOException {
         // Unknown enum values must throw for both nullable and non-nullable properties.
@@ -539,4 +575,37 @@ public void testMapResponse() throws Exception {
         Assert.assertFalse(cr1.isModel);
         Assert.assertTrue(cr1.isMap);
     }
+
+    private Map<String, File> generateIssue23046Models(boolean nonPublicApi) throws IOException {
+        File output = Files.createTempDirectory("test").toFile().getCanonicalFile();
+        output.deleteOnExit();
+        final OpenAPI openAPI = TestUtils.parseFlattenSpec("src/test/resources/3_0/csharp/issue_23046.yaml");
+        final DefaultGenerator defaultGenerator = new DefaultGenerator();
+        final ClientOptInput clientOptInput = new ClientOptInput();
+        clientOptInput.openAPI(openAPI);
+        CSharpClientCodegen cSharpClientCodegen = new CSharpClientCodegen();
+        cSharpClientCodegen.setLibrary("generichost");
+        cSharpClientCodegen.setOutputDir(output.getAbsolutePath());
+        cSharpClientCodegen.setNonPublicApi(nonPublicApi);
+        clientOptInput.config(cSharpClientCodegen);
+        defaultGenerator.opts(clientOptInput);
+
+        Map<String, File> files = defaultGenerator.generate().stream()
+                .collect(Collectors.toMap(File::getPath, Function.identity()));
+        Map<String, File> models = Map.of(
+                "DynamicMetadataValue", getGeneratedModel(files, output, "DynamicMetadataValue"),
+                "ResponseOnlyValue", getGeneratedModel(files, output, "ResponseOnlyValue"),
+                "QueryParameterValue", getGeneratedModel(files, output, "QueryParameterValue"),
+                "ReadOnlyValue", getGeneratedModel(files, output, "ReadOnlyValue"),
+                "ForbiddenValue", getGeneratedModel(files, output, "ForbiddenValue"));
+        return models;
+    }
+
+    private File getGeneratedModel(Map<String, File> files, File output, String modelName) {
+        String path = Paths.get(output.getAbsolutePath(),
+                "src", "Org.OpenAPITools", "Model", modelName + ".cs").toString();
+        File model = files.get(path);
+        assertNotNull(model, "Could not find generated model: " + path);
+        return model;
+    }
 }

modules/openapi-generator/src/test/resources/3_0/csharp/issue_23046.yaml

@@ -0,0 +1,69 @@
+openapi: 3.0.3
+info:
+  title: Minimal repro
+  version: 1.0.0
+paths:
+  /upload:
+    post:
+      requestBody:
+        content:
+          application/json:
+            schema:
+              $ref: '#/components/schemas/InitiateUploadRequest'
+      responses:
+        '200':
+          description: OK
+          content:
+            application/json:
+              schema:
+                $ref: '#/components/schemas/ResponseOnlyValue'
+  /search:
+    get:
+      parameters:
+        - name: filter
+          in: query
+          schema:
+            $ref: '#/components/schemas/QueryParameterValue'
+      responses:
+        '204':
+          description: No content
+components:
+  schemas:
+    InitiateUploadRequest:
+      type: object
+      properties:
+        dynamicMetadata:
+          type: object
+          additionalProperties:
+            $ref: '#/components/schemas/DynamicMetadataValue'
+        readOnlyMetadata:
+          type: object
+          readOnly: true
+          additionalProperties:
+            $ref: '#/components/schemas/ReadOnlyValue'
+        allowedValue:
+          not:
+            $ref: '#/components/schemas/ForbiddenValue'
+    DynamicMetadataValue:
+      oneOf:
+        - type: string
+        - type: number
+        - type: boolean
+        - type: string
+          format: date-time
+    ResponseOnlyValue:
+      oneOf:
+        - type: string
+        - type: integer
+    QueryParameterValue:
+      oneOf:
+        - type: string
+        - type: integer
+    ReadOnlyValue:
+      oneOf:
+        - type: string
+        - type: integer
+    ForbiddenValue:
+      oneOf:
+        - type: string
+        - type: integer

samples/client/petstore/csharp/generichost/latest/OneOfList/src/Org.OpenAPITools/Model/OneOfArrayRequest.cs

@@ -34,7 +34,7 @@ public partial class OneOfArrayRequest : IValidatableObject
         /// Initializes a new instance of the <see cref="OneOfArrayRequest" /> class.
         /// </summary>
         /// <param name="list"></param>
-        internal OneOfArrayRequest(List<string> list)
+        public OneOfArrayRequest(List<string> list)
         {
             List = list;
             OnCreated();
@@ -44,7 +44,7 @@ internal OneOfArrayRequest(List<string> list)
         /// Initializes a new instance of the <see cref="OneOfArrayRequest" /> class.
         /// </summary>
         /// <param name="list1"></param>
-        internal OneOfArrayRequest(List<TestObject> list1)
+        public OneOfArrayRequest(List<TestObject> list1)
         {
             List1 = list1;
             OnCreated();

samples/client/petstore/csharp/generichost/latest/UseDateTimeOffset/src/Org.OpenAPITools/Model/Fruit.cs

@@ -35,7 +35,7 @@ public partial class Fruit : IValidatableObject
         /// </summary>
         /// <param name="apple"></param>
         /// <param name="color">color</param>
-        public Fruit(Apple apple, Option<string?> color = default)
+        internal Fruit(Apple apple, Option<string?> color = default)
         {
             Apple = apple;
             ColorOption = color;
@@ -47,7 +47,7 @@ public Fruit(Apple apple, Option<string?> color = default)
         /// </summary>
         /// <param name="banana"></param>
         /// <param name="color">color</param>
-        public Fruit(Banana banana, Option<string?> color = default)
+        internal Fruit(Banana banana, Option<string?> color = default)
         {
             Banana = banana;
             ColorOption = color;

samples/client/petstore/csharp/generichost/latest/UseDateTimeOffset/src/Org.OpenAPITools/Model/FruitReq.cs

@@ -34,7 +34,7 @@ public partial class FruitReq : IValidatableObject
         /// Initializes a new instance of the <see cref="FruitReq" /> class.
         /// </summary>
         /// <param name="appleReq"></param>
-        public FruitReq(AppleReq appleReq)
+        internal FruitReq(AppleReq appleReq)
         {
             AppleReq = appleReq;
             OnCreated();
@@ -44,7 +44,7 @@ public FruitReq(AppleReq appleReq)
         /// Initializes a new instance of the <see cref="FruitReq" /> class.
         /// </summary>
         /// <param name="bananaReq"></param>
-        public FruitReq(BananaReq bananaReq)
+        internal FruitReq(BananaReq bananaReq)
         {
             BananaReq = bananaReq;
             OnCreated();

samples/client/petstore/csharp/generichost/latest/UseDateTimeOffset/src/Org.OpenAPITools/Model/Mammal.cs

@@ -34,7 +34,7 @@ public partial class Mammal : IValidatableObject
         /// Initializes a new instance of the <see cref="Mammal" /> class.
         /// </summary>
         /// <param name="whale"></param>
-        public Mammal(Whale whale)
+        internal Mammal(Whale whale)
         {
             Whale = whale;
             OnCreated();
@@ -44,7 +44,7 @@ public Mammal(Whale whale)
         /// Initializes a new instance of the <see cref="Mammal" /> class.
         /// </summary>
         /// <param name="zebra"></param>
-        public Mammal(Zebra zebra)
+        internal Mammal(Zebra zebra)
         {
             Zebra = zebra;
             OnCreated();
@@ -54,7 +54,7 @@ public Mammal(Zebra zebra)
         /// Initializes a new instance of the <see cref="Mammal" /> class.
         /// </summary>
         /// <param name="pig"></param>
-        public Mammal(Pig pig)
+        internal Mammal(Pig pig)
         {
             Pig = pig;
             OnCreated();