Disambiguate complex type, better method overload support (#227)

* Update ComplexType.cs.tt to disambiguate

Related to updates to EntityType.cs.tt in 9850b83 and 29d268b

* Disambiguate parameters for actions

* Enable overloads with uniqueness checks

Fixes a hack that was used to restrict generation of a function overload where the overload was based on the binding element type. We now check the overloads for uniqueness based on the parameter list. We also now don't emit duplicate parameters in the CreateReqyest method.

* Structural property type needs to use the same disambiguation scheme as complextype template

Author: MIchaelMainer

Templates/CSharp/Base/IRequestBuilder.Base.template.tt

@@ -117,6 +117,12 @@ public string GetMethodProperties(OdcmClass entity, bool isCollection)
                 var paramVariableName = param.Name.GetSanitizedParameterName();
                 var paramTypeString = param.Type.GetTypeString();
 
+                // Adds support for classes ending in "Request" that have been dismabiguated.
+                if (paramTypeString.EndsWith("Request"))
+                {   
+                    paramTypeString = String.Concat(paramTypeString, "Object");
+                }
+
                 if (param.IsCollection)
                 {
                     paramTypeString = string.Format("IEnumerable<{0}>", paramTypeString);

Templates/CSharp/Base/RequestBuilder.Base.template.tt

@@ -163,6 +163,12 @@ public string GetMethodProperties(OdcmClass entity, bool isCollection)
                 var paramVariableName = param.Name.GetSanitizedParameterName();
                 var paramTypeString = param.Type.GetTypeString();
 
+                // Adds support for classes ending in "Request" that have been dismabiguated.
+                if (paramTypeString.EndsWith("Request"))
+                {   
+                    paramTypeString = String.Concat(paramTypeString, "Object");
+                }
+
                 if (param.IsCollection)
                 {
                     paramTypeString = string.Format("IEnumerable<{0}>", paramTypeString);

Templates/CSharp/Model/ComplexType.cs.tt

@@ -7,9 +7,31 @@ OdcmClass complex = host.CurrentType.AsOdcmClass();
 var complexTypeName = complex.Name.ToCheckedCase();
 var typeDeclaration = complexTypeName;
 
+// In the case a complex type name ends with "Request", we will have a collision with the *Request objects
+// when we generate. For example, there is a complex type named SearchRequest and an entity named Search.
+// The SearchRequest complex type results in a SearchResult class in our models collection. The Search entity
+// results in a SearchRequest request object causing conflicts.
+
+if (typeDeclaration.EndsWith("Request"))
+{
+    typeDeclaration = String.Concat(typeDeclaration, "Object");
+}
+
+// Capture the cstor type declaration as the typedeclaration may get appended with a base type. 
+string cstorTypeDeclaration = typeDeclaration;
+
 if (complex.Base != null)
 {
-    typeDeclaration = string.Format("{0} : {1}", typeDeclaration, complex.Base.Name.ToCheckedCase());
+    // Disambiguate the base type declaration for the same reason we did this 
+	// for typeDeclaration.
+	var baseTypeDeclaration = complex.Base.Name.ToCheckedCase();
+
+    if (baseTypeDeclaration.EndsWith("Request"))
+    {
+        baseTypeDeclaration = String.Concat(baseTypeDeclaration, "Object");
+    }
+
+    typeDeclaration = string.Format("{0} : {1}", typeDeclaration, baseTypeDeclaration);
 }
 
 var isMethodResponse = complex.LongDescriptionContains("methodResponse");
@@ -47,9 +69,9 @@ namespace <#=complex.Namespace.GetNamespaceName()#>
 if (!complex.IsAbstract)
 {
 #>        /// <summary>
-        /// Initializes a new instance of the <see cref="<#=complexTypeName#>"/> class.
+        /// Initializes a new instance of the <see cref="<#=cstorTypeDeclaration#>"/> class.
         /// </summary>
-        public <#=complexTypeName#>()
+        public <#=cstorTypeDeclaration#>()
         {
             this.ODataType = "<#=complex.FullName#>";
         }

Templates/CSharp/Model/EntityType.cs.tt

@@ -93,6 +93,14 @@ namespace <#=entity.Namespace.GetNamespaceName()#>
             var propertyType = property.IsTypeNullable() || property.IsCollection()
                     ? property.GetTypeString()
                     : property.GetTypeString() + "?";
+        
+            // We want to disambiguate structural property return types in case there is a naming 
+            // collision with a request builder, like what we had for the Search entity, and the SearchRequest
+            // complex type. 
+            if (propertyType.EndsWith("Request") && property.IsNavigation() == false)
+            { 
+                propertyType = String.Concat(propertyType, "Object");
+            }
 
             var propertyName = property.Name.ToCheckedCase();
             var propertyCollectionPage = property.IsReference() ? string.Concat(entityName, propertyName, "CollectionWithReferencesPage") : string.Concat(entityName, propertyName, "CollectionPage");

Templates/CSharp/Requests/MethodRequestBuilder.cs.tt

@@ -12,18 +12,12 @@ var requestBuilderType = requestType + "Builder";
 var isPost = method.IsAction() && method.Parameters != null && method.Parameters.Any();
 var methodType = method.IsFunction ? "Function" : "Action";
 
+// Overloads are determined based on the name of method and the type of the binding parameter.
+// There is no differentiation between an entity, and a collection of the same entities.
+// This may cause issues in the future that would require changes.
 var overloads = new List<OdcmMethod>();
 overloads.Add(method);
-
-// So far, it appears that the overloads on the functions are working as expected. 
-// The overloads on actions should be skipped. If we see more issues here, we may need to
-// revisit how VIPR determines overloaded methods. 
-if (method.IsFunction)
-{
-	overloads.AddRange(method.Overloads);
-}
-
-
+overloads.AddRange(method.Overloads);
 
 var methods = overloads
     .Select(m =>
@@ -36,6 +30,11 @@ var methods = overloads
                         var parameterName = p.Name.GetSanitizedParameterName();
                         var propertyName = p.Name.ToCheckedCase();
 
+                        // Adds support for classes ending in "Request" that have been dismabiguated.
+                        if (type.EndsWith("Request"))
+                        {   
+                            type = String.Concat(type, "Object");
+                        }
                         if (p.IsCollection)
                         {
                             type = string.Format("IEnumerable<{0}>", type);
@@ -52,6 +51,8 @@ var methods = overloads
             var paramStrings = parameters.Select(p => string.Format(",\n            {0} {1}", p.Type, p.ParameterName));
             var paramComments = parameters.Select(p => string.Format("\n        /// <param name=\"{0}\">A {0} parameter for the OData method call.</param>", p.ParameterName));
 
+            
+
             return new
             {
                 Parameters = parameters,
@@ -74,10 +75,14 @@ namespace <#=method.Namespace.GetNamespaceName()#>
     public partial class <#=requestBuilderType#> : Base<#=methodType#>MethodRequestBuilder<I<#=requestType#>>, I<#=requestBuilderType#>
     {
 <#
+// We only want to generate unique method signatures. 
+// We'll use the ParametersAsArguments to define the uniqueness of a method.
+HashSet<string> uniqueMethods = new HashSet<string>();
 foreach (var m in methods)
-
 {
-
+    // Only generate a unique method.
+    if (uniqueMethods.Add(m.ParametersAsArguments) == true) 
+    {
 #>
         /// <summary>
         /// Constructs a new <see cref="<#=requestBuilderType#>"/>.
@@ -90,16 +95,17 @@ foreach (var m in methods)
             : base(requestUrl, client)
         {
 <#
-    foreach (var p in m.Parameters)
-    {
+        foreach (var p in m.Parameters)
+        {
 #>
             this.SetParameter("<#=p.Name#>", <#=p.ParameterName#>, <#=p.IsNullable.ToString().ToLowerInvariant()#>);
 <#
-    }
+        }
 #>
         }
 
 <#
+    }
 }
 #>
         /// <summary>
@@ -115,17 +121,23 @@ foreach (var m in methods)
 <#
     if (isPost)
     {
+        HashSet<string> uniqueParameters = new HashSet<string>();
+
         foreach (var m in methods)
         {
             foreach (var p in m.Parameters)
             {
+                // We only want to add unique parameters to CreateRequest. 
+                if (uniqueParameters.Add(p.Name) == true) 
+                {
 #>
             if (this.HasParameter("<#=p.Name#>"))
             {
                 request.RequestBody.<#=p.PropertyName#> = this.GetParameter<<#=p.Type#>>("<#=p.Name#>");
             }
 
 <#
+                }
             }
         }
     }

test/Typewriter.Test/Given_a_valid_metadata_file_to_Typewriter.cs

@@ -280,5 +280,193 @@ public void It_doesnt_generate_odatatype_initialization_for_abstract_entitytypes
             Assert.IsTrue(hasTestString, $"The expected test token string, '{testString}', was not set in the generated test file. We didn't properly generate the cstor code.");
             Assert.IsFalse(hasTestODataInitString, $"The unexpected test token string, '{testODataInitString}', was set in the generated test file. We didn't properly generate the cstor code.");
         }
+
+        [TestMethod]
+        public void It_creates_disambiguated_abstract_base_complextype_models()
+        {
+            const string outputDirectory = "output";
+
+            Options options = new Options()
+            {
+                Output = outputDirectory,
+                Language = "CSharp",
+                GenerationMode = GenerationMode.Files
+            };
+
+            Generator.GenerateFiles(testMetadata, options);
+
+            FileInfo fileInfo = new FileInfo(outputDirectory + generatedOutputUrl + @"\Model\EmptyBaseComplexTypeRequest.cs");
+            Assert.IsTrue(fileInfo.Exists, $"Expected: {fileInfo.FullName}. File was not found.");
+
+            IEnumerable<string> lines = File.ReadLines(fileInfo.FullName);
+            bool hasTestString = false;
+            string testString = "public abstract partial class EmptyBaseComplexTypeRequestObject";
+
+            foreach (var line in lines)
+            {
+                if (line.Contains(testString))
+                {
+                    hasTestString = true;
+                    break;
+                }
+            }
+
+            Assert.IsTrue(hasTestString, $"The expected test token string, '{testString}', was not set in the generated test file. We didn't properly generate the type declaration code.");
+        }
+
+        [TestMethod]
+        public void It_creates_disambiguated_complextype_models()
+        {
+            const string outputDirectory = "output";
+
+            Options options = new Options()
+            {
+                Output = outputDirectory,
+                Language = "CSharp",
+                GenerationMode = GenerationMode.Files
+            };
+
+            Generator.GenerateFiles(testMetadata, options);
+
+            FileInfo fileInfo = new FileInfo(outputDirectory + generatedOutputUrl + @"\Model\DerivedComplexTypeRequest.cs");
+            Assert.IsTrue(fileInfo.Exists, $"Expected: {fileInfo.FullName}. File was not found.");
+
+            IEnumerable<string> lines = File.ReadLines(fileInfo.FullName);
+            bool hasTestTypeDeclaration = false;
+            string testTypeDeclaration = "public partial class DerivedComplexTypeRequestObject : EmptyBaseComplexTypeRequestObject";
+            bool hasTestTypeCstor = false;
+            string testTypeCstor = "public DerivedComplexTypeRequestObject";
+            bool hasTestOdataType = false;
+            string testOdataType = "this.ODataType = \"microsoft.graph.derivedComplexTypeRequest\"";
+
+            foreach (var line in lines)
+            {
+                // We only need to check once.
+                if (line.Contains(testTypeDeclaration) && !hasTestTypeDeclaration)
+                {
+                    hasTestTypeDeclaration = true;
+                    continue;
+                }
+                if (line.Contains(testTypeCstor) && !hasTestTypeCstor)
+                {
+                    hasTestTypeCstor = true;
+                    continue;
+                }
+                if (line.Contains(testOdataType) && !hasTestOdataType)
+                {
+                    hasTestOdataType = true;
+                    break; // This is the last expected line.
+                }
+            }
+
+            Assert.IsTrue(hasTestTypeDeclaration, $"The expected test token string, '{testTypeDeclaration}', was not set in the generated test file. We didn't properly generate the type declaration code.");
+            Assert.IsTrue(hasTestTypeCstor, $"The expected test token string, '{testTypeCstor}', was not set in the generated test file. We didn't properly generate the cstor code.");
+            Assert.IsTrue(hasTestOdataType, $"The expected test token string, '{testOdataType}', was not set in the generated test file. We didn't properly generate the initialized odata.type code.");
+        }
+
+        [TestMethod]
+        public void It_creates_disambiguated_MethodRequestBuilder_parameters()
+        {
+            const string outputDirectory = "output";
+
+            Options options = new Options()
+            {
+                Output = outputDirectory,
+                Language = "CSharp",
+                GenerationMode = GenerationMode.Files
+            };
+
+            Generator.GenerateFiles(testMetadata, options);
+
+            FileInfo fileInfo = new FileInfo(outputDirectory + generatedOutputUrl + @"\Requests\TestTypeQueryRequestBuilder.cs");
+            Assert.IsTrue(fileInfo.Exists, $"Expected: {fileInfo.FullName}. File was not found.");
+
+            IEnumerable<string> lines = File.ReadLines(fileInfo.FullName);
+            bool hasTestParameter = false;
+            string testParameter = "IEnumerable<DerivedComplexTypeRequestObject> requests)";
+            
+
+            foreach (var line in lines)
+            {
+                // We only need to check once.
+                if (line.Contains(testParameter))
+                {
+                    hasTestParameter = true;
+                    break;
+                }
+            }
+
+            Assert.IsTrue(hasTestParameter, $"The expected test token string, '{testParameter}', was not set in the generated test file. We didn't properly generate the parameter.");
+        }
+
+        [TestMethod]
+        public void It_creates_disambiguated_EntityRequestBuilder_parameters()
+        {
+            const string outputDirectory = "output";
+
+            Options options = new Options()
+            {
+                Output = outputDirectory,
+                Language = "CSharp",
+                GenerationMode = GenerationMode.Files
+            };
+
+            Generator.GenerateFiles(testMetadata, options);
+
+            FileInfo fileInfo = new FileInfo(outputDirectory + generatedOutputUrl + @"\Requests\TestTypeRequestBuilder.cs");
+            Assert.IsTrue(fileInfo.Exists, $"Expected: {fileInfo.FullName}. File was not found.");
+
+            IEnumerable<string> lines = File.ReadLines(fileInfo.FullName);
+
+            bool hasTestParameter = false;
+            string testParameter = "IEnumerable<DerivedComplexTypeRequestObject> requests)";
+            
+            foreach (var line in lines)
+            {
+                // We only need to check once.
+                if (line.Contains(testParameter))
+                {
+                    hasTestParameter = true;
+                    break;
+                }
+            }
+
+            Assert.IsTrue(hasTestParameter, $"The expected test token string, '{testParameter}', was not set in the generated test file. We didn't properly generate the parameter.");
+        }
+
+        [TestMethod]
+        public void It_creates_disambiguated_IEntityRequestBuilder_parameters()
+        {
+            const string outputDirectory = "output";
+
+            Options options = new Options()
+            {
+                Output = outputDirectory,
+                Language = "CSharp",
+                GenerationMode = GenerationMode.Files
+            };
+
+            Generator.GenerateFiles(testMetadata, options);
+
+            FileInfo fileInfo = new FileInfo(outputDirectory + generatedOutputUrl + @"\Requests\ITestTypeRequestBuilder.cs");
+            Assert.IsTrue(fileInfo.Exists, $"Expected: {fileInfo.FullName}. File was not found.");
+
+            IEnumerable<string> lines = File.ReadLines(fileInfo.FullName);
+
+            bool hasTestParameter = false;
+            string testParameter = "IEnumerable<DerivedComplexTypeRequestObject> requests)";
+
+            foreach (var line in lines)
+            {
+                // We only need to check once.
+                if (line.Contains(testParameter))
+                {
+                    hasTestParameter = true;
+                    break;
+                }
+            }
+
+            Assert.IsTrue(hasTestParameter, $"The expected test token string, '{testParameter}', was not set in the generated test file. We didn't properly generate the parameter.");
+        }
     }
 }