Use reference equality to compare model instances in EditContext (#18172) (#18649)

* Use reference equality to compare model instances in EditContext

Fixes https://github.com/aspnet/AspNetCore/issues/18069
This commit is contained in:
Pranav K 2020-02-14 08:34:44 -08:00 committed by GitHub
parent 1027e5372f
commit d7f98bd562
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 85 additions and 3 deletions

View File

@ -3,6 +3,7 @@
using System; using System;
using System.Linq.Expressions; using System.Linq.Expressions;
using System.Runtime.CompilerServices;
namespace Microsoft.AspNetCore.Components.Forms namespace Microsoft.AspNetCore.Components.Forms
{ {
@ -35,7 +36,7 @@ namespace Microsoft.AspNetCore.Components.Forms
/// <param name="fieldName">The name of the editable field.</param> /// <param name="fieldName">The name of the editable field.</param>
public FieldIdentifier(object model, string fieldName) public FieldIdentifier(object model, string fieldName)
{ {
if (model == null) if (model is null)
{ {
throw new ArgumentNullException(nameof(model)); throw new ArgumentNullException(nameof(model));
} }
@ -64,7 +65,16 @@ namespace Microsoft.AspNetCore.Components.Forms
/// <inheritdoc /> /// <inheritdoc />
public override int GetHashCode() public override int GetHashCode()
=> (Model, FieldName).GetHashCode(); {
// We want to compare Model instances by reference. RuntimeHelpers.GetHashCode returns identical hashes for equal object references (ignoring any `Equals`/`GetHashCode` overrides) which is what we want.
var modelHash = RuntimeHelpers.GetHashCode(Model);
var fieldHash = StringComparer.Ordinal.GetHashCode(FieldName);
return (
modelHash,
fieldHash
)
.GetHashCode();
}
/// <inheritdoc /> /// <inheritdoc />
public override bool Equals(object obj) public override bool Equals(object obj)
@ -75,7 +85,7 @@ namespace Microsoft.AspNetCore.Components.Forms
public bool Equals(FieldIdentifier otherIdentifier) public bool Equals(FieldIdentifier otherIdentifier)
{ {
return return
otherIdentifier.Model == Model && ReferenceEquals(otherIdentifier.Model, Model) &&
string.Equals(otherIdentifier.FieldName, FieldName, StringComparison.Ordinal); string.Equals(otherIdentifier.FieldName, FieldName, StringComparison.Ordinal);
} }

View File

@ -236,5 +236,35 @@ namespace Microsoft.AspNetCore.Components.Forms
Assert.False(isValid); Assert.False(isValid);
Assert.Equal(new[] { "Some message" }, editContext.GetValidationMessages()); Assert.Equal(new[] { "Some message" }, editContext.GetValidationMessages());
} }
[Fact]
public void LookingUpModel_ThatOverridesGetHashCodeAndEquals_Works()
{
// Test for https://github.com/aspnet/AspNetCore/issues/18069
// Arrange
var model = new EquatableModel();
var editContext = new EditContext(model);
editContext.NotifyFieldChanged(editContext.Field(nameof(EquatableModel.Property)));
model.Property = "new value";
Assert.True(editContext.IsModified(editContext.Field(nameof(EquatableModel.Property))));
}
class EquatableModel : IEquatable<EquatableModel>
{
public string Property { get; set; } = "";
public bool Equals(EquatableModel other)
{
return string.Equals(Property, other?.Property, StringComparison.Ordinal);
}
public override int GetHashCode()
{
return StringComparer.Ordinal.GetHashCode(Property);
}
}
} }
} }

View File

@ -76,6 +76,19 @@ namespace Microsoft.AspNetCore.Components.Forms
Assert.False(fieldIdentifier1.Equals(fieldIdentifier2)); Assert.False(fieldIdentifier1.Equals(fieldIdentifier2));
} }
[Fact]
public void FieldIdentifier_ForModelWithoutField_ProduceSameHashCodesAndEquality()
{
// Arrange
var model = new object();
var fieldIdentifier1 = new FieldIdentifier(model, fieldName: string.Empty);
var fieldIdentifier2 = new FieldIdentifier(model, fieldName: string.Empty);
// Act/Assert
Assert.Equal(fieldIdentifier1.GetHashCode(), fieldIdentifier2.GetHashCode());
Assert.True(fieldIdentifier1.Equals(fieldIdentifier2));
}
[Fact] [Fact]
public void SameContentsProduceSameHashCodesAndEquality() public void SameContentsProduceSameHashCodesAndEquality()
{ {
@ -89,6 +102,20 @@ namespace Microsoft.AspNetCore.Components.Forms
Assert.True(fieldIdentifier1.Equals(fieldIdentifier2)); Assert.True(fieldIdentifier1.Equals(fieldIdentifier2));
} }
[Fact]
public void SameContents_WithOverridenEqualsAndGetHashCode_ProduceSameHashCodesAndEquality()
{
// Arrange
var model = new EquatableModel();
var fieldIdentifier1 = new FieldIdentifier(model, nameof(EquatableModel.Property));
model.Property = "changed value"; // To show it makes no difference if the overridden `GetHashCode` result changes
var fieldIdentifier2 = new FieldIdentifier(model, nameof(EquatableModel.Property));
// Act/Assert
Assert.Equal(fieldIdentifier1.GetHashCode(), fieldIdentifier2.GetHashCode());
Assert.True(fieldIdentifier1.Equals(fieldIdentifier2));
}
[Fact] [Fact]
public void FieldNamesAreCaseSensitive() public void FieldNamesAreCaseSensitive()
{ {
@ -194,5 +221,20 @@ namespace Microsoft.AspNetCore.Components.Forms
{ {
public TestModel Child { get; set; } public TestModel Child { get; set; }
} }
class EquatableModel : IEquatable<EquatableModel>
{
public string Property { get; set; } = "";
public bool Equals(EquatableModel other)
{
return string.Equals(Property, other?.Property, StringComparison.Ordinal);
}
public override int GetHashCode()
{
return StringComparer.Ordinal.GetHashCode(Property);
}
}
} }
} }