Code review changes

George Opritescu 8 years ago
parent
commit
d955ce003a
2 changed files with 23 additions and 18 deletions
  1. 12 10
      app/models/agents/website_agent.rb
  2. 11 8
      spec/models/agents/website_agent_spec.rb

+ 12 - 10
app/models/agents/website_agent.rb

@@ -103,7 +103,7 @@ module Agents
103 103
 
104 104
       Set `unzip` to `gzip` to inflate the resource using gzip.
105 105
 
106
-      Set `consider_http_error_success` to an array of ints, ex: `[404]` to consider also 404 as successes, and to scrape it.
106
+      Set `http_success_codes` to an array of status codes (e.g., `[404, 422]`) to treat HTTP response codes beyond 200 as successes.
107 107
 
108 108
       # Liquid Templating
109 109
 
@@ -170,17 +170,19 @@ module Agents
170 170
     end
171 171
 
172 172
     def validate_consider_http_success_option!
173
-      consider_success = options["consider_http_error_success"]
173
+      consider_success = options["http_success_codes"]
174 174
       if consider_success != nil
175 175
 
176
-        if consider_success.class != Array
177
-          errors.add(:base,"Must be an array and specify at least one status code")
176
+        if (consider_success.class != Array)
177
+          errors.add(:http_success_codes, "must be an array and specify at least one status code")
178 178
         else
179
-          if consider_success.uniq.count != consider_success.count
180
-            errors.add(:base,"Duplicate http code found")
179
+          if consider_success.blank?
180
+            errors.add(:http_success_codes, "must not be empty")
181
+          elsif consider_success.uniq.count != consider_success.count
182
+            errors.add(:http_success_codes, "duplicate http code found")
181 183
           else
182
-            if consider_success.map(&:class).uniq != [Fixnum]
183
-              errors.add(:base,"Please make sure to use only integer values for code")
184
+            if consider_success.any?{|e| e.to_s !~ /^\d+$/ }
185
+              errors.add(:http_success_codes, "please make sure to use only numeric values for code, ex 404, or \"404\"")
184 186
             end
185 187
           end
186 188
         end
@@ -378,8 +380,8 @@ module Agents
378 380
     private
379 381
     def consider_response_successful?(response)
380 382
       response.success? || begin
381
-        consider_success = options["consider_http_error_success"]
382
-        consider_success.present? && consider_success.include?(response.status)
383
+        consider_success = options["http_success_codes"]
384
+        consider_success.present? && (consider_success.include?(response.status.to_s) || consider_success.include?(response.status))
383 385
       end
384 386
     end
385 387
 

+ 11 - 8
spec/models/agents/website_agent_spec.rb

@@ -40,20 +40,23 @@ describe Agents::WebsiteAgent do
40 40
         expect(@checker).not_to be_valid
41 41
       end
42 42
 
43
-      it 'should validate the consider_http_error_success fields' do
44
-        @checker.options['consider_http_error_success'] = [404]
43
+      it 'should validate the http_success_codes fields' do
44
+        @checker.options['http_success_codes'] = [404]
45 45
         expect(@checker).to be_valid
46 46
 
47
-        @checker.options['consider_http_error_success'] = [404, 404]
47
+        @checker.options['http_success_codes'] = [404, 404]
48 48
         expect(@checker).not_to be_valid
49 49
 
50
-        @checker.options['consider_http_error_success'] = [404.0]
50
+        @checker.options['http_success_codes'] = [404, "422"]
51
+        expect(@checker).to be_valid
52
+
53
+        @checker.options['http_success_codes'] = [404.0]
51 54
         expect(@checker).not_to be_valid
52 55
 
53
-        @checker.options['consider_http_error_success'] = ["not_a_code"]
56
+        @checker.options['http_success_codes'] = ["not_a_code"]
54 57
         expect(@checker).not_to be_valid
55 58
 
56
-        @checker.options['consider_http_error_success'] = []
59
+        @checker.options['http_success_codes'] = []
57 60
         expect(@checker).not_to be_valid
58 61
       end
59 62
 
@@ -186,7 +189,7 @@ describe Agents::WebsiteAgent do
186 189
       end
187 190
     end
188 191
 
189
-    describe 'consider_http_error_success' do
192
+    describe 'http_success_codes' do
190 193
       it 'should allow scraping from a 404 result' do
191 194
         json = {
192 195
           'response' => {
@@ -202,7 +205,7 @@ describe Agents::WebsiteAgent do
202 205
           'type' => "json",
203 206
           'url' => "http://gzip.com",
204 207
           'mode' => 'on_change',
205
-          'consider_http_error_success' => [404],
208
+          'http_success_codes' => [404],
206 209
           'extract' => {
207 210
             'version' => { 'path' => 'response.version' },
208 211
           },