Purpose of public NonAction methods in MVC

19,507

Solution 1

(I restructured the answer to better address the questions in the comments)

I think, the attribute is here only for better flexibility. As a framework designer, one wants to relax coding constraints off the end user as much as possible. Requirement of not having public non-actions may sound good "in general" but may be too restrictive for some projects. Adding [NonAction] solves their problem (introduced by their bad design though) - and obviously you're not forced to use the attribute, so it's a win-win from a framework designer perspective.

Another reason may be legacy - in the earlier MVC versions only methods marked with [Action] where considered as actions. So when they relaxed the requirement (and all public methods became treated as actions) they kept [NonAction] so that developers won't get too confused.


In general, using NonAction is a bad practice - exactly for the reasons you stated. If something shouldn't be an action, it should not be public in the first place.

Problem with public non-action methods on the controller is that they make people tempted to instantiate your controller and call the method, instead of separating out the common logic:

Compare

public class MyController : IController
{
    public ActionResult Foo(long orderId)
    {
        var order = new OrdersController().GetOrder(orderId); //GetOrder is public
        ...
    }
}

with

public class MyController : IController
{
    public ActionResult Foo(long orderId)
    {
        var order = _orderService.GetOrder(orderId);
        ...
    }
}

The first approach leads to increased coupling between controllers and non-straightforward code in the actions. Code becomes difficult to follow and refactor, and cumbersome to mock/test.

Besides increased coupling, any public non-action method is a security hole - if you forget to mark it with [NonAction] (or, better, change away from public) - because it's treated as normal action and can be invoked externally. I know the original question kinda implies you surely would never forget to attach the attribute if needed, but it's also kinda important to understand what can happen if you would ;) Oh well, and as we're on this, it seems to me that "forgetting the attribute" is more theoretically probable, comparing to "forgetting to make the method private".


Sometimes people say having public non-actions is necessary for unit testing, but again, when something is not an action it most likely can be isolated in a separate class and tested separately. Moreover, even if it's not feasible for whatever reason, marking a method public for testing purposes only is a bad habit - using internal and InternalsVisibleTo is the recommended way.

Solution 2

This kind of situation may be caused by requirements some testing framework such as you need to do unit testing on that method then you to expose it although its a bad design but can't change these had to bear it out.

By default, the MVC framework treats all public methods of a controller class as action methods. If your controller class contains a public method and you do not want it to be an action method, you must mark that method with the NonActionAttributeattribute.

Real purpose to use public NonAction

To restrict access to non-action method to notify MVC framework that given controller method is not action.

When you try to run a method with NonAction attribute over URL you get the error 404 as response to request.

Ref: http://msdn.microsoft.com/en-us/library/dd410269%28v=vs.90%29.aspx

For Detail: http://weblogs.asp.net/gunnarpeipman/archive/2011/04/09/asp-net-mvc-using-nonactionattribute-to-restrict-access-to-public-methods-of-controller.aspx

Share:
19,507
ChandniShah
Author by

ChandniShah

Updated on June 05, 2022

Comments

  • ChandniShah
    ChandniShah almost 2 years

    i have just started working in MVC and I have one doubt.

    Instead of Nonaction method , we can create private method in controller or we can also write method in model and call that from controller.

    So , what is the real purpose to use public NonAction method in MVC ?

  • ChandniShah
    ChandniShah almost 11 years
    I know how to use NonActionAttribute. My real question is what is a real reason to use Nonaction method.
  • Satpal
    Satpal almost 11 years
    @ChandniShah, When you try to run a method with NonAction attribute over URL you get the error 404 as response to request.
  • ChandniShah
    ChandniShah almost 11 years
    But in place of public non action method , I can use private method also . in that case also I will get error. so why I need to use puble non action method ?
  • Satpal
    Satpal almost 11 years
    @ChandniShah, At times we may need public methods on controllers for some reasons.
  • ChandniShah
    ChandniShah almost 11 years
    I want to know that 'Some resons' only.
  • Imad Alazani
    Imad Alazani almost 11 years
    Good for recommending it private instead of Non Action Method as it can be used in intermediate operation of Action Methods like function calling. Now, they cannot be called outside that controller. So far very good. Question - Why should i use Non-Action Method ? Can you please tell the significance of it's usage ?
  • andreister
    andreister almost 11 years
    "Why should I use [NonAction]" - You don't have to use it. My take is that the attribute exists because the folks who created ASP.NET MVC wanted to make the framework available for poorly designed projects, too.
  • Imad Alazani
    Imad Alazani almost 11 years
    ok Thanks. But what are those bad habits included while using Non Action method ?
  • andreister
    andreister almost 11 years
    Are you asking, why it's bad to have public non-action methods on the controller? First off, it makes others tempted to instantiate your controller and call the method (instead of separating out the common logic), which leads to increased coupling between controllers and non-straightforward code in the actions. Code becomes difficult to follow and cumbersome to mock/test. Besides that, any public non-action method is a security hole - if you forget to mark it with NonAction (or, better, change away from public) - because it's treated as normal action and can be invoked externally.
  • Imad Alazani
    Imad Alazani almost 11 years
    if you forget to mark it with NonAction (or, better, change away from public) - because it's treated as normal action and can be invoked externally. This is not looking good :). Question is sepcific to Non Action. then why will i forget it. :)
  • Imad Alazani
    Imad Alazani almost 11 years
    First off, it makes others tempted to instantiate your controller This do happens while preparing unit test cases ? Are u satisfied with your justification ? Sorry for asking so many queries
  • andreister
    andreister almost 11 years
    Nah they're good questions, no need to excuse. I restructured the answer to better address them - let me know if it's more clear now. Also, I don't understand your "preparing unit test cases" - do you mean public non-action methods are useful there or what?
  • Imad Alazani
    Imad Alazani almost 11 years
    So, What's the advantage of using Non Action in Controller ?
  • Duk
    Duk almost 11 years
    You can use non action methods for long-running, non-CPU bound requests. This avoids blocking the Web server from performing work while the request is being processed.
  • Oliver
    Oliver over 10 years
    Your answer made me clean up my controller and I now have an external service class for two previously public controller methods. Looks like [NonAction] in this case would have just helped me keep my architecture dirty. Good I didn't need to use it, really.
  • Peter
    Peter over 10 years
    I have a case where I have a base controller that has methods like MessageBox() which takes params and uses the MVC controller Json() method and props like ControllerContext. Now I have a case where I need to use MessageBox with the same param values in multiple places. So I'm thinking in this case make the method public NonAction instead of a protected method would make sense. I can't split out the logic because of all the controller props it uses. A util class which takes a MyBaseController param would allow me to wrap the repeated functionality. What do commenters think of this case?
  • andreister
    andreister over 10 years
    Looks like using public MessageBox and the base class is fine, no?