Fix Java generation - support methods that return enums, disambiguate entities that end in "Request" (#251)

* Disambiguate generated class references for type names that end in 'request'
* Added test for generated file.

Author: MIchaelMainer

Templates/Java/BaseJavaModel.template.tt

@@ -937,17 +937,21 @@ import java.util.EnumSet;", host.CurrentModel.NamespaceName());
 import {0}.concurrency.*;
 import {0}.core.*;
 import {0}.models.extensions.*;
-import {0}.models.generated.*;
+import {0}.models.generated.*;{2}
 import {0}.http.*;
 import {0}.requests.extensions.*;
 import {0}.options.*;
 import {0}.serializer.*;
 
 import java.util.Arrays;
 import java.util.EnumSet;";
+
+        // We need this for disambiguation of generated model class/interfaces references.
+        string fullyQualifiedImport = host.GetFullyQualifiedImportStatementForModel(); 
+
         return string.Format(format,
             host.CurrentModel.NamespaceName(),
-            host.TemplateInfo.OutputParentDirectory.Replace("_", "."));
+            host.TemplateInfo.OutputParentDirectory.Replace("_", "."), fullyQualifiedImport);
         }
 
     /// Creates a class declaration

Templates/Java/requests_extensions/BaseMethodCollectionResponse.java.tt

@@ -28,7 +28,7 @@ import com.google.gson.annotations.*;
     }
 
 <#=CreateRawJsonObject()#>
-<# if ( ! ((c as OdcmMethod).ReturnType is OdcmPrimitiveType) ) { #>
+<# if ( ! ((c as OdcmMethod).ReturnType is OdcmPrimitiveType) && ! ((c as OdcmMethod).ReturnType is OdcmEnum) ) { #>
 <#=    UpdateListPropertiesWithinSetRawObject(new [] { "value" })#>
 <# } else { #>
     }

src/GraphODataTemplateWriter/Extensions/OdcmModelExtensions.cs

@@ -8,6 +8,7 @@ namespace Microsoft.Graph.ODataTemplateWriter.Extensions
     using Microsoft.Graph.ODataTemplateWriter.Settings;
     using Vipr.Core.CodeModel;
     using NLog;
+    using Microsoft.Graph.ODataTemplateWriter.TemplateProcessor;
 
     public static class OdcmModelExtensions
     {
@@ -137,6 +138,85 @@ public static IEnumerable<OdcmMethod> GetMethods(this OdcmModel model)
             return model.GetEntityTypes().SelectMany(entityType => entityType.Methods);
         }
 
+        /// <summary>
+        /// This extension method determines whether the current type needs to be disambiguated.
+        /// If the current type needs dismabiguation, we then provide a fully qualified 
+        /// import statement for the model.
+        /// This currently operates only on entities whose name ends in "Request". We do this
+        /// because every entity results in a request object. For example, assume we have the following
+        /// two entities: timeOff and timeOffRequest. The timeOff entity will trigger the generation
+        /// of classes named model.timeOff and request.timeOffRequest. The timeOffRequest entity
+        /// will trigger the generation of a model.timeOffRequest and request.timeOffRequestRequest.
+        /// The request.timeOffRequest and model.timeOffRequest classes will result in a name collision in
+        /// a few files.
+        /// Assumptions: 1) host.CurrentType is an OdcmProperty.
+        /// </summary>
+        /// <param name="host">The T4Host that orchestrates applying templates to the OdcmModel.</param>
+        /// <returns>A boolean value that indicates whether the current type needs to be disambiguated.</returns>
+        public static bool DoesCurrentTypeNeedDisambiguation(this CustomT4Host host)
+        {   
+            // At this point this is only applicable to OdcmProperty.
+            // Challenging this assumption will require a lot more investigation.
+            if (!(host.CurrentType is OdcmProperty))
+                return false; 
+
+            // We only support "Request" dismabiguation at this point. Check whether the
+            // current type ends in "Request".
+            var requestSuffix = "Request";
+            var currentTypeName = (host.CurrentType as OdcmProperty).Type.Name;
+            int index = currentTypeName.IndexOf(requestSuffix);
+            if (index == -1 || !currentTypeName.EndsWith(requestSuffix))
+                return false; // Doesn't need disambiguation
+
+            // If it does end in "Request", let's capture the base name to check if an entity of that name 
+            // exists in the schema.
+            string entityNameToCheckForCollision = currentTypeName.Remove(index, requestSuffix.Length);
+
+            // Search across namespaces, looking only at EntityType, to determine whether this type requires 
+            // disambiguation. This needs to be supported across namespaces.
+            var classes = host.CurrentModel.Namespaces.SelectMany(n => n.Classes);
+            var shouldDisambiguate = classes.Where(entity => entity.Kind == OdcmClassKind.Entity
+                                                       && entity.Name == entityNameToCheckForCollision).Any();
+
+            return shouldDisambiguate;
+        }
+
+        /// <summary>
+        /// An extension method to get an import statement for the fully qualified name of the current type.
+        /// Assumptions: 1) host.CurrentType is an OdcmProperty. 2) the generated namespace of the current type 
+        /// is in models.generated output namespace (in the generated file, not in the metadata).
+        /// This method should support multiple namespaces.
+        /// This currently (6/2020) applies to the following templates:
+        ///   BaseEntityCollectionRequest.java.tt
+        ///   IBaseEntityCollectionRequest.java.tt
+        ///   IBaseEntityCollectionPage.java.tt
+        /// This currently unintentionally applies to the following templates when the disambiguation condition is met.
+        /// This is not an issue as there is already a wild card import for the namespace that we should address first.
+        ///   IBaseEntityCollectionRequestBuilder.java.tt
+        ///   BaseEntityCollectionRequestBuilder.java.tt
+        /// </summary>
+        /// <param name="host">The T4Host that orchestrates applying templates to the OdcmModel.</param>
+        /// <returns>A string that represents the import statement of the fully qualified name of the current type.</returns>
+        public static string GetFullyQualifiedImportStatementForModel(this CustomT4Host host)
+        {
+            // By default, we don't need to disambiguate the model in the generated code file.
+            // This will be the general case.
+            var importStatement = "";
+
+            // Check whether we need to disambiguate the current type for generation of the model in the code file.
+            var shouldDisambiguate = host.DoesCurrentTypeNeedDisambiguation();
+
+            if (shouldDisambiguate)
+            {
+                // Form the import statement to disambiguate the model in the generated code file.
+                var thisNamespace = host.CurrentModel.NamespaceName();
+                var thisTypeName = (host.CurrentType as OdcmProperty).Type.Name.ToUpperFirstChar();
+                importStatement = $"\nimport {thisNamespace}.models.generated.{thisTypeName};";
+            }
+
+            return importStatement;
+        }
+
         /// <summary>
         /// Get the service collection navigation property for the given property type
         /// 

