ServerContext and Dependency Injection

I was reviewing some code today and came across a perfect case study for Dependency Injection. It also required another peak under the hood of Microsoft.Office.Server.ServerContext, the results of which I thought might be worth sharing.

From MSDN,  ServerContext “provides run-time methods for shared services in Microsoft Office SharePoint Server 2007.” If you’re not familiar with Shared Service Providers, Shane Young has a concise overview or check out Ted Pattison‘s developer overview on MSDN. The SSP APIs span several namespaces:

  • Microsoft.Office.Excel.Server
  • Microsoft.Office.Server.ApplicationRegistry (Business Data Catalog)
  • Microsoft.Office.Server.Audience
  • Microsoft.Office.Server.Search
  • Microsoft.Office.Server.UserProfiles (includes My Sites)

The entry point to all of these APIs is ultimately a ServerContext object, made available through several static methods and properties:

  • ServerContext.Default
    Returns a new instance for the local farm’s default provider.
  • ServerContext.Current
    Returns ServerContext.GetContext(HttpContext.Current).
  • ServerContext.GetContext(HttpContext httpContext)
    Returns a shared ServerContext instance for the given request context. The provider is determined first through the microsoft.office.server/sharedService section in Web.config, and from the context WebApplication if that fails.
  • ServerContext.GetContext(WebApplication webApplication)
    Returns a new instance for the web application’s provider or the farm’s default provider if one is not specified.
  • ServerContext.GetContext(SPSite site)
    Returns ServerContext.GetContext(site.WebApplication).
  • ServerContext.GetContext(String sharedResourceProviderName)
    Returns a new instance for the named provider if it exists, either in the local farm or its parent.

These details inform some simple usage guidelines:

  1. If you have an HttpContext, use the shared instance from GetContext(HttpContext) or ServerContext.Current.
  2. If you need a specific SSP, use an overload of GetContext().
  3. Else, use ServerContext.Default.

Code Review

The code I was reviewing today was replacing this…

SearchContext.GetContext(SPContext.Current.Site)

…with this…

SearchContext.GetContext(ServerContext.Default)

…to support use of the service in a non-web context. We’re currently working with a single SSP, so Default should be sufficient; however, we can do better. First, the change in question had to be made in several places. For maintainability, it makes more sense to define a field or property to represent the context wherever we need it. Second, by using ServerContext.Default we’re missing out on the benefits of the shared instance provided for an HttpContext, which we usually have.

Dependency Injection

The Dependency Inversion Principle suggests that we should decouple the source of our ServerContext object from how its consumption. What would that look like here? Well the simplest approach is constructor injection:

private ServerContext { get; set; }
public MyService(ServerContext context)
{
    this.ServerContext = context;
}

This is the purest form of DI in that the class knows nothing about where the ServerContext came from, but it’s often practical to “cheat” and expose a nicer interface. In my case, I chose to supplement that constructor with two others:

public MyService() : this(ServerContext.Default) {}

public MyService(HttpContext httpContext)
       : this(ServerContext.GetContext(httpContext) {}

Now my web part can just call MyService(this.Context) and we still have a convenient default constructor for use in PowerShell. Because everything should be usable from PowerShell. This same idea can be applied to a number of common SharePoint tasks, particularly anywhere you’re using SPContext.Current, with benefits including improved reusability, maintainability and testability.

Do Not Dispose SPFeatureReceiverProperties.Feature.Parent

I can’t find a dedicated post about it, so I thought I’d draw attention to this thread in the MSDN forums where Michael Washam [MSFT] replied…

Do not dispose properties.Feature.Parent..

However, if you use .ParentWeb or anything else the same rules apply..

Now, the rule for SPWeb.ParentWeb is that it should not be disposed if you don’t own the child SPWeb, which we don’t, but you get the idea. It would be great if SPDisposeCheck would catch this—Roger?

I only point it out specifically because feature receiver examples are often posted online and I see Parent disposed all the time.

SPDisposeCheck Is Not A Shortcut

If you’re a SharePoint developer and you haven’t heard about the release of SPDisposeCheck, there are a number of blogs (like the SharePoint Team) you should be reading instead of (or hopefully in addition to) mine. Everyone is understandably enthusiastic—memory management is important and the rules are tricky. However, I’m afraid this is being viewed as something it’s not. To quote Tony Bierman‘s comment on Paul’s post:

I think there should be a “SPDisposeCheck Certified” logo.  This badge could be placed on the download pages and in the documentation of both free software and commercial software products as an indication of quality and adherence to best practices.

Let’s be clear: SPDisposeCheck is not definitive. It is a static analysis tool tuned to specific rules documented on the often-cited Patterns By Example. That post has been through perhaps a dozen iterations in the last year, updated as recently as last week. The team has no doubt put a lot of work into it (thanks for that), but there’s no reason to expect the guidance won’t be changed again.

Furthermore, there are countless perfectly safe code patterns, the so-called “false positives”, that will be reported as leaks. The documentation cites SPContext.Web.Site as an example, and I could probably rattle off a dozen more if pressed to do so. If the developer doesn’t investigate and just “fixes” every warning with using, more harm is done than good!

The tool will certainly be updated with the latest guidance, and the false positives will be addressed over time, but the core problem still remains: SPDisposeCheck will never know your code as well as you should. I’m sure thousands of leaks will be fixed because of it, and I’m not trying to diminish its value as part of a developer’s toolbox. Just don’t get caught thinking that passing SPDisposeCheck is a substitute for knowing what you’re doing.

That said, a big thanks to Roger, Stefan, Paul and the rest of the SPDisposeCheck team for their efforts to improve their documentation and the quality of our custom code.

Generic Method Invocation with Expression Trees

It’s against the rules and completely unsupported, but sometimes it’s just so much easier to use a base class’s private/internal members. Reflection has always been an option, but performance is less than ideal. Lightweight Code Generation is an option, but emitting IL isn’t for everyone. Since .NET 3.5 came out, there have been several discussions of using expression trees as a developer-friendly yet efficient alternative. There is an up-front cost to compile the expression into IL, but the resulting delegate can be reused with performance very close to direct invocation.

Alkampfer provides a great overview of expression tree method invocation in this article, which inspired this more general solution.

First, let’s set up a cache to store our compiled delegates. I didn’t put much effort into making it efficiently thread-safe, but suggestions are certainly welcome.

private static Dictionary<string, Delegate> accessors = new Dictionary<string, Delegate>();
private static object accessorLock = new object();
private static D GetCachedAccessor<D>(string key)
                 where D : class // Constraint cannot be special class 'System.Delegate'
{
    D result = null;
    Delegate cachedDelegate;
    lock (accessorLock)
    {
        if (accessors.TryGetValue(key, out cachedDelegate))
        {
            Debug.WriteLine("Found cache entry for " + key);
            result = cachedDelegate as D;
        }
    }
    return result;
}
private static void SetCachedAccessor(string key, Delegate value)
{
    if (value != null)
        lock (accessorLock)
        {
            accessors[key] = value;
        }
}

GetFieldAccessor

Now we can dive into our expression trees. As a warm-up, here’s a relatively simple cached field accessor, inspired by Roger Alsing‘s great post:

public static Func<T, R> GetFieldAccessor<T, R>(string fieldName)
{
    Type typeT = typeof(T);

    string key = string.Format("{0}.{1}", typeT.FullName, fieldName);
    Func<T, R> result = GetCachedAccessor<Func<T, R>>(key);

    if (result == null)
    {
        var param = Expression.Parameter(typeT, "obj");
        var member = Expression.PropertyOrField(param, fieldName);
        var lambda = Expression.Lambda<Func<T, R>>(member, param);

        Debug.WriteLine("Caching " + key + " : " + lambda.Body);
        result = lambda.Compile();
        SetCachedAccessor(key, result);
    }
    return result;
}

The method returns a function that will accept an object of type T and return its fieldName property with type R. For example, we can wrap this in an extension method to check if an SPWeb has been disposed:

public static bool GetIsClosed(this SPWeb web)
{
    return GetFieldAccessor<SPWeb, bool>("m_closed")(web);
}

Because the delegate is cached, successive calls of GetFieldAccessor() will immediately return the necessary delegate without recompilation.

GetMethodAccessor

Building a method accessor is a bit trickier because of the various combinations of parameter and return types. One option is to explicitly define overloads for various method signatures, as seen in the article referenced earlier. Instead, I figure we can let the caller specify the desired delegate signature and figure out the intended method based on that.

public static D GetMethodAccessor<D>(string methodName, BindingFlags bindingAttr)
                where D : class // Constraint cannot be special class 'System.Delegate'
{
    Type[] args = typeof(D).GetGenericArguments();
    Type objType = args[0];

    Type[] argTypes = args.Skip(1).ToArray();
    string[] argTypesArray = argTypes.Select(t => t.Name).ToArray();
    string key = string.Format("{0}.{1}({2})", objType.FullName, methodName, string.Join(",", argTypesArray));

    D result = GetCachedAccessor<D>(key);
    if (result == null)
    {
        MethodInfo mi = objType.GetMethod(methodName, bindingAttr, null, argTypes, null);

        if (mi == null || mi.ReturnType != typeof(void))
        {
            argTypes = argTypes.Take(argTypesArray.Length - 1).ToArray();
            mi = objType.GetMethod(methodName, bindingAttr, null, argTypes, null);
        }

        if (mi == null)
            throw new ArgumentException("Could not find appropriate overload.", methodName);

        var param = Expression.Parameter(objType, "obj");
        var arguments = argTypes.Select((t, i) => Expression.Parameter(t, "p" + i)).ToArray();
        var invoke = Expression.Call(param, mi, arguments);
        var lambda = Expression.Lambda<D>(invoke, param.AsEnumerable().Concat(arguments));

        Debug.WriteLine("Caching " + key + " : " + lambda.Body);
        result = lambda.Compile();
        SetCachedAccessor(key, result as Delegate);
    }
    return result;
}

As you can see, we depend heavily on the generic arguments of the delegate type. This means passing a closed delegate type to this function won’t work – it needs to be Func, Action, or something compatible with the expected argument structure. So what is that structure? The processing logic works like this:

  1. Take the first generic argument as the type whose method we are going to invoke.
  2. Fetch an array, skipping the first argument, that we pass to GetMethod as the argument types.
  3. If GetMethod can’t find an appropriate overload, or if the method’s return type is not void, then we shouldn’t have used all of the arguments as parameters.
  4. Redefine our parameter array without the last argument – this is our delegate’s non-void return type.
  5. Try GetMethod again with the trimmed array; throw if we still don’t find a match.

Once we have the details of our method, we can build the expression tree. I use the mapi-style Select overload to build an array of typed parameters named p0, p1, etc., which is then passed to Expression.Call to represent the method invocation. Finally, Expression.Lambda expects a list of all parameters including the instance param. Rather than allocate an intermediate data structure, I use a trick I picked up from Keith Rimington:

public static IEnumerable<T> AsEnumerable<T>(this T obj)
{
    yield return obj;
}

By turning param into a single-element IEnumerable<ParameterExpression>, we can simply Concat the rest of the arguments. Beautiful.

GetMethodAccessor Usage

The usage is a bit more complex then GetFieldAccessor, but still quite manageable:

public static bool SetBoolValue(this SPField field, string attrName, bool attrValue)
{
    Func<SPField, string, bool, bool> lambda =
        GetMethodAccessor<Func<SPField, string, bool, bool>>("SetFieldBoolValue",
                                                             BindingFlags.Instance | BindingFlags.NonPublic);
    return lambda(field, attrName, attrValue);
}

The intermediate variable is unnecessary, but makes it easier to see what’s going on. SPField.SetFieldBoolValue is of type Func<string, bool, bool>, so our delegate needs to be Func<SPField, string, bool, bool> to accept the instance variable first. The parameters for GetMethodAccessor are identical to what we would pass to field.GetType().GetMethod() if we were using normal reflection. Then we invoke lambda to effectively call field.SetFieldBoolValue(attrName, attrValue).

For methods that return void, we just pass an Action type instead:

public static void SetHidden(this SPField field, bool value)
{
    GetMethodAccessor<Action<SPField, bool>>("SetHidden", BindingFlags.Instance | BindingFlags.NonPublic)(field, value);
}

And these can be used like any other extension methods:

SPField field = GetField();
field.SetBoolValue("CanToggleHidden", !field.CanToggleHidden);
field.SetBoolValue("CanBeDeleted", !field.CanBeDeleted);
field.SetHidden(!field.Hidden);
field.Update();

Which will show the following in DebugView:

Caching Microsoft.SharePoint.SPField.SetFieldBoolValue(String,Boolean,Boolean) : obj.SetFieldBoolValue(p0, p1)
Found cache entry for Microsoft.SharePoint.SPField.SetFieldBoolValue(String,Boolean,Boolean)
Caching Microsoft.SharePoint.SPField.SetHidden(Boolean) : obj.SetHidden(p0)

Because of the penalty for compilation, this technique is not right for all situations. But for frequent access to inaccessible members, it might be worth a try.

LINQ Tip: Enumerable.OfType

In the past I’ve mentioned LINQ’s Cast<T>() as an efficient way to convert a SharePoint collection into an IEnumerable<T> that has access to LINQ’s various extension methods. Fundamentally, Cast<T>() is implemented like this:

public IEnumerable<T> Cast<T>(this IEnumerable source)
{
  foreach(object o in source)
    yield return (T) o;
}

Using an explicit cast performs well, but will result in an InvalidCastException if the cast fails. A less efficient yet useful variation on this idea is OfType<T>():

public IEnumerable<T> OfType<T>(this IEnumerable source)
{
  foreach(object o in source)
    if(o is T)
      yield return (T) o;
}

The returned enumeration will only include elements that can safely be cast to the specified type. Why would this be useful?

Example 1: SPWindowsServiceInstance

SharePoint, especially with MOSS, has several different services that can run on the various servers in a farm. We know where our web services are running, but where are the various windows services running?

var winsvc = from svr in SPFarm.Local.Servers
             from inst in svr.ServiceInstances.OfType<SPWindowsServiceInstance>()
             select new
             {
                 Server = svr.Name,
                 ID = inst.Id,
                 ServiceType = inst.Service.GetType().Name
             };

Example 2: SPDocumentLibrary

SharePoint provides a few special subclasses of SPList for specific kinds of lists. These include SPDocumentLibrary, SPPictureLibrary and the essentially obsolete SPIssueList. We can use OfType() to retrieve only lists of a certain type, like this LINQified MSDN sample that enumerates all files in a site collection’s libraries, excluding catalogs and form libraries:

SPSite site = SPContext.Current.Site;
var docs = from web in site.AllWebs.AsSafeEnumerable()
           from lib in web.Lists.OfType<SPDocumentLibrary>()
           from SPListItem doc in lib.Items
           where !lib.IsCatalog && lib.BaseTemplate != SPListTemplateType.XMLForm
           select new { WebTitle = web.Title, ListTitle = lib.Title,
                        ItemTitle = doc.Fields.ContainsField("Title") ? doc.Title : "" };

foreach (var doc in docs)
  Label1.Text += SPEncode.HtmlEncode(doc.WebTitle) + " -- " +
                 SPEncode.HtmlEncode(doc.ListTitle) + " -- " +
                 SPEncode.HtmlEncode(doc.ItemTitle) + "<BR>";

Example 3: SPFieldUser

Finally, let’s pull a list of all user fields attached to lists in the root web. This could also be used to easily find instances of a custom field type.

var userFields = from SPList list in site.RootWeb.Lists
                 from fld in list.Fields.OfType<SPFieldUser>()
                 select new
                 {
                     ListTitle = list.Title,
                     FieldTitle = fld.Title,
                     InternalName = fld.InternalName,
                     PresenceEnabled = fld.Presence
                 };

Contrived examples, perhaps, but potentially useful nonetheless.

Avoiding UserProfile.PersonalSite Leaks

Roger Lamb has posted another edge case in the pursuit of leak-free object model code: UserProfile.PersonalSite. To help follow his best practices, I offer another round of extension methods. First, for UserProfile:

public static void ProcessPersonalSite(this UserProfile profile, Action<SPSite> action)
{
    using (SPSite site = profile.PersonalSite)
    {
        action(site);
    }
}

public static T SelectFromPersonalSite<T>(this UserProfile profile, Func<SPSite, T> selector)
{
    using (SPSite site = profile.PersonalSite)
    {
        return selector(site);
    }
}

Usage:

void PersonalSiteNoLeak()
{
    // open a site collection
    using (SPSite siteCollection = new SPSite("http://moss"))
    {
        UserProfileManager profileManager = new UserProfileManager(ServerContext.GetContext(siteCollection));
        UserProfile profile = profileManager.GetUserProfile("domain\\username");
        profile.ProcessPersonalSite(personalSite =>
        {
            // Process personalSite
        });
    }
}

int CountContextPersonalSiteWebs()
{
    UserProfile myProfile = ProfileLoader.GetProfileLoader().GetUserProfile();
    return myProfile.SelectFromPersonalSite(personalSite =>
    {
        return personalSite.AllWebs.Count;
    });
}

I’ve also encapsulated the verification logic for accessing a MySite web part’s PersonalSite, with variations for projection and with/without exceptions:

public static void ProcessPersonalSite(this WebPart webpart, Action<SPSite> action)
{
    IPersonalPage personalPage = webpart.Page as IPersonalPage;
    if (personalPage == null)
        throw new SPException("Unable to access personal site. Invalid page.");
    if (personalPage.IsProfileError)
        throw new SPException("Unable to access personal site because of a profile error.");

    action(personalPage.PersonalSite);
}

public static T SelectFromPersonalSite<T>(this WebPart webpart, Func<SPSite, T> selector)
{
    IPersonalPage personalPage = webpart.Page as IPersonalPage;
    if (personalPage == null)
        throw new SPException("Unable to access personal site. Invalid page.");
    if (personalPage.IsProfileError)
        throw new SPException("Unable to access personal site because of a profile error.");

    return selector(personalPage.PersonalSite);
}

public static bool TryProcessPersonalSite(this WebPart webpart, Action<SPSite> action)
{
    IPersonalPage personalPage = webpart.Page as IPersonalPage;
    if (personalPage == null || personalPage.IsProfileError)
        return false;

    action(personalPage.PersonalSite);
    return true;
}

public static bool TrySelectFromPersonalSite<T>(this WebPart webpart, Func<SPSite, T> selector, out T value)
{
    value = default(T);
    IPersonalPage personalPage = webpart.Page as IPersonalPage;
    if (personalPage == null || personalPage.IsProfileError)
        return false;

    value = selector(personalPage.PersonalSite);
    return true;
}

Usage:

protected override void CreateChildControls()
{
    base.CreateChildControls();

    this.ProcessPersonalSite(personalSite =>
    {
        // Process personalSite
    });
}

As always, feedback is encouraged.

On The Road, Jan. 2009

I’m going to be on the road quite a bit (for me) in the next few weeks for various community events. Tonight I’m headed north to the inaugural meeting of the Cedar Valley .NET User Group. I’ll be giving a PowerShell talk for them in March. From there, I’m heading to Des Moines for Todd Klindt’s talk tomorrow at IADNUG.

On Jan. 24th I’ll be speaking at the Twin Cities SharePoint Camp:

SharePoint 3.5: Leveraging .NET 3.5 Language Features in SharePoint Development

The new versions of C# and VB add several tools to a SharePoint developer’s toolkit to enable more reliable, readable and efficient Object Model code. This session will review these features and provide some specific examples of how they can help you write better SharePoint code. I will also discuss SPSite and SPWeb disposal, and one strategy to minimize the risk of memory leaks.

And finally, on Feb. 4th the Iowa SharePoint User Group will again feature Kirk Hofer Corey Erkes:

SharePoint in Action

At this event you will see SharePoint In Action. Create it, Build it, Deploy it and Debug it. We will build a master page, custom style sheet, web part, feature receiver and stapler to demonstrate the basics in development for SharePoint. After we build it, we will deploy it to SharePoint and debug the whole thing.

Details:

Again, if you have ideas or requests for future IASPUG topics, please send them to info@sharepointia.com.

Posted in Community. Tags: , , , , , . Comments Off on On The Road, Jan. 2009

SPList.ParentWeb Leaks Revisited

A while back I looked into whether or not SPList.ParentWeb really needs to be disposed: Is SPList.ParentWeb a Leak? The specific motivation was to investigate why SPList.BreakRoleInheritance() requires Dispose() on ParentWeb property, as seen in Roger’s Dispose Patterns by Example. I stand by my original conclusion that this advice generally does more harm than good, but thought it would be useful to discuss in a bit more detail.

Why dispose ParentWeb?

As I showed before, the code for SPList.ParentWeb works like this:

if (SPUtility.StsCompareStrings(this.m_Lists.Web.ServerRelativeUrl, this.ParentWebUrl))
  return this.m_Lists.Web;

if (this.m_parentWeb == null)
  this.m_parentWeb = this.m_Lists.Web.Site.OpenWeb(this.ParentWebUrl);
return this.m_parentWeb;

In the vast majority of cases, the URLs will match and return the parent collection’s Web, which should not be disposed (unless, of course, you own it and know it is ready for disposal). Only in the exceptional case that the list’s ParentWebUrl indicates it doesn’t live with its parent collection will a new SPWeb be created. I believe it is this exception, rather than the norm, that leads to Roger’s suggestion that ParentWeb should be disposed.

On a curious and somewhat related note, SPListItem.Web doesn’t use ParentWeb:

public SPWeb get_Web()
{
    return this.m_Items.List.Lists.Web;
}

Why BreakRoleInheritance()?

Though BreakRoleInheritance() has been singled out, the real culprit is an internal property:

private SPSecurableObjectImpl get_SecurableObjectImpl()
{
    if (this.m_SecurableObjectImpl == null)
    {
        Guid guidScopeId = (Guid) this.m_arrListProps[0x22, this.m_iRow];
        bool hasUniquePerm = guidScopeId != this.ParentWeb.RoleAssignments.Id;
        this.m_SecurableObjectImpl = new SPSecurableObjectImpl(this.ParentWeb, this, ...);
    }
    return this.m_SecurableObjectImpl;
}

This property is then used in the implementations of several methods/properties from ISecurableObject:

  • SPList.AllRolesForCurrentUser
  • SPList.BreakRoleInheritance()
  • SPList.HasUniqueRoleAssignments
  • SPList.ResetRoleInheritance()
  • SPList.ReusableAcl
  • SPList.RoleAssignments

(Incidentally, SPSecurableObjectImpl is also used by SPWeb and SPListItem to implement that interface.)

SPList.get_SecurableObjectImplSo now we’re dealing with several more points of entry. For example, in SPListItem:

public ISecurableObject get_FirstUniqueAncestor()
{
    this.InitSecurity();
    if (this.RoleAssignments.Id == this.ParentList.RoleAssignments.Id)
    {
        return this.ParentList.FirstUniqueAncestor;
    }
    if (!this.HasUniqueRoleAssignments)
    {
        return this.Web.GetFolder(this.m_strPermUrl).Item;
    }
    return this;
}

I could show several more obscure code paths with the same result, but the specifics are irrelevant. My point is that trying to figure out if SPList.ParentWeb has been referenced is a wasted effort.

What to do?

The way I see it, we have two options:

  1. Assume that ParentWeb is safe enough and if we have memory problems later we can investigate.
  2. Assume that ParentWeb is leaky and figure out a safe way to Dispose() it.

I’m satisfied with the former, but if you’re not — or if you have a circumstance where the ParentWebUrl comparison actually fails (please share) — then your code should check if it needs to dispose first:

SPWeb web = SPContext.Current.Web;
SPList list;

try
{
  list = web.Lists["ListName"];
  list.BreakRoleInheritance(true);
}
finally
{
  if(list.ParentWeb != web)
    list.ParentWeb.Dispose();
}

Or we can use an extension method:

public static void DisposeParentWeb(this SPList list)
{
  if(list.ParentWeb != list.Lists.Web)
    list.ParentWeb.Dispose();
}

The Semi-Disposable PublishingWeb

As if handling SPSite and SPWeb isn’t tedious enough, MOSS’s PublishingWeb class throws another wrench into things. Its constructors are internal, so references to it always come from within the framework. I’m currently aware of five sources of these objects:

  1. PublishingWeb.GetPublishingWeb() wraps an existing SPWeb reference that should already have a disposal plan.
  2. PublishingWeb.GetVariation() creates an internal SPWeb that is disposed on Close().
  3. PublishingWeb.ParentPublishingWeb creates an internal SPWeb that is disposed on Close(), but also references its Web‘s ParentWeb, which may or may not need to be disposed.
  4. PublishingWebCollection creates an internal SPWebCollection from which it pulls SPWebs that are not disposed on Close() of the child PublishingWebs.
  5. PublishingWebCollection.Add() creates an internal SPWeb that is disposed on Close().

Internally, PublishingWeb is set up relatively well to handle the complexity of wrapping a Disposable object. It provides two constructors: one that accepts an SPWeb, which it stores internally but does not mark as “owned”; and one that accepts a Uri and SPSite, from which it opens an SPWeb (and SPSite if necessary) that are marked as “owned” to be cleaned up later. This job is performed by Close(), which is even kind enough to add a trace message (level Medium) if an attempt is made to close a non-owned web:

PublishingWeb.Close(): currentWeb is owned by caller, so doing nothing. May indicate SPWeb leak in calling code.

One could argue that PublishingWeb should really be IDisposable, calling Close() from Dispose() (like SPWeb does), but for now we can get by with try...finally.

The Good

The internal ownership mechanism serves its purpose wonderfully for GetPublishingWeb(), GetVariation() and PublishingWebCollection.Add(). The former just defers to the SPWeb-accepting constructor with no need to close, and the latter two use the Uri+SPSite constructor and can be closed without hesitation.

using(SPWeb web = site.OpenWeb())
{
  PublishingWeb varPubWeb = null
  PublishingWeb newPubWeb = null;
  try
  {
    PublishingWeb pubWeb = PublishingWeb.GetPublishingWeb(web);

    VariationLabel label = Variations.Current[0];
    varPubWeb = pubWeb.GetVariation(label);
    // Process varPubWeb

    PublishingWebCollection pubWebs = pubWeb.GetPublishingWebs();
    newPubWeb = pubWebs.Add("NewPubWeb");
  }
  finally
  {
    // pubWeb.Close() would generate a trace warning
    if(null != varPubWeb)
      varPubWeb.Close();
    if(null != newPubWeb)
      newPubWeb.Close();
  }
}

The Bad

Things get a bit trickier with ParentPublishingWeb for two reasons:

  1. We are subject to all the usual concerns when dealing with SPWeb.ParentWeb:

    This property will allocate an SPWeb object the first time it is called. The caveat is that once it is disposed, any reference to the property will return the disposed object. If an SPWeb is not owned by the developer, its ParentWeb should be considered not owned as well. For example, there could be a problem if two components both depend on SPContext.Current.Web.ParentWeb and one calls Dispose() before the other is done with it.

  2. Internally ParentPublishingWeb uses the “owned” constructor, so we need to call Close(). However, it gets the parameters for that constructor from its Web‘s ParentWeb property. If we’re in a situation where we own the original SPWeb, we also now own its ParentWeb and will need to Dispose() it.

In a case where we definitely own the web and its ParentWeb, we need to do something like this:

using(SPWeb web = site.OpenWeb())
{
  PublishingWeb parentPubWeb = null;
  try
  {
    PublishingWeb pubWeb = PublishingWeb.GetPublishingWeb(web);
    parentPubWeb = pubWeb.ParentPublishingWeb;
    // Process parentPubWeb
  }
  finally
  {
    if(null != parentPubWeb)
      parentPubWeb.Close();
    if(null != web.ParentWeb)
      web.ParentWeb.Dispose();
  }
}

The Ugly

Given how hard PublishingWeb tries to do the right thing, I’m surprised how little PublishingWebCollection does to help us. It too has an internal constructor, so we can only get a collection from PublishingWeb.GetPublishingWebs(), which passes in the current web’s GetSubwebsForCurrentUser(). This SPWebCollection is then used to build the PublishingWeb objects returned by indexing into or enumerating over the collection. However, unlike PublishingWebCollection.Add(), the indexers use GetPublishingWeb(SPWeb) and thus the “not my problem” constructor. So rather than rely on Close(), we need to manually call Dispose() on the wrapped Web:

using(SPWeb web = site.OpenWeb())
{
  PublishingWeb pubWeb = PublishingWeb.GetPublishingWeb(web);
  PublishingWebCollection pubWebs = pubWeb.GetPublishingWebs());
  foreach(PublishingWeb innerPubWeb in pubWebs)
  {
    try
    {
      // Process innerPubWeb
    }
    finally
    {
      // innerPubWeb.Close() would generate a trace warning
      innerPubWeb.Web.Dispose();
    }
  }
}

