Concurrency issues - want to address these in 4.0

[AnthonySteele]

I have seen more people walk right into the concurrency issues, i.e they expect multiple calls to a fluent api to be isolated from each other and they are not, the state is shared between them.

Now that we are working to version 4.0, we can reconsider breaking changes and can address these.

See also: https://github.com/7digital/SevenDigital.Api.Wrapper/pull/32 and https://github.com/7digital/SevenDigital.Api.Wrapper/issues/20 and https://github.com/7digital/SevenDigital.Api.Wrapper/pull/34

[AnthonySteele]

The way that we work around it is by using this wrapper code that we inject:

public interface IApiFactory
{
    IFluentApi<T> Create<T>() where T : class;
}

public class ApiFactory: IApiFactory
{
    public IFluentApi<T> Create<T>() where T : class
    {
        var api = Api<T>.Create;
        return api;
    }
}

But the it would be better if people don't run into this problem, spend time puzzling over it and go looking for a fix in the first place.

[gregsochanik]

See also https://github.com/7digital/SevenDigital.Api.Wrapper/pull/34

[AnthonySteele]

Simplest fix is to put this (i.e. the IApiFactory code above) in the ExampleUsage project.

This is an extension point where the client may want to customise - i.e. if the consumer codes their own ApiFactory.Create(), it can contain Api configuration and setup - WithCache etc. Pulling it into the framework might break that.

[AnthonySteele]

The apiFactory.Create<T> is pretty similar to the proposal here https://github.com/7digital/SevenDigital.Api.Wrapper/issues/49

[AnthonySteele]

Can we remove the public static class Api<T> and replace it with an Api similar to the Api factory above. This would be injectable if you want to use it as is, and replaceable if you want to use a different implementation of the simple interface and e.g. supply your own creds or cache during fluent interface construction.

Could also have a static method Create<T> to make the simple case really simple

I think this would solve most of the issues.

[gregsochanik]

Yes - sounds good to me!

[AnthonySteele]

So I a branch and have code here. No PR yet.

The example usage program shows the simple case usage,

ie.

    IApi api = new ApiFactory();
    var artist = await api.Create<Artist>()
        .WithArtistId(artistId)
    .Please();

    var artistTopTracks = await api.Create<ArtistTopTracks>()
        .WithArtistId(artistId)
        .Please();

The advantage is that api is of type IApi so it is injectable, and you can make multiple independent request of the same or varying types off it using api.Create<T>().

A slightly more complex usage is for the api user to make their own implementation of IApi supplying creds etc.

The tests all still use the "very simple case" - the static Api class is still there, haven't taken the time to tweak all the tests as I'm not decided on how they will look yet. Maybe there's a better way to do this and still keep simplicity and flexibility?

[AnthonySteele]

Update: See the Pull request