Skip to content
Permalink
Browse files
[JENKINS-44052] Document & fix intended behaviour (#3043)
* [JENKINS-44052] Document & fix intended behaviour

The unit was actually ignored...

* Add tests for getTimeInSeconds
  • Loading branch information
batmat authored and oleg-nenashev committed Oct 1, 2017
1 parent fbaa059 commit ff36adf0d243e2c3461045615ee654eb33665acb
@@ -171,7 +171,7 @@ public void _doBuild(StaplerRequest req, StaplerResponse rsp, @QueryParameter Ti
}

WaitingItem item = Jenkins.getInstance().getQueue().schedule(
getJob(), delay.getTime(), new ParametersAction(values), new CauseAction(new Cause.UserIdCause()));
getJob(), delay.getTimeInSeconds(), new ParametersAction(values), new CauseAction(new Cause.UserIdCause()));
if (item!=null) {
String url = formData.optString("redirectTo");
if (url==null || !Util.isSafeToRedirectTo(url)) // avoid open redirect
@@ -199,7 +199,7 @@ public void buildWithParameters(StaplerRequest req, StaplerResponse rsp, @CheckF
if (delay==null) delay=new TimeDuration(getJob().getQuietPeriod());

Queue.Item item = Jenkins.getInstance().getQueue().schedule2(
getJob(), delay.getTime(), new ParametersAction(values), ParameterizedJobMixIn.getBuildCause(getJob(), req)).getItem();
getJob(), delay.getTimeInSeconds(), new ParametersAction(values), ParameterizedJobMixIn.getBuildCause(getJob(), req)).getItem();

if (item != null) {
rsp.sendRedirect(SC_CREATED, req.getContextPath() + '/' + item.getUrl());
@@ -213,7 +213,7 @@ public final void doBuild(StaplerRequest req, StaplerResponse rsp, @QueryParamet
}


Queue.Item item = Jenkins.getInstance().getQueue().schedule2(asJob(), delay.getTime(), getBuildCause(asJob(), req)).getItem();
Queue.Item item = Jenkins.getInstance().getQueue().schedule2(asJob(), delay.getTimeInSeconds(), getBuildCause(asJob(), req)).getItem();
if (item != null) {
rsp.sendRedirect(SC_CREATED, req.getContextPath() + '/' + item.getUrl());
} else {
@@ -21,27 +21,59 @@ public TimeDuration(long millis) {
this.millis = millis;
}

/**
* Returns the duration of this instance in <em>milliseconds</em>.
* @deprecated use {@link #getTimeInMillis()} instead.
*
* This method has always returned a time in milliseconds, when various callers incorrectly assumed seconds.
* And this spread through the codebase. So this has been deprecated for clarity in favour of more explicitly named
* methods.
*/
@Deprecated
public int getTime() {
return (int)millis;
}

/**
* Returns the duration of this instance in <em>milliseconds</em>.
*/
public long getTimeInMillis() {
return millis;
}

/**
* Returns the duration of this instance in <em>milliseconds</em>.
*/
public int getTimeInSeconds() {
return (int) (millis / 1000L);
}


public long as(TimeUnit t) {
return t.convert(millis,TimeUnit.MILLISECONDS);
}

public static @CheckForNull TimeDuration fromString(@CheckForNull String delay) {
if (delay==null)
/**
* Creates a {@link TimeDuration} from the delay passed in parameter
* @param delay the delay either in milliseconds without unit, or in seconds if suffixed by sec or secs.
* @return the TimeDuration created from the delay expressed as a String.
*/
@CheckForNull
public static TimeDuration fromString(@CheckForNull String delay) {
if (delay == null) {
return null;
}

long unitMultiplier = 1L;
delay = delay.trim();
try {
// TODO: more unit handling
if(delay.endsWith("sec")) delay=delay.substring(0,delay.length()-3);
if(delay.endsWith("secs")) delay=delay.substring(0,delay.length()-4);
return new TimeDuration(Long.parseLong(delay));
if (delay.endsWith("sec") || delay.endsWith("secs")) {
delay = delay.substring(0, delay.lastIndexOf("sec"));
delay = delay.trim();
unitMultiplier = 1000L;
}
return new TimeDuration(Long.parseLong(delay.trim()) * unitMultiplier);
} catch (NumberFormatException e) {
throw new IllegalArgumentException("Invalid time duration value: "+delay);
}
@@ -0,0 +1,29 @@
package jenkins.util;

import org.junit.Test;

import java.util.concurrent.TimeUnit;

import static org.hamcrest.CoreMatchers.is;
import static org.junit.Assert.*;

import static jenkins.util.TimeDuration.*;


public class TimeDurationTest {

@Test
public void fromString() throws Exception {
assertEquals(1, TimeDuration.fromString("1").getTimeInMillis());

assertEquals(1000, TimeDuration.fromString("1sec").getTimeInMillis());
assertEquals(1000, TimeDuration.fromString("1secs").getTimeInMillis());
assertEquals(1000, TimeDuration.fromString("1 secs ").getTimeInMillis());
assertEquals(1000, TimeDuration.fromString(" 1 secs ").getTimeInMillis());
assertEquals(1, TimeDuration.fromString(" 1 secs ").getTimeInSeconds());

assertEquals(21000, TimeDuration.fromString(" 21 secs ").getTimeInMillis());
assertEquals(21, TimeDuration.fromString(" 21 secs ").getTimeInSeconds());
}

}

0 comments on commit ff36adf

Please sign in to comment.