softlayer / softlayer-ruby

http://softlayer.github.io/softlayer-ruby/
MIT License
54 stars 35 forks source link

getObject returning empty strings instead of nil for absent values #49

Closed cbreden closed 7 years ago

cbreden commented 10 years ago

I noticed the getObject behavior changed a bit with the XMLRPC calls. Key without values used to return nil but are now returning as empty strings like "". There are a few cases where we had to alter logic to account for this.

Was this an intended change? I know the use of nil always tends to be a hot topic.

SLsthompson commented 10 years ago

This wasn't an intentional change, but in general it appears that the built-in XMLRPC client does not "like" nil values. You cannot pass a nil value as an input into something that will be converted to XML and, based on your feedback, it translates empty results to the empty string.

I'm at a bit of a loss on how to fix this in the general case.

cbreden commented 10 years ago

Would an inline nil -> "" for input and "" -> nil for output safely solve it? I can't think of any reason to intentionally send the service an empty string, but there definitely could be a lot of edge cases I'm not considering.

SLsthompson commented 10 years ago

Consider for example the "notes" field of a server. It could very well be that the value of that field is actually the empty string (and not nil). In that case blindly translating the "" to nil on output would be bad.

To make a more intelligent choice you would have to know what type was expected. If you were expecting a string and got "" then that might be valid. If you're expecting an integer, but got "" then that should probably translate to nil.

cbreden commented 10 years ago

Well, just to challenge that particular example, is anyone actually using empty strings on for that notes field for a real purpose? It can't be done through the web interface at all.

All I can say is that I have a suspicion that the ability to pass empty strings wouldn't be missed by practically anyone. Whereas on the other hand, trying to bring back the behavior of the 1.x gem might ease adoption -- it certainly caused a few headaches for us.

And maybe as an alternative, maybe this inline translation could be turned off by passing an option during Client.new? Something like translate_empty_strings: false.

SLsthompson commented 10 years ago

The fundamental problem, of course, is XML-RPC itself which does not have a mechanism for representing nil at either end (calling or returning) without extensions. I guess the best response would be to investigate the extensions and see what it takes to support them through the SoftLayer API on both the server and client sides.

renier commented 7 years ago

This is due to Ruby's xmlrpc library. Maybe it changed with newer versions of Ruby, but currently it will parse an empty string node as an empty string.

And an empty string node it gets from the API: <value><string/></value> instead of omitting the field.

So this should be either fixed-enabled in the xmlrpc library or the server, but I don't see this as a bug in softlayer-ruby. Please reopen if you disagree.

P.S. There is a blank? method added to Object by the activesupport gem that will check for both nil and empty string values: e.g.is object.notes.blank?, which would be very convenient in this case.

You can also implement it yourself pretty easily:

class Object
  def blank?
    respond_to?(:empty?) ? !!empty? : !self
  end
end