fiber icon indicating copy to clipboard operation
fiber copied to clipboard

🚀 v3 Request: Merge `static` and `filesystem` middleware as `static` middleware

Open efectn opened this issue 2 years ago • 8 comments

Feature Description

I think we should merge static and filesystem to make Fiber core simple in v3. Also there's no performance problem when we compare fasthttp.FS static and io/fs filesystem middleware. (https://github.com/gofiber/fiber/pull/2027)

Additional Context (optional)

╭─efectn@tearch ~/v3 ‹v3-beta›
╰─$ wrk2 -t12 -c100 -d5s -R20000 http://127.0.0.1:3000/static
Running 5s test @ http://127.0.0.1:3000/static
  12 threads and 100 connections
  Thread Stats   Avg      Stdev     Max   +/- Stdev
    Latency     1.09ms    0.94ms  22.08ms   93.45%
    Req/Sec       -nan      -nan   0.00      0.00%
  99642 requests in 5.00s, 54.54MB read
Requests/sec:  19934.28
Transfer/sec:     10.91MB
╭─efectn@tearch ~/v3 ‹v3-beta›
╰─$ wrk2 -t12 -c100 -d5s -R20000 http://127.0.0.1:3000/filesystem
Running 5s test @ http://127.0.0.1:3000/filesystem
  12 threads and 100 connections
  Thread Stats   Avg      Stdev     Max   +/- Stdev
    Latency    17.42ms   33.75ms 264.19ms   88.56%
    Req/Sec       -nan      -nan   0.00      0.00%
  99428 requests in 5.00s, 53.29MB read
Requests/sec:  19892.46
Transfer/sec:     10.66MB
╭─efectn@tearch ~/v3 ‹v3-beta›
╰─$ wrk2 -t12 -c100 -d5s -R20000 http://127.0.0.1:3000/filesystem/test.txt
Running 5s test @ http://127.0.0.1:3000/filesystem/test.txt
  12 threads and 100 connections
  Thread Stats   Avg      Stdev     Max   +/- Stdev
    Latency     1.53ms    2.15ms  44.67ms   94.69%
    Req/Sec       -nan      -nan   0.00      0.00%
  99646 requests in 5.00s, 14.35MB read
Requests/sec:  19929.40
Transfer/sec:      2.87MB
╭─efectn@tearch ~/v3 ‹v3-beta›
╰─$ wrk2 -t12 -c100 -d5s -R20000 http://127.0.0.1:3000/static/test.txt
Running 5s test @ http://127.0.0.1:3000/static/test.txt
  12 threads and 100 connections
  Thread Stats   Avg      Stdev     Max   +/- Stdev
    Latency     1.10ms  837.88us  17.46ms   89.62%
    Req/Sec       -nan      -nan   0.00      0.00%
  99643 requests in 5.00s, 15.77MB read
Requests/sec:  19930.33
Transfer/sec:      3.16MB

Code Snippet (optional)

package  main

import  (
"log"
"os"

"github.com/gofiber/fiber/v3"
"github.com/gofiber/fiber/v3/middleware/filesystem"
)

func  main()  {
	app  := fiber.New()

	app.Use("/filesystem", filesystem.New(filesystem.Config{
		Root: os.DirFS("proxy"),
		Browse:  true,
	}))

	app.Static("/static",  "proxy", fiber.Static{
		Browse:  true,
	})
	log.Fatal(app.Listen(":3000"))
}

Checklist:

  • [X] I agree to follow Fiber's Code of Conduct.
  • [X] I have checked for existing issues that describe my suggestion prior to opening this one.
  • [X] I understand that improperly formatted feature requests may be closed without explanation.

efectn avatar Aug 19 '22 14:08 efectn

How to deal with embed.FS after merging?

leopku avatar Sep 19 '22 16:09 leopku

How to deal with embed.FS after merging?

embed.FS is compatible with fs.FS (io/fs) interfaces. So it can be used without extra effort. So it'll be easier to use fs interfaces with https://github.com/gofiber/fiber/pull/2027

Fasthttp FS is not well with fs interfaces but it may be improved in the future. https://github.com/valyala/fasthttp/pull/1374 Perhaps we can remove filesystem middleware in v3 cuz of there will no pros over fasthttp FS after this improvement.

efectn avatar Sep 19 '22 16:09 efectn

Got it. Thank you @efectn .

leopku avatar Sep 20 '22 03:09 leopku

@efectn @leopku

How to deal with after merging?embed.FS

embed.FS is compatible with fs.FS (io/fs) interfaces. So it can be used without extra effort. So it'll be easier to use fs interfaces with #2027

Fasthttp FS is not well with fs interfaces but it may be improved in the future. valyala/fasthttp#1374 Perhaps we can remove filesystem middleware in v3 cuz of there will no pros over fasthttp FS after this improvement.

how to suppport Compress option? use app.Static method, you can set Compress option.

app.Static("/", "web", fiber.Static{
	Compress:  true,     // <---- HERE
	ByteRange: true,
})

piyongcai avatar Oct 10 '22 09:10 piyongcai

@efectn @leopku

How to deal with after merging?embed.FS

embed.FS is compatible with fs.FS (io/fs) interfaces. So it can be used without extra effort. So it'll be easier to use fs interfaces with #2027 Fasthttp FS is not well with fs interfaces but it may be improved in the future. valyala/fasthttp#1374 Perhaps we can remove filesystem middleware in v3 cuz of there will no pros over fasthttp FS after this improvement.

how to suppport Compress option? use app.Static method, you can set Compress option.

app.Static("/", "web", fiber.Static{
	Compress:  true,     // <---- HERE
	ByteRange: true,
})

Probably we will remove filesystem middleware after fasthttp FS gets fs.FS support. (For v3) https://github.com/valyala/fasthttp/pull/1374

Don't have much knowledge about PR at the moment. Not reviewed yet

efectn avatar Oct 10 '22 09:10 efectn

The implementation of this patch is far off!

piyongcai avatar Feb 22 '23 07:02 piyongcai

Update, fs.FS support seems to be in fasthttp now. valyala/fasthttp#1640

nickajacks1 avatar Dec 01 '23 16:12 nickajacks1

Update, fs.FS support seems to be in fasthttp now. valyala/fasthttp#1640

Will work on it

efectn avatar Dec 02 '23 15:12 efectn