Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix model import paths + Add support for enum imports #2047

Merged
merged 10 commits into from
May 16, 2024
Merged

Fix model import paths + Add support for enum imports #2047

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
bab150c
Initial work on fixing paths.
SilasKenneth May 14, 2024
65ad35c
Remove unused test.
SilasKenneth May 14, 2024
2e31bfb
Add imports for DateOnly, TimeOnly and Enums.
SilasKenneth May 14, 2024
d6e84f0
Fix issue with spelling.
SilasKenneth May 14, 2024
7fa0ec9
Do slight refactor to improve style.
SilasKenneth May 14, 2024
0305d64
Fix more issues with the snippets.
SilasKenneth May 15, 2024
584a6cb
Take care of corner cases.
SilasKenneth May 15, 2024
142708d
Use static arguments for delegates.
SilasKenneth May 16, 2024
7a958cd
Remove unwanted char.
SilasKenneth May 16, 2024
59c51af
Update CodeSnippetsReflection.OpenAPI.Test/PhpGeneratorTests.cs
SilasKenneth May 16, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion CodeSnippetsReflection.OpenAPI.Test/PhpGeneratorTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,8 @@ public async Task IncludesRequestBodyClassName()
var snippetModel = new SnippetModel(requestPayload, ServiceRootBetaUrl, await GetBetaSnippetMetadata());
var result = _generator.GenerateCodeSnippet(snippetModel);
Assert.Contains("AddPasswordPostRequestBody", result);
Assert.Contains(@"use Microsoft\Graph\Beta\GraphServiceClient;", result);
Assert.Contains(@"use Microsoft\Graph\Beta\Generated\Models\PasswordCredential;", result);
}

[Fact]
Expand Down Expand Up @@ -847,8 +849,11 @@ public async Task GenerateWithCustomDateAndTimeTypes()
};
var snippetModel = new SnippetModel(requestPayload, ServiceRootUrl, await GetV1SnippetMetadata());
var result = _generator.GenerateCodeSnippet(snippetModel);
Assert.Contains(@"use Microsoft\Kiota\Abstractions\Types\Date;", result);
Assert.Contains(@"use Microsoft\Kiota\Abstractions\Types\Time;", result);
Assert.Contains(@"use Microsoft\Graph\Generated\Models\AutomaticUpdateMode;", result);
Assert.Contains("->setQualityUpdatesPauseStartDate(new Date('2016-12-31'))", result);
Assert.Contains("->setScheduledInstallTime(new Time('11:59:31.3170000'))", result);
Assert.Contains("->setScheduledInstallTime(new Time('11:59:31.3170000'))", result);
SilasKenneth marked this conversation as resolved.
Show resolved Hide resolved
}

[Fact]
Expand Down
4 changes: 4 additions & 0 deletions CodeSnippetsReflection.OpenAPI.Test/PythonGeneratorTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -878,6 +878,7 @@ public async Task GeneratesCorrectTypeInCollectionInitializer() {
Assert.Contains("scope = RuleBasedSubjectSet(", result);
Assert.Contains("tasks = [", result);
Assert.Contains("Task(", result);
Assert.Contains("from msgraph.generated.models.lifecycle_workflow_category import LifecycleWorkflowCategory", result);
}
[Fact]
public async Task CorrectlyHandlesTypeFromInUrl()
Expand Down Expand Up @@ -936,6 +937,7 @@ public async Task GeneratesObjectInitializationWithCallToSetters()
var result = _generator.GenerateCodeSnippet(snippetModel);
Assert.Contains("select = [\"displayName\",\"mailNickName\"],", result);
Assert.Contains("account_enabled = True", result);
Assert.Contains("from msgraph import GraphServiceClient", result);
}
[Fact]
public async Task IncludesRequestBodyClassName()
Expand All @@ -950,6 +952,8 @@ public async Task IncludesRequestBodyClassName()
var snippetModel = new SnippetModel(requestPayload, ServiceRootBetaUrl, await GetBetaSnippetMetadata());
var result = _generator.GenerateCodeSnippet(snippetModel);
Assert.Contains("request_body = AddPasswordPostRequestBody(", result);
Assert.Contains("from msgraph_beta.generated.models.password_credential import PasswordCredential", result);
Assert.Contains("from msgraph_beta import GraphServiceClient", result);
}
[Fact]
public async Task FindsPathItemsWithDifferentCasing()
Expand Down
71 changes: 63 additions & 8 deletions CodeSnippetsReflection.OpenAPI/LanguageGenerators/PhpGenerator.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,28 +119,73 @@
}
return payloadSb.ToString().Trim();
}
private static HashSet<string> GetImportStatements(SnippetModel snippetModel)

