Dnn.Platform icon indicating copy to clipboard operation
Dnn.Platform copied to clipboard

Add Dependency Injection Support in Search

Open iJungleboy opened this issue 3 years ago • 7 comments

Description of problem

Just like #4954 and #4955 dependency injection is still missing in crucial places, like search.

This means that as of now, search code - and any code running inside search - cannot rely on dependency injection. This drastically limits what can be written, because module code which is used in the view could use DI, but if the same code will be needed in search it cannot be used.

Please note that this is especially complex because the search crawler would need to change scopes during crawling, as things like Portal and module dependencies should change based on the portal/module that is being crawled.

Description of solution

  1. Modules which have DI based search provider should probably have a new interface to implement
  2. I presume there will need to be another way to register these, or there needs to be a very clear convention in terms of naming, that this can be found and instantiated through DI
  3. Search should instantiate that using a ServiceProvider,
  4. Before it does that, it would need to open a Scope and ensure that the Portal, Module etc. can be available as needed (Lazy?) so that anything down below will be based on this service provider and will also get further services in that scope

Description of alternatives considered

I believe there are no good alternatives. This is essential for moving into more modern APIs.

Affected browser

Not applicable.

iJungleboy avatar Dec 09 '21 07:12 iJungleboy

The current status is that only some scenarios within DNN Platform support dependency injection. However, classes designed for constructor injection can continue to receive their dependencies via constructor arguments, even if the container isn't available (by manually providing the dependencies). In some cases this may be impractical because of lifetime management concerns (e.g. if there's a service in the DI container that should be a singleton).

Regarding changing scope based on portal or module, the DI container does not currently have any portal or module specific dependencies, and there are no current plans to include that feature.

The long-term (hopefully DNN v10) plan is to use the dependency injection container anywhere the platform is creating instances of 3rd party code. This wouldn't require a new interface, it would continue to be based on ModuleSearchBase (or the legacy ISearchable), as well as IPortable, IUpgradeable, and a handful of other scenarios where the business controller class is used. This same scenario is supported for components which inherit from SchedulerClient today. However, the other changes need to wait until a major version because of changes in the lifetimes of the components being instantiated (i.e. many components that DNN creates today are created once and stored forever, but with the ability for a component to have a dependency on a scoped service or a disposable service, that won't be able to continue to be supported).

bdukes avatar Dec 10 '21 18:12 bdukes

I understand that challenge - had many similar problems to solve in 2sxc.

I recommend that this wrapped and to be used to clean up APIs. For example, we could create a IPortalSettings which wraps the current bad implementation but already allows the use or proper DI. this way new code can be written while the architecture is still old.

In additional idea would be that the new IPortalSettings would have a reduced, cleaned API to drop old APIs which were simply left-overs or to ensure better consistency (like sometimes where is PortalId with a small d, sometimes ModuleID with a big D. Offering DI objects which would only have the clean API would be a great way to clean-up.

iJungleboy avatar Dec 12 '21 09:12 iJungleboy

We have detected this issue has not had any activity during the last 90 days. That could mean this issue is no longer relevant and/or nobody has found the necessary time to address the issue. We are trying to keep the list of open issues limited to those issues that are relevant to the majority and to close the ones that have become 'stale' (inactive). If no further activity is detected within the next 14 days, the issue will be closed automatically. If new comments are are posted and/or a solution (pull request) is submitted for review that references this issue, the issue will not be closed. Closed issues can be reopened at any time in the future. Please remember those participating in this open source project are volunteers trying to help others and creating a better DNN Platform for all. Thank you for your continued involvement and contributions!

stale[bot] avatar Mar 30 '22 05:03 stale[bot]

keepalive

iJungleboy avatar Mar 30 '22 06:03 iJungleboy

@iJungleboy do you intend to contribute a PR ?

valadas avatar Mar 30 '22 15:03 valadas

@valadas I believe there first needs to be a comprehensive concept before code is added.

iJungleboy avatar Mar 31 '22 06:03 iJungleboy

We have detected this issue has not had any activity during the last 90 days. That could mean this issue is no longer relevant and/or nobody has found the necessary time to address the issue. We are trying to keep the list of open issues limited to those issues that are relevant to the majority and to close the ones that have become 'stale' (inactive). If no further activity is detected within the next 14 days, the issue will be closed automatically. If new comments are are posted and/or a solution (pull request) is submitted for review that references this issue, the issue will not be closed. Closed issues can be reopened at any time in the future. Please remember those participating in this open source project are volunteers trying to help others and creating a better DNN Platform for all. Thank you for your continued involvement and contributions!

stale[bot] avatar Jul 31 '22 03:07 stale[bot]

This issue has been closed automatically due to inactivity (as mentioned 14 days ago). Feel free to re-open the issue if you believe it is still relevant.

stale[bot] avatar Nov 02 '22 03:11 stale[bot]