Skip to content
This repository was archived by the owner on Dec 19, 2018. It is now read-only.

Commit 8f2e2a2

Browse files
author
NTaylorMullen
committed
Addressed code review comments
1 parent c3b9717 commit 8f2e2a2

File tree

11 files changed

+242
-165
lines changed

11 files changed

+242
-165
lines changed

‎src/Microsoft.AspNet.Razor.Runtime/Properties/Resources.Designer.cs‎

Lines changed: 16 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎src/Microsoft.AspNet.Razor.Runtime/Resources.resx‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -125,4 +125,7 @@
125125
<data name="TagHelperTypeResolver_AddTagHelperAssemblyNameCannotBeEmptyOrNull" xml:space="preserve">
126126
<value>`@addtaghelper` assembly name cannot be null or empty.</value>
127127
</data>
128+
<data name="TagHelperTypeResolver_CannotResolveTagHelperAssembly" xml:space="preserve">
129+
<value>Cannot resolve TagHelper containing assembly '{0}'. Error: '{1}'.</value>
130+
</data>
128131
</root>

‎src/Microsoft.AspNet.Razor.Runtime/TagHelpers/TagHelperDescriptorFactory.cs‎

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -26,19 +26,19 @@ public static class TagHelperDescriptorFactory
2626
/// <returns>A <see cref="TagHelperDescriptor"/> that represents the given <paramref name="type"/>.</returns>
2727
public static TagHelperDescriptor CreateDescriptor(Type type)
2828
{
29-
var tagName = GetTagNameTarget(type);
30-
var tagHelperTypeName = type.FullName;
31-
var attributeDescriptors = GetTagHelperAttributeDescriptors(type);
29+
var tagName = GetTagName(type);
30+
var typeName = type.FullName;
31+
var attributeDescriptors = GetAttributeDescriptors(type);
3232
var contentBehavior = GetContentBehavior(type);
3333

3434
return new TagHelperDescriptor(tagName,
35-
tagHelperTypeName,
35+
typeName,
3636
contentBehavior,
3737
attributeDescriptors);
3838
}
3939

40-
// TODO: Make this method support TagNameAttribute targets: https://github.com/aspnet/Razor/issues/120
41-
private static string GetTagNameTarget(Type tagHelperType)
40+
// TODO: Make this method support TagNameAttribute tag names: https://github.com/aspnet/Razor/issues/120
41+
private static string GetTagName(Type tagHelperType)
4242
{
4343
var name = tagHelperType.Name;
4444

@@ -50,18 +50,18 @@ private static string GetTagNameTarget(Type tagHelperType)
5050
return name;
5151
}
5252

53-
private static IEnumerable<TagHelperAttributeDescriptor> GetTagHelperAttributeDescriptors(Type type)
53+
private static IEnumerable<TagHelperAttributeDescriptor> GetAttributeDescriptors(Type type)
5454
{
5555
var typeInfo = type.GetTypeInfo();
56-
var properties = typeInfo.DeclaredProperties.Where(IsValidTagHelperProperty);
57-
var attributeDescriptors = properties.Select(ToTagHelperAttributeDescriptor);
56+
var properties = typeInfo.DeclaredProperties.Where(IsValidProperty);
57+
var attributeDescriptors = properties.Select(ToAttributeDescriptor);
5858

5959
return attributeDescriptors;
6060
}
6161

6262
// TODO: Make the HTML attribute name support names from a AttributeNameAttribute:
6363
// https://github.com/aspnet/Razor/issues/121
64-
private static TagHelperAttributeDescriptor ToTagHelperAttributeDescriptor(PropertyInfo property)
64+
private static TagHelperAttributeDescriptor ToAttributeDescriptor(PropertyInfo property)
6565
{
6666
return new TagHelperAttributeDescriptor(property.Name, property);
6767
}
@@ -72,7 +72,7 @@ private static ContentBehavior GetContentBehavior(Type type)
7272
return ContentBehavior.None;
7373
}
7474

