Skip to content
This repository has been archived by the owner on Jul 1, 2021. It is now read-only.

Implement KeyAnyValue serialization. Fix integer/long serialization. #36

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Implement KeyAnyValue serialization. Fix integer/long serialization. #36

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
a3f5b13
Implement KeyAnyValue serialization. Fix integer/long serialization.
tacho Mar 31, 2014
bf3606b
Subclass RbVmomi::BasicTypes::Int from Integer, so normal operations …
tacho Mar 31, 2014
0ffae07
Missing method name in Int#respond_to?
tacho Mar 31, 2014
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions lib/rbvmomi/basic_types.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -351,6 +351,16 @@ def initialize x
def to_s
@val.to_s
end

def inspect
@val
end

def == o
@val.to_i == o
end

alias eql? ==
end

class KeyValue
Expand Down
20 changes: 18 additions & 2 deletions lib/rbvmomi/connection.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,21 @@ def obj2xml xml, name, type, is_array, o, attrs={}
expected = type(type)
fail "expected array for '#{name}', got #{o.class.wsdl_name}" if is_array and not (o.is_a? Array or (o.is_a? Hash and expected == BasicTypes::KeyValue))
case o
when VIM::KeyAnyValue
xml.tag! name, attrs do
xml.tag! 'key', o.key.to_s
if o.value.kind_of? Array
types = o.value.map { |e| e.class }.uniq
fail "Expected homogenous array for '#{name}', got '#{types}'" unless types.count == 1
xml.tag! 'value', { 'xsi:type' => "ArrayOf#{types.first}" } do
o.value.each do |v|
xml.tag! v.class.to_s.downcase, v.to_s
end
end
else
obj2xml xml, 'value', BasicTypes::AnyType.wsdl_name, false, o.value, {}
end
end
when Array, BasicTypes::KeyValue
if o.is_a? BasicTypes::KeyValue and expected != BasicTypes::KeyValue
fail "expected #{expected.wsdl_name} for '#{name}', got KeyValue"
Expand Down Expand Up @@ -180,7 +195,7 @@ def obj2xml xml, name, type, is_array, o, attrs={}
when Time
attrs['xsi:type'] = 'xsd:dateTime' if expected == BasicTypes::AnyType
xml.tag! name, o.iso8601, attrs
when BasicTypes::Int
when BasicTypes::Int, Fixnum, Integer
attrs['xsi:type'] = 'xsd:int'
xml.tag! name, o.to_s, attrs
else fail "unexpected object class #{o.class} for '#{name}'"
Expand All @@ -199,7 +214,8 @@ def self.type name
when :anyType then BasicTypes::AnyType
when :boolean then BasicTypes::Boolean
when :string then String
when :int, :long, :short, :byte then Integer
when :long then Integer
when :int, :short, :byte then BasicTypes::Int
when :float, :double then Float
when :dateTime then Time
when :base64Binary then BasicTypes::Binary
Expand Down
4 changes: 3 additions & 1 deletion lib/rbvmomi/deserialization.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ class NewDeserializer
'xsd:byte' => :int,
'xsd:short' => :int,
'xsd:int' => :int,
'xsd:long' => :int,
'xsd:long' => :long,
'xsd:float' => :float,
'xsd:dateTime' => :date,
'PropertyPath' => :string,
Expand Down Expand Up @@ -59,6 +59,8 @@ def deserialize node, type=nil
when :boolean
node.content == '1' || node.content == 'true'
when :int
BasicTypes::Int.new(node.content)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you explain this change? I am worried that existing client code may take offense to seeing this type rather than a regular integer. And I am not sure why it is required. Is it because of serializer changes? If so, I find it quite likely that this breaks something.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure. There were originally 2 issues with using RbVmomi for host profile manipulation. The first one was missing support for KeyAnyValue serialization, the second was casting all integers to long - the deserializer strips the exact type (xsd:int, xsd:long) from the incoming data. This normally isn't an issue, but with host profiles it becomes one.

Let me give an example: to update an existing host profile, you need a HostProfileCompleteConfigSpec, which contains the field applyProfile. You don't fill this by hand, but get it from the existing profile managed object's config.applyProfile field (HostProfile.ConfigInfo has a field applyProfile of type HostApplyProfile). You change what's needed and pass it as parameters to UpdateHostProfile. Since that isn't a newly defined object but comes from the deserializer, any original type info for integers has been lost, and by default the serializer casts every int as xsd:long, which usually isn't an issue, but with this particular case it is.

So the fix is to preserve the original type in the deserializer, so the serializer knows what to emit. I'm of course open to a better approach - you have way more experience with that code, so it is possible that I'm going in the completely wrong direction.

But so far my tests don't indicate any breakage on existing code - and I've tested a lot (admittedly haven't covered the whole API) - template cloning, all sorts of VM, host, distributed/standard vSwitch, etc. operations without failures. I've also verified that the emitted XML matches the ones emitted by the vSphere client and by PyVmomi. So I can be pretty certain that this shouldn't break anything that uses the API correctly.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As an additional comment - existing client code that generates new objects won't hit that. Existing client code that updates existing objects also shouldn't have issues, since for all intents and purposes the BasicTypes::Int mimics Fixnum (assignment, comparison, etc.).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, I get your explanation. But existing code that makes an API call, gets an integer and expects to be able to use it like a ruby int will fail, no? Like "foo.bar + 1" will no longer work if bar is of type int. Isn't that what your change would do?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are of course correct. Would inheriting from Integer solve that? I know it isn't very clean since Integer isn't a normal object in Ruby, but it's doable.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've amended the pull request to make BasicTypes::Int behave like a Ruby Integer in the aforementioned operations.

when :long
node.content.to_i
when :float
node.content.to_f
Expand Down