Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Problems with ${changes} in template #130

Open
buumi opened this issue Mar 15, 2019 · 8 comments
Open

Problems with ${changes} in template #130

buumi opened this issue Mar 15, 2019 · 8 comments

Comments

@buumi
Copy link

buumi commented Mar 15, 2019

Expected Behavior

When adding

            "fields" : [
                { "title" : "Changes", "value" : "${changes}" }
            ]

to template, I would expect that changes would be inserted to message.

Current Behavior

Instead I'm getting either "[]" or an error saying An error occured building the payload preview com.google.gson.JsonSyntaxException: com.google.gson.stream.MalformedJsonException: Unterminated object at line 13 column 55 path $.attachments[0].fields[3].value. This is from the preview window, but the actual alert also had "[]" even though the build failed and it's showing changes in TeamCity (note that those might be coming from snapshot dependencies).

Steps to Reproduce (for bugs)

Add ${changes} to template and test the template.

Your Environment

  • tcWebHooks Version: 1.1.301.388
  • TeamCity Version: 2018.2.3
  • TeamCity server Operating System: CentOS 7
  • Are you using a WebHook Template?: Yes

Example Configuration (xml)

webhook-templates.xml:

[root@bob-ng config]# cat webhook-templates.xml
<?xml version="1.0" encoding="UTF-8" standalone="yes"?>
<webhook-templates>
    <webhook-template id="w_slack_custom" enabled="true" rank="100" format="jsonTemplate">
        <default-template use-for-branch-template="false">{
    "username": "TeamCity",
    "icon_url" : "https://raw.githubusercontent.com/tcplugins/tcWebHooks/master/docs/icons/teamcity-logo-48x48.png",
    "attachments": [
        {
            "title": "${capitalise(buildStateDescription)} : ${buildName} &lt;${buildStatusUrl}|build #${buildNumber}&gt;",
            "fallback": "${capitalise(buildStateDescription)} : ${buildName} build #${buildNumber}",
            "fields" : [
                { "title" : "Status", "value" : "${buildStatus} (${buildStateDescription})" },
                { "title" : "Project Name", "value" : "&lt;${rootUrl}/project.html?projectId=${projectExternalId}|${projectName}&gt;", "short": true },
                { "title" : "Build Name", "value" : "&lt;${rootUrl}/viewType.html?buildTypeId=${buildExternalTypeId}|${buildName}&gt;", "short": true },
                { "title" : "Commit", "value" : "&lt;${buildStatusUrl}&amp;tab=buildChangesDiv|${substr(build.vcs.number,0,7,32)}&gt;" },
                { "title" : "Triggered By", "value" : "${triggeredBy}", "short" : true },
                { "title" : "Agent", "value" : "${agentName}", "short" : true }
            ]
        }
    ]
}</default-template>
        <default-branch-template>{
    "username": "TeamCity",
    "icon_url" : "https://raw.githubusercontent.com/tcplugins/tcWebHooks/master/docs/icons/teamcity-logo-48x48.png",
    "attachments": [
        {
            "title": "${capitalise(buildStateDescription)} : ${buildName} [${branchDisplayName}] &lt;${buildStatusUrl}|build #${buildNumber}&gt;",
            "fallback": "${capitalise(buildStateDescription)} : ${buildName} [${branchDisplayName}] build #${buildNumber}",
            "fields" : [
                { "title" : "Status", "value" : "${buildStatus} (${buildStateDescription})" },
                { "title" : "Project Name", "value" : "&lt;${rootUrl}/project.html?projectId=${projectExternalId}|${projectName}&gt;", "short": true },
                { "title" : "Build Name", "value" : "&lt;${rootUrl}/viewType.html?buildTypeId=${buildExternalTypeId}|${buildName}&gt;", "short": true },
                { "title" : "Branch", "value" : "${branchDisplayName}", "short": true },
                { "title" : "Commit", "value" : "&lt;${buildStatusUrl}&amp;tab=buildChangesDiv|${substr(build.vcs.number,0,7,32)}&gt;", "short": true },
                { "title" : "Triggered By", "value" : "${triggeredBy}", "short" : true },
                { "title" : "Agent", "value" : "${agentName}", "short" : true }
            ]
        }
    ]
}</default-branch-template>
        <template-description>Custom Slack.com JSON template</template-description>
        <template-tool-tip>Supports the slack.com JSON webhooks endpoint</template-tool-tip>
        <preferred-date-format></preferred-date-format>
        <templates max-id="5">
            <template id="0">
                <template-text use-for-branch-template="false">{
    "username": "TeamCity",
    "icon_url" : "https://raw.githubusercontent.com/tcplugins/tcWebHooks/master/docs/icons/teamcity-logo-48x48.png",
    "attachments": [
        {
            "title": "Failed (broken) : ${buildName} &lt;${buildStatusUrl}|build #${buildNumber}&gt;",
            "fallback": "Failed (broken) : ${buildName} build #${buildNumber}",
            "color": "danger",
            "fields" : [
                { "title" : "Status", "value" : "${buildStatus}" },
                { "title" : "Commit", "value" : "&lt;${buildStatusUrl}&amp;tab=buildChangesDiv|${substr(build.vcs.number,0,7,32)}&gt;", "short": true },
                { "title" : "Changes", "value" : "${changes}" }
            ]
        }
    ]
}</template-text>
                <branch-template-text>{
    "username": "TeamCity",
    "icon_url" : "https://raw.githubusercontent.com/tcplugins/tcWebHooks/master/docs/icons/teamcity-logo-48x48.png",
    "attachments": [
        {
            "title": "Failed (broken) : ${buildName} &lt;${buildStatusUrl}|build #${buildNumber}&gt;",
            "fallback": "Failed (broken) : ${buildName} build #${buildNumber}",
            "color": "danger",
            "fields" : [
                { "title" : "Status", "value" : "${buildStatus}" },
                { "title" : "Branch", "value" : "${branchDisplayName}", "short": true },
                { "title" : "Commit", "value" : "&lt;${buildStatusUrl}&amp;tab=buildChangesDiv|${substr(build.vcs.number,0,7,32)}&gt;", "short": true },
                { "title" : "Changes", "value" : "${changes}" }
            ]
        }
    ]
}</branch-template-text>
                <states>
                    <state type="buildBroken" enabled="true"/>
                </states>
            </template>
            <template id="1">
                <template-text use-for-branch-template="false">{
    "username": "TeamCity",
    "icon_url" : "https://raw.githubusercontent.com/tcplugins/tcWebHooks/master/docs/icons/teamcity-logo-48x48.png",
    "attachments": [
        {
            "title": "Success (fixed) : ${buildName} [${branchDisplayName}] &lt;${buildStatusUrl}|build #${buildNumber}&gt;",
            "fallback": "Success (fixed) : ${buildName} [${branchDisplayName}] build #${buildNumber}",
            "color": "good",
            "fields" : [
                { "title" : "Status", "value" : "${buildStatus}" }
            ]
        }
    ]
}</template-text>
                <branch-template-text>{
    "username": "TeamCity",
    "icon_url" : "https://raw.githubusercontent.com/tcplugins/tcWebHooks/master/docs/icons/teamcity-logo-48x48.png",
    "attachments": [
        {
            "title": "Success (fixed) : ${buildName} [${branchDisplayName}] &lt;${buildStatusUrl}|build #${buildNumber}&gt;",
            "fallback": "Success (fixed) : ${buildName} [${branchDisplayName}] build #${buildNumber}",
            "color": "good",
            "fields" : [
                { "title" : "Status", "value" : "${buildStatus}" },
                { "title" : "Branch", "value" : "${branchDisplayName}", "short": true }
            ]
        }
    ]
}</branch-template-text>
                <states>
                    <state type="buildFixed" enabled="true"/>
                </states>
            </template>
        </templates>
    </webhook-template>
</webhook-templates>

@netwolfuk
Copy link
Member

Thanks for the thorough bug report. I'll try to take a look over the weekend

@netwolfuk
Copy link
Member

netwolfuk commented Apr 13, 2019

Hi @buumi. Thanks so much for your patience on this. We have a new baby in the house, and suddenly my spare time has disappeared.

I've imported your template (thanks for all the details) and the changes appear correctly for a simple change.

I've not had a chance to test snapshots yet, as my docker instance running tcwebhooks 1.1 has no relevant builds configured.

My current thoughts are:

  1. You could be right about snapshots
  2. The JSON serialisation in 1.1 is not working correctly. I've made some changes to that as part of the velocity template changes in 1.2, but will need to reproduce your issue before I test that.
  3. Something about your change list is funky. Is there a chance you could post a screenshot of the change list in TeamCity? Perhaps blur out anything sensitive.

@netwolfuk
Copy link
Member

netwolfuk commented Apr 13, 2019

Perhaps your change list consists of more than one VCS? I don't think I've ever tested that.

@buumi
Copy link
Author

buumi commented Apr 16, 2019

Yes my change list consists of 3 VCS roots. Empty changes list with template:
tempsnip

JSON error:
json_error

@krishnanmk
Copy link

I think the problem is because value of ${changes} is not escaped when replacing it in the JSON blob. Right? I'm facing a similar issue.

