diff --git a/app/controllers/api/v1/assets_controller.rb b/app/controllers/api/v1/assets_controller.rb index 37aabe49d..358e49708 100644 --- a/app/controllers/api/v1/assets_controller.rb +++ b/app/controllers/api/v1/assets_controller.rb @@ -5,6 +5,7 @@ module Api class AssetsController < BaseController before_action :load_team, :load_project, :load_experiment, :load_task, :load_protocol, :load_step before_action :load_asset, only: :show + before_action :check_upload_type, only: :create def index attachments = @step.assets @@ -21,9 +22,18 @@ module Api def create raise PermissionError.new(Asset, :create) unless can_manage_protocol_in_module?(@protocol) - asset = @step.assets.new(asset_params) - asset.save!(context: :on_api_upload) + if @form_multipart_upload + asset = @step.assets.new(asset_params) + else + blob = ActiveStorage::Blob.create_after_upload!( + io: StringIO.new(Base64.decode64(asset_params[:file_data])), + filename: asset_params[:file_name], + content_type: asset_params[:file_type] + ) + asset = @step.assets.new(file: blob) + end + asset.save!(context: :on_api_upload) asset.post_process_file render jsonapi: asset, @@ -36,7 +46,9 @@ module Api def asset_params raise TypeError unless params.require(:data).require(:type) == 'attachments' - attr_list = %i(file) + return params.require(:data).require(:attributes).permit(:file) if @form_multipart_upload + + attr_list = %i(file_data file_type file_name) params.require(:data).require(:attributes).require(attr_list) params.require(:data).require(:attributes).permit(attr_list) end @@ -45,6 +57,10 @@ module Api @asset = @step.assets.find(params.require(:id)) raise PermissionError.new(Asset, :read) unless can_read_protocol_in_module?(@asset.step.protocol) end + + def check_upload_type + @form_multipart_upload = true if params.dig(:data, :attributes, :file) + end end end end diff --git a/app/controllers/api/v1/results_controller.rb b/app/controllers/api/v1/results_controller.rb index 27d4d56e5..6d6bcde2d 100644 --- a/app/controllers/api/v1/results_controller.rb +++ b/app/controllers/api/v1/results_controller.rb @@ -17,7 +17,6 @@ module Api def create create_text_result if result_text_params.present? - create_file_result if !@result && result_file_params.present? render jsonapi: @result, @@ -31,6 +30,7 @@ module Api update_file_result if result_file_params.present? && @result.is_asset update_text_result if result_text_params.present? && @result.is_text + if (@result.changed? && @result.save!) || @asset_result_updated render jsonapi: @result, serializer: ResultSerializer, @@ -103,7 +103,12 @@ module Api def create_file_result Result.transaction do @result = @task.results.create!(result_params.merge(user_id: current_user.id)) - asset = Asset.create!(result_file_params) + if @form_multipart_upload + asset = Asset.create!(result_file_params) + else + blob = create_blob_from_params + asset = Asset.create!(file: blob) + end ResultAsset.create!(asset: asset, result: @result) end end @@ -112,12 +117,26 @@ module Api old_checksum, new_checksum = nil Result.transaction do old_checksum = @result.asset.file.blob.checksum - @result.asset.file.attach(result_file_params[:file]) + if @form_multipart_upload + @result.asset.file.attach(result_file_params[:file]) + else + blob = create_blob_from_params + @result.asset.update!(file: blob) + end new_checksum = @result.asset.file.blob.checksum end @asset_result_updated = old_checksum != new_checksum end + def create_blob_from_params + blob = ActiveStorage::Blob.create_after_upload!( + io: StringIO.new(Base64.decode64(result_file_params[:file_data])), + filename: result_file_params[:file_name], + content_type: result_file_params[:file_type] + ) + blob + end + def result_params raise TypeError unless params.require(:data).require(:type) == 'results' @@ -139,8 +158,14 @@ module Api prms = params[:included]&.select { |el| el[:type] == 'result_files' }&.first return nil unless prms - prms.require(:attributes).require(:file) - prms.dig(:attributes).permit(:file) + if prms.require(:attributes)[:file] + @form_multipart_upload = true + return prms.dig(:attributes).permit(:file) + end + attr_list = %i(file_data file_type file_name) + + prms.require(:attributes).require(attr_list) + prms.dig(:attributes).permit(attr_list) end def tiny_mce_asset_params diff --git a/spec/requests/api/v1/assets_controller_spec.rb b/spec/requests/api/v1/assets_controller_spec.rb index 8369648ae..725c04d37 100644 --- a/spec/requests/api/v1/assets_controller_spec.rb +++ b/spec/requests/api/v1/assets_controller_spec.rb @@ -27,13 +27,13 @@ RSpec.describe 'Api::V1::AssetsController', type: :request do create(:step_asset, step: @step, asset: asset) get(api_v1_team_project_experiment_task_protocol_step_assets_path( - team_id: @team.id, - project_id: @project.id, - experiment_id: @experiment.id, - task_id: @task.id, - protocol_id: @protocol.id, - step_id: @step.id - ), headers: @valid_headers) + team_id: @team.id, + project_id: @project.id, + experiment_id: @experiment.id, + task_id: @task.id, + protocol_id: @protocol.id, + step_id: @step.id + ), headers: @valid_headers) expect(response).to have_http_status(200) end @@ -42,13 +42,13 @@ RSpec.describe 'Api::V1::AssetsController', type: :request do context 'when protocol is not found' do it 'renders 404' do get(api_v1_team_project_experiment_task_protocol_step_assets_path( - team_id: @team.id, - project_id: @project.id, - experiment_id: @experiment.id, - task_id: @task.id, - protocol_id: -1, - step_id: @step.id - ), headers: @valid_headers) + team_id: @team.id, + project_id: @project.id, + experiment_id: @experiment.id, + task_id: @task.id, + protocol_id: -1, + step_id: @step.id + ), headers: @valid_headers) expect(response).to have_http_status(404) end @@ -61,14 +61,14 @@ RSpec.describe 'Api::V1::AssetsController', type: :request do create(:step_asset, step: @step, asset: asset) get(api_v1_team_project_experiment_task_protocol_step_asset_path( - team_id: @team.id, - project_id: @project.id, - experiment_id: @experiment.id, - task_id: @task.id, - protocol_id: @protocol.id, - step_id: @step.id, - id: asset.id - ), headers: @valid_headers) + team_id: @team.id, + project_id: @project.id, + experiment_id: @experiment.id, + task_id: @task.id, + protocol_id: @protocol.id, + step_id: @step.id, + id: asset.id + ), headers: @valid_headers) expect(response).to have_http_status(200) end @@ -77,14 +77,14 @@ RSpec.describe 'Api::V1::AssetsController', type: :request do context 'when experiment is not found' do it 'renders 404' do get(api_v1_team_project_experiment_task_protocol_step_asset_path( - team_id: @team.id, - project_id: @project.id, - experiment_id: -1, - task_id: @task.id, - protocol_id: @protocol.id, - step_id: @step.id, - id: asset.id - ), headers: @valid_headers) + team_id: @team.id, + project_id: @project.id, + experiment_id: -1, + task_id: @task.id, + protocol_id: @protocol.id, + step_id: @step.id, + id: asset.id + ), headers: @valid_headers) expect(response).to have_http_status(404) end @@ -93,14 +93,14 @@ RSpec.describe 'Api::V1::AssetsController', type: :request do context 'when asset is not found' do it 'renders 404' do get(api_v1_team_project_experiment_task_protocol_step_asset_path( - team_id: @team.id, - project_id: @project.id, - experiment_id: @experiment.id, - task_id: @task.id, - protocol_id: @protocol.id, - step_id: @step.id, - id: -1 - ), headers: @valid_headers) + team_id: @team.id, + project_id: @project.id, + experiment_id: @experiment.id, + task_id: @task.id, + protocol_id: @protocol.id, + step_id: @step.id, + id: -1 + ), headers: @valid_headers) expect(response).to have_http_status(404) end @@ -113,62 +113,81 @@ RSpec.describe 'Api::V1::AssetsController', type: :request do end before :each do - @file = fixture_file_upload('files/test.jpg', 'image/jpg') allow_any_instance_of(Asset).to receive(:post_process_file) end let(:action) do post(api_v1_team_project_experiment_task_protocol_step_assets_path( - team_id: @team.id, - project_id: @project.id, - experiment_id: @experiment.id, - task_id: @task.id, - protocol_id: @protocol.id, - step_id: @step.id - ), + team_id: @team.id, + project_id: @project.id, + experiment_id: @experiment.id, + task_id: @task.id, + protocol_id: @protocol.id, + step_id: @step.id + ), params: request_body, headers: @valid_headers) end + let(:request_body) do + { + data: { + type: 'attachments', + attributes: attributes + } + } + end context 'when has valid params' do - let(:request_body) do - { - data: { - type: 'attachments', - attributes: { - file: @file - } - } - } + context 'when multipart form' do + let(:file) { fixture_file_upload('files/test.jpg', 'image/jpg') } + let(:attributes) { { file: file } } + + it 'creates new asset' do + expect { action }.to change { Asset.count }.by(1) + end + + it 'returns status 201' do + action + + expect(response).to have_http_status 201 + end + + it 'calls post_process_file function for text extraction' do + expect_any_instance_of(Asset).to receive(:post_process_file) + + action + end end - it 'creates new asset' do - expect { action }.to change { Asset.count }.by(1) - end + context 'when base64 with json' do + let(:filedata_base64) do + 'iVBORw0KGgoAAAANSUhEUgAAAAIAA'\ + 'AACCAIAAAD91JpzAAAACXBIWXMAAC4jAAAuIwF4pT92AAAAE0lE'\ + 'QVQIHWP8//8/AwMDExADAQAkBgMBOOSShwAAAABJRU5ErkJggg==' + end + let(:attributes) { { file_data: filedata_base64, file_name: 'file.png', file_type: 'image/png' } } + let(:request_body) { super().to_json } - it 'returns status 201' do - action + it 'creates new asset' do + expect { action }.to change { Asset.count }.by(1) + end - expect(response).to have_http_status 201 - end + it 'returns status 201' do + action - it 'calls post_process_file function for text extraction' do - expect_any_instance_of(Asset).to receive(:post_process_file) + expect(response).to have_http_status 201 + end - action + it 'calls post_process_file function for text extraction' do + expect_any_instance_of(Asset).to receive(:post_process_file) + + action + end end end context 'when has missing param' do - let(:request_body) do - { - data: { - type: 'attachments', - attributes: { - } - } - } - end + let(:request_body) { { data: { type: 'attachments', attributes: {} } } } it 'renders 400' do action diff --git a/spec/requests/api/v1/results_controller_spec.rb b/spec/requests/api/v1/results_controller_spec.rb index 2e36e4079..8e0745f0a 100644 --- a/spec/requests/api/v1/results_controller_spec.rb +++ b/spec/requests/api/v1/results_controller_spec.rb @@ -268,7 +268,6 @@ RSpec.describe 'Api::V1::ResultsController', type: :request do end context 'when resultType is File' do - let(:file) { fixture_file_upload('files/test.jpg', 'image/jpg') } let(:request_body) do { data: { @@ -277,31 +276,57 @@ RSpec.describe 'Api::V1::ResultsController', type: :request do name: 'my result' } }, - included: [ - { type: 'result_files', - attributes: { - file: file - } } - ] + included: [{ type: 'result_files', attributes: attributes }] } end + let(:action) do post(api_v1_team_project_experiment_task_results_path( - team_id: @teams.first.id, - project_id: @valid_project, - experiment_id: @valid_experiment, - task_id: @valid_task - ), params: request_body, headers: @valid_headers) + team_id: @teams.first.id, + project_id: @valid_project, + experiment_id: @valid_experiment, + task_id: @valid_task + ), params: request_body, headers: @valid_headers) end - it 'creates new asset' do - expect { action }.to change { ResultAsset.count }.by(1) + context 'when sending base64' do + let(:filedata_base64) do + 'iVBORw0KGgoAAAANSUhEUgAAAAIAA'\ + 'AACCAIAAAD91JpzAAAACXBIWXMAAC4jAAAuIwF4pT92AAAAE0lE'\ + 'QVQIHWP8//8/AwMDExADAQAkBgMBOOSShwAAAABJRU5ErkJggg==' + end + let(:attributes) do + { + file_data: filedata_base64, + file_name: 'file.png', + file_type: 'image/png' + } + end + let(:request_body) { super().to_json } + + it 'creates new asset' do + expect { action }.to change { ResultAsset.count }.by(1) + end + + it 'returns status 201' do + action + + expect(response).to have_http_status 201 + end end - it 'returns status 201' do - action + context 'when sending multipart form' do + let(:attributes) { { file: fixture_file_upload('files/test.jpg', 'image/jpg') } } - expect(response).to have_http_status 201 + it 'creates new asset' do + expect { action }.to change { ResultAsset.count }.by(1) + end + + it 'returns status 201' do + action + + expect(response).to have_http_status 201 + end end end end @@ -371,7 +396,6 @@ RSpec.describe 'Api::V1::ResultsController', type: :request do context 'when resultType is file' do let(:result_file) { @valid_task.results.last } let(:file) { fixture_file_upload('files/test.jpg', 'image/jpg') } - let(:second_file) { fixture_file_upload('files/apple.jpg', 'image/jpg') } let(:request_body) do { data: { @@ -390,12 +414,12 @@ RSpec.describe 'Api::V1::ResultsController', type: :request do end let(:action) do put(api_v1_team_project_experiment_task_result_path( - team_id: @teams.first.id, - project_id: @valid_project, - experiment_id: @valid_experiment, - task_id: @valid_task, - id: result_file.id - ), params: request_body, headers: @valid_headers) + team_id: @teams.first.id, + project_id: @valid_project, + experiment_id: @valid_experiment, + task_id: @valid_task, + id: result_file.id + ), params: request_body, headers: @valid_headers) end context 'when has attributes for update' do @@ -412,38 +436,56 @@ RSpec.describe 'Api::V1::ResultsController', type: :request do end end - # ### Refactor without instance variables - # - # context 'when has new image for update' do - # let(:request_body_with_same_name_new_file) do - # { - # data: { - # type: 'results', - # attributes: { - # name: result_file.reload.name - # } - # }, - # included: [ - # { type: 'result_files', - # attributes: { - # file: second_file - # } } - # ] - # } - # end - # - # it 'returns status 200' do - # put(api_v1_team_project_experiment_task_result_path( - # team_id: @teams.first.id, - # project_id: @valid_project, - # experiment_id: @valid_experiment, - # task_id: @valid_task, - # id: result_file.id - # ), params: request_body_with_same_name_new_file, headers: @valid_headers) - # - # expect(response).to have_http_status 200 - # end - # end + context 'when has new image for update' do + let(:action) do + put(api_v1_team_project_experiment_task_result_path( + team_id: @teams.first.id, + project_id: @valid_project, + experiment_id: @valid_experiment, + task_id: @valid_task, + id: result_file.id + ), params: request_body, headers: @valid_headers) + end + + let(:request_body) do + { + data: { type: 'results', attributes: { name: result_file.reload.name } }, + included: [{ type: 'result_files', attributes: attributes }] + } + end + + context 'when sending base64' do + let(:filedata_base64) do + 'iVBORw0KGgoAAAANSUhEUgAAAAIAA'\ + 'AACCAIAAAD91JpzAAAACXBIWXMAAC4jAAAuIwF4pT92AAAAE0lE'\ + 'QVQIHWP8//8/AwMDExADAQAkBgMBOOSShwAAAABJRU5ErkJggg==' + end + let(:attributes) do + { + file_data: filedata_base64, + file_name: 'file.png', + file_type: 'image/png' + } + end + let(:request_body) { super().to_json } + + it 'returns status 200' do + action + + expect(response).to have_http_status 200 + end + end + + context 'when sending multipart form' do + let(:attributes) { { file: fixture_file_upload('files/apple.jpg', 'image/jpg') } } + + it 'returns status 200' do + action + + expect(response).to have_http_status 200 + end + end + end context 'when there is nothing to update' do let(:request_body_with_same_name) do @@ -459,12 +501,12 @@ RSpec.describe 'Api::V1::ResultsController', type: :request do it 'returns 204' do put(api_v1_team_project_experiment_task_result_path( - team_id: @teams.first.id, - project_id: @valid_project, - experiment_id: @valid_experiment, - task_id: @valid_task, - id: result_file.id - ), params: request_body_with_same_name.to_json, headers: @valid_headers) + team_id: @teams.first.id, + project_id: @valid_project, + experiment_id: @valid_experiment, + task_id: @valid_task, + id: result_file.id + ), params: request_body_with_same_name.to_json, headers: @valid_headers) expect(response).to have_http_status 204 end