How to re-use the Action Method validation among multiple Controller classes

Question

i am working on an asp.net mvc-5 web application with entity framework-6.and i have mapped my DB tables using entity framework which generates a .edmx file containing a DBContext class. and currently i have two controller classes one for managing servers and the other for managing vms. When add/edit Server or VMs I want to check if their ip & mac addresses already exists. currently i am going these checks on the action method itself as follow:-

public class Server : Controller{
    [HttpPost]
     [ValidateAntiForgeryToken]
     public ActionResult Create(ServerJoin sj)
            {
                bool ITipunique = repository.ISITIPUnique(vmj.NetworkInfo2.FirstOrDefault().IPADDRESS);
                bool ITmacunique = repository.ISITMACUnique(vmj.NetworkInfo2.FirstOrDefault().MACADDRESS);
                bool Tipunique = repository.ISTIPUnique(vmj.NetworkInfo2.FirstOrDefault().IPADDRESS);
                bool Tmacunique = repository.ISTMACUnique(vmj.NetworkInfo2.FirstOrDefault().MACADDRESS);

                try
                {

                    if ((sj.IsIPUnique == true) && (!ITipunique || !Tipunique))
                    {

                        ModelState.AddModelError("NetworkInfo2[0].IPAddress", "Error occurred. The Same IP is already assigned.");

                    }
                    if ((sj.IsMACUnique == true) && (!ITmacunique || !Tmacunique))
                    {

                        ModelState.AddModelError("NetworkInfo2[0].MACAddress", "Error occurred. The Same MAC Address is already assigned.");

                    }

&

public class VM : Controlelr {    
    [HttpPost]
    [ValidateAntiForgeryToken]

            public ActionResult Create(VMJoin vmj)
            {

                bool ITipunique = repository.ISITIPUnique(vmj.NetworkInfo2.FirstOrDefault().IPADDRESS);
                bool ITmacunique = repository.ISITMACUnique(vmj.NetworkInfo2.FirstOrDefault().MACADDRESS);
                bool Tipunique = repository.ISTIPUnique(vmj.NetworkInfo2.FirstOrDefault().IPADDRESS);
                bool Tmacunique = repository.ISTMACUnique(vmj.NetworkInfo2.FirstOrDefault().MACADDRESS);
                try
                {
                    if ((vmj.IsIPUnique == true) && (!ITipunique || !Tipunique))
                    {

                        ModelState.AddModelError("NetworkInfo2[0].IPAddress", "Error occurred. The Same IP is already assigned.");

                    }
                    if ((vmj.IsMACUnique == true) && (!ITmacunique || !Tmacunique))
                    {

                        ModelState.AddModelError("NetworkInfo2[0].MACAddress", "Error occurred. The Same MAC Address is already assigned.");

                    }
                    if (!repository.IshypervisorServers(vmj.VirtualMachine.ServerID))
                    {
                        ModelState.AddModelError("VirtualMachine.ServerID", "Error occurred. Please select a valid hypervisor server. ");
                    }

this approach is working well, but i am facing a problem is that i have to repeat these validations on all the related action methods mainly (add & edit) and inside other controller classes (server, vms, storage device, etc...) so is there a way to manage the shared validation in a better way , which facilitate re-use and maintainability ?

EDIT

ServerJoin is as follow:-

public class ServerJoin : IValidatableObject
    {
        public Server Server { get; set; }
        public Resource Resource { get; set; }
        public Technology Technology { get; set; }
        public SDOrganization Site { get; set; }
        public SDOrganization Customer { get; set; }
        public NetworkInfo NetworkInfo { get; set; }
        public ICollection<NetworkInfo> NetworkInfo2 { get; set; }
        [Display(Name="Unique")]
        public bool IsMACUnique { get; set; }
        [Display(Name = "Unique")]
        public bool IsIPUnique { get; set; }
        public Nullable<double> SPEED { get; set; }
        public Nullable<Int64> PROCESSORCOUNT { get; set; }
        [Display(Name = "IP Unique")]
        public bool IsTIPUnique { get; set; }
        [Display(Name = "MAC Unique")]
        public bool IsTMACUnique { get; set; }
        public IEnumerable<ValidationResult> Validate(ValidationContext validationContext)
        {
            if (Server != null)
            {


                if (Server.RackUnitID != null && Server.RackID == null)
                {
                    yield return new ValidationResult("Please select a Rack, or remove the current Rack Unit", new[] { "Server.RackUnitID" });
                }
                if (Server.RackUnitIDTo != null && Server.RackUnitID == null)
                {
                    yield return new ValidationResult("Please select a Rack From Value", new[] { "Server.RackUnitID" });
                }
                if (Server.RackUnitIDTo != null && Server.RackUnitID != null && Server.RackUnitID > Server.RackUnitIDTo)
                {
                    yield return new ValidationResult("Rack Unit From must be less than or equal Rack Unit To", new[] { "Server.RackUnitID" });
                }


            }

&

 public class VMJoin
    {
         public VirtualMachine VirtualMachine { get; set; }
         public Resource Resource { get; set; }
         public Technology Technology { get; set; }
         public SDOrganization Site { get; set; }
         public SDOrganization Customer { get; set; }
         public NetworkInfo NetworkInfo { get; set; }
         public ICollection<NetworkInfo> NetworkInfo2 { get; set; }
         public ICollection<TechnologyIP> TechnologyIP { get; set; }
         [Display(Name = "Unique")]
         public bool IsMACUnique { get; set; }
         [Display(Name = "Unique")]
         public bool IsIPUnique { get; set; }
         public Nullable<double> SPEED { get; set; }
         public TechnologyIP TechnologyIP2 { get; set; }
         [Display(Name = "IP Unique")]
         public bool IsTIPUnique  { get; set; }
         [Display(Name = "MAC Unique")]
         public bool IsTMACUnique { get; set; }

    }
}

`vmj.NetworkInfo2.FirstOrDefault().IPADDRESS`: quite an anti-pattern. It will throw a poorly helpful NullReferenceException in case of empty collection (and if NetworkInfo is a `class`, not a `struct`). It should be `vmj.NetworkInfo2.First().IPADDRESS` in order to get a way more meaningful error in case of empty collection. — Frédéric, Apr 20 '16 at 12:03

score 5 · Accepted Answer · edited Apr 20 '16 at 11:04

Start by creating a base class for your models with the common properties which will also reduce the code within the concrete classes.

public class BaseModel
{
    public bool IsIPUnique { get; set; }
    public bool IsMACUnique { get; set; }
    .... // other common properties
}
public class ServerJoin : BaseModel
{
    .... //  properties specific to ServerJoin 
}
public class VMJoin : BaseModel
{
    .... //  properties specific to VMJoin
}

and create a base controller for the common validation code

public class BaseController : Controller
{
    public void Validate(BaseModel model)
    {
        bool ITipunique = repository.ISITIPUnique(vmj.NetworkInfo2.FirstOrDefault().IPADDRESS);
        ....
        if ((model.IsIPUnique == true) && (!ITipunique || !Tipunique))
        {
            ModelState.AddModelError("NetworkInfo2[0].IPAddress", "Error occurred. The Same IP is already assigned.");
        }
        if ((model.IsMACUnique == true) && (!ITmacunique || !Tmacunique))
        {
            ModelState.AddModelError("NetworkInfo2[0].MACAddress", "Error occurred. The Same MAC Address is already assigned.");
        }
        .... // other common validation  
    }
}

and then on the specific controllers

public class ServerController : BaseController
{
    [HttpPost]
    [ValidateAntiForgeryToken]
    public ActionResult Create(ServerJoin sj)
    {
        Validate(sj); // adds the ModelStateErrors
        if (!ModelState.IsValid)
        {
            return View(sj);
        }
        ....
    }
}

public class VMController : BaseController
{
    [HttpPost]
    [ValidateAntiForgeryToken]
    public ActionResult Create(VMJoin vmj)
    {
        Validate(vmj); // adds the ModelStateErrors
        if (!ModelState.IsValid)
        {
            return View(vmj);
        }
        ....
    }
}

although your approach will require a major change inside my code,, but seems will save a lot of time and effort in extending and maintaining my application. but i have a question on my you created the Base class as a controller ? as usually Controller classes are used for users to access their action methods,, so is there any drawbacks of defining the BaseController as a normal class rather than being a controller by itself ? — John John, Apr 20 '16 at 10:21
Its just extending the standard MVC `Controller` class to add an additional method (and you could also include a protected property or field for the repository so it can be accessed in all controllers that derive from it). There is no difference from using the standard `Controller` class - you just derive all your controllers that need access to the common method from `BaseController` rather than `Controller` — , Apr 20 '16 at 10:27

user2216 · Answer 2 · 2016-04-14T16:18:07.170

3

If VMJoin and ServerJoin have the same interface you can just create extension method with ModelState as a second parameter.

Update Here is example of extension method

    public static void TestMethod<T>(this T context, ModelStateDictionary modelsState) where T : YourDbBaseClass
    {
        //check context
        //add errors if exist
        modelsState.AddModelError("Error", "Big Error");
    }

    //Usage
    TestMethod<YourDbContext>(ModelState);

edited Apr 14 '16 at 16:18

answered Apr 14 '16 at 15:45

user2216

809
1
8
24

can you adivce what do u exactly mean ? now VMjoin and ServerJoin represents different context object (different DB tables) – John John Apr 14 '16 at 15:46
1

@johnG I updated the answer. I hope it will help you, but it is just how I saw your problem and my solution. If you add more details about these contexts it will help us understand more clearly. – user2216 Apr 14 '16 at 16:00
thanks for the update,, but how i will be using these inside my Server and VM controller classes ,, i did not get your point ? – John John Apr 14 '16 at 16:05
for the context object i have mapped my DB tables using Entity Framework-6 which generates a .edmx file which contain a DBContext class – John John Apr 14 '16 at 16:06
@johnG Just define extension method in the other class (any static class) and you can call this method from controller like `vmj.TestMethod(ModelState)`. – user2216 Apr 14 '16 at 16:14
1

I provided just an example and sure, you should replace code in the method with your validation. – user2216 Apr 14 '16 at 16:17
but how i can access the vmjoin and serverjoin properties inside the TestMethod ? i mean how i can get the current ip address ,, currently inside the action method i am doing the following vmj.NetworkInfo2.FirstOrDefault().IPADDRESS – John John Apr 14 '16 at 16:20
1

@johnG could you please provide the code of VMJoin or ServerJoin? You can just add their declarations with base classes. – user2216 Apr 14 '16 at 16:24
ok i edited my question and i provided part of the serverjoin and vmjoin classes – John John Apr 14 '16 at 16:31
@johnG Thanks. Unfortunately, I see now that extension method cannot help you. – user2216 Apr 14 '16 at 16:43
can you adivce why it can not help ? – John John Apr 14 '16 at 23:49

Pavel · Answer 3 · 2016-04-22T15:08:30.433

In my opinion the cleanest way to do this is using custom validation data annotation attribute.

You simply create custom attribute

public class IPUniqueValidator : ValidationAttribute
{
    public string ShouldCheck { get; set; }

    protected override ValidationResult IsValid(object value, ValidationContext validationContext)
    {
        if (value != null)
        {
            var IP = value.ToString();
            var isValid = false;

            // Using reflection to get the other property value 
            var shouldCheckPropert = validationContext.ObjectInstance.GetType().GetProperty(this.ShouldCheck);
            var shouldCheckPropertValue = (bool)shouldCheckPropert.GetValue(validationContext.ObjectInstance, null);
            if (shouldCheckPropertValue)
            {
                // do validation code...
            }

            if (isValid)
            {
                return ValidationResult.Success;
            }
            else
            {
                return new ValidationResult("Error occurred. The Same IP is already assigned.");
            }
        }
        else
        {
            return new ValidationResult("" + validationContext.DisplayName + " is required");
        }
    }
}

Mark your models with new attribute:

public class VMJoin
{
    [IPUniqueValidator(ShouldCheck = "ShouldCheck")]
    public string IpAddress { get; set; }

    public bool ShouldCheck { get; set; }
}

public class ServerJoin
{
    [IPUniqueValidator(ShouldCheck = "ShouldCheck")]
    public string IpAddress { get; set; }

    public bool ShouldCheck { get; set; }
}

And it left to you only add check for validation state. The framework will do all work for you.

[HttpPost]
[ValidateAntiForgeryToken]
public ActionResult Create(ServerJoin sj)
{
    if (ModelState.IsValid)
    {
        // do staff 
    }
}

your approach seems valid, but as seen inside my original code, the user have the option to select if he wants to do the validation or not. i mean on the view there is a checkbox which allow user to specify if he want to check if the IP is unique or not as follow:- "<<( if ((model.IsMACUnique == true) && (!ITmacunique || !Tmacunique)))>>" . so not sure if this can be achieved using custom validation data annotation? as if i use the custom validation data annotation attribute it will always do the check,, and i can not pass to it additional field value (the checkbox selection in my case) — John John, Apr 22 '16 at 14:06
You have access to other properties of current object instance. validationContext.ObjectInstance.GetType().GetProperty("PropertyName") I could extend my answer with usage example. — Pavel, Apr 22 '16 at 14:52
I added example how to get the other property from validation method — Pavel, Apr 22 '16 at 15:09

score 0 · Answer 4 · edited May 23 '17 at 10:28

0

I would go with a custom ActionFilter. Check out how to access ModelState from an action filter here:

How do I access the ModelState from an ActionFilter?

You can also setup dependency injection within the custom action filter to receive the required repositories, further increasing testability/maintainability.

edited May 23 '17 at 10:28

Community

1
1

answered Apr 20 '16 at 11:53

DrinkBird

834
8
17

How to re-use the Action Method validation among multiple Controller classes

4 Answers4