Skip to content
Open
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
6 changes: 6 additions & 0 deletions src/Kiota.Builder/CodeDOM/CodeEnum.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
using System.Collections.Concurrent;
using System.Collections.Generic;
using System.Linq;
using Microsoft.OpenApi;

namespace Kiota.Builder.CodeDOM;
#pragma warning disable CA1711
Expand Down Expand Up @@ -42,4 +43,9 @@ public CodeConstant? CodeEnumObject
{
get; set;
}

public JsonSchemaType? BackingType
{
get; set;
}
}
6 changes: 5 additions & 1 deletion src/Kiota.Builder/Extensions/OpenApiSchemaExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,8 @@ public static IEnumerable<string> GetSchemaNames(this IOpenApiSchema schema, boo
return schema switch
{
OpenApiSchemaReference reference => reference.Reference?.Id,
// Quickfix for https://github.com/microsoft/kiota/issues/6776 ToDo: Properly handle nullable types in accordance with OpenAPI 3.
OpenApiSchema {OneOf: [OpenApiSchema{Type: JsonSchemaType.Null}, OpenApiSchemaReference reference]} => reference.Reference?.Id,
OpenApiSchema s when s.GetMergedSchemaOriginalReferenceId() is string originalReferenceId => originalReferenceId,
_ => null,
};
Expand All @@ -59,6 +61,8 @@ public static bool IsReferencedSchema(this IOpenApiSchema schema)
return schema switch
{
OpenApiSchemaReference => true,
// Quickfix for https://github.com/microsoft/kiota/issues/6776 ToDo: Properly handle nullable types in accordance with OpenAPI 3.
OpenApiSchema {OneOf: [OpenApiSchema{Type: JsonSchemaType.Null}, OpenApiSchemaReference reference]} => true,
_ => false,
};
}
Expand Down Expand Up @@ -318,7 +322,7 @@ public static bool IsSemanticallyMeaningful(this IOpenApiSchema schema, bool ign
return schema.HasAnyProperty() ||
(!ignoreEnums && schema.Enum is { Count: > 0 }) ||
(!ignoreArrays && schema.Items != null) ||
(!ignoreType && schema.Type is not null &&
(!ignoreType && schema.Type is not null and not JsonSchemaType.Null &&
((ignoreNullableObjects && !schema.IsObjectType()) ||
!ignoreNullableObjects)) ||
!string.IsNullOrEmpty(schema.Format) ||
Expand Down
1 change: 1 addition & 0 deletions src/Kiota.Builder/KiotaBuilder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2021,6 +2021,7 @@ private CodeElement AddModelDeclarationIfDoesntExist(OpenApiUrlTreeNode currentN
currentNode.GetPathItemDescription(Constants.DefaultOpenApiLabel),
},
Deprecation = schema.GetDeprecationInformation(),
BackingType = schema.Type,
};
SetEnumOptions(schema, newEnum);
return currentNamespace.AddEnum(newEnum).First();
Expand Down
73 changes: 62 additions & 11 deletions src/Kiota.Builder/Writers/CSharp/CodeMethodWriter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
using Kiota.Builder.CodeDOM;
using Kiota.Builder.Extensions;
using Kiota.Builder.OrderComparers;
using Microsoft.OpenApi;

namespace Kiota.Builder.Writers.CSharp;

Expand Down Expand Up @@ -141,10 +142,21 @@ private void WriteFactoryMethodBodyForUnionModel(CodeMethod codeElement, CodeCla
}
else if (propertyType.TypeDefinition is CodeClass && propertyType.IsCollection || propertyType.TypeDefinition is null || propertyType.TypeDefinition is CodeEnum)
{
var readerReference = parseNodeParameter.Name.ToFirstCharacterLowerCase();
var selfReference = $"{ResultVarName}.{property.Name.ToFirstCharacterUpperCase()}";
var deserializationMethodName = GetDeserializationMethodName(propertyType, codeElement);
var typeName = conventions.GetTypeString(propertyType, codeElement, true, (propertyType.TypeDefinition is CodeEnum || conventions.IsPrimitiveType(propertyType.Name)) && propertyType.CollectionKind is not CodeTypeBase.CodeTypeCollectionKind.None);
var valueVarName = $"{property.Name.ToFirstCharacterLowerCase()}Value";
writer.WriteLine($"{(includeElse ? "else " : string.Empty)}if({parseNodeParameter.Name.ToFirstCharacterLowerCase()}.{GetDeserializationMethodName(propertyType, codeElement)} is {typeName} {valueVarName})");
writer.WriteBlock(lines: $"{ResultVarName}.{property.Name.ToFirstCharacterUpperCase()} = {valueVarName};");
if (propertyType.TypeDefinition is CodeType { TypeDefinition: CodeEnum {BackingType: JsonSchemaType.Integer}})
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This need test coverage to ensure the deserialization code is generated correctly for the case of a union model with an integer enum.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not entirely sure what you mean by that, could you provide a sample json?
Also, where are those tests currently located at?

Thanks in advance

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here's where the tests for this method start:

public void WritesModelFactoryBodyForUnionModels()

A union model with and integer enum might looks something like:

openapi: 3.0.3
info:
  title: Union with Integer Enum API
  version: 1.0.0
servers:
  - url: https://api.example.com/v1

paths:
  /status:
    get:
      summary: Get status that can be multiple types
      responses:
        '200':
          description: A union type containing an integer enum
          content:
            application/json:
              schema:
                $ref: '#/components/schemas/StatusUnion'

components:
  schemas:
    # Integer-backed enum
    StatusEnum:
      type: integer
      enum: [0, 1, 2]
      x-ms-enum:
        name: StatusEnum
        modelAsString: false
        values:
          - name: Active
            value: 0
            description: "The resource is active"
          - name: Inactive
            value: 1
            description: "The resource is inactive"
          - name: Pending
            value: 2
            description: "The resource is pending"
    
    # A complex object type
    DetailedStatus:
      type: object
      required:
        - message
      properties:
        message:
          type: string
        code:
          type: integer
    
    # Union model combining integer enum and object
    StatusUnion:
      oneOf:
        - $ref: '#/components/schemas/StatusEnum'
        - $ref: '#/components/schemas/DetailedStatus'
      discriminator:
        propertyName: '@odata.type'
        mapping:
          '#kiota.statusEnum': '#/components/schemas/StatusEnum'
          '#kiota.detailedStatus': '#/components/schemas/DetailedStatus'

{
writer.WriteLine($"{(includeElse ? "else " : string.Empty)}if({readerReference}.{deserializationMethodName} is int {valueVarName})");
writer.WriteBlock(lines: $"{selfReference} = ({typeName}){valueVarName};");
}
else
{
writer.WriteLine($"{(includeElse ? "else " : string.Empty)}if({readerReference}.{deserializationMethodName} is {typeName} {valueVarName})");
writer.WriteBlock(lines: $"{selfReference} = {valueVarName};");
}
}
if (!includeElse)
includeElse = true;
Expand All @@ -162,10 +174,21 @@ private void WriteFactoryMethodBodyForIntersectionModel(CodeMethod codeElement,
{
if (property.Type is CodeType propertyType)
{
var readerReference = parseNodeParameter.Name.ToFirstCharacterLowerCase();
var selfReference = $"{ResultVarName}.{property.Name.ToFirstCharacterUpperCase()}";
var deserializationMethodName = GetDeserializationMethodName(propertyType, codeElement);
var typeName = conventions.GetTypeString(propertyType, codeElement, true, propertyType.TypeDefinition is CodeEnum && propertyType.CollectionKind is not CodeTypeBase.CodeTypeCollectionKind.None);
var valueVarName = $"{property.Name.ToFirstCharacterLowerCase()}Value";
writer.WriteLine($"{(includeElse ? "else " : string.Empty)}if({parseNodeParameter.Name.ToFirstCharacterLowerCase()}.{GetDeserializationMethodName(propertyType, codeElement)} is {typeName} {valueVarName})");
writer.WriteBlock(lines: $"{ResultVarName}.{property.Name.ToFirstCharacterUpperCase()} = {valueVarName};");
if (propertyType.TypeDefinition is CodeType { TypeDefinition: CodeEnum {BackingType: JsonSchemaType.Integer}})
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This need test coverage to ensure the deserialization code is generated correctly for the case of a intersection model with an integer enum.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not entirely sure what you mean by that, could you provide a sample json?
Also, where are those tests currently located at?

Thanks in advance

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Intersection model tests start here:

public void WritesModelFactoryBodyForIntersectionModels()

Example of an intersection model based on the previous union model example

    StatusIntersection:
      allOf:
        - $ref: '#/components/schemas/StatusEnum'
        - $ref: '#/components/schemas/DetailedStatus'

{
writer.WriteLine($"{(includeElse ? "else " : string.Empty)}if({readerReference}.{deserializationMethodName} is int {valueVarName})");
writer.WriteBlock(lines: $"{selfReference} = ({typeName}){valueVarName};");
}
else
{
writer.WriteLine($"{(includeElse ? "else " : string.Empty)}if({readerReference}.{deserializationMethodName} is {typeName} {valueVarName})");
writer.WriteBlock(lines: $"{selfReference} = {valueVarName};");
}
}
if (!includeElse)
includeElse = true;
Expand Down Expand Up @@ -349,7 +372,13 @@ private void WriteDeserializerBodyForInheritedModel(bool shouldHide, CodeMethod
.Where(static x => !x.ExistsInBaseType)
.OrderBy(static x => x.Name, StringComparer.Ordinal))
{
writer.WriteLine($"{{ \"{otherProp.WireName}\", n => {{ {otherProp.Name.ToFirstCharacterUpperCase()} = n.{GetDeserializationMethodName(otherProp.Type, codeElement)}; }} }},");
if (otherProp is {Type: CodeType { TypeDefinition: CodeEnum {BackingType: JsonSchemaType.Integer}} propertyType})
{
var typeName = conventions.GetTypeString(propertyType, codeElement, true, (propertyType.TypeDefinition is CodeEnum || conventions.IsPrimitiveType(propertyType.Name)) && propertyType.CollectionKind is not CodeTypeBase.CodeTypeCollectionKind.None);
writer.WriteLine($"{{ \"{otherProp.WireName}\", n => {{ {otherProp.Name.ToFirstCharacterUpperCase()} = ({typeName}?) n.{GetDeserializationMethodName(otherProp.Type, codeElement)}; }} }},");
}
else
writer.WriteLine($"{{ \"{otherProp.WireName}\", n => {{ {otherProp.Name.ToFirstCharacterUpperCase()} = n.{GetDeserializationMethodName(otherProp.Type, codeElement)}; }} }},");
}
writer.CloseBlock("};");
}
Expand All @@ -370,7 +399,10 @@ private string GetDeserializationMethodName(CodeTypeBase propType, CodeMethod me
return $"GetCollectionOfObjectValues<{propertyType}>({propertyType}.CreateFromDiscriminatorValue){collectionMethod}";
}
else if (currentType.TypeDefinition is CodeEnum enumType)
return $"GetEnumValue<{enumType.GetFullName()}>()";
if (enumType.BackingType is JsonSchemaType.Integer)
return $"GetIntValue()";
else
return $"GetEnumValue<{enumType.GetFullName()}>()";
}
return propertyType switch
{
Expand Down Expand Up @@ -482,7 +514,10 @@ private void WriteSerializerBodyForInheritedModel(bool shouldHide, CodeMethod me
.OrderBy(static x => x.Name))
{
var serializationMethodName = GetSerializationMethodName(otherProp.Type, method);
writer.WriteLine($"writer.{serializationMethodName}(\"{otherProp.WireName}\", {otherProp.Name.ToFirstCharacterUpperCase()});");
if (otherProp.Type is CodeType{TypeDefinition: CodeEnum {BackingType: JsonSchemaType.Integer}})
writer.WriteLine($"writer.{serializationMethodName}(\"{otherProp.WireName}\", (int?){otherProp.Name.ToFirstCharacterUpperCase()});");
else
writer.WriteLine($"writer.{serializationMethodName}(\"{otherProp.WireName}\", {otherProp.Name.ToFirstCharacterUpperCase()});");
}
}
private void WriteSerializerBodyForUnionModel(CodeMethod method, CodeClass parentClass, LanguageWriter writer)
Expand All @@ -494,8 +529,12 @@ private void WriteSerializerBodyForUnionModel(CodeMethod method, CodeClass paren
.OrderBy(static x => x, CodePropertyTypeForwardComparer)
.ThenBy(static x => x.Name))
{
var serializationMethodName = GetSerializationMethodName(otherProp.Type, method);
writer.WriteLine($"{(includeElse ? "else " : string.Empty)}if({otherProp.Name.ToFirstCharacterUpperCase()} != null)");
writer.WriteBlock(lines: $"writer.{GetSerializationMethodName(otherProp.Type, method)}(null, {otherProp.Name.ToFirstCharacterUpperCase()});");
if (otherProp.Type is CodeType{TypeDefinition: CodeEnum {BackingType: JsonSchemaType.Integer}})
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not seeing any tests for serialization of union models that include Integer Enums, let's make sure test cases for this are added.

It feels a bit weird that the integer case is passing otherProp.WireName which is inconsistent with how the existing case works.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am very aware of that, wanted to avoid cross-repository changes tho and have this easily integrateable on my end.

Could you hint me at where those tests are currently placed and how they look? Thanks in advance.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here's where we have an existing test for the union model serialization

public void WritesUnionDeSerializerBody()

It focuses on the code that is generated, rather than the behavior of the serialization library that we have as a dependency.

writer.WriteBlock(lines: $"writer.{serializationMethodName}(\"{otherProp.WireName}\", (int?){otherProp.Name.ToFirstCharacterUpperCase()});");
else
writer.WriteBlock(lines: $"writer.{serializationMethodName}(null, {otherProp.Name.ToFirstCharacterUpperCase()});");
if (!includeElse)
includeElse = true;
}
Expand All @@ -510,8 +549,12 @@ private void WriteSerializerBodyForIntersectionModel(CodeMethod method, CodeClas
.OrderBy(static x => x, CodePropertyTypeBackwardComparer)
.ThenBy(static x => x.Name))
{
var serializationMethodName = GetSerializationMethodName(otherProp.Type, method);
writer.WriteLine($"{(includeElse ? "else " : string.Empty)}if({otherProp.Name.ToFirstCharacterUpperCase()} != null)");
writer.WriteBlock(lines: $"writer.{GetSerializationMethodName(otherProp.Type, method)}(null, {otherProp.Name.ToFirstCharacterUpperCase()});");
if (otherProp.Type is CodeType{TypeDefinition: CodeEnum {BackingType: JsonSchemaType.Integer}})
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not seeing any tests for serialization of intersection models that include Integer Enums, let's make sure test cases for this are added.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not entirely sure what you mean by that, could you provide a sample json?
Also, where are those tests currently located at?

Thanks in advance

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here's the existing test for generating the serialization method

public void WritesIntersectionDeSerializerBody()

I have supplied sample open api for these types of schema on the factory method/deserialization comments

writer.WriteBlock(lines: $"writer.{serializationMethodName}(\"{otherProp.WireName}\", (int?){otherProp.Name.ToFirstCharacterUpperCase()});");
else
writer.WriteBlock(lines: $"writer.{serializationMethodName}(null, {otherProp.Name.ToFirstCharacterUpperCase()});");
if (!includeElse)
includeElse = true;
}
Expand All @@ -529,7 +572,12 @@ private void WriteSerializerBodyForIntersectionModel(CodeMethod method, CodeClas
.Select(static x => x.Name.ToFirstCharacterUpperCase())
.OrderBy(static x => x)
.Aggregate(static (x, y) => $"{x}, {y}");
writer.WriteLine($"writer.{GetSerializationMethodName(complexProperties.First().Type, method)}(null, {propertiesNames});");
var prop = complexProperties.First();
var serializationMethodName = GetSerializationMethodName(prop.Type, method);
if (prop.Type is CodeType{TypeDefinition: CodeEnum {BackingType: JsonSchemaType.Integer}})
writer.WriteLine($"writer.{serializationMethodName}(\"{prop.WireName}\", (int?){prop.Name.ToFirstCharacterUpperCase()});");
else
writer.WriteLine($"writer.{GetSerializationMethodName(complexProperties.First().Type, method)}(null, {propertiesNames});");
if (includeElse)
{
writer.CloseBlock();
Expand Down Expand Up @@ -678,7 +726,10 @@ private string GetSerializationMethodName(CodeTypeBase propType, CodeMethod meth
else
return $"WriteCollectionOfObjectValues<{propertyType}>";
else if (currentType.TypeDefinition is CodeEnum enumType)
return $"WriteEnumValue<{enumType.GetFullName()}>";
if (enumType.BackingType is JsonSchemaType.Integer)
return $"WriteIntValue";
else
return $"WriteEnumValue<{enumType.GetFullName()}>";

}
return propertyType switch
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using System.Collections.Generic;
using System.Linq;
using Kiota.Builder.Extensions;
using Microsoft.OpenApi;
using Xunit;
Expand Down Expand Up @@ -80,6 +81,18 @@ public void ExternalReferencesAreSupported()
Assert.True(mockSchema.IsReferencedSchema());
}
[Fact]
public void ExternalNullableReferencesAreSupported()
{
var mockSchema = new OpenApiSchema
{
OneOf = [
new OpenApiSchema{Type = JsonSchemaType.Null},
new OpenApiSchemaReference("example.json#/path/to/component", null, "http://example.com/example.json")
]
};
Assert.True(mockSchema.IsReferencedSchema());
}
[Fact]
public void SchemasAreNotConsideredReferences()
{
var mockSchema = new OpenApiSchema();
Expand Down Expand Up @@ -356,6 +369,20 @@ public void GetReferenceIdsOneOf()
Assert.Contains("microsoft.graph.user", names);
}
[Fact]
public void GetReferenceIdsOneOfNullable()
{
var schema = new OpenApiSchema
{
OneOf = [
new OpenApiSchema{Type = JsonSchemaType.Null},
new OpenApiSchemaReference("microsoft.graph.user")
]
};
var names = schema.GetSchemaReferenceIds();
var name = Assert.Single(names);
Assert.Contains("microsoft.graph.user", name);
}
[Fact]
public void GetReferenceIdsItems()
{
var schema = new OpenApiSchema
Expand Down
Loading