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

vweb: middleware implementation #17730

Merged
merged 5 commits into from Mar 25, 2023
Merged

Conversation

Casper64
Copy link
Member

@Casper64 Casper64 commented Mar 21, 2023

Implementation of middleware for vweb, inspired by the syntax of discussion #15187 and code in pull request #12961.

Why

Was playing around with vweb and couldn't find a solution for per route middleware. Currently any middleware is only added before every request and it becomes difficult if you only want middleware for specific routes. It doesn't give much freedom or extensability with other (future) modules.

Usage

Middleware functions can be passed directly when creating an App instance.

struct App {
	vweb.Context
	middlewares map[string][]vweb.Middleware
}


fn new_app() &App {
	mut app := &App{
		middlewares: {
			// chaining is allowed, middleware will be evaluated in order
			'/path/to/': [middleware_func, other_func]        
			'/': [global_middleware]
		}
	}
	// do stuff with app
	// ...
	return app
}

Middleware functions will be of type vweb.Middleware and are not methods of App, so they could also be imported from other modules.

pub type Middleware = fn (mut Context) bool

Middleware can also be added to route specific functions via attributes e.g.:

[middleware: check_auth]
['/admin/data']
pub fn (mut app App) admin() vweb.Result {
	// ...
}

// check_auth is a method of App, so we don't need to pass the context as parameter.
pub fn (mut app App) check_auth () bool {
	// ...
	return true
}

