From 201236cf464e05cc8f5f6643cbad0832fc241da5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sybren=20A=2E=20St=C3=BCvel?= Date: Mon, 20 Jun 2022 17:25:56 +0200 Subject: [PATCH] Refactor: take some functions out of `job_compilers.Service` Take some functions out of the `Service` struct, as they are more or less standalone anyway. This will also make it easier later to make things thread-safe, as that'll become important when files can get live-reloaded. --- .../manager/job_compilers/job_compilers.go | 4 ++- internal/manager/job_compilers/scripts.go | 28 ++++++++++++------- .../manager/job_compilers/scripts_test.go | 17 +++++------ 3 files changed, 28 insertions(+), 21 deletions(-) diff --git a/internal/manager/job_compilers/job_compilers.go b/internal/manager/job_compilers/job_compilers.go index 1d7484b8..216e3863 100644 --- a/internal/manager/job_compilers/job_compilers.go +++ b/internal/manager/job_compilers/job_compilers.go @@ -59,7 +59,9 @@ func Load(ts TimeService) (*Service, error) { } staticFileLoader := func(path string) ([]byte, error) { - content, err := compiler.loadScriptBytes(scriptsFS, path) + // TODO: this should try different filesystems, once we allow loading from + // disk as well. + content, err := loadScriptBytes(scriptsFS, path) if err != nil { // The 'require' module uses this to try different variations of the path // in order to find it (without .js, with .js, etc.), so don't log any of diff --git a/internal/manager/job_compilers/scripts.go b/internal/manager/job_compilers/scripts.go index 12d412b5..6505bb76 100644 --- a/internal/manager/job_compilers/scripts.go +++ b/internal/manager/job_compilers/scripts.go @@ -26,24 +26,32 @@ func (s *Service) loadScripts() error { return fmt.Errorf("failed to find embedded 'scripts' directory: %w", err) } - return s.loadScriptsFrom(scriptsSubFS) + compilers, err := loadScriptsFrom(scriptsSubFS) + if err != nil { + return err + } + + s.compilers = compilers + return nil } // loadScriptsFrom iterates over all given directory entries, compiles the // files, and stores the result into `s.compilers`. -func (s *Service) loadScriptsFrom(filesystem fs.FS) error { +func loadScriptsFrom(filesystem fs.FS) (map[string]Compiler, error) { dirEntries, err := fs.ReadDir(filesystem, ".") if err != nil { - return fmt.Errorf("failed to find scripts in %v: %w", filesystem, err) + return nil, fmt.Errorf("failed to find scripts in %v: %w", filesystem, err) } + compilers := map[string]Compiler{} + for _, dirEntry := range dirEntries { filename := dirEntry.Name() if !strings.HasSuffix(filename, ".js") { continue } - script_bytes, err := s.loadScriptBytes(filesystem, filename) + script_bytes, err := loadScriptBytes(filesystem, filename) if err != nil { log.Error().Err(err).Str("filename", filename).Msg("failed to read script") continue @@ -64,7 +72,7 @@ func (s *Service) loadScriptsFrom(filesystem fs.FS) error { } jobTypeName := filenameToJobType(filename) - s.compilers[jobTypeName] = Compiler{ + compilers[jobTypeName] = Compiler{ jobType: jobTypeName, program: program, filename: filename, @@ -76,10 +84,10 @@ func (s *Service) loadScriptsFrom(filesystem fs.FS) error { Msg("loaded script") } - return nil + return compilers, nil } -func (s *Service) loadScriptBytes(filesystem fs.FS, path string) ([]byte, error) { +func loadScriptBytes(filesystem fs.FS, path string) ([]byte, error) { file, err := filesystem.Open(path) if err != nil { return nil, fmt.Errorf("failed to open embedded script: %w", err) @@ -93,7 +101,7 @@ func filenameToJobType(filename string) string { return strings.ReplaceAll(stem, "_", "-") } -func (s *Service) newGojaVM() *goja.Runtime { +func newGojaVM(registry *require.Registry) *goja.Runtime { vm := goja.New() vm.SetFieldNameMapper(goja.UncapFieldNameMapper()) @@ -111,7 +119,7 @@ func (s *Service) newGojaVM() *goja.Runtime { mustSet("formatTimestampLocal", jsFormatTimestampLocal) // Pre-import some useful modules. - s.registry.Enable(vm) + registry.Enable(vm) mustSet("author", require.Require(vm, "author")) mustSet("path", require.Require(vm, "path")) mustSet("process", require.Require(vm, "process")) @@ -127,7 +135,7 @@ func (s *Service) compilerForJobType(jobTypeName string) (*VM, error) { return nil, ErrJobTypeUnknown } - vm := s.newGojaVM() + vm := newGojaVM(s.registry) if _, err := vm.RunProgram(program.program); err != nil { return nil, err } diff --git a/internal/manager/job_compilers/scripts_test.go b/internal/manager/job_compilers/scripts_test.go index 7a614d72..4a97f508 100644 --- a/internal/manager/job_compilers/scripts_test.go +++ b/internal/manager/job_compilers/scripts_test.go @@ -8,26 +8,23 @@ import ( ) func TestLoadScriptsFrom_skip_nonjs(t *testing.T) { - s := Service{} - thisDirFS := os.DirFS(".") - assert.NoError(t, s.loadScriptsFrom(thisDirFS), "input without JS files should not cause errors") - assert.Empty(t, s.compilers) + compilers, err := loadScriptsFrom(thisDirFS) + assert.NoError(t, err, "input without JS files should not cause errors") + assert.Empty(t, compilers) } func TestLoadScriptsFrom_on_disk_js(t *testing.T) { - s := Service{ - compilers: map[string]Compiler{}, - } - scriptsFS := os.DirFS("scripts-for-unittest") - assert.NoError(t, s.loadScriptsFrom(scriptsFS)) + compilers, err := loadScriptsFrom(scriptsFS) + + assert.NoError(t, err) expectKeys := map[string]bool{ "echo-and-sleep": true, "simple-blender-render": true, // Should NOT contain an entry for 'empty.js'. } - assert.Equal(t, expectKeys, keys(s.compilers)) + assert.Equal(t, expectKeys, keys(compilers)) } // keys returns the set of keys of the mapping.