ikyriak / IdempotentAPI

A .NET library that handles the HTTP write operations (POST and PATCH) that can affect only once for the given request data and idempotency-key by using an ASP.NET Core attribute (filter).
MIT License
283 stars 40 forks source link

Make idempotency optional #56

Closed SebastianBienert closed 11 months ago

SebastianBienert commented 1 year ago

Is there a way to make specific endpoints optionally idempontent, meaning, if the request contains an idempotency header we apply the mechanism, if not, we simple ignore it? From a brief lookup into the code, I think it is not possible right now:

https://github.com/ikyriak/IdempotentAPI/blob/b42a5020d723c19bc26ca9775634a98c70da1ceb/src/IdempotentAPI/Core/Idempotency.cs#L157

https://github.com/ikyriak/IdempotentAPI/blob/b42a5020d723c19bc26ca9775634a98c70da1ceb/src/IdempotentAPI/Core/Idempotency.cs#L514-L517

ikyriak commented 1 year ago

Hello @SebastianBienert

Thank you for reporting this issue.

We can use the [Idempotent] attribute in two ways:

1) On the controller to enable it for all POST/PATCH endpoints or

[ApiController]
[Consumes("application/json")]
[Produces("application/json")]
[Idempotent]
public class TestingIdempotentAPIController : ControllerBase
{
    //...
}

2) Separately on each POST/PATCH endpoint we would like to enable idempotency.

[ApiController]
[Consumes("application/json")]
[Produces("application/json")]
public class TestingIdempotentAPIController : ControllerBase
{
    [HttpPost("test")]
    [Idempotent]
    public ActionResult Test()
    {
        return Ok(new ResponseDTOs());
    }

    [HttpPost("test-disabled")]
    public ActionResult TestDisabled()
    {
        return Ok(new ResponseDTOs());
    }
}

The Enabled property seems to have an issue, so we cannot use it. I will investigate if we can fix it or maybe deprecate it.

morgan35543 commented 1 year ago

Hi @ikyriak, this package is great however using it requires a breaking change with the forced header. This makes utilising it a hard sell.

Is it possible to simply check for the header, if it's present then use the idempotent action filter, else skip it and continue as normal?

ikyriak commented 1 year ago

Hello @morgan35543,

Thank you for your message.

In your project, does the client application decide the requests should be idempotent and not the server/API? Please provide me with some information regarding your case to better understand it.

morgan35543 commented 1 year ago

Thanks for the quick reply @ikyriak.

We're attempting to apply idempotency to existing endpoints. Our product team are extremely unhappy with there being a breaking change across every POST & PATCH endpoint.

I did initially fork this repo and change some bits before seeing this issue discussing the problem. Is it feasible to add an option to skip instead of return an error when no key is present?

At the minute this GetKey method throws exceptions when no header is present: https://github.com/ikyriak/IdempotentAPI/blob/master/src/IdempotentAPI/Core/Idempotency.cs#L509

It would be nice to add the ability to configure it to instead continue through the rest of the middleware etc, by not throwing.

Edit: I'll throw together a pr to show you what I mean when I get a chance

vIceBerg commented 11 months ago

I want to add my voice to this.

Exact same case. I want to add idempotency to an existing MobileApp, but there will be a timeframe between the server update and the MobileApp update.

Our only actual choice is to add a version to our API contollers in order to avoid the breaking change.

ikyriak commented 11 months ago

Thank you, @vIceBerg, for your feedback. I am implementing this feature. I plan to prepare a prerelease version next week.

ikyriak commented 11 months ago

Hello @vIceBerg, @morgan35543, and @SebastianBienert

I have pushed the following prerelease NuGet libraries. You can set the IsIdempotencyOptional option in the attribute. When you have time, please check if it is okay for you.

angularsen commented 6 months ago

@ikyriak Could we make this globally configurable via AddIdempotentAPI?

I naively tried to, but it seems to only work when configured for individual API controllers or POST/PATCH endpoints with the [Idempotent(IsIdempotencyOptional = true)] attribute.

By making it nullable, individual attributes could override any globaly configuration.

services.AddIdempotentAPI(new IdempotencyOptions
{
    IsIdempotencyOptional = true, // Backwards compatible with existing clients.
});
angularsen commented 6 months ago

So I just discovered from docs I had to use [Idempotent(UseIdempotencyOption = true)] to make the global config stick.

This works, but I think it would maybe be more intuitive and better follow conventions to let option properties be nullable in order to allow global option properties to be overridden per API controller/action.