Allow customization of GetSecretValueRequest

[avalara-stephen-hickey]

resolves #75

[avalara-stephen-hickey]

Any other changes you'd like to see @Kralizek ?

[Kralizek]

Any other changes you'd like to see @Kralizek ?

yes, a couple. Tomorrow I'l write a proper review :D

[Kralizek]

So, I've been thinking about this, and rather than having the user provide a factory method, I prefer the library creating the request at its best and then pass the request and the context in an Action<request, context> delegate that the user can or cannot customize.

let me know if the comment is unclear.

[avalara-stephen-hickey]

Happy to do that.

I originally wanted to make an events object similar to JwtBearerEvents (see usage in the JwtBearerHandler), but was trying to stick to the minimalistic style of the library. Seeing as adding the context already added complexity, I think it's reasonable to go in this direction.

What do you think of adding an Events object that only has the one action on it right now for adjusting the request before it's sent? That would make it easily expandable in the future.

[Kralizek]

Do you mean in the options class? Not sure. From one side i like to keep it simple, but on the other hand, more and more options are getting added.

Do you think you could add here a preview of what the option class/es would look like?

[avalara-stephen-hickey]

// SecretsManagerConfigurationProviderOptions.cs
public SecretManagerConfigurationEvents Events { get; set; } = new();

// SecretManagerConfigurationEvents.cs
public class SecretManagerConfigurationEvents
{
    public Func<GetSecretValueRequest, Task> OnGetSecretValueRequestCreated { get; set; } = context => Task.CompletedTask;

    public virtual Task GetSecretValueRequestCreated(GetSecretValueRequest context) => OnGetSecretValueRequestCreated(context);
}

// Usage
builder.AddSecretsManager(configurator: options =>
{
    options.Events = new SecretManagerConfigurationEvents
    {
        OnGetSecretValueRequestCreated = request => request.VersionStage = "AWSCURRENT";
    }
});

// I'm not sure yet, but it may be possible to simplify the previous example to this, but I'd have to test.
builder.AddSecretsManager(configurator: options =>
{
    options.Events.OnGetSecretValueRequestCreated = request => request.VersionStage = "AWSCURRENT";
});

// Additionally, a user could create their own class that inherits from
// SecretManagerConfigurationEvents and overrides GetSecretValueRequestCreated
// (or any other added event) and just set that in the configurator
// (i.e. options.Events = new MyEventsClass()).

I think this is neat because if you ever want to add any more hooks, all you have to do is add a function to the events class and make sure to call it at the appropriate spot in the SecretsManagerConfigurationProvider.

[Kralizek]

I'll have to play around with it. 🤔

How urgent is this PR for you?

[avalara-stephen-hickey]

It's blocking a Q4 story for me. I'm happy to update my MR to what I've demonstrated above to make it easier for you to play around with.

[Kralizek]

I understand. It's 18:30 here in Europe now. I'll make sure to allocate more time already on Monday. I'm sorry for the delay. Got caught on few things myself.

[avalara-stephen-hickey]

Thank you for the library! It saved me a lot of effort. I don't mean to rush you too much, I understand this is probably something you work on in your free time, which is why I'm trying to put in most of the work for you.

I've updated the branch that this change request is on to be what I think you're originally asking for (an action to customize an existing GetSecretValueRequest instance).

I've also made another branch (feature/75-2) that shows the events version I mentioned. I chose to make the method synchronous (as opposed to my original post), but we could make them asynchronous if it's possible a user ever wanted to do something asynchronous inside the method.

After working on both, I'm actually leaning in favor of your idea of just the additional action. The existing options class is basically the same as the events class I was trying to make.

[Kralizek]

Thank you for the library! It saved me a lot of effort. I don't mean to rush you too much, I understand this is probably something you work on in your free time, which is why I'm trying to put in most of the work for you.

Thank you for the nice words. I'm sorry I've neglected this and other OSS projects recently. I am trying to catch up back a bit. :)