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

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

Closed
3 tasks done
dozheiny opened this issue Dec 11, 2023 · 21 comments · Fixed by #2776 or #2850
Closed
3 tasks done

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

dozheiny opened this issue Dec 11, 2023 · 21 comments · Fixed by #2776 or #2850

Comments

@dozheiny
Copy link
Contributor

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:

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

welcome bot commented Dec 11, 2023

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

@ReneWerner87
Copy link
Member

We could do it for v3 not for v2

@ReneWerner87 ReneWerner87 added this to v3 Dec 11, 2023
@ReneWerner87 ReneWerner87 added this to the v3 milestone Dec 11, 2023
@brunodmartins
Copy link
Contributor

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

@nickajacks1
Copy link
Member

@brunodmartins v3-beta is the latest v3 development branch

@efectn efectn linked a pull request Dec 23, 2023 that will close this issue
13 tasks
brunodmartins added a commit to brunodmartins/fiber that referenced this issue Dec 23, 2023
@github-project-automation github-project-automation bot moved this to Done in v3 Jan 19, 2024
@ReneWerner87 ReneWerner87 reopened this Jan 19, 2024
@ReneWerner87
Copy link
Member

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

@dozheiny
Copy link
Contributor Author

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

@ReneWerner87
Copy link
Member

Ok thx, every help is appreciated

@brunodmartins
Copy link
Contributor

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

@ReneWerner87
Copy link
Member

@dozheiny How is the progress ?

@dozheiny
Copy link
Contributor Author

dozheiny commented Feb 6, 2024

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

@ReneWerner87
Copy link
Member

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

@ryanbekhen
Copy link
Member

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?

@ReneWerner87
Copy link
Member

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

@ryanbekhen
Copy link
Member

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.

@dozheiny
Copy link
Contributor Author

dozheiny commented Feb 8, 2024

@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.

@brunodmartins
Copy link
Contributor

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

@dozheiny
Copy link
Contributor Author

dozheiny commented Feb 8, 2024

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.

@ReneWerner87
Copy link
Member

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
Copy link
Member

ReneWerner87 commented Feb 8, 2024

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) { 
... 

@dozheiny
Copy link
Contributor Author

dozheiny commented Feb 8, 2024

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.

@efectn
Copy link
Member

efectn commented Feb 11, 2024

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}
{}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment