cmorten / opine

Minimalist web framework for Deno ported from ExpressJS.
https://github.com/cmorten/opine/blob/main/.github/API/api.md
MIT License
854 stars 43 forks source link

[#158] [#159] implement formData parser #161

Closed fucksophie closed 2 years ago

fucksophie commented 2 years ago

Issue

Supersedes #160, fixes #158 and #159

Details

This is a alternative implementation of #160 which works as a Middleware instead of a property.

Checklist.

[?] Handle compression [x] Write tests [x] Handle proper charsets (not sure if umlauts properly parse 🤷🏽

github-actions[bot] commented 2 years ago

Benchmark results

PR to merge main 4d1eed403095b7b0e77e77cfde8273c769faf537 -> main ```console opine: 1 middleware ================================ 1 middleware Running 3s test @ http://localhost:3333/?foo[bar]=baz 2 threads and 10 connections Thread Stats Avg Stdev Max +/- Stdev Latency 2.11ms 542.11us 15.80ms 89.35% Req/Sec 2.38k 309.70 2.70k 85.94% Latency Distribution 50% 1.96ms 75% 2.18ms 90% 2.60ms 99% 3.75ms 15189 requests in 3.21s, 3.07MB read Requests/sec: 4738.26 Transfer/sec: 0.96MB opine: 10 middleware ================================ 10 middleware Running 3s test @ http://localhost:3333/?foo[bar]=baz 2 threads and 10 connections Thread Stats Avg Stdev Max +/- Stdev Latency 2.10ms 564.27us 16.07ms 90.41% Req/Sec 2.39k 303.42 2.68k 87.10% Latency Distribution 50% 1.98ms 75% 2.13ms 90% 2.56ms 99% 3.87ms 14753 requests in 3.11s, 2.98MB read Requests/sec: 4750.11 Transfer/sec: 0.96MB opine: 50 middleware ================================ 50 middleware Running 3s test @ http://localhost:3333/?foo[bar]=baz 2 threads and 10 connections Thread Stats Avg Stdev Max +/- Stdev Latency 2.22ms 553.27us 16.79ms 91.27% Req/Sec 2.26k 270.74 2.51k 83.33% Latency Distribution 50% 2.09ms 75% 2.27ms 90% 2.65ms 99% 3.98ms 13524 requests in 3.00s, 2.73MB read Requests/sec: 4502.46 Transfer/sec: 0.91MB std/http benchmark ================================ Listening on http://localhost:3333/ Running 3s test @ http://localhost:3333/?foo[bar]=baz 2 threads and 10 connections Thread Stats Avg Stdev Max +/- Stdev Latency 597.95us 101.46us 3.91ms 88.43% Req/Sec 8.18k 419.86 8.67k 87.10% Latency Distribution 50% 583.00us 75% 617.00us 90% 664.00us 99% 1.00ms 50456 requests in 3.10s, 5.29MB read Requests/sec: 16275.03 Transfer/sec: 1.71MB deno_http_native benchmark ================================ Server listening on localhost:3333 Running 3s test @ http://localhost:3333/?foo[bar]=baz 2 threads and 10 connections Thread Stats Avg Stdev Max +/- Stdev Latency 588.90us 155.15us 4.62ms 92.97% Req/Sec 8.34k 713.40 8.97k 90.32% Latency Distribution 50% 565.00us 75% 602.00us 90% 674.00us 99% 1.02ms 51438 requests in 3.10s, 5.40MB read Requests/sec: 16594.89 Transfer/sec: 1.74MB ```
fucksophie commented 2 years ago

@cmorten implemented basic tests and got rid of inflate/charset since they .. don't seem to be needed? I could send compressed data straight to it and it worked

fucksophie commented 2 years ago

all discussions closed, seems ready to be merged and version to be upped

fucksophie commented 2 years ago

found quite lot of bugs with previous version. fixed now!

fucksophie commented 2 years ago

yay! ci fixed (probably issues with skypack..)

fucksophie commented 2 years ago

replying to all of the comments at once,

router.post(
  "/uploadImage",
   formData(),
  async (req, res) => {
    const form = (req.body as FormData);
       if (
      form.has("image")
    ) {
      const uploaded = await uploadToMinio(
        "image",
        form.get("image") as File,

this is the pattern I'm using at the moment, maybe we should change it so it's better-typed? not sure how to do that but uh, quite a lot of casting here lol

Feelzor commented 2 years ago

Wouldn't #160 be more relevant than this PR since MultipartReader is deprecated and soon to be removed?

fucksophie commented 2 years ago

Yeah, rather work on #160. @cmorten ?

cmorten commented 2 years ago

Shutting support for this repo down. Folks welcome to fork and continue developing elsewhere.