luckyframework / lucky

A full-featured Crystal web framework that catches bugs for you, runs incredibly fast, and helps you write code that lasts.
https://luckyframework.org
MIT License
2.57k stars 156 forks source link

The RequestBodyReader could use a refactor #1825

Closed jwoertink closed 10 months ago

jwoertink commented 10 months ago

This method is called to parse the request body so we can create our params object for the actions. The current implementation potentially creates 2 IO instances on each request, and the way the method reads is a little confusing. On top of that, it's not documented why it works like this....

https://github.com/luckyframework/lucky/blob/2e929598148caa4c7ee8bca143b02aab84c27e32/src/lucky/request_body_reader.cr#L8-L12

It seems the idea may be that instead of having to manually rewind the body just so you can call params more than once in an action, it's setting the request body back and ensuring that it's an IO::Memory and not some other IO object.

This might be a cleaner solution:

contents = request.body.try(&.gets_to_end) || ""
request.body = IO::Memory.new(contents)
contents

And then just document what it's doing.