Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix scheduling arguments #423

Merged
merged 3 commits into from
Oct 23, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion config/mocks/NamedPropertySet.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion config/mocks/ProfileInfo.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion config/mocks/PropertyInfo.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion config/mocks/SectionInfo.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion monitor/mocks/OutputAnalysis.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

50 changes: 50 additions & 0 deletions schedule/command_arguments.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
package schedule

import "strings"

type CommandArguments struct {
args []string
}

func NewCommandArguments(args []string) CommandArguments {
return CommandArguments{
args: args,
}
}

func (ca CommandArguments) RawArgs() []string {
result := make([]string, len(ca.args))
copy(result, ca.args)
return result
}

// String returns the arguments as a string, with quotes around arguments that contain spaces
func (ca CommandArguments) String() string {
if len(ca.args) == 0 {
return ""
}

var n int
for _, elem := range ca.args {
n += len(elem) + 3 // add 2 if quotes are needed, plus 1 for the space
}

b := new(strings.Builder)
b.Grow(n)
ca.writeString(b, ca.args[0])
for _, s := range ca.args[1:] {
b.WriteString(" ")
ca.writeString(b, s)
}
return b.String()
}

func (ca CommandArguments) writeString(b *strings.Builder, str string) {
if strings.Contains(str, " ") {
b.WriteString(`"`)
b.WriteString(str)
b.WriteString(`"`)
} else {
b.WriteString(str)
}
}
59 changes: 59 additions & 0 deletions schedule/command_arguments_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
package schedule

import (
"testing"
)

func TestRawArgs(t *testing.T) {
tests := []struct {
name string
args []string
}{
{"empty args", []string{}},
{"simple args", []string{"arg1", "arg2"}},
{"args with spaces", []string{"C:\\Program Files\\app.exe", "--config", "C:\\My Documents\\config.toml"}},
{"args with special chars", []string{"--name", "my-task!", "--config=test.conf"}},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
ca := NewCommandArguments(tt.args)
rawArgs := ca.RawArgs()
if len(rawArgs) != len(tt.args) {
t.Errorf("expected %d raw arguments, got %d", len(tt.args), len(rawArgs))
}
for i, arg := range tt.args {
if rawArgs[i] != arg {
t.Errorf("expected raw argument %d to be %s, got %s", i, arg, rawArgs[i])
}
}
})
}
}

func TestString(t *testing.T) {
tests := []struct {
args []string
expected string
}{
{[]string{}, ""},
{[]string{"arg1"}, "arg1"},
{[]string{"arg1 with space"}, `"arg1 with space"`},
{[]string{"arg1", "arg2"}, "arg1 arg2"},
{[]string{"arg1", "arg with spaces"}, `arg1 "arg with spaces"`},
{[]string{"arg1", "arg with spaces", "anotherArg"}, `arg1 "arg with spaces" anotherArg`},
{[]string{"--config", "C:\\Program Files\\config.toml"}, `--config "C:\Program Files\config.toml"`},
{[]string{"--config", "C:\\Users\\John Doe\\Documents\\config.toml", "--name", "backup task"},
`--config "C:\Users\John Doe\Documents\config.toml" --name "backup task"`},
{[]string{"--config", "C:\\My Files\\config.toml", "--no-ansi"},
`--config "C:\My Files\config.toml" --no-ansi`},
}

for _, test := range tests {
ca := NewCommandArguments(test.args)
result := ca.String()
if result != test.expected {
t.Errorf("expected %s, got %s", test.expected, result)
}
}
}
6 changes: 4 additions & 2 deletions schedule/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
Permission string
WorkingDirectory string
Command string // path to resticprofile executable
Arguments []string
Arguments CommandArguments
Environment []string
JobDescription string
TimerDescription string
Expand All @@ -37,10 +37,12 @@
}
}

// SetCommand sets the command details for scheduling. Arguments are automatically
// processed to ensure proper handling of paths with spaces and special characters.
func (s *Config) SetCommand(wd, command string, args []string) {
s.WorkingDirectory = wd
s.Command = command
s.Arguments = args
s.Arguments = NewCommandArguments(args)

Check warning on line 45 in schedule/config.go

View check run for this annotation

Codecov / codecov/patch

schedule/config.go#L45

Added line #L45 was not covered by tests
}

// Priority is either "background" or "standard"
Expand Down
5 changes: 3 additions & 2 deletions schedule/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ func TestScheduleProperties(t *testing.T) {
Permission: "admin",
WorkingDirectory: "home",
Command: "command",
Arguments: []string{"1", "2"},
Arguments: NewCommandArguments([]string{"1", "2"}),
Environment: []string{"test=dev"},
JobDescription: "job",
TimerDescription: "timer",
Expand All @@ -35,7 +35,8 @@ func TestScheduleProperties(t *testing.T) {
assert.Equal(t, "admin", schedule.Permission)
assert.Equal(t, "command", schedule.Command)
assert.Equal(t, "home", schedule.WorkingDirectory)
assert.ElementsMatch(t, []string{"1", "2"}, schedule.Arguments)
assert.ElementsMatch(t, []string{"1", "2"}, schedule.Arguments.RawArgs())
assert.Equal(t, `1 2`, schedule.Arguments.String())
assert.Equal(t, []string{"test=dev"}, schedule.Environment)
assert.Equal(t, "background", schedule.GetPriority()) // default value
}
Expand Down
6 changes: 2 additions & 4 deletions schedule/handler_crond.go
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
package schedule

import (
"strings"

"github.com/creativeprojects/resticprofile/calendar"
"github.com/creativeprojects/resticprofile/constants"
"github.com/creativeprojects/resticprofile/crond"
Expand Down Expand Up @@ -68,7 +66,7 @@
job.ConfigFile,
job.ProfileName,
job.CommandName,
job.Command+" "+strings.Join(job.Arguments, " "),
job.Command+" "+job.Arguments.String(),

Check warning on line 69 in schedule/handler_crond.go

View check run for this annotation

Codecov / codecov/patch

schedule/handler_crond.go#L69

Added line #L69 was not covered by tests
job.WorkingDirectory,
)
if h.config.Username != "" {
Expand All @@ -92,7 +90,7 @@
job.ConfigFile,
job.ProfileName,
job.CommandName,
job.Command+" "+strings.Join(job.Arguments, " "),
job.Command+" "+job.Arguments.String(),

Check warning on line 93 in schedule/handler_crond.go

View check run for this annotation

Codecov / codecov/patch

schedule/handler_crond.go#L93

Added line #L93 was not covered by tests
job.WorkingDirectory,
),
}
Expand Down
3 changes: 1 addition & 2 deletions schedule/handler_darwin.go
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,6 @@ Do you want to start it now?`

func (h *HandlerLaunchd) getLaunchdJob(job *Config, schedules []*calendar.Event) *LaunchdJob {
name := getJobName(job.ProfileName, job.CommandName)
args := job.Arguments
// we always set the log file in the job settings as a default
// if changed in the configuration via schedule-log the standard output will be empty anyway
logfile := name + ".log"
Expand All @@ -165,7 +164,7 @@ func (h *HandlerLaunchd) getLaunchdJob(job *Config, schedules []*calendar.Event)
launchdJob := &LaunchdJob{
Label: name,
Program: job.Command,
ProgramArguments: append([]string{job.Command, "--no-prio"}, args...),
ProgramArguments: append([]string{job.Command, "--no-prio"}, job.Arguments.RawArgs()...),
StandardOutPath: logfile,
StandardErrorPath: logfile,
WorkingDirectory: job.WorkingDirectory,
Expand Down
2 changes: 1 addition & 1 deletion schedule/handler_systemd.go
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@
}

err := systemd.Generate(systemd.Config{
CommandLine: job.Command + " " + strings.Join(append([]string{"--no-prio"}, job.Arguments...), " "),
CommandLine: job.Command + " " + strings.Join(append([]string{"--no-prio"}, job.Arguments.RawArgs()...), " "),

Check warning on line 110 in schedule/handler_systemd.go

View check run for this annotation

Codecov / codecov/patch

schedule/handler_systemd.go#L110

Added line #L110 was not covered by tests
Environment: job.Environment,
WorkingDirectory: job.WorkingDirectory,
Title: job.ProfileName,
Expand Down
6 changes: 3 additions & 3 deletions schedule/handler_test.go
Original file line number Diff line number Diff line change
@@ -1,22 +1,22 @@
package schedule

import (
"runtime"
"testing"

"github.com/creativeprojects/resticprofile/platform"
"github.com/stretchr/testify/assert"
)

func TestLookupExistingBinary(t *testing.T) {
if runtime.GOOS == "windows" {
if platform.IsWindows() {
t.Skip()
}
err := lookupBinary("sh", "sh")
assert.NoError(t, err)
}

func TestLookupNonExistingBinary(t *testing.T) {
if runtime.GOOS == "windows" {
if platform.IsWindows() {
t.Skip()
}
err := lookupBinary("something", "almost_certain_not_to_be_available")
Expand Down
2 changes: 1 addition & 1 deletion schedule/handler_windows.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@
ProfileName: job.ProfileName,
CommandName: job.CommandName,
Command: job.Command,
Arguments: job.Arguments,
Arguments: job.Arguments.String(),

Check warning on line 61 in schedule/handler_windows.go

View check run for this annotation

Codecov / codecov/patch

schedule/handler_windows.go#L61

Added line #L61 was not covered by tests
WorkingDirectory: job.WorkingDirectory,
JobDescription: job.JobDescription,
}
Expand Down
2 changes: 1 addition & 1 deletion schedule/mocks/Handler.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions schedule_jobs.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ func scheduleJobs(handler schedule.Handler, profileName string, configs []*confi
args := []string{
"--no-ansi",
"--config",
`"` + scheduleConfig.ConfigFile + `"`,
scheduleConfig.ConfigFile,
"run-schedule",
scheduleName,
}
Expand Down Expand Up @@ -141,7 +141,7 @@ func scheduleToConfig(sched *config.Schedule) *schedule.Config {
Permission: sched.Permission,
WorkingDirectory: "",
Command: "",
Arguments: []string{},
Arguments: schedule.NewCommandArguments(nil),
Environment: sched.Environment,
JobDescription: "",
TimerDescription: "",
Expand Down
5 changes: 3 additions & 2 deletions schedule_jobs_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,12 +42,13 @@ func TestSimpleScheduleJob(t *testing.T) {
mock.AnythingOfType("[]*calendar.Event"),
mock.AnythingOfType("string")).
RunAndReturn(func(scheduleConfig *schedule.Config, events []*calendar.Event, permission string) error {
assert.Equal(t, []string{"--no-ansi", "--config", `"config.file"`, "run-schedule", "backup@profile"}, scheduleConfig.Arguments)
assert.Equal(t, []string{"--no-ansi", "--config", `config file`, "run-schedule", "backup@profile"}, scheduleConfig.Arguments.RawArgs())
assert.Equal(t, `--no-ansi --config "config file" run-schedule backup@profile`, scheduleConfig.Arguments.String())
return nil
})

scheduleConfig := configForJob("backup", "sched")
scheduleConfig.ConfigFile = "config.file"
scheduleConfig.ConfigFile = "config file"
err := scheduleJobs(handler, "profile", []*config.Schedule{scheduleConfig})
assert.NoError(t, err)
}
Expand Down
2 changes: 1 addition & 1 deletion schtasks/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ type Config struct {
ProfileName string
CommandName string
Command string
Arguments []string
Arguments string
WorkingDirectory string
JobDescription string
}
12 changes: 6 additions & 6 deletions schtasks/taskscheduler.go
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@
task := taskService.NewTaskDefinition()
task.AddAction(taskmaster.ExecAction{
Path: config.Command,
Args: strings.Join(config.Arguments, " "),
Args: config.Arguments,

Check warning on line 117 in schtasks/taskscheduler.go

View check run for this annotation

Codecov / codecov/patch

schtasks/taskscheduler.go#L117

Added line #L117 was not covered by tests
WorkingDir: config.WorkingDirectory,
})
task.Principal.LogonType = taskmaster.TASK_LOGON_PASSWORD
Expand Down Expand Up @@ -154,7 +154,7 @@
task.Definition.Actions = make([]taskmaster.Action, 0, 1)
task.Definition.AddAction(taskmaster.ExecAction{
Path: config.Command,
Args: strings.Join(config.Arguments, " "),
Args: config.Arguments,

Check warning on line 157 in schtasks/taskscheduler.go

View check run for this annotation

Codecov / codecov/patch

schtasks/taskscheduler.go#L157

Added line #L157 was not covered by tests
WorkingDir: config.WorkingDirectory,
})
task.Definition.Principal.LogonType = taskmaster.TASK_LOGON_PASSWORD
Expand Down Expand Up @@ -210,7 +210,7 @@
task := taskService.NewTaskDefinition()
task.AddAction(taskmaster.ExecAction{
Path: config.Command,
Args: strings.Join(config.Arguments, " "),
Args: config.Arguments,
WorkingDir: config.WorkingDirectory,
})
task.Principal.LogonType = taskmaster.TASK_LOGON_INTERACTIVE_TOKEN
Expand Down Expand Up @@ -244,7 +244,7 @@
task.Definition.Actions = make([]taskmaster.Action, 0, 1)
task.Definition.AddAction(taskmaster.ExecAction{
Path: config.Command,
Args: strings.Join(config.Arguments, " "),
Args: config.Arguments,

Check warning on line 247 in schtasks/taskscheduler.go

View check run for this annotation

Codecov / codecov/patch

schtasks/taskscheduler.go#L247

Added line #L247 was not covered by tests
WorkingDir: config.WorkingDirectory,
})
task.Definition.Principal.LogonType = taskmaster.TASK_LOGON_INTERACTIVE_TOKEN
Expand Down Expand Up @@ -278,7 +278,7 @@
task := taskService.NewTaskDefinition()
task.AddAction(taskmaster.ExecAction{
Path: config.Command,
Args: strings.Join(config.Arguments, " "),
Args: config.Arguments,

Check warning on line 281 in schtasks/taskscheduler.go

View check run for this annotation

Codecov / codecov/patch

schtasks/taskscheduler.go#L281

Added line #L281 was not covered by tests
WorkingDir: config.WorkingDirectory,
})
task.Principal.LogonType = taskmaster.TASK_LOGON_SERVICE_ACCOUNT
Expand Down Expand Up @@ -307,7 +307,7 @@
task.Definition.Actions = make([]taskmaster.Action, 0, 1)
task.Definition.AddAction(taskmaster.ExecAction{
Path: config.Command,
Args: strings.Join(config.Arguments, " "),
Args: config.Arguments,

Check warning on line 310 in schtasks/taskscheduler.go

View check run for this annotation

Codecov / codecov/patch

schtasks/taskscheduler.go#L310

Added line #L310 was not covered by tests
WorkingDir: config.WorkingDirectory,
})
task.Definition.Principal.LogonType = taskmaster.TASK_LOGON_SERVICE_ACCOUNT
Expand Down
2 changes: 1 addition & 1 deletion schtasks/taskscheduler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -349,7 +349,7 @@ func TestCreationOfTasks(t *testing.T) {
ProfileName: "test",
CommandName: strconv.Itoa(count),
Command: "echo",
Arguments: []string{"hello"},
Arguments: "hello there",
WorkingDirectory: "C:\\",
JobDescription: fixture.description,
}
Expand Down