Update 1/29/2009: Roger Lamb has added this GetVariation() guidance to his Dispose Patterns by Example.

Elegant SPSite Elevation

A common difficulty with SPSecurity.RunWithElevatedPrivileges (most recently lamented here) is that existing SPSite/SPWeb objects will not work with the elevated security context. The natural workaround is to create a new SPSite within the elevated delegate and off you go. However, Dan Larson has documented a better technique here, leveraging the SPSite constructor that accepts an SPUserToken. By passing the token of the system account (SHAREPOINT\system, an internal alias for the application pool identity), an elevated site is available without the complexity and complications of RunWithElevatedPrivileges.

With the emerging best practice of separating SPSite and SPWeb creation from the code that uses them, I have expanded on Dan’s technique with another series of extension methods. The first step is to get the system token from an existing SPSite, adapted from Dan’s example:

public static SPUserToken GetSystemToken(this SPSite site)
{
    SPUserToken token = null;
    bool tempCADE = site.CatchAccessDeniedException;
    try
    {
        site.CatchAccessDeniedException = false;
        token = site.SystemAccount.UserToken;
    }
    catch (UnauthorizedAccessException)
    {
        SPSecurity.RunWithElevatedPrivileges(() =>
        {
            using (SPSite elevSite = new SPSite(site.ID))
                token = elevSite.SystemAccount.UserToken;
        });
    }
    finally
    {
        site.CatchAccessDeniedException = tempCADE;
    }
    return token;
}

