microsoft / kiota Goto Github PK

Use OpenAPI descriptions to create comments so they display as intellisense.
AB#8500

When writing client.Users["jane"].MessageFolders["inbox"].Messages["id"] it needs to translate under the covers to /users/jane/mailFolders/inbox/messages/id

We currently have implemented one case of inheritance definition which is when all types in the inheritance tree are defined in the all of collection.
In the OpenAPI semantics, there's another way of defining inheritance where the child type will be in the schema and allOf collection will contain the base type.
AB#8905

our language writers are getting bigger and bigger, which makes things hard to maintain, scope, and test. We should break them in a visitor pattern. Each visitor would implement the "write" method of IWriter<T> where T : CodeElement. Visitors would be registered via a method somewhere for each language.
AB#8904

In languages like typescript or java we need additional properties:

• import symbol (that'd be the class name in the case of TypeScript/java)
• external import or not (to understand whether it's something we've generated or not)
• some kind of namespace prefixing information for local imports

The client root class needs a better configuration experience. We could either go the fluent way like current graph clients or leverage a auto registration approach (singleton/reflection that adds things automatically when importing the module/dependencies).

This configuration experience needs to account for:

• middleware addition/reorganisation (see #120)
• serialization/deserialization factories registration
• core engine registration

Note: we might want to rework the authentication handler as a middleware as part of this work.
AB#9082

This repository is lacking getting started information so people can try kiota and contribute. It should contain:

• the list of tools that are required
• the documentation for the different parameters
• links to sample OpenAPI descriptions
• links to the design documentation
  AB#8966

Use OpenAPI descriptions to create comments so they display as intellisense.

People need to be able to inject behavior on responses before things are returned to the caller to handle things like retry, redirect, etc...
Review the design guidance

We probably need to model:

• ResponseInfo (abstract reflection of http response)
• RequestMiddleware (interface for middlewares excuting before the request is sent)
• ResponseMiddleware (interface for middlewares executing after the response is received)
• A property bag to tag the request with options for the middleware
• A way to register (and order?) middlewares
• A way to set default options for requests
  AB#9081

I suggest that we generate an overload that uses type parameters with constraints instead of concrete types in our final request builder. This could serve the following purposes:

1. If the CSDL references a base type, the generated derived type serialization instructions can be used instead of the base type serialization instructions. No derivedtypeconverter
2. In the case that the generated library hasn't been updated and there is a new derived type, a customer can derive from the base type, add the missing properties, and add serialization instructions as a quick fix until the generated library can be updated.
3. Some customers like to use our models as the model for their applications. They GET the object, change the object, and then PATCH the object. This results in the entire object being sent in the PATCH, instead of a PATCH object containing only the updated properties. This results in serialized properties being sent that are read-only and ignored, or even properties that cause errors in the service when the service doesn't gracefully ignore unexpected properties. We provide guidance for this scenario where we have them derive from the base type, and they can potentially enhance their model with something like a PropertyChanged event to provide instruction to the Serialize(writer) method. We should consider how we would instruct the user to propagate PropertyChanged down to the base.Serialize method so that they could control serialization from their custom derived type into our generated base types without having special instructions in the SerializationWriter. OR, we just tell them to new up an object with just their changes.

The Serialize method in the model is interesting.

MessageRequestBuilder

public class MessagesRequestBuilder<T> where T : Message {
    public MessageRequestBuilder this[string position] { get {
        return new MessageRequestBuilder { HttpCore = HttpCore, CurrentPath = CurrentPath + PathSegment  + "/" + position};
    } }
    public async Task<MessagesResponse<T>> GetAsync(Action<GetQueryParameters> q = default, Action<IDictionary<string, string>> h = default, IResponseHandler responseHandler = default) {
        var requestInfo = new RequestInfo {
            HttpMethod = HttpMethod.GET,
            URI = new Uri(CurrentPath),
        };
        if (q != null) {
            var qParams = new GetQueryParameters();
            q.Invoke(qParams);
            qParams.AddQueryParameters(requestInfo.QueryParameters);
        }
        h?.Invoke(requestInfo.Headers);
        return await HttpCore.SendAsync<MessagesResponse<T>>(requestInfo, responseHandler);
    }
    public async Task<T> PostAsync(Action<IDictionary<string, string>> h = default, IResponseHandler responseHandler = default) {
        var requestInfo = new RequestInfo {
            HttpMethod = HttpMethod.POST,
            URI = new Uri(CurrentPath),
        };
        h?.Invoke(requestInfo.Headers);
        return await HttpCore.SendAsync<T>(requestInfo, responseHandler);
    }
    // ....
    }
}

MessageResponse

public class MessagesResponse<T> where T : Message {
    public List<T> Value { get; set; }
    public string NextLink { get; set; }
}

AB#8908

They should either be void or have a model
AB#8680

It'd be nice to have the generator packaged and published a a dotnet tool so developers from the dotnet ecosystem can easily onboard.
More reading
AB#9101

Use OpenAPI descriptions to create comments so they display as intellisense.
AB#8501

The OpenApi files I try playing with are either the common sample PetStore swagger file (ref: https://petstore.swagger.io/ ) or a default dotnet new webapi template with swagger enabled.

Does Kiota accept .json openapi files?

very briefly checking the code, it feels like it's yaml only?

EDIT: i know that the swagger editor can save the json as yml ... but i'm just curious about just json without having to do the manual conversion?

EDIT 2: here's the sample json file i was using: https://petstore.swagger.io/v2/swagger.json
Same error occured when I saved it as yml (via https://editor.swagger.io/ ) and tried that :/

AB#9047

Using this approach we can remove the need to generate hashes on request builder class names to avoid name clashes.

namespace MicrosoftGraph.Users
  UsersRequestBuilder
  
namespace microsoftgraph.Users.Messages
  MessagesRequestBuilder

namespace microsoftgraph.Users.Messages.Item
  MessageRequestBuilder

namespace microsoftgraph.users.Messages.Item.attachments
  AttachmentsRequestBuilder

namespace microsoftgraph.Users.MailFolder.Item.Messages.Item
  MessageRequestBuilder

Can we please have the docker image stored in the newer ghcr.io registry? means we don't need auth/PAT's.

Currently there are 2 GH Docker registries, one at docker.pkg.github.com and a new one at ghcr.io. The original registry always requires a PAT with the read:packages scope (even for public packages). The new ghcr.io registry has the option to create true public container packages.

If you publish an image to ghcr.io you can make it public with no auth required. See here for more information:
https://docs.github.com/en/packages/guides/configuring-access-control-and-visibility-for-container-images#configuring-visibility-of-container-images-for-an-organization
AB#9098

• implementation for dotnet auth provider + http core
• implementation for java auth provider + http core
• implementation for typescript auth provider + http core
• abstraction for authentication provider for dotnet
• abstraction for authentication provider for java
• abstraction for authentication provider for typescript
  AB#8498

They are currently generated as classes when they should be enums.
Look at the message importance
AB#8678

related #64

dotnet test is probably missing, and maybe some configuration to grab the report
AB#8852

The CD should:

• create a tag
• create a release
• build Kiota (release mode)
• upload binaries so people can download them

Currently we depend on OperationId but it is not a required field in OpenAPI, so if we can avoid being dependent on it, that would be good.

private string GetNamespaceNameForModelByOperationId(string operationId) {
            if(string.IsNullOrEmpty(operationId)) throw new ArgumentNullException(nameof(operationId));
            var cleanOperationId = operationId.Split('_').First();
            return $"{this.config.ClientNamespaceName}.{cleanOperationId}";
        }

AB#8847

To support dirty tracking and fluid data structures we need models to have the capability to use more advanced backing stores than simple backing fields.
AB#8886

The primitive types in dotnet (bool, float....) are currently being generated as non nullable when they should semantically be nullable
AB#8679

We need the tool to be available as a docker image
AB#9032

Today Kiota doesn't support adding descriptions for enum values. This can be really useful to guide the SDK user.
The spec work to "hold the information" is in progress json-schema-org/json-schema-vocabularies#47
AB#8917

AB#8884

search of List<Object
AB#8681

https://sonarcloud.io/project/configuration?id=microsoft_kiota
AB#8793

When the tool is integrated with dev workflows (build process & such), significant performance gains on the overall build process can be achieved if kiota NO-OPs. We could hash the yaml + parameters on entry, store this in a local hidden file and compare on next run.
AB#9033

Interface between request builders and core libraries.

AB#8499

This will allow us to generate implementations for navigation properties generated on RequestBuilders.
AB#8504

-o https://.....
AB#9031

there's still a bug in the CSHarp writer because of the recent change of not using namespaces as an entry point. The mapping needs to be updated like Java/Tyepescript, and the usings need to be tested

related #12

We'll need the generic request for batching/parallelization of calls. We need to generate additional methods to be able to get that abstract object without executing the call.
Split the request executors in two halves, the second one calling the first one like this

@javax.annotation.Nullable
    public RequestInfo createGetRequest(@javax.annotation.Nonnull final java.util.function.Consumer<GetQueryParameters> q, @javax.annotation.Nonnull final java.util.function.Consumer<Map<String, String>> h) throws URISyntaxException {
        Objects.requireNonNull(q);
        Objects.requireNonNull(h);
        final RequestInfo requestInfo = new RequestInfo() {{
            uri = new URI(currentPath);
            httpMethod = HttpMethod.GET;
        }};
        final GetQueryParameters qParams = new GetQueryParameters();
        q.accept(qParams);
        qParams.AddQueryParameters(requestInfo.queryParameters);
        h.accept(requestInfo.headers);
        return requestInfo;
    }
    @javax.annotation.Nullable
    public java.util.concurrent.CompletableFuture<MessagesResponse> get(@javax.annotation.Nonnull final java.util.function.Consumer<GetQueryParameters> q, @javax.annotation.Nonnull final java.util.function.Consumer<Map<String, String>> h, @javax.annotation.Nonnull final ResponseHandler responseHandler) {
        Objects.requireNonNull(q);
        Objects.requireNonNull(h);
        Objects.requireNonNull(responseHandler);
        try {
            final RequestInfo requestInfo = this.createGetRequest(q, h);
            return this.httpCore.sendAsync(requestInfo, responseHandler);
        } catch (URISyntaxException ex) {
            return java.util.concurrent.CompletableFuture.failedFuture(ex);
        }
    }

Additionally we might want to add a "addRequestToBatch(batch)" mehtod on the request info to enable fluent style batch building.
AB#8503

look for lastModifiedDateTime
The type is string but a pattern is present and the format is date-time, this should translate to a datetimeoffset in dotnet
AB#8919

This code fails

       protected void FixReferencesToEntityType(CodeElement currentElement, CodeClass entityClass = null){
            if(entityClass == null)
                entityClass = currentElement.GetImmediateParentOfType<CodeNamespace>()
                            .GetRootNamespace()
                            .GetChildElementOfType<CodeClass>(x => x.Name.Equals("entity", StringComparison.InvariantCultureIgnoreCase));

with this OpenAPI

openapi: 3.0.0
info:
  title: "Todo API"
  version: "1.0.0"
paths:
  /todos: 
    get:
      operationId: todos_ListTodos
      parameters:
          - name: active
            in: query
            schema:
              type: boolean
          - name: keyword
            in: query
            schema:
              type: string
      responses:
        '200':
          description: OK
          content:
            application/json:
              schema:
                title: collectionTodos
                type: object
                properties:
                  value:
                    items: 
                      $ref: "#/components/schemas/todo"

    post:
      operationId: todos_CreateTodo
      responses:
        '201':
          description: OK
  /todos/{todoId}:
    get:
      operationId: todos_GetTodo
      responses:
        '200':
          description: OK
          content:
            application/json:
              schema:
                $ref: "#/components/schemas/todo"
    delete:
      operationId: todos_DeleteTodo
      responses:
        '200':
          description: OK
  /tag/{tagId}:
    get:
      operationId: todos_GetTag
      responses:
        '200':
          description: OK
components:
  schemas:
    entity:
      type: object
      properties:
        id: 
          type: string
    tag:
      type: object
      properties:
        id: 
          type: string
        displayName: 
          type: string
    todo:
      allOf:
        - $ref: "#/components/schemas/entity"
        - type: object
          properties:
            subject:
              type: string

AB#8848

This https://github.com/microsoft/OpenAPI.NET/tree/vnext/src/Microsoft.OpenApi.Readers/V3 is how we built the deserializers for the OpenAPI.NET model.
And this is the generic code that reads the YAML/JSON and calls the generated mapping code https://github.com/microsoft/OpenAPI.NET/tree/vnext/src/Microsoft.OpenApi.Readers/ParseNodes
AB#8502

When deserializing, unrecognized properties should be stored in additionalProperties dictionary.

Some of these properties will also be written during serialization so that we can round-trip unknown values.
AB#8675

Like the serializers today (so we can handle yaml and other formats)
AB#8885

We should have a condition that checks whether the information is present and skips the PR all together for external PRs
AB#8953

There's still a bug in Java for the import for the GraphClient. It needs to do relative calculation like typescript does

related #12

https://github.com/microsoft/kiota/actions/runs/722328686
AB#8867

       protected void MoveClassesWithNamespaceNamesUnderNamespace(CodeElement currentElement) {
            if(currentElement is CodeClass currentClass && 
                currentClass.Parent is CodeNamespace parentNamespace) {
                var childNamespaceWithClassName = parentNamespace.InnerChildElements
                                                                .OfType<CodeNamespace>()
                                                                .FirstOrDefault(x => x.Name
                                                                                    .EndsWith(currentClass.Name, StringComparison.InvariantCultureIgnoreCase));
                if(childNamespaceWithClassName != null) {
                    parentNamespace.InnerChildElements.Remove(currentClass);
                    childNamespaceWithClassName.AddClass(currentClass);
                }
            }
            CrawlTree(currentElement, MoveClassesWithNamespaceNamesUnderNamespace);
        }

CrawlTree will fail with elements that have a null name.

openapi: 3.0.0
info:
  title: "Todo API"
  version: "1.0.0"
paths:
  /todos: 
    get:
      operationId: todos_ListTodos
      parameters:
          - name: active
            in: query
            schema:
              type: boolean
          - name: keyword
            in: query
            schema:
              type: string
      responses:
        '200':
          description: OK
          content:
            application/json:
              schema:
                title: collectionTodos
                type: object
                properties:
                  value:
                    items: 
                      $ref: "#/components/schemas/todo"

    post:
      operationId: todos_CreateTodo
      responses:
        '201':
          description: OK
  /todos/{todoId}:
    get:
      operationId: todos_GetTodo
      responses:
        '200':
          description: OK
          content:
            application/json:
              schema:
                $ref: "#/components/schemas/todo"
    delete:
      operationId: todos_DeleteTodo
      responses:
        '200':
          description: OK
  /tag/{tagId}:
    get:
      operationId: todos_GetTag
      responses:
        '200':
          description: OK
components:
  schemas:
    entity:
      type: object
      properties:
        id: 
          type: string
    tag:
      type: object
      properties:
        id: 
          type: string
        displayName: 
          type: string
    todo:
      allOf:
        - $ref: "#/components/schemas/entity"
        - type: object
          properties:
            subject:
              type: string

AB#8849

This will reduce the size of the SharedModels folder and keep the models close to the request builders that use them. We will need to have some OpenAPI annotation to correlate a model with the canonical URL.

Darrel and I had further brainstorming on this and here is a recap.
We have multiple concerns here:

• The launch.json configuration should be valid, the paths should resolve with minimal repos cloning/rework from people cloning this repo.
• The parameters should be documented so people can configure other IDEs, use dotnet run, use a compiled version of kiota... #96
• The unit tests coverage should be ramped up and test aspects, not just generate files #79
• The PRs should provide a generation diff (more on that below)
• We want to avoid overcrowding this repo with too many things

The solution we're suggesting is the following:

• create a samples repo under the Microsoft Org, this repo will contain multiple projects (one for each language)
• add this repo as a submodule to kiota's repository
• update launch.json to point to this submodule's path

Someone PRing kiota to change the generation result should:

1. run the generation on the sample repo
2. create a PR in the generation repo
3. update the submodule reference
4. create the PR in Kiota, linking to that generation PR

This has the advantage of:

• providing a generation diff for reviewers
• not polluting the current repo
• force us to start building proper unit tests
• being simple for anyone who knows about submodules
• not being blocking for anyone who doesn't know about submodule

Thoughts?

Originally posted by @baywet in #48 (comment)
AB#9017

AB#8868