hemerajs / hemera

🔬 Writing reliable & fault-tolerant microservices in Node.js https://hemerajs.github.io/hemera/
MIT License
806 stars 70 forks source link

Add middlewares [Investigation] #23

Closed vitorcamachoo closed 7 years ago

vitorcamachoo commented 7 years ago

Hii, is there any way to use cache for certains hemera services.

I'm looking for something that I've used in Express, a middleware function that caches end points.

Thanks!

StarpTech commented 7 years ago

Hi @vitorcamachoo caching will not be covered by hemera core. You can create a plugin which exposed a caching interface (Redis, memory ...). PR Welcome :)

hemera.add({ topic: 'globalCache', cmd: 'get', key: 'fooBar' }, ....

hemera.add({ topic: 'math', cmd: 'add' }, (req, cb) => {

 hemera.act({ topic: 'globalCache', cmd: 'get', key: 'foobar' }, (err, result) => {

     if(err) {
       return cb(err)
     }
     if(result) {
       return cb(null, result)
     }

     var r = req.a + req.b
     hemera.act({ topic: 'globalCache', cmd: 'set', key: 'foobar', value: r })
     cb(null, r)
  })

})

Here is a simple example how to do it actually.

hemera.add({ topic: 'math', cmd: 'add' }, function (req, cb) {

    var key = req.a + '_' + req.b

    //memory, redis ...
    if(cache[key]) {
       return cb(null, cache[key])
    }

    cb(null, req.a + req.b);
  });
StarpTech commented 7 years ago

I close it. Feel free to reopen. Thanks.

vitorcamachoo commented 7 years ago

Hiii,

I've been thinking about that and I'd love to have the same mechanism of ExpressJs, the possibility to have middlewares, and having that, a more generic cache could be implemented, example:

import cache from 'hemera-cache';

hemera.add({topic: 'databases', cmd: 'list'}, cache, listDatabases);

cache pseudo code:

function cache(req, res, next) {
   if(cache) res(cache) 
   else {
      save(key, response) 
      next(); 
   }
}

I could try this implementation, but I image that I will change a lot of hemera core. I'de like to know your opinion as well.

StarpTech commented 7 years ago

Hi @vitorcamachoo the core should be small as possible. The design principle of hemera is to hide complexity and do only one job right. Functionality can be extended by plugins. Caching should not be land in the core.

You can write a caching wrapper on top of hemera to reach your goal. Hemera has extension there you can hook into.

Everything is an action or an implementation that's the unit of Hemera. In your example it would mean that hemera is responsible for caching (perhaps with redis...) and processing of requests that would break single responsibility. I hope you get it. Thank you!

vitorcamachoo commented 7 years ago

Hii, maybe I didn't explain myself clearly.. What I suggested, it's not only related with cache, is beyond that. I will try explain in a different way.

Imagine hemera have the possibility to have middlewares, like expressjs have, and with that, we can define whatever we want. The add method could be converted into this: hemera.add(options, ...fn) ...fn could be multiple functions like you can register in express, and in that fn, it could be like that: function(req, reply, next) {} Where next, would pass to the next registered function (fn) in hemera.add.

This approach can open hemera to other plugins, like cache, limiter, restrict route, etc etc... Hemera continues to do his job well, and his not part of his job to make things like cache as you said. Its the behaviour you introduce in one of these middlewares.

But as I said, it's my opinion, and if it's to implement, much of the core would be changed...

Thank you.

StarpTech commented 7 years ago

Hi I don't want to change the interface of add. Is your goal to define different "middlewares" per add ? Whats the difference between extension?

  hemera.ext('onClientPreRequest', function (next) {

    let self = this // hemera context
    next() //accept null or error object

  })
vitorcamachoo commented 7 years ago

Of what I know, first you can't answer a request through that event, and second, that's to much generic, if I want to add a specific function behaviour as a middleware for a specific route, I can't.

Correct me if I'm wrong.

StarpTech commented 7 years ago

I agree that could be useful I am thinking about I will inform you.

vitorcamachoo commented 7 years ago

That could be a possibility, in that way, you avoid to change add method. But, wasn't it better to apply that middleware to add method and apply a reduce to all fnregistered ? I think both solutions are good, I'm just putting all the cards in the table hehe

StarpTech commented 7 years ago

@vitorcamachoo its now possible to reply from the extension points onServerPreRequest onServerPreHandler. This was the best way without changing the core a lot.

Reply an error and finish the request i s the index of the handler

      hemera.ext('onServerPreHandler', function (next, i) {
        let self = this
        next(new Error('fail'))
      })

Reply a message and finish the request

      hemera.ext('onServerPreHandler', function (next, i) {
        let self = this
        next(null, { msg: 'unauthorized' })
      })

Reply a message and continue with the next extension handler

      hemera.ext('onServerPreRequest', function (next, i) {
        let self = this
        next(null, { msg: 'unauthorized' }, true)
      })

which informations are available you can take from the default extensions. https://github.com/hemerajs/hemera/blob/master/packages/hemera/lib/extensions.js

I think this will covering the most use cases. If you want to do preprocessing for add specific things you can do it in the add self.

vitorcamachoo commented 7 years ago

Hii, thanks for the changes. I've some questions about that approach.

Calling next will always end the request cycle? ie, it will never reach to the requested topic? Since next end request cycle, I imagine that I can't broadcast data to the add event, ie, if I want to format some received data before reach the add handler. What I trying to say is, I'de like to have the same behaviour that ExpressJs have on app.use() method. I thinks this library will gain a lot with that approach.

Maybe I can create a branch to implement that and create a real complete use case for what I expect.

Thanks a lot.

StarpTech commented 7 years ago

No calling next without passing an error object will call the next handler in the stack.

StarpTech commented 7 years ago

You can manipulate the request with that approach.

 hemera.ext('onServerPreRequest', function (next, i) {
      let req = this._request
      req.foo = "bar"
      next()
})

Your approach would implicit to create seperate objects for request and response. This would change the whole core but of course you can prepare something. But it must bring a lot improvements in contract to the current solution before I will update all plugins :)