And then we simply provide methods to operate on an elevated site created with that token:

public static void RunAsSystem(this SPSite site, Action<SPSite> action)
{
    using (SPSite elevSite = new SPSite(site.ID, site.GetSystemToken()))
        action(elevSite);
}

public static T SelectAsSystem<T>(this SPSite site, Func<SPSite, T> selector)
{
    using (SPSite elevSite = new SPSite(site.ID, site.GetSystemToken()))
        return selector(elevSite);
}

Note that the object(s) returned by selector should not be SharePoint objects that hold references to a parent site/web.

It’s also useful to provide additional methods that simplify working with elevated webs:

public static void RunAsSystem(this SPSite site, Guid webId, Action<SPWeb> action)
{
    site.RunAsSystem(s => action(s.OpenWeb(webId)));
}

public static void RunAsSystem(this SPSite site, string url, Action<SPWeb> action)
{
    site.RunAsSystem(s => action(s.OpenWeb(url)));
}

public static void RunAsSystem(this SPWeb web, Action<SPWeb> action)
{
    web.Site.RunAsSystem(web.ID, action);
}

public static T SelectAsSystem<T>(this SPSite site, Guid webId, Func<SPWeb, T> selector)
{
    return site.SelectAsSystem(s => selector(s.OpenWeb(webId)));
}

