jester
jester copied to clipboard
Adding support for "plugins" to Jester
I've already written a version of this that works but is ugly. I'd like to write a cleaner version and create a PR.
Essentially, this will be a means for adding to Jester with additional nimble libraries. For example:
import htmlgen
import jester
import crazything
routes:
plugin crazyThingPlugin(myName)
get "/":
resp h1("Hello " & myName)
A "plugin" is a name that, by naming convention, will have four associated sections:
-
beforeThread
-
beforeRoutes
-
afterRoutes
-
afterThread
These are inserted into the generated code at the correct places for any router containing the plugin reference. Multiple plugins from disparate sources is supported, but order matters. The first plugin runs it's beforeThread
first and it's afterThread
last; and so on.
Interested?
Actually, only need two sections. I've got this working:
theplugins.nim
:
import jester
proc mystuff_before*(request: Request): string =
result = "Joe"
proc mystuff_after*(firstvar: string, request: Request) =
echo "done with route: " & firstvar
proc dependentCrazyPlugin_before*(request: Request, somevar: var string): int =
if somevar == "Joe":
result = 123
else:
result = 456
somevar &= " A."
proc dependentCrazyPlugin_after*(firstvar: int, request: Request, somevar: string) =
echo "done with route (dcpg): " & $firstvar
proc unrelatedPlugin_before*(request: Request): string =
discard
proc unrelatedPlugin_after*(firstvar: string, request: Request) =
echo "I'm doing my super logging function!"
main.nim
:
import htmlgen
import jester
import theplugins
routes:
plugin foo <- mystuff()
plugin bar <- dependentCrazyPlugin(foo)
plugin xyz <- unrelatedPlugin()
get "/":
foo &= " Smith"
resp h1("Hello " & foo)
this modifies the generated match
procedure to:
proc match(request: Request): Future[ResponseData] {.async, gcsafe.} =
block allRoutes:
setDefaultResp()
var request = request
var foo = mystuff_before(request)
var bar = dependentCrazyPlugin_before(request, foo)
var xyz = unrelatedPlugin_before(request)
block routesList:
block routesList:
case request.reqMethod
of HttpGet:
block outerRoute:
if request.pathInfo == "/":
block route:
foo &= " Smith"
resp h1("Hello " & foo)
if checkAction(result):
result.matched = true
break routesList
of HttpPost:
of HttpPut:
of HttpDelete:
of HttpHead:
of HttpOptions:
of HttpTrace:
of HttpConnect:
of HttpPatch:
block routesList:
unrelatedPlugin_after(xyz, request)
dependentCrazyPlugin_after(bar, request, foo)
mystuff_after(foo, request)
The reason for the "var <- procname(...)" format is I'd like to both:
- make it possible for plugins to communicate with the routing code (and each other); and yet
- make it hard to create a big mess inside the generated
match
procedure.
The "single variable" can, of course, be a object with lots of fields; so it's not that bad of a restriction I suspect.
Proof of concept would be writing two or three real-world plug-ins.
Possible plugins:
-
interpageMsg
-- Uses cookies to pass a list of messages between web pages along with descriptive information such as log_level, priority, and nature (success, fail, warn, info). Useful for websites that use forms and similar activity. -
mongoTracker
-- Uses mymongopool
library to store web logs in MongoDB, which includes progressive summation of hourly, daily, and monthly usage that would be typical of NoSQL dbs. -
loginManager
-- Uses a set of global constants and procedure references to setup programmer-defined user login/logout using hashed session keys stored in cookies.
(This is actually what I want to do for one of my web sites. At the moment, I'm doing all these things in a fairly messy way.)
I've created PR #227 for tracking progress.
is this similar to the express request middleware? https://expressjs.com/en/guide/using-middleware.html
This is nice to have
It will certainly have many of the same characteristics and role as the the express middleware.
The biggest difference is that the mechanism used by plugin
is nim's macro system rather than a dynamic chain.
That reminds me, I'll be posting a new commit fairly soon in the [WIP] PR #227 for resp
modification (internally, a tuple for the result
return variable.)
EDIT: I'll also be likely modifying my loginManager
plugin to use nimble's httpauth
library.
Thanks for working on this @JohnAD. I currently don't have enough time to review your PR which I assume will require quite a bit of time, just want to let you know that I'm keeping this at the back of my mind. Also do rename your PR to get rid of the "WIP" when you're ready for review.
As part of the testing and development, I've created a repo for a plugin:
https://github.com/JohnAD/jestercookiemsgs
It is a plugin for easily passing notice messages into and between web pages.
(Ignore the README's nimble link. I won't be publishing this until if/after the plugin PR is live.)
Also now made a database plugin (for MongoDB via mongopool
library):
https://github.com/JohnAD/jestermongopool
I'm also using about 3 private plugins on one of my websites. Looking good so far. Will be testing against threaded use next. Not too far from cleaning up and finishing the PR.
ready for review ~~...mostly. See the last two notes in the PR.~~ @dom96
Made a short convenience plugin:
https://github.com/JohnAD/jesterjson
The idea is to make a thread-safe json
variable that contains most of the parameters of request
and, eventually, various environmental/docker variables. For any website that depends on JSON for moving data from the controller (jester
) to the rules-model or views, this can make for cleaner code.
Like the other plugins, I'll refrain from posting onto nimble.directory until or if the plugin support goes live.
Made a plugin to automatically pull Geo IP location information for each request, but with a 30-day sqlite3 cache to prevent hitting the upstream API too hard. Details at:
https://github.com/JohnAD/jestergeoip
Again, I'm refraining from posting on nimble until the plugin support is live in jester.
I communicated yesterday with @dom96 about a way to move forward. For the time being, to allow more folks to "try plugins out" before merging the many changes into the mainline jester
library, I will be making a temporary fork called jesterwithplugins
that has plugins and I will publish on nimble. I'll also post the various plugins I've written to work with jesterwithplugins
.
Then, after more real-world testing and development has occurred, we will re-merge the library back into canonical jester
. (Essentially, jesterwithplugins
will just become an alias for jester.)
@dom96 and everyone else:
Should I do a talk about jester plugins at the up-and-coming Nim conference?
Sure, why not.
Great work on this @JohnAD :) I tried out your fork and wrote a couple of simple plugins. Some feedback (note: I haven't read all the comments in the PR, only your documentation in the fork):
-
One of the
jester.resp
overloads overrides any headers that has been set by plugins. I think it would make more sense if this overload would keep the existing headers and just add the new ones on top. This is the offending overload -
From what I understand there is no way for a plugin to add new routes. My use case is a CORS plugin that adds some CORS related headers to preflight requests. As a workaround I can add a
options re""
route that just returnsHttp204 No Content
, but it would be nice if the plugin would "just work" without doing this. -
If I understand correctly, the only semantic difference between
specific:
andbefore:
is thatbefore:
will run for all request paths even if it's placed in a subrouter. The behavior ofbefore:
is confusing imo and doesn't seem useful (if I wanted a globalbefore:
I would put it in the top router). Could the behavior ofbefore:
(andafter:
as well) be changed so that it works as expected inside subrouters, meaning thatspecific:
would no longer be needed? -
Since plugins are "global" (if
plugin f <- pluginName()
is used in a subrouter thenf
is available in the main router, and vice versa) it might make sense to simple disallowplugin:
insidesubrouter:
to avoid confusion. -
Would be nice to support plugins that doesn't need to return anything, e.g
plugin pluginName()
.
@GULPF
Thanks for all the great feedback! This is very useful.
One of the jester.resp overloads overrides any headers that has been set by plugins. I think it would make more sense if this overload would keep the existing headers and just add the new ones on top. This is the offending overload
I will investigate.
From what I understand there is no way for a plugin to add new routes. My use case is a CORS plugin that adds some CORS related headers to preflight requests. As a workaround I can add a options re"" route that just returns Http204 No Content, but it would be nice if the plugin would "just work" without doing this.
I will look into this. I suspect this would be a very non-trivial change. But, if done well, could add quite a bit to the power of the plugins.
If I understand correctly, the only semantic difference between specific: and before: is that before: will run for all request paths even if it's placed in a subrouter. The behavior of before: is confusing imo and doesn't seem useful (if I wanted a global before: I would put it in the top router). Could the behavior of before: (and after: as well) be changed so that it works as expected inside subrouters, meaning that specific: would no longer be needed?
I'm also unsure of the purpose of the before
and after
directives. I've designed my PR so far so that their function does not change; but I can't think of too many reasons to use it. By design, they have their own variable scopes, so unless you are manipulating globals (a no-no in threads) or something specific in the existing thread vars, they don't seem to have any real impact.
I'll ask for feedback from @dom96 . It is possible they should be deprecated in light of plugins.
Since plugins are "global" (if plugin f <- pluginName() is used in a subrouter then f is available in the main router, and vice versa) it might make sense to simple disallow plugin: inside subrouter: to avoid confusion.
The plugins are not supposed to be global (to all the routers.) In fact, I use different plugins and plugin settings in different routers on my websites. But I might have a bug or I haven't thought it through enough. I'll investigate this also.
Would be nice to support plugins that doesn't need to return anything, e.g plugin pluginName().
I agree. I'm going to add support for that.
@JohnAD
The plugins are not supposed to be global (to all the routers.) In fact, I use different plugins and plugin settings in different routers on my websites. But I might have a bug or I haven't thought it through enough. I'll investigate this also.
I'm referring to this construct in one of your examples:
subrouter hutchRouter:
specific:
b.notFast()
get "/@name":
b = @"name" & " " & b
resp h1("Hello Inside " & b)
routes:
extend hutchRouter, "/hutch"
plugin b <- haveBunny()
get "/":
resp h1("Hello " & b)
get "/abc/@name":
b = @"name" & " " & b
resp h1("Hello " & b)
Even though b
is declared in routes
, it's available in hutchRouter
. The code still compiles if the specific
block is moved to routes
and the plugin
declaration is moved to hutchRouter
.
I'm now having some issues with how the plugins interact with Jesters error handling. Consider this example:
import jester, json
proc loggingPlugin*(request: Request, response: ResponseData): bool =
discard
proc loggingPlugin_after*(request: Request, response: ResponseData, _: bool) =
echo "Responded with status code " & $response.code &
" and body '" & response.content & "'"
routes:
plugin x <- loggingPlugin()
post "/json":
type Payload = object
name: string
let payload = request.body.parseJson.to(Payload)
resp "Hello " & payload.name, "text/plain"
error JsonParsingError:
resp Http400, "Invalid JSON", "text/plain"
error Http404:
resp Http404, "Nothing to see here", "text/plain"
- When requesting a route that doesn't exist, the plugins prints
Responded with status code 200 OK and body ''
, because it runs before jesters error handling. - When requesting the
/json
route with an invalid JSON payload the plugin doesn't print anything, because the exception interrupts the route logic.
This could be partially solved by adding another plugin hook that runs at the end of the error handler, but it doesn't really fix the first issue. I suppose the intended behavior for pluginName_after
is that it can inspect the final response, but since the response can be rewritten by the error handler that is not the case. This also means that a plugin cannot for example add a header to all responses, since the header won't be included for error responses.
I've moved the feedback from here to the new branch as issues at:
https://github.com/JohnAD/jesterwithplugins
This new library should make it's way to nimble.directory
fairly soon. I'll then place at least four of the plugins to take advantage of it.