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.

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

Fixes duplicated models in single allOf scenarios #4694

Merged
merged 4 commits into from
May 21, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- Fixed a bug where the hash alias in typescript wasn't being generated uniformly for similar interfaces [microsoftgraph/msgraph-beta-sdk-typescript#84](https://github.com/microsoftgraph/msgraph-beta-sdk-typescript/issues/84)
- Fixes a bug where name collisions would occur in the Typescript refiner if model name also exists with the `Interface` suffix [#4382](https://github.com/microsoft/kiota/issues/4382)
- Fixes a bug where paths without operationIds would not be included in the generated plugins and ensured operationIds are cleaned up [#4642](https://github.com/microsoft/kiota/issues/4642)
- Fixes a bug where models would be duplicated in some allOf scenarios [#4191](https://github.com/microsoft/kiota/issues/4191)
- Fixes a bug where CLI Generation does not handle path parameters of type "string" and format "date", "date-time", "time", etc. [#4615](https://github.com/microsoft/kiota/issues/4615)

## [1.14.0] - 2024-05-02
Expand Down
12 changes: 6 additions & 6 deletions src/Kiota.Builder/KiotaBuilder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@
#pragma warning restore CA1031
{
if (!skipErrorLog)
logger.LogCritical("error getting the API manifest: {ExceptionMessage}", ex.Message);

Check warning on line 139 in src/Kiota.Builder/KiotaBuilder.cs

View workflow job for this annotation

GitHub Actions / Build

Logging in a catch clause should pass the caught exception as a parameter. (https://rules.sonarsource.com/csharp/RSPEC-6667)
return null;
}
}
Expand Down Expand Up @@ -692,7 +692,7 @@
methodToAdd.AddParameter(new CodeParameter
{
Name = "rawUrl",
Type = new CodeType { Name = "string", IsExternal = true },

Check warning on line 695 in src/Kiota.Builder/KiotaBuilder.cs

View workflow job for this annotation

GitHub Actions / Build

Define a constant instead of using this literal 'string' 19 times. (https://rules.sonarsource.com/csharp/RSPEC-1192)
Optional = false,
Documentation = new()
{
Expand Down Expand Up @@ -806,7 +806,7 @@
IsAsync = false,
IsStatic = false,
Documentation = new(new() {
{"TypeName", new CodeType {

Check warning on line 809 in src/Kiota.Builder/KiotaBuilder.cs

View workflow job for this annotation

GitHub Actions / Build

Define a constant instead of using this literal 'TypeName' 5 times. (https://rules.sonarsource.com/csharp/RSPEC-1192)
IsExternal = false,
TypeDefinition = currentClass,
}
Expand Down Expand Up @@ -1048,7 +1048,7 @@

if (!"string".Equals(parameter.Type.Name, StringComparison.OrdinalIgnoreCase) && config.IncludeBackwardCompatible)
{ // adding a second indexer for the string version of the parameter so we keep backward compatibility
//TODO remove for v2

Check warning on line 1051 in src/Kiota.Builder/KiotaBuilder.cs

View workflow job for this annotation

GitHub Actions / Build

Complete the task associated to this 'TODO' comment. (https://rules.sonarsource.com/csharp/RSPEC-1135)
var backCompatibleValue = (CodeIndexer)result[0].Clone();
backCompatibleValue.Name += "-string";
backCompatibleValue.IndexParameter.Type = DefaultIndexerParameterType;
Expand Down Expand Up @@ -1122,15 +1122,15 @@
var primitiveTypeName = (typeName?.ToLowerInvariant(), format?.ToLowerInvariant()) switch
{
("string", "base64url") => "base64url",
("file", _) => "binary",

Check warning on line 1125 in src/Kiota.Builder/KiotaBuilder.cs

View workflow job for this annotation

GitHub Actions / Build

Define a constant instead of using this literal 'binary' 6 times. (https://rules.sonarsource.com/csharp/RSPEC-1192)
("string", "duration") => "TimeSpan",
("string", "time") => "TimeOnly",
("string", "date") => "DateOnly",
("string", "date-time") => "DateTimeOffset",
("string", "uuid") => "Guid",
("string", _) => "string", // covers commonmark and html
("number", "double" or "float" or "decimal") => format.ToLowerInvariant(),

Check warning on line 1132 in src/Kiota.Builder/KiotaBuilder.cs

View workflow job for this annotation

GitHub Actions / Build

Define a constant instead of using this literal 'number' 6 times. (https://rules.sonarsource.com/csharp/RSPEC-1192)
("number" or "integer", "int8") => "sbyte",

Check warning on line 1133 in src/Kiota.Builder/KiotaBuilder.cs

View workflow job for this annotation

GitHub Actions / Build

Define a constant instead of using this literal 'integer' 6 times. (https://rules.sonarsource.com/csharp/RSPEC-1192)
("number" or "integer", "uint8") => "byte",
("number" or "integer", "int64") => "int64",
("number", "int32") => "integer",
Expand Down Expand Up @@ -1202,7 +1202,7 @@
var suffix = $"{operationType}Response";
var modelType = CreateModelDeclarations(currentNode, schema, operation, parentClass, suffix);
if (modelType is not null && config.IncludeBackwardCompatible && config.Language is GenerationLanguage.CSharp or GenerationLanguage.Go && modelType.Name.EndsWith(suffix, StringComparison.Ordinal))
{ //TODO remove for v2

Check warning on line 1205 in src/Kiota.Builder/KiotaBuilder.cs

View workflow job for this annotation

GitHub Actions / Build

Complete the task associated to this 'TODO' comment. (https://rules.sonarsource.com/csharp/RSPEC-1135)
var obsoleteTypeName = modelType.Name[..^suffix.Length] + "Response";
if (modelType is CodeType codeType &&
codeType.TypeDefinition is CodeClass codeClass)
Expand Down Expand Up @@ -1335,7 +1335,7 @@
executorMethod.AddParameter(cancellationParam);// Add cancellation token parameter

if (returnTypes.Item2 is not null && config.IncludeBackwardCompatible)
{ //TODO remove for v2

Check warning on line 1338 in src/Kiota.Builder/KiotaBuilder.cs

View workflow job for this annotation

GitHub Actions / Build

Complete the task associated to this 'TODO' comment. (https://rules.sonarsource.com/csharp/RSPEC-1135)
var additionalExecutorMethod = (CodeMethod)executorMethod.Clone();
additionalExecutorMethod.ReturnType = returnTypes.Item2;
additionalExecutorMethod.OriginalMethod = executorMethod;
Expand Down Expand Up @@ -1595,12 +1595,6 @@
{
var flattenedAllOfs = schema.AllOf.FlattenSchemaIfRequired(static x => x.AllOf).ToArray();
var referenceId = schema.Reference?.Id;
var className = (schema.GetSchemaName(true) is string cName && !string.IsNullOrEmpty(cName) ?
cName :
(!string.IsNullOrEmpty(typeNameForInlineSchema) ?
typeNameForInlineSchema :
currentNode.GetClassName(config.StructuredMimeTypes, operation: operation, suffix: classNameSuffix, schema: schema, requestBody: isRequestBody)))
.CleanupSymbolName();
var shortestNamespaceName = GetModelsNamespaceNameFromReferenceId(referenceId);
var codeNamespaceFromParent = GetShortestNamespace(codeNamespace, schema);
if (rootNamespace is null)
Expand All @@ -1609,6 +1603,12 @@
var inlineSchema = Array.Find(flattenedAllOfs, static x => !x.IsReferencedSchema());
var referencedSchema = Array.Find(flattenedAllOfs, static x => x.IsReferencedSchema());
var rootSchemaHasProperties = schema.HasAnyProperty();
var className = (schema.GetSchemaName(schema.IsSemanticallyMeaningful()) is string cName && !string.IsNullOrEmpty(cName) ?
cName :
(!string.IsNullOrEmpty(typeNameForInlineSchema) ?
typeNameForInlineSchema :
currentNode.GetClassName(config.StructuredMimeTypes, operation: operation, suffix: classNameSuffix, schema: schema, requestBody: isRequestBody)))
.CleanupSymbolName();
var codeDeclaration = (rootSchemaHasProperties, inlineSchema, referencedSchema) switch
{
// greatest parent type
Expand Down Expand Up @@ -2374,7 +2374,7 @@
if (!parameterClass.ContainsPropertyWithWireName(prop.WireName))
{
if (addBackwardCompatibleParameter && config.IncludeBackwardCompatible && config.Language is GenerationLanguage.CSharp or GenerationLanguage.Go)
{ //TODO remove for v2

Check warning on line 2377 in src/Kiota.Builder/KiotaBuilder.cs

View workflow job for this annotation

GitHub Actions / Build

Complete the task associated to this 'TODO' comment. (https://rules.sonarsource.com/csharp/RSPEC-1135)
var modernProp = (CodeProperty)prop.Clone();
modernProp.Name = $"{prop.Name}As{modernProp.Type.Name.ToFirstCharacterUpperCase()}";
modernProp.SerializationName = prop.WireName;
Expand Down
79 changes: 79 additions & 0 deletions tests/Kiota.Builder.Tests/KiotaBuilderTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -7400,6 +7400,85 @@ public async Task InheritanceWithAllOfInBaseType()
Assert.NotNull(codeModel.FindChildByName<CodeClass>("Group"));
}
[Fact]
public async Task InlineSchemaWithSingleAllOfReference()
{
var tempFilePath = Path.Combine(Path.GetTempPath(), Path.GetTempFileName());
await using var fs = await GetDocumentStream(@"openapi: 3.0.1
info:
title: OData Service for namespace microsoft.graph
description: This OData service is located at https://graph.microsoft.com/v1.0
version: 1.0.1
servers:
- url: https://graph.microsoft.com/v1.0
paths:
/user:
get:
responses:
'200':
content:
application/json:
schema:
$ref: '#/components/schemas/microsoft.graph.user'
/group/members:
get:
responses:
'200':
content:
application/json:
schema:
$ref: '#/components/schemas/microsoft.graph.member'
components:
schemas:
microsoft.graph.directoryObject:
required: ['@odata.type']
properties:
'@odata.type':
type: 'string'
default: '#microsoft.graph.directoryObject'
discriminator:
propertyName: '@odata.type'
mapping:
'#microsoft.graph.group': '#/components/schemas/microsoft.graph.group'
microsoft.graph.group:
allOf:
- '$ref': '#/components/schemas/microsoft.graph.directoryObject'
- title: 'group'
type: 'object'
properties:
groupprop:
type: 'string'
microsoft.graph.member:
type: 'object'
properties:
group:
allOf:
- '$ref': '#/components/schemas/microsoft.graph.group'
microsoft.graph.user:
properties:
groups:
type: array
items:
- '$ref': '#/components/schemas/microsoft.graph.group'");
var mockLogger = new Mock<ILogger<KiotaBuilder>>();
var builder = new KiotaBuilder(mockLogger.Object, new GenerationConfiguration { ClientClassName = "Graph", OpenAPIFilePath = tempFilePath }, _httpClient);
var document = await builder.CreateOpenApiDocumentAsync(fs);
var node = builder.CreateUriSpace(document);
var codeModel = builder.CreateSourceModel(node);
var memberClass = codeModel.FindChildByName<CodeClass>("member");
Assert.NotNull(memberClass);
Assert.Equal(2, memberClass.Properties.Count());// single prop plus additionalData
Assert.Null(memberClass.StartBlock.Inherits);//no base
var userClass = codeModel.FindChildByName<CodeClass>("user");
Assert.NotNull(userClass);
Assert.Equal(2, userClass.Properties.Count());// single prop plus additionalData
Assert.Null(userClass.StartBlock.Inherits);//no base
var inlinedClassThatIsDuplicate = codeModel.FindChildByName<CodeClass>("member_group");
Assert.Null(inlinedClassThatIsDuplicate);//no duplicate
var modelsNamespace = codeModel.FindChildByName<CodeNamespace>("ApiSdk.models.microsoft.graph");
Assert.NotNull(modelsNamespace);
Assert.Equal(4, modelsNamespace.Classes.Count());// only 4 classes for user, member, group and directoryObject
}
[Fact]
public async Task InheritanceWithAllOfWith3Parts3SchemaChildClass()
{
var tempFilePath = Path.Combine(Path.GetTempPath(), Path.GetTempFileName());
Expand Down
Loading