RFC: Plugin-Registrable Actions #1380
Replies: 3 comments 33 replies
|
Gap 1, i was thinking free form so plugins can pick the same categories as the core if they want to. Gap 2, dynamic. Gap 3, we could surface it, if we want to support it in the first place. gap 4, I'd lean on UUIDs gap 5, obviously the id would change if the class name changes. that was intentional in the proposal i made in chat, but if you want stable ids across refactors and renames then we can introduce a new key or id field, in which the id is either used as is, or derived from. but now we need to manage key/id collision across all actions across all active plugins, whereas if we leave it on auto inferred from the class name (+ namespace) then there will be less likely that there would be collisions from different plugins. gap 6, for api shape, then whatever you (@hidden4003) or @harshithmohan wants will do. gap 7, depends on what we want gap 8. all actions would require admin. privileges, as normal users shouldn't need to nor have the right to run them. if someone disagrees then we can take it from there. gap 9, ephemeral instances, created when iterated through the service. if a plugin wants a singleton, they can declare it as such. we only ask the DI to initialize or grab an instance for us to use, so not our concern. gap 10, depending on if we run it in the queue or not. if it's ran in the queue, then we log it the same way as when running in the queue. if we run it directly, then we can surface it to the client or caller. gap 11, cancellation would be set by the caller. so for direct calls by a client, then would be bound to the request timeout/abort, whereas for other plugins it would be whatever they decide. timeout would just be a cancellation token which cancels after a set time. gap 12, we won't remove the older actions, but we'll port all of them to use the newer system at once, so there will be a grace period where both the older actions and new actions will function for the client to implement it's needed changes to use the newer system. gap 13, no need to document here. just know there are a few. gap 14, everything would be parameterless (except the cancellation token, which is optional), but there could still be actions to run which depends on other factors than caller parameters. gap 15, I'd lean on opt-in, so the service has a direct call and schedule in queue, and the client can decide to run it then and there, or run it in the queue. if ran then and there, then the cancellation token will be tied to the request, but if ran in the queue, then it would be tied to the system uptime or just not set. gap 16, that's for you (@hidden4003) and @harshithmohan to decide if you want to change it or keep it. gap 17, |
|
But we could
…On Wed, Jul 8, 2026, 20:01 hidden4003 ***@***.***> wrote:
we currently have descriptions for actions, not for categories
—
Reply to this email directly, view it on GitHub
<#1380?email_source=notifications&email_token=AGGOKQTIKRWMT5A4IXYVXV35DZLLVA5CNFSNUABIM5UWIORPF5TWS5BNNB2WEL2ENFZWG5LTONUW63SDN5WW2ZLOOQXTCNZVG4ZTQOJXUZZGKYLTN5XKO3LFNZ2GS33OUVSXMZLOOSWGM33PORSXEX3DNRUWG2Y#discussioncomment-17573897>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AGGOKQUIA3HWTISLBZSKGA35DZLLVAVCNFSNUABHKJSXA33TNF2G64TZHMZDEOJXHE4DQOB3IRUXGY3VONZWS33OHMYTAMZYGE3TQMFBOYBA>
.
You are receiving this because you were mentioned.Message ID:
***@***.***
com>
|
|
will this be User Based/Profile-like settings for plugins ? or it all-for-one ? |
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Plugin-Registerable Actions — Spec (v5, replaces parameterless
Executewith a unified context/parameter model)Problem & Goal
Unchanged from v4 — see discussion #1380 for the original framing. This revision only changes how actions receive context and parameters, and how the API surfaces validation/results around mandatory queueing.
Proposed interface
Category, its default (PluginInferred), and the decision not to add a central per-categorydescriptionfield are carried forward unchanged from the PR #1383 discussion thread — @harshithmohan and @revam already reached consensus on both ("Leave it off then"). Not reopened here.Scope & context — replaces v4's
Scopeproperty and the PR's 8 marker interfacesFour lightweight base classes, one per entity level.
Globalneeds no context and implementsIExecutableActiondirectly; the other three carry a typed, framework-populated context via an explicitly-implemented interface method, invisible on the class's own public surface:A plugin author writing a series-scoped action never sees
SetContext— they just referenceSeriesinsideExecute, already non-null:Why 4 base classes instead of 8 interfaces:
Permissionstays its own orthogonal property (per reopened Gap 8/19), so scope and permission are no longer multiplied together. This also restores the RFC's original two-axis design intent without losing the compile-time non-null guarantee that motivated the PR's sub-interface split in the first place —Executenever has to null-check its own context, it just doesn't have a public setter for it.Free-form parameters — resolves Gap 23
Ordinary public settable properties on the action instance, populated the same way
IQueueJobproperties already are fromJobDataJson— the invoke endpoint's request body is deserialized onto the instance via Newtonsoft'sPopulateObject(the serializer already backing the rest of the API). BecauseSetContextis an explicit interface implementation, a request body can't accidentally overwrite scope context by naming a field the same as the protected property.Invoke request:
POST /Actions/{id}/Executewith a JSON body matching the action's own settable properties. Actions with no free-form parameters simply ignore any body sent.Synchronous validation — resolves Gap 24
Validateruns on the API thread, before anything is enqueued. A non-null result means the invoke endpoint returns 400 with the givenReasonand never touches the queue:Checks that depend on the calling user (e.g. today's
User.IsAniDBUser != 1check) use the calling-user context described below —Validatereceives it the same wayExecutedoes.Calling-user identity — resolves Gap 26
Distinct from
Scope's entity context and from aUser-permission action's subject. Any action — regardless of scope — can opt in to knowing who invoked it by implementing a small marker interface, populated by the framework the same wayIScopedAction.SetContextis:SyncVotesActionabove would implement this to replace today'sUser.JMMUserID/User.IsAniDBUserchecks. This is additive and orthogonal to everything else — most actions won't need it.Registration & validation at plugin load — recovers compile-time safety as load-time safety
Plugins are reflection-loaded regardless of design, so nothing here was ever compiler-checked.
ActionService.AddParts(called fromPluginManager.InitPlugins, same placeGetExports<IExecutableAction>already runs today) validates each registered type at startup and fails fast with a named error, not a null-reference three weeks into production:Scope(via its base class) doesn't match what it derives from — rejected.Permissionnot overridden — rejected (no silent default; every action author must choose).ActionServiceknows each action's scope at registration time, soPOST /Series/{id}/Actions/{actionId}/Executeagainst a type that isn't aSeriesAction400s immediately — no per-request reflection needed.Execution service — one generic wrapper job
A single
ActionExecutionJob : IQueueJobhandles every action, so plugins never write their own job class:IExecutableActioninstance from DI (transient, per Gap 9 — unchanged).JobDataJson.AnimeSeriesby ID, also carried inJobDataJson) via the existing repositories and callsSetContext.IActionCaller, resolves the callingIUserand callsSetCaller.Execute.Result reporting — resolves Gap 25, no new response shape
Rather than reintroducing a tracking ID or a synchronous result payload (both intentionally dropped in v4 Gap 7, and that reasoning still holds for the invoke response), post-execution results use
IQueueJob.Details— theDictionary<string, object>field that already exists for queue display purposes — populated byActionExecutionJobafterExecutecompletes. The queue system already bridges job lifecycle to SignalR (QueueStateEventHandler→QueueEventEmitter→AggregateHub), which the Actions UI needs anyway per Gap 15's "progress/status visible" requirement."Saved 42 AddToMyList Commands"becomes aDetailsentry surfaced through that existing channel instead of an HTTP response body.Migration — corrects Gap 12/13's effort estimate
Of the ~30 actions in
ActionController.cstoday, the roughly 20 that currently run via rawTask.Run/Task.Factory.StartNew(bypassing the queue entirely — e.g.PurgeAllTmdbLinks,RecreateAllGroups,UpdateAllTmdbShows,DownloadAllImages) need actual queue-backedIExecutableActionimplementations written, not just interface conformance. This is real, sequenceable implementation work, not a "known to be a few, non-blocking" bookkeeping pass — v4's Gap 13 estimate was wrong. Recommend inventorying these ~20 explicitly as part of implementation planning (reopens the "no inventory needed" part of Gap 13 specifically for this reason), each noting its real currentPermission, same requirement v4 already stated for the ones that were already job-backed.Unchanged from v4
permission) — Gap 6, extended with the same field for scope's base-class-derived value.permission— Gap 16.IExecutableActionnaming — Gap 17.Gap tracker (v5 deltas only — v4's Gaps 1–7, 9–17, 20–22 carry forward unchanged)
Executedropped as unworkable under mandatory async queueing; replaced by typed context via 4 scope base classes + explicit-interfaceSetContext, invisible to the plugin authorIQueueJobproperties are populated fromJobDataJsonValidatemethod, runs before enqueue, can 400 without touching the queueIQueueJob.Details+ existing SignalR queue events, no new response shapeIActionCallermarker, orthogonal toScopeand toUser-permission's subject7 items touched in v5 (2 reopened, 4 newly added, 1 corrected); all other v4 gaps stand as previously resolved.
All reactions