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

IndexOutOfBoundsException when validating using validation set in 0.2.2 - Update documentation #2

Closed
solussd opened this issue Mar 11, 2013 · 7 comments

Comments

@solussd
Copy link

solussd commented Mar 11, 2013

(bouncer.validators/defvalidatorset testvalidatorset 
  :name bouncer.validators/required)

(bouncer.core/validate {} testvalidatorset)

;;IndexOutOfBoundsException   clojure.lang.PersistentVector.assocN (PersistentVector.java:137)

with version 0.2.2. Documentation should probably note that this does not work with the latest release.

@theleoborges
Copy link
Owner

Hi @solussd ,

Thanks for the heads up. I haven't been able to replicate the issue using 0.2.3-SNAPSHOT.

Would you be able to give it a go with this version? It includes a couple of new features you can read about in the changelog. I plan on releasing 0.2.3 final by the end of the week so any help fail-proofing this version would be appreciated.

For reference this is my repl interaction with your example:

;; Clojure 1.5.0

user=> (bouncer.validators/defvalidatorset testvalidatorset 
  #_=>   :name bouncer.validators/required)

#'user/testvalidatorset

user=> (bouncer.core/validate {} testvalidatorset)
[{:name ("name must be present")} {:bouncer.core/errors {:name ("name must be present")}}]

Once again thanks for the report. If for some reason 0.2.3 takes longer, I'll release a bug fix for 0.2.2.

Cheers,

@solussd
Copy link
Author

solussd commented Mar 12, 2013

Sounds good.

So I've been using your library for a day now and there is the case of 'generated' error messages, e.g. from some other validation, isn't covered. I made some small modifications to the bouncer.core/wrap function that checks if the predicate is returning a vector of the form [true, nil] or [false, errors]. If the latter is the case it appends 'errors' to the message. I have some code from another library that is generating error messages when it does its own validation of some data and I'd like to capture those error messages, in addition to whether or not the other library's validation was successful or not. Thoughts?


Joseph Smith
[email protected]
@solussd

On Mar 11, 2013, at 6:46 PM, Leonardo Borges [email protected] wrote:

Hi @solussd ,

Thanks for the heads up. I haven't been able to replicate the issue using 0.2.3-SNAPSHOT.

Would you be able to give it a go with this version? It includes a couple of new features you can read about in the changelog. I plan on releasing 0.2.3 final by the end of the week so any help fail-proofing this version would be appreciated.

For reference this is my repl interaction with your example:

;; Clojure 1.5.0

user=> (bouncer.validators/defvalidatorset testvalidatorset
#_=> :name bouncer.validators/required)

#'user/testvalidatorset

user=> (bouncer.core/validate {} testvalidatorset)
[{:name ("name must be present")} {:bouncer.core/errors {:name ("name must be present")}}]
Once again thanks for the report. If for some reason 0.2.3 takes longer, I'll release a bug fix for 0.2.2.

Cheers,


Reply to this email directly or view it on GitHub.

@theleoborges
Copy link
Owner

My initial feeling on this is that bouncer shouldn't be trying to accommodate external validation message. That's a job best left to the developer. Since simple data structures are returned, it should be trivial to write a function that combines multiple sources - I just don't think that should be in core.

I'm happy to be proved wrong though so if you could share a snippet where you use this other library and the format in which errors are returned from it I can have a look and have a better understanding in order to make a call on this.

@solussd
Copy link
Author

solussd commented Mar 15, 2013

I guess the issue I'm having is there currently is no way to set the validation 'error message' at validation time, regardless of how I wrap the third-party validation, if it returns an error string to me I can't pass it to Bouncer. I can think of scenarios where one might want to have the validation predicate generate a meaningful error message itself.

At the moment I have some fairly elaborate validation code that returns a vector of the form [result, error_messages], e.g. [false, ["error one", "error two"]] or [true, nil].

My workaround for now was to modify the bouncer.core/wrap function to look like this:

(defn wrap
  "docstring removed for brevity"
  [acc [pred k & args]]
  (let [pred (h/resolve-or-same pred)
        k (if (vector? k) k [k])
        error-path (cons ::errors k)
        {:keys [default-message-format optional]} (meta pred)
        [args opts] (split-with (complement keyword?) args)
        {:keys [message] :or {message default-message-format}} (apply hash-map opts)
        pred-subject (get-in acc k)]
    (if (or (and optional (nil? pred-subject))
            (not (empty? (get-in acc error-path))))
      acc
      (let [pred-result (apply pred pred-subject args)]
        (let [[result errors] (if (vector? pred-result) pred-result [pred-result])]
          (if result
            acc
            (update-in acc error-path
                 #(conj % (format (if errors (str message ": " errors) message) (name (peek k)))))))))))

@theleoborges
Copy link
Owner

Hey, really sorry for taking so long to comment here.

I looked at your code and it's a fair change - thought it's specific to your use case and probably shouldn't be in bouncer.core.

Thus , I'll be closing this issue. Feel free to open a new one if you feel strongly about this. Happy to take another look since you'd probably be coming from a different light 4 months later. Also I have released beta4 which includes a bug fix for issue #7 - hopefully you haven't run into it though.

@solussd
Copy link
Author

solussd commented Jul 17, 2013

No problem- love the library and getting good use out of it.

I have a couple issues I'll open bugs for soon, but nothing I haven't been able to work around.

@theleoborges
Copy link
Owner

Thanks, I'm glad it's useful. And thank you for the bug reports.

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

No branches or pull requests

2 participants