vknet / vk

Vkontakte API for .NET
https://vknet.github.io/vk/
MIT License
568 stars 222 forks source link

Правильное использование DI для VkApi объекта #1578

Open bretbas opened 1 year ago

bretbas commented 1 year ago

Здравствуйте Пилю сейчас проект с использованием вашей библиотеки (спасибо за проект). У меня есть вопрос, касаемо DI, который я не могу понять как правильно решить. Вся проблема заключается в том, что VkApi принимает "почему-то" в параметрах IServiceCollection.

У меня есть вот такой вот код:

    public static void AddVkontakteClient<TProxyProvider, TCaptchaSolver>(
        this IServiceCollection services, 
        Action<VkontakteApiOptions> configure) 
        where TProxyProvider : class, IProxyProvider
        where TCaptchaSolver : class, ICaptchaSolver
    {
        services.Configure(configure);

        services.AddScoped<IVkontakteApiClient, VkontakteApiClient>();

        services.AddScoped(provider =>
        {
            var servicesCollection = new ServiceCollection();

            var proxyProvider = provider.GetRequiredService<IProxyProvider>();
            servicesCollection
                .AddSingleton(proxyProvider);
            servicesCollection
                .AddSingleton<ProxyHttpHandler>();
            servicesCollection
                .AddHttpClient<IRestClient, RestClient>()
                .ConfigurePrimaryHttpMessageHandler<ProxyHttpHandler>();
            servicesCollection
                .AddAudioBypass();
            servicesCollection.AddScoped<ICaptchaSolver, TCaptchaSolver>();

            var vkApi = new VkApi(servicesCollection);
            return vkApi;
        });
    }

Он регистрирует VkontakteApiClient как scoped, при этом VkontakteApiClient принимает у меня VkApi тоже как scoped.

Я хочу узнать два момента:

  1. Насколько в общем хорошо делать так, как сделал я? Мне кажется здесь будет утечка ресурсов (из-за проблемы HttpClient), так как IHttpClientFactory будет каждый раз новая, потому что происходит пересоздание new ServiceCollection.
  2. TCaptchaSolver в моей программе выступает класс AntiCaptchaSolver, который принимает у меня IAntiCaptchaClient, который регистрируется в IoC ВЫШЕ по уровню (тоесть не в том new ServiceCollection(), который создается для VkApi). И тянуть этот клиент антикаптчи не очень хочется в этот метод public static void AddVkontakteClient<TProxyProvider, TCaptchaSolver> не очень хочется (так как он там не нужен)
y0ung3r commented 1 year ago

Если коротко, то ваше решение ужасно.

Вы не понимаете смысл контейнера зависимостей. Его суть в том, что он забирает на себя разрешение всех зависимостей в приложении. А в приведенном вами коде вы пытаетесь зарезолвить что то сами, да еще и в каждом скоупе создаете дополнительные IServiceCollection. В общем, жуть полная :)

Я не пытаюсь принизить, отнюдь. Просто покажу как должно быть правильно.

Фактически IServiceCollection - это просто список. Список всех зависимостей. Причем вообще не важно в каком порядке все зарегистрировано. Воспринимайте каждый тип параметра конструктора вашего класса, который вы зарегистрируйте в контейнер, как тип, который тоже уже зарегистрирован в контейнере:

    public class A
    {
        public A(B b, C c)
        {
            // ...
        }
    }

Если мы хотим serviceProvider.GetRequiredService<A>(), то A, B, C должны быть зарегистрированы в контейнере зависимостей:

    public static void AddA(
        this IServiceCollection services)
    {
        services.AddScoped<A>();
        services.AddScoped<B>();
        services.AddScoped<C>();
    }

Иногда бывает такое, что A принимает дополнительный параметр, который ну никак нельзя зарегистрировать в контейнере зависимостей:

    public class A
    {
        public A(B b, C c, string name)
        {
            // ...
        }
    }

