Photos API Endpoint and unit tests complete

app/controllers/api/v1/photos_controller.rb

@@ -0,0 +1,56 @@
+# frozen_string_literal: true
+
+module Api
+  module V1
+    class PhotosController < Api::V1::BaseController
+      before_action except: %i[create destroy] do
+        require_access_token %w[read]
+      end
+
+      before_action only: %i[create destroy] do
+        require_access_token %w[read write]
+      end
+
+      rescue_from ActiveRecord::RecordNotFound do
+        render json: I18n.t("api.endpoint_errors.photos.not_found"), status: :not_found
+      end
+
+      def index
+        photos = current_user.photos
+        render json: photos.map {|p| PhotoPresenter.new(p).as_api_json(true) }
+      end
+
+      def show
+        photo = photo_service.visible_photo(params.require(:id))
+        raise ActiveRecord::RecordNotFound unless photo
+        render json: PhotoPresenter.new(photo).as_api_json(true)
+      end
+
+      def create
+        image = params.require(:image)
+        base_params = params.permit(:aspect_ids, :pending, :set_profile_photo)
+        photo = photo_service.create_from_params_and_file(base_params, image)
+        raise RuntimeError unless photo
+        render json: PhotoPresenter.new(photo).as_api_json(true)
+      rescue CarrierWave::IntegrityError, ActionController::ParameterMissing, RuntimeError
+        render json: I18n.t("api.endpoint_errors.photos.failed_create"), status: :unprocessable_entity
+      end
+
+      def destroy
+        photo = current_user.photos.where(guid: params[:id]).first
+        raise ActiveRecord::RecordNotFound unless photo
+        if current_user.retract(photo)
+          head :no_content
+        else
+          render json: I18n.t("api.endpoint_errors.photos.failed_delete"), status: :unprocessable_entity
+        end
+      end
+
+      private
+
+      def photo_service
+        @photo_service ||= PhotoService.new(current_user)
+      end
+    end
+  end
+end

app/controllers/api/v1/users_controller.rb

@@ -51,7 +51,7 @@ module Api
       def photos
         person = Person.find_by!(guid: params[:user_id])
         photos = Photo.visible(current_user, person, :all, Time.current)
-        render json: photos.map {|photo| PhotoPresenter.new(photo).as_api_json(false) }
+        render json: photos.map {|photo| PhotoPresenter.new(photo).as_api_json(true) }
       end
 
       def posts

app/controllers/photos_controller.rb

@@ -132,34 +132,11 @@ class PhotosController < ApplicationController
   end
 
   def legacy_create
-    photo_params = params.require(:photo).permit(:pending, :set_profile_photo, aspect_ids: [])
+    base_params = photo_params
-    if photo_params[:aspect_ids] == "all"
+    uploaded_file = file_handler(params)
-      photo_params[:aspect_ids] = current_user.aspects.map(&:id)
-    elsif photo_params[:aspect_ids].is_a?(Hash)
-      photo_params[:aspect_ids] = params[:photo][:aspect_ids].values
-    end
-
-    photo_params[:user_file] = file_handler(params)
-
-    @photo = current_user.build_post(:photo, photo_params)
-
-    if @photo.save
-
-      unless @photo.pending
-        unless @photo.public?
-          aspects = current_user.aspects_from_ids(photo_params[:aspect_ids])
-          current_user.add_to_streams(@photo, aspects)
-        end
-        current_user.dispatch_post(@photo, to: photo_params[:aspect_ids])
-      end
-
-      if photo_params[:set_profile_photo]
-        profile_params = {image_url:        @photo.url(:thumb_large),
-                          image_url_medium: @photo.url(:thumb_medium),
-                          image_url_small:  @photo.url(:thumb_small)}
-        current_user.update_profile(profile_params)
-      end
 
+    @photo = photo_service.create_from_params_and_file(base_params, uploaded_file)
+    if @photo
       respond_to do |format|
         format.json { render(layout: false, json: {"success" => true, "data" => @photo}.to_json) }
         format.html { render(layout: false, json: {"success" => true, "data" => @photo}.to_json) }
@@ -185,4 +162,8 @@ class PhotosController < ApplicationController
       format.html { render(layout: false, json: {"success" => false, "error" => message}.to_json) }
     end
   end
