From d10623042c3c3d703aaa66e7dc49056df7684493 Mon Sep 17 00:00:00 2001 From: Peter Hankiewicz Date: Fri, 4 Feb 2022 20:57:22 +0100 Subject: [PATCH] Start using jsonapi-serializer gem instead of active_model_serializers --- Gemfile | 3 +- Gemfile.lock | 6 +- app/controllers/notices_controller.rb | 6 +- app/controllers/search_controller.rb | 9 ++- app/controllers/topics_controller.rb | 2 +- app/models/notice.rb | 4 +- app/models/notice_serializer_proxy.rb | 2 +- app/serializers/base_serializer.rb | 7 ++ app/serializers/court_order_serializer.rb | 14 ++-- app/serializers/data_protection_serializer.rb | 10 +-- app/serializers/defamation_serializer.rb | 10 +-- app/serializers/entity_serializer.rb | 4 +- .../law_enforcement_request_serializer.rb | 12 ++-- app/serializers/limited_notice_serializer.rb | 24 +++---- app/serializers/notice_serializer.rb | 66 +++++++++---------- app/serializers/other_serializer.rb | 10 ++- .../private_information_serializer.rb | 10 ++- .../rescinded_notice_serializer.rb | 7 +- app/serializers/topic_serializer.rb | 2 +- app/serializers/trademark_serializer.rb | 15 ++--- config/initializers/jsonapi-serializer.rb | 22 +++++++ config/initializers/rack-attack.rb | 4 ++ lib/rack/attack/request.rb | 8 ++- spec/controllers/notices_controller_spec.rb | 7 +- spec/models/notice_serializer_proxy_spec.rb | 2 +- spec/serializers/notice_serializer_spec.rb | 5 +- spec/serializers/trademark_serializer_spec.rb | 4 +- .../serialized_notice_with_base_metadata.rb | 5 +- 28 files changed, 143 insertions(+), 137 deletions(-) create mode 100644 app/serializers/base_serializer.rb create mode 100644 config/initializers/jsonapi-serializer.rb diff --git a/Gemfile b/Gemfile index f2f2c2adc..4220f9607 100644 --- a/Gemfile +++ b/Gemfile @@ -1,7 +1,5 @@ source 'https://rubygems.org' -# Pinned due to development difficulties with AMS 0.9/0.10. -gem 'active_model_serializers', '~> 0.8.3' gem 'activerecord-import' gem 'acts-as-taggable-on' gem 'addressable' @@ -23,6 +21,7 @@ gem 'flutie' gem 'jquery-placeholder-rails' gem 'jquery-rails' gem 'jquery-ui-rails' +gem 'jsonapi-serializer' gem 'kaminari' gem 'lograge' gem 'mime-types' diff --git a/Gemfile.lock b/Gemfile.lock index 281bddd69..3ad579b5d 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -42,8 +42,6 @@ GEM active_link_to (1.0.5) actionpack addressable - active_model_serializers (0.8.4) - activemodel (>= 3.0) activejob (6.1.4.4) activesupport (= 6.1.4.4) globalid (>= 0.3.6) @@ -256,6 +254,8 @@ GEM jquery-ui-rails (6.0.1) railties (>= 3.2.16) json (2.6.1) + jsonapi-serializer (2.2.0) + activesupport (>= 4.2) kaminari (1.2.2) activesupport (>= 4.1.0) kaminari-actionview (= 1.2.2) @@ -533,7 +533,6 @@ PLATFORMS ruby DEPENDENCIES - active_model_serializers (~> 0.8.3) activerecord-import acts-as-taggable-on addressable @@ -562,6 +561,7 @@ DEPENDENCIES jquery-placeholder-rails jquery-rails jquery-ui-rails + jsonapi-serializer kaminari lograge memory_profiler diff --git a/app/controllers/notices_controller.rb b/app/controllers/notices_controller.rb index aeb378370..b16dd2f9b 100644 --- a/app/controllers/notices_controller.rb +++ b/app/controllers/notices_controller.rb @@ -50,7 +50,7 @@ def create Rails.logger.warn "Could not create notice with params: #{params}" flash.alert = 'Notice creation failed. See errors below.' format.html { render :new, status: :unprocessable_entity } - format.json { render json: @notice.errors, status: :unprocessable_entity } + format.json { render json: { notices: @notice.errors }, status: :unprocessable_entity } end end end @@ -68,9 +68,7 @@ def show respond_to do |format| format.html { show_render_html } format.json do - render json: @notice, - serializer: NoticeSerializerProxy, - root: json_root_for(@notice.class) + render json: { json_root_for(@notice.class) => NoticeSerializerProxy.new(@notice) } end end end diff --git a/app/controllers/search_controller.rb b/app/controllers/search_controller.rb index 8d2f1c257..ec2fece2b 100644 --- a/app/controllers/search_controller.rb +++ b/app/controllers/search_controller.rb @@ -52,11 +52,10 @@ def json_renderer # subclass overrides to these constants, rather than pulling in the # original definition. render( - json: @wrapped_instances, - each_serializer: self.class::EACH_SERIALIZER, - serializer: ActiveModel::ArraySerializer, - root: self.class::URL_ROOT, - meta: meta_hash_for(@searchdata) + json: { + self.class::URL_ROOT => @wrapped_instances.map { |instance| self.class::EACH_SERIALIZER.new(instance) }, + meta: meta_hash_for(@searchdata) + } ) end diff --git a/app/controllers/topics_controller.rb b/app/controllers/topics_controller.rb index 5b553f29c..6569feee0 100644 --- a/app/controllers/topics_controller.rb +++ b/app/controllers/topics_controller.rb @@ -1,6 +1,6 @@ class TopicsController < ApplicationController def index - render json: Topic.all + render json: { topics: TopicSerializer.new(Topic.all) } end def show diff --git a/app/models/notice.rb b/app/models/notice.rb index e32ac89ae..53decbcb7 100644 --- a/app/models/notice.rb +++ b/app/models/notice.rb @@ -231,11 +231,11 @@ def language_enum Language.all.inject( {} ) { |memo, l| memo[l.label] = l.code; memo } end - def active_model_serializer + def model_serializer if rescinded? RescindedNoticeSerializer else - super + "#{self.class.name}Serializer".safe_constantize end end diff --git a/app/models/notice_serializer_proxy.rb b/app/models/notice_serializer_proxy.rb index 385110380..16050619d 100644 --- a/app/models/notice_serializer_proxy.rb +++ b/app/models/notice_serializer_proxy.rb @@ -1,6 +1,6 @@ class NoticeSerializerProxy < SimpleDelegator def initialize(instance, *args) - serializer = instance.active_model_serializer || NoticeSerializer + serializer = instance.model_serializer || NoticeSerializer serialized = serializer.new(instance, *args) super(serialized) diff --git a/app/serializers/base_serializer.rb b/app/serializers/base_serializer.rb new file mode 100644 index 000000000..27c12e308 --- /dev/null +++ b/app/serializers/base_serializer.rb @@ -0,0 +1,7 @@ +class BaseSerializer + include JSONAPI::Serializer + + def as_json(options={}) + serializable_hash[:data] + end +end diff --git a/app/serializers/court_order_serializer.rb b/app/serializers/court_order_serializer.rb index d59c7d60a..27ff6defd 100644 --- a/app/serializers/court_order_serializer.rb +++ b/app/serializers/court_order_serializer.rb @@ -1,20 +1,16 @@ class CourtOrderSerializer < NoticeSerializer - attribute :laws_referenced - - def laws_referenced + attribute :laws_referenced do |object| object.laws_referenced.map(&:name) end - private + attributes_to_serialize.delete(:body) + attribute :explanation, &:body - def attributes - attributes = super - swap_keys(attributes, :body, :explanation) - attributes[:works].each do |work| + attribute :works do |object| + works(object).each do |work| swap_keys(work, 'description', 'subject_of_court_order') swap_keys(work, 'infringing_urls', 'targetted_urls') work.delete('copyrighted_urls') end - attributes end end diff --git a/app/serializers/data_protection_serializer.rb b/app/serializers/data_protection_serializer.rb index 8c278f538..3a65f4bff 100644 --- a/app/serializers/data_protection_serializer.rb +++ b/app/serializers/data_protection_serializer.rb @@ -1,10 +1,10 @@ class DataProtectionSerializer < NoticeSerializer - def attributes - attributes = super - swap_keys(attributes, :body, :legal_complaint) - attributes[:works].each do |work| + attributes_to_serialize.delete(:body) + attribute :legal_complaint, &:body + + attribute :works do |object| + works(object).each do |work| swap_keys(work, 'infringing_urls', 'urls_mentioned_in_request') end - attributes end end diff --git a/app/serializers/defamation_serializer.rb b/app/serializers/defamation_serializer.rb index 8977c8335..785ca5e7b 100644 --- a/app/serializers/defamation_serializer.rb +++ b/app/serializers/defamation_serializer.rb @@ -1,11 +1,11 @@ class DefamationSerializer < NoticeSerializer - def attributes - attributes = super - swap_keys(attributes, :body, :legal_complaint) - attributes[:works].each do |work| + attributes_to_serialize.delete(:body) + attribute :legal_complaint, &:body + + attribute :works do |object| + works(object).each do |work| swap_keys(work, 'infringing_urls', 'defamatory_urls') work.delete('copyrighted_urls') end - attributes end end diff --git a/app/serializers/entity_serializer.rb b/app/serializers/entity_serializer.rb index b0e92704f..fd2366049 100644 --- a/app/serializers/entity_serializer.rb +++ b/app/serializers/entity_serializer.rb @@ -1,3 +1,3 @@ -class EntitySerializer < ActiveModel::Serializer - attributes :id, :parent_id, :name, :country_code, :url +class EntitySerializer < BaseSerializer + attributes :id, :parent_id, :name, :country_code, :url end diff --git a/app/serializers/law_enforcement_request_serializer.rb b/app/serializers/law_enforcement_request_serializer.rb index d38b07420..5ede0cf7b 100644 --- a/app/serializers/law_enforcement_request_serializer.rb +++ b/app/serializers/law_enforcement_request_serializer.rb @@ -1,20 +1,18 @@ class LawEnforcementRequestSerializer < NoticeSerializer attributes :regulations, :request_type - def regulations + attribute :regulations do |object| object.regulation_list.map(&:name) end - private + attributes_to_serialize.delete(:body) + attribute :explanation, &:body - def attributes - attributes = super - swap_keys(attributes, :body, :explanation) - attributes[:works].each do |work| + attribute :works do |object| + works(object).each do |work| swap_keys(work, 'description', 'subject_of_enforcement_request') swap_keys(work, 'copyrighted_urls', 'original_work_urls') swap_keys(work, 'infringing_urls', 'urls_in_request') end - attributes end end diff --git a/app/serializers/limited_notice_serializer.rb b/app/serializers/limited_notice_serializer.rb index 2f698e7fa..d590051d3 100644 --- a/app/serializers/limited_notice_serializer.rb +++ b/app/serializers/limited_notice_serializer.rb @@ -1,15 +1,15 @@ -class LimitedNoticeSerializer < ActiveModel::Serializer +class LimitedNoticeSerializer < BaseSerializer attributes :id, :type, :title, :body, :date_sent, :date_received, :topics, :sender_name, :principal_name, :recipient_name, :works, :tags, :jurisdictions, :action_taken, :language # TODO: serialize additional entities - def topics + attribute :topics do |object| object.topics.map(&:name) end - def works + attribute :works do |object| object.works.as_json( only: [:description], include: { @@ -19,25 +19,21 @@ def works ) end - def tags + attribute :tags do |object| object.tag_list end - def jurisdictions + attribute :jurisdictions do |object| object.jurisdiction_list end - private - - def attributes - attributes = super || [] - - attributes[:score] = object._score if object.respond_to?(:_score) - - attributes + attribute :score, if: proc { |record| + record.respond_to?(:_score) + } do |object| + object._score end - def swap_keys(hash, original_key, new_key) + def self.swap_keys(hash, original_key, new_key) original_value = hash.delete(original_key) hash[new_key] = original_value end diff --git a/app/serializers/notice_serializer.rb b/app/serializers/notice_serializer.rb index 48195166d..c43487ba0 100644 --- a/app/serializers/notice_serializer.rb +++ b/app/serializers/notice_serializer.rb @@ -1,4 +1,4 @@ -class NoticeSerializer < ActiveModel::Serializer +class NoticeSerializer < BaseSerializer attributes :id, :type, :title, :body, :date_sent, :date_received, :topics, :sender_name, :principal_name, :recipient_name, :works, :tags, :jurisdictions, :action_taken, :language @@ -8,7 +8,7 @@ class NoticeSerializer < ActiveModel::Serializer # TODO: serialize additional entities - def topics + attribute :topics do |object| object.topics.map(&:name) end @@ -20,8 +20,35 @@ def topics # accessible as a hash (rather than as an object to be serialized) so we can # use hash operations on it within the hooks supported by # active-model-serializer. - def works - if defined?(current_user) && current_user && Ability.new(scope).can?(:view_full_version_api, object) + attribute :works do |object| + works(object) + end + + attribute :tags, &:tag_list + + attribute :jurisdictions, &:jurisdiction_list + + attribute :score, if: proc { |record| + record.respond_to?(:_score) + } do |object| + object._score + end + + def self.swap_keys(hash, original_key, new_key) + original_value = hash.delete(original_key) + hash[new_key] = original_value + end + + def self.cleaned_works(base_works) + base_works.each do |work| + work['infringing_urls'] = FALLBACK if work['infringing_urls'].empty? + work['copyrighted_urls'] = FALLBACK if work['copyrighted_urls'].empty? + end + base_works + end + + def self.works(object) + if defined?(Current.user) && Current.user && Ability.new(Current.user).can?(:view_full_version_api, object) base_works = object.works.as_json( only: [:description], include: { @@ -40,35 +67,4 @@ def works end cleaned_works(base_works) end - - def cleaned_works(base_works) - base_works.each do |work| - work['infringing_urls'] = FALLBACK if work['infringing_urls'].empty? - work['copyrighted_urls'] = FALLBACK if work['copyrighted_urls'].empty? - end - base_works - end - - def tags - object.tag_list - end - - def jurisdictions - object.jurisdiction_list - end - - private - - def attributes - attributes = super || [] - - attributes[:score] = object._score if object.respond_to?(:_score) - - attributes - end - - def swap_keys(hash, original_key, new_key) - original_value = hash.delete(original_key) - hash[new_key] = original_value - end end diff --git a/app/serializers/other_serializer.rb b/app/serializers/other_serializer.rb index a7510a066..a2df675ae 100644 --- a/app/serializers/other_serializer.rb +++ b/app/serializers/other_serializer.rb @@ -1,14 +1,12 @@ class OtherSerializer < NoticeSerializer - private + attributes_to_serialize.delete(:body) + attribute :explanation, &:body - def attributes - attributes = super - swap_keys(attributes, :body, :explanation) - attributes[:works].each do |work| + attribute :works do |object| + works(object).each do |work| swap_keys(work, 'description', 'complaint') swap_keys(work, 'copyrighted_urls', 'original_work_urls') swap_keys(work, 'infringing_urls', 'problematic_urls') end - attributes end end diff --git a/app/serializers/private_information_serializer.rb b/app/serializers/private_information_serializer.rb index 7ab5904d9..f5fdfa90d 100644 --- a/app/serializers/private_information_serializer.rb +++ b/app/serializers/private_information_serializer.rb @@ -1,13 +1,11 @@ class PrivateInformationSerializer < NoticeSerializer - private + attributes_to_serialize.delete(:body) + attribute :explanation, &:body - def attributes - attributes = super - swap_keys(attributes, :body, :explanation) - attributes[:works].each do |work| + attribute :works do |object| + works(object).each do |work| swap_keys(work, 'description', 'complaint') swap_keys(work, 'infringing_urls', 'urls_with_private_information') end - attributes end end diff --git a/app/serializers/rescinded_notice_serializer.rb b/app/serializers/rescinded_notice_serializer.rb index 43a493b01..9a71f22e5 100644 --- a/app/serializers/rescinded_notice_serializer.rb +++ b/app/serializers/rescinded_notice_serializer.rb @@ -1,9 +1,8 @@ -class RescindedNoticeSerializer < ActiveModel::Serializer - self.root = :notice - +class RescindedNoticeSerializer < BaseSerializer + set_type :notice attributes :id, :title, :body - def body + attribute :body do 'Notice Rescinded' end end diff --git a/app/serializers/topic_serializer.rb b/app/serializers/topic_serializer.rb index bb67ac119..208ca39fa 100644 --- a/app/serializers/topic_serializer.rb +++ b/app/serializers/topic_serializer.rb @@ -1,3 +1,3 @@ -class TopicSerializer < ActiveModel::Serializer +class TopicSerializer < BaseSerializer attributes :id, :name, :parent_id end diff --git a/app/serializers/trademark_serializer.rb b/app/serializers/trademark_serializer.rb index da8d496f5..43f7c3102 100644 --- a/app/serializers/trademark_serializer.rb +++ b/app/serializers/trademark_serializer.rb @@ -1,8 +1,10 @@ class TrademarkSerializer < NoticeSerializer attributes :marks, :mark_registration_number - def marks - if current_user && Ability.new(scope).can?(:view_full_version_api, object) + attributes_to_serialize.delete(:works) + + attribute :marks do |object| + if Current.user && Ability.new(Current.user).can?(:view_full_version_api, object) object.works.map do |work| { description: work.description, @@ -18,13 +20,4 @@ def marks end.as_json end end - - private - - def attributes - hsh = super - hsh.delete(:works) - - hsh - end end diff --git a/config/initializers/jsonapi-serializer.rb b/config/initializers/jsonapi-serializer.rb new file mode 100644 index 000000000..e541d7d0e --- /dev/null +++ b/config/initializers/jsonapi-serializer.rb @@ -0,0 +1,22 @@ +# Monkeypatch 1 start +# We are only interested in attributes, don't need any metadata included +module FastJsonapi + module SerializationCore + class_methods do + def record_hash(record, fieldset, includes_list, params = {}) + if cache_store_instance + cache_opts = record_cache_options(cache_store_options, fieldset, includes_list, params) + record_hash = cache_store_instance.fetch(record, **cache_opts) do + temp_hash = attributes_hash(record, fieldset, params) + temp_hash + end + else + record_hash = attributes_hash(record, fieldset, params) + end + + record_hash + end + end + end +end +# MonkeyPatch 1 end diff --git a/config/initializers/rack-attack.rb b/config/initializers/rack-attack.rb index 988b2f118..c5beb27f6 100644 --- a/config/initializers/rack-attack.rb +++ b/config/initializers/rack-attack.rb @@ -29,6 +29,10 @@ class Rack::Attack req.admin? end + safelist('allow from custom ips') do |req| + req.additional_allowed? + end + # Disable the API for users without tokens. blocklist('unauthed api limit') do |req| Rails.logger.debug "[rack-attack] api unauthed ip: #{req.ip}, req.env['HTTP_ACCEPT']: #{req.env['HTTP_ACCEPT']}, content_type: #{req.content_type}" diff --git a/lib/rack/attack/request.rb b/lib/rack/attack/request.rb index c7add9644..637af2ffa 100644 --- a/lib/rack/attack/request.rb +++ b/lib/rack/attack/request.rb @@ -9,6 +9,12 @@ def localhost? ['127.0.0.1', '::1'].include? ip end + def additional_allowed? + return false if ENV['RACK_ATTACK_SAFELISTED_IPS'].nil? + + ENV['RACK_ATTACK_SAFELISTED_IPS'].split(',').include? ip + end + def admin? return false unless user (user.roles.include? Role.admin) || (user.roles.include? Role.super_admin) @@ -26,7 +32,7 @@ def authenticated? def token @token ||= if env.key?(LUMEN_HEADER) - Rails.logger.info "[rack-attack] Authentication Token in header: #{env['HTTP_X_AUTHENTICATION_TOKEN']}" + Rails.logger.info "[rack-attack] Authentication Token in header: #{env[LUMEN_HEADER]}" env[LUMEN_HEADER] elsif params[LUMEN_AUTH_TOKEN].present? Rails.logger.info "[rack-attack] Authentication Token in params: #{params[LUMEN_AUTH_TOKEN]}" diff --git a/spec/controllers/notices_controller_spec.rb b/spec/controllers/notices_controller_spec.rb index 15473c1cd..50d30bb5b 100644 --- a/spec/controllers/notices_controller_spec.rb +++ b/spec/controllers/notices_controller_spec.rb @@ -63,12 +63,11 @@ it "returns a serialized notice for #{model_class}" do notice = stub_find_notice(model_class.new) - serializer_class = model_class.active_model_serializer || NoticeSerializer + serializer_class = notice.model_serializer || NoticeSerializer serialized = serializer_class.new(notice) - allow(serialized).to receive(:current_user).and_return(nil) - expect(serializer_class).to receive(:new). - with(notice, anything) + expect(serializer_class).to receive(:new) + .with(notice) .and_return(serialized) get :show, params: { id: 1, format: :json } diff --git a/spec/models/notice_serializer_proxy_spec.rb b/spec/models/notice_serializer_proxy_spec.rb index c90e70fa0..4105cd6d5 100644 --- a/spec/models/notice_serializer_proxy_spec.rb +++ b/spec/models/notice_serializer_proxy_spec.rb @@ -14,7 +14,7 @@ end it "uses NoticeSerializer when no custom serializer is present" do - object = double('Instance', active_model_serializer: nil) + object = double('Instance', model_serializer: nil) serialized = NoticeSerializer.new(object) allow(serialized).to receive(:a_method).and_return(:a_value) # test delegation expect(NoticeSerializer).to receive(:new).with(object).and_return(serialized) diff --git a/spec/serializers/notice_serializer_spec.rb b/spec/serializers/notice_serializer_spec.rb index aa2f52c06..f497d54a1 100644 --- a/spec/serializers/notice_serializer_spec.rb +++ b/spec/serializers/notice_serializer_spec.rb @@ -12,6 +12,7 @@ %i|infringing_urls copyrighted_urls|.each do |url_relation| it "includes #{url_relation}" do allow_any_instance_of(Ability).to receive(:can?).and_return(true) + allow_any_instance_of(Current).to receive(:user).and_return(create(:user, :admin)) with_a_serialized_notice do |notice, json| relation_json = json[:works].first[url_relation.to_s].map{ |u| u['url'] } @@ -27,9 +28,7 @@ notice = build_notice allow(notice).to receive(:_score).and_return(2) serializer = NoticeSerializer.new(notice) - allow(serializer).to receive(:current_user).and_return(nil) - - score = serializer.as_json[:notice][:score] + score = serializer.as_json[:score] expect(score).to eq 2 end diff --git a/spec/serializers/trademark_serializer_spec.rb b/spec/serializers/trademark_serializer_spec.rb index 526759847..f066ab9f5 100644 --- a/spec/serializers/trademark_serializer_spec.rb +++ b/spec/serializers/trademark_serializer_spec.rb @@ -8,7 +8,7 @@ trademark = build(:trademark, works: [work], mark_registration_number: '1337') serialized = TrademarkSerializer.new(trademark) allow(serialized).to receive(:current_user).and_return(nil) - serialized_trademark = serialized.as_json[:trademark] + serialized_trademark = serialized.as_json first_mark = serialized_trademark[:marks].first @@ -24,7 +24,7 @@ serialized = TrademarkSerializer.new(trademark) allow(serialized).to receive(:current_user).and_return(nil) - serialized_trademark = serialized.as_json[:trademark] + serialized_trademark = serialized.as_json mark = serialized_trademark[:marks].first expect(mark['infringing_urls']).to eq( diff --git a/spec/support/shared_examples/serialized_notice_with_base_metadata.rb b/spec/support/shared_examples/serialized_notice_with_base_metadata.rb index e10fc8891..0327b0c59 100644 --- a/spec/support/shared_examples/serialized_notice_with_base_metadata.rb +++ b/spec/support/shared_examples/serialized_notice_with_base_metadata.rb @@ -34,9 +34,8 @@ def with_a_serialized_notice(factory_name = :dmca) notice = build_notice(factory_name) - serializer = described_class.new(notice, root: factory_name) - allow(serializer).to receive(:current_user).and_return(true) - yield notice, serializer.as_json[factory_name] + serializer = described_class.new(notice) + yield notice, serializer.as_json end def build_notice(factory_name = :dmca)