В таких случаях можно использовать фабрику:

    public static void AddA(
        this IServiceCollection services)
    {
        services.AddScoped<B>();
        services.AddScoped<C>();
        services.AddScoped<A>(provider =>
            new A(provider.GetRequiredService<B>(), provider.GetRequiredService<C>(), "CONST NAME")); // Или через ActivatorUtilities
    }

Здесь стоит оговориться, что фабрики вообще очень мощный инструмент для передачи дополнительных параметров в контейнере зависимостей. Погуглите использование фабрик в IServiceCollection самостоятельно. Но я остановился на самом простом примере.

После того, как все зависимости будут зарегистрированы в контейнере зависимостей, собирается IServiceProvider через services.BuildServiceProvider(), который и будет предоставлять вам зависимости в экземпляр класса указанного типа.

Из всего вышесказанного делаем вывод, что IServiceCollection должен создаваться ровно 1 раз где то в точке входа вашего приложения, обычно это Program.cs (однако в ASP.NET автоматически уже предоставляется IServiceCollection для конфигурации)

В вашем случае код должен быть примерно такой (я не вникал [и не хочу] в суть ваших зависимостей):

    public static void AddVkontakteClient<TProxyProvider, TCaptchaSolver>(
        this IServiceCollection services, 
        Action<VkontakteApiOptions> configure) 
        where TProxyProvider : class, IProxyProvider
        where TCaptchaSolver : class, ICaptchaSolver
    {
        services.Configure(configure);

        services.AddScoped<IVkontakteApiClient, VkontakteApiClient>();

        services.AddSingleton(typeof(IProxyProvider), typeof(TProxyProvider));
        services.AddSingleton<ProxyHttpHandler>();

        services
           .AddHttpClient<IRestClient, RestClient>()
           .ConfigurePrimaryHttpMessageHandler<ProxyHttpHandler>();

        services.AddAudioBypass();

        services.AddSingleton(typeof(ICaptchaSolver), typeof(TCaptchaSolver));

        var vkApi = new VkApi(services);
        services.AddSingleton(vkApi);
    }

Соответственно, когда вам понадобится VkApi, запрашиваем его через конструктор и получаем правильно сконфигурированный сервис:

    public class A
    {
        public A(VkApi apiClient)
        {
            // apiClient уже имеет TCaptchaSolver, AudioBypass и все, что вы ему там нарегистрировали.
        }
    }
bretbas commented 1 year ago

Если коротко, то ваше решение ужасно. Решение было принятно исходя из той реализации VkNet, которая сейчас имеется. Если честно, все равно не понимаю, почему сделано так, что VkNet обязан в конструктора принимать IServiceCollection, где как я смотрю, внутри происходит бидл в IServiceProvider чтобы ТУПО из IoC достать нужные зависимости... Вот это ужас, на мой взгляд.

Мало того, вы показали пример, как зарегистрировать VkApi как синглтон, что на мой взгляд неверно, так как внутри используется IRestClient, у которого я переопределил регистрацию с singleton на transient за счет такой регистрации:

   services
           .AddHttpClient<IRestClient, RestClient>()
           .ConfigurePrimaryHttpMessageHandler<ProxyHttpHandler>();

но получается за счет того, что вы регаете VkApi как синглетон, то и IRestClient в нем будет ТОЖЕ как синглетон, что не очень хотелось бы.

Поправьте если я не прав

y0ung3r commented 1 year ago

Если честно, все равно не понимаю, почему сделано так, что VkNet обязан в конструктора принимать IServiceCollection

Согласен, VkApi должен принимать IServiceProvider (что является анти-паттерном ServiceLocator) или вообще все возможные зависимости (~ штук 10-20 параметров), которые регистрируются библиотекой самостоятельно (что самое правильное решение). Но сделано, как сделано.

но получается за счет того, что вы регаете VkApi как синглетон, то и IRestClient в нем будет ТОЖЕ как синглетон

