Sitecore and FxCop – Accessing Raw Field Values

This is the second post in the Sitecore and FxCop series. Read the first post here. 

The second rule I came up with has to do with how to access raw field values.

Let’s say we want to grab the raw value of the Title field on the current item. There’s two different options we can use:

var fieldValue = Sitecore.Context.Item.Fields["Title"].Value;

Which really isn’t correct – what if there is no Title field? Or what if we have a typo in the fieldname? We’d end up with a NullReferenceException. To get around this we could use:

var field = Sitecore.Context.Item.Fields["Title"];
if (field != null)
{
    var fieldValue = field.Value;
}

The other option is:

var fieldValue = Sitecore.Context.Item["Title"];

In this case, if there is no Title field, or if we have a typo in the fieldname, we’d get an empty string.

To be absolutely clear, this is only to get the raw field value.  To display it on the front end side of things usually one would use a FieldRenderer instead anyway.

So, the second option is shorter and safer. Seems like a good thing to check for.

In the Rules.xml we need to add the following to our existing <Rules> definitions:

<Rule TypeName="AccessRawFieldValueProperly" Category="Sitecore.BestPractice" CheckId="SC002">
  <Name>Get the raw field value properly</Name>
  <Description>
    Accessing the raw field value through the Field property on the item can throw NullReferenceExceptions when
    the fieldname changes, the field doesn't exist or there's a misspelling in the fieldname.
  </Description>
  <Url />
  <Resolution>
    Raw field value is accessed by Item.Fields["Fieldname"].Value. Raw field values should be accessed by Item["fieldname"]
  </Resolution>
  <Email />
  <MessageLevel Certainty="70">Error</MessageLevel>
  <FixCategories>Breaking</FixCategories>
  <Owner />
</Rule>

As always, the TypeName needs to match the name of the class.

internal sealed class AccessRawFieldValueProperly : BaseRule
{
  public AccessRawFieldValueProperly() : base("AccesssRawFieldValueProperly")
  {
  }

  public override ProblemCollection Check(Member member)
  {
    var method = member as Method;
    if (method != null)
    {
      VisitStatements(method.Body.Statements);
    }

    return this.Problems;
  }

  public override void VisitMethodCall(MethodCall methodCall)
  {
    var memberBinding = methodCall.Callee as MemberBinding;
    if (memberBinding != null)
    {
      var methodCalled = memberBinding.BoundMember as Method;
      if (methodCalled != null)
      {
        if (methodCalled.FullName == "Sitecore.Data.Fields.Field.get_Value")
        {
          Problems.Add(new Problem(GetResolution(), methodCall));
        }
      }
    }

    base.VisitMethodCall(methodCall);
  }
}

Then, when I have a solution with the following code:

public void SomeMethod()
{
  var rawFieldValue1 = Sitecore.Context.Item.Fields["fieldname"].Value;
  var rawFieldValue2 = Sitecore.Context.Item["fieldname"];
  var field = Sitecore.Context.Fields["fieldname"];
}

I would expect to get one error in FxCop. I still want to be able to grab the actual field of course.

When I have a couple more rules in here I’m thinking about releasing this to the Sitecore Marketplace, so your input is very welcome. Is there any rule you would like to see added?

Advertisements

Sitecore and FxCop

I’ve been looking to create some FxCop rules for Sitecore development to get easier overviews to check (and possible enforce) using best practices on Sitecore development. This is the first post in a series of  Sitecore specific rules, which eventually (when there’s a couple of useful rules) will be released through the Sitecore Marketplace as well.

Creating custom rules for FxCop isn’t that hard, especially with some great resources such as this post. Following that post, there’s a couple of simple steps which boil down to the following:

Create a new class library project. The project will of course require references to the FxCop assemblies, FxCopSdk.dll and Microsoft.Cci.dll.

After that, create a rule definition XML, a BaseRule and a custom Rule which inherits BaseRule. The custom Rule can now contain some code that will show if the code is abiding by the rules or breaking them.

