Check for credential class implementation is slow

[gregsochanik]

Currently the reflection based check that's made if you use the static ApiWrapper creation method is very slow (adds ~800ms to a call). This doesn't affect users if they are using the FluentApi() constructor, but was wondering if we should take this out and force you to specify your credential implementations.

One way would be to include an IOC hook up library, which could then allow consumers to manually hook up their own credentials implementations... this way they could still use the Api.Create paradigm.

Or on the other hand (which I've done in the fork I'm using) just add a CreateWithCreds() method that takes these concrete implementations as arguments.

e.g.

public static IFluentApi<T> CreateWithCreds(oAuthCredentialsImplementation, apiUriImplementation)
{
    return new FluentApi<T>(oAuthCredentialsImplementation, apiUriImplementation);
}

[knocte]

800ms to every request??

[gregsochanik]

Nope, only if it hasn't got any knowledge of an implementation of IOAuthCredentials, or if you explicitly recreate the fluent api and don't specify creds for each request e.g. Api.Create().

Also, it grows the more assemblies you have, as it crawls through all of them looking for IOAuthCRedentials implementations.

I vote we move towards removing the need for it.

On Mon, Nov 12, 2012 at 3:55 PM, knocte notifications@github.com wrote:

800ms to every request??

— Reply to this email directly or view it on GitHubhttps://github.com/7digital/SevenDigital.Api.Wrapper/issues/19#issuecomment-10292643.

[knocte]

Ah ok.

Wait, I think I recall some trick to scan types quicker in assemblies, where's the code that does it?

[gregsochanik]

https://github.com/7digital/SevenDigital.Api.Wrapper/blob/master/src/SevenDigital.Api.Wrapper/EndpointResolution/EssentialDependencyCheck.cs

On Mon, Nov 12, 2012 at 4:07 PM, knocte notifications@github.com wrote:

Ah ok.

Wait, I think I recall some trick to scan types quicker in assemblies, where's the code that does it?

— Reply to this email directly or view it on GitHubhttps://github.com/7digital/SevenDigital.Api.Wrapper/issues/19#issuecomment-10293165.

[knocte]

Thanks.

My bad, I think what I recalled was GetExportedTypes() method which may be faster, however we shouldn't have the restriction of exposing the Credentials class as public I guess.

Anyway, if it's only 800ms for the first request, I don't consider it so severe.

[AnthonySteele]

CreateWithCreds method is in the master branch. Does this require mentioning in the docs or any other action?

[gregsochanik]

Not sure - I'm wondering if it would be better to handle this in a much less complicated manner.

For example, we remove the scan for IOAuthCredentials (and therefore the need for CreateWithCreds) and just allow the application to throw if no implementation exists.

We could then could just have a simple null check and throw a meaningful exception explaining that you need to supply credentials via an implementation of IOAuthCredentials.

We supply an example within the example console app here https://github.com/7digital/SevenDigital.Api.Wrapper/blob/master/src/SevenDigital.Api.Wrapper.ExampleUsage/AppSettingsCredentials.cs.

[AnthonySteele]

If we remove CreateWithCreds, surely we will need the scan in EssentialDependencyCheck ? I'm happy to have both options

[gregsochanik]

No - that's the point, I want to get rid of EssentialDependencyCheck, and just throw a meaningful run time exception with some basic instructions in it.

[AnthonySteele]

Ok, I'm happy with that.

[AnthonySteele]

Can we can do this by removing some of the overloads of the constructor of FluentApi - particularly the no-argument one, and having the ApiFactory class do the EssentialDependencyCheck once at creation and give the oAuthCredentials and apiUri to the fluent api.

ApiFactory is the class to replace with a different implementation of IApi if you want to customise api setup and not use EssentialDependencyCheck at all.

[gregsochanik]

Having a look at this now