Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature Request: Middleware #1

Open
rgvy opened this issue Feb 27, 2017 · 7 comments
Open

Feature Request: Middleware #1

rgvy opened this issue Feb 27, 2017 · 7 comments

Comments

@rgvy
Copy link

rgvy commented Feb 27, 2017

Hello

This framework is really great and I am very glad to be using it.

However, one feature that I think might be missing is middleware. I have managed to implement a very simple solution, but it is kind of sloppy. Here is an example of my implementation:

namespace Example\Middleware;

abstract class RouteLayer {
	function before($app, $context) { }
	function after($app, $context) { }
}

abstract class RouteHandler {

	private $layers;

	function __construct($layers = array()) {
		$this->layers = $layers;
	}
	
	function execute($app, ...$routeParams) {
		$context = array(); //holds any data used by the layers, route handler implementation, or twig templates
		$context['routeParams'] = $routeParams;

		//loop through the layer pipe forwards handling entrance methods
		for ($i = 0; $i < count($this->layers); $i++) {
			$newContext = $this->layers[$i]->before($app, $context);
			$context = isset($newContext) ? $newContext : $context; //just in case a modified context isn't returned by implementors
		}
		
		//handle the main route code
		$newContext = $this->handle($app, $context);
		$context = isset($newContext) ? $newContext : $context; //just in case a modified context isn't returned by implementors
		
		//loop through the layer pipe backwards handling exit methods
		for ($i = count($this->layers) - 1; $i >= 0 ; $i--) {
			$newContext = $this->layers[$i]->after($app, $context);
			$context = isset($newContext) ? $newContext : $context; //just in case a modified context isn't returned by implementors
		}
	}
	
	abstract protected function handle($app, $context);
}

class ExampleRoute extends RouteHandler {

	function __construct() {
		 //example layers not yet written
		$layers = [
			new \Example\Middleware\LoggingLayer(),
			new \Example\Middleware\AuthenticationLayer(), //redirects to login if unauthenticated
			new \Example\Middleware\AuthorizationLayer(), //checks if user has necessary privileges for the route's action
			new \Example\Middleware\ThrottleLayer(), //to be used on routes which write to db
		];
		parent::__construct($layers);
	}

	function handle($app, $context) {
		echo $app->view('example.html', $context); //context has everything needed for twig template
	}
}


$app->get('/example/:id/:verb', function($app, $id, $verb) {  //uses a closure to cause lazy class autoloading of route handlers/layers
	(new \Example\ExampleRoute())->execute($app, $id, $verb);
});

It may not be pretty but it works. I was wondering, do you think such features have a place in this framework? Or perhaps you prefer another solution or library? It seems like relayphp isn't appropriate for use with this framework, and the other middleware libs are also tailored for other frameworks...

@ocram
Copy link
Contributor

ocram commented Mar 1, 2017

Thanks for this feature request!

Good question, not sure yet whether middleware has a place in this project or not. As you may have seen, this project is intended to be super lightweight and simple, positioning it somewhere between plain PHP and complex frameworks.

But middleware may really be important to many people. Great that you have found a way to add middleware yourself with minimal code and a modular approach. That approach doesn't look that bad!

Are these other middleware libraries incompatible with this project because we don't have HTTP message interfaces as defined in PSR-7 here? Might be another idea then to add those message interfaces, at least optionally, so that middleware libraries work here. What do you think?

@rgvy
Copy link
Author

rgvy commented Mar 2, 2017

Thanks for considering this.

I agree that middleware is not necessary for many projects, and may add much complexity. Perhaps it fits into an optional module (e.g. "PHP-Middleware").

The popular middleware libs I investigated did seem to use PSR-7 interfaces; they are structured around passing through HTTP requests and responses. However, I fail to see an elegant place to share data between layers. Maybe I'm missing some key understanding of that architecture.

