Skip to content

Commit d58824b

Browse files
creativeprojectscreativeprojects
authored
fix: crontab dayofmonth with dayofweek (#630)
* feat: enhance crontab entry handling with day of month and day of week logic * feat: implement day of week handling in crontab entries and tests * test: add cases for crontab entries with day of week conditions * fix: correct regex for single day of week and improve error handling in addWeekDays function * test: add unit tests for addWeekDays function with various input scenarios
1 parent a0406f0 commit d58824b

4 files changed

Lines changed: 263 additions & 63 deletions

File tree

‎crond/crontab.go‎

Lines changed: 92 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -9,24 +9,55 @@ import (
99
"regexp"
1010
"strings"
1111

12+
"github.com/creativeprojects/resticprofile/calendar"
1213
"github.com/creativeprojects/resticprofile/user"
1314
"github.com/spf13/afero"
1415
)
1516

1617
const (
17-
startMarker = "### this content was generated by resticprofile, please leave this line intact ###\n"
18-
endMarker = "### end of resticprofile content, please leave this line intact ###\n"
19-
timeExp = `^(([\d,\/\-\*]+[ \t]?){5})`
20-
userExp = `[\t]+(\w+[\t]+)?`
21-
workDirExp = `(cd .+ && )?`
22-
configExp = `[^\s]+.+--config[ =]"?([^"\n]+)"?`
23-
legacyExp = "(" + configExp + ` [^\n]*--name[ =]([^\s]+)( --.+)? ([a-z]+))$`
24-
runScheduleExp = "(" + configExp + ` run-schedule ([^\s]+)@([^\s]+))$`
18+
startMarker = "### this content was generated by resticprofile, please leave this line intact ###\n"
19+
endMarker = "### end of resticprofile content, please leave this line intact ###\n"
20+
timeExp = `^(([\d,\/\-\*]+[ \t]?){5})`
21+
userExp = `[\t]+(\w+[\t]+)?`
22+
singleDayOfWeekExp = `test\s+\$\(date\s+'\+\\%w'\)\s+-eq\s+\d`
23+
dayOfWeekExtra = `((?:` + singleDayOfWeekExp + `)(?:\s*\|\|\s*` + singleDayOfWeekExp + `)*\s*&&\s*)?`
24+
workDirExp = `(cd .+ && )?`
25+
configExp = `[^\s]+.+--config[ =]"?([^"\n]+)"?`
26+
legacyExp = "(" + configExp + ` [^\n]*--name[ =]([^\s]+)( --.+)? ([a-z]+))$`
27+
runScheduleExp = "(" + configExp + ` run-schedule ([^\s]+)@([^\s]+))$`
28+
)
29+
30+
const (
31+
legacyIndexWhole int = iota
32+
legacyIndexEvent
33+
legacyIndexLastEventField
34+
legacyIndexUser
35+
legacyIndexWorkdir
36+
legacyIndexCommandLine
37+
legacyIndexConfigFile
38+
legacyIndexProfileName
39+
legacyIndexOtherFlags
40+
legacyIndexCommandName
41+
legacyIndexCount
42+
)
43+
44+
const (
45+
indexWhole int = iota
46+
indexEvent
47+
indexLastEventField
48+
indexUser
49+
indexDayOfWeekTest
50+
indexWorkdir
51+
indexCommandLine
52+
indexConfigFile
53+
indexCommandName
54+
indexProfileName
55+
indexCount
2556
)
2657

2758
var (
2859
legacyPattern = regexp.MustCompile(timeExp + userExp + workDirExp + legacyExp)
29-
runSchedulePattern = regexp.MustCompile(timeExp + userExp + workDirExp + runScheduleExp)
60+
runSchedulePattern = regexp.MustCompile(timeExp + userExp + dayOfWeekExtra + workDirExp + runScheduleExp)
3061
)
3162

3263
var (
@@ -352,39 +383,49 @@ func parseEntry(line string) (*Entry, error) {
352383
// 00,15,30,45 * * * * /home/resticprofile --no-ansi --config config.yaml run-schedule backup@profile
353384
// also with quotes around the config file:
354385
// 00,15,30,45 * * * * /home/resticprofile --no-ansi --config "config.yaml" run-schedule backup@profile
386+
// should also be able to parse this newer format:
387+
// 00,15,30,45 * * * * test $(date '+\%w') -eq 2 || test $(date '+\%w') -eq 3 && /home/resticprofile --no-ansi --config config.yaml run-schedule backup@profile
388+
// 00,15,30,45 * * * * test $(date '+\%w') -eq 1 && cd /workdir && /home/resticprofile --no-ansi --config config.yaml run-schedule backup@profile
355389

356390
// try legacy pattern first
357391
matches := legacyPattern.FindStringSubmatch(line)
358-
if len(matches) == 10 {
359-
event, err := parseEvent(matches[1])
392+
if len(matches) == legacyIndexCount {
393+
event, err := parseEvent(matches[legacyIndexEvent])
360394
if err != nil {
361-
return nil, fmt.Errorf("cannot parse %q: %w", matches[1], err)
395+
return nil, fmt.Errorf("cannot parse %q: %w", matches[legacyIndexEvent], err)
362396
}
363397
return &Entry{
364398
event: event,
365-
user: getUserValue(matches[3]),
366-
workDir: getWorkdirValue(matches[4]),
367-
commandLine: matches[5],
368-
configFile: matches[6],
369-
profileName: matches[7],
370-
commandName: matches[9],
399+
user: getUserValue(matches[legacyIndexUser]),
400+
workDir: getWorkdirValue(matches[legacyIndexWorkdir]),
401+
commandLine: matches[legacyIndexCommandLine],
402+
configFile: matches[legacyIndexConfigFile],
403+
profileName: matches[legacyIndexProfileName],
404+
commandName: matches[legacyIndexCommandName],
371405
}, nil
372406
}
407+
373408
// then try the current pattern
374409
matches = runSchedulePattern.FindStringSubmatch(line)
375-
if len(matches) == 9 {
376-
event, err := parseEvent(matches[1])
410+
if len(matches) == indexCount {
411+
event, err := parseEvent(matches[indexEvent])
377412
if err != nil {
378-
return nil, fmt.Errorf("cannot parse %q: %w", matches[1], err)
413+
return nil, fmt.Errorf("cannot parse %q: %w", matches[indexEvent], err)
414+
}
415+
if len(matches[indexDayOfWeekTest]) > 0 {
416+
err := addWeekDays(matches[indexDayOfWeekTest], event)
417+
if err != nil {
418+
return nil, fmt.Errorf("parsing %q: %w", matches[indexDayOfWeekTest], err)
419+
}
379420
}
380421
return &Entry{
381422
event: event,
382-
user: getUserValue(matches[3]),
383-
workDir: getWorkdirValue(matches[4]),
384-
commandLine: matches[5],
385-
configFile: matches[6],
386-
commandName: matches[7],
387-
profileName: matches[8],
423+
user: getUserValue(matches[indexUser]),
424+
workDir: getWorkdirValue(matches[indexWorkdir]),
425+
commandLine: matches[indexCommandLine],
426+
configFile: matches[indexConfigFile],
427+
commandName: matches[indexCommandName],
428+
profileName: matches[indexProfileName],
388429
}, nil
389430
}
390431
return nil, ErrEntryNoMatch
@@ -397,3 +438,27 @@ func getUserValue(user string) string {
397438
func getWorkdirValue(workdir string) string {
398439
return strings.TrimSuffix(strings.TrimPrefix(workdir, "cd "), " && ")
399440
}
441+
442+
func addWeekDays(weekDaysExtra string, event *calendar.Event) error {
443+
if len(weekDaysExtra) == 0 {
444+
return nil
445+
}
446+
weekDaysExtra = strings.TrimSpace(weekDaysExtra)
447+
weekDaysExtra = strings.TrimSuffix(weekDaysExtra, "&&")
448+
weekDaysExtra = strings.TrimSpace(weekDaysExtra)
449+
450+
parts := strings.Split(weekDaysExtra, "||")
451+
for _, part := range parts {
452+
part = strings.TrimSpace(part)
453+
if len(part) < 1 {
454+
continue
455+
}
456+
digit := part[len(part)-1]
457+
dayNum := int(digit - '0')
458+
err := event.WeekDay.AddValue(dayNum)
459+
if err != nil {
460+
return fmt.Errorf("invalid weekday %d: %w", dayNum, err)
461+
}
462+
}
463+
return nil
464+
}

‎crond/crontab_test.go‎

Lines changed: 114 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,14 @@ func TestDeleteLine(t *testing.T) {
8686
{"#\n#\n#\n00,30 * * * * cd /workdir && /home/resticprofile --no-ansi --config \"config.yaml\" run-schedule backup@profile\n", true},
8787
{"#\n#\n#\n00,30 * * * * user cd /workdir && /home/resticprofile --no-ansi --config config.yaml run-schedule backup@profile\n", true},
8888
{"#\n#\n#\n00,30 * * * * user cd /workdir && /home/resticprofile --no-ansi --config \"config.yaml\" run-schedule backup@profile\n", true},
89+
{"#\n#\n#\n# 00,30 * * * * test $(date '+\\%w') -eq 2 || test $(date '+\\%w') -eq 3 && /home/resticprofile --no-ansi --config \"config.yaml\" run-schedule backup@profile\n", false},
90+
{"#\n#\n#\n# 00,30 * * * * test $(date '+\\%w') -eq 2 || test $(date '+\\%w') -eq 3 && cd /workdir && /home/resticprofile --no-ansi --config \"config.yaml\" run-schedule backup@profile\n", false},
91+
{"#\n#\n#\n00,30 * * * * test $(date '+\\%w') -eq 2 || test $(date '+\\%w') -eq 3 && cd /workdir && /home/resticprofile --no-ansi --config \"config.yaml\" run-schedule backup@profile\n", true},
92+
{"#\n#\n#\n00,30 * * * * user test $(date '+\\%w') -eq 2 || test $(date '+\\%w') -eq 3 && cd /workdir && /home/resticprofile --no-ansi --config config.yaml run-schedule backup@profile\n", true},
93+
{"#\n#\n#\n# 00,30 * * * * test $(date '+\\%w') -eq 0 && /home/resticprofile --no-ansi --config \"config.yaml\" run-schedule backup@profile\n", false},
94+
{"#\n#\n#\n# 00,30 * * * * test $(date '+\\%w') -eq 0 && cd /workdir && /home/resticprofile --no-ansi --config \"config.yaml\" run-schedule backup@profile\n", false},
95+
{"#\n#\n#\n00,30 * * * * test $(date '+\\%w') -eq 0 && cd /workdir && /home/resticprofile --no-ansi --config \"config.yaml\" run-schedule backup@profile\n", true},
96+
{"#\n#\n#\n00,30 * * * * user test $(date '+\\%w') -eq 0 && cd /workdir && /home/resticprofile --no-ansi --config config.yaml run-schedule backup@profile\n", true},
8997
}
9098

9199
for _, testRun := range testData {
@@ -373,6 +381,20 @@ func TestParseEntry(t *testing.T) {
373381
_ = e.Minute.AddValue(0)
374382
_ = e.Minute.AddValue(30)
375383
})
384+
scheduledEventOnMondays := calendar.NewEvent(func(e *calendar.Event) {
385+
_ = e.Second.AddValue(0)
386+
_ = e.Minute.AddValue(0)
387+
_ = e.Minute.AddValue(30)
388+
_ = e.WeekDay.AddValue(1)
389+
})
390+
391+
scheduledEventOnSundaysAndWednesdays := calendar.NewEvent(func(e *calendar.Event) {
392+
_ = e.Second.AddValue(0)
393+
_ = e.Minute.AddValue(0)
394+
_ = e.Minute.AddValue(30)
395+
_ = e.WeekDay.AddValue(0)
396+
_ = e.WeekDay.AddValue(3)
397+
})
376398
testData := []struct {
377399
source string
378400
expectEntry *Entry
@@ -417,6 +439,23 @@ func TestParseEntry(t *testing.T) {
417439
source: "00,30 * * * * user cd /workdir && /home/resticprofile --no-ansi --config \"config file.yaml\" run-schedule backup@profile",
418440
expectEntry: &Entry{configFile: "config file.yaml", profileName: "profile", commandName: "backup", user: "user", workDir: "/workdir", event: scheduledEvent, commandLine: "/home/resticprofile --no-ansi --config \"config file.yaml\" run-schedule backup@profile"},
419441
},
442+
// with day of week extra
443+
{
444+
source: "00,30 * * * * test $(date '+\\%w') -eq 1 && /home/resticprofile --no-ansi --config \"config file.yaml\" run-schedule backup@profile",
445+
expectEntry: &Entry{configFile: "config file.yaml", profileName: "profile", commandName: "backup", user: "", event: scheduledEventOnMondays, commandLine: "/home/resticprofile --no-ansi --config \"config file.yaml\" run-schedule backup@profile"},
446+
},
447+
{
448+
source: "00,30 * * * * user test $(date '+\\%w') -eq 0 || test $(date '+\\%w') -eq 3 && /home/resticprofile --no-ansi --config \"config file.yaml\" run-schedule backup@profile",
449+
expectEntry: &Entry{configFile: "config file.yaml", profileName: "profile", commandName: "backup", user: "user", event: scheduledEventOnSundaysAndWednesdays, commandLine: "/home/resticprofile --no-ansi --config \"config file.yaml\" run-schedule backup@profile"},
450+
},
451+
{
452+
source: "00,30 * * * * test $(date '+\\%w') -eq 0 || test $(date '+\\%w') -eq 3 && cd /workdir && /home/resticprofile --no-ansi --config config.yaml run-schedule backup@profile",
453+
expectEntry: &Entry{configFile: "config.yaml", profileName: "profile", commandName: "backup", workDir: "/workdir", event: scheduledEventOnSundaysAndWednesdays, commandLine: "/home/resticprofile --no-ansi --config config.yaml run-schedule backup@profile"},
454+
},
455+
{
456+
source: "00,30 * * * * user test $(date '+\\%w') -eq 1 && cd /workdir && /home/resticprofile --no-ansi --config \"config file.yaml\" run-schedule backup@profile",
457+
expectEntry: &Entry{configFile: "config file.yaml", profileName: "profile", commandName: "backup", user: "user", workDir: "/workdir", event: scheduledEventOnMondays, commandLine: "/home/resticprofile --no-ansi --config \"config file.yaml\" run-schedule backup@profile"},
458+
},
420459
}
421460

422461
for _, testRun := range testData {
@@ -475,3 +514,78 @@ func TestGetEntries(t *testing.T) {
475514
assert.Equal(t, "/some/bin/resticprofile --no-ansi --config config.yaml run-schedule backup@profile", entries[0].commandLine)
476515
})
477516
}
517+
518+
func TestAddWeekDays(t *testing.T) {
519+
tests := []struct {
520+
name string
521+
input string
522+
expectError bool
523+
expectedDays []int
524+
}{
525+
{
526+
name: "empty string",
527+
input: "",
528+
expectError: false,
529+
expectedDays: nil,
530+
},
531+
{
532+
name: "single day with trailing &&",
533+
input: `test $(date '+\%w') -eq 2 &&`,
534+
expectError: false,
535+
expectedDays: []int{2},
536+
},
537+
{
538+
name: "single day with trailing && and spaces",
539+
input: `test $(date '+\%w') -eq 5 && `,
540+
expectError: false,
541+
expectedDays: []int{5},
542+
},
543+
{
544+
name: "two days with trailing &&",
545+
input: `test $(date '+\%w') -eq 2 || test $(date '+\%w') -eq 3 &&`,
546+
expectError: false,
547+
expectedDays: []int{2, 3},
548+
},
549+
{
550+
name: "three days with trailing &&",
551+
input: `test $(date '+\%w') -eq 1 || test $(date '+\%w') -eq 3 || test $(date '+\%w') -eq 5 &&`,
552+
expectError: false,
553+
expectedDays: []int{1, 3, 5},
554+
},
555+
{
556+
name: "sunday (day 0)",
557+
input: `test $(date '+\%w') -eq 0 &&`,
558+
expectError: false,
559+
expectedDays: []int{0},
560+
},
561+
{
562+
name: "saturday (day 6)",
563+
input: `test $(date '+\%w') -eq 6 &&`,
564+
expectError: false,
565+
expectedDays: []int{6},
566+
},
567+
{
568+
name: "invalid day out of range",
569+
input: `test $(date '+\%w') -eq 9 &&`,
570+
expectError: true,
571+
},
572+
}
573+
574+
for _, tt := range tests {
575+
t.Run(tt.name, func(t *testing.T) {
576+
event := calendar.NewEvent()
577+
err := addWeekDays(tt.input, event)
578+
if tt.expectError {
579+
assert.Error(t, err)
580+
return
581+
}
582+
require.NoError(t, err)
583+
if len(tt.expectedDays) == 0 {
584+
assert.False(t, event.WeekDay.HasValue())
585+
} else {
586+
assert.True(t, event.WeekDay.HasValue())
587+
assert.Equal(t, tt.expectedDays, event.WeekDay.GetRangeValues())
588+
}
589+
})
590+
}
591+
}

‎crond/entry.go‎

Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -47,8 +47,11 @@ func (e Entry) SkipUser() bool { return e.NeedsUser() || e.user == "-" }
4747

4848
// String returns the crontab line representation of the entry (end of line included)
4949
func (e Entry) String() string {
50+
// The day of a command's execution can be specified by two fields — day of month, and day of week.
51+
// If both fields are restricted (ie, are not *), the command will be run when either field matches the current time.
52+
// For example, "30 4 1,15 * 5" would cause a command to be run at 4:30 am on the 1st and 15th of each month, plus every Friday.
5053
minute, hour, dayOfMonth, month, dayOfWeek := "*", "*", "*", "*", "*"
51-
wd := ""
54+
dayTest, wd := "", ""
5255
if e.workDir != "" {
5356
wd = fmt.Sprintf("cd %s && ", e.workDir)
5457
}
@@ -65,13 +68,22 @@ func (e Entry) String() string {
6568
month = formatRange(e.event.Month.GetRanges(), twoDecimals)
6669
}
6770
if e.event.WeekDay.HasValue() {
68-
// don't make ranges for days of the week as it can fail with high sunday (7)
69-
dayOfWeek = formatList(e.event.WeekDay.GetRangeValues(), formatWeekDay)
71+
if !e.event.Day.HasValue() {
72+
// don't make ranges for days of the week as it can fail with high sunday (7)
73+
dayOfWeek = formatList(e.event.WeekDay.GetRangeValues(), formatWeekDay)
74+
} else {
75+
days := e.event.WeekDay.GetRangeValues()
76+
dayTests := make([]string, len(days))
77+
for i, day := range days {
78+
dayTests[i] = fmt.Sprintf("test $(date '+\\%%w') -eq %s ", formatWeekDay(day))
79+
}
80+
dayTest = strings.Join(dayTests, "|| ") + "&& "
81+
}
7082
}
7183
if e.HasUser() && !e.SkipUser() {
72-
return fmt.Sprintf("%s %s %s %s %s\t%s\t%s%s\n", minute, hour, dayOfMonth, month, dayOfWeek, e.user, wd, e.commandLine)
84+
return fmt.Sprintf("%s %s %s %s %s\t%s\t%s%s%s\n", minute, hour, dayOfMonth, month, dayOfWeek, e.user, dayTest, wd, e.commandLine)
7385
}
74-
return fmt.Sprintf("%s %s %s %s %s\t%s%s\n", minute, hour, dayOfMonth, month, dayOfWeek, wd, e.commandLine)
86+
return fmt.Sprintf("%s %s %s %s %s\t%s%s%s\n", minute, hour, dayOfMonth, month, dayOfWeek, dayTest, wd, e.commandLine)
7587
}
7688

7789
// Generate writes a cron line in the StringWriter (end of line included)

0 commit comments

Comments
 (0)