Is It Valid to Have NonAction on Private Methods Inside a Controller in ASP.NET Core?

Short Answer

It's not really valid. Though it doesn't matter if both the private keyword and the NonAction attribute is applied when setting up actions because there's two steps that remove a method from being an eligible action:

1. Reflection is used to get only the public methods in a controller class, then
2. A filter for eligible actions runs and one of the cases is to filter out NonAction methods

It's best to just have one or the other. Though having both, while possibly against some sort of standard, will still work as expected.

Long Answer

During a pull request I was asked by the best JS and Git engineer I know why a private action needs NonAction as well. I spouted fragments of knowledge accumulated over time but later stopped and thought "well, I don't actually know how these two concepts work...". With .NET being open source, I dug in, gave a researched answer, and wrote it here.

We'll look into the ASP.NET Core source code to see why we should favour one over the other, but we don't have an issue if both are used. We'll be looking at DefaultApplicationModelProvider.cs under the MVC.Core source folder.

All our investigation can happen inside this foreach loop shown below:

// Point of interest 1: the source for this foreach
foreach (var methodInfo in controllerType.AsType().GetMethods())
{
    // Point of interest 2: CreateActionModel() which calls IsAction()
    var actionModel = CreateActionModel(controllerType, methodInfo);
    if (actionModel == null)
    {
        continue;
    }

    // further code omitted for brevity
}

Reflection

Below is the source for the foreach loop:

controllerType.AsType().GetMethods()

Abbreviated, this says "give me all of the public methods on a given controller type". Which straight away eliminates any private method or "private action" on a controller. Why does this only get public methods? This overload of Type.GetMethods() says:

An array of MethodInfo objects representing all the public methods defined for the current Type.

-or-

An empty array of type MethodInfo, if no public methods are defined for the current Type.

By this point, even if both private and NonAction are applied to an action, it's already filtered out. But let's still look at NonAction to understand how that comes into play to have a solid understanding.

IsAction()

The CreationActionModel() method in the above loop, makes a call to IsAction():

if (!IsAction(typeInfo, methodInfo))
{
    return null;
}

Inside IsAction() are various checks to weed out invalid actions such as if the methods are inherited from System.Object or if it's static. The check we're interested in is:

if (methodInfo.IsDefined(typeof(NonActionAttribute)))
{
    return false;
}

We see reflection again with CustomAttributeExtensions.IsDefined():

Returns true if an attribute of the specified type is applied to element; otherwise, false.

This returns false which causes CreationActionModel() to return null, thus discarding this method from being considered an action.

And if that's not enough, there is another check for being a public method as the final return:

return methodInfo.IsPublic;

To Conclude

While it's prudent to just have one or the other when you want to hide a method from being considered an action, it will not cause compilation or runtime problems.