Skip to content

Separate logic from UI, secure logic with tests #1076

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

Closed
andreasabel opened this issue May 16, 2022 · 0 comments · Fixed by #1077
Closed

Separate logic from UI, secure logic with tests #1076

andreasabel opened this issue May 16, 2022 · 0 comments · Fixed by #1077
Assignees
Labels
re: code quality Concerning the code quality of the implementation of `hackage-server`

Comments

@andreasabel
Copy link
Member

Prompted by

Validators are working directly in the server monad, e.g.:

-- 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 a Left FooProblem...
  • guardFoo :: Foo -> ServerPartE () calls checkFoo and handles the exception Left ... by escalating the problem to the user.
  • Add unit tests for checkFoo
@andreasabel andreasabel added the re: code quality Concerning the code quality of the implementation of `hackage-server` label May 16, 2022
@andreasabel andreasabel self-assigned this May 18, 2022
gbaz added a commit that referenced this issue Jan 5, 2023
* delete unused GitHub Action; fix CI badge

* Fix #1076: separate validators from UI and doctest them (#1077)

* Cachix caching for nix-shell GitHub Action (#1081)

* Add uploaded_at field in package api (#1080)

* package page: Include virtual-modules in module tree (#1085)

* Allow hashable-1.4 and text-2.0 (#1089)

* Divide sitemap into parts

* Add sitemap link for subdirectories

* Fix `non-canonical-return` warnings

* Bump CI to GHC 9.2.3 and restrict to master branch

* Check authorisation (#1111)

* Dynamically add css piece

* Fix #1105: change order of markdown parsers to allow pipes in lists

* Fix #1128, fix #1130 by adding bounds to Cabal-syntax and haddock-library

* Bump CI to 9.2.4 and some deps

* Force .txt and .text to have UTF-8 MIME charset (#1133)

* Upgrade to haddock-library-1.11.0 (#1126)

* attempt to speed up GitHub Action for Nix Shell

* work with cabal 3.8

* Updated accepted licenses (#1092)

* Add dependabot for github workflows

* Bump cachix/cachix-action from 10 to 12

* Bump actions/checkout from 2.4.0 to 3.1.0
* Bump cachix/install-nix-action from 17 to 18

* Build with Cabal-3.8 and GHC 9.4 (#1141)

* Haskell CI: bump to Ubuntu-22.04, GHC 9.2.5 and 9.4.4

* Allow mtl-2.3 and transformers-0.6 (#1150)

* Disable test (#1124)

* allow disable tests on client side

* add deprecated version warning (#1123)

* List maintainers on package page (#1098)

* List maintainers on package page

* Vendor snowball package (#1116)

* Add searchbox metadata (#1115)

* Add captcha for user registration (#1099)

* remove filtering 00-index for cabal version < 2.0 hack (#1152)

* Add lastVersion in listings (#749) (#1140)

* rm icu dep instructions, add libgd

* Add test log display (#1100)

* Add test log

* Reverse Dependencies indexed on PackageName (#1082)

* Rebased Reverse Dependencies

* Add "Quick Jump" to candidate package page (#1122)

* Solves #1029 - Adds paging to recent packages and recent revisions (#1055)

* support for `prefers-color-scheme` (#1008)

* 2x brightness for captions and links in dark color scheme

* table dark color scheme

* prefers-color-scheme for links, footer, and table-of-contents

* paginator css for `prefers-color-scheme`

* Maintainer notifications

* cleanup partial functions for revdeps, elim use of MonadThrow, MonadCatch

* fix tests enablement link

Co-authored-by: Peter Becich <[email protected]>
Co-authored-by: Andreas Abel <[email protected]>
Co-authored-by: Hécate Moonlight <[email protected]>
Co-authored-by: Matthew Pickering <[email protected]>
Co-authored-by: ˌbodʲɪˈɡrʲim <[email protected]>
Co-authored-by: Alias Qli <[email protected]>
Co-authored-by: Ondřej Kubánek <[email protected]>
Co-authored-by: Gautier DI FOLCO <[email protected]>
Co-authored-by: Janus Troelsen <[email protected]>
Co-authored-by: Levi Butcher <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
re: code quality Concerning the code quality of the implementation of `hackage-server`
Projects
None yet
1 participant