Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add the possibility to generate all the Apizr goodness #428

Merged
merged 27 commits into from
Jul 25, 2024

Conversation

JeremyBP
Copy link
Contributor

As discussed in #404, here is the PR giving the aibility to generate all the Apizr goodness with Refitter.

First of all, I introduced an ApizrSettings into the .refitter definition so that we could say if we want to generate it if it's set, or not if it's left to null.

From here, only if we get some ApizrSettings set:

  • On the existing Refit api interface side:
    • It includes a new method parameter: [RequestOptions] IApizrRequestOptions options if asked to
    • It sets no default values but uses method overloads because Apizr relies on expression trees which doesn't support default values. That was my main motivation of my Dynamic Querystring Parameters #417 suggestion actualy :), because it would give the possibility to get a single overload in such scenario.
  • On the new Apizr manager side (if asked to):
    • It builds:
      • Either an extension method to IServiceCollection if DependencyInjectionSettings is set
      • Otherwise a static builder if it's not
    • It gives some informations to the user right in generated top comments:
      • In case of configuration errors
      • If some Nuget packages are required based on user configuration
      • If some more manual actions may be needed

All 400+ tests passed, including the ones added to cover Apizr things.

So now if we provide the following configuration (taken from an included Test):

private readonly RefitGeneratorSettings _extendedSettings = new()
{
    DependencyInjectionSettings = new DependencyInjectionSettings
    {
        BaseUrl = "https://petstore3.swagger.io/api/v3",
        HttpMessageHandlers =
        [
            "AuthorizationMessageHandler",
            "DiagnosticMessageHandler"
        ],
        TransientErrorHandler = TransientErrorHandler.HttpResilience
    },
    ApizrSettings = new ApizrSettings
    {
        WithRequestOptions = true,
        WithRegistrationHelper = true,
        WithCacheProvider = CacheProviderType.InMemory,
        WithPriority = true,
        WithMediation = true,
        WithOptionalMediation = true,
        WithMappingProvider = MappingProviderType.AutoMapper,
        WithFileTransfer = true
    }
};

And ask Refitter to generate from it:

string code = ApizrRegistrationGenerator.Generate(
    _extendedSettings,
    [
        "IPetApi",
        "IStoreApi"
    ],
    "PetStore");

It will generate the following registration helper:

#nullable enable
namespace GeneratedCode
{
    // Please make sure to complete the following steps resulting from your configuration:
    // - Install-Package Apizr.Integrations.FileTransfer.Optional, then register MediatR
    // - Install-Package Apizr.Extensions.Microsoft.Caching, then register your caching provider
    // - Install-Package Apizr.Integrations.AutoMapper, then register AutoMapper
    // - Install-Package Apizr.Integrations.Fusillade
    // - Add your file transfer manager while calling ConfigurePetStoreApizrManagers method thanks to its options builder parameter

    using System;
    using Microsoft.Extensions.DependencyInjection;
    using Microsoft.Extensions.Http.Resilience;
    using AutoMapper;
    using MediatR;
    using Apizr;
    using Apizr.Configuring;
    using Apizr.Extending.Configuring.Common;

  
    public static partial class IServiceCollectionExtensions
    {
        /// <summary>
        /// Register all your Apizr managed apis with common shared options.
        /// You may call WithConfiguration option to adjust settings to your need.
        /// </summary>
        /// <param name="optionsBuilder">Adjust common shared options</param>
        /// <returns></returns>
        public static IServiceCollection ConfigurePetStoreApizrManagers(
            this IServiceCollection services,
            Action<IApizrExtendedCommonOptionsBuilder>? optionsBuilder = null)
        {
            optionsBuilder ??= _ => { }; // Default empty options if null
            optionsBuilder += options => options
                .WithBaseAddress("https://petstore3.swagger.io/api/v3", ApizrDuplicateStrategy.Ignore)
                .WithDelegatingHandler<AuthorizationMessageHandler>()
                .WithDelegatingHandler<DiagnosticMessageHandler>()
                .ConfigureHttpClientBuilder(builder => builder
                    .AddStandardResilienceHandler(config =>
                    {
                        config.Retry = new HttpRetryStrategyOptions
                        {
                            UseJitter = true,
                            MaxRetryAttempts = 6,
                            Delay = TimeSpan.FromSeconds(1)
                        };
                    }))
                .WithInMemoryCacheHandler()
                .WithAutoMapperMappingHandler()
                .WithPriority()
                .WithOptionalMediation()
                .WithFileTransferOptionalMediation();
            
            return services.AddApizr(
                registry => registry
                  .AddManagerFor<IPetApi>()
                  .AddManagerFor<IStoreApi>(),
                optionsBuilder);

        }
    }
}

I still have to publish an Apizr v6.0.0-preview.7 to Nuget to be fully compatible with such new Refitter capabilities.
I'll do it tomorrow morning.

I did not update your Changelog neither your Readme as I thought it was more about what you want to say and how :)

Let me know.
Jérémy

@JeremyBP JeremyBP changed the title Apizr Add the possibility to generate all the Apizr goodness Jul 24, 2024
@christianhelle christianhelle self-assigned this Jul 24, 2024
@christianhelle christianhelle added the enhancement New feature, bug fix, or request label Jul 24, 2024
@christianhelle
Copy link
Owner

Wow! This is some great stuff @JeremyBP !

I would like to take my time to review this thoroughly as this is a bit of a large pull request and I'm not supposed to be in front of the computer during my summer vacation 🌞🍹

Copy link

codecov bot commented Jul 24, 2024

Codecov Report

Attention: Patch coverage is 89.29001% with 89 lines in your changes missing coverage. Please review.

Project coverage is 95.45%. Comparing base (8fac5a9) to head (a4d3a0a).
Report is 11 commits behind head on main.

Files Patch % Lines
src/Refitter.Core/ApizrRegistrationGenerator.cs 80.48% 74 Missing and 14 partials ⚠️
src/Refitter.Core/EnumExtensions.cs 92.30% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #428      +/-   ##
==========================================
- Coverage   97.46%   95.45%   -2.01%     
==========================================
  Files          64       70       +6     
  Lines        2564     3389     +825     
==========================================
+ Hits         2499     3235     +736     
- Misses         40      114      +74     
- Partials       25       40      +15     
Flag Coverage Δ
unittests 95.45% <89.29%> (-2.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@christianhelle
Copy link
Owner

I did not update your Changelog neither your Readme as I thought it was more about what you want to say and how :)

The changelog is machine-generated based on issues and pull requests closed between tags. We can update the README another time. Several documentation files are almost copies of each other, whereas some are a bit trimmed down. Refitter also has a DocFX-generated documentation site hosted at https://refitter.github.io

@christianhelle
Copy link
Owner

@JeremyBP The build seems to fail

Build FAILED.

[/home/runner/work/refitter/refitter/src/Refitter.SourceGenerator/Refitter.SourceGenerator.csproj]
/home/runner/work/refitter/refitter/src/Refitter/GenerateCommand.cs(56,13): error CS0117: 'RefitGeneratorSettings' does not contain a definition for 'UseApizr' [/home/runner/work/refitter/refitter/src/Refitter/Refitter.csproj]
    1 Warning(s)
    1 Error(s)

Looks like RefitGeneratorSettings is missing a property called UseApizr. A bad rebase/merge maybe?

@JeremyBP
Copy link
Contributor Author

Oops! it seems I forgot to change the former UseApizr to the new ApizrSettings.
Will do it tomorrow morning when I'm back to the office.
Feel free to take your time and ask me anything to understand or even to change code in my PR.
I know this is quite a big fish 😅
Regards
Jeremy

@JeremyBP
Copy link
Contributor Author

JeremyBP commented Jul 25, 2024

Ok so now the solution builds as expected and I adjusted some parts of code to improve sonar quality score.
The last 2 issues are about the cognitive complexity of both your Refit interface clients and my Apizr registration generators, but I'm not sure that splitting things down here would improve anything IMHO.
You tell me.

@christianhelle
Copy link
Owner

Ok so now the solution builds as expected and I adjusted some parts of code to improve sonar quality score. The last 2 issues are about the cognitive complexity of both your Refit interface clients and my Apizr registration generators, but I'm not sure that splitting things down here would improve anything IMHO. You tell me.

You don't need to worry about improving on cognitive complexity for now. There is really not much to gain from that. I tend to keep these SonarCloud improvements in separate pull requests

@christianhelle
Copy link
Owner

I still have to publish an Apizr v6.0.0-preview.7 to Nuget to be fully compatible with such new Refitter capabilities.
I'll do it tomorrow morning.

I tried out your branch locally with the following .refitter file

