Skip to content

Commit 66f4e91

Browse files
authored
Use reference equality to compare model instances in EditContext (#18172)
* Use reference equality to compare model instances in EditContext Fixes #18069
1 parent 97a59f1 commit 66f4e91

File tree

3 files changed

+85
-3
lines changed

3 files changed

+85
-3
lines changed

src/Components/Forms/src/FieldIdentifier.cs

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33

44
using System;
55
using System.Linq.Expressions;
6+
using System.Runtime.CompilerServices;
67

78
namespace Microsoft.AspNetCore.Components.Forms
89
{
@@ -35,7 +36,7 @@ public static FieldIdentifier Create<TField>(Expression<Func<TField>> accessor)
3536
/// <param name="fieldName">The name of the editable field.</param>
3637
public FieldIdentifier(object model, string fieldName)
3738
{
38-
if (model == null)
39+
if (model is null)
3940
{
4041
throw new ArgumentNullException(nameof(model));
4142
}
@@ -64,7 +65,16 @@ public FieldIdentifier(object model, string fieldName)
6465

6566
/// <inheritdoc />
6667
public override int GetHashCode()
67-
=> (Model, FieldName).GetHashCode();
68+
{
69+
// 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.
70+
var modelHash = RuntimeHelpers.GetHashCode(Model);
71+
var fieldHash = StringComparer.Ordinal.GetHashCode(FieldName);
72+
return (
73+
modelHash,
74+
fieldHash
75+
)
76+
.GetHashCode();
77+
}
6878

6979
/// <inheritdoc />
7080
public override bool Equals(object obj)
@@ -75,7 +85,7 @@ public override bool Equals(object obj)
7585
public bool Equals(FieldIdentifier otherIdentifier)
7686
{
7787
return
78-
otherIdentifier.Model == Model &&
88+
ReferenceEquals(otherIdentifier.Model, Model) &&
7989
string.Equals(otherIdentifier.FieldName, FieldName, StringComparison.Ordinal);
8090
}
8191

src/Components/Forms/test/EditContextTest.cs

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -236,5 +236,35 @@ public void RequestsValidationWhenValidateIsCalled()
236236
Assert.False(isValid);
237237
Assert.Equal(new[] { "Some message" }, editContext.GetValidationMessages());
238238
}
239+
240+
[Fact]
241+
public void LookingUpModel_ThatOverridesGetHashCodeAndEquals_Works()
242+
{
243+
// Test for https://github.com/aspnet/AspNetCore/issues/18069
244+
// Arrange
245+
var model = new EquatableModel();
246+
var editContext = new EditContext(model);
247+
248+
editContext.NotifyFieldChanged(editContext.Field(nameof(EquatableModel.Property)));
249+
250+
model.Property = "new value";
251+
252+
Assert.True(editContext.IsModified(editContext.Field(nameof(EquatableModel.Property))));
253+
}
254+
255+
class EquatableModel : IEquatable<EquatableModel>
256+
{
257+
public string Property { get; set; } = "";
258+
259+
public bool Equals(EquatableModel other)
260+
{
261+
return string.Equals(Property, other?.Property, StringComparison.Ordinal);
262+
}
263+
264+
public override int GetHashCode()
265+
{
266+
return StringComparer.Ordinal.GetHashCode(Property);
267+
}
268+
}
239269
}
240270
}

src/Components/Forms/test/FieldIdentifierTest.cs

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,19 @@ public void DistinctFieldNamesProduceDistinctHashCodesAndNonEquality()
7676
Assert.False(fieldIdentifier1.Equals(fieldIdentifier2));
7777
}
7878

79+
[Fact]
80+
public void FieldIdentifier_ForModelWithoutField_ProduceSameHashCodesAndEquality()
81+
{
82+
// Arrange
83+
var model = new object();
84+
var fieldIdentifier1 = new FieldIdentifier(model, fieldName: string.Empty);
85+
var fieldIdentifier2 = new FieldIdentifier(model, fieldName: string.Empty);
86+
87+
// Act/Assert
88+
Assert.Equal(fieldIdentifier1.GetHashCode(), fieldIdentifier2.GetHashCode());
89+
Assert.True(fieldIdentifier1.Equals(fieldIdentifier2));
90+
}
91+
7992
[Fact]
8093
public void SameContentsProduceSameHashCodesAndEquality()
8194
{
@@ -89,6 +102,20 @@ public void SameContentsProduceSameHashCodesAndEquality()
89102
Assert.True(fieldIdentifier1.Equals(fieldIdentifier2));
90103
}
91104

105+
[Fact]
106+
public void SameContents_WithOverridenEqualsAndGetHashCode_ProduceSameHashCodesAndEquality()
107+
{
108+
// Arrange
109+
var model = new EquatableModel();
110+
var fieldIdentifier1 = new FieldIdentifier(model, nameof(EquatableModel.Property));
111+
model.Property = "changed value"; // To show it makes no difference if the overridden `GetHashCode` result changes
112+
var fieldIdentifier2 = new FieldIdentifier(model, nameof(EquatableModel.Property));
113+
114+
// Act/Assert
115+
Assert.Equal(fieldIdentifier1.GetHashCode(), fieldIdentifier2.GetHashCode());
116+
Assert.True(fieldIdentifier1.Equals(fieldIdentifier2));
117+
}
118+
92119
[Fact]
93120
public void FieldNamesAreCaseSensitive()
94121
{
@@ -194,5 +221,20 @@ class ParentModel
194221
{
195222
public TestModel Child { get; set; }
196223
}
224+
225+
class EquatableModel : IEquatable<EquatableModel>
226+
{
227+
public string Property { get; set; } = "";
228+
229+
public bool Equals(EquatableModel other)
230+
{
231+
return string.Equals(Property, other?.Property, StringComparison.Ordinal);
232+
}
233+
234+
public override int GetHashCode()
235+
{
236+
return StringComparer.Ordinal.GetHashCode(Property);
237+
}
238+
}
197239
}
198240
}

0 commit comments

Comments
 (0)