This issue is to track work required to support parsing /// comments from C# and generating open api document from it.

Given this annotation,

/// <param name="MS-CorrelationId" cref="string" in="headerZZZ">Correlation ID for the request</param>

The error produced is,

ExceptionType: MissingInAttributeException || Message: In attribute is missing from parameter(s): "MS-CorrelationId-CorrelationId"

There are two bugs,

1. The messaging should indicate the "in" attribute value is not supported. Ideally, it would indicate what values are supported at the same time.
2. The parameter name in the message is wrong.

Rename the project and namespaces to match agreed nomenclature Microsoft.OpenApi.CSharpComment.Reader

• Rename repo to OpenAPI.NET-CSharpAnnotations
• Rename reader csproj to Microsoft.OpenApi.CSharpAnnotations.DocumentGeneration
• Rename generator file to OpenApiGenerator

Our filters should detect and report all issues instead of just the first. This will help customers save time by avoiding peeling the onion.

For example, when the following annotations are present, only the first produces an error.

/// <response code="200"></response>
/// <response code="400"></response>

Generation errors that result from this:

ExceptionType: MissingResponseDescriptionException || Message: Description is missing for response code: "200".

Expected:

ExceptionType: MissingResponseDescriptionException || Message: Description is missing for response code: "200".
ExceptionType: MissingResponseDescriptionException || Message: Description is missing for response code: "400".

As the filter configuration stands, it is difficult to register custom filters outside the context of C# code. This becomes apparent when you start to think about, "how can our VSTS build task allow consumers to register their custom filters?". By means of text fields in the task (or perhaps by using the advanced configuration XML), users will need to specify which custom filters to add.

To this end, I'm of the opinion we need to simplify how filters are setup by doing a few pieces of work:

1. All filters should ultimately inherit from a common interface. There may be more specific filter interfaces, but down the line, they should inherit from a common interface type.
2. All filters should have an identifer/name.
3. The set of filters to apply during generation should come from two sources: (a) a default set of filters that are always registered, (b) a custom set of filters that can be registered by name.

Here is my proposal for the common interface:

// ------------------------------------------------------------
//  Copyright (c) Microsoft Corporation.  All rights reserved.
//  Licensed under the MIT License (MIT). See License.txt in the repo root for license information.
// ------------------------------------------------------------

namespace Microsoft.OpenApi.CSharpAnnotations.DocumentGeneration.Filters
{
    /// <summary>
    /// Defines a filter, which is responsible for transforming data from sources such as, but not limited to, C# annotations, attributes, configuration, etc in order to produce an OpenAPI document.
    /// </summary>
    public interface IFilter
    {
        /// <summary>
        /// Gets the name of this filter.
        /// </summary>
        string Name { get; };
    }
}

Most free CIs require that we open source the project first (make github repo public).

Ones that work with C#: TravisCI, AppVeyor

A guid in params is translated to string type with format of uuid but then min and maxLength are added causing issues with autorest.

swagger: '2.0'
info:
title: Test uuid parameter
version: '1.0'
paths:
'/{id}':
parameters:
- in: path
name: id
type: string
format: uuid
minLength: 36
maxLength: 36

I suspect SchemaReferenceRegistry.cs, line 75

                    else if (input == typeof(Guid))
                    {
                        schema.Example = new OpenApiString(Guid.Empty.ToString());
                        schema.MinLength = 36;
                        schema.MaxLength = 36;
                    }

When assemblies cannot be found, which are required for reflection, the status associated with said issue is Warning. This should probably be changed to Error.

For example, the following exception leads to an overall Warning status:

{"ExceptionType":"FileNotFoundException","Message":"Could not load file or assembly 'file:///D:\\a\\1\\s\\Newtonsoft.Json.dll' or one of its dependencies. The system cannot find the file specified."}

https://github.com/Microsoft/OpenAPI.NET.CSharpComment/blob/9d68a37a130d462e135f319d1ef221547967fefe/test/Microsoft.OpenApi.CSharpAnnotations.DocumentGeneration.Tests/packages.config#L6

Microsoft.OpenApi.CSharpAnnotations.NewtonsoftJsonExtensions.Tests is the project for encapsulating Newtonsoft dependency, so only it should have it.

Not sure why the dependency is needed for testing DocumentGeneration project. Something seems fishy.

Today, the Version field of Info Object is populated with the API version instead of the version of the document; this behavior is incorrect. Stamping the version on Info Object holds for V2 specification as well, since it's the "version of the application API".

I believe the document version should be input to the GenerateDocument and GenerateDocuments methods. There is no way to support a rolling build number reliably via a C# comment, so it should likely go onto the config object we have already.

It will help people play around and get familiar with the library before using it in their code base.

Here are some sample InvalidResponseException from the library today.

GET /v1/services/visualStudioXml [{"ExceptionType":"InvalidResponseException","Message":"The documented response  is empty. is not valid."}]
PUT /v1/services/byod [{"ExceptionType":"InvalidResponseException","Message":"The documented response  is not valid."}]

