From 56786491e8fa851e0801b365c0e66bf5afe1c153 Mon Sep 17 00:00:00 2001 From: jamesread Date: Tue, 13 Jan 2026 17:47:28 +0000 Subject: [PATCH 1/4] fix: Port the calender schedulding fix from 2k to 3k (#830) --- .pre-commit-config.yaml | 23 ++++++ Makefile | 6 ++ frontend/js/websocket.js | 2 +- service/internal/oncalendarfile/calendar.go | 81 ++++++++++++++++++++- 4 files changed, 108 insertions(+), 4 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index ef5f169..4efad37 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -17,3 +17,26 @@ repos: - id: conventional-pre-commit stages: [commit-msg] args: [] # optional: list of Conventional Commits types to allow e.g. [feat, fix, ci, chore, test] + + - repo: local + hooks: + - id: service-codestyle + name: service-codestyle + entry: make service-codestyle + language: system + pass_filenames: false + always_run: true + + - id: frontend-codestyle + name: frontend-codestyle + entry: make frontend-codestyle + language: system + pass_filenames: false + always_run: true + + - id: it + name: it + entry: make service-codestyle frontend-codestyle + language: system + pass_filenames: false + always_run: true diff --git a/Makefile b/Makefile index aa70614..5ce367c 100644 --- a/Makefile +++ b/Makefile @@ -11,6 +11,12 @@ service-prep: service-unittests: $(MAKE) -wC service unittests +service-codestyle: + $(MAKE) -wC service codestyle + +frontend-codestyle: + $(MAKE) -wC frontend codestyle + it: $(MAKE) -wC integration-tests diff --git a/frontend/js/websocket.js b/frontend/js/websocket.js index 72e42c5..8c0200f 100644 --- a/frontend/js/websocket.js +++ b/frontend/js/websocket.js @@ -76,4 +76,4 @@ function onExecutionChanged (evt) { // Clear rate limit if not set rateLimits[logEntry.bindingId] = 0 } -} \ No newline at end of file +} diff --git a/service/internal/oncalendarfile/calendar.go b/service/internal/oncalendarfile/calendar.go index 266ea01..4d424fb 100644 --- a/service/internal/oncalendarfile/calendar.go +++ b/service/internal/oncalendarfile/calendar.go @@ -3,6 +3,7 @@ package oncalendarfile import ( "context" "os" + "sync" "time" "github.com/OliveTin/OliveTin/internal/auth" @@ -13,11 +14,27 @@ import ( "gopkg.in/yaml.v3" ) +type timerEntry struct { + timer *time.Timer + cancel context.CancelFunc +} + +type existingTimers struct { + timers map[time.Time]timerEntry +} + +var ( + scheduleMap = make(map[string]existingTimers) + scheduleMapMutex sync.RWMutex +) + func Schedule(cfg *config.Config, ex *executor.Executor) { for _, action := range cfg.Actions { + captured := action + if action.ExecOnCalendarFile != "" { x := func(filename string) { - parseCalendarFile(action, cfg, ex, filename) + parseCalendarFile(captured, cfg, ex, filename) } go filehelper.WatchFileWrite(action.ExecOnCalendarFile, x) @@ -27,7 +44,30 @@ func Schedule(cfg *config.Config, ex *executor.Executor) { } } +func clearExistingTimers(action *config.Action) { + scheduleMapMutex.Lock() + defer scheduleMapMutex.Unlock() + + if _, exists := scheduleMap[action.ID]; exists { + for instant, entry := range scheduleMap[action.ID].timers { + log.WithFields(log.Fields{ + "instant": instant, + "actionTitle": action.Title, + }).Infof("Clearing existing scheduled action from calendar") + + entry.cancel() + entry.timer.Stop() + } + } + + scheduleMap[action.ID] = existingTimers{ + timers: make(map[time.Time]timerEntry), + } +} + func parseCalendarFile(action *config.Action, cfg *config.Config, ex *executor.Executor, filename string) { + clearExistingTimers(action) + filehelper.Touch(action.ExecOnCalendarFile, "calendar file") log.WithFields(log.Fields{ @@ -61,12 +101,41 @@ func scheduleCalendarActions(entries []string, action *config.Action, cfg *confi continue } - until, _ := time.Parse(time.RFC3339, instant) + until, err := time.Parse(time.RFC3339, instant) + + if err != nil { + log.WithFields(log.Fields{ + "instant": instant, + "actionTitle": action.Title, + }).Warnf("Invalid calendar entry, skipping: %v", err) + continue + } go sleepUntil(ctx, until, action, cfg, ex) } } +func registerTimer(action *config.Action, instant time.Time, timer *time.Timer, cancel context.CancelFunc) { + scheduleMapMutex.Lock() + defer scheduleMapMutex.Unlock() + + if _, exists := scheduleMap[action.ID]; !exists { + scheduleMap[action.ID] = existingTimers{ + timers: make(map[time.Time]timerEntry), + } + } + scheduleMap[action.ID].timers[instant] = timerEntry{ + timer: timer, + cancel: cancel, + } +} + +func unregisterTimer(action *config.Action, instant time.Time) { + scheduleMapMutex.Lock() + delete(scheduleMap[action.ID].timers, instant) + scheduleMapMutex.Unlock() +} + func sleepUntil(ctx context.Context, instant time.Time, action *config.Action, cfg *config.Config, ex *executor.Executor) { if time.Now().After(instant) { log.WithFields(log.Fields{ @@ -82,15 +151,21 @@ func sleepUntil(ctx context.Context, instant time.Time, action *config.Action, c "actionTitle": action.Title, }).Infof("Scheduling action on calendar") + childCtx, cancel := context.WithCancel(ctx) timer := time.NewTimer(time.Until(instant)) + registerTimer(action, instant, timer, cancel) + defer timer.Stop() + defer cancel() select { case <-timer.C: + unregisterTimer(action, instant) exec(instant, action, cfg, ex) return - case <-ctx.Done(): + case <-childCtx.Done(): + unregisterTimer(action, instant) log.Infof("Cancelled scheduled action") return } From b3430e3a5c5fdbd158fc1b5aa5c524b6aa2f41be Mon Sep 17 00:00:00 2001 From: jamesread Date: Wed, 21 Jan 2026 23:12:39 +0000 Subject: [PATCH 2/4] fix: execOnCalendarFile, avoid possible crash deleting nil timers --- service/internal/oncalendarfile/calendar.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/service/internal/oncalendarfile/calendar.go b/service/internal/oncalendarfile/calendar.go index 4d424fb..21d8ba0 100644 --- a/service/internal/oncalendarfile/calendar.go +++ b/service/internal/oncalendarfile/calendar.go @@ -132,7 +132,10 @@ func registerTimer(action *config.Action, instant time.Time, timer *time.Timer, func unregisterTimer(action *config.Action, instant time.Time) { scheduleMapMutex.Lock() - delete(scheduleMap[action.ID].timers, instant) + v := scheduleMap[action.ID] + if v.timers != nil { + delete(v.timers, instant) + } scheduleMapMutex.Unlock() } From aeaddda870d43134e554597e10b720390476bace Mon Sep 17 00:00:00 2001 From: jamesread Date: Fri, 23 Jan 2026 00:04:55 +0000 Subject: [PATCH 3/4] fix: add env support back --- service/internal/entities/storage.go | 11 +++++++++++ service/internal/entities/templates.go | 1 + 2 files changed, 12 insertions(+) diff --git a/service/internal/entities/storage.go b/service/internal/entities/storage.go index 2835558..53cdbbc 100644 --- a/service/internal/entities/storage.go +++ b/service/internal/entities/storage.go @@ -10,6 +10,7 @@ package entities */ import ( + "os" "strings" "sync" @@ -26,6 +27,7 @@ type variableBase struct { CurrentEntity interface{} Arguments map[string]string + Env map[string]string } type installationInfo struct { @@ -41,12 +43,21 @@ var ( func init() { rwmutex.Lock() + envMap := make(map[string]string) + for _, env := range os.Environ() { + parts := strings.SplitN(env, "=", 2) + if len(parts) == 2 { + envMap[parts[0]] = parts[1] + } + } + contents = &variableBase{ OliveTin: installationInfo{ Build: installationinfo.Build, Runtime: installationinfo.Runtime, }, Entities: make(entitiesByClass, 0), + Env: envMap, } rwmutex.Unlock() diff --git a/service/internal/entities/templates.go b/service/internal/entities/templates.go index 53092de..21db7c0 100644 --- a/service/internal/entities/templates.go +++ b/service/internal/entities/templates.go @@ -94,6 +94,7 @@ func ParseTemplateWithArgs(source string, ent *Entity, args map[string]string) s OliveTin: GetAll().OliveTin, Arguments: args, CurrentEntity: entdata, + Env: GetAll().Env, } var sb strings.Builder From 1e774f00786216fc94058aacd78294eeb18ba24c Mon Sep 17 00:00:00 2001 From: jamesread Date: Fri, 23 Jan 2026 08:54:32 +0000 Subject: [PATCH 4/4] chore: integration test for env --- integration-tests/runner.mjs | 4 +- .../tests/envTemplateIcon/config.yaml | 14 +++ .../tests/envTemplateIcon/envTemplateIcon.mjs | 116 ++++++++++++++++++ 3 files changed, 133 insertions(+), 1 deletion(-) create mode 100644 integration-tests/tests/envTemplateIcon/config.yaml create mode 100644 integration-tests/tests/envTemplateIcon/envTemplateIcon.mjs diff --git a/integration-tests/runner.mjs b/integration-tests/runner.mjs index 853d47c..5e0aba0 100644 --- a/integration-tests/runner.mjs +++ b/integration-tests/runner.mjs @@ -38,7 +38,9 @@ class OliveTinTestRunnerStartLocalProcess extends OliveTinTestRunner { console.log(" OliveTin starting local process...") - this.ot = spawn('./../service/OliveTin', ['-configdir', 'tests/' + cfg + '/']) + this.ot = spawn('./../service/OliveTin', ['-configdir', 'tests/' + cfg + '/'], { + env: process.env + }) let logStdout = false diff --git a/integration-tests/tests/envTemplateIcon/config.yaml b/integration-tests/tests/envTemplateIcon/config.yaml new file mode 100644 index 0000000..2db8055 --- /dev/null +++ b/integration-tests/tests/envTemplateIcon/config.yaml @@ -0,0 +1,14 @@ +# +# Integration Test Config: envTemplateIcon +# Tests that .Env template variable works in icon field +# + +listenAddressSingleHTTPFrontend: 0.0.0.0:1337 + +logLevel: "DEBUG" +checkForUpdates: false + +actions: +- title: Test Action with Env Icon + shell: echo "test" + icon: "{{ .Env.ADGUARD_ICON }}" diff --git a/integration-tests/tests/envTemplateIcon/envTemplateIcon.mjs b/integration-tests/tests/envTemplateIcon/envTemplateIcon.mjs new file mode 100644 index 0000000..475376b --- /dev/null +++ b/integration-tests/tests/envTemplateIcon/envTemplateIcon.mjs @@ -0,0 +1,116 @@ +import { describe, it, before, after } from 'mocha' +import { expect } from 'chai' +import { By, Condition } from 'selenium-webdriver' +import * as process from 'node:process' +import { + getRootAndWait, + takeScreenshotOnFailure, +} from '../../lib/elements.js' + +describe('config: envTemplateIcon', function () { + before(async function () { + // Set the environment variable before starting the runner + process.env.ADGUARD_ICON = 'test.png' + + await runner.start('envTemplateIcon') + }) + + after(async () => { + await runner.stop() + + // Clean up the environment variable + delete process.env.ADGUARD_ICON + }) + + afterEach(function () { + takeScreenshotOnFailure(this.currentTest, webdriver); + }); + + it('Action icon is set from .Env template variable', async function () { + await getRootAndWait() + + // Get the dashboard data which contains the actions + // executeScript automatically waits for Promises returned from the script + const dashboardResponse = await webdriver.executeScript(` + return window.client.getDashboard({ title: 'Actions' }) + `) + + expect(dashboardResponse).to.not.be.null + expect(dashboardResponse).to.have.own.property('dashboard') + expect(dashboardResponse.dashboard).to.have.own.property('contents') + expect(dashboardResponse.dashboard.contents).to.be.an('array') + expect(dashboardResponse.dashboard.contents.length).to.be.greaterThan(0) + + // Actions are nested in dashboard contents - find the action with the expected title + // The structure is: dashboard.contents[] -> component.contents[] -> action + let testAction = null + for (const component of dashboardResponse.dashboard.contents) { + if (component.contents && Array.isArray(component.contents)) { + for (const subcomponent of component.contents) { + if (subcomponent.title === 'Test Action with Env Icon') { + testAction = subcomponent + break + } + } + } + // Also check if the component itself is the action + if (component.title === 'Test Action with Env Icon') { + testAction = component + break + } + if (testAction) break + } + + expect(testAction).to.not.be.null + expect(testAction).to.have.own.property('icon') + expect(testAction.icon).to.equal('test.png') + }) + + it('Action button displays icon from .Env template variable', async function () { + await getRootAndWait() + + // Wait for the action button to be rendered + await webdriver.wait(new Condition('wait for action button', async () => { + const btns = await webdriver.findElements(By.css('[title="Test Action with Env Icon"]')) + return btns.length === 1 + }), 10000) + + // Verify the button exists + const buttons = await webdriver.findElements(By.css('[title="Test Action with Env Icon"]')) + expect(buttons).to.have.length(1) + + // The icon should be rendered in the button - we can check via the GetDashboard API response + // which is the most reliable way to verify the template parsing worked + // executeScript automatically waits for Promises returned from the script + const dashboardResponse = await webdriver.executeScript(` + return window.client.getDashboard({ title: 'Actions' }) + `) + + expect(dashboardResponse).to.not.be.null + expect(dashboardResponse).to.have.own.property('dashboard') + + // Find the action with the expected title + // The structure is: dashboard.contents[] -> component.contents[] -> action + let testAction = null + for (const component of dashboardResponse.dashboard.contents) { + if (component.contents && Array.isArray(component.contents)) { + for (const subcomponent of component.contents) { + if (subcomponent.title === 'Test Action with Env Icon') { + testAction = subcomponent + break + } + } + } + // Also check if the component itself is the action + if (component.title === 'Test Action with Env Icon') { + testAction = component + break + } + if (testAction) break + } + + expect(testAction).to.not.be.null + expect(testAction).to.have.own.property('icon') + expect(testAction.icon).to.equal('test.png') + }) +})