StarpTech commented 7 years ago

@vitorcamachoo so finally :)

hemera.ext('<serverExtension>', function (req, res, next, prevValue, i) {

   next()
})
vitorcamachoo commented 7 years ago

Cool, nice work @StarpTech. Maybe tomorrow I will try that feature.

StarpTech commented 7 years ago

@vitorcamachoo thanks for the PR but I will close it because I address all problems. Give me feedback thanks.

vitorcamachoo commented 7 years ago

I change the core a little to accept what I was trying to say in old posts. You can check that in my repository. I have a little test simulating the use of a 'external lib' with that middleware, in this case, to cache response.

It isn't finished, due to the lack of time, but I would like receive your opinion.

StarpTech commented 7 years ago

Where can I find this? Can you explain exactly what you want to solve?

vitorcamachoo commented 7 years ago

As I said in old posts, the chance to do something like this hemera.add(options, ...fn)

I've made a simple change on add method to be able to received n handlers and web can define multiple middlewares before reach the last handler. This can be useful to manipulate data, cache data, etc etc. It's the same behavior that express have.

You can find my changes here: https://github.com/vitorcamachoo/hemera

Thanks.

StarpTech commented 7 years ago

HI @vitorcamachoo I like the idea that you can define middlewares per add but you forgot to update your branch. You feature cannot be merged. I did a lot improvements.

I also prefer a chaining syntax instead parameter overloading.

hemera.add({
  topic: 'test',
  cmd: 'add'
}).use(function(req, next) {
//process request
})
.end(function(req, cb) {
   cb()
})
StarpTech commented 7 years ago

I create a new issue https://github.com/hemerajs/hemera/issues/40

vitorcamachoo commented 7 years ago

Nice work, thanks!