[keyserver] Move validators to endpoint array
ClosedPublic
Actions

Authored by michal on Jul 28 2023, 11:53 AM.

Details

Reviewers

kamil
tomek

Commits

rCOMM7492050ea7d9: [keyserver] Move validators to endpoint array

Summary

(ENG-3879)[https://linear.app/comm/issue/ENG-3879/move-output-and-input-validators-outside-of-the-responders]

Followup to the id schema work. Moving all the validation to the endpoint array makes it less likely for the devs to forgot to add it (which already happened a few times). Additionaly it moves the validation logic outside of the responder and now they are fully focused on the business logic.

An additional change that needed to be made was exporting the input validators, so they could be imported in the endpoints.js.

I choose to not split it into multiple diffs, because I'm doing the same thing everywhere, but if it's hard to review I can split into more diffs.

Test Plan

run yarn flow-all (note: for some reason flow allows wrong output responders, even they are correctly typed with generics)
play around the app and check if everything works correctly (in particular I tested log in, register and siwe flows)

Diff Detail

Repository

rCOMM Comm

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

michal created this revision.Jul 28 2023, 11:53 AM

Herald added a subscriber: ashoat. · View Herald TranscriptJul 28 2023, 11:53 AM

michal added inline comments.Jul 28 2023, 12:02 PM

keyserver/src/responders/handlers.js
39 ↗	(On Diff #29192)	Note that I've made this type `opaque` which basically means that it can't be created outside of this file, which is an additional safeguard against not including validators. But if it's overkill I can remove it.
keyserver/src/responders/responder-validators.test.js
4 ↗	(On Diff #29192)	The name was changed to better match the our conventions
lib/types/request-types.js
301–306 ↗	(On Diff #29192)	This wasn't validated previously, so I added this validator (the endpoint didn't return any ids so it was fine, but we should validate all responders for consistency)

Harbormaster completed remote builds in B21314: Diff 29192.Jul 28 2023, 12:16 PM

michal requested review of this revision.Jul 28 2023, 12:16 PM

Haven't reviewed all the changes too closely - there are a lot of them and they are really similar. All the important changes look good and the overall idea is great!

This revision is now accepted and ready to land.Jul 31 2023, 4:50 AM

Rebase

Harbormaster completed remote builds in B21470: Diff 29448.Aug 1 2023, 10:38 AM

Closed by commit rCOMM7492050ea7d9: [keyserver] Move validators to endpoint array (authored by Michal Gniadek <michal.gniadek@swmansion.com>). · Explain WhyAug 2 2023, 3:08 AM

This revision was automatically updated to reflect the committed changes.

Michal Gniadek <michal.gniadek@swmansion.com> added a commit: rCOMM7492050ea7d9: [keyserver] Move validators to endpoint array.