Coder Social home page Coder Social logo

Comments (25)

devinivy avatar devinivy commented on June 1, 2024 1

Yes, pre-requisites are tied closer to the handler than onPreHandler. Additionally, their results are placed in request.pre, they can be configured to easily run in series and in parallel, and they accept special string options that integrate with server methods.

from hapi-contrib.

devinivy avatar devinivy commented on June 1, 2024

I really like this idea, but it seems very similar to route prerequisites. What are the main distinguishing features between policies and route prereqs? I realize that mrhorse also deals with response modification, which is useful. Do you liken this to express's idea of middleware?

from hapi-contrib.

mark-bradshaw avatar mark-bradshaw commented on June 1, 2024

It does have some similarity to route prereqs. There's some differences, though.

It seems that prereqs are primarily for pre-fetching and processing data. If you want to modify the response, your only option with a prereq is to takeover() and bypass the route handler. Mrhorse allows you to jump in after the route handler. It's like prereqs + postreqs.

Mrhorse makes the decision to continue into the next policy more of a conscious decision. With prereqs it is assumed that they will all run. If you do a takeover() in order to return back a premature 403 error or something, I'm not sure whether the rest of the prereqs wouldn't be executed anyway. I haven't looked at the prereq code so I don't know. Mrhorse asks you to decide whether to continue after each policy. This makes it much easier to use policies for authentication and authorization IMO. It's just more of a focus on the decision of whether to continue, and less on the data collected along the way.

However, with both mrhorse and prereqs, you can both pre-fetch data and short circuit the response with an error if you choose.

Since mrhorse also gives you the option to run a policy as a post handler, or even both pre and post if you want to get interesting, you can handle tasks like response modification and logging.

If hapi core were to add something like route post requirements, then there would definitely be less of a need for this sort of thing. Prereqs are really nice for handling certain types of tasks before routers get involved, but the focus is just different enough that it seems there's room for another take.

I haven't really thought of this in terms of middleware, though I can see the parallel. I like being able to compose on a route by route basis the ordering of components, both before and after the route handler. Prereqs does this in a more sophisticated way, but it's missing the post part.

Prereqs doesn't really give you any help in organizing the code, whereas mrhorse is opinionated about where it should go and loads it up for you. Prereqs tells you exactly where your pre-fetched data will go in the request object. Mrhorse is unopinionated about that.

Finally, I foresee that mrhorse can and probably will expand over time to allow policies to hook in at additional points in the request life cycle. There's no reason a policy couldn't be applied at any other hook point. This allows mrhorse to more quickly provide flexibility that core probably won't either be able to or want to do.

from hapi-contrib.

mark-bradshaw avatar mark-bradshaw commented on June 1, 2024

@hueniverse I just wanted to check back in on this, now that hapi 8 is out and things have (maybe) settled down.

from hapi-contrib.

hueniverse avatar hueniverse commented on June 1, 2024

I don't have a strong view. I don't really see the usefulness of this but I would like to see what other core contributors have to say on this.

from hapi-contrib.

mark-bradshaw avatar mark-bradshaw commented on June 1, 2024

Guess I need to work on my tldr. Here's a shorter pitch:

I want to be able to look at my route table and see immediately what sort of code is running before and after my route handler. Prereqs give me part of that, but not all.

MH is basically prerequisites + postrequisites. It attaches code (policies) to various routes in a configuration-centric way that's super easy to follow. One look at your routing file and you can see what is running where. It'd be easy to extend it to hook policies in at additional points in the request life cycle.

It overlaps with pre-requisites, but gives you a more specific way to setup your code. It'll probably make good sense to folks that are used to sails.js.

The big win at the moment with MH is post handlers. The second win is that all pre and post code is applied to a route in one location. You don't have to go hunt it down.

from hapi-contrib.

mark-bradshaw avatar mark-bradshaw commented on June 1, 2024

@arb @danielb2 @geek @nlf Eran asked for comments from core contributors on the suitability of accepting this plugin as a contrib module. Would any of you care to comment on this?

from hapi-contrib.

geek avatar geek commented on June 1, 2024

@mark-bradshaw looks good to me... I especially like the 100% code coverage and detailed documentation!

