Skip to content

Commit

Permalink
Fix #localtime and #gmtime when they are given numeric offsets
Browse files Browse the repository at this point in the history 
Both methods now use a common arg parser to determine the time zone from
the passed IRubyObject. Additionally, #localtime always modifies the receiver
now per the documentation, rather than returning a new object if an offset
was passed.

Conflicts:
	core/src/main/java/org/jruby/RubyTime.java

[Addendum: Noticed some mistakes in original commit so I changed exactNum a bit]
  • Loading branch information
@cheald @enebo
cheald authored and enebo committed Jan 7, 2015
1 parent b110106 commit dbf58e6
Show file tree
Hide file tree
Showing 2 changed files with 77 additions and 40 deletions.
113 changes: 76 additions & 37 deletions core/src/main/java/org/jruby/RubyTime.java
View file
Expand Up @@ -47,6 +47,7 @@
import java.util.regex.Matcher;
import java.util.regex.Pattern;

import javax.print.DocFlavor;
import org.joda.time.DateTime;
import org.joda.time.DateTimeZone;
import org.joda.time.IllegalFieldValueException;
Expand All @@ -68,6 +69,7 @@

import static org.jruby.CompatVersion.*;
import org.jruby.runtime.Helpers;
import org.jruby.util.TypeConverter;

import static org.jruby.runtime.Helpers.invokedynamic;
import static org.jruby.runtime.invokedynamic.MethodNames.OP_CMP;
Expand Down Expand Up @@ -227,23 +229,68 @@ private static DateTimeZone parseZoneString(Ruby runtime, String zone) {
return parseTZString(runtime, zone);
}

