Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Misc(events): add timestamp validation for create event #2978

Merged
merged 5 commits into from
Dec 20, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion app/services/events/create_batch_service.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
4 changes: 3 additions & 1 deletion app/services/events/create_service.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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!
Expand All @@ -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
Expand Down
12 changes: 12 additions & 0 deletions app/services/events/validate_creation_service.rb
Original file line number Diff line number Diff line change
Expand Up @@ -37,13 +37,21 @@ 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?

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?

Expand Down Expand Up @@ -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
Expand Down
56 changes: 56 additions & 0 deletions spec/requests/api/v1/events_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
29 changes: 29 additions & 0 deletions spec/services/events/create_batch_service_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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')") }

Expand Down
11 changes: 11 additions & 0 deletions spec/services/events/create_service_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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) }

Expand Down
26 changes: 26 additions & 0 deletions spec/services/events/validate_creation_service_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Loading