On Thu, 2013-02-07 at 13:52 +0100, mfoj...@redhat.com wrote:
> From: Michal Fojtik <mfoj...@redhat.com>
>
> The way of how we currently create CIMI entities is not
> really nice. We parse the raw JSON/XML and then we are
> trying to create the CIMI entity.
>
> Besides the code looks ugly, there is no way to add validation
> for required attributes without make it even more uglier.
>
> This patch introduce the 'models/actions/' directory where
> we can store the CIMI 'actions' model, like 'MachineTemplateCreate'
> model.
>
> Then in MachineTemplate model, we can leverage the '.from_xml' and
> the '.from_json' methods to create the action model and then use it
> to create the CIMI entity (MachineTemplate in this case)
Definitely an improvement; a couple things:
* I don't think we need a separate subdirectory for these
(especially since the subdir is not reflected in the class
name ;)
* If you set up the MachineTemplateCreate in the server with
MachineTemplateCreate.parse(request.body, request.content_type)
you can get rid of create_from_json/create_from_xml
* I would make create a method on MachineTemplateCreate. Seems
more logical to me
> diff --git a/server/lib/cimi/models/machine_template.rb
> b/server/lib/cimi/models/machine_template.rb
> index 5ba9b3e..c8fa631 100644
> --- a/server/lib/cimi/models/machine_template.rb
> +++ b/server/lib/cimi/models/machine_template.rb
> @@ -52,28 +52,27 @@ class CIMI::Model::MachineTemplate < CIMI::Model::Base
> end
> end
>
> - def create_from_json(body, context)
> - json = JSON.parse(body)
> + def create(template, context)
> new_template = current_db.add_machine_template(
> - :name => json['name'],
> - :description => json['description'],
> - :machine_config => json['machineConfig']['href'],
> - :machine_image => json['machineImage']['href'],
> - :ent_properties => json['properties'] ? json['properties'].to_json :
> {}
> + :name => template.name,
> + :description => template.description,
> + :machine_config => template.machine_config.href,
> + :machine_image => template.machine_image.href,
> + :ent_properties => template.property.to_json
My DB patches add a Entyt#properties which handles
serializing/deserializing into ent_properties. We need to get machine
templates and similar objects to use the same mechanisms.
It would be great if we could drive the copying of attributes to/from
Entity and model objects off some introspection (e.g., sync all
attributes that Entity and the model object have in common, except for
id)
> @@ -92,7 +91,10 @@ class CIMI::Model::MachineTemplate < CIMI::Model::Base
> :property => (model.ent_properties ?
> JSON::parse(model.ent_properties) : nil),
> :created => Time.parse(model.created_at.to_s).xmlschema,
> :operations => [
> - { :href => context.destroy_machine_template_url(model.id), :rel =>
> 'http://schemas.dmtf.org/cimi/1/action/delete' }
> + {
> + :href => context.destroy_machine_template_url(model.id),
> + :rel => 'http://schemas.dmtf.org/cimi/1/action/delete'
> + }
This hunk is only formatting changes, right ?
David