In addition to stating the response is invalid, it should say why, so the user can take corrective action to fix.

I'd like to know which GenerationError is a "failure" versus "warning" when there are multiple present for a DocumentGenerationResult or OperationGenerationResult.

Currently, Newtonsoft.Json.dll is loaded using the method below, which uses the version that C# Comment Reader depends on and not the consumer's version.

private static Assembly ResolveNewtonsoftJsonVersion(object sender, ResolveEventArgs args)
{
    if (args?.Name != null && args.Name.Contains("Newtonsoft.Json"))
    {
        // For any assembly conflict regarding Newtonsoft.Json versions,
        // just load from the existing version of Newtonsoft.Json.
        return Assembly.LoadFrom("Newtonsoft.Json.dll");
    }

    return null;
}

The consumer should be passing all assembly paths required to do reflection in the public methods of CSharpCommentOpenApiGenerator.

Now that we have successfully removed Newtonsoft.Json as a dependency of Microsoft.OpenApi.CSharpComment.Reader and we allow users to define custom filters, we should consider implementing a custom filter to support our existing Newtonsof.Json users who have JsonProperty attributes annotating their contracts. If we implement the custom filter for them, in a centralized manner, we will reduce the friction for them to onboard.

I'm envisioning the custom filter in a simple, lightweight project that is distinct from Microsoft.OpenApi.CSharpComment.Reader. It would need to be nugetized so users could reference it from their projects and supplied to C# Comment Reader as a dependent assembly to load.

Lastly, the nuspec for said custom filter project for target a low version of Newtonsoft.Json, so users are not forced to update to the latest unnecessarily. In theory we should require a minimum version of Newtonsoft.Json which first introduced JsonAttribute property.

When a request body is missing tag we throw invalid request body exception with no actionable error message.

if a response is documented like below

where S is and T is Payload

document generator fails with
Type "Test[]" could not be found. Ensure that it exists in one of the following assemblies

Currently comment reader takes open api spec version as input and when V2 version is provided, it uses OpenAPI.Net library to serialize the V3 DOM to string, but one of the below overload method , returns a DOM which consumer of library would assume is a V2 DOM but its actually isn't. So i am suggesting Comment reader to only generate V3 and consumer can use OpenAPI.Net to convert it to V2.

OpenApiDocument GenerateDocument(
CSharpCommentOpenApiGeneratorConfig cSharpCommentOpenApiGeneratorConfig,
out GenerationDiagnostic generationDiagnostic)

When running C# Comment reader on our SwaggerFD, it produces a document which violates the spec. I realize the below document violates the spec for other reasons (e.g. I've omitted required sections entirely), but this is done on purpose in order to highlight the issue more concisely.

Trimmed down example

{
  "swagger": "2.0",
  "definitions": {
    "Microsoft.UniversalStore.Swagger.Contracts.Metadata.ApiIdentifier": null,
    "Microsoft.UniversalStore.Swagger.Contracts.ApiMetadataStatus": null,
    "Microsoft.UniversalStore.Swagger.Contracts.ApiMetadata": null,
    "Microsoft.UniversalStore.Swagger.FrontDoor.Contracts.V1.Operation": null,
  }
}

There are three generation errors, which look related and relevant to why this is happening:

POST /api/metadata/status [{"ExceptionType":"MissingInAttributeException","Message":"In attribute is missing from parameter(s) apis, includeSwaggerDocUri"},{"ExceptionType":"FileNotFoundException","Message":"Could not load file or assembly 'file:///E:\\agent_work\\463\\s\\Newtonsoft.Json.dll' or one of its dependencies. The system cannot find the file specified."},{"ExceptionType":"FileNotFoundException","Message":"Could not load file or assembly 'file:///E:\\agent_work\\463\\s\\Newtonsoft.Json.dll' or one of its dependencies. The system cannot find the file specified."}]
POST /api/metadata [{"ExceptionType":"MissingInAttributeException","Message":"In attribute is missing from parameter(s) api, includeRawSwaggerJson"},{"ExceptionType":"FileNotFoundException","Message":"Could not load file or assembly 'file:///E:\\agent_work\\463\\s\\Newtonsoft.Json.dll' or one of its dependencies. The system cannot find the file specified."}]
GET /api/operations [{"ExceptionType":"FileNotFoundException","Message":"Could not load file or assembly 'file:///E:\\agent_work\\463\\s\\Newtonsoft.Json.dll' or one of its dependencies. The system cannot find the file specified."}]

Similar to the content at https://github.com/Microsoft/OpenAPI.NET. The content would serve to let users know what this library is about, how to get started, etc.

These two parameters are taken as input, but not used within the method guts. The resulting document from my testing is always 3.0 and JSON.

https://github.com/Microsoft/OpenAPI.NET.CSharpComment/blob/f47916d3dfbeffc3022ce94349b4f7d2030fd823/src/Microsoft.OpenApi.CSharpComment.Reader/InternalOpenApiDocumentGenerator.cs#L253-L259