test/Typewriter.Test/Given_a_valid_metadata_file_to_Typewriter.cs

@@ -74,6 +74,41 @@ public void It_generates_PHP_models_with_a_property()
             Assert.IsTrue(isExpectedNamespaceSet, $"The expected namespace, {testNamespace}, was not set in the generated test file.");
         }
 
+        /// <summary>
+        /// 
+        /// </summary>
+        [TestMethod]
+        public void It_generates_Java_models_with_disambiguated_import()
+        {
+            const string outputDirectory = "outputJava";
+
+            Options options = new Options()
+            {
+                Output = outputDirectory,
+                Language = "Java",
+                GenerationMode = GenerationMode.Files
+            };
+
+            Generator.GenerateFiles(testMetadata, options);
+
+            FileInfo fileInfo = new FileInfo(outputDirectory + generatedOutputUrl + @"\requests\extensions\TimeOffRequestCollectionRequest.java");
+            Assert.IsTrue(fileInfo.Exists, $"Expected: {fileInfo.FullName}. File was not found.");
+
+            // Check that the namespace applied at the CLI was added to the document.
+            IEnumerable<string> lines = File.ReadLines(fileInfo.FullName);
+            bool isExpectedImportStatementFound = false;
+            string expected = "import com.microsoft.graph.models.generated.TimeOffRequest;";
+            foreach (var line in lines)
+            {
+                if (line.Contains(expected))
+                {
+                    isExpectedImportStatementFound = true;
+                    break;
+                }
+            }
+            Assert.IsTrue(isExpectedImportStatementFound, $"The expected statement was not found. Expected: {expected}");
+        }
+
         [TestMethod]
         public void It_generates_dotNet_client_with_default_beta_baseUrl()
         {

test/Typewriter.Test/Resources/dirtyMetadata.xml

@@ -49,6 +49,19 @@
         <Property Name="email" Type="Edm.String" />
       </ComplexType>
       <ComplexType Name="emptyComplexType" Abstract="true"/>
+      
+      <EntityType Name="timeOffRequest" BaseType="microsoft.graph.entity">
+        <Property Name="name" Type="Edm.String" />
+      </EntityType>
+      <EntityType Name="timeOff" BaseType="microsoft.graph.entity">
+        <Property Name="name" Type="Edm.String" />
+      </EntityType>
+      <EntityType Name="schedule" BaseType="microsoft.graph.entity">
+        <Property Name="enabled" Type="Edm.Boolean" />
+        <NavigationProperty Name="timesOff" Type="Collection(microsoft.graph.timeOff)" ContainsTarget="true" />
+        <NavigationProperty Name="timeOffRequests" Type="Collection(microsoft.graph.timeOffRequest)" ContainsTarget="true" />
+      </EntityType>
+      
       <EntityType Name="onenotePage" HasStream="true" BaseType="microsoft.graph.entity">
         <Property Name="content" Type="Edm.Stream" />
         <NavigationProperty Name="parentNotebook" Type="microsoft.graph.notebook" ContainsTarget="true" />