Skip to content

Commit

Permalink
Watcher: Ignore system locale/timezone in croneval CLI tool (#33215)
Browse files Browse the repository at this point in the history
The elasticsearch-croneval CLI tool uses local dates to display when
something gets triggered the next time. This is very confusing.

This commit ensures, that UTC and local timezone times will be written
out.

The output looks like this and contains localized dates for each trigger
date as well as for `now`.

Now is [Tue, 28 Aug 2018 17:23:51 +0000] in UTC, local time is [ᏔᎵᏁ, 28 ᎦᎶ 2018 12:23:51 -0500]
Here are the next 10 times this cron expression will trigger:
1.	Mon, 2 Jan 2040 11:00:00 +0000
	ᏉᏅᎯ, 2 ᎤᏃ 2040 06:00:00 -0500
2.      ...

This also removes an old outstanding TODO to use the jopt parsing to
cast the count to an integer instead of doing it ourselves.
  • Loading branch information
spinscale authored Nov 7, 2018
1 parent b4173c8 commit 9f3effd
Show file tree
Hide file tree
Showing 2 changed files with 51 additions and 14 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -5,45 +5,48 @@
*/
package org.elasticsearch.xpack.watcher.trigger.schedule.tool;

import java.util.Arrays;
import java.util.List;

import joptsimple.OptionSet;
import joptsimple.OptionSpec;
import org.elasticsearch.cli.ExitCodes;
import org.elasticsearch.cli.LoggingAwareCommand;
import org.elasticsearch.cli.UserException;
import org.elasticsearch.cli.Terminal;
import org.elasticsearch.cli.UserException;
import org.elasticsearch.xpack.core.scheduler.Cron;
import org.joda.time.DateTime;
import org.joda.time.DateTimeZone;
import org.joda.time.format.DateTimeFormat;
import org.joda.time.format.DateTimeFormatter;

import java.util.Arrays;
import java.util.List;
import java.util.Locale;
import java.util.TimeZone;

public class CronEvalTool extends LoggingAwareCommand {

public static void main(String[] args) throws Exception {
exit(new CronEvalTool().main(args, Terminal.DEFAULT));
}

private static final DateTimeFormatter formatter = DateTimeFormat.forPattern("EEE, d MMM yyyy HH:mm:ss");
private static final DateTimeFormatter UTC_FORMATTER = DateTimeFormat.forPattern("EEE, d MMM yyyy HH:mm:ss")
.withZone(DateTimeZone.UTC).withLocale(Locale.ROOT);
private static final DateTimeFormatter LOCAL_FORMATTER = DateTimeFormat.forPattern("EEE, d MMM yyyy HH:mm:ss Z")
.withZone(DateTimeZone.forTimeZone(TimeZone.getDefault()));

private final OptionSpec<String> countOption;
private final OptionSpec<Integer> countOption;
private final OptionSpec<String> arguments;

CronEvalTool() {
super("Validates and evaluates a cron expression");
this.countOption = parser.acceptsAll(Arrays.asList("c", "count"),
"The number of future times this expression will be triggered")
// TODO: change this to ofType(Integer.class) with jopt-simple 5.0
// before then it will cause a security exception in tests
.withRequiredArg().defaultsTo("10");
.withRequiredArg().ofType(Integer.class).defaultsTo(10);
this.arguments = parser.nonOptions("expression");
}

@Override
protected void execute(Terminal terminal, OptionSet options) throws Exception {
int count = Integer.parseInt(countOption.value(options));
int count = countOption.value(options);
List<String> args = arguments.values(options);
if (args.size() != 1) {
throw new UserException(ExitCodes.USAGE, "expecting a single argument that is the cron expression to evaluate");
Expand All @@ -55,8 +58,14 @@ void execute(Terminal terminal, String expression, int count) throws Exception {
Cron.validate(expression);
terminal.println("Valid!");

DateTime date = DateTime.now(DateTimeZone.UTC);
terminal.println("Now is [" + formatter.print(date) + "]");
final DateTime date = DateTime.now(DateTimeZone.UTC);
final boolean isLocalTimeUTC = UTC_FORMATTER.getZone().equals(LOCAL_FORMATTER.getZone());
if (isLocalTimeUTC) {
terminal.println("Now is [" + UTC_FORMATTER.print(date) + "] in UTC");
} else {
terminal.println("Now is [" + UTC_FORMATTER.print(date) + "] in UTC, local time is [" + LOCAL_FORMATTER.print(date) + "]");

}
terminal.println("Here are the next " + count + " times this cron expression will trigger:");

Cron cron = new Cron(expression);
Expand All @@ -68,11 +77,17 @@ void execute(Terminal terminal, String expression, int count) throws Exception {
if (time < 0) {
if (i == 0) {
throw new UserException(ExitCodes.OK, "Could not compute future times since ["
+ formatter.print(prevTime) + "] " + "(perhaps the cron expression only points to times in the past?)");
+ UTC_FORMATTER.print(prevTime) + "] " + "(perhaps the cron expression only points to times in the past?)");
}
break;
}
terminal.println((i + 1) + ".\t" + formatter.print(time));

if (isLocalTimeUTC) {
terminal.println((i + 1) + ".\t" + UTC_FORMATTER.print(time));
} else {
terminal.println((i + 1) + ".\t" + UTC_FORMATTER.print(time));
terminal.println("\t" + LOCAL_FORMATTER.print(time));
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,15 +7,19 @@

import org.elasticsearch.cli.Command;
import org.elasticsearch.cli.CommandTestCase;
import org.joda.time.DateTimeZone;

import java.util.Arrays;
import java.util.Calendar;
import java.util.Locale;
import java.util.TimeZone;

import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.is;
import static org.hamcrest.Matchers.not;

public class CronEvalToolTests extends CommandTestCase {

@Override
protected Command newCommand() {
return new CronEvalTool();
Expand Down Expand Up @@ -48,4 +52,22 @@ public void testGetNextValidTimes() throws Exception {
assertThat(message, containsString("(perhaps the cron expression only points to times in the past?)"));
}
}

// randomized testing sets arbitrary locales and timezones, and we do not care
// we always have to output in standard locale and independent from timezone
public void testEnsureDateIsShownInRootLocale() throws Exception {
String output = execute("-c","1", "0 0 11 ? * MON-SAT 2040");
if (TimeZone.getDefault().equals(DateTimeZone.UTC.toTimeZone())) {
assertThat(output, not(containsString("local time is")));
long linesStartingWithOne = Arrays.stream(output.split("\n")).filter(s -> s.startsWith("\t")).count();
assertThat(linesStartingWithOne, is(0L));
} else {
// check for header line
assertThat(output, containsString("] in UTC, local time is"));
assertThat(output, containsString("Mon, 2 Jan 2040 11:00:00"));
logger.info(output);
long linesStartingWithOne = Arrays.stream(output.split("\n")).filter(s -> s.startsWith("\t")).count();
assertThat(linesStartingWithOne, is(1L));
}
}
}

0 comments on commit 9f3effd

Please sign in to comment.