Merge branch 'feature/FilterByAttribute-key-validation' into develop

JeremyCaney · JeremyCaney · commit c68040ae1971 · 2021-02-19T18:09:21.000-08:00
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.
diff --git a/OnTopic.Tests/TopicMappingServiceTest.cs b/OnTopic.Tests/TopicMappingServiceTest.cs
@@ -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
     \-------------------------------------------------------------------------------------------------------------------------*/
diff --git a/OnTopic.Tests/ViewModels/FilteredInvalidTopicViewModel.cs b/OnTopic.Tests/ViewModels/FilteredInvalidTopicViewModel.cs
@@ -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
diff --git a/OnTopic/Mapping/Annotations/FilterByAttributeAttribute.cs b/OnTopic/Mapping/Annotations/FilterByAttributeAttribute.cs
@@ -4,6 +4,9 @@
 | Project       Topics Library
 \=============================================================================================================================*/
 
+using System;
+using OnTopic.Internal.Diagnostics;
+
 namespace OnTopic.Mapping.Annotations {
 
   /*============================================================================================================================
@@ -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;
+
     }
 
     /*==========================================================================================================================
