PostService: create find!

* find returns nil if nothing found
* find! raises errors if not found or not visible

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
 

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

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

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
+      it "does not return the post, if the post cannot be found" do
-        expect {
+        expect(PostService.new(alice).find("unknown")).to be_nil
-          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
+      it "does not return the post, if user cannot see the post" do
-        expect {
+        expect(PostService.new(eve).find(post.id)).to be_nil
-          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
 
@@ -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