From 6d87fe6057f49c18cb243cfadaa93ecd6993c14c Mon Sep 17 00:00:00 2001 From: Rainer Burgstaller Date: Fri, 10 Feb 2017 22:36:49 +0100 Subject: [PATCH 1/3] Support specification of variable patterns - this change allows the use of system environment patterns for project and branch specification such as ${PROJECT_NAME} - Example: the gerrit host has a configured environment variable defined as export PROJECT_NAME=myproject now when we push a review commit for "myproject" then the gerrit trigger will automatically react - one use case for this is to be able to specfiy a generic seed job for a jenkins instance which will automatically trigger a build on the variable based project --- .../hudsontrigger/data/CompareType.java | 10 +++- .../hudsontrigger/data/CompareUtil.java | 50 +++++++++++++++++-- .../help-GerritTriggerConfiguration.html | 4 +- ...GerritProjectWithFilesInterestingTest.java | 16 ++++++ 4 files changed, 72 insertions(+), 8 deletions(-) diff --git a/src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/hudsontrigger/data/CompareType.java b/src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/hudsontrigger/data/CompareType.java index 451efef4e..4925cfdcb 100644 --- a/src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/hudsontrigger/data/CompareType.java +++ b/src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/hudsontrigger/data/CompareType.java @@ -46,7 +46,13 @@ public enum CompareType { /** * Regular expression comparison. */ - REG_EXP(new RegExpCompareUtil()); + REG_EXP(new RegExpCompareUtil()), + + /** + * Plain comparison however, the pattern will be expanded with the system environment. + * This allows comparisons such as "${PROJECTNAME}". + */ + PLAIN_VAR(new CompareUtil.PlainVarCompareUtil()); /** * Gets a list of all CompareType's displayNames. @@ -88,7 +94,7 @@ public static CompareType findByOperator(char operator) { return PLAIN; } - private CompareUtil util; + CompareUtil util; /** * Private Constructor. diff --git a/src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/hudsontrigger/data/CompareUtil.java b/src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/hudsontrigger/data/CompareUtil.java index b8df9df82..d6cad3567 100644 --- a/src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/hudsontrigger/data/CompareUtil.java +++ b/src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/hudsontrigger/data/CompareUtil.java @@ -23,11 +23,14 @@ */ package com.sonyericsson.hudson.plugins.gerrit.trigger.hudsontrigger.data; -import java.io.File; +import hudson.EnvVars; import org.apache.tools.ant.types.selectors.SelectorUtils; +import java.io.File; + /** * Base interface for the compare-algorithms. + * * @author Robert Sandell <robert.sandell@sonyericsson.com> */ public interface CompareUtil { @@ -56,7 +59,7 @@ public interface CompareUtil { * Compares based on Ant-style paths. * like my/**/something*.git */ - static class AntCompareUtil implements CompareUtil { + class AntCompareUtil implements CompareUtil { @Override public boolean matches(String pattern, String str) { @@ -80,9 +83,9 @@ public char getOperator() { } /** - * Compares with pattern.equals(str). + * Compares with pattern.equalsIgnoreCase(str). */ - static class PlainCompareUtil implements CompareUtil { + class PlainCompareUtil implements CompareUtil { @Override public boolean matches(String pattern, String str) { @@ -105,7 +108,7 @@ public char getOperator() { * string.matches(pattern) * @see java.util.regex.Pattern */ - static class RegExpCompareUtil implements CompareUtil { + class RegExpCompareUtil implements CompareUtil { @Override public boolean matches(String pattern, String str) { @@ -122,4 +125,41 @@ public char getOperator() { return '~'; } } + + /** + * Compares just like plain comparison, however, patterns are expanded using the + * given system environment settings. + */ + class PlainVarCompareUtil implements CompareUtil { + private final EnvVars systemEnvVars; + + public PlainVarCompareUtil() { + systemEnvVars = new EnvVars(EnvVars.masterEnvVars); + } + + /** + * Test only method to allow injecting test properties into our environment. + * @param key the key to inject into the env vars. + * @param value the value associated with the key. + */ + void inject(String key, String value) { + systemEnvVars.put(key, value); + } + + @Override + public boolean matches(String pattern, String str) { + String expandedPattern = systemEnvVars.expand(pattern); + return expandedPattern.equalsIgnoreCase(str); + } + + @Override + public String getName() { + return "PlainVar"; + } + + @Override + public char getOperator() { + return ':'; + } + } } diff --git a/src/main/webapp/trigger/help-GerritTriggerConfiguration.html b/src/main/webapp/trigger/help-GerritTriggerConfiguration.html index ef4bd54ff..fe27dc7bd 100644 --- a/src/main/webapp/trigger/help-GerritTriggerConfiguration.html +++ b/src/main/webapp/trigger/help-GerritTriggerConfiguration.html @@ -8,7 +8,9 @@ You can specify the name pattern in three different ways, as provided by the "Type" drop-down menu.

diff --git a/src/test/java/com/sonyericsson/hudson/plugins/gerrit/trigger/hudsontrigger/data/GerritProjectWithFilesInterestingTest.java b/src/test/java/com/sonyericsson/hudson/plugins/gerrit/trigger/hudsontrigger/data/GerritProjectWithFilesInterestingTest.java index 1d93c3a98..a27893a89 100644 --- a/src/test/java/com/sonyericsson/hudson/plugins/gerrit/trigger/hudsontrigger/data/GerritProjectWithFilesInterestingTest.java +++ b/src/test/java/com/sonyericsson/hudson/plugins/gerrit/trigger/hudsontrigger/data/GerritProjectWithFilesInterestingTest.java @@ -23,6 +23,7 @@ */ package com.sonyericsson.hudson.plugins.gerrit.trigger.hudsontrigger.data; +import org.junit.BeforeClass; import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.Parameterized; @@ -60,6 +61,13 @@ public void testInteresting() { scenarioWithFiles.project, scenarioWithFiles.branch, scenarioWithFiles.topic, scenarioWithFiles.files)); } + @BeforeClass + public static void setupEnv() { + // this is a somewhat hacky way to deal with the fact that the compare util was already set up before + // we get the chance to update the system environment + ((CompareUtil.PlainVarCompareUtil) CompareType.PLAIN_VAR.util).inject("PROJECTNAME", "myproject"); + } + /** * The parameters. * @return parameters @@ -213,6 +221,14 @@ public static Collection getParameters() { parameters.add(new InterestingScenarioWithFiles[]{new InterestingScenarioWithFiles( config, "vendor/semc/master/project", "origin/master", null, files, false), }); + branches = new LinkedList(); + branch = new Branch(CompareType.PLAIN, "master"); + branches.add(branch); + config = new GerritProject(CompareType.PLAIN_VAR, "${PROJECTNAME}", branches, null, null, null, + false); + parameters.add(new InterestingScenarioWithFiles[]{new InterestingScenarioWithFiles( + config, "myproject", "master", null, null, true),}); + return parameters; } From e52b82c48ef028f6a142b3a9ddee7cee3dc68b58 Mon Sep 17 00:00:00 2001 From: Rainer Burgstaller Date: Tue, 21 Feb 2017 06:54:38 +0100 Subject: [PATCH 2/3] implemented review comments, improved env var handling --- .../trigger/hudsontrigger/GerritTrigger.java | 68 ++++++++++++++++--- .../trigger/hudsontrigger/data/Branch.java | 6 +- .../hudsontrigger/data/CompareType.java | 15 ++-- .../hudsontrigger/data/CompareUtil.java | 56 ++++----------- .../trigger/hudsontrigger/data/FilePath.java | 6 +- .../hudsontrigger/data/GerritProject.java | 32 +++++---- .../trigger/hudsontrigger/data/Topic.java | 6 +- .../help-GerritTriggerConfiguration.html | 5 +- .../hudsontrigger/GerritTriggerTest.java | 11 +-- .../data/GerritProjectInterestingTest.java | 7 +- ...GerritProjectWithFilesInterestingTest.java | 15 ++-- 11 files changed, 129 insertions(+), 98 deletions(-) diff --git a/src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/hudsontrigger/GerritTrigger.java b/src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/hudsontrigger/GerritTrigger.java index 56c00415a..d1be8f5f7 100644 --- a/src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/hudsontrigger/GerritTrigger.java +++ b/src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/hudsontrigger/GerritTrigger.java @@ -67,6 +67,7 @@ import com.sonymobile.tools.gerrit.gerritevents.dto.events.RefUpdated; import com.sonymobile.tools.gerrit.gerritevents.dto.rest.Notify; +import hudson.EnvVars; import hudson.Extension; import hudson.ExtensionList; import hudson.Util; @@ -85,8 +86,12 @@ import hudson.model.Queue; import hudson.model.Run; import hudson.model.Result; +import hudson.slaves.EnvironmentVariablesNodeProperty; +import hudson.slaves.NodeProperty; +import hudson.slaves.NodePropertyDescriptor; import hudson.triggers.Trigger; import hudson.triggers.TriggerDescriptor; +import hudson.util.DescribableList; import hudson.util.FormValidation; import hudson.util.ListBoxModel; import hudson.util.ListBoxModel.Option; @@ -178,6 +183,9 @@ public class GerritTrigger extends Trigger { private List triggerOnEvents; private boolean dynamicTriggerConfiguration; private String triggerConfigURL; + private final EnvVars envVars; + private int globalHashVarsHash; + private int systemEnvVarsHash; private GerritTriggerTimerTask gerritTriggerTimerTask; private GerritTriggerInformationAction triggerInformationAction; @@ -194,8 +202,10 @@ public GerritTrigger(List gerritProjects) { this.skipVote = new SkipVote(false, false, false, false); this.escapeQuotes = true; this.serverName = ANY_SERVER; + this.envVars = initEnvVars(); + try { - DescriptorImpl descriptor = (DescriptorImpl)getDescriptor(); + DescriptorImpl descriptor = (DescriptorImpl) getDescriptor(); if (descriptor != null) { ListBoxModel options = descriptor.doFillNotificationLevelItems(this.serverName); if (!options.isEmpty()) { @@ -334,6 +344,23 @@ public GerritTrigger(List gerritProjects, SkipVote skipVote, Inte this.gerritTriggerTimerTask = null; this.triggerInformationAction = new GerritTriggerInformationAction(); this.notificationLevel = notificationLevel; + this.envVars = initEnvVars(); + } + + /** + * Initializes the environment variables used for expansion. + */ + private EnvVars initEnvVars() { + EnvVars result = new EnvVars(EnvVars.masterEnvVars); + DescribableList, NodePropertyDescriptor> globalProps = + Jenkins.getActiveInstance().getGlobalNodeProperties(); + EnvironmentVariablesNodeProperty envVarsProp = globalProps.get(EnvironmentVariablesNodeProperty.class); + EnvVars globalEnvVars = envVarsProp.getEnvVars(); + result.putAll(globalEnvVars); + + systemEnvVarsHash = EnvVars.masterEnvVars.hashCode(); + globalHashVarsHash = globalEnvVars.hashCode(); + return result; } /** @@ -960,14 +987,15 @@ public boolean equals(Object obj) { } logger.trace("entering isInteresting projects configured: {} the event: {}", allGerritProjects.size(), event); + updateEnvVarsIfNecessary(); for (GerritProject p : allGerritProjects) { try { if (event instanceof ChangeBasedEvent) { ChangeBasedEvent changeBasedEvent = (ChangeBasedEvent)event; if (isServerInteresting(event) - && p.isInteresting(changeBasedEvent.getChange().getProject(), - changeBasedEvent.getChange().getBranch(), - changeBasedEvent.getChange().getTopic())) { + && p.isInteresting(changeBasedEvent.getChange().getProject(), + changeBasedEvent.getChange().getBranch(), + changeBasedEvent.getChange().getTopic(), envVars)) { boolean containsFilePathsOrForbiddenFilePaths = ((p.getFilePaths() != null && p.getFilePaths().size() > 0) @@ -975,11 +1003,11 @@ public boolean equals(Object obj) { if (isFileTriggerEnabled() && containsFilePathsOrForbiddenFilePaths) { if (isServerInteresting(event) - && p.isInteresting(changeBasedEvent.getChange().getProject(), - changeBasedEvent.getChange().getBranch(), - changeBasedEvent.getChange().getTopic(), - changeBasedEvent.getFiles( - new GerritQueryHandler(getServerConfig(event))))) { + && p.isInteresting(changeBasedEvent.getChange().getProject(), + changeBasedEvent.getChange().getBranch(), + changeBasedEvent.getChange().getTopic(), + changeBasedEvent.getFiles( + new GerritQueryHandler(getServerConfig(event))), envVars)) { logger.trace("According to {} the event is interesting.", p); return true; } @@ -991,20 +1019,38 @@ public boolean equals(Object obj) { } else if (event instanceof RefUpdated) { RefUpdated refUpdated = (RefUpdated)event; if (isServerInteresting(event) && p.isInteresting(refUpdated.getRefUpdate().getProject(), - refUpdated.getRefUpdate().getRefName(), null)) { + refUpdated.getRefUpdate().getRefName(), null, envVars)) { logger.trace("According to {} the event is interesting.", p); return true; } } } catch (PatternSyntaxException pse) { logger.error(MessageFormat.format("Exception caught for project {0} and pattern {1}, message: {2}", - new Object[]{job.getName(), p.getPattern(), pse.getMessage()})); + new Object[]{job.getName(), p.getPattern(), pse.getMessage()})); } } logger.trace("Nothing interesting here, move along folks!"); return false; } + /** + * Checks if the current system env vars are still the same and updates our local copy if necessary + */ + private void updateEnvVarsIfNecessary() { + EnvironmentVariablesNodeProperty envVarsProp = + Jenkins.getActiveInstance().getGlobalNodeProperties().get(EnvironmentVariablesNodeProperty.class); + EnvVars globalEnvVars = envVarsProp.getEnvVars(); + int newSystemEnvVarsHash = EnvVars.masterEnvVars.hashCode(); + int newGlobalEnvVarsHash = globalEnvVars.hashCode(); + if (newSystemEnvVarsHash != systemEnvVarsHash || newGlobalEnvVarsHash != globalHashVarsHash) { + this.envVars.clear(); + this.envVars.putAll(EnvVars.masterEnvVars); + this.envVars.putAll(globalEnvVars); + systemEnvVarsHash = newSystemEnvVarsHash; + globalHashVarsHash = newGlobalEnvVarsHash; + } + } + /** * Check whether the event provider contains the same server name as the serverName field. * diff --git a/src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/hudsontrigger/data/Branch.java b/src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/hudsontrigger/data/Branch.java index d884dfb8d..1831d11c1 100644 --- a/src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/hudsontrigger/data/Branch.java +++ b/src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/hudsontrigger/data/Branch.java @@ -23,6 +23,7 @@ */ package com.sonyericsson.hudson.plugins.gerrit.trigger.hudsontrigger.data; +import hudson.EnvVars; import hudson.Extension; import hudson.model.AbstractDescribableImpl; import hudson.model.Descriptor; @@ -90,10 +91,11 @@ public void setPattern(String pattern) { /** * Tells if the given branch is matched by this rule. * @param branch the branch + * @param envVars the environment variables exisiting on the jenkins host. * @return true if the branch matches. */ - public boolean isInteresting(String branch) { - return compareType.matches(pattern, branch); + public boolean isInteresting(String branch, EnvVars envVars) { + return compareType.matches(pattern, branch, envVars); } /** diff --git a/src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/hudsontrigger/data/CompareType.java b/src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/hudsontrigger/data/CompareType.java index 4925cfdcb..0fe1a403e 100644 --- a/src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/hudsontrigger/data/CompareType.java +++ b/src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/hudsontrigger/data/CompareType.java @@ -26,6 +26,8 @@ import com.sonyericsson.hudson.plugins.gerrit.trigger.hudsontrigger.data.CompareUtil.AntCompareUtil; import com.sonyericsson.hudson.plugins.gerrit.trigger.hudsontrigger.data.CompareUtil.PlainCompareUtil; import com.sonyericsson.hudson.plugins.gerrit.trigger.hudsontrigger.data.CompareUtil.RegExpCompareUtil; +import hudson.EnvVars; + import java.util.LinkedList; import java.util.List; @@ -46,13 +48,7 @@ public enum CompareType { /** * Regular expression comparison. */ - REG_EXP(new RegExpCompareUtil()), - - /** - * Plain comparison however, the pattern will be expanded with the system environment. - * This allows comparisons such as "${PROJECTNAME}". - */ - PLAIN_VAR(new CompareUtil.PlainVarCompareUtil()); + REG_EXP(new RegExpCompareUtil()); /** * Gets a list of all CompareType's displayNames. @@ -108,10 +104,11 @@ private CompareType(CompareUtil util) { * Tells if the given string matches the given pattern based on the algorithm of this CompareType instance. * @param pattern the pattern * @param str the string + * @param envVars the environment variables exisiting on the jenkins host. * @return true if the string matches the pattern. */ - public boolean matches(String pattern, String str) { - return util.matches(pattern, str); + public boolean matches(String pattern, String str, EnvVars envVars) { + return util.matches(pattern, str, envVars); } /** diff --git a/src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/hudsontrigger/data/CompareUtil.java b/src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/hudsontrigger/data/CompareUtil.java index d6cad3567..6063357f0 100644 --- a/src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/hudsontrigger/data/CompareUtil.java +++ b/src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/hudsontrigger/data/CompareUtil.java @@ -39,9 +39,10 @@ public interface CompareUtil { * Tells if the given pattern matches the string according to the implemented comparer/algorithm. * @param pattern the pattern to use. * @param str the string to match on. + * @param envVars the environment variables exisiting on the jenkins host. * @return true if the string matches the pattern. */ - boolean matches(String pattern, String str); + boolean matches(String pattern, String str, EnvVars envVars); /** * Returns the human-readable name of the util. @@ -62,13 +63,14 @@ public interface CompareUtil { class AntCompareUtil implements CompareUtil { @Override - public boolean matches(String pattern, String str) { + public boolean matches(String pattern, String str, EnvVars envVars) { // Replace the Git directory separator character (always '/') // with the platform specific directory separator before // invoking Ant's platform specific path matching. String safePattern = pattern.replace('/', File.separatorChar); + String expandedPattern = envVars.expand(safePattern); String safeStr = str.replace('/', File.separatorChar); - return SelectorUtils.matchPath(safePattern, safeStr); + return SelectorUtils.matchPath(expandedPattern, safeStr); } @Override @@ -88,8 +90,10 @@ public char getOperator() { class PlainCompareUtil implements CompareUtil { @Override - public boolean matches(String pattern, String str) { - return pattern.equalsIgnoreCase(str); + public boolean matches(String pattern, String str, EnvVars envVars) { + String expandedPattern = envVars.expand(pattern); + + return expandedPattern.equalsIgnoreCase(str); } @Override @@ -111,8 +115,9 @@ public char getOperator() { class RegExpCompareUtil implements CompareUtil { @Override - public boolean matches(String pattern, String str) { - return str.matches(pattern); + public boolean matches(String pattern, String str, EnvVars envVars) { + String expandedPattern = envVars.expand(pattern); + return str.matches(expandedPattern); } @Override @@ -125,41 +130,4 @@ public char getOperator() { return '~'; } } - - /** - * Compares just like plain comparison, however, patterns are expanded using the - * given system environment settings. - */ - class PlainVarCompareUtil implements CompareUtil { - private final EnvVars systemEnvVars; - - public PlainVarCompareUtil() { - systemEnvVars = new EnvVars(EnvVars.masterEnvVars); - } - - /** - * Test only method to allow injecting test properties into our environment. - * @param key the key to inject into the env vars. - * @param value the value associated with the key. - */ - void inject(String key, String value) { - systemEnvVars.put(key, value); - } - - @Override - public boolean matches(String pattern, String str) { - String expandedPattern = systemEnvVars.expand(pattern); - return expandedPattern.equalsIgnoreCase(str); - } - - @Override - public String getName() { - return "PlainVar"; - } - - @Override - public char getOperator() { - return ':'; - } - } } diff --git a/src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/hudsontrigger/data/FilePath.java b/src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/hudsontrigger/data/FilePath.java index 1d332fe3e..3d2851ed4 100644 --- a/src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/hudsontrigger/data/FilePath.java +++ b/src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/hudsontrigger/data/FilePath.java @@ -23,6 +23,7 @@ */ package com.sonyericsson.hudson.plugins.gerrit.trigger.hudsontrigger.data; +import hudson.EnvVars; import hudson.Extension; import hudson.model.AbstractDescribableImpl; import hudson.model.Descriptor; @@ -92,11 +93,12 @@ public void setPattern(String pattern) { /** * Tells if the given files are matched by this rule. * @param files the files in the patch set. + * @param envVars the environment variables exisiting on the jenkins host. * @return true if the files match. */ - public boolean isInteresting(List files) { + public boolean isInteresting(List files, EnvVars envVars) { for (String file : files) { - if (compareType.matches(pattern, file)) { + if (compareType.matches(pattern, file, envVars)) { return true; } } diff --git a/src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/hudsontrigger/data/GerritProject.java b/src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/hudsontrigger/data/GerritProject.java index 693e834e6..554c2d71e 100644 --- a/src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/hudsontrigger/data/GerritProject.java +++ b/src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/hudsontrigger/data/GerritProject.java @@ -25,6 +25,8 @@ package com.sonyericsson.hudson.plugins.gerrit.trigger.hudsontrigger.data; import static com.sonyericsson.hudson.plugins.gerrit.trigger.GerritServer.ANY_SERVER; + +import hudson.EnvVars; import hudson.Extension; import hudson.RelativePath; import hudson.model.Describable; @@ -210,23 +212,24 @@ public void setForbiddenFilePaths(List forbiddenFilePaths) { * @param branch the branch. * @param topic the topic. * @param files the files. + * @param envVars the environment variables exisiting on the jenkins host. * @return true is the rules match. */ - public boolean isInteresting(String project, String branch, String topic, List files) { - if (compareType.matches(pattern, project)) { + public boolean isInteresting(String project, String branch, String topic, List files, EnvVars envVars) { + if (compareType.matches(pattern, project, envVars)) { for (Branch b : branches) { boolean foundInterestingForbidden = false; boolean foundInterestingTopicOrFile = false; - if (b.isInteresting(branch)) { + if (b.isInteresting(branch, envVars)) { if (forbiddenFilePaths != null) { for (FilePath ffp : forbiddenFilePaths) { - if (ffp.isInteresting(files)) { + if (ffp.isInteresting(files, envVars)) { foundInterestingForbidden = true; break; } } } - if (isInterestingTopic(topic) && isInterestingFile(files)) { + if (isInterestingTopic(topic, envVars) && isInterestingFile(files, envVars)) { foundInterestingTopicOrFile = true; } if (disableStrictForbiddenFileVerification) { @@ -253,13 +256,14 @@ public boolean isInteresting(String project, String branch, String topic, List 0) { for (Topic t : topics) { - if (t.isInteresting(topic)) { + if (t.isInteresting(topic, envVars)) { return true; } } @@ -288,12 +293,13 @@ private boolean isInterestingTopic(String topic) { * Compare files to see if the rules specified is a match. * * @param files the files. + * @param envVars the environment variables exisiting on the jenkins host. * @return true if the rules match or no rules. */ - private boolean isInterestingFile(List files) { + private boolean isInterestingFile(List files, EnvVars envVars) { if (filePaths != null && filePaths.size() > 0) { for (FilePath f : filePaths) { - if (f.isInteresting(files)) { + if (f.isInteresting(files, envVars)) { return true; } } diff --git a/src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/hudsontrigger/data/Topic.java b/src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/hudsontrigger/data/Topic.java index 4834d4d2b..416fafce1 100644 --- a/src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/hudsontrigger/data/Topic.java +++ b/src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/hudsontrigger/data/Topic.java @@ -23,6 +23,7 @@ */ package com.sonyericsson.hudson.plugins.gerrit.trigger.hudsontrigger.data; +import hudson.EnvVars; import hudson.Extension; import hudson.model.AbstractDescribableImpl; import hudson.model.Descriptor; @@ -89,13 +90,14 @@ public void setPattern(String pattern) { /** * Tells if the given topic are matched by this rule. * @param topic the topic in change. + * @param envVars the environment variables exisiting on the jenkins host. * @return true if the topic match. */ - public boolean isInteresting(String topic) { + public boolean isInteresting(String topic, EnvVars envVars) { if (topic == null) { topic = ""; } - if (compareType.matches(pattern, topic)) { + if (compareType.matches(pattern, topic, envVars)) { return true; } return false; diff --git a/src/main/webapp/trigger/help-GerritTriggerConfiguration.html b/src/main/webapp/trigger/help-GerritTriggerConfiguration.html index fe27dc7bd..b1257972b 100644 --- a/src/main/webapp/trigger/help-GerritTriggerConfiguration.html +++ b/src/main/webapp/trigger/help-GerritTriggerConfiguration.html @@ -8,9 +8,8 @@ You can specify the name pattern in three different ways, as provided by the "Type" drop-down menu.

    -
  • Plain: The exact name in Gerrit, case insensitive equality.
    • -
  • PlainVar: The exact name in Gerrit, case insensitive equality, allows usage of system environment - variables defined on the gerrit host (e.g. ${ENVIRONMENT}).
    • +
  • Plain: The exact name in Gerrit, case insensitive equality, allows usage of system environment + variables defined on the jenkins host (e.g. ${ENVIRONMENT}).
  • Path: ANT style pattern. Ex: "**/base/*"
  • RegExp: Regular expression.
diff --git a/src/test/java/com/sonyericsson/hudson/plugins/gerrit/trigger/hudsontrigger/GerritTriggerTest.java b/src/test/java/com/sonyericsson/hudson/plugins/gerrit/trigger/hudsontrigger/GerritTriggerTest.java index 84177b39a..31c130f9b 100644 --- a/src/test/java/com/sonyericsson/hudson/plugins/gerrit/trigger/hudsontrigger/GerritTriggerTest.java +++ b/src/test/java/com/sonyericsson/hudson/plugins/gerrit/trigger/hudsontrigger/GerritTriggerTest.java @@ -49,6 +49,7 @@ import com.sonymobile.tools.gerrit.gerritevents.dto.events.PatchsetCreated; import com.sonymobile.tools.gerrit.gerritevents.dto.events.RefUpdated; +import hudson.EnvVars; import hudson.model.AbstractBuild; import hudson.model.AbstractProject; import hudson.model.Action; @@ -758,7 +759,7 @@ public void testGerritEvent() { PowerMockito.when(ToGerritRunListener.getInstance()).thenReturn(listener); GerritProject gP = mock(GerritProject.class); - doReturn(true).when(gP).isInteresting(any(String.class), any(String.class), any(String.class)); + doReturn(true).when(gP).isInteresting(any(String.class), any(String.class), any(String.class), any(EnvVars.class)); when(gP.getFilePaths()).thenReturn(null); @@ -838,7 +839,7 @@ public void testGerritEventNotInteresting() { PowerMockito.when(ToGerritRunListener.getInstance()).thenReturn(listener); GerritProject gP = mock(GerritProject.class); - doReturn(false).when(gP).isInteresting(any(String.class), any(String.class), any(String.class)); + doReturn(false).when(gP).isInteresting(any(String.class), any(String.class), any(String.class), any(EnvVars.class)); when(gP.getFilePaths()).thenReturn(null); GerritTrigger trigger = Setup.createDefaultTrigger(project); @@ -882,7 +883,7 @@ public void testGerritEventManualEvent() { PowerMockito.when(ToGerritRunListener.getInstance()).thenReturn(listener); GerritProject gP = mock(GerritProject.class); - doReturn(true).when(gP).isInteresting(any(String.class), any(String.class), any(String.class)); + doReturn(true).when(gP).isInteresting(any(String.class), any(String.class), any(String.class), any(EnvVars.class)); when(gP.getFilePaths()).thenReturn(null); GerritTrigger trigger = Setup.createDefaultTrigger(project); @@ -1029,7 +1030,7 @@ public void testGerritEventSilentMode() { PowerMockito.when(ToGerritRunListener.getInstance()).thenReturn(listener); GerritProject gP = mock(GerritProject.class); - doReturn(true).when(gP).isInteresting(any(String.class), any(String.class), any(String.class)); + doReturn(true).when(gP).isInteresting(any(String.class), any(String.class), any(String.class), any(EnvVars.class)); when(gP.getFilePaths()).thenReturn(null); GerritTrigger trigger = Setup.createDefaultTrigger(null); @@ -1060,7 +1061,7 @@ public void testGerritEventManualEventSilentMode() { PowerMockito.when(ToGerritRunListener.getInstance()).thenReturn(listener); GerritProject gP = mock(GerritProject.class); - doReturn(true).when(gP).isInteresting(any(String.class), any(String.class), any(String.class)); + doReturn(true).when(gP).isInteresting(any(String.class), any(String.class), any(String.class), any(EnvVars.class)); when(gP.getFilePaths()).thenReturn(null); GerritTrigger trigger = Setup.createDefaultTrigger(project); diff --git a/src/test/java/com/sonyericsson/hudson/plugins/gerrit/trigger/hudsontrigger/data/GerritProjectInterestingTest.java b/src/test/java/com/sonyericsson/hudson/plugins/gerrit/trigger/hudsontrigger/data/GerritProjectInterestingTest.java index 015916357..249c51516 100644 --- a/src/test/java/com/sonyericsson/hudson/plugins/gerrit/trigger/hudsontrigger/data/GerritProjectInterestingTest.java +++ b/src/test/java/com/sonyericsson/hudson/plugins/gerrit/trigger/hudsontrigger/data/GerritProjectInterestingTest.java @@ -23,6 +23,7 @@ */ package com.sonyericsson.hudson.plugins.gerrit.trigger.hudsontrigger.data; +import hudson.EnvVars; import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.Parameterized; @@ -42,6 +43,7 @@ public class GerritProjectInterestingTest { private final InterestingScenario scenario; + private EnvVars envVars; /** * Constructor. @@ -49,17 +51,18 @@ public class GerritProjectInterestingTest { */ public GerritProjectInterestingTest(InterestingScenario scenario) { this.scenario = scenario; + envVars = new EnvVars(); } /** - * Tests {@link GerritProject#isInteresting(String, String, String)}. + * Tests {@link GerritProject#isInteresting(String, String, String, hudson.EnvVars)}. */ @Test public void testInteresting() { assertEquals(scenario.expected, scenario.config.isInteresting( scenario.project, scenario.branch, - scenario.topic)); + scenario.topic, envVars)); } /** diff --git a/src/test/java/com/sonyericsson/hudson/plugins/gerrit/trigger/hudsontrigger/data/GerritProjectWithFilesInterestingTest.java b/src/test/java/com/sonyericsson/hudson/plugins/gerrit/trigger/hudsontrigger/data/GerritProjectWithFilesInterestingTest.java index a27893a89..ba7e438c6 100644 --- a/src/test/java/com/sonyericsson/hudson/plugins/gerrit/trigger/hudsontrigger/data/GerritProjectWithFilesInterestingTest.java +++ b/src/test/java/com/sonyericsson/hudson/plugins/gerrit/trigger/hudsontrigger/data/GerritProjectWithFilesInterestingTest.java @@ -23,6 +23,7 @@ */ package com.sonyericsson.hudson.plugins.gerrit.trigger.hudsontrigger.data; +import hudson.EnvVars; import org.junit.BeforeClass; import org.junit.Test; import org.junit.runner.RunWith; @@ -43,7 +44,7 @@ public class GerritProjectWithFilesInterestingTest { private final InterestingScenarioWithFiles scenarioWithFiles; - + private static EnvVars envVars = new EnvVars(); /** * Constructor. * @param scenarioWithFiles scenarioWithFiles @@ -53,19 +54,23 @@ public GerritProjectWithFilesInterestingTest(InterestingScenarioWithFiles scenar } /** - * Tests {@link GerritProject#isInteresting(String, String, String, java.util.List)}. + * Tests {@link GerritProject#isInteresting(String, String, String, List, hudson.EnvVars)}. */ @Test public void testInteresting() { assertEquals(scenarioWithFiles.expected, scenarioWithFiles.config.isInteresting( - scenarioWithFiles.project, scenarioWithFiles.branch, scenarioWithFiles.topic, scenarioWithFiles.files)); + scenarioWithFiles.project, scenarioWithFiles.branch, scenarioWithFiles.topic, scenarioWithFiles.files, envVars)); } + + /** + * Sets up the test environment. + */ @BeforeClass public static void setupEnv() { // this is a somewhat hacky way to deal with the fact that the compare util was already set up before // we get the chance to update the system environment - ((CompareUtil.PlainVarCompareUtil) CompareType.PLAIN_VAR.util).inject("PROJECTNAME", "myproject"); + envVars.put("PROJECTNAME", "myproject"); } /** @@ -224,7 +229,7 @@ public static Collection getParameters() { branches = new LinkedList(); branch = new Branch(CompareType.PLAIN, "master"); branches.add(branch); - config = new GerritProject(CompareType.PLAIN_VAR, "${PROJECTNAME}", branches, null, null, null, + config = new GerritProject(CompareType.PLAIN, "${PROJECTNAME}", branches, null, null, null, false); parameters.add(new InterestingScenarioWithFiles[]{new InterestingScenarioWithFiles( config, "myproject", "master", null, null, true),}); From e54aa5696bdfbb45a5f7d5902dd82da85810cfe1 Mon Sep 17 00:00:00 2001 From: Rainer Burgstaller Date: Wed, 1 Mar 2017 06:32:33 +0100 Subject: [PATCH 3/3] implemented further review comments - now we synchronize access to the global EnvVars in order to be thread safe - keep original public API and deprecate it --- .../trigger/hudsontrigger/GerritTrigger.java | 3 +- .../hudsontrigger/data/CompareUtil.java | 44 ++++++++++++++++--- .../hudsontrigger/data/GerritProject.java | 13 +++++- 3 files changed, 51 insertions(+), 9 deletions(-) diff --git a/src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/hudsontrigger/GerritTrigger.java b/src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/hudsontrigger/GerritTrigger.java index d1be8f5f7..4b09edbd5 100644 --- a/src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/hudsontrigger/GerritTrigger.java +++ b/src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/hudsontrigger/GerritTrigger.java @@ -1036,12 +1036,13 @@ public boolean equals(Object obj) { /** * Checks if the current system env vars are still the same and updates our local copy if necessary */ - private void updateEnvVarsIfNecessary() { + private synchronized void updateEnvVarsIfNecessary() { EnvironmentVariablesNodeProperty envVarsProp = Jenkins.getActiveInstance().getGlobalNodeProperties().get(EnvironmentVariablesNodeProperty.class); EnvVars globalEnvVars = envVarsProp.getEnvVars(); int newSystemEnvVarsHash = EnvVars.masterEnvVars.hashCode(); int newGlobalEnvVarsHash = globalEnvVars.hashCode(); + if (newSystemEnvVarsHash != systemEnvVarsHash || newGlobalEnvVarsHash != globalHashVarsHash) { this.envVars.clear(); this.envVars.putAll(EnvVars.masterEnvVars); diff --git a/src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/hudsontrigger/data/CompareUtil.java b/src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/hudsontrigger/data/CompareUtil.java index 6063357f0..fc6b32c94 100644 --- a/src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/hudsontrigger/data/CompareUtil.java +++ b/src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/hudsontrigger/data/CompareUtil.java @@ -39,7 +39,16 @@ public interface CompareUtil { * Tells if the given pattern matches the string according to the implemented comparer/algorithm. * @param pattern the pattern to use. * @param str the string to match on. - * @param envVars the environment variables exisiting on the jenkins host. + * @return true if the string matches the pattern. + * @deprecated use {@link #matches(String, String, EnvVars)} instead. + */ + boolean matches(String pattern, String str); + + /** + * Tells if the given pattern matches the string according to the implemented comparer/algorithm. + * @param pattern the pattern to use. + * @param str the string to match on. + * @param envVars the environment variables exisiting on the jenkins host. Can be {@code null}. * @return true if the string matches the pattern. */ boolean matches(String pattern, String str, EnvVars envVars); @@ -56,11 +65,32 @@ public interface CompareUtil { */ char getOperator(); + abstract class AbstractCompareUtil implements CompareUtil { + @Override + public boolean matches(String pattern, String str) { + return matches(pattern, str, null); + } + + /** + * Expands the pattern in case it contains tokens that can be resolved with the given envVars. + * @param pattern the pattern to expand. + * @param envVars the envVars to use for expansion, if {@code null} then the unmodified pattern will be returned + * @return the expanded string, if no envVars were provided will return the initial unmodified pattern + */ + protected String expandWithEnvVarsIfPossible(String pattern, EnvVars envVars) { + if (envVars != null) { + return envVars.expand(pattern); + } else { + return pattern; + } + } + } + /** * Compares based on Ant-style paths. * like my/**/something*.git */ - class AntCompareUtil implements CompareUtil { + class AntCompareUtil extends AbstractCompareUtil { @Override public boolean matches(String pattern, String str, EnvVars envVars) { @@ -68,7 +98,7 @@ public boolean matches(String pattern, String str, EnvVars envVars) { // with the platform specific directory separator before // invoking Ant's platform specific path matching. String safePattern = pattern.replace('/', File.separatorChar); - String expandedPattern = envVars.expand(safePattern); + String expandedPattern = expandWithEnvVarsIfPossible(safePattern, envVars); String safeStr = str.replace('/', File.separatorChar); return SelectorUtils.matchPath(expandedPattern, safeStr); } @@ -87,11 +117,11 @@ public char getOperator() { /** * Compares with pattern.equalsIgnoreCase(str). */ - class PlainCompareUtil implements CompareUtil { + class PlainCompareUtil extends AbstractCompareUtil { @Override public boolean matches(String pattern, String str, EnvVars envVars) { - String expandedPattern = envVars.expand(pattern); + String expandedPattern = expandWithEnvVarsIfPossible(pattern, envVars); return expandedPattern.equalsIgnoreCase(str); } @@ -112,11 +142,11 @@ public char getOperator() { * string.matches(pattern) * @see java.util.regex.Pattern */ - class RegExpCompareUtil implements CompareUtil { + class RegExpCompareUtil extends AbstractCompareUtil { @Override public boolean matches(String pattern, String str, EnvVars envVars) { - String expandedPattern = envVars.expand(pattern); + String expandedPattern = expandWithEnvVarsIfPossible(pattern, envVars); return str.matches(expandedPattern); } diff --git a/src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/hudsontrigger/data/GerritProject.java b/src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/hudsontrigger/data/GerritProject.java index 554c2d71e..e45d140e4 100644 --- a/src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/hudsontrigger/data/GerritProject.java +++ b/src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/hudsontrigger/data/GerritProject.java @@ -256,7 +256,18 @@ public boolean isInteresting(String project, String branch, String topic, List