-
Notifications
You must be signed in to change notification settings - Fork 1
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
Break up HasSchemaResult
to not require all constraints when only one is needed
#74
Comments
✨ This is an old work account. Please reference @brandonchinn178 for all future communication ✨
Sure, that's valid! It would require a lot of refactoring, though, so it
could take a while to implement.
Personally, I would recommend using plain Text anyway. JSON is a language
for communicating over the wire, and IMO things like validation logic don't
belong in JSON serialization/deserialization code. Even without this bug, I
think this would be a better design anyway:
```hs
type Payload = Object [schema|
{ password: Text }
|]
myHandler :: ReqBody Payload -> Handler ...
myHandler obj = do
validatedPassword <- validatePassword [get|obj.password|]
foo validatedPassword
```
But if you still really want PasswordPlain in the schema, you can work
around for now by implementing a dummy ToJSON instance
…On Sun, Jun 20, 2021, 2:26 AM Jihad D. Waspada ***@***.***> wrote:
Describe the bug
First of all, thanks for the amazing plugin 🙂 I just started picking up
Haskell for my pet project so my knowledge is pretty limited. So to my
understanding, this constraint
https://github.com/LeapYear/aeson-schemas/blob/70fc7e2158c4dc2626cbc4ead5998bab126c3ac0/src/Data/Aeson/Schema/Internal.hs#L178
will make sure that every type used in the schema QuasiQuote should
always have both FromJSON and ToJSON instance. I think this can be a bit
problematic if, for instance, I have a password type that can be parsed but
shouldn't be sent over the wire:
data Hash = Plain | Hashed
newtype Password (hash :: Hash) = Password Text
mkPlainPassword :: Text -> Either Text (Password 'Plain)
mkPlainPassword pw
| T.length pw >= 8 = pure $ Password pw
| otherwise = Left "Password should be at least 8 chars"
instance FromJSON (Password 'Plain) where
parseJSON rawPw = do
pw <- parseJSON rawPw
case mkPlainPassword pw of
Right p -> pure p
Left e -> fail $ T.unpack e
type PasswordPlain = Password 'Plain
type PostNewUser =
Object
[schema|
{
username: Text,
email: Text,
password: PasswordPlain,
fullname: Text,
}
|]
At least as far as Servant is concerned, I got a type error saying that Password
'Plain should have ToJSON instance, while PostNewUser is only consumed as
a request body and not as a response.
Expected behavior
I should be able to define a custom type without having ToJSON instance.
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#74>, or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AGUC75IX6JQ35GZN3XXQVBLTTWX4XANCNFSM4674YLSQ>
.
|
Thanks for the answer. From the resources read, it seems to me that many do request validation in So, would you call it a bug? Or shall I close the issue? |
✨ This is an old work account. Please reference @brandonchinn178 for all future communication ✨
I would call this a feature request. It's working as intended, but it would be nice to not require all constraints. |
ToJSON
instance is always necessary for user defined typeHasSchemaResult
to not require all constraints when only one is needed
Describe the bug
First of all, thanks for the amazing plugin 🙂 I just started picking up Haskell for my pet project so my knowledge is pretty limited. So to my understanding, this constraint https://github.com/LeapYear/aeson-schemas/blob/70fc7e2158c4dc2626cbc4ead5998bab126c3ac0/src/Data/Aeson/Schema/Internal.hs#L178 will make sure that every type used in the
schema
QuasiQuote should always have bothFromJSON
andToJSON
instance. I think this can be a bit problematic if, for instance, I have a password type that can be parsed but shouldn't be sent over the wire:At least as far as Servant is concerned, I got a type error saying that
Password 'Plain
should haveToJSON
instance, whilePostNewUser
is only consumed as a request body and not as a response.I'm not sure if this is Servant's specific problem or not, but perhaps you can help me give a clue
Expected behavior
I should be able to define a custom type without having
ToJSON
instance.The text was updated successfully, but these errors were encountered: