Adding support for "plugins" to Jester

[JohnAD]

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?

[JohnAD]

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.

[JohnAD]

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 my mongopool 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.)

[JohnAD]

I've created PR #227 for tracking progress.

[lantos1618]

is this similar to the express request middleware?
https://expressjs.com/en/guide/using-middleware.html

This is nice to have

[JohnAD]

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.

[dom96]

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.

[JohnAD]

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.)

[JohnAD]

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.

[JohnAD]

ready for review ...mostly. See the last two notes in the PR.
@dom96

[JohnAD]

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.

[JohnAD]

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.

[JohnAD]

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.)

[JohnAD]

@dom96 and everyone else:

Should I do a talk about jester plugins at the up-and-coming Nim conference?

[Araq]

Sure, why not.

[GULPF]

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 returns Http204 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: 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?

• 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.

• Would be nice to support plugins that doesn't need to return anything, e.g plugin pluginName().

[JohnAD]

@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.

[GULPF]

@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.

[JohnAD]

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.