One interesting thing I noticed in the above example was the usage of ${capitalise(...)} and ${substr(...)}. Are these in-built functions provided by TeamCity, or are they specific to this plugin? Anyway, so, are their such functions available for escaping characters as well, or simply a ${replace(...)} like option? That might just work, I guess. (BTW, I tried ${replace(...)} but that didn't work. :-/

@netwolfuk
Copy link
Member

Hi @kichnan

The code is supposed to do the serialisation for you here:

Yes, those commands are provided by the plugin here https://github.com/tcplugins/tcWebHooks/blob/1.1.x.x/tcwebhooks-core/src/main/java/webhook/teamcity/payload/util/WebHooksBeanUtilsVariableResolver.java

There is an escape JSON function there too, but as mentioned, this should not be required.

Btw, in 1.2.x, there is also support for the velocity templating engine. In velocity these functions are nicely separated, eg
https://github.com/tcplugins/tcWebHooks/tree/master/tcwebhooks-core/src/main/java/webhook/teamcity/payload/variableresolver/velocity

@netwolfuk
Copy link
Member

Hi @buumi I finally has a chance to look at this with a build that has changes from multiple VCS's.

In my opinion we have a few issues here:
Firstly, the format that Slack wants to see here does not work well with the output from the $changes object. The ${changes} object in tcWebHooks 1.1 looks like this:

    "changes": [
      {
        "version": "a8ea5c10614406fb5a1a7d7b8321bca95fef1c9b",
        "change": {
          "files": [
            "tcwebhooks-web-ui/src/main/resources/buildServerResources/WebHook/templateEdit.jsp"
          ],
          "comment": "Add change count variables to template UI.\n\nAdded the following new variables so that they can be selected when\nediting a template.\n\n - \"changeFileListCount\"\n - \"maxChangeFileListSize\"\n - \"maxChangeFileListCountExceeded\"",
          "username": "netwolfuk",
          "vcsRoot": "https://github.com/tcplugins/tcWebHooks.git#refs/heads/master"
        }
      },
      {
        "version": "89c6500cc9f075c22b99a4b2e3a3dda187a19e3c",
        "change": {
          "files": [
            "tcwebhooks-core/src/main/java/webhook/teamcity/payload/content/WebHookPayloadContent.java",
            "tcwebhooks-core/src/main/java/webhook/teamcity/payload/content/WebHooksChange.java",
            "tcwebhooks-core/src/main/java/webhook/teamcity/payload/content/WebHooksChangeBuilder.java",
            "tcwebhooks-core/src/main/java/webhook/teamcity/payload/format/WebHookPayloadJson.java",
            "tcwebhooks-core/src/main/java/webhook/teamcity/payload/format/WebHookPayloadXml.java",
            "tcwebhooks-core/src/test/java/webhook/teamcity/payload/content/WebHookPayloadContentChangesTest.java"
          ],
          "comment": "Add ability to limit or exclude vcs file list whilst building payload\n\nThis change includes the following new features:\n1. The list of changed VCS files can be disabled (0).\n2. The list of changed VCS files can be disabled if it is too large. The\ndefault value is 100 files across all changes in a build.\n3. The list can be always include (old behaviour) when the value is set\nnegative (-1).\n\nNOTE: In the cases where the change list is disabled or too large (1 & 2\nabove), the payload will contain a `null` files list.\nThis is preferable to returning an empty list, because the empty list\nimplies no actual changed files were included in the change.\nA null change list will typically be serialised to nothing in JSON or\nXML, so the json array or xml element will be missing from the payload.\nIf your endpoint is expecting these, then it should fail in this\nscenario or handle the missing field gracefully.\n\n**Enables control over the maximum number of files changed in a build\nbefore the changed file list is null.** This can be controlled in the\nthree ways below, in order of priority.\n\n1. Add a \u0027param\u0027 named \u0027maxChangeFileListSize\u0027 to a webhook config by\nediting plugin-settings.xml\n2. Add a build parameter to a buildType or project called\n\u0027webhook.maxChangeFileListSize\u0027\n3. Define a property in teamcity\u0027s internal properties file called\n\u0027webhook.maxChangeFileListSize\u0027\n\nIf the total number of files is greater than `maxChangeFileListSize`,\nthen the change list will be null. See note above.\n",
          "username": "netwolfuk",
          "vcsRoot": "https://github.com/tcplugins/tcWebHooks.git#refs/heads/master"
        }
      }
    ]

Simply putting that into the slack payload will have two problems.

  1. The object is JSON, and nesting it into an existing JSON document will require escaping the text. This is what @kichnan is hinting at above.
  2. The format that slack is expecting is quite specific, and the nested JSON does not conform to what they want to see in their markup. Their markup is JSON, but with sort of a markdown payload inside the JSON element.

To achieve what you want, would require some post processing of he ${changes} object, so that the output can be converted into a format that slack will display nicely. Unfortunately, the templating engine I wrote in tcWebHooks 1.1 is very rudimentary and does not support the logic you'd need here.

This is precisely why I have added support for the Velocity templating engine in tcWebHooks 1.2 (currently in alpha). With Velocity, we have loops, conditionals, and macros, which would make this much simpler. See #103 which even has an example of exactly what you are trying to achieve.

The com.google.gson.stream.MalformedJsonException error you see when you trying to preview the payload is because I am trying to be too smart in the presentation layer when rendering the result. Since the payload is not valid JSON (see comment above about nested JSON), the parser which tries to convert the JSON into pretty HTML is failing. I think this might be fixed in 1.2 as well.

You might be better off using the template editor "Preview Template" button. That is less "clever" and does the JSON to HTML rendering client side, and will still display the JSON (albeit not nicely formatted if it's not valid).

@netwolfuk
Copy link
Member

BTW, this issue can be reproduced with any change. It doesn't have to have multiple VCS's

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants