Skip to content

Commit 9997563

Browse files
committed
(maint) Simplify SqlConnection error handling
- The existing SqlConnection class has a public property exception_caught, which is not used. Further, it uses this value to save off exceptions inside the class, rather than simply returning the parsed errors when open_and_run_command is executed, which is presently the only caller that ever looks at the error values. This provides an opportunity to simplify the code paths, and remove unnecessary caching of errors and the 2 unnecessary methods has_errors and error_message from the SqlConnection class. Instead, directly return a new ResultOutput instance as soon as SQL code is executed. Tests are modified accordingly.
1 parent a3b1d3f commit 9997563

File tree

2 files changed

+5
-31
lines changed

2 files changed

+5
-31
lines changed

lib/puppet_x/sqlserver/sql_connection.rb

Lines changed: 2 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -4,20 +4,17 @@ module Sqlserver
44
CONNECTION_CLOSED = 0
55

66
class SqlConnection
7-
attr_reader :exception_caught
8-
97
def open_and_run_command(query, config)
108
begin
119
open(config)
12-
reset_instance
1310
execute(query)
1411
rescue win32_exception => e
15-
@exception_caught = e
12+
return ResultOutput.new(true, e.message)
1613
ensure
1714
close
1815
end
1916

20-
ResultOutput.new(has_errors, error_message)
17+
ResultOutput.new(false, nil)
2118
end
2219

2320
private
@@ -46,25 +43,13 @@ def get_connection_string(config)
4643
params.map { |k, v| "#{k}=#{v}" }.join(';')
4744
end
4845

49-
def has_errors
50-
@exception_caught != nil
51-
end
52-
53-
def error_message
54-
@exception_caught.message unless @exception_caught == nil
55-
end
56-
5746
def close
5847
begin
5948
connection.Close unless connection_closed?
6049
rescue win32_exception => e
6150
end
6251
end
6352

64-
def reset_instance
65-
@exception_caught = nil
66-
end
67-
6853
def connection_closed?
6954
connection.State == CONNECTION_CLOSED
7055
end

spec/unit/puppet_x/sql_connection_spec.rb

Lines changed: 3 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -13,19 +13,13 @@ def stub_connection
1313
subject.stubs(:win32_exception).returns(Exception)
1414
end
1515

16-
def stub_no_errors
17-
subject.stubs(:has_errors).returns(false)
18-
subject.stubs(:error_message).returns(nil)
19-
end
20-
2116
describe 'open_and_run_command' do
2217
context 'command execution' do
2318
before :each do
2419
stub_connection
2520
@connection.stubs(:Open).with('Provider=SQLOLEDB.1;User ID=sa;Password=Pupp3t1@;Initial Catalog=master;Application Name=Puppet;Data Source=localhost')
2621
end
2722
it 'should not raise an error but populate has_errors with message' do
28-
subject.stubs(:win32_exception).returns(Exception)
2923
subject.stubs(:execute).raises(Exception.new("SQL Server\n error has happened"))
3024
expect {
3125
result = subject.open_and_run_command('whacka whacka whacka', config)
@@ -44,7 +38,7 @@ def stub_no_errors
4438
context 'closed connection' do
4539
before :each do
4640
stub_connection
47-
stub_no_errors
41+
@connection.stubs(:Execute)
4842
end
4943
it 'should not add MSSQLSERVER to connection string' do
5044
@connection.stubs(:Open).with('Provider=SQLOLEDB.1;User ID=sa;Password=Pupp3t1@;Initial Catalog=master;Application Name=Puppet;Data Source=localhost')
@@ -66,13 +60,10 @@ def stub_no_errors
6660
end
6761
context 'return result with errors' do
6862
it {
63+
subject.stubs(:win32_exception).returns(Exception)
6964
subject.expects(:open).with({:admin_user => 'sa', :admin_pass => 'Pupp3t1@', :instance_name => 'MSSQLSERVER'})
70-
subject.expects(:execute).with('SELECT * FROM sys.databases')
65+
subject.expects(:execute).with('SELECT * FROM sys.databases').raises(Exception.new("SQL Server\ninvalid syntax provider"))
7166
subject.expects(:close).once
72-
subject.stubs(:has_errors).returns(:true)
73-
subject.stubs(:error_message).returns(
74-
'SQL Server
75-
invalid syntax provider')
7667
result =
7768
subject.open_and_run_command('SELECT * FROM sys.databases', config)
7869
expect(result.exitstatus).to eq(1)
@@ -84,8 +75,6 @@ def stub_no_errors
8475
stub_connection
8576
err_message = "SQL Server\n ConnectionFailed"
8677
@connection.stubs(:Open).raises(Exception.new(err_message))
87-
subject.stubs(:has_errors).returns(true)
88-
subject.stubs(:error_message).returns(err_message)
8978
expect {
9079
result = subject.open_and_run_command('whacka whacka whacka', config)
9180
expect(result.exitstatus).to eq(1)

0 commit comments

Comments
 (0)