Skip to content

Commit

Permalink
[frontend] Synchronize serialization of comment and event payload
Browse files Browse the repository at this point in the history
SendEventEmailsJob is creating notifications from certain events. As part
of this the serialized event payload is copied to the 'event_payload'
attribute.
Both models were using different serialization libraries (Yajl::Encoder
for events, ActiveSupport::JSON.encode for notifications), which could
cause the stored data format to differ, eg. active support encodes
certains chars differently.
In general this was not causing any trouble, because the decoded data
would be the same. But it could happen that the payload of the
notification is bigger than the original event payload and exceeds the
size limit of that field. In such a case the notification creation
failed with a ActiveRecord::ValueTooLong error.

By using the same serialization method in both places, we avoid this
from happening. Since notifications are created from events, and the
event payload length is already sanitized based on that serialization
method used by the event model, we pick the Yajl::Encoder.

Fixes openSUSE#3638 openSUSE#4161
  • Loading branch information
bgeuken committed Jan 8, 2018
1 parent 71a27aa commit 675fb11
Show file tree
Hide file tree
Showing 4 changed files with 16 additions and 4 deletions.
2 changes: 1 addition & 1 deletion src/api/app/jobs/send_event_emails_job.rb
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ def create_rss_notifications(event)
Notification::RssFeedItem.create(
subscriber: subscription.subscriber,
event_type: event.eventtype,
event_payload: event.payload,
event_payload: Yajl::Encoder.encode(event.payload),
subscription_receiver_role: subscription.receiver_role
)
end
Expand Down
6 changes: 4 additions & 2 deletions src/api/app/models/notification.rb
Original file line number Diff line number Diff line change
@@ -1,11 +1,13 @@
class Notification < ApplicationRecord
belongs_to :subscriber, polymorphic: true

serialize :event_payload, JSON

def event
@event ||= event_type.constantize.new(event_payload)
end

def event_payload
@event_payload ||= Yajl::Parser.parse(self[:event_payload])
end
end

# == Schema Information
Expand Down
4 changes: 3 additions & 1 deletion src/api/spec/controllers/webui/feeds_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,9 @@
when: '2017-06-27T10:34:30',
who: 'heino' }
end
let!(:rss_notification) { create(:rss_notification, event_payload: payload, subscriber: user, event_type: 'Event::RequestCreate') }
let!(:rss_notification) do
create(:rss_notification, event_payload: Yajl::Encoder.encode(payload), subscriber: user, event_type: 'Event::RequestCreate')
end

context 'with a working token' do
render_views
Expand Down
8 changes: 8 additions & 0 deletions src/api/spec/jobs/send_event_emails_job_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,14 @@
expect(raw_event_payload).to eq(raw_notification_payload)
end

it 'serializes payloads the same way as event payloads are serialized' do
notification = Notification.find_by(subscriber: user)
event_payload = Event::Base.first.attributes['payload']
notification_payload = notification.attributes['event_payload']

expect(event_payload).to eq(notification_payload)
end

it "creates an rss notification for group's email" do
notification = Notification::RssFeedItem.find_by(subscriber: group)

Expand Down

0 comments on commit 675fb11

Please sign in to comment.