From 02cf6a9eb242a887366ce649f447f354916321ce Mon Sep 17 00:00:00 2001 From: Hank Grabowski Date: Wed, 28 Nov 2018 09:08:44 -0500 Subject: [PATCH] Photos API Endpoint and unit tests complete --- app/controllers/api/v1/photos_controller.rb | 56 ++++ app/controllers/api/v1/users_controller.rb | 2 +- app/controllers/photos_controller.rb | 35 +-- app/presenters/photo_presenter.rb | 11 +- app/services/photo_service.rb | 59 ++++ config/locales/diaspora/en.yml | 4 + config/routes.rb | 1 + .../integration/api/photos_controller_spec.rb | 293 ++++++++++++++++++ spec/presenters/photo_presenter_spec.rb | 47 +++ spec/services/photo_service_spec.rb | 120 +++++++ 10 files changed, 596 insertions(+), 32 deletions(-) create mode 100644 app/controllers/api/v1/photos_controller.rb create mode 100644 app/services/photo_service.rb create mode 100644 spec/integration/api/photos_controller_spec.rb create mode 100644 spec/presenters/photo_presenter_spec.rb create mode 100644 spec/services/photo_service_spec.rb diff --git a/app/controllers/api/v1/photos_controller.rb b/app/controllers/api/v1/photos_controller.rb new file mode 100644 index 000000000..a9a13e633 --- /dev/null +++ b/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 diff --git a/app/controllers/api/v1/users_controller.rb b/app/controllers/api/v1/users_controller.rb index be05d7110..63b09a14f 100644 --- a/app/controllers/api/v1/users_controller.rb +++ b/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 diff --git a/app/controllers/photos_controller.rb b/app/controllers/photos_controller.rb index 0b01d6e49..c9447b495 100644 --- a/app/controllers/photos_controller.rb +++ b/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: []) - if photo_params[:aspect_ids] == "all" - 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 + base_params = photo_params + uploaded_file = file_handler(params) + @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 diff --git a/app/presenters/photo_presenter.rb b/app/presenters/photo_presenter.rb index 91159d1dc..728d5a90e 100644 --- a/app/presenters/photo_presenter.rb +++ b/app/presenters/photo_presenter.rb @@ -17,8 +17,9 @@ class PhotoPresenter < BasePresenter } end - def as_api_json(no_guid=true) - based_data = { + def as_api_json(full=false) + 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 diff --git a/app/services/photo_service.rb b/app/services/photo_service.rb new file mode 100644 index 000000000..3fa51a143 --- /dev/null +++ b/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 diff --git a/config/locales/diaspora/en.yml b/config/locales/diaspora/en.yml index 63236af61..26b0a3fd3 100644 --- a/config/locales/diaspora/en.yml +++ b/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" diff --git a/config/routes.rb b/config/routes.rb index 2f19647d7..15ddfc366 100644 --- a/config/routes.rb +++ b/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" diff --git a/spec/integration/api/photos_controller_spec.rb b/spec/integration/api/photos_controller_spec.rb new file mode 100644 index 000000000..7cd9eb2e0 --- /dev/null +++ b/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 diff --git a/spec/presenters/photo_presenter_spec.rb b/spec/presenters/photo_presenter_spec.rb new file mode 100644 index 000000000..bc7ba1fcf --- /dev/null +++ b/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 diff --git a/spec/services/photo_service_spec.rb b/spec/services/photo_service_spec.rb new file mode 100644 index 000000000..41a7187b1 --- /dev/null +++ b/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