diff --git a/app/controllers/posts_controller.rb b/app/controllers/posts_controller.rb index b40eebc33..8417dc172 100644 --- a/app/controllers/posts_controller.rb +++ b/app/controllers/posts_controller.rb @@ -17,7 +17,7 @@ class PostsController < ApplicationController end def show - post = post_service.find(params[:id]) + post = post_service.find!(params[:id]) post_service.mark_user_notifications(post.id) respond_to do |format| format.html { @@ -32,7 +32,7 @@ class PostsController < ApplicationController def oembed post_id = OEmbedPresenter.id_from_url(params.delete(:url)) - post = post_service.find(post_id) + post = post_service.find!(post_id) oembed = params.slice(:format, :maxheight, :minheight) render json: OEmbedPresenter.new(post, oembed) rescue @@ -40,7 +40,7 @@ class PostsController < ApplicationController end def interactions - post = post_service.find(params[:id]) + post = post_service.find!(params[:id]) respond_with PostInteractionPresenter.new(post, current_user) end diff --git a/app/services/comment_service.rb b/app/services/comment_service.rb index 3f11208cc..4ec3b8835 100644 --- a/app/services/comment_service.rb +++ b/app/services/comment_service.rb @@ -4,7 +4,7 @@ class CommentService end def create(post_id, text) - post = post_service.find(post_id) + post = post_service.find!(post_id) user.comment!(post, text) end @@ -19,7 +19,7 @@ class CommentService end def find_for_post(post_id) - post_service.find(post_id).comments.for_a_stream + post_service.find!(post_id).comments.for_a_stream end private diff --git a/app/services/post_service.rb b/app/services/post_service.rb index 4c3d7a6b5..140c33a0d 100644 --- a/app/services/post_service.rb +++ b/app/services/post_service.rb @@ -3,11 +3,19 @@ class PostService @user = user end - def find(id_or_guid) + def find(id) if user - find_non_public_by_guid_or_id_with_user(id_or_guid) + user.find_visible_shareable_by_id(Post, id) else - find_public(id_or_guid) + Post.find_by_id_and_public(id, true) + end + end + + def find!(id_or_guid) + if user + find_non_public_by_guid_or_id_with_user!(id_or_guid) + else + find_public!(id_or_guid) end end @@ -18,7 +26,7 @@ class PostService end def destroy(post_id) - post = find(post_id) + post = find!(post_id) raise Diaspora::NotMine unless post.author == user.person user.retract(post) end @@ -27,14 +35,14 @@ class PostService attr_reader :user - def find_public(id_or_guid) + def find_public!(id_or_guid) Post.where(post_key(id_or_guid) => id_or_guid).first.tap do |post| raise ActiveRecord::RecordNotFound, "could not find a post with id #{id_or_guid}" unless post raise Diaspora::NonPublic unless post.public? end end - def find_non_public_by_guid_or_id_with_user(id_or_guid) + def find_non_public_by_guid_or_id_with_user!(id_or_guid) user.find_visible_shareable_by_id(Post, id_or_guid, key: post_key(id_or_guid)).tap do |post| raise ActiveRecord::RecordNotFound, "could not find a post with id #{id_or_guid} for user #{user.id}" unless post end diff --git a/spec/services/post_service_spec.rb b/spec/services/post_service_spec.rb index 1d78706b4..2f2d4764f 100644 --- a/spec/services/post_service_spec.rb +++ b/spec/services/post_service_spec.rb @@ -10,10 +10,6 @@ describe PostService do expect(PostService.new(alice).find(post.id)).to eq(post) end - it "works with guid" do - expect(PostService.new(alice).find(post.guid)).to eq(post) - end - it "returns the post, if the user can see the it" do expect(PostService.new(bob).find(post.id)).to eq(post) end @@ -22,16 +18,12 @@ describe PostService do expect(PostService.new(eve).find(public.id)).to eq(public) end - it "RecordNotFound if the post cannot be found" do - expect { - PostService.new(alice).find("unknown") - }.to raise_error ActiveRecord::RecordNotFound, "could not find a post with id unknown for user #{alice.id}" + it "does not return the post, if the post cannot be found" do + expect(PostService.new(alice).find("unknown")).to be_nil end - it "RecordNotFound if user cannot see the post" do - expect { - PostService.new(eve).find(post.id) - }.to raise_error ActiveRecord::RecordNotFound, "could not find a post with id #{post.id} for user #{eve.id}" + it "does not return the post, if user cannot see the post" do + expect(PostService.new(eve).find(post.id)).to be_nil end end @@ -40,19 +32,65 @@ describe PostService do expect(PostService.new.find(public.id)).to eq(public) end + it "does not return the post, if the post is private" do + expect(PostService.new.find(post.id)).to be_nil + end + + it "does not return the post, if the post cannot be found" do + expect(PostService.new.find("unknown")).to be_nil + end + end + end + + describe "#find!" do + context "with user" do + it "returns the post, if it is the users post" do + expect(PostService.new(alice).find!(post.id)).to eq(post) + end + it "works with guid" do - expect(PostService.new.find(public.guid)).to eq(public) + expect(PostService.new(alice).find!(post.guid)).to eq(post) + end + + it "returns the post, if the user can see the it" do + expect(PostService.new(bob).find!(post.id)).to eq(post) + end + + it "returns the post, if it is public" do + expect(PostService.new(eve).find!(public.id)).to eq(public) + end + + it "RecordNotFound if the post cannot be found" do + expect { + PostService.new(alice).find!("unknown") + }.to raise_error ActiveRecord::RecordNotFound, "could not find a post with id unknown for user #{alice.id}" + end + + it "RecordNotFound if user cannot see the post" do + expect { + PostService.new(eve).find!(post.id) + }.to raise_error ActiveRecord::RecordNotFound, "could not find a post with id #{post.id} for user #{eve.id}" + end + end + + context "without user" do + it "returns the post, if it is public" do + expect(PostService.new.find!(public.id)).to eq(public) + end + + it "works with guid" do + expect(PostService.new.find!(public.guid)).to eq(public) end it "NonPublic if the post is private" do expect { - PostService.new.find(post.id) + PostService.new.find!(post.id) }.to raise_error Diaspora::NonPublic end it "RecordNotFound if the post cannot be found" do expect { - PostService.new.find("unknown") + PostService.new.find!("unknown") }.to raise_error ActiveRecord::RecordNotFound, "could not find a post with id unknown" end end @@ -63,13 +101,13 @@ describe PostService do it "assumes ids less than 16 chars are ids and not guids" do post = Post.where(id: public.id) expect(Post).to receive(:where).with(hash_including(id: "123456789012345")).and_return(post).at_least(:once) - PostService.new(alice).find("123456789012345") + PostService.new(alice).find!("123456789012345") end it "assumes ids more than (or equal to) 16 chars are actually guids" do post = Post.where(guid: public.guid) expect(Post).to receive(:where).with(hash_including(guid: "1234567890123456")).and_return(post).at_least(:once) - PostService.new(alice).find("1234567890123456") + PostService.new(alice).find!("1234567890123456") end end end