From 67f7ffa79270e3591478a95a1a083bfffa1fd218 Mon Sep 17 00:00:00 2001 From: Akihiko Odaki Date: Thu, 8 Feb 2018 00:35:44 +0900 Subject: [PATCH 1/5] Change user_id column non-nullable (#6435) --- app/models/invite.rb | 2 +- app/models/web/setting.rb | 2 +- db/migrate/20180206000000_change_user_id_nonnullable.rb | 6 ++++++ db/schema.rb | 6 +++--- 4 files changed, 11 insertions(+), 5 deletions(-) create mode 100644 db/migrate/20180206000000_change_user_id_nonnullable.rb diff --git a/app/models/invite.rb b/app/models/invite.rb index b87a3b722..4ba5432d2 100644 --- a/app/models/invite.rb +++ b/app/models/invite.rb @@ -4,7 +4,7 @@ # Table name: invites # # id :integer not null, primary key -# user_id :integer +# user_id :integer not null # code :string default(""), not null # expires_at :datetime # max_uses :integer diff --git a/app/models/web/setting.rb b/app/models/web/setting.rb index 12b9d1226..0a5129d17 100644 --- a/app/models/web/setting.rb +++ b/app/models/web/setting.rb @@ -7,7 +7,7 @@ # data :json # created_at :datetime not null # updated_at :datetime not null -# user_id :integer +# user_id :integer not null # class Web::Setting < ApplicationRecord diff --git a/db/migrate/20180206000000_change_user_id_nonnullable.rb b/db/migrate/20180206000000_change_user_id_nonnullable.rb new file mode 100644 index 000000000..4eecb6154 --- /dev/null +++ b/db/migrate/20180206000000_change_user_id_nonnullable.rb @@ -0,0 +1,6 @@ +class ChangeUserIdNonnullable < ActiveRecord::Migration[5.1] + def change + change_column_null :invites, :user_id, false + change_column_null :web_settings, :user_id, false + end +end diff --git a/db/schema.rb b/db/schema.rb index 02e84cbd1..281110124 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 20180204034416) do +ActiveRecord::Schema.define(version: 20180206000000) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -195,7 +195,7 @@ ActiveRecord::Schema.define(version: 20180204034416) do end create_table "invites", force: :cascade do |t| - t.bigint "user_id" + t.bigint "user_id", null: false t.string "code", default: "", null: false t.datetime "expires_at" t.integer "max_uses" @@ -516,7 +516,7 @@ ActiveRecord::Schema.define(version: 20180204034416) do t.json "data" t.datetime "created_at", null: false t.datetime "updated_at", null: false - t.bigint "user_id" + t.bigint "user_id", null: false t.index ["user_id"], name: "index_web_settings_on_user_id", unique: true end From 2bb393684baf5da1112923db7bcda920d606be78 Mon Sep 17 00:00:00 2001 From: Kazushige Tominaga Date: Thu, 8 Feb 2018 08:17:53 +0900 Subject: [PATCH 2/5] Added #link_header spec (#6439) --- spec/services/fetch_atom_service_spec.rb | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/spec/services/fetch_atom_service_spec.rb b/spec/services/fetch_atom_service_spec.rb index 5491fd027..fcba55e8d 100644 --- a/spec/services/fetch_atom_service_spec.rb +++ b/spec/services/fetch_atom_service_spec.rb @@ -1,4 +1,24 @@ require 'rails_helper' RSpec.describe FetchAtomService do + describe '#link_header' do + context 'Link is Array' do + target = FetchAtomService.new + target.instance_variable_set('@response', 'Link' => [ + '; rel="up"; meta="bar"', + '; rel="self"', + ]) + + it 'set first link as link_header' do + expect(target.send(:link_header).links[0].href).to eq 'http://example.com/' + end + end + + context 'Link is not Array' do + target = FetchAtomService.new + target.instance_variable_set('@response', 'Link' => '; rel="self", ; rel = "up"') + + it { expect(target.send(:link_header).links[0].href).to eq 'http://example.com/foo' } + end + end end From cf32f7da5c5af7c86af3cab89d18cdbe7b35f4a2 Mon Sep 17 00:00:00 2001 From: abcang Date: Thu, 8 Feb 2018 13:00:45 +0900 Subject: [PATCH 3/5] Fix response of signature_verification_failure_reason (#6441) --- .../activitypub/inboxes_controller.rb | 2 +- app/controllers/api/salmon_controller.rb | 4 +++- spec/controllers/api/salmon_controller_spec.rb | 16 +++++++++++++++- 3 files changed, 19 insertions(+), 3 deletions(-) diff --git a/app/controllers/activitypub/inboxes_controller.rb b/app/controllers/activitypub/inboxes_controller.rb index 7d0bc74d3..af51e32d5 100644 --- a/app/controllers/activitypub/inboxes_controller.rb +++ b/app/controllers/activitypub/inboxes_controller.rb @@ -11,7 +11,7 @@ class ActivityPub::InboxesController < Api::BaseController process_payload head 202 else - [signature_verification_failure_reason, 401] + render plain: signature_verification_failure_reason, status: 401 end end diff --git a/app/controllers/api/salmon_controller.rb b/app/controllers/api/salmon_controller.rb index 143e9d3cd..ac5f3268d 100644 --- a/app/controllers/api/salmon_controller.rb +++ b/app/controllers/api/salmon_controller.rb @@ -1,6 +1,8 @@ # frozen_string_literal: true class Api::SalmonController < Api::BaseController + include SignatureVerification + before_action :set_account respond_to :txt @@ -9,7 +11,7 @@ class Api::SalmonController < Api::BaseController process_salmon head 202 elsif payload.present? - [signature_verification_failure_reason, 401] + render plain: signature_verification_failure_reason, status: 401 else head 400 end diff --git a/spec/controllers/api/salmon_controller_spec.rb b/spec/controllers/api/salmon_controller_spec.rb index 323d85b61..8af8b83a8 100644 --- a/spec/controllers/api/salmon_controller_spec.rb +++ b/spec/controllers/api/salmon_controller_spec.rb @@ -40,7 +40,7 @@ RSpec.describe Api::SalmonController, type: :controller do end end - context 'with invalid post data' do + context 'with empty post data' do before do request.env['RAW_POST_DATA'] = '' post :update, params: { id: account.id } @@ -50,5 +50,19 @@ RSpec.describe Api::SalmonController, type: :controller do expect(response).to have_http_status(400) end end + + context 'with invalid post data' do + before do + service = double(call: false) + allow(VerifySalmonService).to receive(:new).and_return(service) + + request.env['RAW_POST_DATA'] = File.read(File.join(Rails.root, 'spec', 'fixtures', 'salmon', 'mention.xml')) + post :update, params: { id: account.id } + end + + it 'returns http client error' do + expect(response).to have_http_status(401) + end + end end end From 298c81c00f951241f026b0b3f711ead405b78fbe Mon Sep 17 00:00:00 2001 From: abcang Date: Thu, 8 Feb 2018 23:33:23 +0900 Subject: [PATCH 4/5] Clear account cache of notification target_status (#6442) --- app/models/notification.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/app/models/notification.rb b/app/models/notification.rb index 733f89cf7..7f8dae5ec 100644 --- a/app/models/notification.rb +++ b/app/models/notification.rb @@ -69,7 +69,7 @@ class Notification < ApplicationRecord class << self def reload_stale_associations!(cached_items) - account_ids = cached_items.map(&:from_account_id).uniq + account_ids = (cached_items.map(&:from_account_id) + cached_items.map { |item| item.target_status&.account_id }.compact).uniq return if account_ids.empty? @@ -77,6 +77,7 @@ class Notification < ApplicationRecord cached_items.each do |item| item.from_account = accounts[item.from_account_id] + item.target_status.account = accounts[item.target_status.account_id] if item.target_status end end From 1167c6dbf8d5b8411d9924350b2b9735da6d9d26 Mon Sep 17 00:00:00 2001 From: Kazushige Tominaga Date: Fri, 9 Feb 2018 08:12:35 +0900 Subject: [PATCH 5/5] Perform request spec (#6446) * Added #link_header spec * Added #perform_request spec --- spec/services/fetch_atom_service_spec.rb | 39 ++++++++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/spec/services/fetch_atom_service_spec.rb b/spec/services/fetch_atom_service_spec.rb index fcba55e8d..d7f162b85 100644 --- a/spec/services/fetch_atom_service_spec.rb +++ b/spec/services/fetch_atom_service_spec.rb @@ -21,4 +21,43 @@ RSpec.describe FetchAtomService do it { expect(target.send(:link_header).links[0].href).to eq 'http://example.com/foo' } end end + + describe '#perform_request' do + let(:url) { 'http://example.com' } + context 'Check method result' do + before do + WebMock.stub_request(:get, url).to_return(status: 200, body: '', headers: {}) + @target = FetchAtomService.new + @target.instance_variable_set('@url', url) + end + + it 'HTTP::Response instance is returned and set to @response' do + expect(@target.send(:perform_request).status.to_s).to eq '200 OK' + expect(@target.instance_variable_get('@response')).to be_instance_of HTTP::Response + end + end + + context 'check passed parameters to Request' do + before do + @target = FetchAtomService.new + @target.instance_variable_set('@url', url) + @target.instance_variable_set('@unsupported_activity', unsupported_activity) + allow(Request).to receive(:new).with(:get, url) + expect(Request).to receive_message_chain(:new, :add_headers).with('Accept' => accept) + allow(Request).to receive_message_chain(:new, :add_headers, :perform).with(no_args) + end + + context '@unsupported_activity is true' do + let(:unsupported_activity) { true } + let(:accept) { 'text/html' } + it { @target.send(:perform_request) } + end + + context '@unsupported_activity is false' do + let(:unsupported_activity) { false } + let(:accept) { 'application/activity+json, application/ld+json, application/atom+xml, text/html' } + it { @target.send(:perform_request) } + end + end + end end