From 89c6500cc9f075c22b99a4b2e3a3dda187a19e3c Mon Sep 17 00:00:00 2001 From: Net Wolf UK Date: Tue, 5 Nov 2019 10:53:04 +1300 Subject: [PATCH] Add ability to limit or exclude vcs file list whilst building payload This change includes the following new features: 1. The list of changed VCS files can be disabled (0). 2. The list of changed VCS files can be disabled if it is too large. The default value is 100 files across all changes in a build. 3. The list can be always include (old behaviour) when the value is set negative (-1). NOTE: In the cases where the change list is disabled or too large (1 & 2 above), the payload will contain a `null` files list. This is preferable to returning an empty list, because the empty list implies no actual changed files were included in the change. A null change list will typically be serialised to nothing in JSON or XML, so the json array or xml element will be missing from the payload. If your endpoint is expecting these, then it should fail in this scenario or handle the missing field gracefully. **Enables control over the maximum number of files changed in a build before the changed file list is null.** This can be controlled in the three ways below, in order of priority. 1. Add a 'param' named 'maxChangeFileListSize' to a webhook config by editing plugin-settings.xml 2. Add a build parameter to a buildType or project called 'webhook.maxChangeFileListSize' 3. Define a property in teamcity's internal properties file called 'webhook.maxChangeFileListSize' If the total number of files is greater than `maxChangeFileListSize`, then the change list will be null. See note above. --- .../content/WebHookPayloadContent.java | 91 +++-- .../payload/content/WebHooksChange.java | 17 +- .../content/WebHooksChangeBuilder.java | 4 +- .../payload/format/WebHookPayloadJson.java | 5 +- .../payload/format/WebHookPayloadXml.java | 5 +- .../WebHookPayloadContentChangesTest.java | 334 ++++++++++++++++++ 6 files changed, 421 insertions(+), 35 deletions(-) create mode 100644 tcwebhooks-core/src/test/java/webhook/teamcity/payload/content/WebHookPayloadContentChangesTest.java diff --git a/tcwebhooks-core/src/main/java/webhook/teamcity/payload/content/WebHookPayloadContent.java b/tcwebhooks-core/src/main/java/webhook/teamcity/payload/content/WebHookPayloadContent.java index 841499aa..be6a277e 100644 --- a/tcwebhooks-core/src/main/java/webhook/teamcity/payload/content/WebHookPayloadContent.java +++ b/tcwebhooks-core/src/main/java/webhook/teamcity/payload/content/WebHookPayloadContent.java @@ -19,6 +19,7 @@ import jetbrains.buildServer.serverSide.SProject; import jetbrains.buildServer.serverSide.SQueuedBuild; import jetbrains.buildServer.serverSide.SRunningBuild; +import jetbrains.buildServer.serverSide.TeamCityProperties; import jetbrains.buildServer.vcs.SVcsModification; import lombok.Getter; import lombok.Setter; @@ -37,6 +38,9 @@ import webhook.teamcity.payload.variableresolver.VariableResolver; public class WebHookPayloadContent { + private static final String MAX_CHANGE_FILE_LIST_SIZE = "maxChangeFileListSize"; + private static final String WEBHOOK_MAX_CHANGE_FILE_LIST_SIZE = "webhook." + MAX_CHANGE_FILE_LIST_SIZE; + String buildStatus, buildResult, buildResultPrevious, buildResultDelta, notifyType, @@ -81,6 +85,9 @@ public class WebHookPayloadContent { List buildTags; ExtraParametersMap extraParameters; private ExtraParametersMap teamcityProperties; + @Getter private int maxChangeFileListSize = 100; + @Getter private boolean maxChangeFileListCountExceeded = false; + @Getter private int changeFileListCount = 0; private List changes = new ArrayList<>(); private WebHookResponsibility responsibilityInfo; private String pinEventUsername; @@ -142,7 +149,7 @@ public WebHookPayloadContent(VariableResolverFactory variableResolverFactory, SB this.extraParameters = new ExtraParametersMap(extraParameters); this.teamcityProperties = new ExtraParametersMap(teamcityProperties); populateCommonContent(variableResolverFactory, server, sBuild, null, buildState, customTemplates); - populateMessageAndText(sBuild, buildState, customTemplates); + populateMessageAndText(sBuild, buildState); populateArtifacts(sBuild); if (username != null) { this.pinEventUsername = username; @@ -172,7 +179,7 @@ public WebHookPayloadContent(VariableResolverFactory variableResolverFactory, SB this.extraParameters = new ExtraParametersMap(extraParameters); this.teamcityProperties = new ExtraParametersMap(teamcityProperties); populateCommonContent(variableResolverFactory, server, sRunningBuild, previousBuild, buildState, customTemplates); - populateMessageAndText(sRunningBuild, buildState, customTemplates); + populateMessageAndText(sRunningBuild, buildState); populateArtifacts(sRunningBuild); } @@ -216,15 +223,15 @@ private void populateCommonContent(VariableResolverFactory variableResolverFacto } catch (Exception e) {} if (responsibilityHolder.getSBuildType() != null) { - SBuildType buildType = responsibilityHolder.getSBuildType(); - setBuildRunners(buildType.getBuildRunners()); - setBuildFullName(buildType.getFullName()); - setBuildName(buildType.getName()); - setBuildTypeId(TeamCityIdResolver.getBuildTypeId(buildType)); - setBuildInternalTypeId(TeamCityIdResolver.getInternalBuildId(buildType)); - setBuildExternalTypeId(TeamCityIdResolver.getExternalBuildId(buildType)); - setBuildStatusUrl(getRootUrl() + "viewLog.html?buildTypeId=" + buildType.getBuildTypeId() + "&buildId=lastFinished"); - setMessage("Build " + buildType.getFullName() + SBuildType sBuildType = responsibilityHolder.getSBuildType(); + setBuildRunners(sBuildType.getBuildRunners()); + setBuildFullName(sBuildType.getFullName()); + setBuildName(sBuildType.getName()); + setBuildTypeId(TeamCityIdResolver.getBuildTypeId(sBuildType)); + setBuildInternalTypeId(TeamCityIdResolver.getInternalBuildId(sBuildType)); + setBuildExternalTypeId(TeamCityIdResolver.getExternalBuildId(sBuildType)); + setBuildStatusUrl(getRootUrl() + "viewLog.html?buildTypeId=" + sBuildType.getBuildTypeId() + "&buildId=lastFinished"); + setMessage("Build " + sBuildType.getFullName() + " has changed responsibility from " + oldUser + " to " @@ -233,7 +240,7 @@ private void populateCommonContent(VariableResolverFactory variableResolverFacto + getComment().trim() + "'" ); - setText(buildType.getFullName() + setText(sBuildType.getFullName() + " changed responsibility from " + oldUser + " to " @@ -292,14 +299,14 @@ private void populateCommonContent(VariableResolverFactory variableResolverFacto } private void populateMessageAndText(SBuild sRunningBuild, - BuildStateEnum state, Map templates) { + BuildStateEnum state) { // Message is a long form message, for on webpages or in email. setMessage("Build " + sRunningBuild.getBuildType().getFullName() + " has " + state.getDescriptionSuffix() + ". This is build number " + sRunningBuild.getBuildNumber() + ", has a status of \"" + this.buildResult + "\" and was triggered by " + sRunningBuild.getTriggeredBy().getAsString()); // Text is designed to be shorter, for use in Text messages and the like. - setText(sRunningBuild.getBuildType().getFullName().toString() + setText(sRunningBuild.getBuildType().getFullName() + " has " + state.getDescriptionSuffix() + ". Status: " + this.buildResult); } @@ -361,7 +368,12 @@ private void populateCommonContent(VariableResolverFactory variableResolverFacto setTriggeredBy(sBuild.getTriggeredBy().getAsString()); setComment(WebHooksComment.build(sBuild.getBuildComment())); setTags(sBuild.getTags()); - setChanges(sBuild.getContainingChanges()); + int fileChangeCount = 0; + for (SVcsModification mod : sBuild.getContainingChanges()) { + fileChangeCount += mod.getChangeCount(); + } + this.changeFileListCount = fileChangeCount; + setChanges(sBuild.getContainingChanges(), includeVcsFileList()); try { if (sBuild.getBranch() != null){ setBranch(sBuild.getBranch()); @@ -378,10 +390,46 @@ private void populateCommonContent(VariableResolverFactory variableResolverFacto setRootUrl(StringUtils.stripTrailingSlash(server.getRootUrl()) + "/"); setBuildStatusUrl(getRootUrl() + "viewLog.html?buildTypeId=" + getBuildTypeId() + "&buildId=" + getBuildId()); setBuildStateDescription(buildState.getDescriptionSuffix()); - setBuildStatusHtml(variableResolverFactory, buildState, templates.get(WebHookPayloadDefaultTemplates.HTML_BUILDSTATUS_TEMPLATE)); + setBuildStatusHtml(variableResolverFactory, templates.get(WebHookPayloadDefaultTemplates.HTML_BUILDSTATUS_TEMPLATE)); setBuildIsPersonal(sBuild.isPersonal()); } + private boolean includeVcsFileList() { + // Firstly update the "maxChangeFileListSize" value from webhook config or build parameters. + if (extraParameters.containsKey(MAX_CHANGE_FILE_LIST_SIZE)) { + try { + this.maxChangeFileListSize = Integer.parseInt(extraParameters.get(MAX_CHANGE_FILE_LIST_SIZE)); + } catch (NumberFormatException ex) { + Loggers.SERVER.info("WebHookPayloadContent : Unable to convert 'maxChangeFileListSize' value to a valid integer. Defaut value will be used '" + this.maxChangeFileListSize + "'"); + } + } else if (teamcityProperties.containsKey(WEBHOOK_MAX_CHANGE_FILE_LIST_SIZE)) { + try { + this.maxChangeFileListSize = Integer.parseInt(teamcityProperties.get(WEBHOOK_MAX_CHANGE_FILE_LIST_SIZE)); + } catch (NumberFormatException ex) { + Loggers.SERVER.info("WebHookPayloadContent : Unable to convert 'webhook.maxChangeFileListSize' value to a valid integer. Defaut value will be used '" + this.maxChangeFileListSize + "'"); + } + } else if (Objects.nonNull(TeamCityProperties.getPropertyOrNull(WEBHOOK_MAX_CHANGE_FILE_LIST_SIZE))){ + try { + this.maxChangeFileListSize = Integer.parseInt(TeamCityProperties.getProperty(WEBHOOK_MAX_CHANGE_FILE_LIST_SIZE)); + } catch (NumberFormatException ex) { + Loggers.SERVER.info("WebHookPayloadContent : Unable to convert TeamCity global property 'webhook.maxChangeFileListSize' value to a valid integer. Defaut value will be used '" + this.maxChangeFileListSize + "'"); + } + } + + // If the value is negative, checking is disabled and maxChangeFileListSize is effectively unlimited. + if (this.maxChangeFileListSize < 0) { + this.maxChangeFileListCountExceeded = false; + return true; + + // Or calculate that the count we found is less then the preferred one. + } else if (this.changeFileListCount > this.maxChangeFileListSize) { + this.maxChangeFileListCountExceeded = true; + return false; + } + + return true; + } + public List getBuildTags() { return buildTags; } @@ -402,8 +450,8 @@ private void setComment(WebHooksComment webHooksComment) { } } - private void setChanges(List modifications){ - this.changes = WebHooksChangeBuilder.build(modifications); + private void setChanges(List modifications, boolean includeVcsFileModifications){ + this.changes = WebHooksChangeBuilder.build(modifications, includeVcsFileModifications); } public List getChanges(){ @@ -721,7 +769,7 @@ public void setBuildStatusHtml(String buildStatusHtml) { } - private void setBuildStatusHtml(VariableResolverFactory variableResolverFactory, BuildStateEnum buildState, final String htmlStatusTemplate) { + private void setBuildStatusHtml(VariableResolverFactory variableResolverFactory, final String htmlStatusTemplate) { VariableMessageBuilder builder = variableResolverFactory.createVariableMessageBuilder( htmlStatusTemplate, @@ -804,7 +852,7 @@ public String getResponsibilityUserNew() { } public Map getAllParameters(){ - Map allParameters = new LinkedHashMap(); + Map allParameters = new LinkedHashMap<>(); allParameters.put("teamcity", this.teamcityProperties); allParameters.put("webhook", this.extraParameters); @@ -819,9 +867,6 @@ public ExtraParametersMap getExtraParameters(VariableResolverFactory variableRes VariableResolver resolver = variableResolverFactory.buildVariableResolver(new SimpleSerialiser(), this, getAllParameters()); ExtraParametersMap resolvedParametersMap = new ExtraParametersMap(extraParameters); -// ExtraParametersMap resolvedParametersMap = new ExtraParametersMap(this.teamcityProperties); -// resolvedParametersMap.putAll(extraParameters); - for (Entry entry : extraParameters.getEntriesAsSet()){ builder = variableResolverFactory.createVariableMessageBuilder(entry.getValue(), resolver); resolvedParametersMap.put(entry.getKey(), builder.build()); diff --git a/tcwebhooks-core/src/main/java/webhook/teamcity/payload/content/WebHooksChange.java b/tcwebhooks-core/src/main/java/webhook/teamcity/payload/content/WebHooksChange.java index a38ef852..eafbc662 100644 --- a/tcwebhooks-core/src/main/java/webhook/teamcity/payload/content/WebHooksChange.java +++ b/tcwebhooks-core/src/main/java/webhook/teamcity/payload/content/WebHooksChange.java @@ -3,11 +3,11 @@ import java.util.ArrayList; import java.util.List; +import org.jetbrains.annotations.Nullable; + import jetbrains.buildServer.vcs.SVcsModification; import jetbrains.buildServer.vcs.VcsFileModification; import jetbrains.buildServer.vcs.VcsRootInstance; -import org.jetbrains.annotations.Nullable; - public class WebHooksChange { @@ -28,19 +28,24 @@ public class WebHooksChange { return vcsRoot.getName(); } - public static WebHooksChange build(SVcsModification modification) { + + + public static WebHooksChange build(SVcsModification modification, boolean includeVcsFileModifications) { WebHooksChange change = new WebHooksChange(); change.setComment(modification.getDescription()); change.setUsername(modification.getUserName()); change.setVcsRoot(tryGetVcsRootName(modification)); - for (VcsFileModification fileModification: modification.getChanges()){ - change.files.add(fileModification.getRelativeFileName()); + if (includeVcsFileModifications) { + change.files = new ArrayList<>(); + for (VcsFileModification fileModification: modification.getChanges()){ + change.files.add(fileModification.getRelativeFileName()); + } } return change; } - private List files = new ArrayList<>(); + private List files; private String comment; private String username; private String vcsRoot; diff --git a/tcwebhooks-core/src/main/java/webhook/teamcity/payload/content/WebHooksChangeBuilder.java b/tcwebhooks-core/src/main/java/webhook/teamcity/payload/content/WebHooksChangeBuilder.java index 2a01ede4..9ac913d3 100644 --- a/tcwebhooks-core/src/main/java/webhook/teamcity/payload/content/WebHooksChangeBuilder.java +++ b/tcwebhooks-core/src/main/java/webhook/teamcity/payload/content/WebHooksChangeBuilder.java @@ -8,11 +8,11 @@ public class WebHooksChangeBuilder{ private WebHooksChangeBuilder(){} - public static List build (List mods){ + public static List build (List mods, boolean includeVcsFileModifications){ List changes = new ArrayList<>(); for (SVcsModification modification: mods){ - changes.add(new WebHooksChanges(modification.getDisplayVersion(), WebHooksChange.build(modification))); + changes.add(new WebHooksChanges(modification.getDisplayVersion(), WebHooksChange.build(modification, includeVcsFileModifications))); } return changes; } diff --git a/tcwebhooks-core/src/main/java/webhook/teamcity/payload/format/WebHookPayloadJson.java b/tcwebhooks-core/src/main/java/webhook/teamcity/payload/format/WebHookPayloadJson.java index 03ddbc55..b58ccdd2 100644 --- a/tcwebhooks-core/src/main/java/webhook/teamcity/payload/format/WebHookPayloadJson.java +++ b/tcwebhooks-core/src/main/java/webhook/teamcity/payload/format/WebHookPayloadJson.java @@ -19,7 +19,7 @@ public class WebHookPayloadJson extends WebHookPayloadGeneric implements WebHookPayload { - public static final String FORMAT_SHORT_NAME = "json"; + public static final String FORMAT_SHORT_NAME = "json"; Integer rank = 100; String charset = "UTF-8"; @@ -36,7 +36,7 @@ public String getFormatDescription() { } public String getFormatShortName() { - return FORMAT_SHORT_NAME; + return FORMAT_SHORT_NAME; } public String getFormatToolTipText() { @@ -52,6 +52,7 @@ protected String getStatusAsString(WebHookPayloadContent content,WebHookTemplate xstream.setMode(XStream.NO_REFERENCES); xstream.registerConverter(new ExtraParametersMapToJsonConvertor()); xstream.registerConverter(new UserSingleValueConverter()); + xstream.autodetectAnnotations(true); xstream.alias("build", WebHookPayloadContent.class); /* For some reason, the items are coming back as "@name" and "@value" * so strip those out with a regex. diff --git a/tcwebhooks-core/src/main/java/webhook/teamcity/payload/format/WebHookPayloadXml.java b/tcwebhooks-core/src/main/java/webhook/teamcity/payload/format/WebHookPayloadXml.java index 26b2f34e..7b994fbe 100644 --- a/tcwebhooks-core/src/main/java/webhook/teamcity/payload/format/WebHookPayloadXml.java +++ b/tcwebhooks-core/src/main/java/webhook/teamcity/payload/format/WebHookPayloadXml.java @@ -13,7 +13,7 @@ public class WebHookPayloadXml extends WebHookPayloadGeneric { - public static final String FORMAT_SHORT_NAME = "xml"; + public static final String FORMAT_SHORT_NAME = "xml"; private Integer rank = 100; public WebHookPayloadXml(WebHookPayloadManager wpm, WebHookVariableResolverManager variableResolverManager) { @@ -37,7 +37,7 @@ public String getFormatDescription() { } public String getFormatShortName() { - return FORMAT_SHORT_NAME; + return FORMAT_SHORT_NAME; } public String getFormatToolTipText() { @@ -61,6 +61,7 @@ protected String getStatusAsString(WebHookPayloadContent content, WebHookTemplat XStream xstream = new XStream(); xstream.setMode(XStream.NO_REFERENCES); xstream.registerConverter(new ExtraParametersMapToXmlConvertor()); + xstream.autodetectAnnotations(true); xstream.alias("build", WebHookPayloadContent.class); return xstream.toXML(content); } diff --git a/tcwebhooks-core/src/test/java/webhook/teamcity/payload/content/WebHookPayloadContentChangesTest.java b/tcwebhooks-core/src/test/java/webhook/teamcity/payload/content/WebHookPayloadContentChangesTest.java new file mode 100644 index 00000000..61491d49 --- /dev/null +++ b/tcwebhooks-core/src/test/java/webhook/teamcity/payload/content/WebHookPayloadContentChangesTest.java @@ -0,0 +1,334 @@ +package webhook.teamcity.payload.content; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertTrue; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +import java.util.ArrayList; +import java.util.Date; +import java.util.HashMap; +import java.util.List; +import java.util.Map; + +import org.junit.Before; +import org.junit.Test; +import org.mockito.Mock; +import org.mockito.Mockito; +import org.mockito.MockitoAnnotations; + +import jetbrains.buildServer.StatusDescriptor; +import jetbrains.buildServer.messages.Status; +import jetbrains.buildServer.serverSide.SBuild; +import jetbrains.buildServer.serverSide.SBuildAgent; +import jetbrains.buildServer.serverSide.SBuildServer; +import jetbrains.buildServer.serverSide.SFinishedBuild; +import jetbrains.buildServer.serverSide.SProject; +import jetbrains.buildServer.serverSide.SRunningBuild; +import jetbrains.buildServer.serverSide.TriggeredBy; +import jetbrains.buildServer.vcs.SVcsModification; +import jetbrains.buildServer.vcs.VcsFileModification; +import jetbrains.buildServer.vcs.VcsRootInstance; +import webhook.teamcity.BuildStateEnum; +import webhook.teamcity.MockSBuildType; +import webhook.teamcity.payload.WebHookPayloadDefaultTemplates; +import webhook.teamcity.payload.variableresolver.VariableMessageBuilder; +import webhook.teamcity.payload.variableresolver.VariableResolver; +import webhook.teamcity.payload.variableresolver.VariableResolverFactory; + +public class WebHookPayloadContentChangesTest { + + @Mock + VariableResolverFactory variableResolverFactory; + + @Mock + VariableResolver variableResolver; + + @Mock + VariableMessageBuilder variableMessageBuilder; + + @Mock + SBuildServer sBuildServer; + + @Mock + SFinishedBuild previousBuild; + + @Before + public void setup() { + MockitoAnnotations.initMocks(this); + when(sBuildServer.getRootUrl()).thenReturn("http://localhost/"); + when(variableResolverFactory.createVariableMessageBuilder(any(), any())).thenReturn(variableMessageBuilder); + } + + @Test + public void testTenChangedFilesFoundWithDefaultLimits() { + + SBuild sRunningBuild = getMockedBuild(); + List mod = getMockedChanges(10); + when(sRunningBuild.getContainingChanges()).thenReturn(mod); + + WebHookPayloadContent content = new WebHookPayloadContent( + variableResolverFactory, sBuildServer, + sRunningBuild, previousBuild, + BuildStateEnum.BEFORE_BUILD_FINISHED, + new ExtraParametersMap(new HashMap()), + new ExtraParametersMap(new HashMap()), + WebHookPayloadDefaultTemplates.getDefaultEnabledPayloadTemplates()); + + assertEquals(100, content.getMaxChangeFileListSize()); + assertEquals(10, content.getChangeFileListCount()); + assertEquals(10, content.getChanges().get(0).getChange().getFiles().size()); + // Verify that the changes were loaded once + Mockito.verify(mod.get(0), Mockito.times(1)).getChanges(); + assertFalse(content.isMaxChangeFileListCountExceeded()); + } + + @Test + public void test80ChangedFilesFoundAcross4VcsRootsWithDefaultLimits() { + + SBuild sRunningBuild = getMockedBuild(); + List mod = getMockedChanges(4, 20); + when(sRunningBuild.getContainingChanges()).thenReturn(mod); + + WebHookPayloadContent content = new WebHookPayloadContent( + variableResolverFactory, sBuildServer, + sRunningBuild, previousBuild, + BuildStateEnum.BEFORE_BUILD_FINISHED, + new ExtraParametersMap(new HashMap()), + new ExtraParametersMap(new HashMap()), + WebHookPayloadDefaultTemplates.getDefaultEnabledPayloadTemplates()); + + assertEquals(100, content.getMaxChangeFileListSize()); + assertEquals(80, content.getChangeFileListCount()); + assertEquals(20, content.getChanges().get(0).getChange().getFiles().size()); + // Verify that the changes were loaded once + Mockito.verify(mod.get(0), Mockito.times(1)).getChanges(); + assertFalse(content.isMaxChangeFileListCountExceeded()); + } + + @Test + public void testChangedFilesIsNullWithDefaultLimitsAnd500Files() { + + SBuild sRunningBuild = getMockedBuild(); + List mod = getMockedChanges(500); + when(sRunningBuild.getContainingChanges()).thenReturn(mod); + + WebHookPayloadContent content = new WebHookPayloadContent( + variableResolverFactory, sBuildServer, + sRunningBuild, previousBuild, + BuildStateEnum.BEFORE_BUILD_FINISHED, + new ExtraParametersMap(new HashMap()), + new ExtraParametersMap(new HashMap()), + WebHookPayloadDefaultTemplates.getDefaultEnabledPayloadTemplates()); + + assertEquals(100, content.getMaxChangeFileListSize()); + assertEquals(500, content.getChangeFileListCount()); + assertNull(content.getChanges().get(0).getChange().getFiles()); + // Verify that the changes were never loaded. + // This is the expensive operation we are trying to avoid. + Mockito.verify(mod.get(0), Mockito.times(0)).getChanges(); + assertTrue(content.isMaxChangeFileListCountExceeded()); + } + + @Test + public void testChangedFilesIsNullWithDefaultLimitsAnd120FilesAcrossSixVcsRoots() { + + SBuild sRunningBuild = getMockedBuild(); + List mod = getMockedChanges(6,20); + when(sRunningBuild.getContainingChanges()).thenReturn(mod); + + WebHookPayloadContent content = new WebHookPayloadContent( + variableResolverFactory, sBuildServer, + sRunningBuild, previousBuild, + BuildStateEnum.BEFORE_BUILD_FINISHED, + new ExtraParametersMap(new HashMap()), + new ExtraParametersMap(new HashMap()), + WebHookPayloadDefaultTemplates.getDefaultEnabledPayloadTemplates()); + + assertEquals(100, content.getMaxChangeFileListSize()); + assertEquals(120, content.getChangeFileListCount()); + assertNull(content.getChanges().get(0).getChange().getFiles()); + // Verify that the changes were never loaded. + // This is the expensive operation we are trying to avoid. + Mockito.verify(mod.get(0), Mockito.times(0)).getChanges(); + assertTrue(content.isMaxChangeFileListCountExceeded()); + } + + @Test + public void testChangedFilesIsNullWith50FilesButLimitIs10SetViaTeamCityBuildParameter() { + + SBuild sRunningBuild = getMockedBuild(); + Map teamCityMap = new HashMap<>(); + teamCityMap.put("webhook.maxChangeFileListSize", "10"); + List mod = getMockedChanges(50); + when(sRunningBuild.getContainingChanges()).thenReturn(mod); + + WebHookPayloadContent content = new WebHookPayloadContent( + variableResolverFactory, sBuildServer, + sRunningBuild, previousBuild, + BuildStateEnum.BEFORE_BUILD_FINISHED, + new ExtraParametersMap(new HashMap()), + new ExtraParametersMap(teamCityMap), + WebHookPayloadDefaultTemplates.getDefaultEnabledPayloadTemplates()); + + assertEquals(10, content.getMaxChangeFileListSize()); + assertEquals(50, content.getChangeFileListCount()); + assertNull(content.getChanges().get(0).getChange().getFiles()); + // Verify that the changes were never loaded. + // This is the expensive operation we are trying to avoid. + Mockito.verify(mod.get(0), Mockito.times(0)).getChanges(); + assertTrue(content.isMaxChangeFileListCountExceeded()); + } + + @Test + public void testChangedFilesIsNullWith50FilesButLimitIs20SetViaWebHookProperty() { + + SBuild sRunningBuild = getMockedBuild(); + Map propertiesMap = new HashMap<>(); + propertiesMap.put("maxChangeFileListSize", "20"); + List mod = getMockedChanges(50); + when(sRunningBuild.getContainingChanges()).thenReturn(mod); + + WebHookPayloadContent content = new WebHookPayloadContent( + variableResolverFactory, sBuildServer, + sRunningBuild, previousBuild, + BuildStateEnum.BEFORE_BUILD_FINISHED, + new ExtraParametersMap(propertiesMap), + new ExtraParametersMap(new HashMap()), + WebHookPayloadDefaultTemplates.getDefaultEnabledPayloadTemplates()); + + assertEquals(20, content.getMaxChangeFileListSize()); + assertEquals(50, content.getChangeFileListCount()); + assertNull(content.getChanges().get(0).getChange().getFiles()); + // Verify that the changes were never loaded. + // This is the expensive operation we are trying to avoid. + Mockito.verify(mod.get(0), Mockito.times(0)).getChanges(); + assertTrue(content.isMaxChangeFileListCountExceeded()); + } + + @Test + public void testChangedFilesContainsAllWhenUnlimitedViaWebHookProperty() { + + SBuild sRunningBuild = getMockedBuild(); + Map propertiesMap = new HashMap<>(); + propertiesMap.put("maxChangeFileListSize", "-1"); + List mod = getMockedChanges(500); + when(sRunningBuild.getContainingChanges()).thenReturn(mod); + + WebHookPayloadContent content = new WebHookPayloadContent( + variableResolverFactory, sBuildServer, + sRunningBuild, previousBuild, + BuildStateEnum.BEFORE_BUILD_FINISHED, + new ExtraParametersMap(propertiesMap), + new ExtraParametersMap(new HashMap()), + WebHookPayloadDefaultTemplates.getDefaultEnabledPayloadTemplates()); + + assertEquals(-1, content.getMaxChangeFileListSize()); + assertEquals(500, content.getChangeFileListCount()); + assertEquals(500, content.getChanges().get(0).getChange().getFiles().size()); + Mockito.verify(mod.get(0), Mockito.times(1)).getChanges(); + assertFalse(content.isMaxChangeFileListCountExceeded()); + } + + @Test + public void testChangedFilesContainsAllFromMulitpleChangesWhenUnlimitedViaWebHookProperty() { + + SBuild sRunningBuild = getMockedBuild(); + Map propertiesMap = new HashMap<>(); + propertiesMap.put("maxChangeFileListSize", "-1"); + List mod = getMockedChanges(5, 500); + when(sRunningBuild.getContainingChanges()).thenReturn(mod); + + WebHookPayloadContent content = new WebHookPayloadContent( + variableResolverFactory, sBuildServer, + sRunningBuild, previousBuild, + BuildStateEnum.BEFORE_BUILD_FINISHED, + new ExtraParametersMap(propertiesMap), + new ExtraParametersMap(new HashMap()), + WebHookPayloadDefaultTemplates.getDefaultEnabledPayloadTemplates()); + + assertEquals(-1, content.getMaxChangeFileListSize()); + assertEquals(2500, content.getChangeFileListCount()); + + assertEquals(500, content.getChanges().get(0).getChange().getFiles().size()); + assertEquals(500, content.getChanges().get(1).getChange().getFiles().size()); + assertEquals(500, content.getChanges().get(2).getChange().getFiles().size()); + assertEquals(500, content.getChanges().get(3).getChange().getFiles().size()); + assertEquals(500, content.getChanges().get(4).getChange().getFiles().size()); + Mockito.verify(mod.get(0), Mockito.times(1)).getChanges(); + Mockito.verify(mod.get(1), Mockito.times(1)).getChanges(); + Mockito.verify(mod.get(2), Mockito.times(1)).getChanges(); + Mockito.verify(mod.get(3), Mockito.times(1)).getChanges(); + Mockito.verify(mod.get(4), Mockito.times(1)).getChanges(); + assertFalse(content.isMaxChangeFileListCountExceeded()); + } + + @Test + public void testChangedFilesIsNullWhenDisabledViaTeamCityBuildParameter() { + + SBuild sRunningBuild = getMockedBuild(); + Map teamcityBuildParameters = new HashMap<>(); + teamcityBuildParameters.put("webhook.maxChangeFileListSize", "0"); + List mod = getMockedChanges(50); + when(sRunningBuild.getContainingChanges()).thenReturn(mod); + + WebHookPayloadContent content = new WebHookPayloadContent( + variableResolverFactory, sBuildServer, + sRunningBuild, previousBuild, + BuildStateEnum.BEFORE_BUILD_FINISHED, + new ExtraParametersMap(new HashMap()), + new ExtraParametersMap(teamcityBuildParameters), + WebHookPayloadDefaultTemplates.getDefaultEnabledPayloadTemplates()); + + assertEquals(0, content.getMaxChangeFileListSize()); + assertEquals(50, content.getChangeFileListCount()); + assertNull(content.getChanges().get(0).getChange().getFiles()); + // Verify that the changes were never loaded. + // This is the expensive operation we are trying to avoid. + Mockito.verify(mod.get(0), Mockito.times(0)).getChanges(); + assertTrue(content.isMaxChangeFileListCountExceeded()); + } + + private List getMockedChanges(int fileNumber) { + return getMockedChanges(1, fileNumber); + } + + private List getMockedChanges(int vcsNumber, int fileNumber) { + List mods = new ArrayList<>(); + for (int i = 0; i < vcsNumber; i++) { + SVcsModification mod = mock(SVcsModification.class); + VcsRootInstance vcs = mock(VcsRootInstance.class); + when(vcs.getVcsName()).thenReturn("myVcsName" + i); + when(mod.getVcsRoot()).thenReturn(vcs); + when(mod.getChangeCount()).thenReturn(fileNumber); + List files = new ArrayList<>(); + for (int j = 0; j < fileNumber; j++) { + VcsFileModification fileMod = mock(VcsFileModification.class); + when(fileMod.getRelativeFileName()).thenReturn("myFile" + j + ".txt"); + files.add(fileMod); + } + when(mod.getChanges()).thenReturn(files); + mods.add(mod); + } + return mods; + } + + private SBuild getMockedBuild() { + SProject sProject = mock(SProject.class); + SBuild sBuild = mock(SRunningBuild.class); + MockSBuildType sBuildType = new MockSBuildType("My Name", "My Description", "bt01"); + SBuildAgent buildAgent = mock(SBuildAgent.class); + TriggeredBy triggeredBy = mock(TriggeredBy.class); + sBuildType.setProject(sProject); + when(sBuild.getStartDate()).thenReturn(new Date()); + when(sBuild.getBuildType()).thenReturn(sBuildType); + when(sBuild.getAgent()).thenReturn(buildAgent); + when(sBuild.getTriggeredBy()).thenReturn(triggeredBy); + when(sBuild.getStatusDescriptor()).thenReturn(new StatusDescriptor(Status.NORMAL, "Running")); + return sBuild; + } + +}