floraison / fugit

time tools (cron, parsing, durations, ...) for Ruby, rufus-scheduler, and flor
MIT License
353 stars 29 forks source link

Fugit::Cron#to_cron_s produces invalid cron string for "wed%2" #68

Closed lstrzebinczyk closed 2 years ago

lstrzebinczyk commented 2 years ago

Issue description

My team is creating a bot, which allows people to schedule something with a cron-like string to indicate time interval. We have a bug report which seem to boil down to inconsistency with how fugit deals with strings including a % syntax.

How to reproduce

The simplest piece of code that reproduces the issue, for example:

require 'fugit'

# The expression below returns a `Fugit::Cron` instance.
Fugit.parse(Fugit.parse("0 13 * * *").to_cron_s)
# => #<Fugit::Cron:0x000055c554c49f18 ... >

# However, this one returns `EtOrbi::EoTime`, which we can't use to calculate next time
Fugit.parse(Fugit.parse("0 13 * * wed%2").to_cron_s)
=> #<EtOrbi::EoTime:0x000055c554d310c0 ...>>

Expected behaviour

My understanding is, that these 2 cases should behave the same, and return Fugit::Cron instance.

Context

Linux killa-MS-7A34 5.13.0-39-generic #44~20.04.1-Ubuntu SMP Thu Mar 24 16:43:35 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux

ruby 3.0.3p157 (2021-11-24 revision 3fb7d2cadc) [x86_64-linux]

[:env_tz, nil]

(secs:1648816706.2644165,utc~:"2022-04-01 12:38:26.2644164562225342",ltz~:"CEST")
(etz:nil,tnz:"CEST",tziv:"2.0.4",tzidv:nil,rv:"3.0.3",rp:"x86_64-linux",win:false,rorv:nil,astz:nil,eov:"1.2.6",eotnz:#<TZInfo::DataTimezone: 

Europe/Warsaw>,eotnfz:"+0200",eotlzn:"Europe/Warsaw",eotnfZ:"CEST",debian:"Europe/Warsaw",centos:nil,osx:"Europe/Warsaw")

[:fugit, "1.5.2"]

[:now, 2022-04-01 14:39:44.473285554 +0200, :zone, "CEST"]
jmettraux commented 2 years ago
Fugit.parse("0 13 * * wed").to_cron_s
# => "0 13 * * 3"

Fugit.parse("0 13 * * wed%2").to_cron_s
# => "0 13 * * 3#2#0"

Thanks for reporting that. I will fix it.

jmettraux commented 2 years ago

I will make sure Fugit::Cron#to_cron_s is correct for other uses of % and of #, eventually fix, and then release 1.5.3.

jmettraux commented 2 years ago

Nota bene:

Fugit.parse("0 13 * * 3#2#0")
# => => #<EtOrbi::EoTime:0x000009991c9cc9d0 ...>
Fugit::Cron.do_parse("0 13 * * 3#2#0")
# => ArgumentError (invalid cron string "0 13 * * 3#2#0")

Fugit.parse should reject "0 13 * * 3#2#0" and return nil. Fugit.do_parse should raise an error.

jmettraux commented 2 years ago

Closing. Thanks again for reporting. I will work on gh-69 and then release 1.5.3 with the fix for your issue. If there is anything else here, feel free to comment.

jmettraux commented 2 years ago

fugit 1.5.3 out https://rubygems.org/gems/fugit/versions/1.5.3

Thanks!

lstrzebinczyk commented 2 years ago
irb(main):001:0> Fugit.parse(Fugit.parse("0 13 * * *").to_cron_s)
=> #<Fugit::Cron:0x00005630fc4ed4b8 @cron_s=nil, @hours=[13], @minutes=[0], @monthdays=nil, @months=nil, @original="0 13 * * *", @seconds=[0], @timezone=nil, @weekdays=nil, @zone=nil>
irb(main):002:0> Fugit.parse(Fugit.parse("0 13 * * wed%2").to_cron_s)
=> #<Fugit::Cron:0x00005630fc69fe28 @cron_s=nil, @hours=[13], @minutes=[0], @monthdays=nil, @months=nil, @original="0 13 * * 3%2", @seconds=[0], @timezone=nil, @weekdays=[[3, [2, 0]]], @zone=nil>

For a quick check, it seems to work! Awesome work, and a quick response. Thank you very much!

jmettraux commented 2 years ago

You're welcome!