From 17d176b9a9be0d85f74b61d67ab6ad0f76013dbc Mon Sep 17 00:00:00 2001 From: Max Martin Date: Thu, 21 Apr 2011 16:47:44 -0700 Subject: Revert "Adding json support to Puppet::Node" This reverts commit d3c94e62386ec03617015f6e6269b1de805954ea. The JSON patch series has caused problems with the inventory service, and further discussion is needed to decide how to serialize objects to PSON with regards to future compatibility. Conflicts (Manually resolved): spec/unit/node_spec.rb Paired-with: Matt Robinson --- lib/puppet/node.rb | 23 ------------------ spec/unit/node_spec.rb | 63 -------------------------------------------------- 2 files changed, 86 deletions(-) diff --git a/lib/puppet/node.rb b/lib/puppet/node.rb index 4bd4d1de6..5b0a98615 100644 --- a/lib/puppet/node.rb +++ b/lib/puppet/node.rb @@ -20,29 +20,6 @@ class Puppet::Node attr_accessor :name, :classes, :source, :ipaddress, :parameters attr_reader :time - def self.from_pson(pson) - raise ArgumentError, "No name provided in pson data" unless name = pson['name'] - - node = new(name) - node.classes = pson['classes'] - node.parameters = pson['parameters'] - node.environment = pson['environment'] - node - end - - def to_pson(*args) - result = { - 'document_type' => "Puppet::Node", - 'data' => {} - } - result['data']['name'] = name - result['data']['classes'] = classes unless classes.empty? - result['data']['parameters'] = parameters unless parameters.empty? - result['data']['environment'] = environment.name - - result.to_pson(*args) - end - def environment return super if @environment diff --git a/spec/unit/node_spec.rb b/spec/unit/node_spec.rb index e8f826dca..169a9cdcf 100755 --- a/spec/unit/node_spec.rb +++ b/spec/unit/node_spec.rb @@ -36,69 +36,6 @@ describe Puppet::Node do node.environment.name.should == :bar end end - - describe "when converting to json" do - before do - @node = Puppet::Node.new("mynode") - end - - it "should provide its name" do - @node.should set_json_attribute('name').to("mynode") - end - - it "should include the classes if set" do - @node.classes = %w{a b c} - @node.should set_json_attribute("classes").to(%w{a b c}) - end - - it "should not include the classes if there are none" do - @node.should_not set_json_attribute('classes') - end - - it "should include parameters if set" do - @node.parameters = {"a" => "b", "c" => "d"} - @node.should set_json_attribute('parameters').to({"a" => "b", "c" => "d"}) - end - - it "should not include the parameters if there are none" do - @node.should_not set_json_attribute('parameters') - end - - it "should include the environment" do - @node.environment = "production" - @node.should set_json_attribute('environment').to('production') - end - end - - describe "when converting from json" do - before do - @node = Puppet::Node.new("mynode") - @format = Puppet::Network::FormatHandler.format('pson') - end - - def from_json(json) - @format.intern(Puppet::Node, json) - end - - it "should set its name" do - Puppet::Node.should read_json_attribute('name').from(@node.to_pson).as("mynode") - end - - it "should include the classes if set" do - @node.classes = %w{a b c} - Puppet::Node.should read_json_attribute('classes').from(@node.to_pson).as(%w{a b c}) - end - - it "should include parameters if set" do - @node.parameters = {"a" => "b", "c" => "d"} - Puppet::Node.should read_json_attribute('parameters').from(@node.to_pson).as({"a" => "b", "c" => "d"}) - end - - it "should include the environment" do - @node.environment = "production" - Puppet::Node.should read_json_attribute('environment').from(@node.to_pson).as(Puppet::Node::Environment.new(:production)) - end - end end describe Puppet::Node, "when initializing" do -- cgit From aaf7e2300b12a0ef03b620efc5eea7af0dc6f71b Mon Sep 17 00:00:00 2001 From: Max Martin Date: Thu, 21 Apr 2011 16:53:26 -0700 Subject: Revert "(7080) Adding json support to Indirector Request" This reverts commit e0615cbc1eea67ef8caf4edbc8b7b3d3ce618f4d. The JSON patch series has caused problems with the inventory service, and further discussion is needed to decide how to serialize objects to PSON with regards to future compatibility. Conflicts: spec/unit/indirector/request_spec.rb Paired-with:Matt Robinson --- lib/puppet/indirector/request.rb | 51 +------------------ spec/unit/indirector/request_spec.rb | 95 ------------------------------------ 2 files changed, 2 insertions(+), 144 deletions(-) diff --git a/lib/puppet/indirector/request.rb b/lib/puppet/indirector/request.rb index 1918a3fb5..fd8d654dd 100644 --- a/lib/puppet/indirector/request.rb +++ b/lib/puppet/indirector/request.rb @@ -14,51 +14,6 @@ class Puppet::Indirector::Request OPTION_ATTRIBUTES = [:ip, :node, :authenticated, :ignore_terminus, :ignore_cache, :instance, :environment] - def self.from_pson(json) - raise ArgumentError, "No indirection name provided in json data" unless indirection_name = json['type'] - raise ArgumentError, "No method name provided in json data" unless method = json['method'] - raise ArgumentError, "No key provided in json data" unless key = json['key'] - - request = new(indirection_name, method, key, json['attributes']) - - if instance = json['instance'] - klass = Puppet::Indirector::Indirection.instance(request.indirection_name).model - if instance.is_a?(klass) - request.instance = instance - else - request.instance = klass.from_pson(instance) - end - end - - request - end - - def to_pson(*args) - result = { - 'document_type' => 'Puppet::Indirector::Request', - 'data' => { - 'type' => indirection_name, - 'method' => method, - 'key' => key - } - } - data = result['data'] - attributes = {} - OPTION_ATTRIBUTES.each do |key| - next unless value = send(key) - attributes[key] = value - end - - options.each do |opt, value| - attributes[opt] = value - end - - data['attributes'] = attributes unless attributes.empty? - data['instance'] = instance if instance - - result.to_pson(*args) - end - # Is this an authenticated request? def authenticated? # Double negative, so we just get true or false @@ -106,11 +61,9 @@ class Puppet::Indirector::Request self.indirection_name = indirection_name self.method = method - options = options.inject({}) { |hash, ary| hash[ary[0].to_sym] = ary[1]; hash } - set_attributes(options) - @options = options + @options = options.inject({}) { |hash, ary| hash[ary[0].to_sym] = ary[1]; hash } if key_or_instance.is_a?(String) || key_or_instance.is_a?(Symbol) key = key_or_instance @@ -200,7 +153,7 @@ class Puppet::Indirector::Request def set_attributes(options) OPTION_ATTRIBUTES.each do |attribute| - if options.include?(attribute.to_sym) + if options.include?(attribute) send(attribute.to_s + "=", options[attribute]) options.delete(attribute) end diff --git a/spec/unit/indirector/request_spec.rb b/spec/unit/indirector/request_spec.rb index ba7dc815e..87b9af438 100755 --- a/spec/unit/indirector/request_spec.rb +++ b/spec/unit/indirector/request_spec.rb @@ -301,99 +301,4 @@ describe Puppet::Indirector::Request do lambda { @request.query_string }.should raise_error(ArgumentError) end end - - describe "when converting to json" do - before do - @request = Puppet::Indirector::Request.new(:facts, :find, "foo") - end - - it "should produce a hash with the document_type set to 'request'" do - @request.should set_json_document_type_to("Puppet::Indirector::Request") - end - - it "should set the 'key'" do - @request.should set_json_attribute("key").to("foo") - end - - it "should include an attribute for its indirection name" do - @request.should set_json_attribute("type").to("facts") - end - - it "should include a 'method' attribute set to its method" do - @request.should set_json_attribute("method").to("find") - end - - it "should add all attributes under the 'attributes' attribute" do - @request.ip = "127.0.0.1" - @request.should set_json_attribute("attributes", "ip").to("127.0.0.1") - end - - it "should add all options under the 'attributes' attribute" do - @request.options["opt"] = "value" - PSON.parse(@request.to_pson)["data"]['attributes']['opt'].should == "value" - end - - it "should include the instance if provided" do - facts = Puppet::Node::Facts.new("foo") - @request.instance = facts - PSON.parse(@request.to_pson)["data"]['instance'].should be_instance_of(Puppet::Node::Facts) - end - end - - describe "when converting from json" do - before do - @request = Puppet::Indirector::Request.new(:facts, :find, "foo") - @klass = Puppet::Indirector::Request - @format = Puppet::Network::FormatHandler.format('pson') - end - - def from_json(json) - @format.intern(Puppet::Indirector::Request, json) - end - - it "should set the 'key'" do - from_json(@request.to_pson).key.should == "foo" - end - - it "should fail if no key is provided" do - json = PSON.parse(@request.to_pson) - json['data'].delete("key") - lambda { from_json(json.to_pson) }.should raise_error(ArgumentError) - end - - it "should set its indirector name" do - from_json(@request.to_pson).indirection_name.should == :facts - end - - it "should fail if no type is provided" do - json = PSON.parse(@request.to_pson) - json['data'].delete("type") - lambda { from_json(json.to_pson) }.should raise_error(ArgumentError) - end - - it "should set its method" do - from_json(@request.to_pson).method.should == "find" - end - - it "should fail if no method is provided" do - json = PSON.parse(@request.to_pson) - json['data'].delete("method") - lambda { from_json(json.to_pson) }.should raise_error(ArgumentError) - end - - it "should initialize with all attributes and options" do - @request.ip = "127.0.0.1" - @request.options["opt"] = "value" - result = from_json(@request.to_pson) - result.options[:opt].should == "value" - result.ip.should == "127.0.0.1" - end - - it "should set its instance as an instance if one is provided" do - facts = Puppet::Node::Facts.new("foo") - @request.instance = facts - result = from_json(@request.to_pson) - result.instance.should be_instance_of(Puppet::Node::Facts) - end - end end -- cgit From 2a2226c0b71aafcda953057d3ecc8df5638447f2 Mon Sep 17 00:00:00 2001 From: Max Martin Date: Thu, 21 Apr 2011 16:56:03 -0700 Subject: Revert "Fixing Facts pson methods more resilient" This reverts commit 07a7a68a25eb9b21189751c27f90f972224ea533. The JSON patch series has caused problems with the inventory service, and further discussion is needed to decide how to serialize objects to PSON with regards to future compatibility. Conflicts: spec/unit/node/facts_spec.rb Paired-with:Matt Robinson --- lib/puppet/node/facts.rb | 20 ++++++++------------ spec/unit/node/facts_spec.rb | 30 ++++++------------------------ 2 files changed, 14 insertions(+), 36 deletions(-) diff --git a/lib/puppet/node/facts.rb b/lib/puppet/node/facts.rb index 2ff7156c8..577b62b62 100755 --- a/lib/puppet/node/facts.rb +++ b/lib/puppet/node/facts.rb @@ -61,22 +61,18 @@ class Puppet::Node::Facts def self.from_pson(data) result = new(data['name'], data['values']) - result.timestamp = Time.parse(data['timestamp']) if data['timestamp'] - result.expiration = Time.parse(data['expiration']) if data['expiration'] + result.timestamp = Time.parse(data['timestamp']) + result.expiration = Time.parse(data['expiration']) result end def to_pson(*args) - result = { - 'document_type' => "Puppet::Node::Facts", - 'data' => {} - } - - result['data']['name'] = name - result['data']['expiration'] = expiration if expiration - result['data']['timestamp'] = timestamp if timestamp - result['data']['values'] = strip_internal - result.to_pson(*args) + { + 'expiration' => expiration, + 'name' => name, + 'timestamp' => timestamp, + 'values' => strip_internal, + }.to_pson(*args) end # Add internal data to the facts for storage. diff --git a/spec/unit/node/facts_spec.rb b/spec/unit/node/facts_spec.rb index 1b6991ca0..efaa76e12 100755 --- a/spec/unit/node/facts_spec.rb +++ b/spec/unit/node/facts_spec.rb @@ -110,11 +110,7 @@ describe Puppet::Node::Facts, "when indirecting" do end it "should accept properly formatted pson" do - facts = Puppet::Node::Facts.new("foo") - facts.values = {"a" => "1", "b" => "2", "c" => "3"} - facts.expiration = Time.now - #pson = %Q({"document_type": "Puppet::Node::Facts", "data: {"name": "foo", "expiration": "#{@expiration}", "timestamp": "#{@timestamp}", "values": {"a": "1", "b": "2", "c": "3"}}}) - pson = %Q({"data": {"name":"foo", "expiration":"#{@expiration}", "timestamp": "#{@timestamp}", "values":{"a":"1","b":"2","c":"3"}}, "document_type":"Puppet::Node::Facts"}) + pson = %Q({"name": "foo", "expiration": "#{@expiration}", "timestamp": "#{@timestamp}", "values": {"a": "1", "b": "2", "c": "3"}}) format = Puppet::Network::FormatHandler.format('pson') facts = format.intern(Puppet::Node::Facts,pson) facts.name.should == 'foo' @@ -126,25 +122,11 @@ describe Puppet::Node::Facts, "when indirecting" do Time.stubs(:now).returns(@timestamp) facts = Puppet::Node::Facts.new("foo", {'a' => 1, 'b' => 2, 'c' => 3}) facts.expiration = @expiration - facts.to_pson.should == %Q[{"data":{"name":"foo","timestamp":"#{@timestamp}","expiration":"#{@expiration}","values":{"a":1,"b":2,"c":3}},"document_type":"Puppet::Node::Facts"}] - end - - it "should not include nil values" do - facts = Puppet::Node::Facts.new("foo", {'a' => 1, 'b' => 2, 'c' => 3}) - - # XXX:LAK For some reason this is resurrection the full instance, instead - # of just returning the hash. This code works, but I can't figure out what's - # going on. - newfacts = PSON.parse(facts.to_pson) - newfacts.expiration.should be_nil - end - - it "should be able to handle nil values" do - pson = %Q({"name": "foo", "values": {"a": "1", "b": "2", "c": "3"}}) - format = Puppet::Network::FormatHandler.format('pson') - facts = format.intern(Puppet::Node::Facts,pson) - facts.name.should == 'foo' - facts.expiration.should be_nil + result = PSON.parse(facts.to_pson) + result['name'].should == facts.name + result['values'].should == facts.values.reject { |key, value| key.to_s =~ /_/ } + result['timestamp'].should == facts.timestamp.to_s + result['expiration'].should == facts.expiration.to_s end end end -- cgit