One advantage of implementing PSR-7 is being able to reuse prewritten middleware. Some examples:
https://github.com/oscarotero/psr7-middlewares
https://github.com/relayphp/Relay.Middleware

@rgvy
Copy link
Author

rgvy commented Mar 2, 2017

(however, I now see that these prewritten middleware require significant dependencies)

@ocram
Copy link
Contributor

ocram commented Mar 3, 2017

I now see that these prewritten middleware require significant dependencies

Do they? As for "Relay", this repository seems to contain the actual core, while the other one seems to just contain some exemplary pieces of middleware, right?

Again, this other repository is just a collection of exemplary pieces that can be used.

For the three repositories mentioned so far, what we'd need, judging from their composer.json files, is just the PSR-7 interfaces and an implementation of those interfaces that we either write ourselves or get somewhere else.

Doesn't look too bad, does it?

I agree that middleware is not necessary for many projects, and may add much complexity. Perhaps it fits into an optional module

Yes, optional would be perfect. Because right now, almost everything in this project is optional. It basically just bootstraps several components, options and directives for you but then does not force any particular coding style on you, which means you're free to choose one yourself.

However, I fail to see an elegant place to share data between layers. Maybe I'm missing some key understanding of that architecture.

The documentation for "Relay" is here. It does look similar to your implementation. Might be worth finding out about the major differences between your quick implementation and these dedicated libraries then.

One advantage of implementing PSR-7 is being able to reuse prewritten middleware

Definitely a huge advantage!

@rgvy
Copy link
Author

rgvy commented Mar 3, 2017

Again, this other repository is just a collection of exemplary pieces that can be used.

Maybe I'm not understanding composer.json properly, but their "suggested" dependencies are actually required for various middleware implementations. The implemation .php files import classes from those suggested (not required) dependencies. So there is still some dependency wrangling for these prewritten middlewares.

However, I fail to see an elegant place to share data between layers. Maybe I'm missing some key understanding of that architecture.

I should clarify: the PSR-7 request/response interfaces only provide http header/body information which doesn't seem extremely useful to pipe through a series of middleware layers. However, since they are merely interfaces, anything else can be added to their instances (including the global $app object). However, attaching $app or other contextual data feels like a violation of object oriented principles hence this is why I don't consider it "elegant".

Might be worth finding out about the major differences between your quick implementation and these dedicated libraries then.

It seems like the biggest difference is what data gets passed to the handlers (as mentioned above). Their choice of a queue vs my choice of a stack is trivial.

@ocram
Copy link
Contributor

ocram commented Mar 3, 2017

Maybe I'm not understanding composer.json properly, but their "suggested" dependencies are actually required for various middleware implementations. The implemation .php files import classes from those suggested (not required) dependencies. So there is still some dependency wrangling for these prewritten middlewares.

100% correct! It's generally up to them how they use the "suggested" dependencies exactly, but they're usually required for some parts, at least.

The core of "Relay" has no such requirements, however. It's just the pieces of middleware that you may re-use. So you have to import some dependencies for some of them, which makes sense, depending on what they do.

the PSR-7 request/response interfaces only provide http header/body information which doesn't seem extremely useful to pipe through a series of middleware layers. However, since they are merely interfaces, anything else can be added to their instances (including the global $app object).

Right. You may add something to your response's body or headers in some layers via the implementations of the PSR-7 interfaces, though. Apart from that, we could certainly add the $app object as well. Don't think that would be too bad, but every middleware making use of that object would be tied to it and not freely re-usable.

And that's what you hoped to get from the existing middleware as well: compatibility. So it's best if everyone just uses the PSR-7 interfaces. If that's possible. Everything on top of that just prevents easy re-use between implementations.

@rgvy
Copy link
Author

rgvy commented Mar 3, 2017

Good point about reuse.

I suppose simple HTTP header/body data would be suitable for many middleware implementations. In terms of OOP, each layer could take what it needs from $app in its constructor.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants