Merge pull request #1646 from yeti-switch/1644-Statistics-WITH-FILL-T…

…O-value-cannot-be-less-than-FROM

#1644, Validate date filters for origination-active-calls controller

app/controllers/api/rest/customer/v1/origination_active_calls_controller.rb

@@ -16,6 +16,9 @@ def show
     begin
       rows = statistic.collection
       render json: rows, status: 200
+    rescue ClickhouseReport::Base::FromDateTimeInFutureError => e
+      Rails.logger.error { "<#{e.class}>: #{e.message}" }
+      render json: [], status: 200
     rescue ClickhouseReport::Base::ParamError => e
       Rails.logger.error { "Bad Request <#{e.class}>: #{e.message}" }
       render json: { error: e.message }, status: 400

app/controllers/api/rest/customer/v1/origination_statistics_controller.rb

@@ -16,6 +16,9 @@ def show
     begin
       rows = statistic.collection
       render json: rows, status: 200
+    rescue ClickhouseReport::Base::FromDateTimeInFutureError => e
+      Rails.logger.error { "<#{e.class}>: #{e.message}" }
+      render json: [], status: 200
     rescue ClickhouseReport::Base::ParamError => e
       Rails.logger.error { "Bad Request <#{e.class}>: #{e.message}" }
       render json: { error: e.message }, status: 400

app/controllers/api/rest/customer/v1/origination_statistics_quality_controller.rb

@@ -16,6 +16,9 @@ def show
     begin
       rows = statistic.collection
       render json: rows, status: 200
+    rescue ClickhouseReport::Base::FromDateTimeInFutureError => e
+      Rails.logger.error { "<#{e.class}>: #{e.message}" }
+      render json: [], status: 200
     rescue ClickhouseReport::Base::ParamError => e
       Rails.logger.error { "Bad Request <#{e.class}>: #{e.message}" }
       render json: { error: e.message }, status: 400

app/lib/date_utilities.rb

@@ -0,0 +1,11 @@
+# frozen_string_literal: true
+
+module DateUtilities
+  module_function
+
+  def safe_datetime_parse(date_string)
+    DateTime.parse(date_string)
+  rescue ArgumentError, TypeError
+    nil
+  end
+end

app/models/clickhouse_report/base.rb

@@ -92,6 +92,8 @@ class InvalidParamValue < ParamError
     class InvalidResponseError < Error
     end
 
+    FromDateTimeInFutureError = Class.new(ParamError)
+
     class_attribute :operations, instance_accessor: false, default: {}
     class_attribute :filters, instance_accessor: false, default: {}
 

app/models/clickhouse_report/origination_active_calls.rb

@@ -18,12 +18,25 @@ class OriginationActiveCalls < Base
            column: :snapshot_timestamp,
            type: 'DateTime',
            operation: :gteq,
-           required: true
+           required: true,
+           format_value: lambda { |value, _options|
+             from_time = DateUtilities.safe_datetime_parse(value)
+             raise InvalidParamValue, 'invalid value from-time' if from_time.nil?
+             raise FromDateTimeInFutureError, 'from-time cannot be in the future' if from_time > DateTime.now
+
+             value
+           }
 
     filter :'to-time',
            column: :snapshot_timestamp,
            type: 'DateTime',
-           operation: :lteq
+           operation: :lteq,
+           format_value: lambda { |value, _options|
+             to_time = DateUtilities.safe_datetime_parse(value)
+             raise InvalidParamValue, 'invalid value to-time' if to_time.nil?
+
+             value
+           }
 
     filter :'src-country-id',
            column: :src_country_id,

app/models/clickhouse_report/origination_statistic.rb

@@ -27,12 +27,25 @@ class OriginationStatistic < Base
            column: :time_start,
            type: 'DateTime',
            operation: :gteq,
-           required: true
+           required: true,
+           format_value: lambda { |value, _options|
+             from_time = DateUtilities.safe_datetime_parse(value)
+             raise InvalidParamValue, 'invalid value from-time' if from_time.nil?
+             raise FromDateTimeInFutureError, 'from-time cannot be in the future' if from_time > DateTime.now
+
+             value
+           }
 
     filter :'to-time',
            column: :time_start,
            type: 'DateTime',
