Skip to content

Commit

Permalink
Merge branch 'feature/FilterByAttribute-key-validation' into develop
Browse files Browse the repository at this point in the history
Previously, a common scenario for using the `[FilterByAttribute()]` attribute was to filter by content type. As `ContentType` is no longer stored as an attribute (4a9f5b7), this no longer works. To mitigate this, we previously introduced the new `[FilterByContentType()]` attribute (105cfdd). Legacy implementations that previously used `[FilterByAttribute("ContentType", …)]`, however, would not get an error. In fact, it might even appear that it is working if their database contains legacy values for `ContentType` in `Topic.Attributes`. To help avoid errors and confusion, while offering better guidance for migrations, the `[FilterByAttribute()]` attribute has been updated to throw an `ArgumentException` if the implementation attempts to set the `Key` to `ContentType`. In this case, it will advise migrating to `[FilterByContentType()]` instead. Unfortunately, this doesn't offer design-time feedback, but it will offer runtime guidance that the existing behavior isn't working.
  • Loading branch information
JeremyCaney committed Feb 20, 2021
2 parents 13b1114 + 5b5dae0 commit c68040a
Show file tree
Hide file tree
Showing 3 changed files with 63 additions and 0 deletions.
19 changes: 19 additions & 0 deletions OnTopic.Tests/TopicMappingServiceTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -967,6 +967,25 @@ public async Task Map_FilterByAttribute_ReturnsFilteredCollection() {

}

/*==========================================================================================================================
| TEST: MAP: FILTER BY INVALID ATTRIBUTE: THROWS EXCEPTION
\-------------------------------------------------------------------------------------------------------------------------*/
/// <summary>
/// Attempts to map a view model that has an invalid <see cref="FilterByAttributeAttribute.Key"/> value of <c>ContentType
/// </c>; throws an <see cref="ArgumentException"/>.
/// </summary>
[TestMethod]
[ExpectedException(typeof(ArgumentException))]
public async Task Map_FilterByInvalidAttribute_ThrowsExceptions() {

var topic = TopicFactory.Create("Test", "FilteredInvalid");

var target = await _mappingService.MapAsync<FilteredInvalidTopicViewModel>(topic).ConfigureAwait(false);

Assert.AreEqual<int>(2, target.Children.Count);

}

/*==========================================================================================================================
| TEST: MAP: FILTER BY CONTENT TYPE: RETURNS FILTERED COLLECTION
\-------------------------------------------------------------------------------------------------------------------------*/
Expand Down
28 changes: 28 additions & 0 deletions OnTopic.Tests/ViewModels/FilteredInvalidTopicViewModel.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
/*==============================================================================================================================
| Author Ignia, LLC
| Client Ignia, LLC
| Project Topics Library
\=============================================================================================================================*/
using OnTopic.Mapping.Annotations;
using OnTopic.ViewModels;
using OnTopic.ViewModels.Collections;

namespace OnTopic.Tests.ViewModels {

/*============================================================================================================================
| VIEW MODEL: FILTERED TOPIC (INVALID)
\---------------------------------------------------------------------------------------------------------------------------*/
/// <summary>
/// Provides a strongly-typed data transfer object for testing views properties annotated with the <see cref="
/// FilterByAttributeAttribute"/>. Includes an invalid <see cref="FilterByAttributeAttribute"/>.
/// </summary>
/// <remarks>
/// This is a sample class intended for test purposes only; it is not designed for use in a production environment.
/// </remarks>
public class FilteredInvalidTopicViewModel {

[FilterByAttribute("ContentType", "Page")]
public TopicViewModelCollection<TopicViewModel> Children { get; } = new();

} //Class
} //Namespace
16 changes: 16 additions & 0 deletions OnTopic/Mapping/Annotations/FilterByAttributeAttribute.cs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,9 @@
| Project Topics Library
\=============================================================================================================================*/

using System;
using OnTopic.Internal.Diagnostics;

namespace OnTopic.Mapping.Annotations {

/*============================================================================================================================
Expand All @@ -30,9 +33,22 @@ public sealed class FilterByAttributeAttribute : System.Attribute {
/// <param name="key">The key of the attribute to filter by.</param>
/// <param name="value">The value of the attribute to filter by.</param>
public FilterByAttributeAttribute(string key, string value) {

/*------------------------------------------------------------------------------------------------------------------------
| Validate input
\-----------------------------------------------------------------------------------------------------------------------*/
TopicFactory.ValidateKey(key, false);
Contract.Requires<ArgumentException>(
!key.Equals("ContentType", StringComparison.OrdinalIgnoreCase),
"The ContentType is not stored as an attribute. To filter by ContentType, use [FilterByContentType] instead."
);

/*------------------------------------------------------------------------------------------------------------------------
| Set properties
\-----------------------------------------------------------------------------------------------------------------------*/
Key = key;
Value = value;

}

/*==========================================================================================================================
Expand Down

0 comments on commit c68040a

Please # to comment.