from hapi-contrib.

nlf avatar nlf commented on June 1, 2024

I agree that this module looks nice, but I also don't really see how useful it is personally. I'm pretty neutral about it, but not opposed since it does have very good test coverage and documentation.

If nothing else, you should definitely submit a pull request to https://github.com/hapijs/hapijs.com to list it there.

from hapi-contrib.

arb avatar arb commented on June 1, 2024

I agree with nlf in terms of general usefulness as a whole. Not sure what the benefit would be to move this under the hapijs umbrella. I would think only things that hapi uses or is generically useful would be in the hapijs organization.

You should definitely make sure to add it to hapijs.com though as this looks like a well put together plugin 👍

from hapi-contrib.

mark-bradshaw avatar mark-bradshaw commented on June 1, 2024

Thanks all. It seems that the general consensus, if you'll allow me, is that the plugin seems to be well put together, has 100% test coverage and reasonable documentation. It also seems that the consensus is that there is skepticism about the value of the plugin. A few people think it's a good idea. A few others are not interested. That's ok.

Given the response, I'm not sure if that's a strong enough to pull the package in or not. I still think this is a net win for the ecosystem, and provides some interesting functionality. I'd like to see it adopted.

However, if not, I'd like to see if @hueniverse is interested in adding a post-requirements functionality to core. If hapi supported postrequirements like it does prerequirements, then I'd have much less interest in maintaining this module. So, Eran, if you aren't interested in pulling MrHorse in as a contrib module, are you interested in postreqs in hapi? If you are, then I would be happy to do the work to implement them in the same pattern as prereqs. I just don't want to do the work if you aren't interested in pulling that in.

from hapi-contrib.

devinivy avatar devinivy commented on June 1, 2024

I would support having a more structured system to add post-handlers to particular routes.

from hapi-contrib.

hueniverse avatar hueniverse commented on June 1, 2024

I don't see the point in post requirements. You can just organize your pres to include the before, handler, and after and then in the handler just grab the last value and move it along.

from hapi-contrib.

devinivy avatar devinivy commented on June 1, 2024

That's a good point. I guess the distinction for me would be in keeping "true" to the request lifecycle. The method you describe means post-handlers aren't required to obtain an equivalent effect, but also it doesn't allow me to think of there being pre-requisites, a handler, then post-transformations in the lifecycle of a request/response. I have to make that abstraction in my head, then lump the whole process into pre-reqs, which isn't entirely true to the name pre-requirement and doesn't exactly take advantage of the entire request/response lifecycle. I would like to be able to think in that way, which is my main draw to MrHorse: pre-requirements to complete handler logic -> handler logic -> post-transformations of handler output.

from hapi-contrib.

mark-bradshaw avatar mark-bradshaw commented on June 1, 2024

@hueniverse, I agree (as you would expect) with @devinivy, on the usefulness of the post abstraction. Having a life cycle for requests is a good thing. I assume you agree with that since you put it in the framework. But hapi only allows the end user to easily capitalize on two parts of it (pre-requisites and route handler). Having posts fits in really well with the life cycle you already put in place.

As an example, in a project I'm working on now we had route controllers that were tacking on some analytics information to some of the responses from an api. The main data was cacheable. The analytics part was not. The handler was combining both pieces of data into the response. It was workable, but logically it was harder to reason about. Controllers were doing things that didn't really have anything to do with the data they were responding with, and you couldn't look at the route table and easily see where the analytics bits were getting added and where they were missing. Having post-reqs via mrhorse allowed us to extract those bits and apply them via configuration in a repeatable manner without muddying up the controllers. Our controllers got much smaller and cleaner, and we can see exactly which routes are getting the analytics modifications applied, and which aren't. Posts FTW.

I'd also suggest that your argument against posts, saying "just put everything in pres", works equally well against pres. Just do everything in the route handler and be done with it would be the equivalent, and we could just drop pre-requisites too. That'd be a net loss though. Pre-requisites make hapi better. Post-requisites would do the same.

from hapi-contrib.

hueniverse avatar hueniverse commented on June 1, 2024