public static T SelectAsSystem<T>(this SPSite site, string url, Func<SPWeb, T> selector)
{
    return site.SelectAsSystem(s => selector(s.OpenWeb(url)));
}

public static T SelectAsSystem<T>(this SPWeb web, Func<SPWeb, T> selector)
{
    return web.Site.SelectAsSystem(web.ID, selector);
}

I don’t bother to Dispose the webs created by these calls to s.OpenWeb() because I know s will be disposed shortly, cleaning up the web with it.

Usage

As an example, let’s refactor Nigel’s skeleton code:

protected override void CreateChildControls()
{
    SPWeb web = SPControl.GetContextWeb(Context);
    web.RunAsSystem(UpdateVisitorList);
}

private void UpdateVisitorList(SPWeb web)
{
    SPList visList = web.Lists[_visitorList];
    int existItemId = GetVisitorItemId(visList);

    web.AllowUnsafeUpdates = true;
    if (existItemId < 1)
    {
        SPListItem newItem = visList.Items.Add();
        // Set field values
        newItem.Update();
    }
    else
    {
        SPListItem oldItem = visList.GetItemById(existItemId);
        // Update field values
        oldItem.Update();
    }
    web.AllowUnsafeUpdates = false;
}

By separating the elevated operation from the logic required to elevate, the code is much easier to read and less prone to errors like leaking an elevated SPSite.