public static DateTimeZone getTimeZoneFromUtcOffset(Ruby runtime, String utcOffset) {
DateTimeZone cachedZone = runtime.getTimezoneCache().get(utcOffset);
public static DateTimeZone getTimeZoneFromUtcOffset(Ruby runtime, IRubyObject utcOffset) {
String strOffset = utcOffset.toString();

DateTimeZone cachedZone = runtime.getTimezoneCache().get(strOffset);
if (cachedZone != null) return cachedZone;

Matcher offsetMatcher = TIME_OFFSET_PATTERN.matcher(utcOffset);
if (!offsetMatcher.matches()) {
throw runtime.newArgumentError("\"+HH:MM\" or \"-HH:MM\" expected for utc_offset");

DateTimeZone dtz;
if(utcOffset instanceof RubyString) {
Matcher offsetMatcher = TIME_OFFSET_PATTERN.matcher(strOffset);
if (!offsetMatcher.matches()) {
throw runtime.newArgumentError("\"+HH:MM\" or \"-HH:MM\" expected for utc_offset");
}
String sign = offsetMatcher.group(1);
String hours = offsetMatcher.group(2);
String minutes = offsetMatcher.group(3);
dtz = getTimeZoneFromHHMM(runtime, "", !sign.equals("-"), hours, minutes);
} else {
IRubyObject numericOffset = numExact(runtime, utcOffset);
int newOffset = (int)Math.round(numericOffset.convertToFloat().getDoubleValue() * 1000);
dtz = getTimeZoneWithOffset(runtime, "", newOffset);
}
String sign = offsetMatcher.group(1);
String hours = offsetMatcher.group(2);
String minutes = offsetMatcher.group(3);
DateTimeZone dtz = getTimeZoneFromHHMM(runtime, "", !sign.equals("-"), hours, minutes);

runtime.getTimezoneCache().put(utcOffset, dtz);
runtime.getTimezoneCache().put(strOffset, dtz);
return dtz;
}

// mri: time.c num_exact
private static IRubyObject numExact(Ruby runtime, IRubyObject v) {
IRubyObject tmp;
if (v instanceof RubyFixnum || v instanceof RubyBignum) return v;
if (v.isNil()) exactTypeError(runtime, v);
if (!(v instanceof RubyRational)) { // Default unknown
if (v.respondsTo("to_r")) {
tmp = v.callMethod(runtime.getCurrentContext(), "to_r");
// WTF is this condition for? It responds to to_r and makes something which thinks it is a String?
if (tmp != null && v.respondsTo("to_str")) exactTypeError(runtime, v);
} else {
tmp = TypeConverter.checkIntegerType(runtime, v, "to_int");
if (tmp.isNil()) exactTypeError(runtime, v);
}
v = tmp;
}

if (v instanceof RubyFixnum || v instanceof RubyBignum) {
return v;
} else if (v instanceof RubyRational) {
RubyRational r = (RubyRational) v;
if (r.denominator(runtime.getCurrentContext()) == RubyFixnum.newFixnum(runtime, 1)) {
return r.numerator(runtime.getCurrentContext());
}
} else {
exactTypeError(runtime, v);
}

return v;
}

private static void exactTypeError(Ruby runtime, IRubyObject received) {
throw runtime.newTypeError(
String.format("Can't convert %s into an exact number", received.getMetaClass().getRealClass()));
}

private static DateTimeZone getTimeZoneFromHHMM(Ruby runtime, String name, boolean positive, String hours, String minutes) {
int h = Integer.parseInt(hours);
int m = 0;
Expand Down Expand Up @@ -404,11 +451,14 @@ public RubyTime localtime() {

@JRubyMethod(name = "localtime", optional = 1, compat = RUBY1_9)
public RubyTime localtime19(ThreadContext context, IRubyObject[] args) {
if (args.length == 0) return localtime();

String offset = args[0].asJavaString();
DateTimeZone dtz = getTimeZoneFromUtcOffset(context.runtime, offset);
return newTime(context.runtime, dt.withZone(dtz), nsec);
DateTimeZone newDtz;
if (args.length == 0) {
newDtz = getLocalTimeZone(context.runtime);
} else {
newDtz = getTimeZoneFromUtcOffset(context.runtime, args[0]);
}
dt = dt.withZone(newDtz);
return this;
}

@JRubyMethod(name = {"gmt?", "utc?", "gmtime?"})
Expand All @@ -431,18 +481,8 @@ public RubyTime getlocal19(ThreadContext context, IRubyObject[] args) {
if (args.length == 0 || args[0].isNil()) {
return newTime(getRuntime(), dt.withZone(getLocalTimeZone(getRuntime())), nsec);
} else {
Matcher tzMatcher = TIME_OFFSET_PATTERN.matcher(args[0].toString());
int hours, minutes, millis = 0;
if (tzMatcher.matches()) {
hours = Integer.parseInt(tzMatcher.group(2));
minutes = Integer.parseInt(tzMatcher.group(3));
millis = (hours * 60 + minutes) * 60 * 1000;
if (tzMatcher.group(1).equals("-"))
millis = -millis;
} else {
throw getRuntime().newArgumentError("\"+HH:MM\" or \"-HH:MM\" expected for utc_offset");
}
return newTime(getRuntime(), dt.withZone(DateTimeZone.forOffsetMillis(millis)), nsec);
DateTimeZone dtz = getTimeZoneFromUtcOffset(context.runtime, args[0]);
return newTime(getRuntime(), dt.withZone(dtz), nsec);
}
}

Expand Down Expand Up @@ -725,8 +765,8 @@ private IRubyObject inspectCommon(DateTimeFormatter formatter, DateTimeFormatter
@JRubyMethod(name = "to_a")
@Override
public RubyArray to_a() {
return getRuntime().newArrayNoCopy(new IRubyObject[] { sec(), min(), hour(), mday(), month(),
year(), wday(), yday(), isdst(), zone() });
return getRuntime().newArrayNoCopy(new IRubyObject[]{sec(), min(), hour(), mday(), month(),
year(), wday(), yday(), isdst(), zone()});
}

@JRubyMethod(name = "to_f")
Expand Down Expand Up @@ -804,7 +844,7 @@ public RubyInteger year() {

@JRubyMethod(name = "wday")
public RubyInteger wday() {
return getRuntime().newFixnum((dt.getDayOfWeek()%7));
return getRuntime().newFixnum((dt.getDayOfWeek() % 7));
}

@JRubyMethod(name = "yday")
Expand All @@ -826,8 +866,7 @@ public IRubyObject subsec() {
@JRubyMethod(name = {"gmt_offset", "gmtoff", "utc_offset"})
public RubyInteger gmt_offset() {
int offset = dt.getZone().getOffset(dt.getMillis());

return getRuntime().newFixnum((int)(offset/1000));
return getRuntime().newFixnum(offset / 1000);
}

@JRubyMethod(name = {"isdst", "dst?"})
Expand Down Expand Up @@ -995,7 +1034,7 @@ public RubyTime round(ThreadContext context, IRubyObject[] args) {
public static IRubyObject s_new(IRubyObject recv, IRubyObject[] args, Block block) {
Ruby runtime = recv.getRuntime();
RubyTime time = new RubyTime(runtime, (RubyClass) recv, new DateTime(getLocalTimeZone(runtime)));
time.callInit(args,block);
time.callInit(args, block);
return time;
}

Expand Down Expand Up @@ -1278,10 +1317,10 @@ private static RubyTime createTime(IRubyObject recv, IRubyObject[] args, boolean
dtz = DateTimeZone.UTC;
} else if (args.length == 10 && args[9] instanceof RubyString) {
if (utcOffset) {
dtz = getTimeZoneFromUtcOffset(runtime, ((RubyString) args[9]).toString());
dtz = getTimeZoneFromUtcOffset(runtime, args[9]);
setTzRelative = true;
} else {
dtz = getTimeZoneFromString(runtime, ((RubyString) args[9]).toString());
dtz = getTimeZoneFromString(runtime, args[9].toString());
}
} else if (args.length == 10 && args[9].respondsTo("to_int")) {
IRubyObject offsetInt = args[9].callMethod(runtime.getCurrentContext(), "to_int");
Expand All @@ -1291,7 +1330,7 @@ private static RubyTime createTime(IRubyObject recv, IRubyObject[] args, boolean
}

if (args.length == 10) {
if (args[8] instanceof RubyBoolean) isDst = ((RubyBoolean) args[8]).isTrue();
if (args[8] instanceof RubyBoolean) isDst = args[8].isTrue();

args = new IRubyObject[] { args[5], args[4], args[3], args[2], args[1], args[0], runtime.getNil() };
} else {
Expand Down
4 changes: 1 addition & 3 deletions spec/tags/1.9/ruby/core/time/getlocal_tags.txt
View file
@@ -1,8 +1,6 @@
fails:Time#getlocal returns a Time with UTC offset specified as an Integer number of seconds
fails:Time#getlocal returns a Time with a UTC offset of the specified number of Rational seconds
fails:Time#getlocal raises ArgumentError if the String argument is not in an ASCII-compatible encoding
fails:Time#getlocal raises ArgumentError if the argument represents a value less than or equal to -86400 seconds
fails:Time#getlocal raises ArgumentError if the argument represents a value greater than or equal to 86400 seconds
fails:Time#getlocal with an argument that responds to #to_int coerces using #to_int
fails:Time#getlocal with an argument that responds to #to_r coerces using #to_r
fails:Time#getlocal with an argument that responds to #to_str coerces using #to_str
fails:Time#getlocal raises ArgumentError if the String argument is not in an ASCII-compatible encoding

0 comments on commit dbf58e6

Please sign in to comment.