Addressing the issue of System.ArgumentException: An item with the same key has already been added

[arashcomsense]

This PR is related to issue #94

The fix consists of the following key enhancements:

• Added UseTypeFullName boolean property to OpenApiPayloadAttribute class and defaulted it to false
• Introduced the local function of UtilizeTypeFullName in GetOpenApiResponses() method to determine whether to use Type.Name or Type.FullName

bool UtilizeTypeFullName(Type type, List<OpenApiResponseWithBodyAttribute> openApiResponseWithBodyAttributes)
{
    var attribute = openApiResponseWithBodyAttributes.Where(p => p.BodyType.FullName == type.FullName);
    var typeFullNameUtilized = attribute.FirstOrDefault()?.UseTypeFullName ?? false;

     if (typeFullNameUtilized && !this._acceptor.TypesAcceptedWithFullName.Contains(type.FullName))
     {
           this._acceptor.TypesAcceptedWithFullName.Add(type.FullName);
     }
     return typeFullNameUtilized;
}

• Added List<string> TypesAcceptedWithFullName { get; set; } property to the accelerator to track eligible types' full names
• GetOpenApiReferenceId(...) method augmented to receive the boolean parameter of UseTypeFullName
• Replaced Id = type.Value.GetOpenApiReferenceId(isDictionary: false, isList: false, namingStrategy) with Id = type.Value.GetOpenApiReferenceId(isDictionary: false, isList: false, namingStrategy, (acceptor as IOpenApiSchemaAcceptor)?.TypesAcceptedWithFullName?.Contains(type.Value.FullName) ?? false) in the Visit method of ObjectTypeVisitor class

[ghost]

All CLA requirements met.

[justinyoo]

@arashcomsense Thanks for your contribution! I've just taken a quick look, and I like your idea!

What I'm concerned of your approach, using the type's full name, is that it could be too long for certain circumstances.

For example with our sample app, there is a model called, DummyRequestModel whose full name is Microsoft.Azure.WebJobs.Extensions.OpenApi.FunctionApp.Models.DummyRequestModel. If there are two DummyRequestModel classes with different structure, then one of the reference value could be #/components/schemas/Microsoft.Azure.WebJobs.Extensions.OpenApi.FunctionApp.Models.DummyRequestModel.

It has two potential issues against your PR:

• Too long: as mentioned above,
• Not conformed: I don't see any logic around handling the dots(.)

I guess we can improve this by doing something else. What's your thought?

This issue only occurs for the nested classes/records/structs. What if we use the OuterClass_NestedClass approach, instead of using the full name one? With your specific example, they could be MySubmittedData_MyRequest and YourSubmittedData_MyRequest.

[mark-comeau]

I like the idea of the shorter names with configuration options to allow for customizations when necessary to avoid naming collisions. The ability to specify identity a shortened name for a namespace probably handles most situations(IOpenApiConfigurationOptions? Function Startup?):

• This.Is.My.Library.Functions > This_ becomes This_MySubmittedData_MyRequest.
• That.Is.My.Library.Functions > That_ becomes That_MySubmittedData_MyRequest.
• My.Library.Functions.Models.Order > Order_ becomes Order_MySubmittedData_MyRequest.
• My.Library.Functions.Models.Customer > Customer_ becomes Customer_MySubmittedData_MyRequest.

[arashcomsense]

@justinyoo Thanks for your reply.

While I like shortening names, I have a few questions related to it and your comments though:

• If we do not shorten the names and use full names instead, they can easily be mapped to their backend types when they appear on the Swagger UI's page. Shortening names requires another level of decoding in people's minds and understanding of how name mapping works.
• Since I am not 100% familiar with this library's code, I was wondering why we should handle the dots instead of keeping them?

[justinyoo]

• If we do not shorten the names and use full names instead, they can easily be mapped to their backend types when they appear on the Swagger UI's page. Shortening names requires another level of decoding in people's minds and understanding of how name mapping works.

It's really a good question. I chose, by design, to have a class/struct name, rather than the full name.

• Since I am not 100% familiar with this library's code, I was wondering why we should handle the dots instead of keeping them?

As far as I know, the dots are only for the reference URLs, not the object types defined within the same document. As we're not using the full name, we don't need to handle dots, but for this specific issue, we need to concatenate both outer class and nested one with a connector. By design, the underscore(_) is chosen.

[arashcomsense]

@justinyoo I made the following changes in the code and pushed them to this PR:

1. I ensured that the referenceIds are conformed by replacing dots . and plus signs + with an underscore _. E.g. MyNamespace.MyClass+MyResponse is referenced as myNamespace_MyClass_MyResponse
2. I introduced a property called BodyTypeAlias to give the users the option of choosing a different (shortened) name for response classes.

Examples and BodyTypeAlias usage in OpenApiResponseWithBody attribute:

        [OpenApiResponseWithBody(
           statusCode: HttpStatusCode.OK, 
            contentType: "application/json", 
            bodyType: typeof(MySubmittedData.MyResponse), 
            BodyTypeAlias = "MyAliasResponse",
            UseTypeFullName = true,
            Description = "The OK response")]
            public static async Task<IActionResult> Run1(
               [HttpTrigger(AuthorizationLevel.Function, "post", Route = null)] HttpRequest req,
               ILogger log)
        {
            return new OkObjectResult(new MySubmittedData.MyResponse { Code = 1, Message = "Test response" });
        }

        [OpenApiResponseWithBody(
            statusCode: HttpStatusCode.OK, 
            contentType: "application/json", 
            bodyType: typeof(YourSubmittedData.MyResponse),
            BodyTypeAlias = "MyAliasResponseNo2",
            UseTypeFullName = true,
            Description = "The OK response")]
        public static async Task<IActionResult> Run2(
        [HttpTrigger(AuthorizationLevel.Function, "post", Route = null)] HttpRequest req,
        ILogger log)
        {
            return new OkObjectResult(new YourSubmittedData.MyResponse { Code = 1, Desccription = "Test response" });
        }

We could implement a routine to get the code to shorten nested classes' references by picking the first and last parts of the type's full name string, but the following challenges arise:

a) Imagine we have two, three, or more nested response classes following the pattern below, assuming OuterClass and NestedClass are different but sharing the same names:

Part1OfNamespace.OuterClass+NestedClass Part1OfNamespace.Part2OfNameSpace.OuterClass+NestedClass Part1OfNamespace.Part2OfNameSpace.Part3OfNameSpace.OuterClass+NestedClass Part1OfNamespace.AnotherOuterClass+NestedClass

As you see, picking the first and last portions of the type names do not suffice to build a unique referenceId, and a bit deeper algorithm is required to determine such referenceIds. This algorithm will have to record and track the built-up referenceIds to avoid possible duplications. I would suggest that we defer this discussion and solution to another Github issue.

If you think that this updated PR addresses the issue #94, please merge it with the main branch to help us progress our internal projects. Thanks.

[level120]

I think that this PR can be resolved below cases. It is the same as a nested case.

namespace Company.Some
{
    public enum Status { ... }
}

namespace Company.Any
{
    public enum Status { ... }
}

In my opinion, How about adding an option in DefaultOpenApiConfigurationOptions.

Maybe..

enum TypeNameOption { None, Global, Special }

This option is three values and the default is 'None'. The 'Global' option can apply to all projects. The 'Special' option is only to follow...

[OpenApiRequestWithBody(UseTypeFullName = true)]

// or

[OpenApiResponseWithBody(UseTypeFullName = true)]

[RealGoodAnthony]

I'm not sure that this will solve all instances of this exception: #356

[justinyoo]

#610 has taken care of this issue.