Before I give an example of my custom rules, please note that it’s very important to set the Build Action for the XML file as Embedded Resource, otherwise the error ‘Analysis was not performed; at least one valid rules assembly and one valid target file must be specified’ will be thrown.

Oops…

I would strongly advise having a read through the blog post to get a better understanding what’s going on.

Sitecore rules

I’ve created a baseclass called BaseRule, which is exactly the same as in the blog mentioned earlier so I won’t describe that one. As for the rule I’ll describe here, I want to make sure that the GetItem method is called using a GUID, not a path.

To do this, I’ve created a Rules.xml which looks like this:

<?xml version="1.0" encoding="utf-8" ?>
<Rules>
  <Rule TypeName="GetItemUsingID" Category="Sitecore.BestPractice" CheckId="SC0001">
    <Name>Enforce calling the GetItem method using ID</Name>
    <Description>
      Use GUIDs where possible instead of path / name. This will improve performance, as well as prevent any breakage if you move content to a new location in the content tree.
    </Description>
    <Url />
    <Resolution>GetItem is called passing in {0}. GetItem should be called passing in an ID.</Resolution>
    <Email />
    <MessageLevel Certainty="80">Error</MessageLevel>
    <FixCategories>NonBreaking</FixCategories>
    <Owner />
  </Rule>
</Rules>

A couple clarifications: I’ve put the certainty on 80 for now, since all I’m doing is checking if there’s a string parameter that does not have a GUID. I completely ignore things like queries, empty IDs and the like.

Also, the Resolution message could be a little clearer, pointing out where the GetItem method gets called for instance, but for the purpose of this example it’s clear enough.

namespace Sitecore.FxCop
{
  using System;
  using System.Collections.Generic;
  using System.Text.RegularExpressions;
  using Microsoft.FxCop.Sdk;

  internal sealed class GetItemUsingID : BaseRule
  {
    public GetItemUsingID() : base("GetItemUsingID")
    {
      public override ProblemCollection Check(Member member)
      {
        Method method = member as Method;
        if (method != null)
        {
          this.Visit(method.Body);
        }

        return this.Problems;
      }

      public override void VisitMethodCall(MethodCall call)
      {
        base.VisitMethodCall(call);

        Method targetMethod = (Method)((MemberBinding)call.Callee).BoundMember;
        if (targetMethod.DeclaringType.FullName.Equals("Sitecore.Data.Database", StringComparison.Ordinal) && targetMethod.Name.Name.Equals("GetItem", StringComparison.Ordinal))
        {
          Expression endResponse = call.Operands["0"];
          if (endResponse.NodeType = NodeType.Literal)
          {
            var value = ((Literal)endResponse).Value.ToString();
            var match = Regex.IsMatch(value, @"\b[A-F0-9]{8}(?:-[A-F0-9]{4}){3}-[A-F0-9]{12}\b", RegexOptions.IgnoreCase);
            if (!match)
            {
              this.Problems.Add(new Problem(this.GetResolution(value, call))
            }
          }
        }
      }
    }
  }
}

A couple of interesting things here:

• On line 8: The name of the class needs to be exactly the same as what’s defined in the XML definition.
• On line 30: we only check the first parameter (so the parameter at index 0). This is because the GetItem method, when a path or GUID is used, will always have that path or GUID as the 0th parameter.
• On line 31: we are only interested in that parameter when it’s a string. Remember, the first parameter for GetItem is either string, DataUri or ID. We could check them all of course, but right now I just want to make sure the first parameter is a string, and contains a GUID
• On line 37: I pass in the value of the string that gets passed to GetItem, which will replace the {0} token defined in the XML with that value

So now consider the following code:

  var database = Sitecore.Data.Database.GetDatabase("master");
  var item1 = database.GetItem("/sitecore/content/home");
  var item2 = database.GetItem("{4E7AB8D1-6A39-4C8C-BF5B-816F8105BFFD}");

When I then add my custom rule to be inspected by FxCop, I get the following:

A grand start, although I would still only get one message about GetItem even if I would have multiple instances of calling GetItem with a path in the same method.

Which FxCop rules would you like to have regarding Sitecore best practices?