Skip to content

Commit

Permalink
Merge branch 'feature/MemberAccessor' into develop
Browse files Browse the repository at this point in the history
Major refactoring of the internal `OnTopic.Internal.Reflection` classes. Previously, these classes relied on the .NET Reflection libraries to dynamically create delegate signatures for properties and methods before getting or setting values. Now, the delegate signature is created by the new `MemberAccessor` class and cached as part of a `TypeAccessorCache`. This promises to improve performance.

As part of this, I significantly refactored the organization and approach to the code. `TypeMemberInfoCollection` has been replaced by `TypeAccessorCache`, `MemberInfoCollection<T>` has been replaced by `TypeAccessor`, and `MemberDispatcher` has been replaced by `MemberAccessor`. This takes a more intuitive top-down approach, where as previously `MemberDispatcher` maintained its own `TypeMemberInfoCollection` cache which complicated the relationship between objects.

I also improved the validation of accessors, thus helping prevent unnecessary conversions, or attempts to set members that can't be set (e.g., by setting `null` to a non-nullable property).

In theory, this is supposed to have a major performance benefit—and, indeed, according to my initial testing directly against the new and old methods, it did. In real-world testing, however, we're not seeing that much benefit. That said, this also isn't any slower, and while it adds some complexity when calculating the delegate signatures, it's also much better organized.

This resolves Issue #90.
  • Loading branch information
JeremyCaney committed Dec 8, 2021
2 parents 34084fd + c084fe5 commit a82f7e6
Show file tree
Hide file tree
Showing 19 changed files with 2,132 additions and 1,352 deletions.
1 change: 0 additions & 1 deletion OnTopic.AspNetCore.Mvc/TopicViewResultExecutor.cs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
using Microsoft.AspNetCore.Mvc.Razor;
using Microsoft.AspNetCore.Mvc.ViewEngines;
using Microsoft.AspNetCore.Mvc.ViewFeatures;
using Microsoft.Extensions.Logging;
using Microsoft.Extensions.Options;
using OnTopic.Internal.Diagnostics;

Expand Down
51 changes: 51 additions & 0 deletions OnTopic.Tests/Fixtures/TypeAccessorFixture.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
/*==============================================================================================================================
| Author Ignia, LLC
| Client Ignia, LLC
| Project Topics Library
\=============================================================================================================================*/
using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Threading.Tasks;
using OnTopic.Data.Caching;
using OnTopic.Internal.Reflection;
using OnTopic.Lookup;
using OnTopic.Mapping;
using OnTopic.Repositories;
using OnTopic.TestDoubles;
using OnTopic.Tests.TestDoubles;
using OnTopic.ViewModels;

namespace OnTopic.Tests.Fixtures {

/*============================================================================================================================
| CLASS: TYPE ACCESSOR FIXTURE
\---------------------------------------------------------------------------------------------------------------------------*/
/// <summary>
/// Introduces a shared context to use for unit tests depending on an <see cref="TypeAccessor"/>.
/// </summary>
public class TypeAccessorFixture<T> {

/*==========================================================================================================================
| CONSTRUCTOR
\-------------------------------------------------------------------------------------------------------------------------*/
public TypeAccessorFixture() {

/*------------------------------------------------------------------------------------------------------------------------
| Create type accessor
\-----------------------------------------------------------------------------------------------------------------------*/
TypeAccessor = new TypeAccessor(typeof(T));

}

/*==========================================================================================================================
| TYPE ACCESSOR
\-------------------------------------------------------------------------------------------------------------------------*/
/// <summary>
/// A <see cref="TypeAccessor"/> for accessing <see cref="MemberAccessor"/> instances.
/// </summary>
internal TypeAccessor TypeAccessor { get; private set; }

}
}
410 changes: 410 additions & 0 deletions OnTopic.Tests/MemberAccessorTest.cs

Large diffs are not rendered by default.

6 changes: 3 additions & 3 deletions OnTopic.Tests/TopicMappingServiceTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1463,11 +1463,11 @@ await _mappingService.MapAsync<FilteredInvalidTopicViewModel>(topic).ConfigureAw

var topic = new Topic("Test", "Filtered", null, 5);

var target1 = (FilteredTopicViewModel?)await cachedMappingService.MapAsync<FilteredTopicViewModel>(topic).ConfigureAwait(false);
var target2 = (FilteredTopicViewModel?)await cachedMappingService.MapAsync<FilteredTopicViewModel>(topic).ConfigureAwait(false);
var target1 = await cachedMappingService.MapAsync<FilteredTopicViewModel>(topic).ConfigureAwait(false);
var target2 = await cachedMappingService.MapAsync<FilteredTopicViewModel>(topic).ConfigureAwait(false);
var target3 = (TopicViewModel?)await cachedMappingService.MapAsync<PageTopicViewModel>(topic).ConfigureAwait(false);

Assert.Equal<FilteredTopicViewModel?>(target1, target2);
Assert.Equal(target1, target2);
Assert.NotEqual<object?>(target1, target3);

}
Expand Down
6 changes: 4 additions & 2 deletions OnTopic.Tests/TopicReferenceCollectionTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -439,7 +439,8 @@ public class TopicReferenceCollectionTest {
| TEST: ADD: TOPIC REFERENCE WITH BUSINESS LOGIC: THROWS EXCEPTION
\-------------------------------------------------------------------------------------------------------------------------*/
/// <summary>
/// Sets a topic reference on a topic instance with an invalid value; ensures an exception is thrown.
/// Sets a topic reference on a topic instance with an invalid value; ensures an <see cref="ArgumentOutOfRangeException"/>
/// is thrown.
/// </summary>
[Fact]
public void Add_TopicReferenceWithBusinessLogic_ThrowsException() {
Expand All @@ -457,7 +458,8 @@ public class TopicReferenceCollectionTest {
| TEST: SET: TOPIC REFERENCE WITH BUSINESS LOGIC: THROWS EXCEPTION
\-------------------------------------------------------------------------------------------------------------------------*/
/// <summary>
/// Sets a topic reference on a topic instance with an invalid value; ensures an exception is thrown.
/// Sets a topic reference on a topic instance with an invalid value; ensures an <see cref="ArgumentOutOfRangeException"/>
/// is thrown.
/// </summary>
[Fact]
public void Set_TopicReferenceWithBusinessLogic_ThrowsException() {
Expand Down
Loading

0 comments on commit a82f7e6

Please sign in to comment.