While tyring trying to fetch schema for request/response, generation fails with BadImageFormatException and that operation is included in the generated json which is not correct.

Take for example the following code and comments.

public enum DayOfWeek
{
    Monday,
    Tuesday,
    ...
    Sunday
}

public class FooController : Controller
{
    public const string DefaultValue = "Default";
    public const int DefaultValue2 = 10;

    /// <param name="stringCriteria" required="false" cref="string" in="query">
    /// Optional string criteria of resource to query. Default is <see cref="DefaultValue"/>.
    /// </param>
    /// <param name="dayOfWeek" required="false" cref="DayOfWeek" in="query">
    /// Optional day of week of resource to query. Default is <see cref="DayOfWeek.Tuesday"/>.
    /// </param>
    /// <param name="numberCriteria" required="false" cref="int" in="query">
    /// Optional number criteria of resource to query. Default is <see cref="DefaultValue2"/>.
    /// </param>
    public void Get(
        string stringCriteria = DefaultValue,
        DayOfWeek dayOfWeek = DayOfWeek.Tuesday,
        int numberCriteria = DefaultValue2 )
    {
        ...
    }
}

I would love to see the descriptions in the resulting OpenAPI document to be:

Optional string criteria of resource to query. Default is 'Default'.
Optional day of week of resource to query. Default is 'Tuesday'.
Optional number criteria of resource to query. Default is '10'.

Today, the descriptions are filled with the literal cref text, so for the examples above, they would be something like:

Optional string criteria of resource to query. Default is 'DefaultValue'.
Optional day of week of resource to query. Default is 'DayOfWeek.Tuesday'.
Optional number criteria of resource to query. Default is 'DefaultValue2'.

Naturally, we cannot do this convenience for complex referenced types, but for simple types like primitives and enums, we should be able to. It would keep documentation in sync with the code values, which is a plus for readers of the OpenAPI documents since they don't have to refer back to code.

Now that we tried out AppVeyor, we need the following:

• build failure should block check-in.
• after build, artifact should be available.
• after build, nuget package should be created.

optional at this point

• after build, nuget package is uploaded automatically to some private feed.

Currently for a param tag with in = "body", the cref attribute is not recognized. https://github.com/Microsoft/OpenAPI.NET.CSharpComment/blob/master/test/Microsoft.OpenApi.CSharpComment.Reader.Tests.SampleApis/Controllers/SampleControllerV1.cs

Let's move cref tag inside the param tag (instead of having it in a separate see tag>

For example,
https://github.com/Microsoft/OpenAPI.NET.CSharpComment/blob/master/src/Microsoft.OpenApi.CSharpComment.Reader/OpenApiDocumentGeneratorConfig.cs

There may be others, so the whole library should be scrubbed before release.

Some services/applications set global serialization settings instead of specifying attributes on every contract class property. For applications that do this, their properties in the resulting OpenAPI Document model may have incorrect casing. One such example is our FD application.

OpenAPI.Net is updated to not throw validation error if a required field is empty, so fix generator accordingly.

Make C# Comment Reader actually implement the IOpenApiReader.

We need to wrap the different inputs into an object and use the interface IOpenApiReader<TInput, TDiagnostic>

SerializedOverallGenerationResult can go into TDiagnostic for now. The construct is a bit awkward since IOpenApiReader assumes that there's one document returned. We will have to fix that as well.

https://github.com/OAI/OpenAPI-Specification/blob/master/versions/3.0.0.md#fixed-fields-10

We need to add "required" : true to every parameter that has "in": "path"

Custom contract classes can be in separate dll than the api's, so the property description for contract will be in separate xml than api documentation. C# comment reader should be updated to allow contract annotation xml and processing it to fetch property description.

Issue discovered by @scott-lin. Thanks!

When the library is invoked from a PowerShell context, the AppDomain's "AppBase" path is set to the location of PowerShell.exe on the machine. This causes an issue when calling Generate document method, given that a new app domain is spun off.

The caller (PowerShell in this case) will try to load the Reader dll again in the new app domain, but it will be looking in the wrong folder (i.e. the AppBase aforementioned) instead of the folder where the DLLs actually are.

The PowerShell script itself can workaround this issue by controlling the AppBase manually, but we can also allow the user to specify the path specifically in AppDomainCreator. This can ensure the caller looks at the right place for the reader Dll.

Edit by Scott-Lin: fixing a few comments to make the issue accurate

We have a model in our FD code which is incorrect in the resulting document because there's no way to hide specific properties.

For example, the following model is incorrect compared to the behavior of our actual code; StatusCode and Headers are not serialized, but this model indicates they are.

{
  "Description": "string",
  "ErrorCode": "string",
  "Details": [
    "string"
  ],
  "Headers": {
    "additionalProp1": "string",
    "additionalProp2": "string",
    "additionalProp3": "string"
  },
  "Source": "string",
  "StatusCode": "Continue"
}