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]: Using Number as Valid Boolean Query Parameter #2723

Open
3 tasks done
afifurrohman-id opened this issue Nov 14, 2023 · 10 comments
Open
3 tasks done

🚀 [Feature]: Using Number as Valid Boolean Query Parameter #2723

afifurrohman-id opened this issue Nov 14, 2023 · 10 comments

Comments

@afifurrohman-id
Copy link

Feature Description

It would be better right if Query Parameter Number is Valid Boolean Value using QueryBool() method

The mechanism just like a python when casting type int to bool.
0 is false otherwise is true

image

Additional Context (optional)

curl https://example.com/?is_ok=1&is=0

Code Snippet (optional)

package main

import "github.com/gofiber/fiber/v2"
import "log"

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

  app.Get("/", func (ctx *fiber.Ctx) {
     fmt.Println(ctx.QueryBool("is_ok")) // true
     fmt.Println(ctx.QueryBool("is")) // false

     return nil
  })

  log.Fatal(app.Listen(":3000"))
}

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.
@ReneWerner87
Copy link
Member

@dozheiny what do you think ?

@dozheiny
Copy link
Contributor

dozheiny commented Dec 3, 2023

The QueryBool function uses strconv.ParseBool function for parsing boolean value types. And it supports 1 and 0 values.
But for a -1 or any other value, it didn't support it and returned an error.
Should we add support for integers values not in the strconv.ParseBool?

@dozheiny
Copy link
Contributor

dozheiny commented Dec 3, 2023

@ReneWerner87
Copy link
Member

The current behavior and internal usage is clear. What is your opinion to change the return value to true if the internal function returns an error because the values are not in the intersection.

@dozheiny
Copy link
Contributor

dozheiny commented Dec 3, 2023

In #2329 and #2391, We discussed and changed it to false from true.

@dozheiny
Copy link
Contributor

dozheiny commented Dec 3, 2023

I think returning a false value if it returns an error is better than returning a true.

@ReneWerner87
Copy link
Member

ReneWerner87 commented Dec 3, 2023

I agree and would rather not take the python approach
But the approach where we use false as value if our value list for the boolean value detection is not matched (which corresponds to the underlying golang functionality)

To consider multiple opinions I would like to vote with thumbs up or thumbs down in the initial feature request comment
@gofiber/contributors @gofiber/maintainers

@gaby
Copy link
Member

gaby commented Dec 3, 2023

Any non-zero value should return True. For example:

curl https://example.com/?is_ok=-1

Should return True.

@sixcolors
Copy link
Member

sixcolors commented Dec 3, 2023

As a proponent of developer autonomy, I believe query parameter handling should typically be managed by the application developer. Nonetheless, gofiber provides Query* functions for convenience. Therefore, I maintain that QueryBool() should default to the Zero Value, false, for bool when no default is provided:

  • Consistency: Modifying QueryBool() could disrupt applications that depend on it for parsing query parameters as boolean values. Maintaining the current behavior ensures the gofiber framework remains consistent and predictable.

  • Go Standards: Adapting QueryBool() to Python’s type casting would conflict with Go’s explicit type conversion conventions and the strings package’s ParseBool() function, which only recognizes specific string values, “1”, “t”, “T”, “true”, “TRUE”, “True”, as true. See https://go.dev/ref/spec#The_zero_value, https://go.dev/ref/spec#Conversions, https://pkg.go.dev/strconv#ParseBool and https://go.dev/play/p/ula8bAP4iJw.

  • Developer Control: gofiber’s QueryBool() supports an optional default value, allowing developers to specify what should happen when a query parameter is missing or invalid. For example, c.QueryBool("key", true) returns true when a parameter is absent or unparseable.

Preserving the current implementation of QueryBool() respects Go’s conventions and developers’ expectations, ensuring framework consistency and reliability.

@wangjq4214
Copy link
Member

As a proponent of developer autonomy, I believe query parameter handling should typically be managed by the application developer. Nonetheless, gofiber provides Query* functions for convenience. Therefore, I maintain that QueryBool() should default to the Zero Value, false, for bool when no default is provided:

  • Consistency: Modifying QueryBool() could disrupt applications that depend on it for parsing query parameters as boolean values. Maintaining the current behavior ensures the gofiber framework remains consistent and predictable.
  • Go Standards: Adapting QueryBool() to Python’s type casting would conflict with Go’s explicit type conversion conventions and the strings package’s ParseBool() function, which only recognizes specific string values, “1”, “t”, “T”, “true”, “TRUE”, “True”, as true. See https://go.dev/ref/spec#The_zero_value, https://go.dev/ref/spec#Conversions, https://pkg.go.dev/strconv#ParseBool and https://go.dev/play/p/ula8bAP4iJw.
  • Developer Control: gofiber’s QueryBool() supports an optional default value, allowing developers to specify what should happen when a query parameter is missing or invalid. For example, c.QueryBool("key", true) returns true when a parameter is absent or unparseable.

Preserving the current implementation of QueryBool() respects Go’s conventions and developers’ expectations, ensuring framework consistency and reliability.

I agree.

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

No branches or pull requests

6 participants