jeffhollan / LogicAppTemplateCreator

Script to convert Logic Apps into templates for deployment
MIT License
143 stars 74 forks source link

Function id is not generating properly #15

Closed wsilveiranz closed 7 years ago

wsilveiranz commented 7 years ago

Hi guys,

Found a new issue on parsing the function action. Seems like when the function app is been parsed the order of the regex fixes causes the template to became invalid, as it generates this:

"function": { "id": "[concat('/subscriptions/',subscription().subscriptionId,'/resourceGroups/', resourceGroup().name,'/providers/Microsoft.Web/sites/', **parameters('', parameters('ActivityCallIDGenerator-FunctionName'),'-FunctionApp'**),'/functions/', parameters('ActivityCallIDGenerator-FunctionName'),'')]" }

Instead of this:

"function": { "id": "[concat('/subscriptions/',subscription().subscriptionId,'/resourceGroups/', resourceGroup().name,'/providers/Microsoft.Web/sites/', **parameters('ActivityCallIDGenerator-FunctionApp'),**'/functions/', parameters('ActivityCallIDGenerator-FunctionName'),'')]" }

This is caused by the following lines in TemplateGenerator.cs:

curr = curr.Replace(matches.Groups["functionApp"].Value, "', parameters('" + AddTemplateParameter(action.Name + "-FunctionApp", "string", matches.Groups["functionApp"].Value) + "'),'"); curr = curr.Replace(matches.Groups["functionName"].Value, "', parameters('" + AddTemplateParameter(action.Name + "-FunctionName", "string", matches.Groups["functionName"].Value) + "'),'");

As they are in the wrong order.

I'm submitting a pull request to fix that issue.

Also, noticed that there is no Unit Tests for Function samples. I tried to create one but I am struggling at the moment on creating a good sample file (can't seem to extract the template from the right place). If one of you guys add a sample file with function ID there, I can write the unit tests this don't break at a later stage.

Cheers, Wagner.

MLogdberg commented 7 years ago

Thanks again Wagner for finding these issues,

I don't think that is the actaul problem, I think you are just lucky to get around it with this fix.

The real problem is the function AddTemplateParameter since it is checking the value for not been added twice, it came along with alot of URL's beeing the same and removing alot of parameters.

private string AddTemplateParameter(string paramname, string type, JProperty defaultvalue)
        {
            string realParameterName = paramname;
            JObject param = new JObject();
            param.Add("type", JToken.FromObject(type));
            param.Add(defaultvalue);

            if (!string.IsNullOrEmpty(defaultvalue.Value.ToString()) && type.Equals("string",StringComparison.CurrentCultureIgnoreCase))
            {
                foreach (var c in template.parameters)
                {
                    if (c.Value.Value<string>("type").Equals("string", StringComparison.CurrentCultureIgnoreCase) &&  c.Value.Value<string>("defaultValue").Equals(defaultvalue.Value.ToString()))
                    {
                        return c.Key;
                    }
                }
}

But I've always disliked this part since it easy get things confused. What do you say?

wsilveiranz commented 7 years ago

Take a look at this proposed fix. As I mentioned offline, I think it will make the function ID work as expected. We can review the code you are discussing above and see if it is related or if there is any improvements we can do there. I would need to get my head around that part of the code, as I was treating GenerateTemplate as a black box until now. ;-D

Cheers, Wagner.