ASP.NET MVC5 model not being validated

Question

I am using Data Annotations on my model for validations, but when I test the app, the model is considered valid, even if the form is completely empty. This leads to an exception since the registration method in the controller is referencing an object that doesn't exist (since the model wasn't supposed to get instantiated if the form was invalid).

This is my model

using System;
using System.Collections.Generic;
using System.ComponentModel.DataAnnotations;
using System.Linq;
using System.Web;

namespace MVCExample.Models
{
    public class Client
    {
        [Required(ErrorMessage="ID Cannot be empty.")]
        [RegularExpression(@"^(0\d-\d{4}-\d{4})|(\d-\d{3}-\d{6})$", ErrorMessage ="ID Format is Invalid.")]
        [StringLength(12, ErrorMessage="Length cannot be greater than 12.", MinimumLength = 12)]
        public string Id { get; set; }
        [Required(ErrorMessage = "Name cannot be empty.")]
        [StringLength(60, ErrorMessage = "Name must be between 10 and 60 characters.", MinimumLength = 10)]
        public string FullName { get; set; }
        [Required(ErrorMessage = "Phone number cannot be empty.")]
        [StringLength(9, ErrorMessage = "Phone number must have 9 characters.", MinimumLength = 9)]
        [RegularExpression(@"^\d{4}-\d{4}$", ErrorMessage ="Phone Format is incorrect.")]
        public string PhoneNumber { get; set; }
        [Required(ErrorMessage = "Must choose a discount option.")]
        
        public bool Discount { get; set; }
        [Required(ErrorMessage = "Invalid Amount.")]
        [RegularExpression(@"^\d+$", ErrorMessage ="Value must be a number.")]
        [Range(1, double.MaxValue, ErrorMessage ="Amount must be greater than 0.")]
        public decimal RecentPurchasesAmount { get; set; }
        [Required(ErrorMessage = "Invalid Amount.")]
        [RegularExpression(@"^\d+$", ErrorMessage = "Value must be a number.")]
        [Range(1, double.MaxValue, ErrorMessage ="Amount must be greater than 0.")]
        public decimal LastYearPurchases { get; set; }
        [Required(ErrorMessage = "Invalid Amount.")]
        [RegularExpression(@"^\d+$", ErrorMessage = "Value must be a number.")]
        [Range(1, int.MaxValue, ErrorMessage ="Amount must be greater than 0.")]
        public int AcUnitsPurchased { get; set; }

        public Client() { }

        public Client(string pId, string pName, string pPhone, bool pDisc, decimal pRecentPurchase, decimal pLastYearPurchase, int pUnits)
        {
            this.Id = pId;
            this.FullName = pName;
            this.PhoneNumber = pPhone;
            this.Discount = pDisc;
            this.RecentPurchasesAmount = pRecentPurchase;
            this.LastYearPurchases = pLastYearPurchase;
            this.AcUnitsPurchased = pUnits;

        }
    }
}

cshtml view

@model MVCExample.Models.Client

@{
    ViewBag.Title = "Example";
    ViewBag.Message = "Registration";
}

<h1 class="store-title">@ViewBag.Title</h1>
<h2 class="page-title">@ViewBag.Message</h2>
@using (Html.BeginForm("RegisterClient", "AppRegistration"))
{

    @Html.AntiForgeryToken();


    <label for="inp_registryId">Id</label>

    <input type="text" id="inp_registryId" name="Id" />
    //This is never showing up in the browser.
    @Html.ValidationMessageFor(model => model.Id, null, new { @class="text-danger"})

    <label for="inp_registryFullName">Full Name</label>
    <input type="text" id="inp_registryFullName" name="FullName" />

    <label for="inp_registryTelephone">Phone</label>
    <input type="text" id="inp_registryTelephone" name="Phone" />

    <div class="radio-box">
        <p>Client qualifies for discount</p>
        <label for="radioBtn_registryDiscountYes">Yes</label>
        <input type="radio" id="radioBtn_registryDiscountYes" value="true" name="radioBtnDiscount" />

        <label for="radioBtn_registryDiscountNo">No</label>
        <input type="radio" id="radioBtn_registryDiscountNo" value="false" name="radioBtnDiscount" />

    </div>

    <label for="inp_registryRecentPurchases">Amount for recent purchases</label>
    <input type="number" id="inp_registryRecentPurchases" name="RecentPurchases"/>

    <label for="inp_registryLastYearPurchases">Amount for last year purchases</label>
    <input type="number" id="inp_registryLastYearPurchases" name="LastYearPurchases" />

    <label for="inp_registryAcUnitsPurchased">Units purchased</label>
    <input type="number" id="inp_registryAcUnitsPurchased" name="UnitsPurchased" />

    <button type="submit" id="btnRegister">Register</button>

}

Controller method being called

[HttpPost]
        [ValidateAntiForgeryToken]
        
        public ActionResult RegisterClient(FormCollection collection)
        {
            try {
                //This code is executing even if the form is empty.
                if (ModelState.IsValid) { 
                ViewData["Id"] = collection["Id"];
                ViewData["FullName"] = collection["FullName"];
                ViewData["Phone"] = collection["Phone"];
                ViewData["Discount"] = collection["radioBtnDiscount"];
                ViewData["RecentPurchases"] = collection["RecentPurchases"];
                ViewData["LastYearPurchases"] = collection["LastYearPurchases"];
                ViewData["UnitsPurchased"] = collection["UnitsPurchased"];

                string id = ViewData["Id"].ToString();
                string fullName = ViewData["FullName"].ToString();
                string phone = ViewData["Phone"].ToString();
                /*Exceptions happen from this point, since it's trying to reference null values */
                bool discount = Boolean.Parse(ViewData["Discount"].ToString());
                decimal recentPurchases = Decimal.Parse(ViewData["RecentPurchases"].ToString());
                decimal lastYearPurchases = Decimal.Parse(ViewData["LastYearPurchases"].ToString());
                int unitsPurchased = Int32.Parse(ViewData["UnitsPurchased"].ToString());

                registeredClients.Add(new Client(id, fullName, phone, discount, recentPurchases, lastYearPurchases, unitsPurchased));
                    return View("Registration");
                } else
                {
                    Debug.WriteLine("Not registered.");
                    return View();
                }

                
            } catch(Exception ex)
            {
                Debug.WriteLine(ex.StackTrace);
                return View("Registration");
            }
        }

It's not just that the error messages aren't being printed on the page, the model itself is considered valid, even though every field is required.

Answer 1

ModelState.IsValid will alway be true sinice FormCollection object has no validation rules against it.

You should refactor your code to accept a Client object instead of FormCollection as a parameter into your RegisterClient method. This way - MVC will automatically instanciate the object for you and use the attributes to validate it.