-
Notifications
You must be signed in to change notification settings - Fork 6
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
Introduce High Level Validation #19
Comments
I was thinking about something similar to Ecto's changests. Struct which holds applied parameters and be passed to validation functions. When validation finds an error, it is added to That way. using Ecto's ideas, we have composition, single source of truth and separation of side-effects with deterministic functions. I see it that way, what do you think @Dzol , @michalwski . |
I think it's a nice idea but I've been thinking about it for a while and I'm not sure how we could make it quite like Ecto's Changesets. Do you have any ideas on that? I was thinking of something along the lines of what I suggested this morning (for the server-side). It's largely or completely side-effect free except for reading-from and writing-to the wire. Perhaps we could mould Ecto-like Changesets into this... Let's write a list of the kinds of validation and changes we'll need to make:
This pseudo-snippet helps explore the idea: alias Fennec.Changeset
@spec respond_changeset(Jerboa.Format.t, parameters = %{Jerboa.Attribute.t}) :: Jerboa.Format.t
def allocate_changeset(request, %{Realm, TransitiveCandidate}) do
request
|> cast/change(parameters)
|> assert_required([Nonce, Realm, TransitiveCandidate, ...])
|> validate_nonces ## Old against new.
|> change_nonces ## New one for the old one.
|> ...
|> change_class(:success) ## This doesn't seem to belong here if we're gonna have `valid?/1`
end
if allocate_changeset(..., ...).valid? do
:hooray
else
e = Changeset.traverse_errors(...)
send(..., failure(e))
end The immediate benefit I can see is that we have a really readable fragment we can use to make sure we have the correct data in our response (or whaterver). I.e. the changeset says step-by-step what we need to do to get everything right. Get the changeset right and we get the response right! Aside: we have to remember that peers don't talk STUN. |
If we could make it like in your snippet, that would be awesome! We can check out how Ecto implements changesets, but its all about wrapping some struct with data with another struct holding errors and parameters to apply. In our case it's even more simple, because we exactly now what parameters are in our data structure. I really like the approach. |
We've made good progress on the server-side esl/MongooseICE#2. Something similar for the client I suspect. Whatever it is will happen, to some extent, when we build the client in #21. |
Even though PR #21 introduces a client it falls short of an elegant solution for the client-side which is okay for the time-being because our main concern is the server. |
Apparently this should be further discussed, but I don't think it's vital point for 0.1 version. I'm removing it from the milestone and we'll get back to that. |
This is in some point addressed by #67 . This PR introduces |
Currently Jerboa only does encoding + decoding (all this is under the format directory) but there are higher level concerns.
Only certain STUN classes are permissible on each STUN method (e.g. we can't have a binding indication). The
Format
modules are wholly and exclusively responsible for encoding + decoding so this is not their concern.We need to introduce a layer to handle this logic which is more akin to the kind of validation a parser does (as opposed to a scanner/lexer which is sort of what we have at the moment).
Parts common to server and client can make their way into Jerboa (though I expect this is going to be quite thin).
The text was updated successfully, but these errors were encountered: