NMS-20111: Updates BSF in Notifd to JSR-223 - #8718
Open
marshallmassengill wants to merge 4 commits into
Open
Conversation
Apache BSF is retired to the Apache Attic; the strategy now runs
scripts through javax.script. The class name, all 25 script variable
names, the results["status"]=="OK" contract, run-type exec/eval
semantics, and the 0/-1 return mapping are unchanged, so existing
notificationCommands.xml entries and scripts keep working. The engine
is chosen by lang-class (JSR-223 name) or the file extension, with
BSF's .gy alias mapped to Groovy; bsf-engine and file-extensions are
accepted but ignored with a warning.
Compiled scripts are cached per path+engine with lastModified
invalidation (Groovy leaks a class per compilation, same rationale as
provisiond's JSR223ScriptCache); the compile attempt catches Throwable
because BeanShell declares Compilable but its compile() throws
Error("unimplemented"), in which case scripts are evaluated from
source on a fresh per-invocation engine. Bindings are built per
invocation, which drops the old static synchronized serialization of
every scripted notification and the declare/undeclare dance.
The IT grows from one to nine cases (eval, Groovy, .gy, lang-class
resolution, missing file, non-OK status, invalid run-type, deprecated
switches) and uses the database populator's actual node id: node ids
are sequence-assigned, so the hardcoded 1 only worked while the class
had a single test.
The strategy was never documented; adds a Scripted Notifications page (parameters, script variables, the results.status success contract, BSF migration notes) to the notification strategies section, plus the release-note entries for the JSR-223 port.
- BeanShell scripts see null variables again: bsh's JSR-223 engine treats a null binding as an undefined variable, breaking scripts that test node fields against null (worked under BSF). The BeanShell path now drives bsh.Interpreter directly, as BSF did; Interpreter.set defines nulls properly. Reflective, since bsh is a runtime-scope dependency. - The command's <substitution> element is honored when the argument value is empty, so the strategy's own switches (file-name, lang-class, run-type) can be configured in notificationCommands.xml the way the docs show; notifd passes empty values for switches without a notification parameter, which previously made the documented setup - and the bsf-engine deprecation warning - unreachable. - Compile failures no longer latch the script-cache entry when the script itself is at fault: a fixed script compiles again. Only engines that cannot compile (BeanShell throws Error) latch. Cache keys derive from lang-class/extension so cache hits build no throwaway engine, and mtime comparison uses != so restored-older files also invalidate. - Groovy engines come from a runtime-scope groovy-dependencies entry instead of relying on provision-persistence keeping groovy on the classpath; a GLOBAL_SCOPE bindings object avoids a bsh NPE path. - IT covers the review gaps: null bindings, substitution-shaped config, cache reuse + mtime invalidation, missing file-name. - Docs: substitution vs notification-parameter configuration spelled out, microblog_username listed as deprecated, release note no longer overstates engine-state isolation and mentions strict UTF-8.
marshallmassengill
requested review from
cgorantla,
Copilot,
dino2gnt and
joseanesONMS
July 31, 2026 12:55
Contributor
There was a problem hiding this comment.
Pull request overview
This pull request migrates BSFNotificationStrategy in notifd from the retired Apache BSF runtime to JSR-223 (javax.script) while keeping the existing strategy class name, script variable names, and the results.status == "OK" success contract for compatibility.
Changes:
- Reimplemented
BSFNotificationStrategyto execute scripts via JSR-223, including engine resolution (bylang-classor file extension) and a compiled-script cache for Groovy. - Expanded integration test coverage to validate engine selection, run-type behavior (
exec/eval), null-variable behavior (BeanShell), substitution handling, failure cases, and cache invalidation. - Added/updated documentation and release notes describing the behavioral changes and how to configure scripted notifications; added Groovy runtime dependencies.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| opennms-services/src/main/java/org/opennms/netmgt/notifd/BSFNotificationStrategy.java | Ports script execution from BSF to JSR-223, adds engine resolution and compiled-script caching, preserves variable contract. |
| opennms-services/src/test/java/org/opennms/netmgt/notifd/BSFNotificationStrategyIT.java | Adds comprehensive IT coverage for the new JSR-223 behavior (engines, run types, nulls, substitution, cache, failure modes). |
| opennms-services/pom.xml | Ensures Groovy JSR-223 runtime dependencies are available for scripted notifications. |
| docs/modules/releasenotes/pages/whatsnew.adoc | Documents the breaking-change notes for the BSF → JSR-223 migration. |
| docs/modules/releasenotes/pages/changelog.adoc | Adds a changelog entry for the migration. |
| docs/modules/operation/pages/deep-dive/notifications/strategies/scripting.adoc | New operator documentation for configuring and writing scripted notifications. |
| docs/modules/operation/pages/deep-dive/notifications/bonus-strategies.adoc | Updates the overview description to include scripted notifications. |
| docs/modules/operation/nav.adoc | Adds navigation entry to the new scripted notifications page. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…1-smoke Three conflicts, all from notifd work that landed on release-36.x while this branch was open. BSFNotificationStrategy: the microblog removal deleted the two PARAM_MICROBLOG_USERNAME lines, and this branch had rewritten the same loop from BSF declareBean calls to JSR-223 bindings. Kept the JSR-223 form and dropped the microblog binding, since the constant no longer exists in NotificationManager. changelog.adoc and bonus-strategies.adoc: kept both sides, ordering the 36.0.3 entries by issue number and listing scripted notifications alongside trouble tickets in the page description.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Updates BSF in Notifd to JSR-223 and adds some examples.
I'll need to resolve the merge conflicts on this one for whats new.
Assissted by Anthropic Claude Opus 5.
External References