Skip to content
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

welcome :string schema #205

Merged
merged 3 commits into from
Jun 8, 2020
Merged

welcome :string schema #205

merged 3 commits into from
Jun 8, 2020

Conversation

ikitommi
Copy link
Member

@ikitommi ikitommi commented Jun 8, 2020

(m/validate :string "")
; => true

(m/validate [:string {:min 1}] "")
; => false

(json-schema/transform [:string {:min 1, :max 4}])
; => {:type "string", :minLength 1, :maxLenght 4}

(mg/sample [:string {:min 1, :max 4}])
; => ("RMmR" "5" "oL" "G7" "ENo" "cAh" "iOb" "jG" "l8" "kuo")

(-> [:map
     [:a :string]
     [:b [:string {:min 1}]]
     [:c [:string {:max 4}]]
     [:d [:string {:min 1, :max 4}]]]
    (m/explain
      {:a 123
       :b ""
       :c "invalid"
       :d ""})
    (me/humanize))
;{:a ["should be string"],
; :b ["should be at least 1 characters"],
; :c ["should be at most 4 characters"],
; :d ["should be between 1 and 4 characters"]}

@pithyless
Copy link
Contributor

Any chance for adding an option along the lines of blank? or present?? Or is that too specific to add to malli-core? It's the first thing I have to add to all my string specs, since you almost never want to allow user input of just blank characters.

@ikitommi
Copy link
Member Author

ikitommi commented Jun 8, 2020

[:string {:min 1}] is effectively a non-blank string.

@pithyless
Copy link
Contributor

I specifically want to avoid situations where user inputs " " to get around {:min 1}. My most common clojure.core spec in any project is (s/and string? (complement str/blank?))

@pithyless
Copy link
Contributor

I use a version of :string that applies str/trim before checking :min and :max, but not sure if that's something that is appropriate to malli.core.

@rschmukler
Copy link
Contributor

Just chiming in that a :trim true could be added, along with the :min and :max options. Potentially also a :trim-l, :trim-r. The only downside with this is there's some nuance of where it ends, one could argue that padding could also be supported with this. Eg. :pad-l "0".

@ikitommi
Copy link
Member Author

ikitommi commented Jun 8, 2020

Thanks for explaining. There are two things here:

  1. validation of trimmed strings
  2. trimming/padding strings

the latter can be done using a (new) transformer, the first would be currently:

[:and :string [:fn (complement str/blank?)]]

agree it's not pretty.

@ikitommi
Copy link
Member Author

ikitommi commented Jun 8, 2020

JSON Schema has :format for strings. Would that be ok here, with value :non-empty?

https://json-schema.org/understanding-json-schema/reference/string.html#built-in-formats

@ikitommi
Copy link
Member Author

ikitommi commented Jun 8, 2020

example trimmer:

(require '[malli.transform :as mt])
(require '[malli.core :as m])

(defn str-transformer []
  (mt/transformer
    {:decoders {:string {:compile (fn [schema _]
                                    (let [{:string/keys [trim]} (m/properties schema)]
                                      (when trim #(cond-> % (string? %) str/trim))))}}}))

(m/decode [:string {:min 1}] "    " (str-transformer))
; => "    "

(m/decode [:string {:string/trim true, :min 1}] "    " (str-transformer))
; => ""

(m/decoder :string (mt/str-transformer))
; => #object[clojure.core$identity] (no-op)

@ikitommi
Copy link
Member Author

ikitommi commented Jun 8, 2020

Happy to take of a tested and lean string-trimmer PR with all the trims, pads etc.

@ikitommi
Copy link
Member Author

ikitommi commented Jun 8, 2020

@pithyless could you write an separate issue of the trim-validated thing? Will merge this now.

@ikitommi ikitommi merged commit 0430463 into master Jun 8, 2020
@miikka
Copy link
Contributor

miikka commented Jun 9, 2020

Post-merge review: the user guide -style example in the README is good, but a reference style documentation that lists the possible options would be also needed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants