[core.spec] Stricter map validations?

[Yuri Govorushchenko]

Hi!

I have some noobie questions for which I couldn't google the compelling answers.

1) Is there any way to ensure that the keys I used in `s/keys` have the associated specs defined? At compile time or at least at runtime. Maybe via an additional library? I could imagine a macro (smt. like `s/keys-strict` or `s/map-pairs`, as maps can also be viewed as sets of spec'ed pairs) which additionally checks that all keys have specs registered. I'm OK with sacrificing some flexibility (e.g. being able to define key specs after map specs, dynamically, etc.) in favour of more strictness.

Motivation: I don't fully trust my map validation code when using `core.spec`. `s/keys` doesn't require that the key has the spec registered to validate its value. Although this may be flexible but in practice can lead to errors. Specifically, it's quite easy to forget to create a spec for a key, mistype it or forget to require the namespace in which key spec is defined (e.g. if the common key specs reside in a dedicated ns):

```

; totally forgot to define a spec for ::foo

(s/def ::bar (s/keys :req [::foo]))

; fooo vs. foo typo

(s/def ::fooo string?)

(s/def ::bar (s/keys :req [::foo]))

; :common/foo vs. ::common/foo typo

(s/def ::bar (s/keys :req [:common/foo]))

; didn't require common.core ns (spec for :common.core/foo is not added to global registry)

(s/def ::bar (s/keys :req [:common.core/foo]))

```

These subtle mistakes can lead to map validations passing silently (as long as keysets are correct).

Related to this: there're feature requests for Cursive IDE which try to address typing and reading mistakes related to keywords, e.g. https://github.com/cursive-ide/cursive/issues/1846 and https://github.com/cursive-ide/cursive/issues/1864.

After using Schema for a while it's difficult to appreciate the way `core.spec` defines it's own global registry which uses keywords instead of using spec instances and good old variables, especially since Cursive IDE has quite a nice support for variables already. But I think this is another topic which was already discussed, e.g. in https://groups.google.com/forum/#!topic/clojure/4jhSCZaFQFY ("Spec without global registry?").

2) What is the motivation for library having a "loose" default behaviour of `s/keys` and no "strict" variant at all for spec-ing both keys and values at the same tome? I think in majority of cases I'd need to spec both keys and values of the map instead of only keys and would expect the library to have built-in API for this. Maybe for the future references it would be beneficial to add concrete code examples into motivation in the core.spec guide (https://clojure.org/about/spec#_map_specs_should_be_of_keysets_only) which would better illustrate the described benefits of the current lib behaviour?

Thanks.

[Beau Fabry]

> 1) Is there any way to ensure that the keys I used in `s/keys` have the associated specs defined?

I think you should be able to do this by writing a function that uses s/registry s/form and s/get-spec

[Leon Grapenthin]

I second this from my experience, using spec quite extensively since its release.

We already had some invalid data passing silently because of this. It can easily happen if you have a typo in the spec.

Also we never experienced benefits from being able to not spec keys required in s/keys. It appears to be a pretty obsolete feature, making vulnerabilities more likely.

[Beau Fabry]

fwiw, I vote for leaving it. It's extra flexibility and is afaict a fairly easy error to catch. Here's a function that I think should give you every qualified keyword that is used in a spec but that does not have a spec defined for it:

boot.user=> (let [kws (atom #{})]

       #_=>   (clojure.walk/postwalk (fn [x] (when (qualified-keyword? x) (swap! kws conj x)) x) (map s/form (vals (s/registry)))) (clojure.set/difference @kws (set (keys (s/registry))))

       #_=> )

#{:clojure.spec.alpha/v :clojure.spec.alpha/k}

boot.user=>

[Didier]

I vote for strict validation also. It's a security threat on most applications not to strictly validate inputs and outputs. I would rather Clojure defaults to be secure, and people needing to implement their own less strict validation consciously. I'm afraid a lot of people will rely on spec for their validation, and not think of this, opening themselves to new attack vectors.

Here's two example:

1) You accidentally put extra sensitive data on maps that end up persisted unencrypted. If spec doesn't fail a map validation if it contains extra keys, this is easy to mistakenly have happen.

2) You mistype the spec of a key, and inadvertly now don't validate a user input field, allowing it to pass validation undetected. You're now opened to code injection attacks.

Or at least include in core a strict spec, like s/keys and s/keys-strict. I know personally, I'll need the strict variant in 100% of my use cases, as I work on backend services.

[Yuri Govorushchenko]

Thank you the pointers! So far I ended up with writing a small `map` macro which is similar to `s/keys` but checks that keys are already in the registry: https://gist.github.com/metametadata/5f600e20e0e9b0ce6bce146c6db429e2

[Alex Miller]

On Monday, October 2, 2017 at 10:37:31 AM UTC-5, Yuri Govorushchenko wrote:

Hi!

I have some noobie questions for which I couldn't google the compelling answers.

1) Is there any way to ensure that the keys I used in `s/keys` have the associated specs defined?

Specs can be defined or added later, so there is no valid way to do this.

This is all about evolution of specs over time. If you make something "strict" then you are saying "this is not allowed". You thus can never extend the key set in the future without causing breakage. Whereas if you say what a key set must have and validate what it does have, you can also grow the spec in the future. Rich expanded at length on this idea and this particular case in https://www.youtube.com/watch?v=oyLBGkS5ICk .

Alex 

[Alex Miller]

On Monday, October 2, 2017 at 12:30:57 PM UTC-5, Leon Grapenthin wrote:

I second this from my experience, using spec quite extensively since its release.

We already had some invalid data passing silently because of this. It can easily happen if you have a typo in the spec.

Also we never experienced benefits from being able to not spec keys required in s/keys. It appears to be a pretty obsolete feature, making vulnerabilities more likely.

It is more common to experience the negative effects of restricting key sets as you evolve through versions, and in fact we have experienced on virtually every consulting project we'd done including spec at Cognitect. So I do not think it is at all obsolete.

[Didier]

| we have experienced on virtually every consulting project we'd done including spec at Cognitect

I'm sure this is sometimes true, but I can't think of how that would happen. Could you detail it a little?

For me, whenever I needed to add more keys, it was simple to evolve the spec with the function, why would you leave the spec behind?

[Mark Engelberg]

Yesterday, I was checking a map of info submitted via web before putting its contents into a database.  To prevent people from spamming the database, it's necessary to make sure there aren't additional keys thrown into the map.  It would be nice to have a convenient way to express this in spec, to make it easier to use for these validation purposes in addition to its routine use as specifying a minimal contract on function inputs.

--
You received this message because you are subscribed to the Google
Groups "Clojure" group.
To post to this group, send email to clo...@googlegroups.com
Note that posts from new members are moderated - please be patient with your first post.
To unsubscribe from this group, send email to
clojure+unsubscribe@googlegroups.com
For more options, visit this group at
http://groups.google.com/group/clojure?hl=en
---
You received this message because you are subscribed to the Google Groups "Clojure" group.
To unsubscribe from this group and stop receiving emails from it, send an email to clojure+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[Peter Hull]

I have found, as an application evolves, some extra data is required which comes from elsewhere.

For example, I have an application where data is read from a network socket, transformed and stored. For auditing, I later decided it was useful to capture the remote address from the socket right at the start, and finally to log it. With restrictive specs, I would have had to re-spec all the intermediate functions. As it was, they didn't know or need to know about the extra key/value in the map, just pass it along.

On puzzler's database example, I would have thought that restricting the keys that go into the DB should not be the job of spec (since functions may not be instrumented anyway), but the job of the 'core logic'. Maybe I am misunderstanding though.

[Mark Engelberg]

On Tue, Oct 3, 2017 at 2:55 AM, Peter Hull <peter...@gmail.com> wrote:

On puzzler's database example, I would have thought that restricting the keys that go into the DB should not be the job of spec (since functions may not be instrumented anyway), but the job of the 'core logic'. Maybe I am misunderstanding though.

Even when functions are not instrumented, one powerful use of spec is to call valid? or conform from within your code to test data against specs as part of the function's logic.  One of the value propositions of spec is that it handles a whole range of use cases.  You define your spec once, and one of the many things you can do with it is to use it as part of your validation logic.

[Leon Grapenthin]

My critique is not towards closed keysets but about being able to (s/keys :req [::some-kw]) without having defined the spec ::some-kw, but e.g. ::soem-kw instead. This can trip you up badly, and not at compile time. I'd be surprised if this never happened to you.

Regarding open maps and keysets I'm fine with them. However, I use select-keys after validating data over the wire. Would be nice if there were some "select-spec" for open specs.

[Yuri Govorushchenko]

1) About `s/keys` silently ignoring missing value specs. My question was: "Is there any way to ensure that the keys I used in `s/keys` have the associated specs defined?."

 Specs can be defined or added later, so there is no valid way to do this.

OK, so requiring that values are spec-ed can't be enforced at compilation time because this would make it impossible to define value specs after `s/keys`:

```

(s/def ::foo (s/keys :req [::bar]))

,,,

(s/def ::bar ,,,)

```

This can explain why it's not built into the library. In such case I'm fine with using a custom macro instead of `s/keys` (see my gist in the previous post).

But what about enforcing existence of value specs at runtime, during validation? `s/cat` does that, e.g.:

```

cljs.user=> (s/def ::foo (s/cat :bar ::baz))

:cljs.user/foo

cljs.user=> (s/valid? ::foo [123])

Unable to resolve spec: :cljs.user/baz

```

Why is it then not a default behaviour for `s/keys` as well? I.e.:

```

; current behavior

cljs.user=> (s/def ::foo (s/keys :req [::x]))

:cljs.user/foo

cljs.user=> (s/valid? ::foo {::x 123})

true

; desired behavior

cljs.user=> (s/def ::foo (s/keys :req [::x]))

:cljs.user/foo

cljs.user=> (s/valid? ::foo {::x 123})

Unable to resolve spec: :cljs.user/x

```

I don't think Spec-ulation Keynote addresses this behaviour.

2) There's no *built-in* way restrict the keyset of the map in `core.spec`.

The reasoning for this seems to be based around the idea of backwards compatible evolving specs (see Spec-alution Keynote). But there are several good examples already covered in this thread which demonstrate that it would be very convenient to also have strict keyset validations available in `core.spec`. After all, the library is about specifying the structure of data and not only about specifying API contracts.

3) Thinking more about `s/keys` vs. `s/cat` *specifically* in the context of asserting API contracts (I'm stressing on the context here because `core.spec` is a general library and should take in account other use cases too).

In this context it's even more apparent that `s/keys` should behave in the same way as `s/cat` because there's not much difference between positional arguments and keyword arguments. I'll try to illustrate what I mean with an example. Let's say there's a function with positional arguments:

```

(defn foo-pos [x y z])

; call example:

(foo xxx yyy zzz)

```

I hope we can agree that it's more or less equivalent to this function with the keyword arguments where each keyword corresponds to the position number in `foo-pos`:

```

(defn foo-pos* [{x 1 y 2 z 3}])

; call example:

(foo-pos* {1 xxx 2 yyy 3 zzz})

```

And making a step further to better naming:

```

(defn foo-kw [{:keys [::x ::y ::z]}])

; call example:

(foo-kw {::x xxx ::y yyy ::z zzz})

```

So, the biggest difference is the syntax of function calls. Keyword arguments are usually more readable (esp. when there are several args) and easier to maintain (since they can be reordered at the call site and function definition safely). Let's now spec-ify the arguments using `s/cat` for positional args and `s/keys` for keyword args (as recommended in docs). These specs ensure that argument `x` is present and is of type `::x`, `y` is present and is of type `::y`, etc.:

```

(s/def ::foo-pos-args (s/cat :x ::x :y ::y :z ::z))

(s/def ::foo-kw-args (s/keys :req [::x ::y ::z]))

```

Now (because the functions are equivalent) I'd expect their specs to validate equivalent inputs in the similar way. But it's not the case if developer forgets to define the `::y` spec!

```

; ::y spec is missing

(s/def ::x int?)

(s/def ::z int?)

; positional args

(def pos-inputs [1 2 3])

(s/valid? ::foo-pos-args pos-inputs) ; => Unable to resolve spec: :cljs.user/y (good)

; keyword args

(def kw-inputs {::x 1 ::y 2 ::z 3})

(s/valid? ::foo-kw-args kw-inputs) ; => true (ouch!)

```

TL/DR: (specifically in the context of function contracts) `core.spec` shouldn't treat APIs with positional arguments and APIs with keyword arguments differently and thus `s/keys` should check arg values at keys in the same way `s/cat` checks arg values at positions.

[Alex Miller]

On Tuesday, October 3, 2017 at 8:10:30 AM UTC-5, Yuri Govorushchenko wrote:

1) About `s/keys` silently ignoring missing value specs. My question was: "Is there any way to ensure that the keys I used in `s/keys` have the associated specs defined?."

 Specs can be defined or added later, so there is no valid way to do this.

OK, so requiring that values are spec-ed can't be enforced at compilation time because this would make it impossible to define value specs after `s/keys`:

```

(s/def ::foo (s/keys :req [::bar]))

,,,

(s/def ::bar ,,,)

```

This can explain why it's not built into the library. In such case I'm fine with using a custom macro instead of `s/keys` (see my gist in the previous post).

But what about enforcing existence of value specs at runtime, during validation? `s/cat` does that, e.g.:

```

cljs.user=> (s/def ::foo (s/cat :bar ::baz))

:cljs.user/foo

cljs.user=> (s/valid? ::foo [123])

Unable to resolve spec: :cljs.user/baz

```

This is required because you are essentially parsing a sequential data structure - one component could be a single value or many values, so you must know it's definition.

 

Why is it then not a default behaviour for `s/keys` as well? I.e.:

Because all map keys are independent and don't rely on each other for ordering.

 

```

; current behavior

cljs.user=> (s/def ::foo (s/keys :req [::x]))

:cljs.user/foo

cljs.user=> (s/valid? ::foo {::x 123})

true

; desired behavior

cljs.user=> (s/def ::foo (s/keys :req [::x]))

:cljs.user/foo

cljs.user=> (s/valid? ::foo {::x 123})

Unable to resolve spec: :cljs.user/x

```

I don't think Spec-ulation Keynote addresses this behaviour.

2) There's no *built-in* way restrict the keyset of the map in `core.spec`.

The reasoning for this seems to be based around the idea of backwards compatible evolving specs (see Spec-alution Keynote). But there are several good examples already covered in this thread which demonstrate that it would be very convenient to also have strict keyset validations available in `core.spec`. After all, the library is about specifying the structure of data and not only about specifying API contracts.

You can add additional checks to verify stricter things on top of open specs. You can't add openness to something that is already closed.

 

3) Thinking more about `s/keys` vs. `s/cat` *specifically* in the context of asserting API contracts (I'm stressing on the context here because `core.spec` is a general library and should take in account other use cases too).

In this context it's even more apparent that `s/keys` should behave in the same way as `s/cat` because there's not much difference between positional arguments and keyword arguments.

But there is - namely, the positional part. s/cat must know the spec for every component to successfully conform the entire sequence. s/keys can independently conform (or not conform) every key.

 

I'll try to illustrate what I mean with an example. Let's say there's a function with positional arguments:

```

(defn foo-pos [x y z])

; call example:

(foo xxx yyy zzz)

```

I hope we can agree that it's more or less equivalent to this function with the keyword arguments where each keyword corresponds to the position number in `foo-pos`:

Yeah but it's NOT equivalent. s/cat allows you to nest arbitrary sequential structure in a component. The spec for this could be:

(s/def ::last-args (s/cat :y any? :z any?))

(s/fdef foo :args (s/cat :x any? :last ::last-args))

Without knowing the structure of ::last-args, s/cat can't validate the input.

Not going to respond to the rest of this post, as it uses this false argument of equivalence.

[Tommi Reiman]

Hi.

Spec-tools (https://github.com/metosin/spec-tools) has some tools for this: the spec visitor (walking over all core specs, e.g. to collect all registered specs) and map-conformers: fail-on-extra-keys and strip-extra-keys. 

Here's an example to strip away extra keys:

(require '[clojure.spec.alpha :as s])
(require '[spec-tools.core :as st])

(s/def ::name string?)
(s/def ::street string?)
(s/def ::address (st/spec (s/keys :req-un [::street])))
(s/def ::user (st/spec (s/keys :req-un [::name ::address])))

(def inkeri
  {:name "Inkeri"
   :age 102
   :address {:street "Satamakatu"
             :city "Tampere"}})

(st/select-spec ::user inkeri)
; {:name "Inkeri"
;  :address {:street "Satamakatu"}}

There is CLJ-2116 that would enable this without any wrapping of specs: https://dev.clojure.org/jira/browse/CLJ-2116. Seems stalled.

If runtime transformations would be supported (currently: out of scope), I think we could go even further - we could rewrite conform* as walk* to support all of fast coercion, explain and conform. We are using conform + unform as a substitute of coercion but sadly, it's quite slow, especially for maps.

Tommi

[Yuri Govorushchenko]

Thank you, it looks handy!

[Yuri Govorushchenko]

3) I agree that my argumentation was vague but the sentiment still holds true: I'd like to easily validate keyword args with the same level of confidence as I can validate positional args. 

1,2)

> Because all map keys are independent and don't rely on each other for ordering.

> You can add additional checks to verify stricter things on top of open specs. You can't add openness to something that is already closed.

If I understand you correctly, library currently is only concerned with doing simplest things and that's why `s/keys` is not very strict? But then I'm not sure if it's the simplest behaviour though since `s/keys` specs validate 2 things: keys AND (sometimes) values. And that "sometimes" part makes production code more error-prone.

Is there any chance that in the future core.spec will have additional API to allow specifying other levels of map validation strictness? Possible options:

* Keyset validation strictness:

  - open (current behavior, allows putting any additional keys into map)

  - closed (map must contain only the listed keys)

* Values validation strictness: 

  - never validate (i.e. any values will pass, not sure about the use cases though)

  - only if spec for key is registered (current behavior)

  - always validate values (should fail if there's no spec for value registered)

Thanks.

[Didier]

| I would have had to re-spec all the intermediate functions

I'm not able to follow how that is?

You would have only needed this if you are somewhere validating or have setup test.check to run. In both cases, if you wanted to validate, why don't you also want to validate this new field? Or why not test.check your code with it?

Also, if you had a keys spec for this map, it would only require you adding one key to that one spec. Why would all your intermediate functions not share the same map spec?

Anyhow, I'm not arguing against open key specs, I'm arguing for adding to spec a closed keys spec. Either as an option to s/keys or another spec macro. There clearly a large number of people who seem to have use cases for both, and implementing your own is non trivial, I think it would be a great addition to spec for practical completeness.

[Didier]

| Spec-tools (https://github.com/metosin/spec-tools) has some tools for this: the spec visitor (walking over all core specs, e.g. to collect all registered specs) and map-conformers: fail-on-extra-keys and strip-extra-keys.

I understand the core team wanting to take a minimal approach to spec, and that open is easier to restrict later. But I worry that already in alpha state, spec is unpractical for many people as is, and Orchestra and Spec-tools are already needed supplement.

For instrumentation, it's no big deal, but for specs I think it is. Having a canonical set of specs accross Clojure shops is a way to form a common language. If I start having my custom map specs, and so does everyone, I'd be tempted to say something core is missing. Strict map specs which also vallidates each key has a registerer spec I think is a glaring omission.

Having used spec in one of my design, I've had to justify already to 6 people, some being senior security engineers, why the validation allows for open keys. Other team members were just confused as to why that was, and the only argument I had was that Rich doesn't like to break APIs :p. But I've had to add validation on top to pass security audit. So I think while not breaking APIs when incrementally adding specs to legacy code is a good use case, there's the security and safety use case which seems to be shared by a large swat of Clijurist, and I think spec is in need of a core support for it.

[Gary Trakhman]

I don't disagree that there might be a security use-case, and regardless of how realistic it is businesses need people to tick boxes (that doesn't seem like something core should worry about?), but what is the actual security risk of an unspec'd map value, or things being referenced and unused?  The ocaml (strict) type system allows for this for example with row polymorphic records.

--
You received this message because you are subscribed to the Google
Groups "Clojure" group.
To post to this group, send email to clo...@googlegroups.com
Note that posts from new members are moderated - please be patient with your first post.
To unsubscribe from this group, send email to

clojure+u...@googlegroups.com

For more options, visit this group at
http://groups.google.com/group/clojure?hl=en
---
You received this message because you are subscribed to the Google Groups "Clojure" group.

To unsubscribe from this group and stop receiving emails from it, send an email to clojure+u...@googlegroups.com.

[Alex Miller]

On Tuesday, October 3, 2017 at 11:48:04 AM UTC-5, Yuri Govorushchenko wrote:

3) I agree that my argumentation was vague but the sentiment still holds true: I'd like to easily validate keyword args with the same level of confidence as I can validate positional args. 

Nothing is preventing you from adding additional validation for your keyword args. But that doesn't have to be in spec.

 

1,2)

> Because all map keys are independent and don't rely on each other for ordering.

> You can add additional checks to verify stricter things on top of open specs. You can't add openness to something that is already closed.

If I understand you correctly, library currently is only concerned with doing simplest things and that's why `s/keys` is not very strict? But then I'm not sure if it's the simplest behaviour though since `s/keys` specs validate 2 things: keys AND (sometimes) values. And that "sometimes" part makes production code more error-prone.

No, this is not about "simple", it's about creating specs that can grow, rather than break. I would really urge you to watch the hour+ Spec-ulation keynote from Rich at last year's Conj that explains all of this. It doesn't make sense for me to repeat it. He explicitly cover this aspect in the talk.

 

Is there any chance that in the future core.spec will have additional API to allow specifying other levels of map validation strictness? Possible options:

No, see above. It is possible that there will be a variant of keys that will tell you the parts that weren't validated or something like that.

 

* Keyset validation strictness:

  - open (current behavior, allows putting any additional keys into map)

  - closed (map must contain only the listed keys)

not every going to do that for reasons above

 

* Values validation strictness: 

  - never validate (i.e. any values will pass, not sure about the use cases though)

Can do this now with a `map?` or `s/map-of` spec.

[Alex Miller]

On Tuesday, October 3, 2017 at 12:25:40 PM UTC-5, Didier wrote:

| Spec-tools (https://github.com/metosin/spec-tools) has some tools for this: the spec visitor (walking over all core specs, e.g. to collect all registered specs) and map-conformers: fail-on-extra-keys and strip-extra-keys.

I understand the core team wanting to take a minimal approach to spec, and that open is easier to restrict later.

It's not about easier, it's about possible. Open grows, closed breaks.

 

But I worry that already in alpha state, spec is unpractical for many people as is

This is demonstrably false. Many people are using it and happy with it now.

 

, and Orchestra and Spec-tools are already needed supplement.

For instrumentation, it's no big deal, but for specs I think it is. Having a canonical set of specs accross Clojure shops is a way to form a common language. If I start having my custom map specs, and so does everyone, I'd be tempted to say something core is missing. Strict map specs which also vallidates each key has a registerer spec I think is a glaring omission.

I'm not sure how to say this other than that we are playing a longer game than you, looking out into the future to other potential functionality and where code bases are likely to go over time. 

We have already seen a number of cases where strict map specs broke during evolution both inside and outside Cognitect projects as people added keys. Again, nothing prevents you from both validating your spec AND applying whatever additional validation you wish to perform. But if you put the strict check into the spec, you have poured concrete on the spec and it is dead.

 

Having used spec in one of my design, I've had to justify already to 6 people, some being senior security engineers, why the validation allows for open keys.

Those other people are right - you should check that if it's important. But it doesn't have to be part of the spec. Specs should say what must be true (open), not what MUSTN'T be true (closed). This is in some ways another variant of the open/closed principle (the O in SOLID). 

 

Other team members were just confused as to why that was, and the only argument I had was that Rich doesn't like to break APIs :p.

This is an exceptionally good argument. Why would you dismiss it?

 

But I've had to add validation on top to pass security audit.

Good!

 

So I think while not breaking APIs when incrementally adding specs to legacy code is a good use case, there's the security and safety use case which seems to be shared by a large swat of Clijurist, and I think spec is in need of a core support for it.

No, it doesn't! Spec doesn't have to do everything. We've got this whole beautiful language to use.

 

[Tommi Reiman]

Open Specs seem like a good idea.

In real life, we need to close our Data at system borders.

With Java, there is Jackson, which fails by default on extra keys. The Schema, the models are closed by default. JSON Schemas can be closed.

Saying "use normal clojure" on top of Spec mean double-validation, which is both error-prone and boilerplate.

Open/Closed is a great principle.

Spec should be just more Open to allow people (or 3rd party libs) to extend it to support "close my (open) specs" at runtime = coercion.

If Runtime Transformations stay out of scope, It would be good to have a guide on how to write Spec + (beautiful) clojure transformations for web/ring apps. There might be tricks we just haven't thought of.

cheers,

Tommi

[Didier]

I'm loving Spec as an FYI, and I'll use it even if it stays as is, even in its alpha state it is a great tool. I also highly value the Clojure core value to avoid breakage, as the cost to enterprise of every migration is very high.

I'm just wanting to give more data points to the Core team that I hope would be taken into consideration in the decisions of where to put the line on what Spec provides and doesn't.

In my case, Spec has already not gone where my code base has. I'm sure there's use cases out there where a closed map spec would have been problematic, so I'm glad spec has an open map spec, but I've never had this problem, and I've failed to see anyone describe it to me where I felt the problem was real. What I do know is that I've experienced the problem of an open map spec that others have clearly described in this thread. When I need to validate for security that no extra keys were added to my maps, and when I want to be sure, for correctness, that my instrumentation is not accidentally not asserting my input because of a missing key spec, or if I want to be reminded that I forgot to update the spec when I extended the map to have additional keys, then clojure.spec falls short.

I wouldn't ask of this to be taken into account by the core spec team, if it was easy to extend Spec to support this, but from what I could tell from the source, it is non trivial for me to add my own strict s/keys, and it doesn't seem possible to easily extend s/keys itself with more features. Especially if I want all the features of s/keys, except with strict key spec validation.

I trust the core team to have good judgement, as they've shown to have with much of Clojure till date, but I think this is valuable data points for them to discuss and decide what's best. This was also part of a real enterprise project, that took 4 months with 5 engineers to develop. We're using it in production, so this is not feedback out of a toy project, or just experimentation.

Maybe there's a better way to solve this problem then a strict spec, maybe we need a strict s/validate?, or some other more powerful and flexible construct. And maybe there are easy to extend spec for my needs, then I'd love to learn about those.

Anyways, for the purpose of gathering data points about this use case. It currently seems like Me, Yury, Leon, Puzzler, and Tommi have all faced this issue. I've fixed it by doing my own validation on top of spec. So I only use spec as a data description language, and not as a validation tool. I use s/describe to get the keys, and then I resolve the specs, and assert that no more keys are on the map then what I got from s/describe. The downside is that I can't use instrumentation, assert, validate?, etc. So its hard to guarantee that the spec is always validated as my data should. The spec data description language is not powerful enough to be able to model invariants such as a closed set of keys, or keys which must have associated specs, unless I implement that predicate spec myself, which I couldn't figure out how to do.

Since spec is still alpha, I think this is a great time to bring up these issues.

[Alan Thompson]

While it doesn't use spec, you can do this using the validate-map-keys helper function in the Tupelo library.  It verify that a map does not contain any keys other than those you specify.  The unit test shows it in action:

(ns tst.demo.core

  (:use demo.core tupelo.test )

  (:require

    [tupelo.core :as t] ))

(t/refer-tupelo)

(dotest

  (let [map-ab   {:a 1 :b 2}

        map-abc  {:a 1 :b 2 :c 3}]

    (is= map-ab  (validate-map-keys map-ab [:a :b]))

    (is= map-ab  (validate-map-keys map-ab [:a :b :x]))

    (is= map-ab  (validate-map-keys map-ab #{:a :b}))

    (is= map-ab  (validate-map-keys map-ab #{:a :b :x}))

    (is= map-abc (validate-map-keys map-abc [:a :b :c :x]))

    (is (thrown? IllegalArgumentException (validate-map-keys map-ab [:a])))

    (is (thrown? IllegalArgumentException (validate-map-keys map-ab [:b])))

    (is (thrown? IllegalArgumentException (validate-map-keys map-ab [:a :x])))

    (is (thrown? IllegalArgumentException (validate-map-keys map-abc [:a :b])))

    (is (thrown? IllegalArgumentException (validate-map-keys map-abc [:a :c :x])))))

The API docs are hosted on Github Pages, but it seems to be down at the moment.  You can find some older API docs (May 2017) on CrossClj:  https://crossclj.info/doc/tupelo/0.9.1/index.html

Alan

--

You received this message because you are subscribed to the Google
Groups "Clojure" group.
To post to this group, send email to clo...@googlegroups.com
Note that posts from new members are moderated - please be patient with your first post.
To unsubscribe from this group, send email to

clojure+unsubscribe@googlegroups.com

For more options, visit this group at
http://groups.google.com/group/clojure?hl=en
---
You received this message because you are subscribed to the Google Groups "Clojure" group.

To unsubscribe from this group and stop receiving emails from it, send an email to clojure+unsubscribe@googlegroups.com.

[Moritz Ulrich]

Yuri Govorushchenko <yuri....@gmail.com> writes:

> Thank you the pointers! So far I ended up with writing a small `map` macro
> which is similar to `s/keys` but checks that keys are already in the
> registry: https://gist.github.com/metametadata/5f600e20e0e9b0ce6bce146c6db429e2

Note that you can simply combine a custom predicate and `s/keys` in
clojure.spec to verify that all keys in a given map have a underlying
spec:

```
(s/def ::whatever (s/and (s/keys ...)
#(every? keyword? (keys %))
#(every? (comp boolean s/get-spec) (keys %)) )
```

[Yuri Govorushchenko]

Thanks. This approach is also different from the macro because it will check specs existence at the validation time, not at the s/def call.

[Beau Fabry]

Seems like that's the reasonable place to check it, otherwise you're forced into an ordering for your specs and cannot write recursive strict map specs.

[Leon Grapenthin]

Open maps/specs are fine.

s/keys supporting unregistered specs are not.

At least to me. I just fixed two more bugs in production that were would not have happened.

What are the supposed benefits of this feature? 

I can only infer "being able to require keys without their spec being known" which is a usecase I had exactly 0.00% of the time so far.

Anything I have missed?

Kind regards,

 Leon.

[Beau Fabry]

Leon, perhaps you could add this code to your test suite?

boot.user=> (let [kws (atom #{})]

       #_=>   (clojure.walk/postwalk (fn [x] (when (qualified-keyword? x) (swap! kws conj x)) x) (map s/form (vals (s/registry)))) (clojure.set/difference @kws (set (keys (s/registry))))

       #_=> )

#{:clojure.spec.alpha/v :clojure.spec.alpha/k}

boot.user=>

[Leon Grapenthin]

Thanks, Beau. 

I am still interested why this default behavior has been chosen. It doesn't seem like a reasonable trade-off at this point.

It enables me to say: "The map must have this key", without specifying how the data mapped to it will look like.

If I ever wanted to do that, I could as well spec that key with "any?".

What are other benefits? They must justify the expense of likely runtime errors.

[Beau Fabry]

A use case that comes to mind is a system/library that specifies the structure of some inputs/outputs, but lets users/consumers (optionally) specify further validation of the leaves. I suppose that would be possible with (s/def ::foo any?) but you'd have to be a bit more careful about load order. The other use case (which is mine) is I'm just lazy and only want to write out broad strokes specs sometimes without getting into the nitty gritty.

If s/keys were to validate that the keys it's provided have specs it would have to do it at validation time, so you wouldn't get the error until something was actually validated against that key spec. Trying to do it at definition time would break recursive specs.

As one of the (apparently pretty uncommon) users who actually does happily define s/keys specs without correspondingly speccing the leaves as an "incrementally lock down/validate" approach, I wouldn't be too upset if I lost that ability and it started throwing an error. I mean it throws an error if I go to generate it anyway.

[Sean Corfield]

As one of the (apparently pretty uncommon) users who actually does happily define s/keys specs without correspondingly speccing the leaves as an "incrementally lock down/validate" approach, I wouldn't be too upset if I lost that ability and it started throwing an error. I mean it throws an error if I go to generate it anyway.

 

*puts hand up!*

 

I don’t want to have to write (s/def ::some-key any?) all over the place as I’m developing specs, just to satisfy an overly eager checker (in my mind). Worse, since the check would need to be deferred until validation time, as Beau notes, the omission of an “any?” key spec might not even show up until much further down the line.

 

To me, this default behavior of silently not checking the _value_ associated with a _key_ is in keeping with the design principles of spec which focus on maps being based on a key set, while offering functions to allow you to optionally check values.

 

Sean Corfield -- (970) FOR-SEAN -- (904) 302-SEAN
An Architect's View -- http://corfield.org/

"If you're not annoying somebody, you're not really alive."
-- Margaret Atwood

 

---

From: clo...@googlegroups.com <clo...@googlegroups.com> on behalf of Beau Fabry <imf...@gmail.com>
Sent: Friday, October 6, 2017 9:10:36 AM
To: Clojure
Subject: Re: [core.spec] Stricter map validations?

 

--
You received this message because you are subscribed to the Google
Groups "Clojure" group.
To post to this group, send email to clo...@googlegroups.com
Note that posts from new members are moderated - please be patient with your first post.
To unsubscribe from this group, send email to

clojure+u...@googlegroups.com

For more options, visit this group at
http://groups.google.com/group/clojure?hl=en
---
You received this message because you are subscribed to the Google Groups "Clojure" group.

To unsubscribe from this group and stop receiving emails from it, send an email to clojure+u...@googlegroups.com.

[Leon Grapenthin]

The argument that existence of specs provided to s/keys can only be checked at runtime is false.

The argument that that recursive specs are impossible if existence of specs provided to s/keys was checked at compile time is also false. 

The usecase for libraries is not convincing: If the libraries author states "the map has to have a key K" nobody can spec K further since that would be a race condition among consumers (who s/defs K first?). Requiring the libraries author to declare K as any? would at least require him to decide and convey his intent.

The argument that not checking a value associated with a key is corresponding to a guding design principle of map specs being based on a keyset is not stating enough to justify discussed behavior. The utility of knowing that a keyset is present is close to none, which should be the main reasons why s/keys validates values. Again: Saying "A map that has a key called ::foo" is pretty pointless in Clojure. If every map in every Clojure program I wrote had a key ::foo they would all produce the exact same results as if they didn't and I bet yours would, too. 

Prototyping is indeed a bit more easy if one does not have to to declare every spec used in a s/keys. However, that is particularly damning if you forget to add that spec later or mistype its name when doing so. Which happens, and which is why I'm unhappy with this design letting such typical human errors pass compilation. It would also help my prototyping needs if I could reference symbols that are not declared, but I prefer the compiler errors before going live. 

[Beau Fabry]

> The argument that existence of specs provided to s/keys can only be checked at runtime is false.

> The argument that that recursive specs are impossible if existence of specs provided to s/keys was checked at compile time is also false. 

Could you explain to us why this is false? Clojure is a dynamic language, as such I don't see how you could define a time when all specs need to be present. How would I enter this spec at the repl if spec definition was required at s/keys invocation time?

boot.user=> (s/def ::val string?)

:boot.user/val

boot.user=> (s/def ::tree (s/keys :opt-un [::val ::branches]))

:boot.user/tree

boot.user=> (s/def ::branches (s/tuple ::tree ::tree))

:boot.user/branches

[Leon Grapenthin]

In terms of code loading, acyclic dependencies turned out to be a great design choice in Clojure - why its benefits shouldn't apply to or be justified for spec loading is totally unclear to me.

To make my point more clear let me recap. I am simply asking for s/keys to throw if provided specs aren't registered. Because my colleagues and I myself made costly mistakes that would have been prevented. The most common scenario is a typo like the one I have illustrated above.

I have asked what benefits justify current behavior?

The only justification comes from Sean saying that it helps him prototyping. While I agree I also observe that this is simultaneously the trapdoor leading to such silently passing specs. And why prototyping needs should not be a primary concern in how s/keys behaves.

I have tried to make a case for current behavior: It allows to say a key is there, without saying anything about its value. I have pointed out (s. a.) why this IMO has too little utility to justify anything.

Regarding Clojure being a dynamic lanugage this doesn't really make a difference here: There is not much dynamic going on about registration and spec in general. Registration etc. is evaluated at compile time.  Note that s/def, s/keys etc. are all macros whose expansion is evaluated at compile time.

On Monday, October 9, 2017 at 7:20:42 PM UTC+2, Beau Fabry wrote:

> The argument that existence of specs provided to s/keys can only be checked at runtime is false.

> The argument that that recursive specs are impossible if existence of specs provided to s/keys was checked at compile time is also false. 

Could you explain to us why this is false? Clojure is a dynamic language, as such I don't see how you could define a time when all specs need to be present. How would I enter this spec at the repl if spec definition was required at s/keys invocation time?

 

[Stuart Halloway]

Hi Leon,

I think it would be a mistake to introduce temporal coupling to prevent typos. Here is an alternative that lets you identify "missing" keys specs at

the time and place of your choosing, and then handle them as you deem appropriate, without imposing those decisions on other users of spec:

https://gist.github.com/stuarthalloway/f4c4297d344651c99827769e1c3d34e9

Regards,

Stu

clojure+unsubscribe@googlegroups.com

For more options, visit this group at
http://groups.google.com/group/clojure?hl=en
---
You received this message because you are subscribed to the Google Groups "Clojure" group.

To unsubscribe from this group and stop receiving emails from it, send an email to clojure+unsubscribe@googlegroups.com.

[Didier]

I just stumbled upon another potential mistake here. When you have specs split across namespaces. Its possible for a map spec in one namespace to have one of its key's use a spec from another namespace. If you forget to require that other namespace though, you won't know, and your map will always validate.

(s/def ::spec (s/keys :req [:other/spec]))

If :other/spec is not registered, ::spec still will succeed at being registered. And then, assuming :other/spec is defined as:

(s/def :other/spec int?)

I wouldn't matter, since:

(s/valid? ::spec {:other/spec "123"})

Will return true.

But if you register :other/spec, it would return false.

Normally, this has not been an issue for me, but now that I share my specs more, I've got specs in different namespace using one another, and I've already made this mistakes a few time, causing validation I thought was there to protect me, to actually be missing.

So I made my own keys macro:

(defmacro known-keys

  [& {:keys [req req-un opt opt-un gen] :as args}]

  (letfn [(known-spec? [k] (boolean (s/get-spec k)))]

    (doseq [e (concat req req-un opt opt-un)]

      (when (not (known-spec? e))

        (throw (ex-info (str e " is not a currently registered spec.") args)))))

  `(s/keys ~@(interleave (keys args) (vals args))))

Which first checks that all keys are currently registered, and if so, it delegates back to s/keys. Otherwise it throws an exception at macro expansion time.

I think this would also solve OPs problem, since it would throw if typos are made also.

[Yuri Govorushchenko]

I too ended up using a custom macro instead of `s/keys` everywhere. I posted my solution somewhere above: https://gist.github.com/metametadata/5f600e20e0e9b0ce6bce146c6db429e2
I didn't know it's possible to check if keys are registered at macro expansion time, will try this out. Thanks Didier.

[Beau Fabry]

One way to prevent that would be to only ever use aliases for fully qualified keywords, never type out the whole thing. 

[Didier]

One way to prevent that would be to only ever use aliases for fully qualified keywords, never type out the whole thing.

Uh, you forced me to go learn more about keywords. I didn't know you could do ::namespace/name to alias keywords.

I've been manually namespacing my keywords until now, which was tedious and error prone. Also, because I was doing it manually, I made my namespace real short, as its annoying to have to type :com.company.project.wtv/name everywhere.

Thanks for the tip.

I do feel that for this though, it asks for a bit too much discipline on the programmer, and in this case, also a good amount of knowledge around spec and keywords, so I might still use the macro, but also switch to aliased keywords.

[Eric Normand]

Hey everybody!

I'm chiming in after seeing this linked to in The Repl (https://therepl.net/).

On Alex's suggestion, I rewatched Spec-ulation last night. The parts about negation and evolution are towards the end. I was struck (once again) by how clearly he picked apart changes. Relaxing a requirement is growth. And adding requirements is breakage. But it left me with a question:

Isn't disallowing a key and then allowing it (as optional) growth (instead of breakage)? All of the old clients are still fine, and new clients can use the key if they choose. You're relaxing the requirements. Taking the opposite approach, I require some keys plus allow anything else. Some clients will inevitably send me something with extra keys, which is okay, they pass my specs. Later, I add in an optional key with a defined spec. So I'm now restricting what used to be completely open. Isn't that breakage? I feel like I'm seeing it exactly opposite as Rich Hickey. He says if you disallow things, it's forever, because if you need to allow it later, that's breakage. But there's not enough explanation for me to understand. It seems like relaxing requirements. I feel like I'm missing something. In short: why is it forever?

He does mention is that logic engines don't have negation. Does this hint that we will want to be using logic engines to reason over our specs?

Thanks

Eric

[Seth Verrinder]

I took part of the goal to be that specs themselves would remain compatible, so an old set of specs wouldn't start failing on data that conforms to a new but compatible set of specs. That sort of compatibility isn't possible when you go from disallowing something to allowing it.

[Eric Normand]

Oh, I see! That makes sense. Thanks!

[Sean Corfield]

Eric does raise an interesting question tho’:

 

If you have an API that cares about ‘a’, ‘b’, and ‘c’ and you later specify that ‘d’ is optional and should be an ‘int?’, does that qualify as breakage or growth? If clients were sending ‘d’ as a string before but you ignored it, it will break those clients. Clients that were not sending ‘d’ will not be affected by the change. The old spec – allowing ‘d’ to be ‘any?’ essentially – won’t fail on any data that omits ‘d’ or passes it as ‘int?’ so it passes your compatibility test.

 

(we actually ran into this at work because a client app was passing a field we didn’t care about and we later decided that was an optional field but couldn’t be an empty string and it broke that client)

 

Sean Corfield -- (970) FOR-SEAN -- (904) 302-SEAN
An Architect's View -- http://corfield.org/

"If you're not annoying somebody, you're not really alive."
-- Margaret Atwood

 

---

From: clo...@googlegroups.com <clo...@googlegroups.com> on behalf of Seth Verrinder <set...@gmail.com>
Sent: Tuesday, November 14, 2017 8:45:30 AM

To: Clojure
Subject: Re: [core.spec] Stricter map validations?

--

[Eric Normand]

Yeah, it seems like if s/keys specs add any keys (required or not) that have specs defined, you're breaking things. Removing the spec definition and removing the key is growth. This explains why people would want things to be stricter at first because it would allow them to relax later.

The idea that specs have to be future-compatible as well as backwards compatible is interesting but I haven't ever made that a goal of my systems. Old specs have to work on new data and new specs have to work on old data. Has this been explored somewhere? I'd love to read up on it.

I'm curious: are there any resources laying out these design principles of spec and how it means we should be designing systems that use it? There are a ton of things on the mechanics of it and how to do X, Y and Z with it. But I haven't seen anything that talks about evolution on long timescales. Can someone point me to some resources?

clojure+unsubscribe@googlegroups.com

For more options, visit this group at
http://groups.google.com/group/clojure?hl=en
---

You received this message because you are subscribed to the Google Groups "Clojure" group.

To unsubscribe from this group and stop receiving emails from it, send an email to clojure+unsubscribe@googlegroups.com.

For more options, visit https://groups.google.com/d/optout.

--

You received this message because you are subscribed to the Google
Groups "Clojure" group.
To post to this group, send email to clo...@googlegroups.com
Note that posts from new members are moderated - please be patient with your first post.
To unsubscribe from this group, send email to

clojure+unsubscribe@googlegroups.com

For more options, visit this group at
http://groups.google.com/group/clojure?hl=en
---

You received this message because you are subscribed to a topic in the Google Groups "Clojure" group.
To unsubscribe from this topic, visit https://groups.google.com/d/topic/clojure/i8Rz-AnCoa8/unsubscribe.
To unsubscribe from this group and all its topics, send an email to clojure+unsubscribe@googlegroups.com.

[Robin Heggelund Hansen]

Isn't this precisely why you should use namespaced keywords?

[Didier]

Eric does raise an interesting question tho 

I think so too. I'm still finding it hard to come up with a single example of why allowing extra keys to validate would ever be useful even for non-breakage. I can't see how it would break anything.

I believe the only advantage of allowing extra keys, is to allow partial specs. So say you never had specs, and your API takes a ton of keys, and you start by only speccing half the keys it takes. That's the only use case I could think of.

Personally I also wouldn't consider a client who sends a key that is not used, to be broken if I now validate that he is not supposed to send it. I did not break the API, I'm only making him realize he might have been wrong on how the API worked since the beginning, if he thought somehow that this key was a valid input and maybe expected it to alter the behavior. But in terms of semantics and behavior, nothing has changes or been broken on my API.

[Eric Normand]

I think so too. I'm still finding it hard to come up with a single example of why allowing extra keys to validate would ever be useful even for non-breakage. I can't see how it would break anything.

I think you're assuming you're validating API endpoints where client and server are tightly coupled. I can imagine a microservices system where some services put maps on a queue and others consume them. The consumers should allow keys they don't understand. They and also need to validate that the keys they need are there and of the right shape.

 

 

clojure+unsubscribe@googlegroups.com

For more options, visit this group at
http://groups.google.com/group/clojure?hl=en
---

You received this message because you are subscribed to a topic in the Google Groups "Clojure" group.
To unsubscribe from this topic, visit https://groups.google.com/d/topic/clojure/i8Rz-AnCoa8/unsubscribe.
To unsubscribe from this group and all its topics, send an email to clojure+unsubscribe@googlegroups.com.

[Eric Normand]

If you have an API that cares about ‘a’, ‘b’, and ‘c’ and you later specify that ‘d’ is optional and should be an ‘int?’, does that qualify as breakage or growth? If clients were sending ‘d’ as a string before but you ignored it, it will break those clients. Clients that were not sending ‘d’ will not be affected by the change. The old spec – allowing ‘d’ to be ‘any?’ essentially – won’t fail on any data that omits ‘d’ or passes it as ‘int?’ so it passes your compatibility test.

Isn't this precisely why you should use namespaced keywords?

Yes! Great point.

That sounds like a great thing to document in your API. Something like: "all keys in com.xyz namespace are reserved and could be restricted at any time. Please don't use them." You could send a warning back in your API if they use any that don't validate.

Are there any resources that document these best practices? It seems like Clojure is trying to push us down this road but there's no map yet at the moment.

 

tirsdag 14. november 2017 19.43.55 UTC+1 skrev Sean Corfield følgende:

Eric does raise an interesting question tho’:

 

 

(we actually ran into this at work because a client app was passing a field we didn’t care about and we later decided that was an optional field but couldn’t be an empty string and it broke that client)

clojure+unsubscribe@googlegroups.com

For more options, visit this group at
http://groups.google.com/group/clojure?hl=en
---

You received this message because you are subscribed to a topic in the Google Groups "Clojure" group.
To unsubscribe from this topic, visit https://groups.google.com/d/topic/clojure/i8Rz-AnCoa8/unsubscribe.
To unsubscribe from this group and all its topics, send an email to clojure+unsubscribe@googlegroups.com.

[Daniel Compton]

> I think so too. I'm still finding it hard to come up with a single example of why allowing extra keys to validate would ever be useful even for non-breakage. I can't see how it would break anything.

> 

> I believe the only advantage of allowing extra keys, is to allow partial specs. So say you never had specs, and your API takes a ton of keys, and you start by only speccing half the keys it takes. That's the only use case I could think of.

This is my core concern as well. It seems like it is going to lead to bugs from typos or people expecting specs to have been defined for keywords that weren't defined. It also is incongruous with other parts of spec like s/cat, which does require specs to be registered at check time. I understand there is a technical/implementation detail for the reasons here, but it strikes me as a rough edge that people are likely to continually run into. I've hit it myself already in projects where I spec'ed a map, but didn't have some of the keys spec'ed. I discovered later my mistake, but like Didier, I can't see any time that I would actually want this feature. 

Because spec's information model is all about namespaced keywords that have a unique unambiguous meaning (at least I think that's the idea?), allowing specing of a map with keywords that don't exist because they may be defined by some other part of the system doesn't make a lot of sense to me. Even if this is a valid use case, it seems like a niche one that would be better served by defining a new function that allows it (perhaps in another library), rather than it being part of the behaviour of the more general purpose s/keys.

On Wed, Nov 15, 2017 at 8:53 AM Eric Normand <ericwn...@gmail.com> wrote:

If you have an API that cares about ‘a’, ‘b’, and ‘c’ and you later specify that ‘d’ is optional and should be an ‘int?’, does that qualify as breakage or growth? If clients were sending ‘d’ as a string before but you ignored it, it will break those clients. Clients that were not sending ‘d’ will not be affected by the change. The old spec – allowing ‘d’ to be ‘any?’ essentially – won’t fail on any data that omits ‘d’ or passes it as ‘int?’ so it passes your compatibility test.

Isn't this precisely why you should use namespaced keywords?

Yes! Great point.

That sounds like a great thing to document in your API. Something like: "all keys in com.xyz namespace are reserved and could be restricted at any time. Please don't use them." You could send a warning back in your API if they use any that don't validate.

Are there any resources that document these best practices? It seems like Clojure is trying to push us down this road but there's no map yet at the moment.

 

tirsdag 14. november 2017 19.43.55 UTC+1 skrev Sean Corfield følgende:

Eric does raise an interesting question tho’:

 

 

(we actually ran into this at work because a client app was passing a field we didn’t care about and we later decided that was an optional field but couldn’t be an empty string and it broke that client)

 

Sean Corfield -- (970) FOR-SEAN -- (904) 302-SEAN
An Architect's View -- http://corfield.org/

"If you're not annoying somebody, you're not really alive."
-- Margaret Atwood

 

From: clo...@googlegroups.com <clo...@googlegroups.com> on behalf of Seth Verrinder <set...@gmail.com>

You received this message because you are subscribed to a topic in the Google Groups "Clojure" group.

To unsubscribe from this topic, visit https://groups.google.com/d/topic/clojure/i8Rz-AnCoa8/unsubscribe.

To unsubscribe from this group and all its topics, send an email to clojure+u...@googlegroups.com.

[Didier]

| I think you're assuming you're validating API endpoints where client and server are tightly coupled. I can imagine a microservices system where some services put maps on a queue and others consume them. The consumers should allow keys they don't understand. They and also need to validate that the keys they need are there and of the right shape.

That use case sounds more like the exception then the rule to me. I still think its valid, and for that I recognise there should be a way to have a spec which allows any key and requires specific ones.

Now that I've listened to everyone, I see that there might be many things needed by different people, and its not clear if all should be provided by spec directly.

Mostly I can see wanting:
A. To spec a map in which you expect the keys to have a registered spec.
B. To spec a map in which you don't expect the keys to have a registered spec.
1. To spec a map in which the keys should be closed to only the req and opt keys that are defined.
2. To spec a map in which the keys should be open to what is defined, but where certain keys are required and others reserved as optional.

Where we want to be able to mix and match A,B with 1,2.

I see a safe default being A,1. Though I'd be okay with A,2. I think B is the bigger issue not to have as default, and is less intuitive. It's prone to typos and forgetting to add the spec or require the proper namespace, etc.

My conclusion: I think s/keys is a funny part of spec, because compared to all other provided speccing tool, this one is really just syntax sugar for convenience. Yet it makes sense, because it is just so damn convenient. But for a lot of people like me, its convenient in a surprising way, which leans towards compatibility instead of correctness and security. And most baffling, trying to spec a map in a correct, secure and convenient way is not possible, at least not with the same convenience. So I'd say if there was just also convenient s/closed-keys and s/specified-keys and s/specified-closed-keys everyone would be happy, and it would also be more explicit what distinction exist between all 4. I'd probably always reach first for s/specified-closed-keys, and relax if I need too only, but all least everyone would have what they want and could chose for themselves.

[Greg Mitchell]

Saw this thread on the REPL, interesting discussion.

On Tuesday, October 3, 2017 at 10:57:34 AM UTC-7, Alex Miller wrote:

It's not about easier, it's about possible. Open grows, closed breaks.

  

I agree with the broad brush, but it's important to listen to this usability feedback before releasing an important feature like Spec. As an aside, seriously great work with Spec; it's the biggest thing Clojure was missing for institutional support I think. 

At Google, we use Protocol Buffers heavily to achieve some similar goals to Spec, and I think there are good lessons to look at. Like you said, the spec (whether proto or capital-s Spec) needs to be forwards compatible across distributed clients if it's going to describe data in an useful way as the project evolves. Protos achieve that by convention - best practice recommends fields are always optional and the type can never change - and technically by always accepting unknown fields.

Where they differ highlights the pain points that people in this thread mentioned. 

- Protos cannot be created with extra fields, so there's nothing like the sanitizing issue puzzler mentioned. The generalization is to strictly specify the output of a function/system, but gracefully handle input. This makes me think there should be something like a s/conform-strict function that coerces data to only the Spec'd keys, and ensures keys are defined.

- Protos specify the type of a field along with its name, and both are checked at compile time, so there aren't the late-caught errors like the OP pointed out. Rich has good reasoning for wanting to re-use keys, but typically the solution is to create a named composite type (a message in proto terms) that is trivial to re-use as a field type. The idea of specifying keys separately is different from how literally every type system does things, so it's inherently not "easy" in Rich's definition of being mentally at hand. That's going to create pain for people learning the tool. You should strongly think about ways to reduce that friction.

Protos describe a strictly specified open message, which doesn't neatly fit in your categories or open or closed. It's an important technical precedent to think about though.

[Eric Normand]

Wow, this has been a really great discussion. I have only played with spec a little, not used it in production. Reading this and participating has really helped me clarify a lot of things. Thanks!

In particular, I've been thinking a lot about the three things you mentioned, Didier: safety, compatibility, and correctness. I think it's great that you pulled them apart. I've written a number of public-facing APIs in my day, and what I always wound up doing was being really strict with the clients because it actually helps correctness. It helps give really fast feedback to third parties writing client code because they get error messages about everything that's wrong with their request. Anything that's silently accepted could mask a bigger problem and, especially with third-party clients, you want to get those problems out in the open as soon as possible.

But the more I look at the behavior of s/keys, the more I'm starting to appreciate it. It's not trying to be strict to educate clients. It's for asking "does this have the minimum I need to handle this map?" And this is very much how idiomatic Clojure is written. We ignore keys we don't care about and pass them along. Or we select only the keys we care about and operate on those. I wonder why we don't want to do this with our APIs. It's so comfortable and practical internally.

Now, the educational value of strictness (as developers write clients for our API) is important. But it doesn't seem to be in the purview of spec. I've been thinking about a few guidelines, because I haven't seen them written down:

1. Use namespaced keywords with globally unique namespaces that you own.

2. Don't "invent" keys in a namespace you don't own.

3. Validate the keys you care about, to ensure that you have the minimum you need to continue (using a s/keys spec) with appropriate types.

4. Handle extra keys you can't use in the appropriate way (ignore them and pass them on OR select only the ones you want).

With these, you actually do get a lot of the safety and correctness you're talking about, but it's more of a systemic safety (unexpected input won't break the system) and safety across changes (growth).

There are still two problems that this won't solve. The first is the OP's original problem, which was typos in keys allowing values with bad types to get through a s/valid? check. That's serious but could easily be checked with Stuart's code, using what Spec provides. So I think that's a real solution provided by Spec. You could check in a test, for instance.

The second problem is that education problem: you want clients to know when they send typos, not just silently ignore them. I think this is important. We've already got s/valid?, which asks "can I handle this?". Maybe you could implement something called strict? that takes a spec and a value and makes sure there aren't any extra keys. It doesn't seem to belong in the spec. To me, it should go in a function which you could deploy when you need it.

Rock on!

Eric

[Sean Corfield]

Personally I also wouldn't consider a client who sends a key that is not used, to be broken if I now validate that he is not supposed to send it. I did not break the API, I'm only making him realize he might have been wrong on how the API worked since the beginning, if he thought somehow that this key was a valid input and maybe expected it to alter the behavior. But in terms of semantics and behavior, nothing has changes or been broken on my API.

 

Yes, I think I agree with this position and don’t consider it a “breakage” – even tho’ it may actually “break” code that was assuming the API would always ignore any additional keys.

 

Sean Corfield -- (970) FOR-SEAN -- (904) 302-SEAN
An Architect's View -- http://corfield.org/

"If you're not annoying somebody, you're not really alive."
-- Margaret Atwood

 

---

From: clo...@googlegroups.com <clo...@googlegroups.com> on behalf of Didier <did...@gmail.com>
Sent: Tuesday, November 14, 2017 11:21:04 AM

[John Newman]

Great conversation! I'm still catching up on Spec, but it seems like there's a correlation here to type systems. Type systems can introduce a certain kind of technical debt - type debt? It seems that leaving maps open here is also attempting to avoid that sort of type debt - parochial maps that don't play nice with others. And when we constrain maps in that closed way, aren't we creating some new subtype of a map, with fundamentally different semantics? If you are going to fully close a map, you might as well use a deftype and make a custom object and not call it a map, right?

[Nico Schneider]

Hello everyone,

On Thursday, 16 November 2017 23:29:56 UTC+1, John Newman wrote:

[...] when we constrain maps in that closed way, aren't we creating some new subtype of a map, with fundamentally different semantics? If you are going to fully close a map, you might as well use a deftype and make a custom object and not call it a map, right?

Just to add my two cents, I've been following the discussion and this has been my thinking for quite some time. Is it not a valid argument? Having a validation mechanism pick only certain keys, or ensuring that keys in a map are specced, look as trivial to me than other data wrangling we do in Clojure. My (preliminary) conclusion in cases like this is to build validation tooling around spec, instead of using it directly.

[Ben Brinckerhoff]

I ran into a case today where I mistyped the keyword in a `keys` spec and was surprised that validation was not catching invalid data. For my purposes, it would be sufficient to have a automated test that looks at my specs and identifies typos in the spirit of https://gist.github.com/stuarthalloway/f4c4297d344651c99827769e1c3d34e9

However, I ran into a problem because my `keys` specs are often generated by the various multi-methods in a multi-spec. I took a stab at implementing a version of a "spec linter" that would catch typos in multi-specs, but I couldn't get anything that worked (at least in Clojurescript, I haven't tried implementing it in Clojure).

Is it possible to write a CLJS linter that will catch the missing spec in the example below? If so, great! If not, I worry that the recommended approach of adding a linter on top of spec may not be sufficient in this case.

 (s/def ::name string?)
 (defmulti msg :msg-type)
 (defmethod msg :greeting [x]
    (s/keys :req-un [::name]))
 (defmethod msg :count [x]
   (s/keys :req-un [::num]))
 (s/def ::msg (s/multi-spec msg :msg-type))

(I suppose it's always going to be possible to write a linter that walks the Clojure code and understands the semantics of the above macros, but I was hoping I could write one using the data contained in the spec registry instead)

Ben

[Andy Fingerhut]

I don't know if this is a fruitful avenue to pursue, but the Eastwood lint tool [1] uses tools.reader to read Clojure code, and the tools.analyzer library to analyze it, while expanding all macros.

If someone can figure out a way to scan through the Abstract Syntax Tree output of tools.analyzer to detect that such a missing spec seems to be occurring, this is the kind of check that fits right in with Eastwood's goals. Very often, such checks simply involve walking the AST looking for certain types of nodes, or sometimes pairs of parent-child node pairs of particular types, and checking certain properties on those, and/or building up maps of info about names or things of interest.  I have never experienced writing a linter where it required understanding all of the possible AST data structures to get the job done.  Most of the time is in learning enough about the AST to experiment with some approaches, and then run your linter on as many live examples of code that might use code that can trigger the warning, to 'tune' it in case it creates warnings in cases  you don't expect it to.

There is currently an issue on Eastwood suggesting using Stuart Halloway's approach to implement a check in Eastwood for what Stuart's code already checks for.  I haven't been spending much time on Eastwood development for the last year or so, but if someone gets the itch to want to look into it, let me know and I may be able to give advice.

Andy

[1] https://github.com/jonase/eastwood
[2] https://github.com/jonase/eastwood/issues/237

--

You received this message because you are subscribed to the Google
Groups "Clojure" group.
To post to this group, send email to clo...@googlegroups.com
Note that posts from new members are moderated - please be patient with your first post.
To unsubscribe from this group, send email to

clojure+unsubscribe@googlegroups.com

For more options, visit this group at
http://groups.google.com/group/clojure?hl=en
---
You received this message because you are subscribed to the Google Groups "Clojure" group.

To unsubscribe from this group and stop receiving emails from it, send an email to clojure+unsubscribe@googlegroups.com.

[Juan Monetta]

I added some stuff on https://gist.github.com/stuarthalloway/f4c4297d344651c99827769e1c3d34e9 that makes your example work I think.

We can use in Clojure and ClojureScript something like:

(defn multi-spec-sub-specs

  "Given a multi-spec form, call its multi method methods to retrieve

  its subspecs in the form of [multi-method-key sub-spec-form]."

  [multi-spec-form]

  (let [[_ multi-method-symbol & _] multi-spec-form]

    (->> (resolve multi-method-symbol)

         deref

         methods 

         (map (fn [[spec-k method]]

                [spec-k (s/form (method nil))])))))

to go down inside multi-specs.

Juan

[Ben Brinckerhoff]

Thanks Juan! This works in Clojure, but it doesn't work for me in Clojurescript due to the use of "resolve"

" Assert failed: Argument to resolve must be a quoted symbol"

This is where I got stuck too.

It looks like spec does have access to the predicate at runtime (after all, it can figure out the right spec when I call 'valid', etc), but there doesn't seem to be a way for a caller to get access to that predicate. If there was, I think this would be solvable.

Andy - yes, you're right, a tool like eastwood could inspect the AST and catch this. Maybe that's the best approach if the spec registry and spec implementations don't expose the information to support a linter like this.