Skip to content

Commit

Permalink
Address review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
jskeet committed Oct 20, 2020
1 parent 9ce61e4 commit 318061c
Show file tree
Hide file tree
Showing 4 changed files with 33 additions and 26 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,10 @@ namespace Google.Cloud.Functions.Examples.IntegrationTests
{
/// <summary>
/// By default, the test server will use the same Functions Startup classes
/// as normal. Applying the FunctionsStartup attribute to the test class (or the assembly) signals to FunctionTestBase
/// which startups should be used to inject test dependencies instead of the production ones.
/// An alternative is to declare a parameterless constructor that creates a FunctionTestServer
/// to pass into the base class constructor.
/// as normal. Applying the FunctionsStartup attribute to the test class (and/or the assembly)
/// signals to FunctionTestBase which startups should be used to inject test dependencies
/// instead of the production ones. An alternative is to declare a parameterless constructor
/// that creates a FunctionTestServer to pass into the base class constructor.
/// </summary>
[FunctionsStartup(typeof(TestStartup))]
public class TestableDependenciesTest : FunctionTestBase<TestableDependencies.Function>
Expand Down
25 changes: 16 additions & 9 deletions src/Google.Cloud.Functions.Hosting/HostingInternals.cs
Original file line number Diff line number Diff line change
Expand Up @@ -90,9 +90,10 @@ static void ReturnNotFound(IApplicationBuilder app) =>

/// <summary>
/// Validates that the startup classes which would be used for the finally-selected function are the
/// same as the startup classes we *actually* used. This method is only called when the app is started
/// by <see cref="EntryPoint"/>, just to check that nothing really weird has happened (such as a startup
/// changing the function target to a different function which needs different startups).
/// same (and in the same order) as the startup classes we *actually* used. This method is only called
/// when the app is started by <see cref="EntryPoint"/>, just to check that nothing really weird has
/// happened (such as a startup changing the function target to a different function which needs
/// different startups).
/// </summary>
private static IApplicationBuilder ValidateStartupClasses(IApplicationBuilder app)
{
Expand Down Expand Up @@ -128,6 +129,10 @@ internal static async Task Execute(HttpContext context)
await function.HandleAsync(context);
}

/// <summary>
/// Determines the function target based on a configuration and the function assembly.
/// This will throw an exception if it can't determine the right function type.
/// </summary>
internal static Type GetFunctionTarget(IConfiguration configuration, Assembly assembly)
{
var options = FunctionsFrameworkOptions.FromConfiguration(configuration);
Expand All @@ -137,13 +142,15 @@ internal static Type GetFunctionTarget(IConfiguration configuration, Assembly as
: assembly.GetType(target) ?? throw new InvalidOperationException($"Can't load specified function type '{target}'");
}

/// <summary>
/// Determines the function target based on a configuration and the function assembly.
/// This will return null if it can't determine the right function type.
/// </summary>
internal static Type? TryGetFunctionTarget(IConfiguration configuration, Assembly assembly)
{
var options = FunctionsFrameworkOptions.FromConfiguration(configuration);
string? target = options.FunctionTarget;
return target is null
? TryFindDefaultFunctionType(assembly.GetTypes())
: assembly.GetType(target) ?? throw new InvalidOperationException($"Can't load specified function type '{target}'");
return target is null ? TryFindDefaultFunctionType(assembly.GetTypes()) : assembly.GetType(target);
}

internal static IServiceCollection AddServicesForFunctionTarget(IServiceCollection services, Type functionType)
Expand Down Expand Up @@ -180,7 +187,7 @@ internal static IServiceCollection AddServicesForFunctionTarget(IServiceCollecti
/// discover problems there easily enough anyway.)
/// </summary>
/// <remarks>
/// This method is internal rather private for the sake of testability.
/// This method is internal rather than private for the sake of testability.
/// </remarks>
/// <param name="types">The types to search through.</param>
/// <returns>The function type to use by default</returns>
Expand All @@ -202,8 +209,8 @@ internal static Type FindDefaultFunctionType(params Type[] types)
/// Attempts to find a single valid non-abstract function class within the given set of types, but returns
/// null (instead of failing) if there isn't exactly one such class.
/// </summary>
/// <param name="types"></param>
/// <returns></returns>
/// <param name="types">The types to check for a valid function class.</param>
/// <returns>The single valid function class, or null if there are either 0 or more than 1 such classes.</returns>
internal static Type? TryFindDefaultFunctionType(params Type[] types)
{
var validTypes = FindValidFunctionTypes(types);
Expand Down
12 changes: 6 additions & 6 deletions src/Google.Cloud.Functions.Testing/FunctionTestServer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -51,11 +51,11 @@ public FunctionTestServer(Type functionTarget) : this(functionTarget, null)
/// type to inspect for <see cref="FunctionsStartupAttribute"/> attributes.
/// </summary>
/// <param name="functionTarget">The function type to host in the server.</param>
/// <param name="attributedStartupType">A type to examine for <see cref="FunctionsStartupAttribute"/> attributes to provide further customization.
/// <param name="startupAttributeSource">A type to examine for <see cref="FunctionsStartupAttribute"/> attributes to provide further customization.
/// If this is null, or no attributes are specified in the type (or its base class hierarchy) or the assembly it's declared in,
/// the startup classes in the function assembly are used.</param>
public FunctionTestServer(Type functionTarget, Type? attributedStartupType)
: this(FunctionTestServerBuilder.Create(functionTarget).MaybeUseFunctionsStartupsFromAttributes(attributedStartupType).BuildTestServer(), functionTarget)
public FunctionTestServer(Type functionTarget, Type? startupAttributeSource)
: this(FunctionTestServerBuilder.Create(functionTarget).MaybeUseFunctionsStartupsFromAttributes(startupAttributeSource).BuildTestServer(), functionTarget)
{
}

Expand Down Expand Up @@ -130,10 +130,10 @@ public FunctionTestServer() : this(null)
/// Constructs a new instance serving <typeparamref name="TFunction"/>, with an optional
/// type to inspect for <see cref="FunctionsStartupAttribute"/> attributes.
/// </summary>
/// <param name="attributedStartupType">A type to examine for <see cref="FunctionsStartupAttribute"/> attributes to provide further customization.
/// <param name="startupAttributeSource">A type to examine for <see cref="FunctionsStartupAttribute"/> attributes to provide further customization.
/// If this is null, or no attributes are specified in the type (or its base class hierarchy) or the assembly it's declared in,
/// the startup classes in the function assembly are used.</param>
internal FunctionTestServer(Type? attributedStartupType) : base(typeof(TFunction), attributedStartupType)
/// the usual startup classes for the target function are used.</param>
internal FunctionTestServer(Type? startupAttributeSource) : base(typeof(TFunction), startupAttributeSource)
{
}
}
Expand Down
14 changes: 7 additions & 7 deletions src/Google.Cloud.Functions.Testing/FunctionTestServerBuilder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -72,25 +72,25 @@ public FunctionTestServerBuilder UseFunctionsStartups(IEnumerable<FunctionsStart
}

/// <summary>
/// Examines <paramref name="attributedType"/> (and its base class hierarchy, and the assembly in which it's declared)
/// Examines <paramref name="startupAttributeSource"/> (and its base class hierarchy, and the assembly in which it's declared)
/// for <see cref="FunctionsStartupAttribute" /> annotations.
/// If any such attributes are found, the startup classes specified by the attributes are used
/// instead of the startup classes specified in the assembly containing the function target. If no attributes
/// are found, or if <paramref name="attributedType"/> is null, this method has no effect.
/// instead of the usual startup classes for the target function. If no attributes
/// are found, or if <paramref name="startupAttributeSource"/> is null, this method has no effect.
/// </summary>
/// <remarks>
/// This method is automatically called by the parameterless constructor of <see cref="FunctionTestBase{TFunction}"/>,
/// with the test class type.
/// </remarks>
/// <param name="attributedType">The type to examine (including base class hierarchy) for attributes.</param>
/// <param name="startupAttributeSource">The type to examine (including base class hierarchy) for attributes.</param>
/// <returns>The same test server builder, for method chaining.</returns>
public FunctionTestServerBuilder MaybeUseFunctionsStartupsFromAttributes(Type? attributedType)
public FunctionTestServerBuilder MaybeUseFunctionsStartupsFromAttributes(Type? startupAttributeSource)
{
if (attributedType is null)
if (startupAttributeSource is null)
{
return this;
}
var startups = FunctionsStartupAttribute.GetStartupTypes(attributedType.Assembly, attributedType)
var startups = FunctionsStartupAttribute.GetStartupTypes(startupAttributeSource.Assembly, startupAttributeSource)
.Select(type => Activator.CreateInstance(type))
.Cast<FunctionsStartup>()
.ToList();
Expand Down

0 comments on commit 318061c

Please sign in to comment.