[http-client-csharp] Omit model factory method if it contains interna…

…l param type (#5657) fixes: #4913
microsoft · Jan 17, 2025 · 428f839 · 428f839
1 parent 4cc9005
commit 428f839
Show file tree

Hide file tree

Showing 4 changed files with 90 additions and 2 deletions.
diff --git a/...-client-csharp/generator/Microsoft.Generator.CSharp/src/Providers/ModelFactoryProvider.cs b/...-client-csharp/generator/Microsoft.Generator.CSharp/src/Providers/ModelFactoryProvider.cs
@@ -70,16 +70,23 @@ protected override MethodProvider[] BuildMethods()
             foreach (var model in _models)
             {
                 var modelProvider = CodeModelPlugin.Instance.TypeFactory.CreateModel(model);
-                if (modelProvider is null || modelProvider.DeclarationModifiers.HasFlag(TypeSignatureModifiers.Internal))
+
+                if (modelProvider is null)
                     continue;
 
+                var fullConstructor = modelProvider.FullConstructor;
+                if (modelProvider.DeclarationModifiers.HasFlag(TypeSignatureModifiers.Internal)
+                    || fullConstructor.Signature.Parameters.Any(p => !p.Type.IsPublic))
+                {
+                    continue;
+                }
+
                 var typeToInstantiate = modelProvider.DeclarationModifiers.HasFlag(TypeSignatureModifiers.Abstract)
                     ? modelProvider.DerivedModels.FirstOrDefault(m => m.IsUnknownDiscriminatorModel)
                     : modelProvider;
                 if (typeToInstantiate is null)
                     continue;
 
-                var fullConstructor = modelProvider.FullConstructor;
                 var binaryDataParam = fullConstructor.Signature.Parameters.FirstOrDefault(p => p.Name.Equals(AdditionalBinaryDataParameterName));
 
                 // Use a custom constructor if the generated full constructor was suppressed or customized

diff --git a/...rosoft.Generator.CSharp/test/Providers/ModelFactories/ModelFactoriesCustomizationTests.cs b/...rosoft.Generator.CSharp/test/Providers/ModelFactories/ModelFactoriesCustomizationTests.cs
@@ -125,6 +125,56 @@ public async Task CanChangeAccessibilityOfModelFactory()
             ValidateModelFactoryCommon(modelFactory);
         }
 
+        // This test validates that the model factory method for a model is omitted if the
+        // model type is customized to be internal.
+        [Test]
+        public async Task OmitsModelFactoryMethodIfModelTypeInternal()
+        {
+            var plugin = await MockHelpers.LoadMockPluginAsync(
+                inputModelTypes: [
+                    InputFactory.Model(
+                        "mockInputModel",
+                        properties:
+                        [
+                            InputFactory.Property("Prop1", InputPrimitiveType.String),
+                            InputFactory.Property("OptionalBool", InputPrimitiveType.Boolean, isRequired: false)
+                        ])
+                ],
+                compilation: async () => await Helpers.GetCompilationFromDirectoryAsync());
+            var csharpGen = new CSharpGen();
+
+            await csharpGen.ExecuteAsync();
+
+            // Model factory should be omitted since there are no methods to generate
+            var modelFactory = plugin.Object.OutputLibrary.TypeProviders.SingleOrDefault(t => t is ModelFactoryProvider);
+            Assert.IsNull(modelFactory);
+        }
+
+        // This test validates that the model factory method for a model is omitted if the
+        // any of the model's serialization ctor have parameters whose type are customized to be internal.
+        [Test]
+        public async Task OmitsModelFactoryMethodIfParamTypeInternal()
+        {
+            var modelProperty = InputFactory.Property("Prop1", InputFactory.Model("otherModel"));
+            var plugin = await MockHelpers.LoadMockPluginAsync(
+                inputModelTypes: [
+                    InputFactory.Model(
+                        "mockInputModel",
+                        properties:
+                        [
+                            modelProperty,
+                        ])
+                ],
+                compilation: async () => await Helpers.GetCompilationFromDirectoryAsync());
+            var csharpGen = new CSharpGen();
+
+            await csharpGen.ExecuteAsync();
+
+            // Model factory should be omitted since there are no methods to generate
+            var modelFactory = plugin.Object.OutputLibrary.TypeProviders.SingleOrDefault(t => t is ModelFactoryProvider);
+            Assert.IsNull(modelFactory);
+        }
+
         [TestCase(true)]
         [TestCase(false)]
         public async Task CanCustomizeModelFullConstructor(bool extraParameters)

diff --git a/...lFactoriesCustomizationTests/OmitsModelFactoryMethodIfModelTypeInternal/MockInputModel.cs b/...lFactoriesCustomizationTests/OmitsModelFactoryMethodIfModelTypeInternal/MockInputModel.cs
@@ -0,0 +1,13 @@
+using System;
+using System.Collections.Generic;
+using System.Collections.ObjectModel;
+using Microsoft.Generator.CSharp.Customization;
+
+#nullable disable
+
+namespace Sample.Models;
+
+internal partial class MockInputModel
+{
+
+}
diff --git a/...lFactoriesCustomizationTests/OmitsModelFactoryMethodIfParamTypeInternal/MockInputModel.cs b/...lFactoriesCustomizationTests/OmitsModelFactoryMethodIfParamTypeInternal/MockInputModel.cs
@@ -0,0 +1,18 @@
+using System;
+using System.Collections.Generic;
+using System.Collections.ObjectModel;
+using Microsoft.Generator.CSharp.Customization;
+
+#nullable disable
+
+namespace Sample.Models;
+
+public partial class MockInputModel
+{
+    internal OtherModel Prop1 { get; set; }
+}
+
+internal partial class OtherModel
+{
+
+}