-           operation: :lteq
+           operation: :lteq,
+           format_value: lambda { |value, _options|
+             to_time = DateUtilities.safe_datetime_parse(value)
+             raise InvalidParamValue, 'invalid value to-time' if to_time.nil?
+
+             value
+           }
 
     filter :'src-country-id',
            column: :src_country_id,

app/models/clickhouse_report/origination_statistic_quality.rb

@@ -30,12 +30,25 @@ class OriginationStatisticQuality < Base
            column: :time_start,
            type: 'DateTime',
            operation: :gteq,
-           required: true
+           required: true,
+           format_value: lambda { |value, _options|
+             from_time = DateUtilities.safe_datetime_parse(value)
+             raise InvalidParamValue, 'invalid value from-time' if from_time.nil?
+             raise FromDateTimeInFutureError, 'from-time cannot be in the future' if from_time > DateTime.now
+
+             value
+           }
 
     filter :'to-time',
            column: :time_start,
            type: 'DateTime',
-           operation: :lteq
+           operation: :lteq,
+           format_value: lambda { |value, _options|
+             to_time = DateUtilities.safe_datetime_parse(value)
+             raise InvalidParamValue, 'invalid value to-time' if to_time.nil?
+
+             value
+           }
 
     filter :'src-country-id',
            column: :src_country_id,

spec/requests/api/rest/customer/v1/origination_active_calls_controller_spec.rb

@@ -0,0 +1,116 @@
+# frozen_string_literal: true
+
+RSpec.describe Api::Rest::Customer::V1::OriginationActiveCallsController do
+  include_context :json_api_customer_v1_helpers, type: :accounts
+
+  let(:auth_headers) { { 'Authorization' => json_api_auth_token } }
+
+  describe 'GET /api/rest/customer/v1/origination-active-calls' do
+    subject { get '/api/rest/customer/v1/origination-active-calls', params: query, headers: auth_headers }
+
+    shared_examples :responds_400 do |error_message|
+      it 'responds with correct error data' do
+        expect(CaptureError).not_to receive(:capture)
+        subject
+
+        expect(response.status).to eq 400
+        expect(response_json).to eq error: error_message
+      end
+    end
+
+    shared_examples :responds_success do
+      it 'responds with correct data' do
+        expect(CaptureError).not_to receive(:capture)
+        subject
+
+        expect(response.status).to eq 200
+        expect(response_json).to eq active_calls_response_body
+      end
+    end
+
+    let!(:account) { create(:account, contractor: customer) }
+    let(:clickhouse_query) do
+      <<-SQL.squish
+        SELECT
+          toUnixTimestamp(snapshot_timestamp) as t,
+          toUInt32(count(*)) AS calls
+        FROM active_calls
+        WHERE
+          customer_acc_id = {account_id: UInt32} AND
+          snapshot_timestamp >= {from_time: DateTime}
+        GROUP BY t
+        ORDER BY t
+        WITH FILL
+          FROM toUnixTimestamp(toStartOfMinute({from_time: DateTime}))
+          TO toUnixTimestamp(now())
+          STEP 60
+        FORMAT JSONColumnsWithMetadata
+      SQL
+    end
+    let(:clickhouse_params) do
+      {
+        param_account_id: account.id,
+        param_from_time: from_time
+      }
+    end
+    let!(:stub_clickhouse_query) do
+      stub_request(:post, ClickHouse.config.url)
+        .with(
+          basic_auth: [ClickHouse.config.username, ClickHouse.config.password],
+          query: {
+            database: ClickHouse.config.database,
+            **clickhouse_params,
+            query: clickhouse_query,
+            send_progress_in_http_headers: 1
+          }
+        ).to_return(
+          status: active_calls_response_status,
+          body: active_calls_response_body.to_json
+        )
+    end
+    let(:active_calls_response_status) { 200 }
+    let(:active_calls_response_body) { [{ call_id: 'abc123', duration: 123 }] }
+    let(:from_time) { 1.day.ago.iso8601 }
+    let(:query) do
+      {
+        'account-id': account.uuid,
+        'from-time': from_time
+      }
+    end
+
+    it_behaves_like :responds_success
+
+    context 'without params' do
+      let(:query) { nil }
+      let(:stub_clickhouse_query) { nil }
+
+      include_examples :responds_400, 'missing required param(s) account-id, from-time'
+    end
+
+    context 'with from-time in the future', freeze_time: Time.parse('2024-12-01 00:00:00 UTC') do
+      let(:query) { super().merge 'from-time': '2024-12-25T05:00:00Z', 'to-time': '2024-12-26T05:00:00Z' }
+
+      before { Log::ApiLog.add_partitions }
+
+      it 'should NOT perform POST request to ClickHouse server and then return empty collection' do
+        expect(CaptureError).not_to receive(:capture)
+        subject
+
+        expect(stub_clickhouse_query).not_to have_been_requested
+        expect(response_json).to eq([])
+      end
+    end
+
+    context 'when the "from-time" is invalid Date' do
+      let(:query) { super().merge 'from-time': 'invalid' }
+
+      include_examples :responds_400, 'invalid value from-time'
+    end
+
+    context 'when the "to-time" is invalid Date' do
+      let(:query) { super().merge 'to-time': 'invalid' }
+
+      include_examples :responds_400, 'invalid value to-time'
+    end
+  end
+end

spec/requests/api/rest/customer/v1/origination_statistics_controller_spec.rb

@@ -0,0 +1,82 @@
+# frozen_string_literal: true
+
+RSpec.describe Api::Rest::Customer::V1::OriginationStatisticsController do
+  include_context :json_api_customer_v1_helpers, type: :accounts
+
+  let(:auth_headers) { { 'Authorization' => json_api_auth_token } }
+
+  describe 'GET /api/rest/customer/v1/origination-statistics' do
+    subject { get '/api/rest/customer/v1/origination-statistics', params: query, headers: auth_headers }
+
+    shared_examples :responds_400 do |error_message|
+      it 'responds with correct error data' do
+        expect(CaptureError).not_to receive(:capture)
+        subject
+
+        expect(response.status).to eq 400
+        expect(response_json).to eq error: error_message
+      end
+    end
+
+    shared_examples :responds_success do
+      it 'responds with correct data' do
+        expect(CaptureError).not_to receive(:capture)
+        subject
+
+        expect(response.status).to eq 200
+        expect(response_json).to eq clickhouse_response_body
+      end
+    end
+
+    let!(:account) { create(:account, contractor: customer) }
+    let(:clickhouse_response_body) { [{ some_key: 'value' }] }
+    let(:from_time) { 1.day.ago.iso8601 }
+    let!(:stub_clickhouse_query) do
+      stub_request(:post, /#{ClickHouse.config.url}/).and_return(status: 200, body: clickhouse_response_body)
+    end
+    let(:query) do
+      {
+        'account-id': account.uuid,
+        'from-time': from_time,
+        sampling: 'minute'
+      }
+    end
+
+    context 'when valid params' do
+      it_behaves_like :responds_success
+    end
+
+    context 'without params' do
+      let(:query) { {} }
+      let(:stub_clickhouse_query) { nil }
+
+      include_examples :responds_400, 'missing required param(s) account-id, from-time, sampling'
+    end
+
+    context 'with from-time in the future', freeze_time: Time.parse('2024-12-01 00:00:00 UTC') do
+      let(:query) { super().merge 'from-time': '2024-12-25T05:00:00Z', 'to-time': '2024-12-26T05:00:00Z' }
+
+      before { Log::ApiLog.add_partitions }
+
+      it 'should NOT perform POST request to ClickHouse server and then return empty collection' do
+        expect(CaptureError).not_to receive(:capture)
+        subject
+
+        expect(stub_clickhouse_query).not_to have_been_requested
+        expect(response_json).to eq([])
+      end
+    end
+
+    context 'when the "from-time" is invalid Date' do
+      let(:query) { super().merge 'from-time': 'invalid' }
+
+      include_examples :responds_400, 'invalid value from-time'
+    end
+
+    context 'when the "to-time" is invalid Date' do
+      let(:query) { super().merge 'to-time': 'invalid' }
+
+      include_examples :responds_400, 'invalid value to-time'
+    end
+  end
+end