75-
private static bool IsValidTagHelperProperty(PropertyInfo property)
75+
private static bool IsValidProperty(PropertyInfo property)
7676
{
7777
return property.GetMethod != null &&
7878
property.GetMethod.IsPublic &&

‎src/Microsoft.AspNet.Razor.Runtime/TagHelpers/TagHelperDescriptorResolver.cs‎

Lines changed: 16 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -13,12 +13,12 @@ namespace Microsoft.AspNet.Razor.Runtime.TagHelpers
1313
/// </summary>
1414
public class TagHelperDescriptorResolver : ITagHelperDescriptorResolver
1515
{
16-
private readonly TagHelperTypeResolver _tagHelperTypeResolver;
16+
private readonly TagHelperTypeResolver _typeResolver;
1717

1818
// internal for testing
19-
internal TagHelperDescriptorResolver(TagHelperTypeResolver tagHelperTypeResolver)
19+
internal TagHelperDescriptorResolver(TagHelperTypeResolver typeResolver)
2020
{
21-
_tagHelperTypeResolver = tagHelperTypeResolver;
21+
_typeResolver = typeResolver;
2222
}
2323

2424
/// <summary>
@@ -48,55 +48,36 @@ public IEnumerable<TagHelperDescriptor> Resolve(string lookupText)
4848

4949
private IEnumerable<Type> ResolveTagHelperTypes(string lookupText)
5050
{
51-
var data = lookupText.Split(new[] { ',' }, StringSplitOptions.RemoveEmptyEntries);
51+
var lookupStrings = lookupText.Split(new[] { ',' }, StringSplitOptions.RemoveEmptyEntries);
5252

53-
// Ensure that we have valid data to work with. Valid formats are:
53+
// Ensure that we have valid lookupStrings to work with. Valid formats are:
5454
// "assemblyName"
5555
// "typeName, assemblyName"
56-
if (data.Length != 1 && data.Length != 2)
56+
if (lookupStrings.Length != 1 && lookupStrings.Length != 2)
5757
{
5858
throw new InvalidOperationException(
5959
Resources.FormatTagHelperDescriptorResolver_InvalidAddTagHelperLookupText(lookupText));
6060
}
6161

62-
// Grab the assembly name from the lookup text data.
63-
var assemblyName = GetAssemblyName(data);
62+
// Grab the assembly name from the lookup text strings. Due to our supported lookupText formats it will
63+
// always be the last element provided.
64+
var assemblyName = lookupStrings.Last().Trim();
6465

6566
// Resolve valid tag helper types form the assembly.
66-
var types = _tagHelperTypeResolver.Resolve(assemblyName);
67+
var types = _typeResolver.Resolve(assemblyName);
6768

68-
// Check if the user provided a type name.
69-
var typeLookup = GetTypeLookup(data);
69+
// If the user provided a type name (lookupStrings.Length == 2) then we need to retrieve the value and
70+
// trim it.
71+
var typeName = lookupStrings.Length == 2 ? lookupStrings[0].Trim() : null;
7072

7173
// Check if the lookupText specifies a type to search for.
72-
if (typeLookup != null)
74+
if (typeName != null)
7375
{
74-
// Iterate through the assembly and find the specified type
75-
types = types.Where(type => type.Namespace + "." + type.Name == typeLookup);
76+
types = types.Where(type =>
77+
string.Equals(type.Namespace + "." + type.Name, typeName, StringComparison.Ordinal));
7678
}
7779

7880
return types;
7981
}
80-
81-
private static string GetAssemblyName(string[] lookupTextData)
82-
{
83-
// Assembly name must always be provided
84-
var lookupName = lookupTextData.Length == 2 ? lookupTextData[1] : lookupTextData[0];
85-
86-
return lookupName.Trim();
87-
}
88-
89-
private static string GetTypeLookup(string[] lookupTextData)
90-
{
91-
var typeSpecified = lookupTextData.Length == 2;
92-
93-
if (typeSpecified)
94-
{
95-
return lookupTextData[0].Trim();
96-
}
97-
98-
return null;
99-
}
100-
10182
}
10283
}

‎src/Microsoft.AspNet.Razor.Runtime/TagHelpers/TagHelperTypeResolver.cs‎

Lines changed: 21 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ namespace Microsoft.AspNet.Razor.Runtime.TagHelpers
1313
/// </summary>
1414
public class TagHelperTypeResolver
1515
{
16-
private static readonly TypeInfo TagHelperTypeInfo = typeof(ITagHelper).GetTypeInfo();
16+
private static readonly TypeInfo ITagHelperTypeInfo = typeof(ITagHelper).GetTypeInfo();
1717

1818
/// <summary>
1919
/// Instantiates a new instance of the <see cref="TagHelperTypeResolver"/> class.
@@ -23,33 +23,42 @@ public TagHelperTypeResolver()
2323
}
2424

2525
/// <summary>
26-
/// Loads the <see cref="Assembly"/> that is defined by the given <paramref name="assemblyName"/> and resolves
26+
/// Loads an <see cref="Assembly"/> using the given <paramref name="name"/> and resolves
2727
/// all valid <see cref="ITagHelper"/> <see cref="Type"/>s.
2828
/// </summary>
29-
/// <param name="assemblyName">A <see cref="string"/> lookup name for an <see cref="Assembly"/>.</param>
29+
/// <param name="name">The name of an <see cref="Assembly"/> to search.</param>
3030
/// <returns>An <see cref="IEnumerable{Type}"/> of valid <see cref="ITagHelper"/> <see cref="Type"/>s.</returns>
31-
public IEnumerable<Type> Resolve(string assemblyName)
31+
public IEnumerable<Type> Resolve(string name)
3232
{
33-
if (string.IsNullOrEmpty(assemblyName))
33+
if (string.IsNullOrEmpty(name))
3434
{
3535
throw new InvalidOperationException(
3636
Resources.TagHelperTypeResolver_AddTagHelperAssemblyNameCannotBeEmptyOrNull);
3737
}
3838

39-
var assemblyRef = new AssemblyName(assemblyName);
40-
var libraryTypes = GetLibraryDefinedTypes(assemblyRef);
39+
var assemblyName = new AssemblyName(name);
40+
var libraryTypes = GetLibraryDefinedTypes(assemblyName);
4141
var validTagHelpers = libraryTypes.Where(IsTagHelper);
4242

43-
// Need to convert from TypeInfo[] to Type[]
43+
// Convert from TypeInfo[] to Type[]
4444
return validTagHelpers.Select(type => type.AsType());
4545
}
4646

4747
// Internal for testing, don't want to be loading assemblies during a test.
48-
internal virtual IEnumerable<TypeInfo> GetLibraryDefinedTypes(AssemblyName assemblyRef)
48+
internal virtual IEnumerable<TypeInfo> GetLibraryDefinedTypes(AssemblyName assemblyName)
4949
{
50-
var assembly = Assembly.Load(assemblyRef);
50+
try
51+
{
52+
var assembly = Assembly.Load(assemblyName);
5153

52-
return assembly.DefinedTypes;
54+
return assembly.DefinedTypes;
55+
}
56+
catch (Exception ex)
57+
{
58+
throw new InvalidOperationException(
59+
Resources.FormatTagHelperTypeResolver_CannotResolveTagHelperAssembly(assemblyName.Name,
60+
ex.Message));
61+
}
5362
}
5463

5564
private static bool IsTagHelper(TypeInfo typeInfo)
@@ -58,7 +67,7 @@ private static bool IsTagHelper(TypeInfo typeInfo)
5867
!typeInfo.IsAbstract &&
5968
!typeInfo.IsGenericType &&
6069
!typeInfo.IsNested &&
61-
TagHelperTypeInfo.IsAssignableFrom(typeInfo);
70+
ITagHelperTypeInfo.IsAssignableFrom(typeInfo);
6271
}
6372
}
6473
}

‎src/Microsoft.AspNet.Razor/TagHelpers/ITagHelperDescriptorResolver.cs‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,13 +11,13 @@ namespace Microsoft.AspNet.Razor.TagHelpers
1111
public interface ITagHelperDescriptorResolver
1212
{
1313
/// <summary>
14-
/// Resolves <see cref="TagHelperDescriptor"/>s based on the given <paramref name="lookupText"/>.
14+
/// Resolves <see cref="TagHelperDescriptor"/>s matching the given <paramref name="lookupText"/>.
1515
/// </summary>
1616
/// <param name="lookupText">
17-
/// A <see cref="string"/> location on where to find tag helper <see cref="Type"/>s.
17+
/// A <see cref="string"/> used to find tag helper <see cref="Type"/>s.
1818
/// </param>
19-
/// <returns>An <see cref="IEnumerable{TagHelperDescriptor}"/> that represent tag helpers associated with the
20-
/// given <paramref name="lookupText"/>.</returns>
19+
/// <returns>An <see cref="IEnumerable{TagHelperDescriptor}"/> of <see cref="TagHelperDescriptor"/>s matching
20+
/// the given <paramref name="lookupText"/>.</returns>
2121
IEnumerable<TagHelperDescriptor> Resolve(string lookupText);
2222
}
2323
}

‎test/Microsoft.AspNet.Razor.Runtime.Test/TagHelpers/CommonTagHelpers.cs‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
// Copyright (c) Microsoft Open Technologies, Inc. All rights reserved.
22
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
33

4-
using System;
54
using Microsoft.AspNet.Razor.Runtime.TagHelpers;
65

76
namespace Microsoft.AspNet.Razor.Runtime.Test.TagHelpers
@@ -19,14 +18,14 @@ public class SingleAttributeTagHelper : ITagHelper
1918
public int IntAttribute { get; set; }
2019
}
2120

22-
public class TwoInvalidAttributeTagHelper : ITagHelper
21+
public class MissingAccessorTagHelper : ITagHelper
2322
{
2423
public string ValidAttribute { get; set; }
2524
public string InvalidNoGetAttribute { set { } }
2625
public string InvalidNoSetAttribute { get { return string.Empty; } }
2726
}
2827

29-
public class TwoInvalidAccessorAttributeTagHelper : ITagHelper
28+
public class PrivateAccessorTagHelper : ITagHelper
3029
{
3130
public string ValidAttribute { get; set; }
3231
public string InvalidPrivateSetAttribute { get; private set; }

‎test/Microsoft.AspNet.Razor.Runtime.Test/TagHelpers/CompleteTagHelperDescriptorComparer.cs‎

Lines changed: 0 additions & 48 deletions
This file was deleted.

0 commit comments

Comments
 (0)