For now you can only add 1 middleware to a route specific function. But this could be changed easily if the attributes syntax changes ( see discussion #17715 )

Example

module main

import vweb

const (
	http_port = 8080
)

struct App {
	vweb.Context
	middlewares map[string][]vweb.Middleware
mut:
	is_authenticated bool
}

fn main() {
	mut app := new_app()
	vweb.run(app, http_port)
}

fn new_app() &App {
	mut app := &App{
		middlewares: {
			'/admin/': [other_func1, other_func2]
			'/early':  [middleware_early]
		}
	}
	// do stuff with app
	// ...
	return app
}

['/']
pub fn (mut app App) index() vweb.Result {
	println('Index page')
	title := 'Home Page'

	content := $tmpl('../templates/index.html')
	base := $tmpl('../templates/base.html')
	return app.html(base)
}

[middleware: check_auth]
['/admin/secrets']
pub fn (mut app App) secrets() vweb.Result {
	println('Secrets page')
	title := 'Secret Admin Page'

	content := $tmpl('../templates/secret.html')
	base := $tmpl('../templates/base.html')
	return app.html(base)
}

['/admin/:sub']
pub fn (mut app App) dynamic(sub string) vweb.Result {
	println('Dynamic page')
	title := 'Secret dynamic'

	content := sub
	base := $tmpl('../templates/base.html')
	return app.html(base)
}

['/early']
pub fn (mut app App) early() vweb.Result {
	println('Early page')
	title := 'Early Exit'

	content := $tmpl('../templates/early.html')
	base := $tmpl('../templates/base.html')
	return app.html(base)
}

pub fn (mut app App) before_request() {
	app.is_authenticated = false
	println('0')
}

pub fn (mut app App) check_auth() bool {
	println('3')
	if app.is_authenticated == false {
		app.redirect('/')
	}
	return app.is_authenticated
}

fn other_func1(mut ctx vweb.Context) bool {
	println('1')
	return true
}

fn other_func2(mut ctx vweb.Context) bool {
	println('2')
	// ...
	return true
}

fn middleware_early(mut ctx vweb.Context) bool {
	println('4')
	ctx.text(':(')
	return false
}

v_middleware_test

Explanation

Context.before_request is always executed first before any other middleware.

When visiting "/admin/secrets" the path starts with "/admin/" so after Context.before_request other_func1 and other_func2 are executed. The function itself also has the attribute [middleware: check_auth] so App.check_auth is executed.
If any middleware returns false the propogation is stopped.

In this example we can see that everything until App.check_auth returns true, so we expect that 0, 1, 2, 3 are printed, but not "Secrets page".
Indeed the method App.secrets is not executed. But we can see an html page, because App.check_auth calls App.redirect before returning false, thus sending an http response to the client.

middleware_early returns false, so we expect to see a ":(" instead of the html page.

Middleware also works for dynamic routes.

Drawbacks (?)

For each request the middlewares map is cloned to each new App, like db. Feels a bit hacky.

Limited comptime error checking(?)

Cool

It was really fun to dive deeper into the workings of V! I've learned a lot about the language.

@JalonSolov
Copy link
Contributor

If you want something to be shared across all sessions, mark it with the [vweb_global] attribute. You can check the gitly source to see an example.

@Casper64
Copy link
Member Author

I've added some tests. And fixed post requests.

@Casper64
Copy link
Member Author

I can see that my new test file is failing in the CI and that the other vweb tests are skipped in v test-self. I can only test on my on pc and they pass aleast. Should my new vweb test also be skipped? Any idea how to fix this?

@medvednikov
Copy link
Member

It only fails on windows, not sure what the issue is

Failed command 1:    "D:\a\v\v\v.exe"   -o "C:\Users\runneradmin\AppData\Local\Temp\v_0\tsession_420_169548300\middleware_test.exe" "D:\a\v\v\vlib\vweb\tests\middleware_test.v"

the error is not printed

@medvednikov
Copy link
Member

Great job by the way!

This is a great addition.

@enghitalo
Copy link
Contributor

Good work!!
Can you also, please, update the markdown file? https://github.com/vlang/v/tree/master/vlib/vweb#middleware

@Casper64
Copy link
Member Author

It turns out windows runs a weird resolution algorithm when using localhost as host in a TCP connection. Changing this to 127.0.0.1 reduces the time for 1 request from about 5 seconds to a couple of milliseconds that's why the tests kept failing on windows.

@medvednikov
Copy link
Member

./vlib/vweb/README.md:308:134: error: must be less than 100 characters
Middleware functions will be of type `vweb.Middleware` and are not methods of App, so they could also be imported from other modules.
/tmp/v_1001/v/vcheck_1001/check_README_md_example_260__264__01GWCZK1PBP1D6NGJ3T1RXCN[9](https://github.com/vlang/v/actions/runs/4520495176/jobs/7961642400?pr=17730#step:4:10)V.v:1:17: error: unknown type `App`
    1 | pub fn (mut app App) before_request() {
      |                 ~~~
    2 |     app.user_id = app.get_cookie('id') or { '0' }
    3 | }
./vlib/vweb/README.md:261:1: error: example failed to compile
./vlib/vweb/README.md:261:1: error: example is not formatted
pub fn (mut app App) before_request() {
    app.user_id = app.get_cookie('id') or { '0' }
}

you can use

v oksyntax

for such code blocks

@medvednikov
Copy link
Member

You successfully got deep into vweb, good job! :)

Perhaps when you have time you could help me out with a similar feature I've been trying to do for a while.

Something similar to "controllers" in MVC. Right now we can only have one App struct per /.

Would be nice to have Admin{} for /admin, Foo{} for /foo etc. What do you think?

@Casper64
Copy link
Member Author

Casper64 commented Mar 25, 2023

Sure sounds interesting! I think it would be a good addition to make vweb more powerful. You could for example generate an admin page (controller) to visually interact with your models/data. Will think about it.

@medvednikov medvednikov merged commit 1fe5aca into vlang:master Mar 25, 2023
36 checks passed
@xiusin
Copy link
Contributor

xiusin commented Mar 26, 2023

@Casper64 @medvednikov I have implemented a similar function in a web framework, currently using the net/http/server module, but it is not yet capable of concurrency. the repository url is https://github.com/xiusin/very

@Casper64 Casper64 deleted the vweb-middleware branch March 26, 2023 11:31
l1mey112 pushed a commit to l1mey112/v that referenced this pull request Jun 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants