Merge pull request #1063 from brianlong/188704452-tweak-ping-times

188704452-tweak-ping-times
brianlong · Dec 23, 2024 · 41d3a58 · 41d3a58
2 parents 0ee36b2 + 3d57e4d
commit 41d3a58
Show file tree

Hide file tree

Showing 5 changed files with 22 additions and 23 deletions.
diff --git a/app/controllers/api/v1/api_controller.rb b/app/controllers/api/v1/api_controller.rb
@@ -10,29 +10,11 @@ class ApiController < BaseController
       include CollectorLogic
       include ActionController::ImplicitRender
 
-      # POST api/v1/collector
-      def collector
-        render json: { 'status' => 'Method Gone' }, status: 410
-      end
-
       # This is a simple endpoint to test API connections.
       # GET api/v1/ping => { 'answer' => 'pong' }
       def ping
         render json: { 'answer' => 'pong' }, status: 200
       end
-
-      def ping_times
-        render json: { 'status' => 'Method Gone' }, status: 410
-      end
-
-      # Filter params for the Collector
-      def collector_params
-        params.require(:collector).permit(
-          :payload_type,
-          :payload_version,
-          :payload
-        )
-      end
     end
   end
 end
diff --git a/app/controllers/api/v1/ping_times_controller.rb b/app/controllers/api/v1/ping_times_controller.rb
@@ -2,7 +2,7 @@ module Api
   module V1
       class PingTimesController < BaseController
         def ping_times
-          limit = ping_time_params[:limit] || 1000
+          limit = [(ping_time_params[:limit] || 1000).to_i, 9999].min
           render json: PingTime.where(network: ping_time_params[:network])
                                .order('created_at desc')
                                .limit(limit).to_json, status: 200
@@ -17,8 +17,7 @@ def collector
           params[:payload] = params[:payload]&.to_json
           @collector = ::Collector.new(
             collector_params.merge(
-              user_id: User.where(api_token: request.headers['Token']).first.id,
-              ip_address: request.remote_ip
+              user_id: User.where(api_token: request.headers['Token']).first.id
             )
           )
 

diff --git a/app/logic/collector_logic.rb b/app/logic/collector_logic.rb
@@ -3,6 +3,7 @@
 # CollectorLogic
 module CollectorLogic
   include PipelineLogic
+  REQUIRED_PAYLOAD_FIELDS = %w[network avg_ms min_ms max_ms from_account to_account from_ip to_ip].freeze
 
   # guard_ping_times will check the collector payload type & version
   #
@@ -23,6 +24,12 @@ def ping_times_guard
         return Pipeline.new(400, p[:payload], 'Wrong payload_version')
       end
 
+      fields_count = REQUIRED_PAYLOAD_FIELDS.map do |field|
+        collector.payload.scan(/#{field}/).size
+      end
+
+      return Pipeline.new(400, p[:payload], "Invalid payload fields count") unless !0.in?(fields_count) && fields_count.uniq.count == 1
+
       # Pass the collector object with the payload for subsquent steps
       return Pipeline.new(200, p[:payload].merge(collector: collector))
     rescue StandardError => e

diff --git a/app/views/public/api_documentation.html.erb b/app/views/public/api_documentation.html.erb
@@ -66,7 +66,7 @@
 <p><strong class="text-muted">`payload_type=string`</strong> = ping_times</p>
 <p><strong class="text-muted">`payload_version=NN`</strong> = 1</p>
 <p><strong class="text-muted">`payload=string(json)`</strong> json containing array with all ping details (see example above).</p>
-All options listed above are required.
+<p>Reqired payload fields: <%= CollectorLogic::REQUIRED_PAYLOAD_FIELDS.join(", ")%></p>
 
 <h3 class="h6">Response:</h3>
 <pre>{"status":"Accepted"}</pre>
@@ -81,7 +81,7 @@ All options listed above are required.
 <h3 class="h6">Request:</h3>
 <pre>curl -H "Token: secret-api-token" '<%= @api_path %>/ping-times/:network.json'</pre>
 <h3 class="h6">Parameter Options:</h3>
-<p><strong class="text-muted">`limit=NN`</strong> will limit the results to NN entries, defaults to 1_000.</p>
+<p><strong class="text-muted">`limit=NN`</strong> will limit the results to NN entries, defaults to 1_000 (max 9_999).</p>
 
 <h3 class="h6">Response:</h3>
 <p>An Array of ping times.</p>

diff --git a/test/logic/collector_logic_test.rb b/test/logic/collector_logic_test.rb
@@ -41,4 +41,15 @@ class CollectorLogicTest < ActiveSupport::TestCase
     assert_equal 3.428, ping_time_stat.overall_max_time
     assert_equal 1.738, ping_time_stat.overall_average_time
   end
+
+  test '#ping_times_guard returns 400 if there are some fields missing in pipeline' do
+    @collector.update!(payload: { ping_times: nil }.to_json)
+
+    payload = { collector_id: @collector.id }
+    result = Pipeline.new(200, payload)
+                      .then(&ping_times_guard)
+
+    assert_equal 400, result.code
+    assert result.message.include?('Invalid payload fields count')
+  end
 end