+
+  def photo_service
+    @photo_service ||= PhotoService.new(current_user, false)
+  end
 end

app/presenters/photo_presenter.rb

@@ -17,8 +17,9 @@ class PhotoPresenter < BasePresenter
     }
   end
 
-  def as_api_json(no_guid=true)
+  def as_api_json(full=false)
-    based_data = {
+    api_json = {
+      author:     PersonPresenter.new(author).as_api_json,
       dimensions: {
         height: height,
         width:  width
@@ -29,7 +30,9 @@ class PhotoPresenter < BasePresenter
         large:  url(:scaled_full)
       }
     }
-    return based_data if no_guid
+
-    based_data.merge(guid: guid)
+    api_json[:guid] = guid if full
+    api_json[:post] = status_message_guid if full && status_message_guid
+    api_json
   end
 end

app/services/photo_service.rb

@@ -0,0 +1,59 @@
+# frozen_string_literal: true
+
+class PhotoService
+  def initialize(user=nil, deny_raw_files=true)
+    @user = user
+    @deny_raw_files = deny_raw_files
+  end
+
+  def visible_photo(photo_guid)
+    Photo.owned_or_visible_by_user(@user).where(guid: photo_guid).first
+  end
+
+  def create_from_params_and_file(base_params, uploaded_file)
+    photo_params = build_params(base_params)
+
+    raise RuntimeError if @deny_raw_files && !confirm_uploaded_file_settings(uploaded_file)
+    photo_params[:user_file] = uploaded_file
+
+    @photo = @user.build_post(:photo, photo_params)
+    raise RuntimeError unless @photo.save
+    unless @photo.pending
+      unless @photo.public?
+        aspects = @user.aspects_from_ids(photo_params[:aspect_ids])
+        @user.add_to_streams(@photo, aspects)
+      end
+      @user.dispatch_post(@photo, to: photo_params[:aspect_ids])
+    end
+
+    if photo_params[:set_profile_photo]
+      profile_params = {image_url:        @photo.url(:thumb_large),
+                        image_url_medium: @photo.url(:thumb_medium),
+                        image_url_small:  @photo.url(:thumb_small)}
+      @user.update_profile(profile_params)
+    end
+
+    @photo
+  end
+
+  private
+
+  def build_params(base_params)
+    photo_params = base_params.permit(:pending, :set_profile_photo, aspect_ids: [])
+    if base_params.permit(:aspect_ids)[:aspect_ids] == "all"
+      photo_params[:aspect_ids] = @user.aspects.map(&:id)
+    elsif photo_params[:aspect_ids].is_a?(Hash)
+      photo_params[:aspect_ids] = params[:photo][:aspect_ids].values
+    end
+    photo_params
+  end
+
+  def confirm_uploaded_file_settings(uploaded_file)
+    unless uploaded_file.is_a?(ActionDispatch::Http::UploadedFile) || uploaded_file.is_a?(Rack::Test::UploadedFile)
+      return false
+    end
+    return false if uploaded_file.original_filename.empty?
+    return false if uploaded_file.content_type.empty?
+    true
+  end
+end

config/locales/diaspora/en.yml

@@ -977,6 +977,10 @@ en:
       notifications:
         not_found: "Notification with provided guid could not be found"
         cant_process: "Couldn't process the notifications request"
+      photos:
+        not_found: "Photo with provided guid could not be found"
+        failed_create: "Failed to create the photo"
+        failed_delete: "Not allowed to delete the photo"
       posts:
         post_not_found: "Post with provided guid could not be found"
         failed_create: "Failed to create the post"

config/routes.rb

@@ -225,6 +225,7 @@ Rails.application.routes.draw do
     resources :aspects, only: %i[show index create destroy update] do
       resources :contacts, only: %i[index create destroy]
     end
+    resources :photos, only: %i[show index create destroy]
     resources :posts, only: %i[show create destroy] do
       resources :comments, only: %i[create index destroy] do
         post "report" => "comments#report"

spec/integration/api/photos_controller_spec.rb

@@ -0,0 +1,293 @@
+# frozen_sTring_literal: true
+
+require "spec_helper"
+
+describe Api::V1::PhotosController do
+  let(:auth) { FactoryGirl.create(:auth_with_read_and_write) }
+  let(:auth_read_only) { FactoryGirl.create(:auth_with_read) }
+  let!(:access_token) { auth.create_access_token.to_s }
+  let!(:access_token_read_only) { auth_read_only.create_access_token.to_s }
+
+  before do
+    alice_private_spec = alice.aspects.create(name: "private aspect")
+    alice.share_with(eve.person, alice_private_spec)
+    @private_photo1 = alice.post(:photo, pending: false, user_file: File.open(photo_fixture_name),
+                                to: alice_private_spec.id)
+    @alice_public_photo = alice.post(:photo, pending: false, user_file: File.open(photo_fixture_name), public: true)
+    @user_photo1 = auth.user.post(:photo, pending: true, user_file: File.open(photo_fixture_name), to: "all")
+    @user_photo2 = auth.user.post(:photo, pending: true, user_file: File.open(photo_fixture_name), to: "all")
+    message_data = {status_message: {text: "Post with photos"}, public: true, photos: [@user_photo2.id.to_s]}
+    @status_message = StatusMessageCreationService.new(auth.user).create(message_data)
+    @user_photo2.reload
+  end
+
+  describe "#show" do
+    context "succeeds" do
+      it "with correct GUID user's photo and access token" do
+        get(
+          api_v1_photo_path(@user_photo1.guid),
+          params: {access_token: access_token}
+        )
+        expect(response.status).to eq(200)
+        photo = JSON.parse(response.body)
+        expect(photo.has_key?("post")).to be_falsey
+        confirm_photo_format(photo, @user_photo1, auth.user)
+      end
+
+      it "with correct GUID user's photo used in post and access token" do
+        get(
+          api_v1_photo_path(@user_photo2.guid),
+          params: {access_token: access_token}
+        )
+        expect(response.status).to eq(200)
+        photo = JSON.parse(response.body)
+        expect(photo.has_key?("post")).to be_truthy
+        confirm_photo_format(photo, @user_photo2, auth.user)
+      end
+
+      it "with correct GUID of other user's public photo and access token" do
+        get(
+          api_v1_photo_path(@alice_public_photo.guid),
+          params: {access_token: access_token}
+        )
+        expect(response.status).to eq(200)
+        photo = JSON.parse(response.body)
+        confirm_photo_format(photo, @alice_public_photo, alice)
+      end
+    end
+
+    context "fails" do
+      it "with other user's private photo" do
+        get(
+          api_v1_photo_path(@private_photo1.guid),
+          params: {access_token: access_token}
+        )
+        expect(response.status).to eq(404)
+        expect(response.body).to eq(I18n.t("api.endpoint_errors.photos.not_found"))
+      end
+
+      it "with invalid GUID" do
+        get(
+          api_v1_photo_path("999_999_999"),
+          params: {access_token: access_token}
+        )
+        expect(response.status).to eq(404)
+        expect(response.body).to eq(I18n.t("api.endpoint_errors.photos.not_found"))
+      end
+
+      it "with invalid access token" do
+        delete(
+          api_v1_photo_path(@user_photo1.guid),
+          params: {access_token: "999_999_999"}
+        )
+        expect(response.status).to eq(401)
+      end
+    end
+  end
+
+  describe "#index" do
+    context "succeeds" do
+      it "with correct access token" do
+        get(
+          api_v1_photos_path,
+          params: {access_token: access_token}
+        )
+        expect(response.status).to eq(200)
+        photos = JSON.parse(response.body)
+        expect(photos.length).to eq(2)
+      end
+    end
+
+    context "fails" do
+      it "with invalid access token" do
+        delete(
+          api_v1_photos_path,
+          params: {access_token: "999_999_999"}
+        )
+        expect(response.status).to eq(401)
+      end
+    end
+  end
+
+  describe "#create" do
+    before do
+      @encoded_photo = Rack::Test::UploadedFile.new(
+        Rails.root.join("spec", "fixtures", "button.png").to_s,
+        "image/png"
+      )
+    end
+
+    context "succeeds" do
+      it "with valid encoded file no arguments" do
+        post(
+          api_v1_photos_path,
+          params: {image: @encoded_photo, access_token: access_token}
+        )
+        expect(response.status).to eq(200)
+        photo = JSON.parse(response.body)
+        ref_photo = auth.user.photos.reload.find_by(guid: photo["guid"])
+        expect(ref_photo.pending).to be_falsey
+        confirm_photo_format(photo, ref_photo, auth.user)
+      end
+
+      it "with valid encoded file set as pending" do
+        post(
+          api_v1_photos_path,
+          params: {image: @encoded_photo, pending: false, access_token: access_token}
+        )
+        expect(response.status).to eq(200)
+        photo = JSON.parse(response.body)
+        expect(photo.has_key?("post")).to be_falsey
+        ref_photo = auth.user.photos.reload.find_by(guid: photo["guid"])
+        expect(ref_photo.pending).to be_falsey
+        confirm_photo_format(photo, ref_photo, auth.user)
+
+        post(
+          api_v1_photos_path,
+          params: {image: @encoded_photo, pending: true, access_token: access_token}
+        )
+        expect(response.status).to eq(200)
+        photo = JSON.parse(response.body)
+        ref_photo = auth.user.photos.reload.find_by(guid: photo["guid"])
+        expect(ref_photo.pending).to be_truthy
+      end
+
+      it "with valid encoded file as profile photo" do
+        post(
+          api_v1_photos_path,
+          params: {image: @encoded_photo, set_profile_photo: true, access_token: access_token}
+        )
+        expect(response.status).to eq(200)
+        photo = JSON.parse(response.body)
+        expect(auth.user.reload.person.profile.image_url_small).to eq(photo["sizes"]["small"])
+      end
+    end
+
+    context "fails" do
+      it "with no image" do
+        post(
+          api_v1_photos_path,
+          params: {access_token: access_token}
+        )
+        expect(response.status).to eq(422)
+        expect(response.body).to eq(I18n.t("api.endpoint_errors.photos.failed_create"))
+      end
+
+      it "with non-image file" do
+        text_file = Rack::Test::UploadedFile.new(
+          Rails.root.join("README.md").to_s,
+          "text/plain"
+        )
+        post(
+          api_v1_photos_path,
+          params: {image: text_file, access_token: access_token}
+        )
+        expect(response.status).to eq(422)
+        expect(response.body).to eq(I18n.t("api.endpoint_errors.photos.failed_create"))
+      end
+
+      it "with impromperly identified file" do
+        text_file = Rack::Test::UploadedFile.new(
+          Rails.root.join("README.md").to_s,
+          "image/png"
+        )
+        post(
+          api_v1_photos_path,
+          params: {image: text_file, access_token: access_token}
+        )
+        expect(response.status).to eq(422)
+        expect(response.body).to eq(I18n.t("api.endpoint_errors.photos.failed_create"))
+      end
+
+      it "with invalid access token" do
+        post(
+          api_v1_photos_path,
+          params: {image: @encoded_photo, access_token: "999_999_999"}
+        )
+        expect(response.status).to eq(401)
+      end
+
+      it "with read only access token" do
+        post(
+          api_v1_photos_path,
+          params: {image: @encoded_photo, access_token: access_token_read_only}
+        )
+        expect(response.status).to eq(403)
+      end
+    end
+  end
+
+  describe "#destroy" do
+    context "succeeds" do
+      it "with correct GUID and access token" do
+        expect(auth.user.photos.find_by(id: @user_photo1.id)).to eq(@user_photo1)
+        delete(
+          api_v1_photo_path(@user_photo1.guid),
+          params: {access_token: access_token}
+        )
+        expect(response.status).to eq(204)
+        expect(auth.user.photos.find_by(id: @user_photo1.id)).to be_nil
+      end
+    end
+
+    context "fails" do
+      it "with other user's photo GUID and access token" do
+        delete(
+          api_v1_photo_path(@alice_public_photo.guid),
+          params: {access_token: access_token}
+        )
+        expect(response.status).to eq(404)
+        expect(response.body).to eq(I18n.t("api.endpoint_errors.photos.not_found"))
+      end
+
+      it "with other invalid GUID" do
+        delete(
+          api_v1_photo_path("999_999_999"),
+          params: {access_token: access_token}
+        )
+        expect(response.status).to eq(404)
+        expect(response.body).to eq(I18n.t("api.endpoint_errors.photos.not_found"))
+      end
+
+      it "with invalid access token" do
+        delete(
+          api_v1_photo_path(@user_photo1.guid),
+          params: {access_token: "999_999_999"}
+        )
+        expect(response.status).to eq(401)
+      end
+
+      it "with read only access token" do
+        delete(
+          api_v1_photo_path(@user_photo1.guid),
+          params: {access_token: access_token_read_only}
+        )
+        expect(response.status).to eq(403)
+      end
+    end
+  end
+
+  # rubocop:disable Metrics/AbcSize
+  def confirm_photo_format(photo, ref_photo, ref_user)
+    expect(photo["guid"]).to eq(ref_photo.guid)
+    if ref_photo.status_message_guid
+      expect(photo["post"]).to eq(ref_photo.status_message_guid)
+    else
+      expect(photo.has_key?("post")).to be_falsey
+    end
+    expect(photo["dimensions"].has_key?("height")).to be_truthy
+    expect(photo["dimensions"].has_key?("width")).to be_truthy
+    expect(photo["sizes"]["small"]).to be_truthy
+    expect(photo["sizes"]["medium"]).to be_truthy
+    expect(photo["sizes"]["large"]).to be_truthy
+    confirm_person_format(photo["author"], ref_user)
+  end
+
+  def confirm_person_format(post_person, user)
+    expect(post_person["guid"]).to eq(user.guid)
+    expect(post_person["diaspora_id"]).to eq(user.diaspora_handle)
+    expect(post_person["name"]).to eq(user.name)
+    expect(post_person["avatar"]).to eq(user.profile.image_url)
+  end
+  # rubocop:enable Metrics/AbcSize
+end

spec/presenters/photo_presenter_spec.rb

@@ -0,0 +1,47 @@
+# frozen_string_literal: true
+
+describe PhotoPresenter do
+  before do
+    @photo = bob.post(:photo, pending: true, user_file: File.open(photo_fixture_name), to: "all")
+  end
+
+  it "presents limited API JSON" do
+    photo_json = PhotoPresenter.new(@photo).as_api_json(false)
+    expect(photo_json.has_key?(:guid)).to be_falsey
+  end
+
+  it "presents full API JSON" do
+    photo_json = PhotoPresenter.new(@photo).as_api_json(true)
+    expect(photo_json[:guid]).to eq(@photo.guid)
+    confirm_photo_format(photo_json, @photo, bob)
+  end
+
+  it "defaults to limited API JSON" do
+    photo_json_limited = PhotoPresenter.new(@photo).as_api_json(false)
+    photo_json_default = PhotoPresenter.new(@photo).as_api_json
+    expect(photo_json_limited).to eq(photo_json_default)
+  end
+
+  # rubocop:disable Metrics/AbcSize
+  def confirm_photo_format(photo, ref_photo, ref_user)
+    if ref_photo.status_message_guid
+      expect(photo[:post]).to eq(ref_photo.status_message_guid)
+    else
+      expect(photo.has_key?(:post)).to be_falsey
+    end
+    expect(photo[:dimensions].has_key?(:height)).to be_truthy
+    expect(photo[:dimensions].has_key?(:width)).to be_truthy
+    expect(photo[:sizes][:small]).to be_truthy
+    expect(photo[:sizes][:medium]).to be_truthy
+    expect(photo[:sizes][:large]).to be_truthy
+    confirm_person_format(photo[:author], ref_user)
+  end
+
+  def confirm_person_format(post_person, user)
+    expect(post_person[:guid]).to eq(user.guid)
+    expect(post_person[:diaspora_id]).to eq(user.diaspora_handle)
+    expect(post_person[:name]).to eq(user.name)
+    expect(post_person[:avatar]).to eq(user.profile.image_url)
+  end
+  # rubocop:enable Metrics/AbcSize
+end

spec/services/photo_service_spec.rb

@@ -0,0 +1,120 @@
+# frozen_string_literal: true
+
+describe PhotoService do
+  before do
+    alice_eve_spec = alice.aspects.create(name: "eve aspect")
+    alice.share_with(eve.person, alice_eve_spec)
+    alice_bob_spec = alice.aspects.create(name: "bob aspect")
+    alice.share_with(bob.person, alice_bob_spec)
+    @alice_eve_photo = alice.post(:photo, pending: false, user_file: File.open(photo_fixture_name),
+                                 to: alice_eve_spec.id)
+    @alice_bob_photo = alice.post(:photo, pending: false, user_file: File.open(photo_fixture_name),
+                                 to: alice_bob_spec.id)
+    @alice_public_photo = alice.post(:photo, pending: false, user_file: File.open(photo_fixture_name), to: "all")
+    @bob_photo1 = bob.post(:photo, pending: true, user_file: File.open(photo_fixture_name), to: "all")
+  end
+
+  describe "visible_photo" do
+    it "returns a user's photo" do
+      photo = photo_service.visible_photo(@bob_photo1.guid)
+      expect(photo.guid).to eq(@bob_photo1.guid)
+    end
+
+    it "returns another user's public photo" do
+      photo = photo_service.visible_photo(@alice_public_photo.guid)
+      expect(photo.guid).to eq(@alice_public_photo.guid)
+    end
+
+    it "returns another user's shared photo" do
+      photo = photo_service.visible_photo(@alice_bob_photo.guid)
+      expect(photo.guid).to eq(@alice_bob_photo.guid)
+    end
+
+    it "returns nil for other user's private photo" do
+      photo = photo_service.visible_photo(@alice_eve_photo.guid)
+      expect(photo).to be_nil
+    end
+  end
+
+  describe "create" do
+    before do
+      @image_file = Rack::Test::UploadedFile.new(
+        Rails.root.join("spec", "fixtures", "button.png").to_s,
+        "image/png"
+      )
+    end
+
+    context "succeeds" do
+      it "accepts a photo from a regular form uploaded file no parameters" do
+        params = ActionController::Parameters.new
+        photo = photo_service.create_from_params_and_file(params, @image_file)
+        expect(photo).not_to be_nil
+        expect(photo.pending?).to be_falsey
+        expect(photo.public?).to be_falsey
+      end
+
+      it "honors pending" do
+        params = ActionController::Parameters.new(pending: true)
+        photo = photo_service.create_from_params_and_file(params, @image_file)
+        expect(photo).not_to be_nil
+        expect(photo.pending?).to be_truthy
+        expect(photo.public?).to be_falsey
+      end
+
+      it "accepts sets a user profile when requested" do
+        original_profile_pic = bob.person.profile.image_url
+        params = ActionController::Parameters.new(set_profile_photo: true)
+        photo = photo_service.create_from_params_and_file(params, @image_file)
+        expect(photo).not_to be_nil
+        expect(bob.reload.person.profile.image_url).not_to eq(original_profile_pic)
+      end
+
+      it "has correct aspects settings for limited shared" do
+        params = ActionController::Parameters.new(pending: false, aspect_ids: [bob.aspects.first.id])
+        photo = photo_service.create_from_params_and_file(params, @image_file)
+        expect(photo).not_to be_nil
+        expect(photo.pending?).to be_falsey
+        expect(photo.public?).to be_falsey
+      end
+
+      it "allow raw file if explicitly allowing" do
+        params = ActionController::Parameters.new
+        photo = PhotoService.new(bob, false).create_from_params_and_file(params, uploaded_photo)
+        expect(photo).not_to be_nil
+      end
+    end
+    context "fails" do
+      before do
+        @params = ActionController::Parameters.new
+      end
+
+      it "fails if given a raw file" do
+        expect {
+          photo_service.create_from_params_and_file(@params, uploaded_photo)
+        }.to raise_error RuntimeError
+      end
+
+      it "file type isn't an image" do
+        text_file = Rack::Test::UploadedFile.new(
+          Rails.root.join("README.md").to_s,
+          "text/plain"
+        )
+        expect {
+          photo_service.create_from_params_and_file(@params, text_file)
+        }.to raise_error CarrierWave::IntegrityError
+
+        text_file = Rack::Test::UploadedFile.new(
+          Rails.root.join("README.md").to_s,
+          "image/png"
+        )
+        expect {
+          photo_service.create_from_params_and_file(@params, text_file)
+        }.to raise_error CarrierWave::IntegrityError
+      end
+    end
+  end
+
+  def photo_service(user=bob)
+    PhotoService.new(user, true)
+  end
+end