Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

most of the APIs remain the same however notable changes have been made #16

Merged
merged 2 commits into from
Oct 7, 2020

Conversation

alexandru-bagu
Copy link
Contributor

most of the APIs remain the same however notable changes have been made:

  1. no longer using raw Dictionary<string, string> but either MetadataCollection or OptionCollection; MetadataCollection contains strict validation on keys - this will help prevent invalid metadata key name issues where the user inputs an invalid metadata key and is accepted but is changed to fit the tus protocol.

  2. no longer using raw actions for events; defined fully typed delegates for both upload progress and completition; those events contain two parameters now: 1st is source, 2nd is event context with the type of an interface - main issue here is that because ITusClient is now a transitive instance the instance should be provided as a source of the event.

  3. TusBuild::DefaultTusClientBuild returns a DefaultTusBuilder. The only notable change here is that the configure functions contain two parameters: TusClientOptions and IHttpClientBuilder. Both TusBuild::DefaultTusClientBuild and the IServiceCollection extension provide access to the IHttpClientBuilders used, however only DefaultTusBuilder has the Build function. Both methods available (standalone call to Build or dependency injection) provide a transient ITusClient. Build can be called multiple times resulting in different ITusClient instances. - this allowed me to rewrite the TusBuild class to use what should easily be used by dotnet Dependency Injection architecture. Since most of the projects created with dotnet nowadays rely on its dependency injection for running, ITusClient should be injected in the already existing context and not create its own context. There are cases where one might want an isolated ITusClient and they can do that by using the TusBuild::DefaultTusClientBuild to create an isolated instance and maybe inject it in the main DI using a factory implementation.

I formatted all the text files and removed unused using statements as well as added more comments to functions/parameters.

…ade:

1. no longer using raw Dictionary<string, string> but either MetadataCollection or OptionCollection; MetadataCollection contains strict validation on keys
2. no longer using raw actions for events; defined fully typed delegates for both upload progress and completition; those events contain two parameters now: 1st is source, 2nd is event context with the type of an interface
3. TusBuild::DefaultTusClientBuild returns a DefaultTusBuilder. The only notable change here is that the configure functions contain two parameters: TusClientOptions and IHttpClientBuilder. Both TusBuild::DefaultTusClientBuild and the IServiceCollection extension provide access to the IHttpClientBuilders used, however only DefaultTusBuilder has the Build function. Both methods available (standalone call to Build or dependency injection) provide a transient ITusClient. Build can be called multiple times resulting in different ITusClient instances.
@bluetianx
Copy link
Owner

ok,I merged, Do you have email? I want to send message to you

@bluetianx bluetianx merged commit bfa7119 into bluetianx:dev Oct 7, 2020

namespace BirdMessenger.Builder
{
public interface ITusHttpClientBuilder<TService>
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it should rename ITusHttpClientConfigre ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok,I merged, Do you have email? I want to send message to you

I updated my profile. You can send me an email using that.

it should rename ITusHttpClientConfigre ?

That sounds good.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants