kiota icon indicating copy to clipboard operation
kiota copied to clipboard

Endpoint returning `ActionResult<Guid>` breaks generated code

Open sam-eah opened this issue 1 year ago • 4 comments

Hello, I've noticed that returning ActionResult<Guid> from dotnet method endpoints seem to break generation.

The generated files (3 files in particular) throw the same error: Cannot find name 'createGuidFromDiscriminatorValue'.

For example output\subscription\createTrial\item\index.ts

/* tslint:disable */
/* eslint-disable */
// Generated by Microsoft Kiota
// @ts-ignore
import { createProblemDetailsFromDiscriminatorValue, serializeMspSubscriptionCmd, type MspSubscriptionCmd, type ProblemDetails } from '../../../models/';
// @ts-ignore
import { type BaseRequestBuilder, type Parsable, type ParsableFactory, type RequestConfiguration, type RequestInformation, type RequestsMetadata } from '@microsoft/kiota-abstractions';
// @ts-ignore
import { type Guid } from 'guid-typescript';

/**
 * Builds and executes requests for operations under /Subscription/CreateTrial/{mspSubscriptionTechnicalId}
 */
export interface WithMspSubscriptionTechnicalItemRequestBuilder extends BaseRequestBuilder<WithMspSubscriptionTechnicalItemRequestBuilder> {
    /**
     * @param body The request body
     * @param requestConfiguration Configuration for the request such as headers, query parameters, and middleware options.
     * @returns {Promise<Guid>}
     * @throws {ProblemDetails} error when the service returns a 400 status code
     * @throws {ProblemDetails} error when the service returns a 404 status code
     */
     post(body: MspSubscriptionCmd, requestConfiguration?: RequestConfiguration<object> | undefined) : Promise<Guid | undefined>;
    /**
     * @param body The request body
     * @param requestConfiguration Configuration for the request such as headers, query parameters, and middleware options.
     * @returns {RequestInformation}
     */
     toPostRequestInformation(body: MspSubscriptionCmd, requestConfiguration?: RequestConfiguration<object> | undefined) : RequestInformation;
}
/**
 * Uri template for the request builder.
 */
export const WithMspSubscriptionTechnicalItemRequestBuilderUriTemplate = "{+baseurl}/Subscription/CreateTrial/{mspSubscriptionTechnicalId}";
/**
 * Metadata for all the requests in the request builder.
 */
export const WithMspSubscriptionTechnicalItemRequestBuilderRequestsMetadata: RequestsMetadata = {
    post: {
        uriTemplate: WithMspSubscriptionTechnicalItemRequestBuilderUriTemplate,
        responseBodyContentType: "text/plain;q=0.9",
        errorMappings: {
            400: createProblemDetailsFromDiscriminatorValue as ParsableFactory<Parsable>,
            404: createProblemDetailsFromDiscriminatorValue as ParsableFactory<Parsable>,
        },
        adapterMethodName: "send",
        responseBodyFactory:  createGuidFromDiscriminatorValue,
        requestBodyContentType: "application/json-patch+json",
        requestBodySerializer: serializeMspSubscriptionCmd,
        requestInformationContentSetMethod: "setContentFromParsable",
    },
};
/* tslint:enable */
/* eslint-enable */

We can indeed see that createGuidFromDiscriminatorValue was never declared nor imported (I could not find any export named as such anyway).

    "@microsoft/kiota-abstractions": "^1.0.0-preview.50",
    "@microsoft/kiota-http-fetchlibrary": "^1.0.0-preview.49",
    "@microsoft/kiota-serialization-form": "^1.0.0-preview.39",
    "@microsoft/kiota-serialization-json": "^1.0.0-preview.50",
    "@microsoft/kiota-serialization-multipart": "^1.0.0-preview.28",
    "@microsoft/kiota-serialization-text": "^1.0.0-preview.47",

After replacing my endoints with ActionResult<GuidDTO>, using the following class, the ts files are correctly generated!

using System;

namespace Watsoft.Backend.API.Controllers
{
    public class GuidDTO
    {
        public Guid Guid { get; set; }
    }
}

sam-eah avatar Apr 12 '24 10:04 sam-eah

Hi @sam-eah Thanks for using kiota and for reporting this. This is caused because the following method needs to be special cased for Guid. https://github.com/microsoft/kiota/blob/cc234643a9fdb1b01546eb27db7e3d0a401573f8/src/Kiota.Builder/Writers/TypeScript/CodeConstantWriter.cs#L184 To use Guid.parse instead.

Then this method (its call) also needs to be investigate since it should have been considered a primitive type and used the "sendPrimitive" method instead.

https://github.com/microsoft/kiota/blob/cc234643a9fdb1b01546eb27db7e3d0a401573f8/src/Kiota.Builder/Writers/TypeScript/CodeConstantWriter.cs#L212

Is this something you'd be willing to submit a pull request for?

baywet avatar Apr 17 '24 15:04 baywet

I could take a look at it, but honestly, I just think guid-typescript doesn't add much value. It seems to be unmaintained and I still encounter some issues opened 5 years ago https://github.com/snico-dev/guid-typescript/issues/9 (toString method returning [object Object] - most likely because isGuid returning false - for a valid Guid). Those issues completely break most of my code using Kiota with Guid route params, Guid query params, or Guid payload. Not using the string primitive seems like pure overhead, but I could be wrong. What would be the arguments in favor of Guid?

sam-eah avatar Apr 18 '24 16:04 sam-eah

Thanks for the perspective. Arguments for it would be:

  1. type validation at "build" time
  2. developer guidance (you known this is a guid and not just any random string)
  3. alignment with other languages

I think we'd be open to using an alternative implementation if you can suggest a better one. Or even wrapping it under our own type so we can swap implementations transparently if needs be like we've done in other languages for time/date only, duration, etc...

baywet avatar Apr 19 '24 12:04 baywet

Thanks for the explanation. Those points make sense, though I'm not sure I need type validation for a string, moreover when given type validation is partially "broken" (or it seems). I've opened this PR: https://github.com/microsoft/kiota-typescript/pull/1154

sam-eah avatar Apr 20 '24 18:04 sam-eah