I am not planning on adding this to hapi anytime soon. I suggest getting this plugin listed on the site and see how people use it. If it becomes a common pattern I will reconsider.

from hapi-contrib.

mark-bradshaw avatar mark-bradshaw commented on June 1, 2024

OK. I'll do that.

from hapi-contrib.

mark-bradshaw avatar mark-bradshaw commented on June 1, 2024

outmoded/hapijs.com#112

from hapi-contrib.

paulovieira avatar paulovieira commented on June 1, 2024

Speaking as a end-user, I think post-requisites would be a natural and useful addition to hapi.

This would be the right place to do stuff like this:

  • close database connections
  • apply Hoek.transform to format the object / array of objects of the response (delete some properties, move properties from nested objectes to the top object, etc)

Essentially, post-requisites would be the place where common post-processing utilities would be used.

The plugin fills this need partially. Having this functionality available directly in Hapi would be perfect.

from hapi-contrib.

devinivy avatar devinivy commented on June 1, 2024

For a more native hapi feel (specifically for post-requisites), I've pulled-together https://github.com/devinivy/loveboat-postreqs. This takes advantage of route-level request extensions, which is something mrhorse cannot currently do (because there is no programmatic API for adding route-level extensions).

from hapi-contrib.

einnjo avatar einnjo commented on June 1, 2024

I just want to share my use case in support of post-requisites.

We are storing "translatable" fields as a json object in out database:

// A payload ready to be inserted to a database.
{
    title: { "en": "My title" },
    description: { "en": "My description" }
}

We want to handle this transparently to the clients, this is what a client would send to the server:

POST /places 
Accept-Language: en
title=My title 
description=My description

We can tranform from client version -> database version with a pre-requisite like this:

server.route({
    method: 'POST',
    path: '/places',
    config: {
        pre: [
            // title: 'My title' -> title: { lang: 'My title' }
            { method: Lang.toTranslatable(['title', 'description']) }
        ],
        handler: function (request, reply) {

            return reply(Place.query().insertAndFetch(request.payload));
        }
    }
});

After the database inserts the record, we need to transform the data back into the client version.
I think this will work perfectly with post-requisites like this:

pre: [
    // title: 'My title' -> title: { lang: 'My title' }
    { method: Lang.toTranslatable(['title', 'description']) }
],
post: [
    // title: { lang: 'My title' } -> title: 'My title'.
    { method: Lang.fromTranslatable(['title', 'description']) }
]

I think this would be a very clean and maintainable way to share this functionality between all the routes that accept and respond with "translatable" fields.

from hapi-contrib.

nlf avatar nlf commented on June 1, 2024

your use case feels like what reply decorators are for. instead of using a post-requisite, create a reply decorator so you'd do something like reply.translate(Place.query().insertAndFetch(request.payload))

from hapi-contrib.

devinivy avatar devinivy commented on June 1, 2024

@juanjoLenero today you can also use the ext route config to add onPostHandler extensions per-route. This is codified into a module here, but in lieu of actually adopting loveboat (which is no small deal), you might look at the logic used internally to the module to make adding post-requisites simpler.

from hapi-contrib.

einnjo avatar einnjo commented on June 1, 2024

@nlf That works, but I think it would quickly become unmanageable reply.translateAndCleanSensitiveAndSomethingElse(...) whereas with the post-requisites option you would:

post: [
    { method: Lang.fromTranslatable(['title', 'description']) },
    { method: Security.cleanSensitive(['password', 'token']) },
    ...
]

@devinivy Thank you, I was evaluatingloveboat just now, I'll take your advise and look at the internals to see how per-route onPostHandlers work.

from hapi-contrib.

einnjo avatar einnjo commented on June 1, 2024

Just wanted to comment back to say that may use case is handled by route-level extensions like so:

ext: {
    onPostHandler: [
        { method: Lang.fromTranslatable(['title', 'description']) }
    ]
}

Can someone comment on why I would use pre-requisites intead of onPreHandler extension point like this:

ext: {
    onPreHandler: [
        { method: Lang.toTranslatable(['title', 'description']) }
    ],
    onPostHandler: [
        { method: Lang.fromTranslatable(['title', 'description']) }
    ]
}

from hapi-contrib.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.