Add option to generate public/internal setters for generated models for Testing scenarios

[NikiforovAll]

Hi,

I would like to be able to stub a client generated by Kiota. But, currently, the models has private setters, like this:

I use OpenAPI spec: kiota search microsoft.com:cognitiveservices-NewsSearch

namespace NewsSearch.Sdk.Models {
    public class NewsTopic : Thing, IParsable {
        /// <summary>A Boolean value that indicates whether the topic is considered breaking news. If the topic is considered breaking news, the value is true.</summary>
        public bool? IsBreakingNews { get; private set; }
        /// <summary>The URL to the Bing News search results for the search query term</summary>
#if NETSTANDARD2_1_OR_GREATER || NETCOREAPP3_1_OR_GREATER
#nullable enable
        public string? NewsSearchUrl { get; private set; }
#nullable restore
#else
        public string NewsSearchUrl { get; private set; }
#endif
        /// <summary>Defines a search query.</summary>
#if NETSTANDARD2_1_OR_GREATER || NETCOREAPP3_1_OR_GREATER
#nullable enable
        public NewsSearch.Sdk.Models.Query? Query { get; set; }
#nullable restore
#else
        public NewsSearch.Sdk.Models.Query Query { get; set; }
#endif 

      // omitted
    }
}

I would like to suggest adding a configuration that allows control aspects of model generation.

Thank you, I appreciate your work.

[baywet]

Hi @NikiforovAll Thanks for using kiota, and for the great blog post! The private access modifier is added because the property is marked as readonly in the description. We generally do not recommend mocking the fluent API and models themselves, but the underlying HTTP request/client. Is this something you've explored for this scenario?

[NikiforovAll]

Thank you @baywet ,

As shown in the exemplary test, I want to mock IRequestAdapter so I need to create an instance of a class. Unfortunately, NewsTopic doesn't have required public setters so I can setup various test data.

  [Fact]
  public async Task TrendingTopic_GetUS_SuccessAsync()
  {
      // Arrange
      var adapter = Substitute.For<IRequestAdapter>();
      adapter.SetupSendAsyncWithResponse(new TrendingTopics() { Value = [] });

      var newsSearchApiClient = new NewsSearchApiClient(adapter);

      // Act
      var response = await newsSearchApiClient
          .News
          .Trendingtopics
          .GetAsync(r => r.QueryParameters.Cc = "US");

      // Assert
      Assert.NotNull(response);
  }

https://github.com/NikiforovAll/kiota-getting-started/blob/5b2d95a32aecde630822f8a27a32b02de02a3856/tests/NewsSearch.Sdk.Tests/TrendingTopic.Tests.cs#L9

I'm not sure if I understand how to properly mock Models in this case.

[marcinjahn]

I'd opt for removing any special treatment of readonly/writeonly properties from Open API spec, and just making all these fields public. Ideally, the models would be records so we could use init and with, but I do understand that's not so straightforward. For the time being, public seems like a fine alternative. I don't see any benefit in restricting those readonly properties.

In my case, I wanted to test a service that maps API responses (models generated with Kiota) to domain models. I need to be able to mock the Kiota models. It works with Autofixture, I suppose it uses reflection to achieve that, but in some cases it'd be nice to craft some mock manually.

[baywet]

Thank you both for the additional context here. @NikiforovAll my comment was more about mocking the underlying http response on the http client (used by the request adapter, used in turns by the client). Generally speaking, the client is "infrastructure code" (c.f. DDD), we recommend wrapping into a service, so it can be mocked, and that service returns object domain models. The idea of "hiding the setters" for readonly fields was to avoid inducing the client developer in error into thinking a specific field was settable on the API when in fact it's not. Think of it as a developer experience improvement. If we choose to "remove this feature", which wouldn't be breaking, I'd like to get @sebastienlevert (our PM) input first.

[NikiforovAll]

Yes, I understand the general recommendation of wrapping code and it is something that I would generally do, but I think it is still valuable to be able to mock IRequestAdapter.

I guess another option would be using internal setters instead of private since we can make use of InternalsVisibleTo modification.

[baywet]

But such a change would have the same effect as switching from private to public in some scenarios where the client is in the same project as the consuming code from a developer experience perspective. Let's see what Seb has to say.