diff --git a/app/services/events/create_batch_service.rb b/app/services/events/create_batch_service.rb index 14d19685128..5a7b89c321c 100644 --- a/app/services/events/create_batch_service.rb +++ b/app/services/events/create_batch_service.rb @@ -47,13 +47,15 @@ def validate_events event.external_subscription_id = event_params[:external_subscription_id] event.properties = event_params[:properties] || {} event.metadata = metadata || {} - event.timestamp = Time.zone.at(event_params[:timestamp] ? event_params[:timestamp].to_f : timestamp) + event.timestamp = Time.zone.at(event_params[:timestamp] ? Float(event_params[:timestamp]) : timestamp) event.precise_total_amount_cents = event_params[:precise_total_amount_cents] CalculateExpressionService.call(organization:, event:).raise_if_error! result.events.push(event) result.errors = result.errors.merge({index => event.errors.messages}) unless event.valid? + rescue ArgumentError + result.errors = result.errors.merge({index => {timestamp: ['invalid_format']}}) end end diff --git a/app/services/events/create_service.rb b/app/services/events/create_service.rb index 1fae7bbdbcb..925f539ae85 100644 --- a/app/services/events/create_service.rb +++ b/app/services/events/create_service.rb @@ -18,7 +18,7 @@ def call event.external_subscription_id = params[:external_subscription_id] event.properties = params[:properties] || {} event.metadata = metadata || {} - event.timestamp = Time.zone.at(params[:timestamp] ? params[:timestamp].to_f : timestamp) + event.timestamp = Time.zone.at(params[:timestamp] ? Float(params[:timestamp]) : timestamp) event.precise_total_amount_cents = params[:precise_total_amount_cents] CalculateExpressionService.call(organization:, event:).raise_if_error! @@ -35,6 +35,8 @@ def call result.record_validation_failure!(record: e.record) rescue ActiveRecord::RecordNotUnique result.single_validation_failure!(field: :transaction_id, error_code: 'value_already_exist') + rescue ArgumentError + result.single_validation_failure!(field: :timestamp, error_code: 'invalid_format') end private diff --git a/app/services/events/validate_creation_service.rb b/app/services/events/validate_creation_service.rb index a4ced8c4254..46578d639cb 100644 --- a/app/services/events/validate_creation_service.rb +++ b/app/services/events/validate_creation_service.rb @@ -37,6 +37,7 @@ def validate_create return missing_subscription_error end + return invalid_timestamp_error unless valid_timestamp? return transaction_id_error unless valid_transaction_id? return invalid_code_error unless valid_code? return invalid_properties_error unless valid_properties? @@ -44,6 +45,13 @@ def validate_create nil end + def valid_timestamp? + return true if params[:timestamp].blank? + + # timestamp is a number of seconds + valid_number?(params[:timestamp]) + end + def valid_transaction_id? return false if params[:transaction_id].blank? @@ -92,6 +100,10 @@ def invalid_customer_error result.not_found_failure!(resource: 'customer') end + def invalid_timestamp_error + result.validation_failure!(errors: {timestamp: ['invalid_format']}) + end + def billable_metric @billable_metric ||= organization.billable_metrics.find_by(code: params[:code]) end diff --git a/spec/requests/api/v1/events_controller_spec.rb b/spec/requests/api/v1/events_controller_spec.rb index d9053105430..14a66ab94ef 100644 --- a/spec/requests/api/v1/events_controller_spec.rb +++ b/spec/requests/api/v1/events_controller_spec.rb @@ -58,6 +58,28 @@ expect(response).to have_http_status(:unprocessable_entity) end end + + context 'when sending wrong format for the timestamp' do + let(:create_params) do + { + code: metric.code, + transaction_id: SecureRandom.uuid, + external_subscription_id: subscription.external_id, + timestamp: Time.current.to_s, + precise_total_amount_cents: '123.45', + properties: { + foo: 'bar' + } + } + end + + it 'returns a not found response' do + expect { subject }.not_to change(Event, :count) + + expect(response).to have_http_status(:unprocessable_entity) + expect(json[:error_details]).to eq({timestamp: ["invalid_format"]}) + end + end end describe 'POST /api/v1/events/batch' do @@ -88,6 +110,40 @@ expect(response).to have_http_status(:ok) expect(json[:events].first[:external_subscription_id]).to eq(subscription.external_id) end + + context 'with invalid timestamp for one event' do + let(:batch_params) do + [ + { + code: metric.code, + transaction_id: SecureRandom.uuid, + external_subscription_id: subscription.external_id, + timestamp: Time.current.to_i, + precise_total_amount_cents: '123.45', + properties: { + foo: 'bar' + } + }, + { + code: metric.code, + transaction_id: SecureRandom.uuid, + external_subscription_id: subscription.external_id, + timestamp: Time.current.to_s, + precise_total_amount_cents: '123.45', + properties: { + foo: 'bar' + } + } + ] + end + + it 'returns an error indicating which event contained which error' do + expect { subject }.not_to change(Event, :count) + + expect(response).to have_http_status(:unprocessable_entity) + expect(json[:error_details]).to eq({'1': {timestamp: ["invalid_format"]}}) + end + end end describe 'GET /api/v1/events' do diff --git a/spec/services/events/create_batch_service_spec.rb b/spec/services/events/create_batch_service_spec.rb index 8ff38306cb7..8d859e3c516 100644 --- a/spec/services/events/create_batch_service_spec.rb +++ b/spec/services/events/create_batch_service_spec.rb @@ -196,6 +196,35 @@ end end + context 'when timestamp is in a wrong format' do + let(:timestamp) { Time.current.to_s } + let(:events_params) do + { + events: [ + { + external_customer_id: SecureRandom.uuid, + external_subscription_id: SecureRandom.uuid, + code:, + transaction_id: SecureRandom.uuid, + precise_total_amount_cents:, + properties: {foo: 'bar'}, + timestamp: + } + ] + } + end + + it 'returns an error' do + result = nil + expect { result = create_batch_service.call }.not_to change(Event, :count) + + expect(result).not_to be_success + expect(result.error).to be_a(BaseService::ValidationFailure) + expect(result.error.messages.keys).to include(0) + expect(result.error.messages[0][:timestamp]).to include('invalid_format') + end + end + context "with an expression configured on the billable metric" do let(:billable_metric) { create(:billable_metric, code:, organization:, field_name: "result", expression: "concat(event.properties.foo, '-bar')") } diff --git a/spec/services/events/create_service_spec.rb b/spec/services/events/create_service_spec.rb index d512753ce32..cd87e1b0e63 100644 --- a/spec/services/events/create_service_spec.rb +++ b/spec/services/events/create_service_spec.rb @@ -117,6 +117,17 @@ end end + context 'when timestamp is given in a wrong format' do + let(:timestamp) { Time.current.to_s } + + it 'returns an error' do + result = create_service.call + + expect(result).not_to be_success + expect(result.error.messages).to include({timestamp: ['invalid_format']}) + end + end + context 'when kafka is configured' do let(:karafka_producer) { instance_double(WaterDrop::Producer) } diff --git a/spec/services/events/validate_creation_service_spec.rb b/spec/services/events/validate_creation_service_spec.rb index 3bb0c8d208e..58685b453e1 100644 --- a/spec/services/events/validate_creation_service_spec.rb +++ b/spec/services/events/validate_creation_service_spec.rb @@ -254,5 +254,31 @@ end end end + + context 'when timestamp is in a wrong format' do + let(:params) do + {external_customer_id: customer.external_id, code: billable_metric.code, transaction_id:, timestamp: '2025-01-01'} + end + + it 'returns a timestamp_is_not_valid error' do + validate_event + + expect(result).not_to be_success + expect(result.error).to be_a(BaseService::ValidationFailure) + expect(result.error.messages.keys).to include(:timestamp) + expect(result.error.messages[:timestamp]).to include('invalid_format') + end + end + + context 'when timestamp is valid' do + let(:params) do + {external_customer_id: customer.external_id, code: billable_metric.code, transaction_id:, timestamp: Time.current.to_i + 0.11} + end + + it 'does not raise any errors' do + expect(validate_event).to be_nil + expect(result).to be_success + end + end end end