jwmach1 / parameterized-scheduler

A Jenkins Plugin to support setting parameters in the build schedule
MIT License
15 stars 52 forks source link

'You can only use one percent sign to separate the cron from the parameters.' displayed while shouldn't. #2

Open SialSurion opened 8 years ago

SialSurion commented 8 years ago

Hi,

While I wrote a schedule:

0-15 17 * * 3 % environment=RC
0-15 17 * * 4 % environment=QA

There is yellow warning message displayed:

You can only use one percent sign to separate the cron from the parameters.

I think this message should be only displayed if there is more than one percent sign in one line rather than in all box.

Could you please take a look?

Cheers, Sial

AlexBaranowski commented 8 years ago

I have same issue with Jenkins ver. 1.655. Example: jenkins

I tried to fix it in src/main/java/org/jenkinsci/plugins/parameterizedschedular/ParameterParser.java by change checkSanity but then tests failed :cry:

  public String checkSanity(String cronTabSpec, ParametersDefinitionProperty parametersDefinitionProperty) {
        for (String s : cronTabSpec.split("\n")){   
            String[] split = s.split(PARAMETER_SEPARATOR);
            if (split.length < 2) {
                continue; 
            }
            if (split.length > 2) {
                return Messages.ParameterizedTimerTrigger_MoreThanOnePercent();
            }
            // split is equal 2 so we have "time % parameters"
            try {
                Map<String, String> parsedParameters = parse(split[1]);
                List<String> parameterDefinitionNames = parametersDefinitionProperty.getParameterDefinitionNames();
                List<String> parsedKeySet = new ArrayList<String>(parsedParameters.keySet());
                parsedKeySet.removeAll(parameterDefinitionNames);
                if (!parsedKeySet.isEmpty()) {
                    return Messages.ParameterizedTimerTrigger_UndefinedParameter(parsedKeySet, parameterDefinitionNames);
                }
            } catch (IllegalArgumentException e) {
                return e.getMessage();
            }
        }
        return null;
    }

Any further help will be welcome :+1:

jwmach1 commented 7 years ago

The little bit of time I had to look at this, it looks like the crontab definition used to be sent into the code above one line at a time. I know that when I was using this (I'm not anymore) it was being used to execute projects with more than one property. I know that's not much help.

rexroof commented 7 years ago

It also looks like the checkSanity function would never get to the second check for null parameters. The first if-statement returns in both cases.