{
  "openApiPath": "https://petstore3.swagger.io/api/v3/openapi.json",
  "namespace": "Petstore.Apizr",
  "outputFolder": "GeneratedCode",
  "outputFilename": "SwaggerPetstoreApizr.cs",
  "naming": {    
    "useOpenApiTitle": false,
    "interfaceName": "SwaggerPetstoreApizr"
  },
  "dependencyInjectionSettings": {
    "baseUrl": "https://petstore3.swagger.io/api/v3",
    "transientErrorHandler": "HttpResilience",
    "maxRetryCount": 3,
    "firstBackoffRetryInSeconds": 0.5
  },
  "apizrSettings": {
    "withRequestOptions": true,
    "withRegistrationHelper": true,
    "withCacheProvider": "InMemory",
    "withPriority": true,
    "withMediation": true,
    "withOptionalMediation": true,
    "withMappingProvider": "AutoMapper",
    "withFileTransfer": true
  }
}

and a .csproj file that looks like this:

<Project Sdk="Microsoft.NET.Sdk">

  <PropertyGroup>
    <OutputType>Exe</OutputType>
    <TargetFramework>net8.0</TargetFramework>
    <ImplicitUsings>enable</ImplicitUsings>
    <Nullable>enable</Nullable>
  </PropertyGroup>

  <ItemGroup>
    <PackageReference Include="Apizr.Extensions.Microsoft.Caching" Version="6.0.0-preview.6" />
    <PackageReference Include="Apizr.Integrations.AutoMapper" Version="6.0.0-preview.6" />
    <PackageReference Include="Apizr.Integrations.FileTransfer.Optional" Version="6.0.0-preview.6" />
    <PackageReference Include="Apizr.Integrations.Fusillade" Version="6.0.0-preview.6" />
  </ItemGroup>

</Project>

and as expected the build fails due to

Apizr.csproj : error NU1108: Cycle detected. 
Apizr.csproj : error NU1108:   Apizr -> Apizr.Integrations.AutoMapper 6.0.0-preview.6 -> Apizr (>= 6.0.0-preview.6).

Copy link
Owner

@christianhelle christianhelle left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Everything looks good, all-in-all. I only found one thing that I think needs changing, and that's suggesting dotnet add package instead of Install-Package for non-Windows users.

I'd like to wait with merging this in until it can produce code that can build, and I guess that depends on an upcoming Apizr release you mentioned in the description

src/Refitter.Core/Settings/ApizrPackages.cs Outdated Show resolved Hide resolved
Copy link

@JeremyBP
Copy link
Contributor Author

Hey, I just pushed Apizr-v6.0.0-preview.7 and pushed a little commit to Refitter too.

  1. From here, if I change ProjectFileContents.cs to:
public static class ProjectFileContents
{
    public const string Net70App = @"
<Project Sdk=""Microsoft.NET.Sdk"">
  <PropertyGroup>
    <TargetFramework>net70</TargetFramework>
  </PropertyGroup>
  <ItemGroup>
    <PackageReference Include=""Newtonsoft.Json"" Version=""13.0.1"" />
    <PackageReference Include=""System.ComponentModel.Annotations"" Version=""4.5.0"" />
    <PackageReference Include=""System.Runtime.Serialization.Primitives"" Version=""4.3.0"" />
    <PackageReference Include=""Microsoft.Extensions.DependencyInjection"" Version=""8.0.0"" />
    <PackageReference Include=""Microsoft.Extensions.Http.Polly"" Version=""8.0.4"" />
    <PackageReference Include=""Microsoft.Extensions.Options.ConfigurationExtensions"" Version=""8.0.0"" />
    <PackageReference Include=""Microsoft.Extensions.Http.Resilience"" Version=""8.0.0"" />
    <PackageReference Include=""Polly.Contrib.WaitAndRetry"" Version=""1.1.1"" />
    <PackageReference Include=""Polly.Extensions.Http"" Version=""3.0.0"" />
    <PackageReference Include=""System.Reactive"" Version=""6.0.0"" />
    <PackageReference Include=""Apizr.Extensions.Microsoft.Caching"" Version=""6.0.0-preview.7"" />
    <PackageReference Include=""Apizr.Integrations.AutoMapper"" Version=""6.0.0-preview.7"" />
    <PackageReference Include=""Apizr.Integrations.FileTransfer.Optional"" Version=""6.0.0-preview.7"" />
    <PackageReference Include=""Apizr.Integrations.Fusillade"" Version=""6.0.0-preview.7"" />
  </ItemGroup>
</Project>";
}

