How to avoid passing a context reference among classes

Question

Dynamics CRM 2011 on premise. (But this problem exists in many situations away from Dynamics CRM.)

CRM plugins have an entry point:

void IPlugin.Execute (IServiceProvider serviceProvider)

(http://msdn.microsoft.com/en-us/library/microsoft.xrm.sdk.iplugin.execute.aspx)

serviceProvider is a reference to the plugin execution context. Anything useful that a plugin does requires accessing serviceProvider, or a member of it.

Some plugins are large and complex and contain several classes. For example, I'm working on a plugin that has a class which is instantiated multiple times. This class needs to use serviceProvider.

One way to get access to serviceProvider from all the classes that need it would be to add a property to all those classes and then to set that property. Or to add properties for the parts of serviceProvider that each class needs. Either of these approaches would result in lots of duplicate code.

Another approach would be to have a global variable in the scope of the thread. However, according to http://msdn.microsoft.com/en-us/library/cc151102.aspx one "should not use global class variables in plug-ins."

So what is the best way to have access to serviceProvider without passing it around everywhere?

P.S. If an example helps, serviceProvider provides access to a logging object. I want almost every class to log. I don't want to pass a reference to the logging object to every class.

Answer 1

That's not quite what the warning in the documentation is getting at. The IServiceProvider isn't a global variable in this context; it's a method parameter, and so each invocation of Execute gets its own provider.

For improved performance, Microsoft Dynamics CRM caches plug-in instances. The plug-in's Execute method should be written to be stateless because the constructor is not called for every invocation of the plug-in. In addition, multiple threads could be running the plug-in at the same time. All per invocation state information is stored in the context. This means that you should not use global class variables in plug-ins [Emphasis mine].

There's nothing wrong with passing objects from the context to helper classes which need them. The warning advises against storing something in a field ("class variable") on the plugin class itself, which may affect a subsequent call to Execute on the same instance, or cause concurrency problems if Execute is called by multiple threads on the same instance simultaneously.

Of course, this "globalness" has to be considered transitively. If you store anything in either the plugin class or in a helper class in any way that multiple calls to Execute can access (using fields on the plugin class or statics on either plugin or helper classes, for example), you leave yourself open to the same problem.

As a separate consideration, I would write the helper classes involved to accept types as specific to their function as possible - down to the level of individual entities - rather than the entire IServiceProvider. It's much easier to test a class which needs only an EntityReference than one which needs to have an entire IServiceProvider and IPluginExecutionContext mocked up.

---

On global variables vs injecting values required by classes

You're right, this is something that comes up everywhere in object-oriented code. Take a look at these two implementations:

public class CustomEntityFrubber
{
    public CustomEntityFrubber(IOrganizationService service, Guid entityIdToFrub)
    {
        this.service = service;
        this.entityId = entityIdToFrub;
    }

    public void FrubTheEntity()
    {
        // Do something with service and entityId.
    }

    private readonly IOrganizationService service;
    private readonly Guid entityId;
}

---

// Initialised by the plugin's Execute method.
public static class GlobalPluginParameters
{
    public static IOrganizationService Service
    {
        get { return service; }
        set { service = value; }
    }

    public static Guid EntityIdToFrub
    {
        get { return entityId; }
        set { entityId = value; }
    }

    [ThreadStatic]
    private static IOrganizationService service;

    [ThreadStatic]
    private static Guid entityId;
}

public class CustomEntityFrubber
{
    public FrubTheEntity()
    {
        // Do something with the members on GlobalPluginParameters.
    }
}

So assume you've implemented something like the second approach, and now you have a bunch of classes using GlobalPluginParameters. Everything is going fine until you discover that one of them is occasionally failing because it needs an instance of IOrganizationService obtained by calling CreateOrganizationService(null), so it accesses CRM as the system user rather than the calling user (who doesn't always have the required privileges).

Fixing the second approach requires you to add another field to your growing list of global variables, remembering to make it ThreadStatic to avoid concurrency problems, then changing the code of CustomEntityFrubber to use the new SystemService property. You have tight coupling between all these classes.

Not only that, all these global variables hang around between plugin invocations. If your code has a bug that somehow bypasses the assignment of GlobalPluginParameters.EntityIdToFrub, suddenly your plugin is inexplicably operating on data that wasn't passed to it by the current call to Execute.

It's also not obvious exactly which of these global variables the CustomEntityFrubber requires, unless you read its code. Multiply that by however many helper classes you have, and maintaining this code starts to become a headache. "Now, does this object need me to have set Guid1 or Guid2 before I call it?" On top of that, the class itself can't be sure that some other code won't go and change the values of global variables it was relying on.

If you used the first approach, you simply pass in a different value to the CustomEntityFrubber constructor, with no further code changes needed. Furthermore, there's no stale data hanging around. The constructor makes it obvious which dependencies the class has, and once it has them, it can be sure that they don't change except in ways they were designed for.

Answer 2

As you say, you shouldn't put a member variable on the plugin since instances are cached and reused between requests by the plugin pipeline.

The approach I take is to create a class that perform the task you need and pass a modified LocalPluginContext (making it a public class) provided by the Developer Toolkit (http://msdn.microsoft.com/en-us/library/hh372957.aspx) on the constructor. Your class then can store the instance for the purposes of executing it's work just in the same way you would with any other piece of code. You are essentially de-coupling from the restrictions imposed by the Plugin framework. This approach also makes it easier to unit test since you only need to provide the execution context to your class rather than mocking the entire plugin pipeline.

It's worth noting that there is a bug in the automatically generated Plugin.cs class in the Developer Toolkit where it doesn't set the ServiceProvider property - At the end of the constructor of the LocalPluginContext add the line:

this.ServiceProvider = serviceProvider;

I have seen some implementations of an IoC approach in Plugins - but IMHO it makes the plugin code way too complex. I'd recommend making your plugins lean and simple to avoid threading/performance issues.

Answer 3

There are multiple things I would worry about in this design request (not that it's bad, just that one should be aware of, and anticipate).

1. IOrganizationService is not multi-thread safe. I'm assuming that other aspects of the IServiceProvider are not as well.
2. Testing things at an IServiceProvider level is much more complicated due to the additional properties that have to be mocked
3. You'd need a method for handle logging if you ever decided to call logic that is currently in your plugin, outside of the plugin (e.g. a command line service).

If you don't want to be passing the object around everywhere, the simple solution is to create a static property on some class that you can set it upon plugin execution, and then access from anywhere.

Of course now you have to handle issue #1 from above, so it'd have to be a singleton manager of some sort, that would probably use the current thread's id to set and retrieve the value for that thread. That way if the plugin is fired twice, you could retrieve the correct context based on your currently executing thread. (Edit Rather than some funky thread id lookup dictionary, @shambulator's ThreadStatic property should work)

For issue #2, I wouldn't be storing the IServiceProvider as is, but split up it's different properties (e.g. IPluginExecutionContext, IOrganizationService, etc)

For issue #3, it might make sense to store an action or a function in your manager rather than the object values themselves. For example, if rather than storing the IPluginExecutionContext, store a func that accepts a string to log and uses the IPlurginExeuctionContext to log. This allows other code to setup it's own logging, without being dependent on executing from within a plugin.