Skip to content

Commit db0983c

Browse files
committed
Merge branch 'bugfix/AttributeBindingModel.Value-required' into release/5.1.0
In a previous update, we changed the syntax for non-nullable properties to use the null-forgiving operator instead of the nullable attributes (e.g., `[NotNull]`, `[DisallowNull]`). These should be equivalent, but the ASP.NET Core model validation syntax doesn't treat them as such. As a result, it started throwing model validation exceptions for `Value` when a radio button wasn't selected, in which case no value is set. Technically, this should have been a long-standing bug; the new behavior is actually correct. This exposes an issue that the `Value` _should_ be nullable, for the specific case of radio buttons. By marking this nullable, some downstream calls in `EditorController` raised potential null dereference warnings. In both cases, we would never expect the value to be null, since those types (`Key`, relationships) don't use radio buttons. As such, these were both decorated with the null-forgiving operator as well; if they are _not_ null, a dereference exception _should_ occur—though we never expect this to occur, so we're not _explicitly_ checking for this scenario.
2 parents b635bf8 + 2f6ec9b commit db0983c

2 files changed

Lines changed: 4 additions & 4 deletions

File tree

OnTopic.Editor.AspNetCore/Controllers/EditorController.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -438,7 +438,7 @@ public async Task<IActionResult> Edit(
438438
SetReference(topic, attribute, attributeValue);
439439
}
440440
else if (attribute.Key is "Key") {
441-
topic.Key = attributeValue.Value.Replace(" ", "", StringComparison.Ordinal);
441+
topic.Key = attributeValue.Value!.Replace(" ", "", StringComparison.Ordinal);
442442
}
443443
else if (topic.BaseTopic is null && String.IsNullOrEmpty(attributeValue.Value)) {
444444
topic.Attributes.SetValue(attribute.Key, attribute.DefaultValue);
@@ -471,7 +471,7 @@ public async Task<IActionResult> Edit(
471471
/// Private helper function that saves relationship values to the topic.
472472
/// </summary>
473473
private void SetRelationships(Topic topic, AttributeDescriptor attribute, AttributeBindingModel attributeValue) {
474-
var relatedTopics = attributeValue.Value.Split(',', StringSplitOptions.RemoveEmptyEntries).ToList();
474+
var relatedTopics = attributeValue.Value!.Split(',', StringSplitOptions.RemoveEmptyEntries).ToList();
475475
topic.Relationships.Clear(attribute.Key);
476476
foreach (var topicIdString in relatedTopics) {
477477
Topic? relatedTopic = GetAssociatedTopic(topicIdString);

OnTopic.Editor.AspNetCore/Models/AttributeBindingModel.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ public AttributeBindingModel(string editorType) {
5858
/// <summary>
5959
/// The value associated with the attribute.
6060
/// </summary>
61-
public string Value { get; init; } = default!;
61+
public string? Value { get; init; } = default!;
6262

6363
/*==========================================================================================================================
6464
| GET VALUE
@@ -71,7 +71,7 @@ public AttributeBindingModel(string editorType) {
7171
/// <see cref="GetValue()"/> method is intended to be overwritten by derived versions of the <see cref="AttributeBindingModel"/>
7272
/// class, in order to provide specific serialization instructions.
7373
/// </remarks>
74-
public virtual string GetValue() => Value;
74+
public virtual string GetValue() => Value?? "";
7575

7676
} // Class
7777
} // Namespace

0 commit comments

Comments
 (0)