Fix issue #227

[arnd27]

Issue #227

• Move the code from the consturctor into a new private start method
• Constructor takes in a delayStart boolean, with the default keeping the existing behavior
• If the _server is null then run the new private start method.
• Continuing processing request as done today.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

[costleya]

I would change this to autoStart, and set default to true. That way, you can check using the affirmative case.

if(autoStart)
{
    Start();
}

[costleya]

Please wrap if bodies with {}, unless it is an exception or a return

[costleya]

spelling - derived

[costleya]

if statement body needs { }

[costleya]

You are missing an invocation to a new virtual method that you could override to set the environment variables.

1. You need a new virtual PreStart method that takes in the ILambdaContext. I wouldn't pass in the request since this method would only be invoked the on the first request, and we want to prevent misuse of this method.

2. You need to call this method in the !IsStarted if statement. Since this method will only be useful for altering the properties of the environment before the service starts, we should isolate this call so it doesn't cause inadvertent side effects.

[arnd27]

For what you need a new virtual PreStart method, for me it makes no sense. I use this fix with the following class:

    public class APIGatewayProxyFunctionExt : Amazon.Lambda.AspNetCoreServer.APIGatewayProxyFunction
    {
        private string environmentName;

        public APIGatewayProxyFunctionExt(string environmentName, Action<string, IWebHostBuilder> configureDelegate)
            : base(true)
        {
            this.environmentName = environmentName;
            this.configureDelegate = configureDelegate;
            Start();
        }

        /// <summary>
        /// The builder has configuration, logging and Amazon API Gateway already configured. The startup class
        /// needs to be configured in this method using the UseStartup<>() method.
        /// </summary>
        /// <param name="builder"></param>
        protected override void Init(IWebHostBuilder builderRoot)
        {
            configureDelegate(environmentName, builderRoot);
        }

        private Action<string, IWebHostBuilder> configureDelegate;

        protected override IWebHostBuilder CreateWebHostBuilder()
        {
            return new WebHostBuilder()
                .UseApiGateway()
                .ConfigureAppConfiguration((context, builder) =>
                {
                    var env = context.HostingEnvironment;
                    env.EnvironmentName = environmentName ?? "NoEnv";
                });
        }
    }

Now I can set the environment.

@costleya: Or should I add this modifications to the base class?

And than I use this such as (I know it's only a workaround):

    public abstract class LambdaEntryPointExt
    {        
	private readonly IDictionary<string, APIGatewayProxyFunctionExt> dict;

        public LambdaEntryPointExt()
        {
            this.dict = new ConcurrentDictionary<string, APIGatewayProxyFunctionExt>();
        }

        [LambdaSerializer(typeof(Amazon.Lambda.Serialization.Json.JsonSerializer))]
        public Task<APIGatewayProxyResponse> FunctionHandlerAsync(APIGatewayProxyRequest request, ILambdaContext lambdaContext)
        {
            //arn:aws:lambda:eu-west-1:123456789123:function:demo-web:production
            var envName = lambdaContext.InvokedFunctionArn.Split(':').Last();

            if (!string.IsNullOrEmpty(envName))
                envName = envName.FirstCharToUpper();

            APIGatewayProxyFunctionExt apiGatewayProxyFunctionExt = null;
            if (!dict.Keys.Contains(envName))
            {
                apiGatewayProxyFunctionExt = new APIGatewayProxyFunctionExt(envName, ConfigureContext);
                dict.Add(envName, apiGatewayProxyFunctionExt);
            }
            else
            {
                apiGatewayProxyFunctionExt = dict[envName];
            }              

            return apiGatewayProxyFunctionExt.FunctionHandlerAsync(request, lambdaContext);
        }

        protected abstract void ConfigureContext(string environmentName, IWebHostBuilder builder);
    }

The concrete LambdaEntryPoint looks like the following:

  public class LambdaEntryPoint : LambdaEntryPointExt
  {
        protected override void ConfigureContext(string environmentName, IWebHostBuilder builder)
        {
            builder
                .AppSharedConfiguration()
                .ConfigureAppConfiguration((context, b) =>
                {
                    b.AddAWSConfigurationFromParameterStore($"/demo-auth/{environmentName.ToLower()}");
                });
        }
  }

[normj]

Is the main point of this pull request to get the environment name in so you can find the SSM parameter store variables for the environment? If so that seems like an important use case for us to handle and I'm wondering if we have made it too hard to do that.

Also I just discovered you can find the lambda function name during the constructor phase by checking the environment variable AWS_LAMBDA_FUNCTION_NAME which seems then we don't need to do this change. Here are the list of available environment variables Lambda sets. https://docs.aws.amazon.com/lambda/latest/dg/current-supported-versions.html

[arnd27]

Thats the problem, but I need the aliase name (e.g. arn:aws:lambda:eu-west-1:123456789:function:demo-api:<alias>) and not the AWS_LAMBDA_FUNCTION_NAME (e.g. demo-api). Your lambda runtime and your library don't set the alias and I don't want to wait for this decision. With my simple pull reqeuest, everyone can extend your library and set what he want.

[normj]

Status update. I have merged this into the dev branch to go out with the next release.

[normj]

Version 2.0.4 was released with this pull request. I made a slight tweak to use an enum instead of a boolean in the constructor to make it more clear and give us room to make changes in the future.