Skip to content

Commit

Permalink
[api][webui] Update Notification to use JSON serialisation.
Browse files Browse the repository at this point in the history
Otherwise the Event.payload gets parsed from json, then serialised as YAML
into Notification.event_payload which sometimes causes errors because
the payload can fit into the events table (with JSON serialisation) but
might not fit into notifications.event_payload becuase YAML
serialisation takes up more characters than JSON does.
  • Loading branch information
evanrolfe authored and Evan Rolfe committed Oct 23, 2017
1 parent a825ee6 commit b42ddff
Show file tree
Hide file tree
Showing 6 changed files with 92 additions and 1 deletion.
5 changes: 5 additions & 0 deletions ReleaseNotes-2.9
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,11 @@ OBS Appliance users who have setup their LVM can just replace
their appliance image without data loss. The migration will
happen automatically.

Upgrading from previous versions:
=================================

* Please run this rake task if you notifications table has any rows in it:
rake db:convert_notifications_serialization

Features
========
Expand Down
2 changes: 1 addition & 1 deletion src/api/app/models/notification.rb
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
class Notification < ApplicationRecord
belongs_to :subscriber, polymorphic: true

serialize :event_payload, Hash
serialize :event_payload, JSON

def event
@event ||= event_type.constantize.new(event_payload)
Expand Down
53 changes: 53 additions & 0 deletions src/api/lib/tasks/databases.rake
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
require_relative '../../app/models/application_record'

module Rake
module TaskManager
def redefine_task(task_class, *args, &block)
Expand Down Expand Up @@ -120,4 +122,55 @@ namespace :db do
Rake::Task["db:structure:load"].invoke
Rake::Task["db:seed"].invoke
end

desc "Convert existing notifications to use JSON serialization for the event_payload column"
task convert_notifications_serialization: :environment do
NotificationForRakeTask.transaction do
NotificationForRakeTask.all.find_each do |notification|
json = yaml_to_json(notification.event_payload)
notification.update_attributes!(event_payload: json)
end
end
end
end

def yaml_to_json(yaml)
YAML.safe_load(yaml)
.traverse do |value|
if value.is_a? String
value.force_encoding('UTF-8')
else
value
end
end
.to_json
end

# Notification model only for migration in order to avoid errors coming from the serialization in the actual Notification model
class NotificationForRakeTask < ::ApplicationRecord
self.table_name = 'notifications'
self.inheritance_column = :_type_disabled
end

# Hash extension is used to run force_encoding against each string value in the hash
# in rake tasks to convert yaml to json serialisation for event payloads
class Hash
def traverse(&block)
traverse_value(self, &block)
end

private

def traverse_value(value, &block)
if value.is_a? Hash
value.each { |key, sub_value| value[key] = traverse_value(sub_value, &block) }

elsif value.is_a? Array
value.map { |element| traverse_value(element, &block) }

else
yield(value)

end
end
end
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 @@ -43,6 +43,14 @@
expect(notification.delivered).to be_falsey
end

it "creates an rss notification with the same raw value of the corresponding event's payload" do
notification = Notification.find_by(subscriber: user)
raw_event_payload = Event::Base.first.attributes_before_type_cast['payload']
raw_notification_payload = notification.attributes_before_type_cast['event_payload']

expect(raw_event_payload).to eq(raw_notification_payload)
end

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

Expand Down
21 changes: 21 additions & 0 deletions src/api/spec/lib/tasks/databases_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
require 'rails_helper'

RSpec.describe 'rake db' do
describe '#convert_notifications_serialization' do
let!(:yaml) { "---\nhello: world\nhow:\n- are\n- you\n- today?\nim: fine thanks\n" }
let!(:notification) { create(:notification, type: 'Notification::RssFeedItem') }
let!(:task) { Rake::Task['db:convert_notifications_serialization'] }

before do
sql = "UPDATE `notifications` SET `event_payload` = '#{yaml}' WHERE id = #{notification.id}"
ActiveRecord::Base.connection.execute(sql)
end

subject! { task.execute }

it 'converts the notifications event_payload from yaml to json' do
json_hash = { "hello" => "world", "how" => ["are", "you", "today?"], "im" => "fine thanks"}
expect(Notification.first.event_payload).to eq(json_hash)
end
end
end
4 changes: 4 additions & 0 deletions src/api/spec/spec_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,10 @@
config.formatter = 'NyanUnicornFormatter'
end
end

config.before(:suite) do
Rails.application.load_tasks
end
end

# We never want the backend to autostart itself...
Expand Down

0 comments on commit b42ddff

Please sign in to comment.