Check warning on line 122 in CodeSnippetsReflection.OpenAPI/LanguageGenerators/PhpGenerator.cs

View workflow job for this annotation

GitHub Actions / Build 

Refactor this method to reduce its Cognitive Complexity from 30 to the 15 allowed. (https://rules.sonarsource.com/csharp/RSPEC-3776)
{
const string modelImportPrefix = "use Microsoft\\Graph\\Generated\\Models";
const string requestBuilderImportPrefix = "use Microsoft\\Graph\\Generated";

var snippetImports = new HashSet<string>();
var packagePrefix = snippetModel.ApiVersion switch

Check warning on line 124 in CodeSnippetsReflection.OpenAPI/LanguageGenerators/PhpGenerator.cs

View workflow job for this annotation

GitHub Actions / Build 

The switch expression does not handle all possible values of its input type (it is not exhaustive). For example, the pattern '""' is not covered.
{
"v1.0" => @"Microsoft\Graph",
"beta" => @"Microsoft\Graph\Beta",
};
var modelImportPrefix = $@"use {packagePrefix}\Generated\Models";
var requestBuilderImportPrefix = $@"use {packagePrefix}\Generated";
const string customTypesPrefix = @"use Microsoft\Kiota\Abstractions\Types";

snippetImports.Add("use Microsoft\\Graph\\GraphServiceClient;");
var snippetImports = new HashSet<string> { $@"use {packagePrefix}\GraphServiceClient;" };

var imports = ImportsGenerator.GenerateImportTemplates(snippetModel);
foreach (var import in imports)
{
switch (import.Kind)
{
case ImportKind.Model:
if (import.ModelProperty.PropertyType is PropertyType.DateOnly or PropertyType.TimeOnly)
{
snippetImports.Add($"{customTypesPrefix}\\{GetPropertyTypeName(import.ModelProperty)};");
continue;
}
var typeDefinition = import.ModelProperty.TypeDefinition;
if (typeDefinition != null){
snippetImports.Add($"{modelImportPrefix}\\{typeDefinition};");
const string modelsNamespaceName = "models.microsoft.graph";
var modelNamespaceStringLen = modelsNamespaceName.Length;
var modelNamespace = import.ModelProperty.NamespaceName;
var inModelsNamespace =
modelNamespace.Equals(modelsNamespaceName,
StringComparison.OrdinalIgnoreCase);
var nested = !inModelsNamespace && modelNamespace.StartsWith(modelsNamespaceName);
// This takes care of models in nested namespaces inside the model namespace for instance
// models inside IdentityGovernance namespace
var othersParts = nested switch
{
true => import.ModelProperty.NamespaceName[modelNamespaceStringLen..]
.Split('.', StringSplitOptions.RemoveEmptyEntries)
.Select(x => x.ToFirstCharacterUpperCase())
.Aggregate((x, y) => $@"{x}\{y}"),
false => string.Empty
};

var namespaceValue = !string.IsNullOrEmpty(othersParts) ? $@"\{othersParts}" : string.Empty;
if (typeDefinition != null)
{
if (inModelsNamespace)
{
snippetImports.Add($@"{modelImportPrefix}{namespaceValue}\{typeDefinition};");
}
else
{
var imported = import.ModelProperty.NamespaceName.Split('.')
.Select(x => x.ToFirstCharacterUpperCase())
.Aggregate((a, b) => $@"{a}\{b}")
.Replace(@"Me\", @"Users\Item\");
snippetImports.Add($@"{requestBuilderImportPrefix}\{imported}\{typeDefinition}");
}
// check if model has a nested namespace and append it to the import statement
continue; // Move to the next import.
}

if (import.ModelProperty.PropertyType == PropertyType.Enum)
{
snippetImports.Add($@"{modelImportPrefix}{namespaceValue}\{import.ModelProperty.Name.ToFirstCharacterUpperCase()};");
}
break;

case ImportKind.Path:
if (!string.IsNullOrEmpty(import.Path) && !string.IsNullOrEmpty(import.RequestBuilderName))
{
Expand Down Expand Up @@ -257,7 +302,7 @@
WriteCodeProperty(childPropertyName ?? propertyAssignment, payloadSb, codeProperty, child, indentManager, ++childPosition, newChildName);
}
}
private static void WriteCodeProperty(string propertyAssignment, StringBuilder payloadSb, CodeProperty parent, CodeProperty child, IndentManager indentManager, int childPosition = 0, string childPropertyName = default, bool fromMap = false)

Check warning on line 305 in CodeSnippetsReflection.OpenAPI/LanguageGenerators/PhpGenerator.cs

View workflow job for this annotation

GitHub Actions / Build 

Remove this unused method parameter 'childPosition'. (https://rules.sonarsource.com/csharp/RSPEC-1172)
{
var isArray = parent.PropertyType == PropertyType.Array;
var isMap = parent.PropertyType == PropertyType.Map;
Expand Down Expand Up @@ -567,4 +612,14 @@

return result.EndsWith("()()") ? result[..^2] : result;
}

private static string GetPropertyTypeName(CodeProperty property)
{
shemogumbe marked this conversation as resolved.
Show resolved Hide resolved
return property.PropertyType switch
{
PropertyType.DateOnly => "Date",
PropertyType.TimeOnly => "Time",
_ => property.TypeDefinition
};
}
}
43 changes: 38 additions & 5 deletions CodeSnippetsReflection.OpenAPI/LanguageGenerators/PythonGenerator.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,34 +76,67 @@
snippetBuilder.Insert(0, string.Join(Environment.NewLine, importStatements));
return snippetBuilder.ToString();
}
private static HashSet<string> GetImportStatements(SnippetModel snippetModel)

Check warning on line 79 in CodeSnippetsReflection.OpenAPI/LanguageGenerators/PythonGenerator.cs

View workflow job for this annotation

GitHub Actions / Build 

Refactor this method to reduce its Cognitive Complexity from 44 to the 15 allowed. (https://rules.sonarsource.com/csharp/RSPEC-3776)
{
const string modelImportPrefix = "from msgraph.generated.models";
const string requestBuilderImportPrefix = "from msgraph.generated";
var packageName = snippetModel.ApiVersion switch

Check warning on line 81 in CodeSnippetsReflection.OpenAPI/LanguageGenerators/PythonGenerator.cs

View workflow job for this annotation

GitHub Actions / Build 

The switch expression does not handle all possible values of its input type (it is not exhaustive). For example, the pattern '""' is not covered.
{
"v1.0" => "msgraph",
SilasKenneth marked this conversation as resolved.
Show resolved Hide resolved
"beta" => "msgraph_beta",
};
var modelImportPrefix = $"from {packageName}.generated.models";
var requestBuilderImportPrefix = $"from {packageName}.generated";
const string BaseRequestConfigImport = "from kiota_abstractions.base_request_configuration import RequestConfiguration";

var snippetImports = new HashSet<string>();

snippetImports.Add("from msgraph import GraphServiceClient");
snippetImports.Add($"from {packageName} import GraphServiceClient");

var imports = ImportsGenerator.GenerateImportTemplates(snippetModel);
foreach (var import in imports)
{
switch (import.Kind)
{
case ImportKind.Model:
// We don't use custom DateOnly and TimeOnly types for python snippets.
if (import.ModelProperty.PropertyType is PropertyType.DateOnly or PropertyType.TimeOnly)
continue;
var typeDefinition = import.ModelProperty.TypeDefinition;
const string modelsNamespaceName = "models.microsoft.graph";
var modelNamespaceStringLen = modelsNamespaceName.Length;
var importModelNamespace = import.ModelProperty.NamespaceName;
var inModelsNamespace = importModelNamespace.Equals(modelsNamespaceName);

var nested = !inModelsNamespace && importModelNamespace.StartsWith(modelsNamespaceName);
// This takes care of models in nested namespaces inside the model namespace for instance
// models inside IdentityGovernance namespace
var othersParts = nested switch
{
true => importModelNamespace[modelNamespaceStringLen..]
.Split('.', StringSplitOptions.RemoveEmptyEntries)
.Select(x => x.ToSnakeCase())
.Aggregate((x, y) => $"{x}.{y}"),
false => string.Empty
};

var namespaceValue = !string.IsNullOrEmpty(othersParts) ? $@".{othersParts}" : string.Empty;
if (typeDefinition != null){
if(typeDefinition.EndsWith("RequestBody",StringComparison.OrdinalIgnoreCase)){
var namespaceParts = import.ModelProperty.NamespaceName.Split('.').Select((s, i) => i == import.ModelProperty.NamespaceName.Split('.').Length - 1 ? s.ToSnakeCase() : s.ToLowerInvariant());
var namespaceParts = importModelNamespace.Split('.').Select((s, i) => i == import.ModelProperty.NamespaceName.Split('.').Length - 1 ? s.ToSnakeCase() : s.ToLowerInvariant());
var importString = $"{requestBuilderImportPrefix}.{string.Join(".", namespaceParts)}.{typeDefinition.ToSnakeCase()} import {typeDefinition}";
snippetImports.Add($"{importString.Replace(".me.", ".users.item.")}");

}
else{
snippetImports.Add($"{modelImportPrefix}.{typeDefinition.ToSnakeCase()} import {typeDefinition}");
snippetImports.Add($"{modelImportPrefix}{namespaceValue}.{typeDefinition.ToSnakeCase()} import {typeDefinition}");
}
}

if (import.ModelProperty.PropertyType == PropertyType.Enum)
{
var enumName = import.ModelProperty.Value.Split('.').First();

Check warning on line 136 in CodeSnippetsReflection.OpenAPI/LanguageGenerators/PythonGenerator.cs

View workflow job for this annotation

GitHub Actions / Build 

Indexing at 0 should be used instead of the "Enumerable" extension method "First" (https://rules.sonarsource.com/csharp/RSPEC-6608)
snippetImports.Add(
$"{modelImportPrefix}.{enumName.ToSnakeCase()} import {enumName}");
}


break;
Expand Down Expand Up @@ -184,7 +217,7 @@
var className = codeGraph.Nodes.Last().GetClassName().ToFirstCharacterUpperCase();
var itemSuffix = codeGraph.Nodes.Last().Segment.IsCollectionIndex() ? "Item" : string.Empty;
var requestBuilderName = $"{className}{itemSuffix}RequestBuilder";
var requestConfigurationName =

Check warning on line 220 in CodeSnippetsReflection.OpenAPI/LanguageGenerators/PythonGenerator.cs

View workflow job for this annotation

GitHub Actions / Build 

Remove the unused local variable 'requestConfigurationName'. (https://rules.sonarsource.com/csharp/RSPEC-1481)
$"{requestBuilderName}{codeGraph.HttpMethod.Method.ToLowerInvariant().ToFirstCharacterUpperCase()}RequestConfiguration";

var classNameQueryParameters = $"{requestBuilderName}.{requestBuilderName}{codeGraph.HttpMethod.Method.ToLowerInvariant().ToFirstCharacterUpperCase()}QueryParameters";
Expand Down
2 changes: 1 addition & 1 deletion CodeSnippetsReflection.OpenAPI/LanguageGenerators/SnippetImports.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@

public static class ImportsGenerator
{
public static List<ImportTemplate> imports = new();

Check warning on line 52 in CodeSnippetsReflection.OpenAPI/LanguageGenerators/SnippetImports.cs

View workflow job for this annotation

GitHub Actions / Build 

Use an immutable collection or reduce the accessibility of the public static field 'imports'. (https://rules.sonarsource.com/csharp/RSPEC-2386)

Check warning on line 52 in CodeSnippetsReflection.OpenAPI/LanguageGenerators/SnippetImports.cs

View workflow job for this annotation

GitHub Actions / Build 

Make this field 'private' and encapsulate it in a 'public' property. (https://rules.sonarsource.com/csharp/RSPEC-1104)

Check warning on line 52 in CodeSnippetsReflection.OpenAPI/LanguageGenerators/SnippetImports.cs

View workflow job for this annotation

GitHub Actions / Build 

Change the visibility of 'imports' or make it 'const' or 'readonly'. (https://rules.sonarsource.com/csharp/RSPEC-2223)

public static List<ImportTemplate> GenerateImportTemplates(SnippetModel snippetModel)
{
Expand Down Expand Up @@ -79,7 +79,7 @@

private static void AddModelImportTemplates(CodeProperty node, List<ImportTemplate> imports)
{
if (!string.IsNullOrEmpty(node.NamespaceName))
if (!string.IsNullOrEmpty(node.NamespaceName) || (node.PropertyType is PropertyType.DateOnly or PropertyType.TimeOnly))
{
imports.Add(new ImportTemplate
{
Expand Down