SteveDunn / Vogen

A semi-opinionated library which is a source generator and a code analyser. It Source generates Value Objects
Apache License 2.0
887 stars 46 forks source link

Add compatibility with Refit #560

Closed arteny closed 7 months ago

arteny commented 9 months ago

Describe the feature

It is not compatible with Refit wide-using library. For such kind of request definition:

    [Get("/api/getMarket")]
    Task<IApiResponse> GetMarket(MarketId marketId);

it generates request /api/getMarket?Value=myValue when expected is /api/getMarket?marketId=myValue

Could you please to add compatibility with Refit(https://github.com/reactiveui/refit)?

SteveDunn commented 7 months ago

Hi @arteny - this should be fixed with v4.0. I created an example here: https://github.com/SteveDunn/Vogen/blob/main/samples/WebApplicationConsumer/RefitExample/RefitRunner.cs Please let me know if it works for you.

fbestfriends commented 7 months ago

@SteveDunn The provided example

 [Get("/weatherforecast/{city}")]
    Task<List<WeatherForecast>> GetWeatherForecastByCity(City city);

is not what I asked. This type was worked before also. What was not working it is:

 [Get("/weatherforecast")]
    Task<List<WeatherForecast>> GetWeatherForecastByCity(City city);

where request is /weatherforecast?city=London instead of /weatherforecast/London

Is this type also fixed?

SteveDunn commented 7 months ago

Thanks for clarifying. I'll check shortly. Hopefully it will be.

SteveDunn commented 7 months ago

Hi @fbestfriends - I've just confirmed that it works:

image

fbestfriends commented 6 months ago

Hi @fbestfriends - I've just confirmed that it works: it is minimal api I think, when I asked about Refit query parameter

SteveDunn commented 6 months ago

Hi @fbestfriends - I've just confirmed that it works: it is minimal api I think, when I asked about Refit query parameter

Are you still having issues? I've checked with both a minimal API and a controller.

arteny commented 3 months ago

Hi @fbestfriends - I've just confirmed that it works: it is minimal api I think, when I asked about Refit query parameter

Are you still having issues? I've checked with both a minimal API and a controller.

Yes, still have the issue for it. if I call

    [Post("/api/ChangeResult")]
    Task ChangeResult(MarketId marketId, FancyResult result);
        ...
    await _api.ChangeResult(MarketId.From("test"), FancyResult.From(1));

it produce following error on server side:

Result: Function 'ChangeResult', Invocation id '302b97c6-da8f-4424-a1b8-b6a1e2b03dc4': An exception was thrown by the invocation.
Exception: System.NullReferenceException: Object reference not set to an instance of an object.
   at lambda_method300(Closure , ChangeFancyResult , Object[] )
   at Microsoft.Azure.Functions.Worker.Invocation.MethodInvokerWithReturnValue`2.InvokeAsync(TReflected instance, Object[] arguments) in D:\a\_work\1\s\src\DotNetWorker.Core\Invocation\MethodInvokerWithReturnValue.cs:line 22
   at Microsoft.Azure.Functions.Worker.Invocation.DefaultFunctionInvoker`2.InvokeAsync(Object instance, Object[] arguments) in D:\a\_work\1\s\src\DotNetWorker.Core\Invocation\DefaultFunctionInvoker.cs:line 31
   at Microsoft.Azure.Functions.Worker.Invocation.DefaultFunctionExecutor.ExecuteAsync(FunctionContext context) in D:\a\_work\1\s\src\DotNetWorker.Core\Invocation\DefaultFunctionExecutor.cs:line 45
   at Microsoft.Azure.Functions.Worker.OutputBindings.OutputBindingsMiddleware.Invoke(FunctionContext context, FunctionExecutionDelegate next) in D:\a\_work\1\s\src\DotNetWorker.Core\OutputBindings\OutputBindingsMiddleware.cs:line 13
   at Microsoft.Azure.Functions.Worker.FunctionsApplication.InvokeFunctionAsync(FunctionContext context) in D:\a\_work\1\s\src\DotNetWorker.Core\FunctionsApplication.cs:line 77
Stack:    at lambda_method300(Closure , ChangeFancyResult , Object[] )
   at Microsoft.Azure.Functions.Worker.Invocation.MethodInvokerWithReturnValue`2.InvokeAsync(TReflected instance, Object[] arguments) in D:\a\_work\1\s\src\DotNetWorker.Core\Invocation\MethodInvokerWithReturnValue.cs:line 22
   at Microsoft.Azure.Functions.Worker.Invocation.DefaultFunctionInvoker`2.InvokeAsync(Object instance, Object[] arguments) in D:\a\_work\1\s\src\DotNetWorker.Core\Invocation\DefaultFunctionInvoker.cs:line 31
   at Microsoft.Azure.Functions.Worker.Invocation.DefaultFunctionExecutor.ExecuteAsync(FunctionContext context) in D:\a\_work\1\s\src\DotNetWorker.Core\Invocation\DefaultFunctionExecutor.cs:line 45
   at Microsoft.Azure.Functions.Worker.OutputBindings.OutputBindingsMiddleware.Invoke(FunctionContext context, FunctionExecutionDelegate next) in D:\a\_work\1\s\src\DotNetWorker.Core\OutputBindings\OutputBindingsMiddleware.cs:line 13
   at Microsoft.Azure.Functions.Worker.FunctionsApplication.InvokeFunctionAsync(FunctionContext context) in D:\a\_work\1\s\src\DotNetWorker.Core\FunctionsApplication.cs:line 77 

same time if I don't use Vogen:

    [Post("/api/ChangeResult")]
    Task ChangeResult(string marketId, int result);
    ...
    await _api.ChangeResult("test", 1);

then it is execued successfully

SteveDunn commented 3 months ago

@arteny - that looks like an Azure function. I'll see if I can set up an environment where I can test this.

arteny commented 3 months ago

@arteny - that looks like an Azure function. I'll see if I can set up an environment where I can test this.

ok but not sure that you are checking requested issue. last time you prepared the test for asp.net minimal api which is server part using vogen object as parameter, when my issue is created about when client part organized by refit interface is using vogen object as parameter, looks like the http request itself is malformed

SteveDunn commented 3 months ago

@arteny - there is an example using Refit. Does this not work for you? If you could provide a minimal repo with non-working code, I'd be happy to take a look.

arteny commented 3 months ago

@arteny - there is an example using Refit. Does this not work for you? If you could provide a minimal repo with non-working code, I'd be happy to take a look.

this example works, but it covers different case. please check @fbestfriends case. don't use route embedding for parameters, use them like regular url parameters:

 [Get("/weatherforecast")]
    Task<List<WeatherForecast>> GetWeatherForecastByCity(City city);
fbestfriends commented 1 month ago

@SteveDunn any progress with this?

SteveDunn commented 1 month ago

@fbestfriends - sorry for not posting back sooner. I completely forgot where I was with this one, but I just added a check and it works. I added this to the interface:

    [Get("/weatherforecast")]
    Task<List<WeatherForecast>> GetWeatherForecastByCityUsingUrlParameters(City city);

and added this in the sample runner:

        async Task GetWeatherForecastUsingUrlParameters(City city)
        {
            try
            {
                var forecasts = await api.GetWeatherForecastByCityUsingUrlParameters(city);
                foreach (var f in forecasts)
                {
                    Console.WriteLine($"City: {f.City}, TempC: {f.TemperatureC} ({f.TemperatureF.Value}F) - {f.Summary}");
                }
            }
            catch (Exception ex)
            {
                Console.WriteLine($"Error: {ex.Message}");
            }
        }

Please let me know how you get on.