And ApizrTests.cs to:

    private static async Task<string> GenerateCode()
    {
        var swaggerFile = await CreateSwaggerFile(OpenApiSpec);
        var settings = new RefitGeneratorSettings
        {
            OpenApiPath = swaggerFile,
            UseCancellationTokens = true,
            OptionalParameters = true,
            Naming = new NamingSettings
            {
                UseOpenApiTitle = false,
                InterfaceName = "JobDetails"
            },
            DependencyInjectionSettings = new DependencyInjectionSettings
            {
                BaseUrl = "https://petstore3.swagger.io/api/v3",
                TransientErrorHandler = TransientErrorHandler.HttpResilience
            },
            ApizrSettings = new ApizrSettings
            {
                WithRequestOptions = true,
                WithRegistrationHelper = true,
                WithCacheProvider = CacheProviderType.InMemory,
                WithPriority = true,
                WithMediation = true,
                WithOptionalMediation = true,
                WithMappingProvider = MappingProviderType.AutoMapper,
                WithFileTransfer = true
            }
        };

        var sut = await RefitGenerator.CreateAsync(settings);
        var generateCode = sut.Generate();
        return generateCode;
    }

Then, calling the following test create the right code and builds the project like a charm:

    [Fact]
    public async Task Can_Build_Generated_Code()
    {
        string generateCode = await GenerateCode();
        BuildHelper
            .BuildCSharp(generateCode)
            .Should()
            .BeTrue();
    }
  1. Also, if I run refitter --settings-file ./Test.refitter from the project bin folder with Test.refitter in the same folder and looking like:
{
  "openApiPath": "https://petstore3.swagger.io/api/v3/openapi.json",
  "namespace": "Petstore",
  "outputFolder": "Test",
  "outputFilename": "Generated.cs",
  "naming": {    
    "useOpenApiTitle": false,
    "interfaceName": "PetstoreApi"
  },
  "dependencyInjectionSettings": {
    "baseUrl": "https://petstore3.swagger.io/api/v3",
    "transientErrorHandler": "HttpResilience",
    "maxRetryCount": 3,
    "firstBackoffRetryInSeconds": 0.5
  },
  "apizrSettings": {
    "withRequestOptions": true,
    "withRegistrationHelper": true,
    "withCacheProvider": "InMemory",
    "withPriority": true,
    "withMediation": true,
    "withOptionalMediation": true,
    "withMappingProvider": "AutoMapper",
    "withFileTransfer": true
  }
}

And a Project.csproj in a Test sub folder looking like:

<Project Sdk="Microsoft.NET.Sdk">
  <PropertyGroup>
    <TargetFramework>net70</TargetFramework>
  </PropertyGroup>
  <ItemGroup>
    <PackageReference Include="Newtonsoft.Json" Version="13.0.1" />
    <PackageReference Include="System.ComponentModel.Annotations" Version="4.5.0" />
    <PackageReference Include="System.Runtime.Serialization.Primitives" Version="4.3.0" />
    <PackageReference Include="Microsoft.Extensions.DependencyInjection" Version="8.0.0" />
    <PackageReference Include="Microsoft.Extensions.Http.Polly" Version="8.0.4" />
    <PackageReference Include="Microsoft.Extensions.Options.ConfigurationExtensions" Version="8.0.0" />
    <PackageReference Include="Microsoft.Extensions.Http.Resilience" Version="8.0.0" />
    <PackageReference Include="Polly.Contrib.WaitAndRetry" Version="1.1.1" />
    <PackageReference Include="Polly.Extensions.Http" Version="3.0.0" />
    <PackageReference Include="System.Reactive" Version="6.0.0" />
    <PackageReference Include="Apizr.Extensions.Microsoft.Caching" Version="6.0.0-preview.7" />
    <PackageReference Include="Apizr.Integrations.AutoMapper" Version="6.0.0-preview.7" />
    <PackageReference Include="Apizr.Integrations.FileTransfer.Optional" Version="6.0.0-preview.7" />
    <PackageReference Include="Apizr.Integrations.Fusillade" Version="6.0.0-preview.7" />
  </ItemGroup>
</Project>

It builds without any issue.

Actually, I wonder if your issue could be due to your ".Apizr" into your namespace and project name...
Let me know if there're still issues and maybe share a repro sample to let me investigate.
I mean your in vacation after all 🌞

@christianhelle
Copy link
Owner

christianhelle commented Jul 25, 2024

Actually, I wonder if your issue could be due to your ".Apizr" into your namespace and project name...
Let me know if there're still issues and maybe share a repro sample to let me investigate.
I mean your in vacation after all 🌞

You were right! I never knew that you couldn't have your .csproj file be the same as a package reference, in my case my project was called Apizr.csproj. I renamed it to Sample.csproj and that did the trick

@christianhelle christianhelle merged commit e7c1b4b into christianhelle:main Jul 25, 2024
421 of 423 checks passed
@christianhelle
Copy link
Owner

Thanks for this amazing contribution @JeremyBP !!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature, bug fix, or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants