diff --git a/app/controllers/messages_controller.rb b/app/controllers/messages_controller.rb index c5387799b..a217aab87 100644 --- a/app/controllers/messages_controller.rb +++ b/app/controllers/messages_controller.rb @@ -18,14 +18,6 @@ class MessagesController < ApplicationController logger.info "event=create type=message user=#{current_user.diaspora_handle} status=success " \ "message=#{message.id} chars=#{params[:message][:text].length}" Diaspora::Federation::Dispatcher.defer_dispatch(current_user, message) - - # TODO: can be removed when messages are not relayed anymore - conversation_owner = conversation.author.owner - if conversation_owner && conversation_owner != current_user - remote_subs = conversation.participants.remote.ids - opts = {subscriber_ids: remote_subs} - Diaspora::Federation::Dispatcher.defer_dispatch(conversation_owner, message, opts) unless remote_subs.empty? - end else flash[:error] = I18n.t('conversations.new_conversation.fail') end diff --git a/app/models/message.rb b/app/models/message.rb index 82c551262..2a56ea47a 100644 --- a/app/models/message.rb +++ b/app/models/message.rb @@ -7,9 +7,6 @@ class Message < ActiveRecord::Base delegate :name, to: :author, prefix: true - # TODO: can be removed when messages are not relayed anymore - alias_attribute :parent, :conversation - validates :conversation, presence: true validates :text, presence: true validate :participant_of_parent_conversation @@ -31,11 +28,7 @@ class Message < ActiveRecord::Base # @return [Array] def subscribers - if author.local? - conversation.participants - else # for relaying, TODO: can be removed when messages are not relayed anymore - conversation.participants.remote - end + conversation.participants end private diff --git a/db/migrate/20170430022507_remove_message_signature.rb b/db/migrate/20170430022507_remove_message_signature.rb new file mode 100644 index 000000000..f2519dba5 --- /dev/null +++ b/db/migrate/20170430022507_remove_message_signature.rb @@ -0,0 +1,5 @@ +class RemoveMessageSignature < ActiveRecord::Migration + def change + remove_column :messages, :author_signature, :text + end +end diff --git a/db/schema.rb b/db/schema.rb index 7e0e55ec0..48db06ecd 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -11,7 +11,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 20161107100840) do +ActiveRecord::Schema.define(version: 20170430022507) do create_table "account_deletions", force: :cascade do |t| t.string "diaspora_handle", limit: 255 @@ -214,13 +214,12 @@ ActiveRecord::Schema.define(version: 20161107100840) do add_index "mentions", ["person_id"], name: "index_mentions_on_person_id", using: :btree create_table "messages", force: :cascade do |t| - t.integer "conversation_id", limit: 4, null: false - t.integer "author_id", limit: 4, null: false - t.string "guid", limit: 255, null: false - t.text "text", limit: 65535, null: false - t.datetime "created_at", null: false - t.datetime "updated_at", null: false - t.text "author_signature", limit: 65535 + t.integer "conversation_id", limit: 4, null: false + t.integer "author_id", limit: 4, null: false + t.string "guid", limit: 255, null: false + t.text "text", limit: 65535, null: false + t.datetime "created_at", null: false + t.datetime "updated_at", null: false end add_index "messages", ["author_id"], name: "index_messages_on_author_id", using: :btree diff --git a/lib/diaspora/federation/entities.rb b/lib/diaspora/federation/entities.rb index f806ec158..da8e0bb33 100644 --- a/lib/diaspora/federation/entities.rb +++ b/lib/diaspora/federation/entities.rb @@ -98,10 +98,7 @@ module Diaspora guid: message.guid, text: message.text, created_at: message.created_at, - parent_guid: message.conversation.guid, - conversation_guid: message.conversation.guid, - author_signature: message.author_signature, - parent: related_entity(message.conversation) + conversation_guid: message.conversation.guid ) end diff --git a/lib/diaspora/federation/receive.rb b/lib/diaspora/federation/receive.rb index 697ad4536..18c642fd7 100644 --- a/lib/diaspora/federation/receive.rb +++ b/lib/diaspora/federation/receive.rb @@ -59,7 +59,9 @@ module Diaspora end def self.message(entity) - save_message(entity).tap {|message| relay_relayable(message) if message } + ignore_existing_guid(Message, entity.guid, author_of(entity)) do + build_message(entity).tap(&:save!) + end end def self.participation(entity) @@ -215,15 +217,6 @@ module Diaspora end end - private_class_method def self.save_message(entity) - ignore_existing_guid(Message, entity.guid, author_of(entity)) do - build_message(entity).tap do |message| - message.author_signature = entity.author_signature if message.conversation.author.local? - message.save! - end - end - end - private_class_method def self.save_photo(entity) Photo.create!( author: author_of(entity), diff --git a/spec/controllers/messages_controller_spec.rb b/spec/controllers/messages_controller_spec.rb index 7e753c4f0..6e7bea228 100644 --- a/spec/controllers/messages_controller_spec.rb +++ b/spec/controllers/messages_controller_spec.rb @@ -102,19 +102,6 @@ describe MessagesController, :type => :controller do expect(Diaspora::Federation::Dispatcher).to receive(:defer_dispatch).with(alice, instance_of(Message)) post :create, @message_params end - - it "dispatches the message twice if the conversation author is local and it has remote users" do - @conversation_params[:participant_ids] = [bob.person.id, alice.person.id, remote_raphael.id] - conversation = Conversation.create!(@conversation_params) - @message_params[:conversation_id] = conversation.id - - expect(Diaspora::Federation::Dispatcher).to receive(:defer_dispatch).with(alice, instance_of(Message)) - expect(Diaspora::Federation::Dispatcher).to receive(:defer_dispatch).with( - bob, instance_of(Message), subscriber_ids: [remote_raphael.id] - ) - - post :create, @message_params - end end context 'on a post from a stranger' do diff --git a/spec/integration/federation/federation_helper.rb b/spec/integration/federation/federation_helper.rb index 56e8a86f9..f16cd604c 100644 --- a/spec/integration/federation/federation_helper.rb +++ b/spec/integration/federation/federation_helper.rb @@ -35,14 +35,12 @@ def create_relayable_entity(entity_name, parent, diaspora_id) :fetch_private_key, alice.diaspora_handle ).at_least(1).times.and_return(nil) if parent == local_parent - parent_guid = parent.guid Fabricate( entity_name, - conversation_guid: parent_guid, - parent_guid: parent_guid, - author: diaspora_id, - poll_answer_guid: parent.respond_to?(:poll_answers) ? parent.poll_answers.first.guid : nil, - parent: Diaspora::Federation::Entities.related_entity(parent) + parent_guid: parent.guid, + author: diaspora_id, + poll_answer_guid: parent.respond_to?(:poll_answers) ? parent.poll_answers.first.guid : nil, + parent: Diaspora::Federation::Entities.related_entity(parent) ) end diff --git a/spec/integration/federation/receive_federation_messages_spec.rb b/spec/integration/federation/receive_federation_messages_spec.rb index d16676468..4a017cedf 100644 --- a/spec/integration/federation/receive_federation_messages_spec.rb +++ b/spec/integration/federation/receive_federation_messages_spec.rb @@ -126,24 +126,59 @@ describe "Receive federation messages feature" do end context "with message" do - let(:local_parent) { - FactoryGirl.build(:conversation, author: alice.person).tap do |target| - target.participants << remote_user_on_pod_b.person - target.participants << remote_user_on_pod_c.person - target.save - end - } - let(:remote_parent) { - FactoryGirl.build(:conversation, author: remote_user_on_pod_b.person).tap do |target| - target.participants << alice.person - target.participants << remote_user_on_pod_c.person - target.save - end - } - let(:entity_name) { :message_entity } - let(:klass) { Message } + context "local" do + let(:parent) { + FactoryGirl.build(:conversation, author: alice.person).tap do |target| + target.participants << remote_user_on_pod_b.person + target.participants << remote_user_on_pod_c.person + target.save + end + } + let(:message) { + Fabricate( + :message_entity, + conversation_guid: parent.guid, + author: sender_id, + parent: Diaspora::Federation::Entities.related_entity(parent) + ) + } - it_behaves_like "it deals correctly with a relayable" + it "receives the message correctly" do + expect(Workers::ReceiveLocal).to receive(:perform_async) + post_message(generate_payload(message, sender, recipient), recipient) + + received_message = Message.find_by(guid: message.guid) + expect(received_message).not_to be_nil + expect(received_message.author.diaspora_handle).to eq(sender_id) + end + end + + context "remote" do + let(:parent) { + FactoryGirl.build(:conversation, author: remote_user_on_pod_b.person).tap do |target| + target.participants << alice.person + target.participants << remote_user_on_pod_c.person + target.save + end + } + let(:message) { + Fabricate( + :message_entity, + conversation_guid: parent.guid, + author: remote_user_on_pod_c.diaspora_handle, + parent: Diaspora::Federation::Entities.related_entity(parent) + ) + } + + it "receives the message correctly" do + expect(Workers::ReceiveLocal).to receive(:perform_async) + post_message(generate_payload(message, remote_user_on_pod_c, recipient), recipient) + + received_message = Message.find_by(guid: message.guid) + expect(received_message).not_to be_nil + expect(received_message.author.diaspora_handle).to eq(remote_user_on_pod_c.diaspora_handle) + end + end end end end diff --git a/spec/lib/diaspora/federation/entities_spec.rb b/spec/lib/diaspora/federation/entities_spec.rb index bce5d7f21..428990bcd 100644 --- a/spec/lib/diaspora/federation/entities_spec.rb +++ b/spec/lib/diaspora/federation/entities_spec.rb @@ -104,7 +104,7 @@ describe Diaspora::Federation::Entities do end it "builds a message" do - diaspora_entity = FactoryGirl.create(:message, author_signature: "abc") + diaspora_entity = FactoryGirl.create(:message) federation_entity = described_class.build(diaspora_entity) expect(federation_entity).to be_instance_of(DiasporaFederation::Entities::Message) @@ -113,7 +113,6 @@ describe Diaspora::Federation::Entities do expect(federation_entity.conversation_guid).to eq(diaspora_entity.conversation.guid) expect(federation_entity.text).to eq(diaspora_entity.text) expect(federation_entity.created_at).to eq(diaspora_entity.created_at) - expect(federation_entity.author_signature).to eq(diaspora_entity.author_signature) end it "builds a participation" do diff --git a/spec/lib/diaspora/federation/receive_spec.rb b/spec/lib/diaspora/federation/receive_spec.rb index 680a34190..f871dad6c 100644 --- a/spec/lib/diaspora/federation/receive_spec.rb +++ b/spec/lib/diaspora/federation/receive_spec.rb @@ -271,7 +271,6 @@ describe Diaspora::Federation::Receive do let(:entity) { message_entity } it_behaves_like "it ignores existing object received twice", Message - it_behaves_like "it relays relayables", Message end describe ".participation" do diff --git a/spec/models/message_spec.rb b/spec/models/message_spec.rb index 051b384ce..e2588a502 100644 --- a/spec/models/message_spec.rb +++ b/spec/models/message_spec.rb @@ -39,11 +39,6 @@ describe Message, type: :model do message = Message.create(author: local_luke.person, text: "yo", conversation: remote_conv) expect(message.subscribers).to match_array([local_luke.person, local_leia.person, remote_raphael]) end - - it "returns only remote participants, if the conversation is local, but the author is remote" do - message = Message.create(author: remote_raphael, text: "yo", conversation: local_conv) - expect(message.subscribers).to match_array([remote_raphael]) - end end describe "#increase_unread" do