-
Notifications
You must be signed in to change notification settings - Fork 201
Closed
Labels
re: code qualityConcerning the code quality of the implementation of `hackage-server`Concerning the code quality of the implementation of `hackage-server`
Description
Prompted by
Validators are working directly in the server monad, e.g.:
hackage-server/src/Distribution/Server/Util/Validators.hs
Lines 23 to 30 in 1c35e03
-- Make sure this roughly corresponds to the frontend validation in user-details-form.html.st | |
guardValidLookingEmail :: T.Text -> ServerPartE () | |
guardValidLookingEmail str = either errBadEmail return $ do | |
guard (T.length str <= 100) ?! "Sorry, we didn't expect email addresses to be longer than 100 characters." | |
guard (T.all isPrint str) ?! "Unexpected character in email address, please use only printable Unicode characters." | |
guard hasAtSomewhere ?! "Oops, that doesn't look like an email address." | |
guard (T.all (not.isSpace) str) ?! "Oops, no spaces in email addresses please." | |
guard (T.all (not.isAngle) str) ?! "Please use just the email address, not \"name\" <[email protected]> style." |
This makes it hard to test them cheaply.
There is a purely functional core that struggles to get out here: the actual validation does not need IO
etc., it is pure logic.
Suggested restructuring:
data FooError = FooProblem1 | FooProblem2 | ...
checkFoo :: Foo -> Either FooError ()
does the actual check, indicating the violation by aLeft FooProblem...
guardFoo :: Foo -> ServerPartE ()
callscheckFoo
and handles the exceptionLeft ...
by escalating the problem to the user.- Add unit tests for
checkFoo
Metadata
Metadata
Assignees
Labels
re: code qualityConcerning the code quality of the implementation of `hackage-server`Concerning the code quality of the implementation of `hackage-server`