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

Introduce service container #198

Closed
Kittyfisto opened this issue Jun 24, 2019 · 2 comments
Closed

Introduce service container #198

Kittyfisto opened this issue Jun 24, 2019 · 2 comments

Comments

@Kittyfisto
Copy link
Owner

Kittyfisto commented Jun 24, 2019

Current behaviour

Currently, plugin interfaces such as IFileFormatPlugin.Open(string fileName, ITaskScheduler taskScheduler) are very rigid, yet are expected to call constructors of objects from Tailviewer.Core, such asTextLogFile(ITaskScheduler scheduler, string fileName, ITimestampParser timestampParser = null, ILogLineTranslator translator = null, Encoding encoding = null). This is all fine and dandy, however what happens if TextLogFile requires another dependency (for example a custom list of log levels, see #184)? Do we break plugin compatibility for every new dependency?

Expected behaviour

Plugins should not break because tailviewer needs to forward more dependencies through a plugin to one its own classes. In other words, if a plugin wants to modify the log line translation aspect of a file, then it should not be expected to care about any other aspect of log files.

Concrete implementation

Introduce a service container interface such as:

public interface IServiceContainer
{
    T Resolve<T>() where T : class;
}

and break plugin compability only once by changing the following method signatures:

  • IFileFormatPlugin.Open(string fileName, ITaskScheduler taskScheduler) => IFileFormatPlugin.Open(string fileName, IServiceContainer services)
  • ILogAnalyserPlugin.Create(ITaskScheduler scheduler, ILogFile source, ILogAnalyserConfiguration configuration) => ILogAnalyserPlugin.Create(ILogFile source, ILogAnalyserConfiguration configuration, IServiceContainer services)
  • IDataSourceAnalyserPlugin.Create(AnalyserId id, ITaskScheduler scheduler, ILogFile logFile, ILogAnalyserConfiguration configuration) => IDataSourceAnalyserPlugin.Create(AnalyserId id, ILogFile logFile, ILogAnalyserConfiguration configuration, IServiceContainer services)
  • IWidgetPlugin.CreateViewModel(IWidgetTemplate template, IDataSourceAnalyser dataSourceAnalyser) ==> IWidgetPlugin.CreateViewModel(IWidgetTemplate template, IDataSourceAnalyser dataSourceAnalyser, IServiceContainer services)

This change will allow tailviewer to inject any future dependency into plugins without breaking backwards compatibility on plugins: Plugins will not need to be re-compiled in case tailviewer needs to pass yet another dependency to one of its classes (such as TextLogFile). The downside to this is obviously that if a plugin author requires a given dependency, then it will be harder to find out how to obtain it (since it's no longer part of the static singature).
Therefore this change requires better documentation on tailviewer's part.

Constraints

  • Do NOT introduce additional dependencies in TV's nuget package (i.e. neither Tailviewer.Api nor Tailviewer.Core may contain references to unity or other DI frameworks)
  • Tailviewer.Core should maintain a simple IServiceContainer implementation where singletons are registered with the container (this is to not require every plugin author to implement this interface in case he/she wants to write a test for their plugin):
class ServiceContainer
{
    public void RegisterInstance<TService, TImplementation>(TImplementation service) where TImplementation : TService where TService : class
    {
         _services.Add(....);
    }

    public T Resolve<T>()
    {
        return _services.Get<T>(...);
    }
  • Tailviewer (the main project) may implement IServiceContainer by re-using existing DI frameworks, but it doesn't have to - mabye the ServiceContainer as part of Tailviewer.Core is enough at first
@Kittyfisto
Copy link
Owner Author

Kittyfisto commented Jun 24, 2019

TODO:

  • Add IServiceContainer interface to Tailviewer.Api
  • Add ServiceContainer implementation to Tailviewer.Core
  • Modify the following classes' constructors to accept an IServiceContainer instead of ITaskScheduler (they should obviously resolve() the task scheduler):
  • EventLogFile
  • FilteredLogFile
  • LogFileProxy
  • MergedLogFile
  • MultiLineLogFile
  • NoThrowLogFile
  • TextLogFile
  • AbstractWidgetViewModel
  • LogAnalyser
  • Increase the PluginInterfaceVersion of any plugin who's signature changes as part of this issue:
  • IFileFormatPlugin
  • IDataSourceAnalyserPlugin
  • ILogAnalyserPlugin
  • IWidgetPlugin
  • Use this chance to finally implement refactoring of IDataSourceAnalyser (since that would break compatibility as well)
  • Tag all interfaces which are now served through the IServiceContainer (maybe a remark in its documentation, maybe an attribute)
  • Write new plugin compatability tests
  • Update plugin documentation now that method signatures have changed

Kittyfisto added a commit that referenced this issue Jun 24, 2019
Plugins don't break yet (will come in a future commit)
Kittyfisto added a commit that referenced this issue Jun 24, 2019
…ture dependencies #198

Every plugin interface has been changed to accept an IServiceContainer through which a plugin can resolve dependencies which it previously was given directly as a parameter. This way it is possible to NOT break binary backwards compatability with plugin and still inject more parameters into newer plugin which require them.
As a result, the plugin interface version of every plugin interface has been increased and ALL existing plugins will no longer load due to this change: They will have to be updated (targeting the newest nuget package).
Apart from pending refactorings of IDataSourceAnalyserPlugin and IDataSourceAnalyser (which are due BEFORE the next release), this commit represents the last breaking change made to the plugin system.
@Kittyfisto Kittyfisto added this to In progress in v1.0 Jun 24, 2019
@Kittyfisto Kittyfisto moved this from In progress to Done in v1.0 Jun 25, 2019
@Kittyfisto Kittyfisto moved this from Done to In progress in v1.0 Jun 25, 2019
Kittyfisto added a commit that referenced this issue Jun 25, 2019
Kittyfisto added a commit that referenced this issue Jun 25, 2019
v1.0 automation moved this from In progress to Done Feb 14, 2021
@Kittyfisto
Copy link
Owner Author

Fixed with Tailviewer v1.0.0-RC1.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
v1.0
  
Done
Development

No branches or pull requests

1 participant