groovy-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Fabio de Matos Quaresma Gonçalves <fmato...@gmail.com>
Subject Re: Bug: Dates.internalDate() fails to initialize miliseconds field properly
Date Wed, 10 Jun 2015 18:49:25 GMT
PR is submitted.

Thank you so much Pascal for helping me get it done.

PS: maybe you guys could add a link to
https://help.github.com/articles/fork-a-repo/  on
http://www.groovy-lang.org/contribute.html#code for total github newbies
like me.



2015-06-10 12:10 GMT-03:00 Fabio de Matos Quaresma Gonçalves <
fmatosqg@gmail.com>:

> Thank you so much Pascal, I'm always overwhelmed by those layers of
> administrative security, I guess you know what I mean.
>
> I'm about to submit the PR, I have a couple of questions first:
>
>
>    - should I send it based on master or GROOVY_2_4_X branch?
>    - how do I reference it? is the following commit message enought?
>    "GROOVY-7462: Dates factory fix when initializing miliseconds field"
>
>
> BTW I was having problems building the whole thing, I'm not sure exactly
> where it failed and can't afford putting more time on this right now.
> Although I could run ./gradlew  :groovy-json:test with 0 errors (2 new
> unit tests included) , I'd be more comfortable with someone else running
> the full build+test suite for me.
>
>
>
>
>
> 2015-06-09 13:43 GMT-03:00 Pascal Schumacher <pascalschumacher@gmx.net>:
>
>>  Hi Fabio,
>>
>> I created https://issues.apache.org/jira/browse/GROOVY-7462 for this.
>>
>> It would be nice if you could reference it from your pull request.
>>
>> Thanks and kind regards,
>> Pascal
>>
>> Am 09.06.2015 um 18:35 schrieb Fabio de Matos Quaresma Gonçalves:
>>
>> Thanks,
>>
>>  I'll take my chances building gradle, so I can try the patch and
>> writing a unit test for that.
>>
>>  BTW, do you guys need a JIRA issue to go along with the pull request?
>> I'd stay away from it unless you get very sad ;)  I've used JIRA before but
>> I don't have an apache account.
>>
>> 2015-06-09 13:26 GMT-03:00 Pascal Schumacher <pascalschumacher@gmx.net>:
>>
>>> Hi Fabio,
>>>
>>> thanks for reporting this. :)
>>>
>>> You should be able to create a issue for the groovy project here (as
>>> long as you have a Apache JIRA account):
>>>
>>> https://issues.apache.org/jira/secure/CreateIssue!default.jspa
>>>
>>> You should be able to submit a pull request here:
>>>
>>> https://github.com/apache/incubator-groovy
>>>
>>> Thanks and kind regards,
>>> Pascal
>>>
>>>
>>> Am 09.06.2015 um 17:17 schrieb Fabio de Matos Quaresma Gonçalves:
>>>
>>>> I found a bug on Dates class, on how it instantiates objects without
>>>> explicitly specifying the miliseconds parameter. By omission, it ends up
>>>> using the value provided by Calendar.getInstance(), which is copied over
>>>> from System.currentTimeMillis(). This is a problem when comparing dates,
>>>> even if I don't care about such high resolution.
>>>>
>>>>
>>>> I reproduced it using groovy console v 2.4.3 on JVM 1.8. I also cloned
>>>> the git repo and I'm sure the bug is still unsolved.
>>>>
>>>> Unfortunately this code is tricky to run, I had it returning the same
>>>> amount of miliseconds if the Thread.sleep line is removed. Even with a
>>>> println in between constructors didn't guarantee the expected result.
>>>> >>>>>>>>>>>>>>>
>>>> import groovy.json.internal.Dates
>>>> import groovy.transform.TypeChecked
>>>>         Date d1 =
>>>> Dates.toDate(TimeZone.getTimeZone("GMT"),2015,06,07,23,55,59)
>>>>         Thread.sleep(1) // lets get some time between calling
>>>> constructors
>>>>         Date d2 =
>>>> Dates.toDate(TimeZone.getTimeZone("GMT"),2015,06,07,23,55,59)
>>>>
>>>>
>>>>         println Date.getMillisOf(d1) + "!=" + Date.getMillisOf(d2)
>>>>
>>>>         assert d1 == d2
>>>> <<<<<<<<<<<<<<<
>>>>
>>>> The problem is in the following code (suggested fix is commented within
>>>> snippet):
>>>> >>>>>>>>>>>>>>>
>>>>     private static Date internalDate(TimeZone tz, int year, int month,
>>>> int day, int hour, int minute, int second) {
>>>>         Calendar calendar = Calendar.getInstance();
>>>>
>>>>         calendar.set(Calendar.YEAR, year);
>>>>         calendar.set(Calendar.MONTH, month - 1);
>>>>         calendar.set(Calendar.DAY_OF_MONTH, day);
>>>>         calendar.set(Calendar.HOUR_OF_DAY, hour);
>>>>         calendar.set(Calendar.MINUTE, minute);
>>>>         calendar.set(Calendar.SECOND, second);
>>>>         calendar.setTimeZone(tz);
>>>>
>>>>         // missed setting ms to some deterministic value
>>>> calendar.set(Calendar.MILLISECOND, 0);
>>>>
>>>>         return calendar.getTime();
>>>>     }
>>>> <<<<<<<<<<<<<<<
>>>>
>>>> An alternative way to see the bug and fix it:
>>>>
>>>> >>>>>>>>>>>>>>>
>>>>     public static Date toDate(TimeZone tz, int year, int month, int day,
>>>>                               int hour, int minute, int second) {
>>>>         return internalDate(tz, year, month, day, hour, minute,
>>>> second);  // this is bugged
>>>>         //return internalDate(tz, year, month, day, hour, minute,
>>>> second,0); //this will work
>>>>     }
>>>> <<<<<<<<<<<<<<<
>>>>
>>>> I'd gladly submit a jira issue or a pull request, but apparently I need
>>>> permission for any of those.
>>>>
>>>> I'd like to stay unsubscribed to the emailing list if possible, so
>>>> please CC me in the reply.
>>>>
>>>>
>>>> --
>>>> Fabio de Matos Gonçalves
>>>>
>>>
>>>
>>
>>
>>  --
>> Fabio de Matos Gonçalves
>>
>>
>>
>
>
> --
> Fabio de Matos Gonçalves
>



-- 
Fabio de Matos Gonçalves

Mime
View raw message