fiber icon indicating copy to clipboard operation
fiber copied to clipboard

🚀 [Feature]: Implement Generic APIs for Enhanced Variable Type

Open dozheiny opened this issue 1 year ago • 21 comments

Feature Description

Some APIs like ctx.QueryBool or ctx.QueryFloat convert given variables. Why don't add APIs that let users convert the needed variable types?

For example, Query convertor API can be implemented like this (Since we can't use generics in methods):

func QueryFunc[T any](c *Ctx, key string, convertor func(string)(T, error), defaultValue ...T) (T, error)

Usage:

package main

import (
    "github.com/gofiber/fiber/v2"
    "github.com/google/uuid"
    "log"
)

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

	// [GET] /?id=01827964-1320-47b4-b2fa-c67fa9c39bed
	app.Get("/", func(c *fiber.Ctx) error {

		id, err := fiber.QueryFunc(c, "id", uuid.Parse)
       		if err != nil {
			log.Fatal(err)
			}

       		// some functionality
    })

    app.Listen(":3000")
}

We can implement other APIs like ctx.Params and ctx.Get functions.

Additional Context (optional)

No response

Code Snippet (optional)

No response

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.

dozheiny avatar Dec 11 '23 14:12 dozheiny

Thanks for opening your first issue here! 🎉 Be sure to follow the issue template! If you need help or want to chat with us, join us on Discord https://gofiber.io/discord

welcome[bot] avatar Dec 11 '23 14:12 welcome[bot]

We could do it for v3 not for v2

ReneWerner87 avatar Dec 11 '23 15:12 ReneWerner87

@ReneWerner87 I would like to work on this issue. Can I work on it? In case so, which branch should I use to fork?

brunodmartins avatar Dec 23 '23 01:12 brunodmartins

@brunodmartins v3-beta is the latest v3 development branch

nickajacks1 avatar Dec 23 '23 01:12 nickajacks1

Reopened for Param and Get function @brunodmartins Can you abstract these functionalities and also the queryType and the sub methods so that we have the same for the other 2 topic methods

ReneWerner87 avatar Jan 19 '24 16:01 ReneWerner87

@ReneWerner87 I would like to do this task. If it's possible

dozheiny avatar Jan 19 '24 18:01 dozheiny

Ok thx, every help is appreciated

ReneWerner87 avatar Jan 19 '24 18:01 ReneWerner87

Thanks @ReneWerner87 =) @dozheiny it's yours =)

brunodmartins avatar Jan 20 '24 00:01 brunodmartins

@dozheiny How is the progress ?

ReneWerner87 avatar Feb 05 '24 14:02 ReneWerner87

Sorry, I was too busy to do this task. I'll start today.

dozheiny avatar Feb 06 '24 08:02 dozheiny

please try to abstract the existing functionality so that it can also be used for the other functions without creating code duplicates https://github.com/gofiber/fiber/blob/main/ctx.go#L1118-L1160

ReneWerner87 avatar Feb 08 '24 12:02 ReneWerner87

please try to abstract the existing functionality so that it can also be used for the other functions

without creating code duplicates

https://github.com/gofiber/fiber/blob/main/ctx.go#L1118-L1160

@ReneWerner87 What do you think if QueryType is changed to GenericValue which suits its purpose so that in the future it can be used for other needs?

ryanbekhen avatar Feb 08 '24 13:02 ryanbekhen

you have this generic function and use it in a QueryType function, which then only fetches the value from the fasthttp resource and passes it into the following function

so the QueryType is small and only contains the part for fetching the data + the new generic function and you can do the same for Params and Get

only the source of the data changes, the following processing should be the same

ReneWerner87 avatar Feb 08 '24 13:02 ReneWerner87

you have this generic function and use it in a QueryType function, which then only fetches the value from the fasthttp resource and passes it into the following function

so the QueryType is small and only contains the part for fetching the data + the new generic function

and you can do the same for Params and Get

only the source of the data changes, the following processing should be the same

Oh I see. Thank you for the valuable suggestion.

ryanbekhen avatar Feb 08 '24 14:02 ryanbekhen

@ReneWerner87 What do you think about allowing users to pass their parser function in arguments? For example, in the Params function it's like this:

func Params[T any](c Ctx, key string, convertor func(string) (T, error), defaultValue ...T) (*T, error) {
	value, err := convertor(c.Params(key))
	if err != nil {
		if len(defaultValue) > 0 {
			return &defaultValue[0], nil
		}

		return nil, fmt.Errorf("failed to convert: %w", err)
	}

	return &value, nil
}

It helps users to parse their data type directly, like objectId in Mongo database or UUID.

dozheiny avatar Feb 08 '24 17:02 dozheiny

That was my proposed solution on https://github.com/gofiber/fiber/pull/2777. I think that could be good for the users @dozheiny and @ReneWerner87

brunodmartins avatar Feb 08 '24 18:02 brunodmartins

That was my proposed solution on https://github.com/gofiber/fiber/pull/2777. I think that could be good for the users @dozheiny and @ReneWerner87

I agree with you. My opinion is not to force users to use the data types supported by Fiber. Let's give them what they want.

dozheiny avatar Feb 08 '24 18:02 dozheiny

I rather like the way where fiber provides the standard types

If I have a custom mapping as a user, I can rebuild the short snippet given here myself The real logic is in the conversion and that could be done by fiber, otherwise the function doesn't really do anything and every consumer would always have to write a converter for the common simple data types themselves

ReneWerner87 avatar Feb 08 '24 19:02 ReneWerner87

Of course, we can also provide such a function in addition, but then additionally and without reference to the source, so that the string is given in as input

Instead of the ctx and key param

Convert[T any](value string, convertor func(string) (T, error), defaultValue ...T) (*T, error) { 
... 

ReneWerner87 avatar Feb 08 '24 19:02 ReneWerner87

I like this method, With the Convert function it's like we eat the cake and have it too. I'll implement this and create a PR.

dozheiny avatar Feb 08 '24 20:02 dozheiny

That was my proposed solution on #2777. I think that could be good for the users @dozheiny and @ReneWerner87

I agree with you. My opinion is not to force users to use the data types supported by Fiber. Let's give them what they want.

I think we can create something like https://pkg.go.dev/fmt#Stringer to allow people implement own data types.

package main

import (
	"fmt"
	"strconv"
	"strings"
)

type GenericType interface {
	int | string | any
}

type Convertible[T any] interface {
	Convert(string) T
}

func genericValue[T GenericType](s string) T {
	var t T
	switch any(t).(type) {
	case int:
		i, _ := strconv.Atoi(s)
		return assertValueType[T](i)
	case string:
		return t
	default:
		c, ok := any(t).(Convertible[T])
		if ok {
			return c.Convert(s)
		}
		return t
	}
}

type testStruct struct {
	key   string
	value string
}

func (testStruct) Convert(s string) testStruct {
	k, v, _ := strings.Cut(s, "=")
	return testStruct{k, v}
}

type testStruct2 struct {
	key string
}

func main() {
	i := genericValue[int]("42")
	fmt.Println(i)

	j := genericValue[testStruct]("foo=bar")
	fmt.Println(j)

	k := genericValue[testStruct2]("baz")
	fmt.Println(k)
}

Output:

42
{foo bar}
{}

efectn avatar Feb 11 '24 15:02 efectn