Add wrap-validation middleware based on Malli

[KingMob]

@arnaudgeiser What do you think of this?

Also moves the schemas to a common aleph.http.schema ns, so they can be shared.

Not sure we need each individual var schema just yet, so I've hidden the ns from cljdoc for now.

[KingMob]

We should probably tighten the schemas soon. I've had bad experiences in the past using spec, when people let specs be loose for too long, they became concerned that locking them down would break something (E.g., one team only accepts ports as numbers, but another team also accepts them as strings which it parses...)

[KingMob]

...and I'm already reminded of why :pedantic :abort was a huge pain. It lead to people excluding things at random because they just want their build to succeed.

[arnaudgeiser]

@arnaudgeiser What do you think of this?

I'm totally fine with using Malli instead of spec, I don't have strong opinions about them. However, I'm still wondering if the overhead of validating spec is worth. Actually, only the presence of the request-method is strongly required and currently leads to intelligible error.

We should probably tighten the schemas soon. I've had bad experiences in the past using spec, when people let specs be loose for too long, they became concerned that locking them down would break something (E.g., one team only accepts ports as numbers, but another team also accepts them as strings which it parses...)

Isn't it too late already?

...and I'm already reminded of why :pedantic :abort was a huge pain. It lead to people excluding things at random because they just want their build to succeed.

You're a bit harsh here. :smile: We don't add dependencies everyday and I prefer having errors at build time instead of at runtime with libraries incompatibilities. It might be annoying, but I would say it's manageable on a project like Aleph.

In the current example, I'm wondering if we should also pin the org.clojure/clojure like metosin does for malli.

[DerGuteMoritz]

However, I'm still wondering if the overhead of validating spec is worth. Actually, only the presence of the request-method is strongly required and currently leads to intelligible error.

Maybe make validation opt-in instead? That way we could even justify making the schemas stricter than what is currently implemented (e.g. get rid of the :maybe schemas).

Or perhaps even better: Use defn schemas instead of middleware for this purpose? Then users can choose to enable instrumentation e.g. only for their test suites. This would also unlock static checking via clj-kondo.

[KingMob]

Let's take a step back and decide what problem we actually want to solve.

The original issue (#162) was lvh complaining about the error message he got when he forgot the method key.

What else (if anything) do we want out of validation? Just to return some better error messages for bad requests? To lock down the request format users submit to the client? To capture our current loose schemas, but then transform the input into something more stringent for Aleph's use? Get kondo support for our own use?

Personally, I:

• am slightly in favor of better error messages for users
• think locking it down will cause breakage
• think transforming is nice to have, but not important
• think kondo support would help a lot, given how kondo-unfriendly aleph and manifold are

[arnaudgeiser]

Just to return some better error messages for bad requests?

For me, that should be the focus.

[KingMob]

@DerGuteMoritz Any thoughts on the goal of validation?

[DerGuteMoritz]

@KingMob Thanks for providing context of the original issue, I was missing that :pray:

Personally, I:

• am slightly in favor of better error messages for users

Looking at the original issue, that definitely should be the priority. Question is whether this alone justifies pulling in malli, especially if it's only for this particular API. We'd end up with a mixed bag of schema-based and hand-rolled validation (and errors). I feel like adopting a schema-based approach should be done in a more holistic fashion. For https://github.com/clj-commons/aleph/issues/162 I now lean towards a minimally invasive patch with hand-rolled validation.

• think locking it down will cause breakage

Agreed.

• think transforming is nice to have, but not important

Agreed.

• think kondo support would help a lot, given how kondo-unfriendly aleph and manifold are

Indeed. As mentioned above, I think doing this via malli is worthwhile since it will also give us opt-in runtime validation (which could then supercede the hand-rolled one) and generative testing support at the same time. Ideally then for all of Aleph's public API.

[KingMob]

I'd like to pull in Malli as a vote for the future, then, and because kondo support would help me, personally, a lot. We might not need it for any given small PR, but if it's already a dep, we can start to build and rely on it.

[KingMob]

Any further thoughts on this? @DerGuteMoritz do you still want changes, or is that stale?