Don't get me wrong, I have used and loved this class for years. It wraps up all the objects I need very nicely and provides them in properties for me. But you know what else it does? It makes your plugins un-unit-testable, if I am allowed to create such a word. This is of course if you don't change / rewrite the class yourself.
There are a few mistake that are made by the developer toolkit:
Firstly, it embeds the LocalPluginContext class as a protected class and makes all the important properties internal. Worse again, it privatizes all the property setters on those most used interfaces. Now I cannot mock them with most mocking libraries!
Secondly, it creates a list of registered events to check if the plugin should run for the given message (the infamous tuple that dirties your constructors and has driven me daft for years). This is like a double plugin registration and really isn't required. Think about it, you register your plugin to fire in the Plugin Registration Tool, but if you forget to register it a second time in the constructor it will refuse to execute!
Finally, it needs a serious code review with resharper installed. There are a few redundant statements and lines that could be simplified or even removed.
I think it's time for new Plugin and LocalPluginContext classes and here is my suggestion:
Rework the base plugin class
I don't like the idea of inheriting from a Plugin class that implements IPlugin. It feels wrong to me that we have a class we can register as a plugin, but it's not really a plugin! So I want to rework this as a proper Base Plugin class. And this is my version of it: public abstract class BasePlugin : IPlugin
{
private readonly string _className;
protected BasePlugin()
{
_className = GetType().Name;
}
public void Execute(IServiceProvider serviceProvider)
{
// Construct the Local plug-in context.
var localContext = new LocalPluginContext(serviceProvider);
localContext.Trace("Entered {0}.Execute()", _className);
try
{
Execute(localContext);
}
catch (FaultException<OrganizationServiceFault> e)
{
// Trace the exception before bubbling so that we ensure everything we need hits the log
localContext.Trace(e);
// Bubble the exception
throw;
}
finally
{
localContext.Trace("Exiting {0}.Execute()", _className);
}
}
public abstract void Execute(ILocalPluginContext localContext);
}
So we still have the Execute method as before, but instead of an over complicated execution process (using the dreaded tuple) I just expect you to implement an Execute method accepting an ILocalPluginContext when inheriting this class.. So it will work almost exactly as before. Also, although our BasePlugin implements IPlugin it is abstract, so it's not going to appear in the list of available plugins when you load the DLL.
You also may have noticed I have introduced an ILocalPluginContext. This brings me on to my next suggestion.
Rework LocalPluginContext
Another area that always concerned me was how this internal class was written. I'm going to simplify it a little. Rather than pulling everything out of the necessary areas in a constructor let's just create lazy loaded properties. At least then we're only pulling out an IOrganizationService if it's used, a tracer if it's used, etc. I generally expect that all plugins will use a tracer, but quite often you won't need an IOrganizationService, e.g. validation plugins.I also want to add an interface to this to allow easier mocking while unit testing.
So here's my idea for the LocalPluginContext class:
public interface ILocalPluginContext
{
IOrganizationService OrganizationService { get; }
IPluginExecutionContext PluginExecutionContext { get; }
ITracingService TracingService { get; }
void Trace(string message, params object[] o);
void Trace(FaultException<OrganizationServiceFault> exception);
}
public class LocalPluginContext : ILocalPluginContext
{
private readonly IServiceProvider _serviceProvider;
private IPluginExecutionContext _pluginExecutionContext;
private ITracingService _tracingService;
private IOrganizationServiceFactory _organizationServiceFactory;
private IOrganizationService _organizationService;
public IOrganizationService OrganizationService
{
get
{
return _organizationService ?? (_organizationService = OrganizationServiceFactory.CreateOrganizationService(PluginExecutionContext.UserId));
}
}
public IPluginExecutionContext PluginExecutionContext
{
get
{
return _pluginExecutionContext ??
(_pluginExecutionContext = (IPluginExecutionContext)_serviceProvider.GetService(typeof(IPluginExecutionContext)));
}
}
public ITracingService TracingService
{
get
{
return _tracingService ?? (_tracingService = (ITracingService)_serviceProvider.GetService(typeof(ITracingService)));
}
}
private IOrganizationServiceFactory OrganizationServiceFactory
{
get { return _organizationServiceFactory ?? (_organizationServiceFactory = (IOrganizationServiceFactory)_serviceProvider.GetService(typeof(IOrganizationServiceFactory))); }
}
public LocalPluginContext(IServiceProvider serviceProvider)
{
if (serviceProvider == null)
{
throw new ArgumentNullException("serviceProvider");
}
_serviceProvider = serviceProvider;
}
public void Trace(string message, params object []o)
{
if (PluginExecutionContext == null)
{
SafeTrace(message, o);
}
else
{
SafeTrace(
"{0}, Correlation Id: {1}, Initiating User: {2}",
string.Format(message, o),
PluginExecutionContext.CorrelationId,
PluginExecutionContext.InitiatingUserId);
}
}
public void Trace(FaultException<OrganizationServiceFault> exception)
{
// Trace the first message using the embedded Trace to get the Correlation Id and User Id out.
Trace("Exception: {0}", exception.Message);
// From here on use the tracing service trace
SafeTrace(exception.StackTrace);
if (exception.Detail != null)
{
SafeTrace("Error Code: {0}", exception.Detail.ErrorCode);
SafeTrace("Detail Message: {0}", exception.Detail.Message);
if (!string.IsNullOrEmpty(exception.Detail.TraceText))
{
SafeTrace("Trace: ");
SafeTrace(exception.Detail.TraceText);
}
foreach (var item in exception.Detail.ErrorDetails)
{
SafeTrace("Error Details: ");
SafeTrace(item.Key);
SafeTrace(item.Value.ToString());
}
if (exception.Detail.InnerFault != null)
{
Trace(new FaultException<OrganizationServiceFault>(exception.Detail.InnerFault));
}
}
}
private void SafeTrace(string message, params object[] o)
{
if (string.IsNullOrWhiteSpace(message) || TracingService == null)
{
return;
}
TracingService.Trace(message, o);
}
}
One extra feature I have added is how it traces. The new LocalPluginContext contains a function that performs better tracing of FaultExceptions (ToString does not cut it) and I've matched the standard trace to how the tracing service works and included a param of objects.
What the new plugins look like!
A basic plugin looks like this which in my opinion is a lot cleaner: public class MyPlugin : BasePlugin
{
override public void Execute(ILocalPluginContext localContext)
{
// do what needs to be done!
}
}
One big upside to doing it this way is we have a much more testable framework where we simply pass in a mocked up ILocalPluginContext.
