On Thu, 2013-02-07 at 13:52 +0100, [email protected] wrote:
> From: Michal Fojtik <[email protected]>
>
> This addition will make possible to specify the 'required => true'
> option for all attributes in CIMI models, like:
>
> class Machine < Base
> text :name, :required => true
> end
>
> To run a check if the instance of Machine is valid or not you can do
> following:
>
> Machine.from_xml(xml_body).validate!
>
> This method will raise the CIMI::Model::ValidationError exception if
> attribute marked as 'required' have no value set.
Nice; yes, we'll need that and more validation (e.g., that things need
to be valid URL's, numbers, or from a fixed set of values)
We should mimic AR's validation stuff as much as possible. In that vein,
we shouldn't expose a validate! method, just valid? is enough. In fact,
wouldn't it be enough to run validation (1) at the end of parsing (2)
just before serializing internally generated objects ?
Also, I don't think that throwing errors is the right thing to do here -
we just want to store human-readable error messages in the model
objects. If the error is from deserializing a request, we want to give
the frontend a chance to produce a 400 Bad Request that lists all
errors. If the error happens when serializing an object we generated
internally we need to produce a 500 ISE with some indication of what
went wrong (an exception for that is perfectly fine)
> diff --git a/server/lib/cimi/models/base.rb b/server/lib/cimi/models/base.rb
> index ac838a4..998c36d 100644
> --- a/server/lib/cimi/models/base.rb
> +++ b/server/lib/cimi/models/base.rb
> @@ -73,6 +73,24 @@ require_relative '../helpers/database_helper'
>
> module CIMI::Model
>
> + class ValidationError < StandardError
> +
> + def initialize(attribute)
> + @attr_name = attribute
> + super("Required attribute '#{@attr_name}' must be set.")
The error message should at least include either the id of the offending
object or, if there is no id, its class name.
> @@ -150,6 +160,10 @@ class CIMI::Model::Schema
> json
> end
>
> + def valid?(value)
> + @schema.required_attributes.any? { |a| a.valid?(value.send(a.name) )}
You mean all?, right ?
David