Вы не правы. IRestClient не будет ТОЖЕ Singleton'ом, потому что IRestClient регистрируется отдельно. Я объясню: у вас есть IServiceCollection, в который вы регистрируете IRestClient как Transient, а потом этот IServiceCollection передаете в VkApi, который увидит в этом списке уже зарегистрированный IRestClient и не будет ничего переопределять.

Решение было принятно исходя из той реализации VkNet, которая сейчас имеется

Не нужно вообще думать о том, как что то внешнее реализовано. Вы должны ориентироваться только на публичное API. Представьте, что у вас нет исходников VkNet вообще (или они обфусцированы)

bretbas commented 1 year ago

Вы не правы. IRestClient не будет ТОЖЕ Singleton'ом, потому что IRestClient регистрируется отдельно. Я объясню: у вас есть IServiceCollection, в который вы регистрируете IRestClient как Transient, а потом этот IServiceCollection передаете в VkApi, который увидит в этом списке уже зарегистрированный IRestClient и не будет ничего переопределять.

Но как RestClient не будет синглетоном, если VkNet его в зависимостях принимает?)) VkNet имеет один экземпляр на все приложение, а значит зависимость IRestClient внутри него будет ТОЖЕ на все приложение

y0ung3r commented 1 year ago

А, я понял что вы имеете ввиду. Нет, IRestClient будет Transient, а VkApi будет Singleton в любом случае. Моя ошибка тут скорее в том, что IRestClient может помереть в процессе жизни VkApi, что 100% уронит приложение. Но я же и написал, что не вникал в суть ваших зависимостей, а лишь показал что регистрировать зависимости нужно по-другому.

В общем, что мешает сделать так?

    services.AddScoped(provider => new VkApi(services)); 
    // Или если версия .NET больше 6-ой: services.AddScoped(_ => new VkApi(services));
bretbas commented 1 year ago

мешает то, что так не выйдет. попробуйте сами.

а не выходит из-за того, что внутри VkApi зачем-то билд идет в IServiceProvider этой коллекциию.

bretbas commented 1 year ago

Моя ошибка тут скорее в том, что IRestClient может помереть в процессе жизни VkApi, что 100% уронит приложение.

Нет, вы не правы. Приложение не уронит, но вот то что экземпляр RestClient будет один единственный на такой вот VkApi - это 100%

bretbas commented 1 year ago

Не проще ли просто взять, и сделать НОРМАЛЬНЫЙ DI для вашего VkApi?

Rast1234 commented 1 month ago

может, кому-то поможет обойти текущие ограничения. я пока вот таким костылем обошелся: сделал сервис который возвращает инстанс VkApi, и позволяет использовать DI-контейнер приложения. сам сервис регаю как singleton. понятно что инстанс надо кэшить и все такое, тут только пример инициализации

public class VkApiProvider(IServiceProvider serviceProvider)
{
    /// <summary>
    /// Hack for VkApi to use existing services
    /// </summary>
    /// <remarks>
    /// * Will break if VkNet.AudioBypassService and VkNet assemblies have different versions<br/>
    /// * Works only because VkNet constructor inits stuff once and then service provider field is not used anywhere
    /// </remarks>
    public VkApi GetVkApi()
    {
        var instance = new VkApi();
        instance.GetType().GetTypeInfo().GetDeclaredMethod("Initialization")!.Invoke(instance, [serviceProvider]);
        return instance;
    }
}

фокус в том, что service collection внутри VkApi никак не используется, а инициализацию можно безболезненно повторить после вызова конструктора, уже со своим service provider

CaCTuCaTu4ECKuu commented 1 month ago

Я написал сервис VkApiManager с методами GetInstace внутри которых ручками создаётся VkApi и все нужные зависимости я уже получаю от ServiceProvider и кладу туда (например CaptchaSolver, логгер) Там же я и кеширую инстанс. В зависимости от потребностей можно сделать чтоб при запросе сервис автоматически находил токен (или логин пароль) и использовать как ключ инстанса внутренний id аккаунта или его логин для метода GetInstance или напрямую передавать токен/логинпароль Если кому то надо могу код скинуть