对现有和未来代码影响最小的 Bug 修复
有时候，一个 bug 修复无法通过回归测试的挑战。因此，为了更好地解决问题和修复错误，进行深思熟虑是值得的。
引言
许多开发者可能会认为，解决问题和修复错误比纯粹的开发更具挑战性。是的，即使有非常好的架构和源代码也是如此。
背景
我被分配了一个 bug，该 bug 指出，建筑商和承包商的入口和占用许可证不应该是强制性的。
该 Web 项目是一个基于 ASP.NET MVC 和 Code-First Entity Framework 的许可证审批系统，目前支持四种许可证申请
- 建筑和土地利用
- 占用
- 入口
- Sign
验证后的页面 UI 如下图所示。建筑商和承包商的数据库表有四个对应的必填列。
代码和分析
Contractor 模型类看起来像这样
[MetadataType(typeof(ContractorMetadata))]
public partial class Contractor
{
}
public class ContractorMetadata
{
[Display(ResourceType = typeof(Rsc), Name = "ContactNameLabel")]
[Required(ErrorMessageResourceName =
"ErrorFieldRequiredMessage", ErrorMessageResourceType = typeof(Rsc))]
public string ContactName { get; set; }
[Display(ResourceType = typeof(Rsc), Name = "BusinessNameLabel")]
[Required(ErrorMessageResourceName =
"ErrorFieldRequiredMessage", ErrorMessageResourceType = typeof(Rsc))]
public string BusinessName { get; set; }
[Display(ResourceType = typeof(Rsc), Name = "PhoneCountryCodeLabel")]
public int? PhoneCountryCode { get; set; }
[Display(ResourceType = typeof(Rsc), Name = "PhoneAreaCodeLabel")]
[RegularExpression(@"^(|[0-9]{3,7})$",
ErrorMessageResourceName = "PhoneAreaCodeInvalid",
ErrorMessageResourceType = typeof(Rsc))]
public int? PhoneAreaCode { get; set; }
[Display(ResourceType = typeof(Rsc), Name = "PhoneNumberLabel")]
[Required(ErrorMessageResourceName = "ErrorFieldRequiredMessage",
ErrorMessageResourceType = typeof(Rsc))]
[RegularExpression(@"^(|[0-9]{3}[-. ]?[0-9]{3}[ ]?[0-9][0-9]{0,2}[ ]?[0-9]{0,3}[ ]?[0-9]{0,3})$",
ErrorMessageResourceName = "PhoneNumberInvalid",
ErrorMessageResourceType = typeof(Rsc))]
public string PhoneNumber { get; set; }
[Display(ResourceType = typeof(Rsc), Name = "PhoneExtLabel")]
[RegularExpression(@"^(|[0-9]{0,6})$",
ErrorMessageResourceName = "PhoneExtensionInvalid",
ErrorMessageResourceType = typeof(Rsc))]
public int? PhoneExtension { get; set; }
[Display(ResourceType = typeof(Rsc), Name = "MobilePhoneCountryCodeLabel")]
public long? MobilePhoneCountryCode { get; set; }
[Display(ResourceType = typeof(Rsc), Name = "MobilePhoneAreaCodeLabel")]
[RegularExpression(@"^(|[0-9]{3,7})$",
ErrorMessageResourceName = "PhoneAreaCodeInvalid",
ErrorMessageResourceType = typeof(Rsc))]
public long? MobilePhoneAreaCode { get; set; }
[Display(ResourceType = typeof(Rsc), Name = "MobilePhoneNumberLabel")]
[Required(ErrorMessageResourceName = "ErrorFieldRequiredMessage",
ErrorMessageResourceType = typeof(Rsc))]
[RegularExpression(@"^(|[0-9]{3}[-. ]?[0-9]{3}[ ]?[0-9][0-9]{0,2}[ ]?[0-9]{0,3}[ ]?[0-9]{0,3})$",
ErrorMessageResourceName = "PhoneNumberInvalid",
ErrorMessageResourceType = typeof(Rsc))]
public string MobilePhoneNumber { get; set; }
[Display(ResourceType = typeof(Rsc), Name = "MobilePhoneExtLabel")]
public int? MobilePhoneExtension { get; set; }
}
如果建筑和土地利用许可证和标牌许可证不需要建筑商和承包商的任何信息（实际上确实不需要），或者他们不强制要求这些信息，那么有一个简单的修复方法：只需注释掉那些 Required 注释，并修改数据库表，以便这些列允许 null 值。
当然，修复将在重新测试时得到验证并关闭 bug。这是一个快速修复，让我们放松一下。
但是等等
- 为什么，作为一个负责任的开发者，你要允许
Contractor表在用户没有输入任何内容的情况下填充空行？ - 如果用户只输入电话号码，而没有输入联系人姓名或公司名称，那么表中的新行就毫无意义了，怎么办？
- 如果建筑和土地利用许可证和标牌许可证对建筑商和承包商的要求变为强制性的，怎么办？
- 如果添加了一个新的许可证类型，并且有一些特殊要求，怎么办？
糟了，现在不是放松的时候。让我们回去吧。
首先，我向模型类 Contractor 添加了一个新属性，用于标记是否满足必需属性，如下所示
[MetadataType(typeof(ContractorMetadata))]
public partial class Contractor
{
public bool RequiredPropertiesFulfilled { get; set; }
}
由于这个新属性只是一个帮助程序，用于说明 Contractor 数据是否良好，并且它与数据库无关，因此它应该在映射类中被忽略
public class ContractorMap : EntityTypeConfiguration<contractor>
{
public ContractorMap()
{
this.HasKey(t => t.ContractorId);
this.Property(t => t.ContactName)
.IsRequired()
.HasMaxLength(200);
this.Property(t => t.BusinessName)
.IsRequired()
.HasMaxLength(200);
this.Property(t => t.PhoneNumber)
.IsRequired()
.HasMaxLength(15);
......
this.Ignore(t => t.RequiredPropertiesFulfilled);
}
}
然后，我们需要一个自定义模型绑定器，以使模型 Required 注释对不同的许可证类型有条件地起作用
public class CustomModelBinder : DefaultModelBinder
{
public override object BindModel
(ControllerContext controllerContext, ModelBindingContext bindingContext)
{
var modelName = bindingContext.ModelName;
Type modelType = bindingContext.ModelType;
if (modelType == typeof(Models.Contractor))
{
var permitType = bindingContext.ModelName.Split('.')[0];
//the mvc view is intentionally added with HtmlTemplatePrefix so that
//bindingContext.ModelName looks like
//Encroachment.DescriptionOfActivity.Contractors[0]
if (permitType != null && (
permitType.Equals("Encroachment",
StringComparison.CurrentCultureIgnoreCase) ||
permitType.Equals("Entrance",
StringComparison.CurrentCultureIgnoreCase))
)
{
var contractor = base.BindModel(controllerContext, bindingContext);
if (bindingContext.ModelState.IsValid)
{
//if user entered all the required info, just let it go.
(contractor as Models.Contractor).RequiredPropertiesFulfilled =
true;
}
else
{
var contractorKeys = bindingContext.ModelState.Keys.Where
(x => x.Contains("Contractors"));
if (contractorKeys != null && contractorKeys.Count() > 0)
{
foreach (var k in contractorKeys)
{
bindingContext.ModelState[k].Errors.Clear();
}
}
//check if required fields have been entered or not.
//If not, mark it as not fulfilled, and remove it by
//function PurgeApplicationDataContractors
//before application is submitted.
FlagDescriptionDataContractorFulfilledness
(contractor as Models.Contractor, controllerContext,
bindingContext);
}
return contractor;
}
}
return base.BindModel(controllerContext, bindingContext);
}
}
除了其他原因，选择一个自定义模型绑定器来修复此问题的一个非常重要的原因是它有一个 ModelBindingContext 参数传递给我们，以便我们可以轻松获取必需的属性
private IList<string>
GetBindingContextModelRequiredKeys(ModelBindingContext bindingContext)
{
var requiredKeys = new List<string>();
foreach (var k in bindingContext.PropertyMetadata.Keys)
{
if (bindingContext.PropertyMetadata[k].ModelType == typeof(short) ||
bindingContext.PropertyMetadata[k].ModelType == typeof(long)) continue;
if (bindingContext.PropertyMetadata[k].IsRequired)
{
requiredKeys.Add(k);
}
}
return requiredKeys;
}
如果输入了所有有效信息的 Contractor，则只需保存它。如果未满足必需条件，我们需要在进一步操作之前做更多事情。
我们使用以下函数来确定 Contractor 模型是否已满足
private void FlagDescriptionDataContractorFulfilledness
(Models.Contractor contractor,
ControllerContext controllerContext, ModelBindingContext bindingContext)
{
if (contractor == null) return;
var requiredKeys = GetBindingContextModelRequiredKeys(bindingContext);
var nonFulfilledProperties = contractor.GetType().GetProperties().Where
(x => x.GetValue(contractor) == null).Select(x => x.Name);
var intersect = requiredKeys.Intersect(nonFulfilledProperties);
contractor.RequiredPropertiesFulfilled = intersect == null;
}
然后，在保存到数据库之前，删除那些未输入必需信息的承包商，这样就不会将无意义的承包商信息填充到 Contractor 表中
private void PurgeApplicationDataContractors(IEnumerable<Models.Contractor> contractors)
{
if (contractors == null) return;
for (var i = contractors.Count; i > 0; i--)
{
var c = contractors[i - 1];
if (!c.RequiredPropertiesFulfilled) contractors.Remove(c);
}
}
不要忘记将自定义绑定器添加到应用程序模型绑定器字典中
protected void Application_Start()
{
ModelBinders.Binders.Add(typeof(Models.Contractor), new CustomModelBinder());
}
现在，我们可以看到它完美地工作，对现有代码几乎没有影响。我们不必删除必需属性（从逻辑角度来看，必需性是有意义的）。如果在将来，其他许可证类型需要承包商，并且它需要承包商的必需属性，则无需更改任何内容；如果它不强制要求承包商，则可以将许可证类型轻松添加到模型绑定器中的 if 条件中，并且它将按预期继续正常工作。
通过此修复，我们在可预见的未来将不再有任何担忧。
有人可能会争辩说，可以继承 IValidatableObject 来进行自定义验证。但我还没有找到一种方法来区分许可证类型，并从模型类中获取承包商的必需属性。虽然可以使用会话变量来携带许可证类型名称，但似乎没有一种简单的方法来获取在相关映射类中定义的必需属性。
关注点
ASP.NET MVC ModelBinder 非常强大。它通过 ControllerContext 和 ModelBindingContext 携带了大量信息，供开发者将自己的逻辑写入代码中。