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

IntCodec cannot take negative values #2378

Closed
Ellzord opened this issue Aug 9, 2023 · 8 comments · Fixed by #2393
Closed

IntCodec cannot take negative values #2378

Ellzord opened this issue Aug 9, 2023 · 8 comments · Fixed by #2393
Labels
💎 Bounty bug Something isn't working 💰 Rewarded

Comments

@Ellzord
Copy link

Ellzord commented Aug 9, 2023

Describe the bug
it's not possible to have negative int query params.

  case object IntCodec extends TextCodec[Int] {
    def apply(value: String): Int = Integer.parseInt(value)

    def describe: String = "an integer"

    def encode(value: Int): String = value.toString

    def isDefinedAt(value: String): Boolean = {
      var i = 0
      var defined = true
      while (i < value.length) {
        if (!value.charAt(i).isDigit) {
          defined = false
          i = value.length
        }
        i += 1
      }
      defined && i >= 1
    }

    override def toString(): String = "TextCodec.int"
  }

Due to the code above and that TextCodec is a sealed trait it's not possible to take a negative integer value as a query param. You also aren't able to create your own. The only workaround is to have as a string and parse yourself - this means that if you tried to generate any documentation in the future it would be wrongly typed.

This is quite an opinionated approach, what if -100 was to go back 100 items for example.

@Ellzord Ellzord added the bug Something isn't working label Aug 9, 2023
@jdegoes
Copy link
Member

jdegoes commented Aug 17, 2023

/bounty $100

We can allow parsing of negative values. Since they are, after all, ints.

@algora-pbc
Copy link

algora-pbc bot commented Aug 17, 2023

💎 $100 bounty created by jdegoes
🙋 If you start working on this, comment /attempt #2378 to notify everyone
👉 To claim this bounty, submit a pull request that includes the text /claim #2378 somewhere in its body
📝 Before proceeding, please make sure you can receive payouts in your country
💵 Payment arrives in your account 2-5 days after the bounty is rewarded
💯 You keep 100% of the bounty award
🙏 Thank you for contributing to zio/zio-http!

Attempt Started (GMT+0) Solution
🟢 @danieletorelli Aug 18, 2023, 4:43:29 AM #2393
🟢 @davidtboc Aug 20, 2023, 5:25:40 PM WIP

@yashug
Copy link

yashug commented Aug 17, 2023

@jdegoes can this condition inside while loop solves our problem ?

!value.charAt(i).isDigit && (i == 0 && value.charAt(i) != '-')

@danieletorelli
Copy link
Contributor

danieletorelli commented Aug 18, 2023

/attempt #2378

Options

@davidtboc
Copy link

davidtboc commented Aug 20, 2023

/attempt #2378 - I think isDefinedAt needs to be modified to account for a potential minus sign at the beginning of the string.

def isDefinedAt(value: String): Boolean = {
if (value.isEmpty) return false

var i = 0
if (value.charAt(0) == '-') {
if (value.length == 1) return false // just a minus sign without digits
i = 1
}

var defined = true
while (i < value.length) {
if (!value.charAt(i).isDigit) {
defined = false
i = value.length
}
i += 1
}
defined
}

Options

@algora-pbc
Copy link

algora-pbc bot commented Aug 20, 2023

Note: The user @danieletorelli is already attempting to complete issue #2378 and claim the bounty. If you attempt to complete the same issue, there is a chance that @danieletorelli will complete the issue first, and be awarded the bounty. We recommend discussing with @danieletorelli and potentially collaborating on the same solution versus creating an alternate solution.

@algora-pbc
Copy link

algora-pbc bot commented Aug 21, 2023

💡 @danieletorelli submitted a pull request that claims the bounty. You can visit your org dashboard to reward.

@algora-pbc
Copy link

algora-pbc bot commented Aug 24, 2023

🎉🎈 @danieletorelli has been awarded $100! 🎈🎊

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
💎 Bounty bug Something isn't working 💰 Rewarded
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants