Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP
Browse files

Merge pull request #4 from ayanko/master

Lazy load connection adapter (ayanko)

Implemented client errors handling on create/update (ayanko)
  • Loading branch information...
commit f491affa69bf55974bb09e0a34773c14885070e8 2 parents 3a51a55 + 4bd4f88
@pjdavis authored
View
9 Gemfile
@@ -0,0 +1,9 @@
+source :rubygems
+
+gemspec
+
+gem 'rspec', '2.8.0'
+
+gem 'activesupport', '3.0.12'
+
+gem 'i18n'
View
57 Gemfile.lock
@@ -0,0 +1,57 @@
+PATH
+ remote: .
+ specs:
+ roart (0.1.9)
+ activesupport (>= 2.0.0)
+ mechanize (>= 1.0.0)
+
+GEM
+ remote: http://rubygems.org/
+ specs:
+ activesupport (3.0.12)
+ bones (3.8.0)
+ little-plugger (~> 1.1.3)
+ loquacious (~> 1.9.1)
+ rake (>= 0.8.7)
+ diff-lcs (1.1.3)
+ domain_name (0.5.2)
+ unf (~> 0.0.3)
+ i18n (0.6.0)
+ little-plugger (1.1.3)
+ loquacious (1.9.1)
+ mechanize (2.3)
+ domain_name (~> 0.5, >= 0.5.1)
+ mime-types (~> 1.17, >= 1.17.2)
+ net-http-digest_auth (~> 1.1, >= 1.1.1)
+ net-http-persistent (~> 2.5, >= 2.5.2)
+ nokogiri (~> 1.4)
+ ntlm-http (~> 0.1, >= 0.1.1)
+ webrobots (~> 0.0, >= 0.0.9)
+ mime-types (1.17.2)
+ net-http-digest_auth (1.2)
+ net-http-persistent (2.5.2)
+ nokogiri (1.5.2)
+ ntlm-http (0.1.1)
+ rake (0.9.2.2)
+ rspec (2.8.0)
+ rspec-core (~> 2.8.0)
+ rspec-expectations (~> 2.8.0)
+ rspec-mocks (~> 2.8.0)
+ rspec-core (2.8.0)
+ rspec-expectations (2.8.0)
+ diff-lcs (~> 1.1.2)
+ rspec-mocks (2.8.0)
+ unf (0.0.5)
+ unf_ext
+ unf_ext (0.0.4)
+ webrobots (0.0.13)
+
+PLATFORMS
+ ruby
+
+DEPENDENCIES
+ activesupport (= 3.0.12)
+ bones (>= 2.5.1)
+ i18n
+ roart!
+ rspec (= 2.8.0)
View
12 lib/roart/connection.rb
@@ -22,7 +22,6 @@ def initialize(conf)
if Roart::check_keys(conf, Roart::Connections::RequiredConfig)
@agent = @conf[:login]
add_methods!
- @connection = ConnectionAdapter.new(@conf)
else
raise RoartError, "Configuration Error"
end
@@ -30,7 +29,7 @@ def initialize(conf)
def authenticate(conf)
if Roart::check_keys(conf, Roart::Connections::RequiredToLogin)
- @connection.authenticate(conf)
+ connection.authenticate(conf)
self
end
end
@@ -40,15 +39,18 @@ def rest_path
end
def get(uri)
- @connection.get(uri)
+ connection.get(uri)
end
def post(uri, payload)
- @connection.post(uri, payload)
+ connection.post(uri, payload)
end
protected
+ def connection
+ @connection ||= ConnectionAdapter.new(conf)
+ end
def add_methods!
@conf.each do |key, value|
@@ -60,4 +62,4 @@ def add_methods!
end
-end
+end
View
74 lib/roart/ticket.rb
@@ -59,24 +59,7 @@ def save
if self.id == "ticket/new"
self.create
else
- self.before_update
- uri = "#{self.class.connection.server}/REST/1.0/ticket/#{self.id}/edit"
- payload = @attributes.clone
- payload.delete("text")
- payload.delete("id") # Can't have text in an update, only create, use comment for updateing
- payload = payload.to_content_format
- resp = self.class.connection.post(uri, :content => payload)
- resp = resp.split("\n")
- raise TicketSystemError, "Ticket Update Failed" unless resp.first.include?("200")
- resp.each do |line|
- if line.match(/^# Ticket (\d+) updated./)
- self.after_update
- return true
- else
- #TODO: Add warnign to ticket
- end
- end
- return false
+ self.update
end
end
@@ -113,19 +96,50 @@ def create #:nodoc:
uri = "#{self.class.connection.server}/REST/1.0/ticket/new"
payload = @attributes.to_content_format
resp = self.class.connection.post(uri, :content => payload)
- resp = resp.split("\n")
- raise TicketSystemError, "Ticket Create Failed" unless resp.first.include?("200")
- resp.each do |line|
- if tid = line.match(/^# Ticket (\d+) created./)
- @attributes[:id] = tid[1].to_i
- self.after_create
- @new_record = false
- return true
- else
- #TODO: Add Warnings To Ticket
- end
+
+ process_save_response(resp, :create)
+ end
+
+ def update
+ self.before_update
+ uri = "#{self.class.connection.server}/REST/1.0/ticket/#{self.id}/edit"
+ payload = @attributes.clone
+ payload.delete("text")
+ payload.delete("id") # Can't have text in an update, only create, use comment for updateing
+ payload = payload.to_content_format
+ resp = self.class.connection.post(uri, :content => payload)
+
+ process_save_response(resp, :update)
+ end
+
+ SUCCESS_CODES = (200..299).to_a
+ CLIENT_ERROR_CODES = (400..499).to_a
+
+ def process_save_response(response, action)
+ errors.clear
+ action_name = action.to_s.capitalize
+
+ lines = response.split("\n").reject { |l| l.blank? }
+
+ status_line = lines.shift
+ status_line.present? or
+ raise TicketSystemError, "Ticket #{action_name} Failed (blank response)"
+
+ version, status_code, status_text = status_line.split(/\s+/,2)
+
+ if SUCCESS_CODES.include?(status_code.to_i) && lines[0] =~ /^# Ticket (\d+) (created|updated)/
+ @attributes[:id] = $1.to_i if $2 == 'created'
+ @new_record = false
+ @saved = true
+ self.__send__("after_#{action}")
+ return true
+ elsif (SUCCESS_CODES + CLIENT_ERROR_CODES).include?(status_code.to_i)
+ lines[0] =~ /^# Could not (create|update) ticket/ and lines.shift
+ lines.each { |line| errors.add_to_base(line) if line =~ /^#/ }
+ return false
+ else
+ raise TicketSystemError, "Ticket #{action_name} Failed (#{status_line})"
end
- return false
end
def create! #:nodoc:
View
3  roart.gemspec
@@ -24,13 +24,16 @@ Gem::Specification.new do |s|
if Gem::Version.new(Gem::VERSION) >= Gem::Version.new('1.2.0') then
s.add_runtime_dependency(%q<mechanize>, [">= 1.0.0"])
+ s.add_runtime_dependency(%q<activesupport>, [">= 2.0.0"])
s.add_development_dependency(%q<bones>, [">= 2.5.1"])
else
s.add_dependency(%q<mechanize>, [">= 1.0.0"])
+ s.add_dependency(%q<activesupport>, [">= 2.0.0"])
s.add_dependency(%q<bones>, [">= 2.5.1"])
end
else
s.add_dependency(%q<mechanize>, [">= 1.0.0"])
+ s.add_dependency(%q<activesupport>, [">= 2.0.0"])
s.add_dependency(%q<bones>, [">= 2.5.1"])
end
end
View
56 spec/roart/connection_spec.rb
@@ -3,48 +3,52 @@
describe "Connection" do
- describe 'get and post' do
-
- before do
- @agent = mock("agent", :null_object => true)
- @options = {:server => 'server', :user => 'user', :pass => 'pass', :adapter => 'whatev'}
- Roart::ConnectionAdapter.should_receive(:new).and_return(@agent)
- end
-
- it 'should respond to get' do
- @agent.should_receive(:get).with('some_uri').and_return('body')
- connection = Roart::Connection.new(@options)
- connection.get("some_uri").should == 'body'
- end
+ before do
+ @options = {:server => 'server', :user => 'user', :pass => 'pass', :adapter => 'whatev'}
+ end
- it 'should respond to post' do
- @agent.should_receive(:post).with('some_uri', 'a payload').and_return('body')
- connection = Roart::Connection.new(@options)
- connection.post("some_uri", "a payload").should == 'body'
- end
-
+ it "should lazy load connection adapter" do
+ @options[:adapter] = 'mechanize'
+ Roart::ConnectionAdapter.should_not_receive(:new)
+ connection = Roart::Connection.new(@options)
end
-
+
it 'should raise an exception if it doesnt have all the options' do
lambda{Roart::Connection.new(:user => 'bad')}.should raise_error
end
it 'should give us the rest path' do
- @agent = mock("agent", :null_object => true)
- @options = {:server => 'server', :user => 'user', :pass => 'pass', :adapter => 'whatev'}
- Roart::ConnectionAdapter.should_receive(:new).and_return(@agent)
connection = Roart::Connection.new(@options)
connection.rest_path.should == 'server/REST/1.0/'
end
it 'should give us back the whole thing' do
+ @options[:adapter] = 'mechanize'
mock_mech = mock('mech')
- @options = {:server => 'server', :user => 'user', :pass => 'pass', :adapter => 'mechanize'}
Roart::ConnectionAdapters::MechanizeAdapter.should_receive(:new).with(@options).and_return(mock_mech)
- mock_mech.should_receive(:login).with(@options)
+ mock_mech.should_receive(:login).with(@options)
mock_mech.should_receive(:get).with('uri').and_return('body')
connection = Roart::Connection.new(@options)
connection.get('uri').should == 'body'
end
+
+ describe 'get and post' do
+ before do
+ @agent = mock("agent").as_null_object
+ Roart::ConnectionAdapter.should_receive(:new).and_return(@agent)
+ end
+
+ it 'should respond to get' do
+ @agent.should_receive(:get).with('some_uri').and_return('body')
+ connection = Roart::Connection.new(@options)
+ connection.get("some_uri").should == 'body'
+ end
+
+ it 'should respond to post' do
+ @agent.should_receive(:post).with('some_uri', 'a payload').and_return('body')
+ connection = Roart::Connection.new(@options)
+ connection.post("some_uri", "a payload").should == 'body'
+ end
+ end
-end
+end
View
163 spec/roart/ticket_spec.rb
@@ -361,27 +361,64 @@ class MyTicket < Roart::Ticket; end
post_data.update(:id => 'ticket/new')
post_data = to_content_format(post_data)
mock_connection = mock('connection')
- mock_connection.should_receive(:post).with('uri/REST/1.0/ticket/new', {:content => post_data}).and_return("RT/3.6.6 200 Ok\n\n# Ticket 267783 created.")
- mock_connection.should_receive(:server).and_return('uri')
Roart::Ticket.should_receive(:connection).twice.and_return(mock_connection)
+ mock_connection.should_receive(:server).and_return('uri')
+ mock_connection.should_receive(:post).
+ with('uri/REST/1.0/ticket/new', {:content => post_data}).
+ and_return(response_body)
end
-
- it 'should be able to save a new ticket' do
- ticket = Roart::Ticket.new(@payload)
- ticket.new_record?.should be_true
- ticket.save
- ticket.new_record?.should be_false
- end
+
+ let(:ticket) { Roart::Ticket.new(@payload) }
+
+ context "on success" do
+ let(:response_body) { "RT/3.6.6 200 Ok\n\n# Ticket 267783 created." }
+
+ it 'should be able to save a new ticket' do
+ ticket.new_record?.should be_true
+ ticket.save
+ ticket.new_record?.should be_false
+ end
+
+ it 'should be able to create a ticket' do
+ ticket = Roart::Ticket.create(@payload)
+ end
- it 'should be able to create a ticket' do
- ticket = Roart::Ticket.create(@payload)
+ it 'should return a newly created ticket' do
+ ticket.class.should == Roart::Ticket
+ ticket.save
+ ticket.id.should == 267783
+ end
+
+ it 'should NOT have errors' do
+ ticket.save
+ ticket.errors.should be_empty
+ end
+
+ it 'should be saved' do
+ ticket.save
+ ticket.saved.should == true
+ end
end
-
- it 'should return a newly created ticket' do
- ticket = Roart::Ticket.new(@payload)
- ticket.class.should == Roart::Ticket
- ticket.save
- ticket.id.should == 267783
+
+ context "on failure" do
+ let(:response_body) { "RT/4.0.5 200 Ok\n\n# Could not create ticket.\n# Status 'huj' isn't a valid status for tickets in this queue." }
+
+ it 'should return an unsaved ticket' do
+ ticket.class.should == Roart::Ticket
+ ticket.save
+ ticket.id.should == 'ticket/new'
+ ticket.should be_new_record
+ end
+
+ it 'should return false' do
+ ticket.save.should == false
+ end
+
+ it 'should have errors' do
+ ticket.save
+ ticket.errors.size.should == 1
+ ticket.errors[:base].should == ["# Status 'huj' isn't a valid status for tickets in this queue."]
+ end
end
end
@@ -396,36 +433,82 @@ class MyTicket < Roart::Ticket; end
@mock_connection = mock('connection')
@mock_connection.should_receive(:server).and_return('uri')
Roart::Ticket.should_receive(:connection).twice.and_return(@mock_connection)
+ @mock_connection.should_receive(:post).
+ with('uri/REST/1.0/ticket/1/edit', {:content => @post_data}).
+ and_return(response_body)
end
- it 'should be able to update a ticket' do
- @mock_connection.should_receive(:post).with('uri/REST/1.0/ticket/1/edit', {:content => @post_data}).and_return("RT/3.6.6 200 Ok\n\n# Ticket 267783 updated.")
- ticket = Roart::Ticket.send(:instantiate, @payload.update(:id => 1))
- ticket.subject = 'An Old Ticket'
- ticket.save.should == true
+ context "on success" do
+ let(:response_body) { "RT/3.6.6 200 Ok\n\n# Ticket 267783 updated." }
+
+ it 'should be able to update a ticket' do
+ ticket = Roart::Ticket.send(:instantiate, @payload.update(:id => 1))
+ ticket.subject = 'An Old Ticket'
+ ticket.save.should == true
+ end
+
+ it 'should keep the same id' do
+ ticket = Roart::Ticket.send(:instantiate, @payload.update(:id => 1))
+ ticket.subject = 'An Old Ticket'
+ ticket.save
+ ticket.id.should == 1
+ end
+
+ it 'should save the ticket' do
+ ticket = Roart::Ticket.send(:instantiate, @payload.update(:id => 1))
+ ticket.subject = 'An Old Ticket'
+ ticket.save
+ ticket.subject.should == 'An Old Ticket'
+ end
end
- it 'should keep the same id' do
- @mock_connection.should_receive(:post).with('uri/REST/1.0/ticket/1/edit', {:content => @post_data}).and_return("RT/3.6.6 200 Ok\n\n# Ticket 267783 updated.")
- ticket = Roart::Ticket.send(:instantiate, @payload.update(:id => 1))
- ticket.subject = 'An Old Ticket'
- ticket.save
- ticket.id.should == 1
+ context "on 502 failure" do
+ let(:response_body) { "RT/3.6.6 502 Not OK\n\n# Dead gateway" }
+
+ it 'should raise an error on failed save' do
+ ticket = Roart::Ticket.send(:instantiate, @payload.update(:id => 1))
+ ticket.subject = 'An Old Ticket'
+ expect { ticket.save }.to raise_error(Roart::TicketSystemError)
+ end
end
- it 'should save the ticket' do
- @mock_connection.should_receive(:post).with('uri/REST/1.0/ticket/1/edit', {:content => @post_data}).and_return("RT/3.6.6 200 Ok\n\n# Ticket 267783 updated.")
- ticket = Roart::Ticket.send(:instantiate, @payload.update(:id => 1))
- ticket.subject = 'An Old Ticket'
- ticket.save
- ticket.subject.should == 'An Old Ticket'
+ context "on 200 failure" do
+ let(:response_body) { "RT/3.6.6 200 Probably OK\n\n# Could not update ticket\n# Queue is missing" }
+
+ it 'should set errors' do
+ ticket = Roart::Ticket.send(:instantiate, @payload.update(:id => 1))
+ ticket.subject = 'An Old Ticket'
+ expect { ticket.save }.not_to raise_error
+ ticket.errors.size.should == 1
+ ticket.errors[:base].should == ["# Queue is missing"]
+ end
end
- it 'should raise an error on failed save' do
- @mock_connection.should_receive(:post).with('uri/REST/1.0/ticket/1/edit', {:content => @post_data}).and_return("RT/3.6.6 400 Not OK\n\n# U's A SUKKA, FOO!.")
- ticket = Roart::Ticket.send(:instantiate, @payload.update(:id => 1))
- ticket.subject = 'An Old Ticket'
- lambda {ticket.save}.should raise_error(Roart::TicketSystemError)
+ context "on 400 failure" do
+ let(:response_body) { "RT/3.6.6 400 Not OK\n\n# U's A SUKKA, FOO!." }
+
+ it 'should set errors' do
+ ticket = Roart::Ticket.send(:instantiate, @payload.update(:id => 1))
+ ticket.subject = 'An Old Ticket'
+ expect { ticket.save }.not_to raise_error
+ ticket.errors.size.should == 1
+ ticket.errors[:base].should == ["# U's A SUKKA, FOO!."]
+ end
+ end
+
+ context "on 409 failure" do
+ let(:response_body) { "RT/4.0.5 409 Syntax Error\n# Ticket 13 updated.\n# status2: Unknown field.\n\nid: \nStatus2: huj"}
+
+ it 'should set errors' do
+ ticket = Roart::Ticket.send(:instantiate, @payload.update(:id => 1))
+ ticket.subject = 'An Old Ticket'
+ expect { ticket.save }.not_to raise_error
+ ticket.errors.size.should == 2
+ ticket.errors[:base].should == [
+ "# Ticket 13 updated.",
+ "# status2: Unknown field."
+ ]
+ end
end
end
@@ -467,4 +550,4 @@ class MyTicket < Roart::Ticket; end
end
-end
+end
View
8 spec/spec_helper.rb
@@ -1,8 +1,4 @@
-
-dir = File.dirname(__FILE__)
-
-$:.unshift(File.join(dir, '/../lib/'))
-require dir + '/../lib/roart'
+require 'roart'
def dbtime(time)
time.strftime("%Y-%m-%d %H:%M:%S")
@@ -13,7 +9,7 @@ def to_content_format(data)
fields.compact.sort.join("\n")
end
-Spec::Runner.configure do |config|
+RSpec.configure do |config|
# == Mock Framework
#
# RSpec uses it's own mocking framework by default. If you prefer to
Please sign in to comment.
Something went wrong with that request. Please try again.