From d5dc783b5ca7650bd9edd1dec14ed421484f9497 Mon Sep 17 00:00:00 2001 From: Sergey Krutous Date: Wed, 24 Jun 2015 17:12:58 +0300 Subject: [PATCH 01/37] adding theme and theme_status models --- Gemfile | 1 + Gemfile.lock | 3 +++ app/models/theme.rb | 11 +++++++++++ app/models/theme_status.rb | 6 ++++++ db/migrate/20150624111841_create_themes.rb | 9 +++++++++ .../20150624130545_create_theme_statuses.rb | 7 +++++++ db/schema.rb | 12 +++++++++++- spec/factories/theme_statuses.rb | 5 +++++ spec/factories/themes.rb | 7 +++++++ spec/models/theme_spec.rb | 17 +++++++++++++++++ spec/models/theme_status_spec.rb | 8 ++++++++ 11 files changed, 85 insertions(+), 1 deletion(-) create mode 100644 app/models/theme.rb create mode 100644 app/models/theme_status.rb create mode 100644 db/migrate/20150624111841_create_themes.rb create mode 100644 db/migrate/20150624130545_create_theme_statuses.rb create mode 100644 spec/factories/theme_statuses.rb create mode 100644 spec/factories/themes.rb create mode 100644 spec/models/theme_spec.rb create mode 100644 spec/models/theme_status_spec.rb diff --git a/Gemfile b/Gemfile index 90dcc0f..1940d09 100644 --- a/Gemfile +++ b/Gemfile @@ -59,6 +59,7 @@ group :test do gem 'factory_girl', '~> 4.5.0' gem 'rspec-expectations', '~> 3.2.1' gem 'factory_girl_rails', '~> 4.0' + gem 'shoulda-matchers', '~> 2.8.0' gem 'faker', '~> 1.4.3' #gem 'capybara', '~> 2.4.4' end diff --git a/Gemfile.lock b/Gemfile.lock index 2a9ba39..447c9fe 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -158,6 +158,8 @@ GEM sdoc (0.4.1) json (~> 1.7, >= 1.7.7) rdoc (~> 4.0) + shoulda-matchers (2.8.0) + activesupport (>= 3.0.0) slim (3.0.6) temple (~> 0.7.3) tilt (>= 1.3.3, < 2.1) @@ -212,6 +214,7 @@ DEPENDENCIES rspec-rails (~> 3.2.3) sass-rails (~> 5.0) sdoc (~> 0.4.0) + shoulda-matchers (~> 2.8.0) slim-rails (~> 3.0.1) spring sqlite3 diff --git a/app/models/theme.rb b/app/models/theme.rb new file mode 100644 index 0000000..05c53f7 --- /dev/null +++ b/app/models/theme.rb @@ -0,0 +1,11 @@ +class Theme < ActiveRecord::Base + NAME_LIMIT = 100 + ZIP_FILE_URL_LIMIT = 2000 + + validates_presence_of :name, :zip_file_url, :theme_status + validates_length_of :name, maximum: NAME_LIMIT + validates_length_of :zip_file_url, maximum: ZIP_FILE_URL_LIMIT + validates_uniqueness_of :name, :zip_file_url, case_sensitive: false + + belongs_to :theme_status +end \ No newline at end of file diff --git a/app/models/theme_status.rb b/app/models/theme_status.rb new file mode 100644 index 0000000..a49d689 --- /dev/null +++ b/app/models/theme_status.rb @@ -0,0 +1,6 @@ +class ThemeStatus < ActiveRecord::Base + NAME_LIMIT = 20 + validates_presence_of :name + validates_length_of :name, maximum: NAME_LIMIT + validates_uniqueness_of :name, case_sensitive: false +end diff --git a/db/migrate/20150624111841_create_themes.rb b/db/migrate/20150624111841_create_themes.rb new file mode 100644 index 0000000..3965d51 --- /dev/null +++ b/db/migrate/20150624111841_create_themes.rb @@ -0,0 +1,9 @@ +class CreateThemes < ActiveRecord::Migration + def change + create_table :themes do |t| + t.string :name, null:false, limit:100 + t.string :zip_file_url, null:false, limit:2000 + t.integer :theme_status_id, null:false + end + end +end diff --git a/db/migrate/20150624130545_create_theme_statuses.rb b/db/migrate/20150624130545_create_theme_statuses.rb new file mode 100644 index 0000000..4705864 --- /dev/null +++ b/db/migrate/20150624130545_create_theme_statuses.rb @@ -0,0 +1,7 @@ +class CreateThemeStatuses < ActiveRecord::Migration + def change + create_table :theme_statuses do |t| + t.string :name, null: false, limit:20 + end + end +end diff --git a/db/schema.rb b/db/schema.rb index 4dfbb16..bd85a87 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -11,6 +11,16 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 0) do +ActiveRecord::Schema.define(version: 20150624130545) do + + create_table "theme_statuses", force: :cascade do |t| + t.string "name", limit: 20, null: false + end + + create_table "themes", force: :cascade do |t| + t.string "name", limit: 100, null: false + t.string "zip_file_url", limit: 2000, null: false + t.integer "theme_status_id", null: false + end end diff --git a/spec/factories/theme_statuses.rb b/spec/factories/theme_statuses.rb new file mode 100644 index 0000000..3451eb9 --- /dev/null +++ b/spec/factories/theme_statuses.rb @@ -0,0 +1,5 @@ +FactoryGirl.define do + factory :theme_status do + name { Faker::Lorem.characters(20) } + end +end diff --git a/spec/factories/themes.rb b/spec/factories/themes.rb new file mode 100644 index 0000000..b683b90 --- /dev/null +++ b/spec/factories/themes.rb @@ -0,0 +1,7 @@ +FactoryGirl.define do + factory :theme do + name { Faker::Internet.slug } + zip_file_url { Faker::Internet.url } + theme_status + end +end \ No newline at end of file diff --git a/spec/models/theme_spec.rb b/spec/models/theme_spec.rb new file mode 100644 index 0000000..54fedb0 --- /dev/null +++ b/spec/models/theme_spec.rb @@ -0,0 +1,17 @@ +RSpec.describe Theme, type: :model do + context "when validate" do + it {is_expected.to validate_presence_of :name} + it {expect(build(:theme)).to validate_uniqueness_of(:name).case_insensitive } + it {expect(build(:theme)).to validate_length_of(:name).is_at_most Theme::NAME_LIMIT } + it {is_expected.to have_db_column(:name).with_options limit: Theme::NAME_LIMIT} + + it {is_expected.to validate_presence_of :zip_file_url} + it {expect(build(:theme)).to validate_uniqueness_of(:zip_file_url).case_insensitive} + it {is_expected.to validate_length_of(:zip_file_url).is_at_most Theme::ZIP_FILE_URL_LIMIT } + it {is_expected.to have_db_column(:zip_file_url).with_options limit: Theme::ZIP_FILE_URL_LIMIT} + + it {is_expected.to belong_to :theme_status} + it {is_expected.to validate_presence_of :theme_status} + it {is_expected.to have_db_column(:theme_status_id).with_options null: false} + end +end \ No newline at end of file diff --git a/spec/models/theme_status_spec.rb b/spec/models/theme_status_spec.rb new file mode 100644 index 0000000..92da1fc --- /dev/null +++ b/spec/models/theme_status_spec.rb @@ -0,0 +1,8 @@ +RSpec.describe ThemeStatus, type: :model do + context "when validate" do + it {is_expected.to validate_presence_of :name} + it {expect(build(:theme_status)).to validate_uniqueness_of(:name).case_insensitive } + it {is_expected.to validate_length_of(:name).is_at_most ThemeStatus::NAME_LIMIT } + it {is_expected.to have_db_column(:name).with_options limit: ThemeStatus::NAME_LIMIT} + end +end From 51bc06ecadb789fb064623266ae7842c40505a2d Mon Sep 17 00:00:00 2001 From: Sergey Krutous Date: Wed, 24 Jun 2015 18:24:37 +0300 Subject: [PATCH 02/37] adding empty slim views for index and new actions of themes_controller --- app/controllers/themes_controller.rb | 7 +++++++ app/views/themes/index.html.slim | 0 config/routes.rb | 2 ++ 3 files changed, 9 insertions(+) create mode 100644 app/controllers/themes_controller.rb create mode 100644 app/views/themes/index.html.slim diff --git a/app/controllers/themes_controller.rb b/app/controllers/themes_controller.rb new file mode 100644 index 0000000..5795cf8 --- /dev/null +++ b/app/controllers/themes_controller.rb @@ -0,0 +1,7 @@ +class ThemesController < ApplicationController + def index + end + + def new + end +end diff --git a/app/views/themes/index.html.slim b/app/views/themes/index.html.slim new file mode 100644 index 0000000..e69de29 diff --git a/config/routes.rb b/config/routes.rb index 4e252aa..2fe7189 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -5,6 +5,8 @@ # You can have the root of your site routed with "root" root 'home#index' + resources :themes, only: [:index, :new] + # Example of regular route: # get 'products/:id' => 'catalog#view' From b4be15be8571444512490c24634be6185108f647 Mon Sep 17 00:00:00 2001 From: Sergey Krutous Date: Wed, 24 Jun 2015 18:28:45 +0300 Subject: [PATCH 03/37] adding missing files of new action view and rspec of themes_controller --- app/views/themes/new.html.slim | 0 spec/controllers/themes_controller_spec.rb | 17 +++++++++++++++++ 2 files changed, 17 insertions(+) create mode 100644 app/views/themes/new.html.slim create mode 100644 spec/controllers/themes_controller_spec.rb diff --git a/app/views/themes/new.html.slim b/app/views/themes/new.html.slim new file mode 100644 index 0000000..e69de29 diff --git a/spec/controllers/themes_controller_spec.rb b/spec/controllers/themes_controller_spec.rb new file mode 100644 index 0000000..8f247db --- /dev/null +++ b/spec/controllers/themes_controller_spec.rb @@ -0,0 +1,17 @@ +RSpec.describe ThemesController, type: :controller do + describe "GET #index" do + subject { get :index } + + it { is_expected.to have_http_status(:ok) } + + it { is_expected.to render_template(:index) } + end + + describe "GET #new" do + subject { get :new } + + it { is_expected.to have_http_status(:ok) } + + it { is_expected.to render_template(:new) } + end +end From e8a6165168611fc1ce6dc6e1edb189c145e78a82 Mon Sep 17 00:00:00 2001 From: Sergey Krutous Date: Thu, 25 Jun 2015 10:51:02 +0300 Subject: [PATCH 04/37] shortening validations of theme and theme_status models --- app/models/theme.rb | 9 ++++----- app/models/theme_status.rb | 4 +--- 2 files changed, 5 insertions(+), 8 deletions(-) diff --git a/app/models/theme.rb b/app/models/theme.rb index 05c53f7..c34de25 100644 --- a/app/models/theme.rb +++ b/app/models/theme.rb @@ -2,10 +2,9 @@ class Theme < ActiveRecord::Base NAME_LIMIT = 100 ZIP_FILE_URL_LIMIT = 2000 - validates_presence_of :name, :zip_file_url, :theme_status - validates_length_of :name, maximum: NAME_LIMIT - validates_length_of :zip_file_url, maximum: ZIP_FILE_URL_LIMIT - validates_uniqueness_of :name, :zip_file_url, case_sensitive: false - + validates :name, presence: true, length: { maximum: NAME_LIMIT }, uniqueness: { case_sensitive: false } + validates :zip_file_url, presence: true, length: { maximum: ZIP_FILE_URL_LIMIT }, uniqueness: { case_sensitive: false } + validates_presence_of :theme_status + belongs_to :theme_status end \ No newline at end of file diff --git a/app/models/theme_status.rb b/app/models/theme_status.rb index a49d689..c4b971b 100644 --- a/app/models/theme_status.rb +++ b/app/models/theme_status.rb @@ -1,6 +1,4 @@ class ThemeStatus < ActiveRecord::Base NAME_LIMIT = 20 - validates_presence_of :name - validates_length_of :name, maximum: NAME_LIMIT - validates_uniqueness_of :name, case_sensitive: false + validates :name, presence: true, length: { maximum: NAME_LIMIT}, uniqueness: { case_sensitive: false } end From 5675050b2bb6d0b4d967366fd33187f4cf61ed20 Mon Sep 17 00:00:00 2001 From: Sergey Krutous Date: Thu, 25 Jun 2015 16:59:21 +0300 Subject: [PATCH 05/37] adding policy conditions tests for theme_upload model --- Gemfile | 1 + Gemfile.lock | 4 +++ app/models/theme_upload.rb | 32 +++++++++++++++++++++ config/environments/test.rb | 2 ++ spec/models/theme_upload_spec.rb | 48 ++++++++++++++++++++++++++++++++ 5 files changed, 87 insertions(+) create mode 100644 app/models/theme_upload.rb create mode 100644 spec/models/theme_upload_spec.rb diff --git a/Gemfile b/Gemfile index 1940d09..b254951 100644 --- a/Gemfile +++ b/Gemfile @@ -56,6 +56,7 @@ group :test do gem 'cucumber-rails', '~> 1.4.2', require: false gem 'database_cleaner', '~> 1.4.1' gem 'rspec-rails', '~> 3.2.3' + gem 'rspec-its', '~> 1.2.0' gem 'factory_girl', '~> 4.5.0' gem 'rspec-expectations', '~> 3.2.1' gem 'factory_girl_rails', '~> 4.0' diff --git a/Gemfile.lock b/Gemfile.lock index 447c9fe..69d0efe 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -136,6 +136,9 @@ GEM rspec-expectations (3.2.1) diff-lcs (>= 1.2.0, < 2.0) rspec-support (~> 3.2.0) + rspec-its (1.2.0) + rspec-core (>= 3.0.0) + rspec-expectations (>= 3.0.0) rspec-mocks (3.2.1) diff-lcs (>= 1.2.0, < 2.0) rspec-support (~> 3.2.0) @@ -211,6 +214,7 @@ DEPENDENCIES jquery-rails rails (= 4.2.1) rspec-expectations (~> 3.2.1) + rspec-its (~> 1.2.0) rspec-rails (~> 3.2.3) sass-rails (~> 5.0) sdoc (~> 0.4.0) diff --git a/app/models/theme_upload.rb b/app/models/theme_upload.rb new file mode 100644 index 0000000..953965b --- /dev/null +++ b/app/models/theme_upload.rb @@ -0,0 +1,32 @@ +class ThemeUpload + UPLOADS_FOLDER_NAME = "uploads" + attr_reader :policy_conditions + + def initialize + @policy_conditions = Array.new + end + + def add_policy_condition condition, param1, param2 + @policy_conditions << [condition, param1, param2] + end + + def add_bucket_policy_condition bucket_name + add_policy_condition :eq, :$bucket, bucket_name + end + + def add_acl_policy_condition + add_policy_condition :eq, :$acl, :private + end + + def add_content_length_policy_condition + add_policy_condition :"content-length-range", 1, Rails.configuration.max_theme_zip_file_length + end + + def add_key_policy_condition + add_policy_condition :"starts-with", :$key, UPLOADS_FOLDER_NAME + "/" + end + + def add_redirect_policy_condition redirect_url + add_policy_condition :eq, :$success_action_redirect, redirect_url + end +end diff --git a/config/environments/test.rb b/config/environments/test.rb index 1c19f08..20f4a18 100644 --- a/config/environments/test.rb +++ b/config/environments/test.rb @@ -39,4 +39,6 @@ # Raises error for missing translations # config.action_view.raise_on_missing_translations = true + + Rails.configuration.max_theme_zip_file_length = 1000000000 end diff --git a/spec/models/theme_upload_spec.rb b/spec/models/theme_upload_spec.rb new file mode 100644 index 0000000..60a5789 --- /dev/null +++ b/spec/models/theme_upload_spec.rb @@ -0,0 +1,48 @@ +RSpec.shared_examples "adding first policy condition" do + its(:policy_conditions) { is_expected.to include [:condition, :param_1, :param_2] } +end + +RSpec.describe ThemeUpload, type: :model do + context "has required configuration settings" do + describe "max size of theme zip file" do + it { expect(Rails.configuration.max_theme_zip_file_length).to be_between(1, 5*1024*1024*1024-1).inclusive } + end + end + + context "when initialized" do + its(:policy_conditions) { is_expected.to eq []} + end + + context "when adding policy condition" do + before(:each) { subject.add_policy_condition :condition, :param_1, :param_2 } + + it_behaves_like "adding first policy condition" + + context "and adding second policy condition" do + before(:each) { subject.add_policy_condition :another_condition, :another_param_1, :another_param_2 } + it_behaves_like "adding first policy condition" + its(:policy_conditions) { is_expected.to include [:another_condition, :another_param_1, :another_param_2] } + end + end + + context "when adding bucket policy condition" do + its(:policy_conditions) { expect(subject.add_bucket_policy_condition :bucket_name).to include [:eq, :$bucket, :bucket_name] } + end + + context "when adding acl policy condition" do + its(:policy_conditions) { expect(subject.add_acl_policy_condition).to include [:eq, :$acl, :private] } + end + + context "when adding content length policy condition" do + its(:policy_conditions) { expect(subject.add_content_length_policy_condition).to include [:"content-length-range", 1, Rails.configuration.max_theme_zip_file_length] } + end + + context "when adding key policy condition" do + its(:policy_conditions) { expect(subject.add_key_policy_condition).to include [:"starts-with", :$key, ThemeUpload::UPLOADS_FOLDER_NAME + "/"] } + end + + context "when adding redirect policy condition" do + let(:url){ Faker::Internet.url } + its(:policy_conditions) { expect(subject.add_redirect_policy_condition url).to include [:eq, :$success_action_redirect, url] } + end +end From b0dd7eb406871d484bbf9928608b691c09a0a9d5 Mon Sep 17 00:00:00 2001 From: Sergey Krutous Date: Thu, 25 Jun 2015 17:44:11 +0300 Subject: [PATCH 06/37] adding nullability tests of theme and theme_status model columns --- spec/models/theme_spec.rb | 4 ++-- spec/models/theme_status_spec.rb | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/spec/models/theme_spec.rb b/spec/models/theme_spec.rb index 54fedb0..7be7d87 100644 --- a/spec/models/theme_spec.rb +++ b/spec/models/theme_spec.rb @@ -3,12 +3,12 @@ it {is_expected.to validate_presence_of :name} it {expect(build(:theme)).to validate_uniqueness_of(:name).case_insensitive } it {expect(build(:theme)).to validate_length_of(:name).is_at_most Theme::NAME_LIMIT } - it {is_expected.to have_db_column(:name).with_options limit: Theme::NAME_LIMIT} + it {is_expected.to have_db_column(:name).with_options limit: Theme::NAME_LIMIT, null: false } it {is_expected.to validate_presence_of :zip_file_url} it {expect(build(:theme)).to validate_uniqueness_of(:zip_file_url).case_insensitive} it {is_expected.to validate_length_of(:zip_file_url).is_at_most Theme::ZIP_FILE_URL_LIMIT } - it {is_expected.to have_db_column(:zip_file_url).with_options limit: Theme::ZIP_FILE_URL_LIMIT} + it {is_expected.to have_db_column(:zip_file_url).with_options limit: Theme::ZIP_FILE_URL_LIMIT, null: false } it {is_expected.to belong_to :theme_status} it {is_expected.to validate_presence_of :theme_status} diff --git a/spec/models/theme_status_spec.rb b/spec/models/theme_status_spec.rb index 92da1fc..324ca29 100644 --- a/spec/models/theme_status_spec.rb +++ b/spec/models/theme_status_spec.rb @@ -3,6 +3,6 @@ it {is_expected.to validate_presence_of :name} it {expect(build(:theme_status)).to validate_uniqueness_of(:name).case_insensitive } it {is_expected.to validate_length_of(:name).is_at_most ThemeStatus::NAME_LIMIT } - it {is_expected.to have_db_column(:name).with_options limit: ThemeStatus::NAME_LIMIT} + it {is_expected.to have_db_column(:name).with_options limit: ThemeStatus::NAME_LIMIT, null: false} end end From 88fdf169fff47671f528fdfbaacebe586fd8288c Mon Sep 17 00:00:00 2001 From: Sergey Krutous Date: Fri, 26 Jun 2015 13:12:16 +0300 Subject: [PATCH 07/37] added tests for theme_upload model --- app/models/theme_upload.rb | 38 +++++++--------------- config/environments/test.rb | 1 + spec/models/theme_upload_spec.rb | 54 +++++++++++++------------------- 3 files changed, 35 insertions(+), 58 deletions(-) diff --git a/app/models/theme_upload.rb b/app/models/theme_upload.rb index 953965b..2efabaa 100644 --- a/app/models/theme_upload.rb +++ b/app/models/theme_upload.rb @@ -1,32 +1,18 @@ class ThemeUpload UPLOADS_FOLDER_NAME = "uploads" attr_reader :policy_conditions + attr_reader :policy + attr_reader :encoded_policy + attr_reader :encoded_signature - def initialize - @policy_conditions = Array.new - end - - def add_policy_condition condition, param1, param2 - @policy_conditions << [condition, param1, param2] - end - - def add_bucket_policy_condition bucket_name - add_policy_condition :eq, :$bucket, bucket_name - end - - def add_acl_policy_condition - add_policy_condition :eq, :$acl, :private - end - - def add_content_length_policy_condition - add_policy_condition :"content-length-range", 1, Rails.configuration.max_theme_zip_file_length - end - - def add_key_policy_condition - add_policy_condition :"starts-with", :$key, UPLOADS_FOLDER_NAME + "/" - end - - def add_redirect_policy_condition redirect_url - add_policy_condition :eq, :$success_action_redirect, redirect_url + def initialize bucket_name, redirect_url + @policy_conditions = [[:eq, :$bucket, bucket_name],\ + [:eq, :$acl, :private],\ + [:"content-length-range", 1, Rails.configuration.max_theme_zip_file_length],\ + [:"starts-with", :$key, UPLOADS_FOLDER_NAME + "/"],\ + [:eq, :$success_action_redirect, redirect_url]] + @policy = { conditions: @policy_conditions, expiration: (Time.now + 10*60*60).utc.iso8601 } + @encoded_policy = Base64.encode64(@policy.to_json).gsub("\n","") + @encoded_signature = Base64.encode64(OpenSSL::HMAC.digest(OpenSSL::Digest.new('sha1'), Rails.configuration.aws_secret_key, @encoded_policy)).gsub("\n","") end end diff --git a/config/environments/test.rb b/config/environments/test.rb index 20f4a18..4607653 100644 --- a/config/environments/test.rb +++ b/config/environments/test.rb @@ -41,4 +41,5 @@ # config.action_view.raise_on_missing_translations = true Rails.configuration.max_theme_zip_file_length = 1000000000 + Rails.configuration.aws_secret_key = "xbnXDiIvtqVAmoaFT5yYZWu4Wye/l+CvQt6Cp2bm" end diff --git a/spec/models/theme_upload_spec.rb b/spec/models/theme_upload_spec.rb index 60a5789..ae83ad4 100644 --- a/spec/models/theme_upload_spec.rb +++ b/spec/models/theme_upload_spec.rb @@ -1,48 +1,38 @@ -RSpec.shared_examples "adding first policy condition" do - its(:policy_conditions) { is_expected.to include [:condition, :param_1, :param_2] } -end +RSpec.describe ThemeUpload do + let(:url){ Faker::Internet.url } + let(:bucket_name){ :bucket_name } + subject { ThemeUpload.new bucket_name, url } -RSpec.describe ThemeUpload, type: :model do context "has required configuration settings" do describe "max size of theme zip file" do it { expect(Rails.configuration.max_theme_zip_file_length).to be_between(1, 5*1024*1024*1024-1).inclusive } end - end - context "when initialized" do - its(:policy_conditions) { is_expected.to eq []} + describe "AWS secret key" do + it { expect(Rails.configuration.aws_secret_key).to be_a String } + end end - context "when adding policy condition" do - before(:each) { subject.add_policy_condition :condition, :param_1, :param_2 } + context "when constructing policy" do + let(:utc_now){ (Time.now + 10*60*60).utc.iso8601 } + let(:utc_next_minute){ (Time.now + 10*60*60 + 60).utc.iso8601 } - it_behaves_like "adding first policy condition" + its(:policy) { is_expected.to be_a(Hash)} - context "and adding second policy condition" do - before(:each) { subject.add_policy_condition :another_condition, :another_param_1, :another_param_2 } - it_behaves_like "adding first policy condition" - its(:policy_conditions) { is_expected.to include [:another_condition, :another_param_1, :another_param_2] } + describe "policy expiration" do + it { expect(subject.policy[:expiration]).to be_between utc_now, utc_next_minute } end - end - - context "when adding bucket policy condition" do - its(:policy_conditions) { expect(subject.add_bucket_policy_condition :bucket_name).to include [:eq, :$bucket, :bucket_name] } - end - context "when adding acl policy condition" do - its(:policy_conditions) { expect(subject.add_acl_policy_condition).to include [:eq, :$acl, :private] } - end + its(:policy) { is_expected.to include(conditions: subject.policy_conditions)} + its(:policy_conditions) { is_expected.to be_an Array} + its(:policy_conditions) { is_expected.to include [:eq, :$bucket, :bucket_name] } + its(:policy_conditions) { is_expected.to include [:eq, :$acl, :private] } + its(:policy_conditions) { is_expected.to include [:"content-length-range", 1, Rails.configuration.max_theme_zip_file_length] } + its(:policy_conditions) { is_expected.to include [:"starts-with", :$key, ThemeUpload::UPLOADS_FOLDER_NAME + "/"] } + its(:policy_conditions) { is_expected.to include [:eq, :$success_action_redirect, url] } - context "when adding content length policy condition" do - its(:policy_conditions) { expect(subject.add_content_length_policy_condition).to include [:"content-length-range", 1, Rails.configuration.max_theme_zip_file_length] } - end - - context "when adding key policy condition" do - its(:policy_conditions) { expect(subject.add_key_policy_condition).to include [:"starts-with", :$key, ThemeUpload::UPLOADS_FOLDER_NAME + "/"] } - end + its(:encoded_policy) { is_expected.to eq Base64.encode64(subject.policy.to_json).gsub("\n","")} - context "when adding redirect policy condition" do - let(:url){ Faker::Internet.url } - its(:policy_conditions) { expect(subject.add_redirect_policy_condition url).to include [:eq, :$success_action_redirect, url] } + its(:encoded_signature) { is_expected.to eq Base64.encode64(OpenSSL::HMAC.digest(OpenSSL::Digest::Digest.new('sha1'), Rails.configuration.aws_secret_key, subject.encoded_policy)).gsub("\n","")} end end From a0dc853c4524fb1340f44fdc2c30c1bd36f57ffc Mon Sep 17 00:00:00 2001 From: Sergey Krutous Date: Fri, 26 Jun 2015 16:27:18 +0300 Subject: [PATCH 08/37] saving Theme in create_completed method of theme_controller --- app/controllers/themes_controller.rb | 9 ++++++++ config/routes.rb | 1 + spec/controllers/themes_controller_spec.rb | 24 +++++++++++++++++----- 3 files changed, 29 insertions(+), 5 deletions(-) diff --git a/app/controllers/themes_controller.rb b/app/controllers/themes_controller.rb index 5795cf8..520d5d8 100644 --- a/app/controllers/themes_controller.rb +++ b/app/controllers/themes_controller.rb @@ -4,4 +4,13 @@ def index def new end + + #redirect from AWS + def create_completed + key = params[:key] + theme_name = key.sub /[^_]*_/, "" #theme zip file name (theme name) is located after the first _ (underscore) + theme_status = ThemeStatus.find_by_name! :Processing + Theme.create! name: theme_name, zip_file_url: key, theme_status: theme_status + redirect_to action: :index + end end diff --git a/config/routes.rb b/config/routes.rb index 2fe7189..c514ebc 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -6,6 +6,7 @@ root 'home#index' resources :themes, only: [:index, :new] + get 'themes/create_completed' => "themes#create_completed" #redirect from AWS # Example of regular route: # get 'products/:id' => 'catalog#view' diff --git a/spec/controllers/themes_controller_spec.rb b/spec/controllers/themes_controller_spec.rb index 8f247db..172c59d 100644 --- a/spec/controllers/themes_controller_spec.rb +++ b/spec/controllers/themes_controller_spec.rb @@ -1,17 +1,31 @@ RSpec.describe ThemesController, type: :controller do describe "GET #index" do subject { get :index } - it { is_expected.to have_http_status(:ok) } - it { is_expected.to render_template(:index) } end describe "GET #new" do subject { get :new } - it { is_expected.to have_http_status(:ok) } - it { is_expected.to render_template(:new) } end -end + + describe "GET #create_completed (redirect from AWS)" do + context "with valid parameters" do + before (:all) { ThemeStatus.create! name: :Processing } + + let(:theme_name) { "#{Faker::Lorem.sentence}zip" } + let(:key) { "_#{theme_name}" } + + subject { get :create_completed, key: key } + + it "creates Theme in DB" do + subject + puts "RSPEC THEMES: #{Theme.all[0]}" + expect(Theme.find_by_name(theme_name)).to be_a Theme + end + it { is_expected.to redirect_to action: :index } + end + end +end \ No newline at end of file From 772a68af586768195429f4effe50af27fd57e3c3 Mon Sep 17 00:00:00 2001 From: Sergey Krutous Date: Fri, 26 Jun 2015 16:40:47 +0300 Subject: [PATCH 09/37] removing debug-related line from the working code --- spec/controllers/themes_controller_spec.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/spec/controllers/themes_controller_spec.rb b/spec/controllers/themes_controller_spec.rb index 172c59d..503a0c6 100644 --- a/spec/controllers/themes_controller_spec.rb +++ b/spec/controllers/themes_controller_spec.rb @@ -22,7 +22,6 @@ it "creates Theme in DB" do subject - puts "RSPEC THEMES: #{Theme.all[0]}" expect(Theme.find_by_name(theme_name)).to be_a Theme end it { is_expected.to redirect_to action: :index } From 8719d6cc2c2663b1678af968660adf6263989344 Mon Sep 17 00:00:00 2001 From: Sergey Krutous Date: Fri, 26 Jun 2015 17:08:03 +0300 Subject: [PATCH 10/37] avoiding deprecation warning with OpenSSL::Digest --- spec/models/theme_upload_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/models/theme_upload_spec.rb b/spec/models/theme_upload_spec.rb index ae83ad4..1dce947 100644 --- a/spec/models/theme_upload_spec.rb +++ b/spec/models/theme_upload_spec.rb @@ -33,6 +33,6 @@ its(:encoded_policy) { is_expected.to eq Base64.encode64(subject.policy.to_json).gsub("\n","")} - its(:encoded_signature) { is_expected.to eq Base64.encode64(OpenSSL::HMAC.digest(OpenSSL::Digest::Digest.new('sha1'), Rails.configuration.aws_secret_key, subject.encoded_policy)).gsub("\n","")} + its(:encoded_signature) { is_expected.to eq Base64.encode64(OpenSSL::HMAC.digest(OpenSSL::Digest.new('sha1'), Rails.configuration.aws_secret_key, subject.encoded_policy)).gsub("\n","")} end end From 8821e1628f9772bb87f00f2dadb0d2acffe70f89 Mon Sep 17 00:00:00 2001 From: Sergey Krutous Date: Fri, 26 Jun 2015 17:29:43 +0300 Subject: [PATCH 11/37] requiring rails_helper in specs --- spec/controllers/themes_controller_spec.rb | 12 +++++++----- spec/models/theme_spec.rb | 2 ++ spec/models/theme_status_spec.rb | 2 ++ spec/models/theme_upload_spec.rb | 2 ++ 4 files changed, 13 insertions(+), 5 deletions(-) diff --git a/spec/controllers/themes_controller_spec.rb b/spec/controllers/themes_controller_spec.rb index 503a0c6..b46832e 100644 --- a/spec/controllers/themes_controller_spec.rb +++ b/spec/controllers/themes_controller_spec.rb @@ -1,3 +1,5 @@ +require 'rails_helper' + RSpec.describe ThemesController, type: :controller do describe "GET #index" do subject { get :index } @@ -12,12 +14,12 @@ end describe "GET #create_completed (redirect from AWS)" do + let(:theme_name) { "#{Faker::Lorem.sentence}zip" } + let(:key) { "_#{theme_name}" } + + before (:all) { ThemeStatus.create! name: :Processing } + context "with valid parameters" do - before (:all) { ThemeStatus.create! name: :Processing } - - let(:theme_name) { "#{Faker::Lorem.sentence}zip" } - let(:key) { "_#{theme_name}" } - subject { get :create_completed, key: key } it "creates Theme in DB" do diff --git a/spec/models/theme_spec.rb b/spec/models/theme_spec.rb index 7be7d87..02ee125 100644 --- a/spec/models/theme_spec.rb +++ b/spec/models/theme_spec.rb @@ -1,3 +1,5 @@ +require 'rails_helper' + RSpec.describe Theme, type: :model do context "when validate" do it {is_expected.to validate_presence_of :name} diff --git a/spec/models/theme_status_spec.rb b/spec/models/theme_status_spec.rb index 324ca29..edad6ba 100644 --- a/spec/models/theme_status_spec.rb +++ b/spec/models/theme_status_spec.rb @@ -1,3 +1,5 @@ +require 'rails_helper' + RSpec.describe ThemeStatus, type: :model do context "when validate" do it {is_expected.to validate_presence_of :name} diff --git a/spec/models/theme_upload_spec.rb b/spec/models/theme_upload_spec.rb index 1dce947..21a8ed3 100644 --- a/spec/models/theme_upload_spec.rb +++ b/spec/models/theme_upload_spec.rb @@ -1,3 +1,5 @@ +require 'rails_helper' + RSpec.describe ThemeUpload do let(:url){ Faker::Internet.url } let(:bucket_name){ :bucket_name } From 4ea4e38843c168dbb947a3ccd71517d6eaa660a3 Mon Sep 17 00:00:00 2001 From: Sergey Krutous Date: Fri, 26 Jun 2015 17:42:15 +0300 Subject: [PATCH 12/37] renaming secret key configuration setting according to AWS terminology --- app/models/theme_upload.rb | 2 +- config/environments/test.rb | 2 +- spec/models/theme_upload_spec.rb | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/app/models/theme_upload.rb b/app/models/theme_upload.rb index 2efabaa..c80baf1 100644 --- a/app/models/theme_upload.rb +++ b/app/models/theme_upload.rb @@ -13,6 +13,6 @@ def initialize bucket_name, redirect_url [:eq, :$success_action_redirect, redirect_url]] @policy = { conditions: @policy_conditions, expiration: (Time.now + 10*60*60).utc.iso8601 } @encoded_policy = Base64.encode64(@policy.to_json).gsub("\n","") - @encoded_signature = Base64.encode64(OpenSSL::HMAC.digest(OpenSSL::Digest.new('sha1'), Rails.configuration.aws_secret_key, @encoded_policy)).gsub("\n","") + @encoded_signature = Base64.encode64(OpenSSL::HMAC.digest(OpenSSL::Digest.new('sha1'), Rails.configuration.aws_secret_access_key, @encoded_policy)).gsub("\n","") end end diff --git a/config/environments/test.rb b/config/environments/test.rb index 4607653..2fd4393 100644 --- a/config/environments/test.rb +++ b/config/environments/test.rb @@ -41,5 +41,5 @@ # config.action_view.raise_on_missing_translations = true Rails.configuration.max_theme_zip_file_length = 1000000000 - Rails.configuration.aws_secret_key = "xbnXDiIvtqVAmoaFT5yYZWu4Wye/l+CvQt6Cp2bm" + Rails.configuration.aws_secret_access_key = "xbnXDiIvtqVAmoaFT5yYZWu4Wye/l+CvQt6Cp2bm" end diff --git a/spec/models/theme_upload_spec.rb b/spec/models/theme_upload_spec.rb index 21a8ed3..6225b2c 100644 --- a/spec/models/theme_upload_spec.rb +++ b/spec/models/theme_upload_spec.rb @@ -11,7 +11,7 @@ end describe "AWS secret key" do - it { expect(Rails.configuration.aws_secret_key).to be_a String } + it { expect(Rails.configuration.aws_secret_access_key).to be_a String } end end @@ -35,6 +35,6 @@ its(:encoded_policy) { is_expected.to eq Base64.encode64(subject.policy.to_json).gsub("\n","")} - its(:encoded_signature) { is_expected.to eq Base64.encode64(OpenSSL::HMAC.digest(OpenSSL::Digest.new('sha1'), Rails.configuration.aws_secret_key, subject.encoded_policy)).gsub("\n","")} + its(:encoded_signature) { is_expected.to eq Base64.encode64(OpenSSL::HMAC.digest(OpenSSL::Digest.new('sha1'), Rails.configuration.aws_secret_access_key, subject.encoded_policy)).gsub("\n","")} end end From 6a2f7a6cafb459b9df0a8ee60e506f6725790137 Mon Sep 17 00:00:00 2001 From: Sergey Krutous Date: Tue, 30 Jun 2015 13:03:43 +0300 Subject: [PATCH 13/37] moving theme_upload_spec to classes folder --- spec/{models => classes}/theme_upload_spec.rb | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename spec/{models => classes}/theme_upload_spec.rb (100%) diff --git a/spec/models/theme_upload_spec.rb b/spec/classes/theme_upload_spec.rb similarity index 100% rename from spec/models/theme_upload_spec.rb rename to spec/classes/theme_upload_spec.rb From d3c6b411c0b9ce48270ca85ca54bceb21a543ee1 Mon Sep 17 00:00:00 2001 From: Sergey Krutous Date: Tue, 30 Jun 2015 13:21:26 +0300 Subject: [PATCH 14/37] better formatting of theme_upload tests --- app/models/theme_upload.rb | 14 +++++++------- config/environments/test.rb | 2 +- spec/classes/theme_upload_spec.rb | 10 +++++----- 3 files changed, 13 insertions(+), 13 deletions(-) diff --git a/app/models/theme_upload.rb b/app/models/theme_upload.rb index c80baf1..6d89fe3 100644 --- a/app/models/theme_upload.rb +++ b/app/models/theme_upload.rb @@ -6,13 +6,13 @@ class ThemeUpload attr_reader :encoded_signature def initialize bucket_name, redirect_url - @policy_conditions = [[:eq, :$bucket, bucket_name],\ - [:eq, :$acl, :private],\ - [:"content-length-range", 1, Rails.configuration.max_theme_zip_file_length],\ - [:"starts-with", :$key, UPLOADS_FOLDER_NAME + "/"],\ + @policy_conditions = [[:eq, :$bucket, bucket_name], + [:eq, :$acl, :private], + [:"content-length-range", 1, Rails.configuration.max_theme_zip_file_length], + [:"starts-with", :$key, UPLOADS_FOLDER_NAME + "/"], [:eq, :$success_action_redirect, redirect_url]] - @policy = { conditions: @policy_conditions, expiration: (Time.now + 10*60*60).utc.iso8601 } - @encoded_policy = Base64.encode64(@policy.to_json).gsub("\n","") - @encoded_signature = Base64.encode64(OpenSSL::HMAC.digest(OpenSSL::Digest.new('sha1'), Rails.configuration.aws_secret_access_key, @encoded_policy)).gsub("\n","") + @policy = { conditions: @policy_conditions, expiration: (Time.now + 10.hours).utc.iso8601 } + @encoded_policy = Base64.strict_encode64(@policy.to_json) + @encoded_signature = Base64.strict_encode64(OpenSSL::HMAC.digest(OpenSSL::Digest.new('sha1'), Rails.configuration.aws_secret_access_key, @encoded_policy)) end end diff --git a/config/environments/test.rb b/config/environments/test.rb index 2fd4393..ab58f5e 100644 --- a/config/environments/test.rb +++ b/config/environments/test.rb @@ -40,6 +40,6 @@ # Raises error for missing translations # config.action_view.raise_on_missing_translations = true - Rails.configuration.max_theme_zip_file_length = 1000000000 + Rails.configuration.max_theme_zip_file_length = 1.gigabyte Rails.configuration.aws_secret_access_key = "xbnXDiIvtqVAmoaFT5yYZWu4Wye/l+CvQt6Cp2bm" end diff --git a/spec/classes/theme_upload_spec.rb b/spec/classes/theme_upload_spec.rb index 6225b2c..0ad28e9 100644 --- a/spec/classes/theme_upload_spec.rb +++ b/spec/classes/theme_upload_spec.rb @@ -7,7 +7,7 @@ context "has required configuration settings" do describe "max size of theme zip file" do - it { expect(Rails.configuration.max_theme_zip_file_length).to be_between(1, 5*1024*1024*1024-1).inclusive } + it { expect(Rails.configuration.max_theme_zip_file_length).to be_between(1, 5.gigabytes - 1).inclusive } end describe "AWS secret key" do @@ -16,8 +16,8 @@ end context "when constructing policy" do - let(:utc_now){ (Time.now + 10*60*60).utc.iso8601 } - let(:utc_next_minute){ (Time.now + 10*60*60 + 60).utc.iso8601 } + let(:utc_now){ (Time.now + 10.hours).utc.iso8601 } + let(:utc_next_minute){ (Time.now + 10.hours + 1.minute).utc.iso8601 } its(:policy) { is_expected.to be_a(Hash)} @@ -33,8 +33,8 @@ its(:policy_conditions) { is_expected.to include [:"starts-with", :$key, ThemeUpload::UPLOADS_FOLDER_NAME + "/"] } its(:policy_conditions) { is_expected.to include [:eq, :$success_action_redirect, url] } - its(:encoded_policy) { is_expected.to eq Base64.encode64(subject.policy.to_json).gsub("\n","")} + its(:encoded_policy) { is_expected.to eq Base64.strict_encode64(subject.policy.to_json) } - its(:encoded_signature) { is_expected.to eq Base64.encode64(OpenSSL::HMAC.digest(OpenSSL::Digest.new('sha1'), Rails.configuration.aws_secret_access_key, subject.encoded_policy)).gsub("\n","")} + its(:encoded_signature) { is_expected.to eq Base64.strict_encode64(OpenSSL::HMAC.digest(OpenSSL::Digest.new('sha1'), Rails.configuration.aws_secret_access_key, subject.encoded_policy))} end end From 6e71e24d364a6d23f3fd023b418de169d6b908df Mon Sep 17 00:00:00 2001 From: Sergey Krutous Date: Tue, 30 Jun 2015 13:36:36 +0300 Subject: [PATCH 15/37] better formatting for theme_controller status --- spec/controllers/themes_controller_spec.rb | 6 +++--- spec/factories/theme_statuses.rb | 4 ++++ 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/spec/controllers/themes_controller_spec.rb b/spec/controllers/themes_controller_spec.rb index b46832e..fc97de2 100644 --- a/spec/controllers/themes_controller_spec.rb +++ b/spec/controllers/themes_controller_spec.rb @@ -17,13 +17,13 @@ let(:theme_name) { "#{Faker::Lorem.sentence}zip" } let(:key) { "_#{theme_name}" } - before (:all) { ThemeStatus.create! name: :Processing } + before (:all) { create :processing_theme_status } context "with valid parameters" do - subject { get :create_completed, key: key } + before(:each) { get :create_completed, key: key } + subject { response } it "creates Theme in DB" do - subject expect(Theme.find_by_name(theme_name)).to be_a Theme end it { is_expected.to redirect_to action: :index } diff --git a/spec/factories/theme_statuses.rb b/spec/factories/theme_statuses.rb index 3451eb9..94a14b8 100644 --- a/spec/factories/theme_statuses.rb +++ b/spec/factories/theme_statuses.rb @@ -2,4 +2,8 @@ factory :theme_status do name { Faker::Lorem.characters(20) } end + + factory :processing_theme_status, class: ThemeStatus do + name { :Processing } + end end From 78fc950122edeffa59fa24abdbe04ec0c3f28e99 Mon Sep 17 00:00:00 2001 From: Sergey Krutous Date: Tue, 30 Jun 2015 14:19:45 +0300 Subject: [PATCH 16/37] adding more tests for key format in create_completed method of themes_controller --- spec/controllers/themes_controller_spec.rb | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/spec/controllers/themes_controller_spec.rb b/spec/controllers/themes_controller_spec.rb index fc97de2..ca42678 100644 --- a/spec/controllers/themes_controller_spec.rb +++ b/spec/controllers/themes_controller_spec.rb @@ -14,19 +14,22 @@ end describe "GET #create_completed (redirect from AWS)" do - let(:theme_name) { "#{Faker::Lorem.sentence}zip" } - let(:key) { "_#{theme_name}" } - before (:all) { create :processing_theme_status } context "with valid parameters" do - before(:each) { get :create_completed, key: key } - subject { response } - - it "creates Theme in DB" do - expect(Theme.find_by_name(theme_name)).to be_a Theme - end + let(:theme_name) { "#{Faker::Lorem.sentence}zip" } + let(:key) { "_#{theme_name}" } + subject { get :create_completed, key: key } it { is_expected.to redirect_to action: :index } end + + context "when parsing theme name in key" do + {"_qwerty" => "qwerty", "1234_qwerty" => "qwerty", "12345_67890_qwerty" => "67890_qwerty"}.each do |key, theme_name| + it "creates Theme in DB when key is #{key}" do + get :create_completed, key: key + expect(Theme.find_by_name(theme_name)).to be_a Theme + end + end + end end end \ No newline at end of file From 51a2e1aaa469a98e005ce6d5110aaf545030dce2 Mon Sep 17 00:00:00 2001 From: Sergey Krutous Date: Tue, 30 Jun 2015 15:20:51 +0300 Subject: [PATCH 17/37] adding more theme attribute assertions for create_completed method of themes_controller --- spec/controllers/themes_controller_spec.rb | 23 +++++++++++++++------- 1 file changed, 16 insertions(+), 7 deletions(-) diff --git a/spec/controllers/themes_controller_spec.rb b/spec/controllers/themes_controller_spec.rb index ca42678..d77ef27 100644 --- a/spec/controllers/themes_controller_spec.rb +++ b/spec/controllers/themes_controller_spec.rb @@ -1,5 +1,16 @@ require 'rails_helper' +RSpec.shared_examples "creating Theme in DB" do |key, theme_name| + before(:each) { get :create_completed, key: key } + subject(:theme) { Theme.find_by_name theme_name } + #let(:processing_theme_status) { ThemeStatus.find_by_name :Processing } + describe "Theme with name #{theme_name}" do + it { is_expected.to be } + end + its(:zip_file_url) { is_expected.to eq key } + its(:theme_status) { is_expected.to eq processing_theme_status } +end + RSpec.describe ThemesController, type: :controller do describe "GET #index" do subject { get :index } @@ -14,7 +25,8 @@ end describe "GET #create_completed (redirect from AWS)" do - before (:all) { create :processing_theme_status } + before (:all) { @processing_theme_status = create :processing_theme_status } + let (:processing_theme_status) { @processing_theme_status } context "with valid parameters" do let(:theme_name) { "#{Faker::Lorem.sentence}zip" } @@ -23,12 +35,9 @@ it { is_expected.to redirect_to action: :index } end - context "when parsing theme name in key" do - {"_qwerty" => "qwerty", "1234_qwerty" => "qwerty", "12345_67890_qwerty" => "67890_qwerty"}.each do |key, theme_name| - it "creates Theme in DB when key is #{key}" do - get :create_completed, key: key - expect(Theme.find_by_name(theme_name)).to be_a Theme - end + {"_qwerty" => "qwerty", "1234_qwerty" => "qwerty", "12345_67890_qwerty" => "67890_qwerty"}.each do |key, theme_name| + describe "when parsing key: #{key}" do + it_behaves_like "creating Theme in DB", key, theme_name end end end From f6554d0249cdcad05f7b159483d57cdb4f37f377 Mon Sep 17 00:00:00 2001 From: Sergey Krutous Date: Tue, 30 Jun 2015 17:13:58 +0300 Subject: [PATCH 18/37] replacing theme_status model with status enum in theme model --- app/controllers/themes_controller.rb | 3 +-- app/models/theme.rb | 6 +++--- app/models/theme_status.rb | 4 ---- db/migrate/20150624111841_create_themes.rb | 2 +- .../20150624130545_create_theme_statuses.rb | 7 ------- db/schema.rb | 16 ++++++---------- spec/controllers/themes_controller_spec.rb | 8 ++------ spec/factories/theme_statuses.rb | 9 --------- spec/factories/themes.rb | 2 +- spec/models/theme_spec.rb | 5 ++--- spec/models/theme_status_spec.rb | 10 ---------- 11 files changed, 16 insertions(+), 56 deletions(-) delete mode 100644 app/models/theme_status.rb delete mode 100644 db/migrate/20150624130545_create_theme_statuses.rb delete mode 100644 spec/factories/theme_statuses.rb delete mode 100644 spec/models/theme_status_spec.rb diff --git a/app/controllers/themes_controller.rb b/app/controllers/themes_controller.rb index 520d5d8..0b925c1 100644 --- a/app/controllers/themes_controller.rb +++ b/app/controllers/themes_controller.rb @@ -9,8 +9,7 @@ def new def create_completed key = params[:key] theme_name = key.sub /[^_]*_/, "" #theme zip file name (theme name) is located after the first _ (underscore) - theme_status = ThemeStatus.find_by_name! :Processing - Theme.create! name: theme_name, zip_file_url: key, theme_status: theme_status + Theme.create! name: theme_name, zip_file_url: key, status: :processing redirect_to action: :index end end diff --git a/app/models/theme.rb b/app/models/theme.rb index c34de25..f6ab6a4 100644 --- a/app/models/theme.rb +++ b/app/models/theme.rb @@ -2,9 +2,9 @@ class Theme < ActiveRecord::Base NAME_LIMIT = 100 ZIP_FILE_URL_LIMIT = 2000 + enum status: { processing: 0, uploaded: 1 } + validates :name, presence: true, length: { maximum: NAME_LIMIT }, uniqueness: { case_sensitive: false } validates :zip_file_url, presence: true, length: { maximum: ZIP_FILE_URL_LIMIT }, uniqueness: { case_sensitive: false } - validates_presence_of :theme_status - - belongs_to :theme_status + validates :status, presence: true end \ No newline at end of file diff --git a/app/models/theme_status.rb b/app/models/theme_status.rb deleted file mode 100644 index c4b971b..0000000 --- a/app/models/theme_status.rb +++ /dev/null @@ -1,4 +0,0 @@ -class ThemeStatus < ActiveRecord::Base - NAME_LIMIT = 20 - validates :name, presence: true, length: { maximum: NAME_LIMIT}, uniqueness: { case_sensitive: false } -end diff --git a/db/migrate/20150624111841_create_themes.rb b/db/migrate/20150624111841_create_themes.rb index 3965d51..12a5b68 100644 --- a/db/migrate/20150624111841_create_themes.rb +++ b/db/migrate/20150624111841_create_themes.rb @@ -3,7 +3,7 @@ def change create_table :themes do |t| t.string :name, null:false, limit:100 t.string :zip_file_url, null:false, limit:2000 - t.integer :theme_status_id, null:false + t.integer :status, null:false, default: 0 end end end diff --git a/db/migrate/20150624130545_create_theme_statuses.rb b/db/migrate/20150624130545_create_theme_statuses.rb deleted file mode 100644 index 4705864..0000000 --- a/db/migrate/20150624130545_create_theme_statuses.rb +++ /dev/null @@ -1,7 +0,0 @@ -class CreateThemeStatuses < ActiveRecord::Migration - def change - create_table :theme_statuses do |t| - t.string :name, null: false, limit:20 - end - end -end diff --git a/db/schema.rb b/db/schema.rb index 7987654..86eb557 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -34,6 +34,12 @@ add_index "roles", ["name"], name: "index_roles_on_name", unique: true, using: :btree + create_table "themes", force: :cascade do |t| + t.string "name", limit: 100, null: false + t.string "zip_file_url", limit: 2000, null: false + t.integer "status", default: 0, null: false + end + create_table "users", force: :cascade do |t| t.string "email", default: "", null: false t.string "encrypted_password", default: "", null: false @@ -54,14 +60,4 @@ add_index "users", ["reset_password_token"], name: "index_users_on_reset_password_token", unique: true, using: :btree add_index "users", ["role_id"], name: "index_users_on_role_id", using: :btree - create_table "theme_statuses", force: :cascade do |t| - t.string "name", limit: 20, null: false - end - - create_table "themes", force: :cascade do |t| - t.string "name", limit: 100, null: false - t.string "zip_file_url", limit: 2000, null: false - t.integer "theme_status_id", null: false - end - end diff --git a/spec/controllers/themes_controller_spec.rb b/spec/controllers/themes_controller_spec.rb index d77ef27..8eb0aa7 100644 --- a/spec/controllers/themes_controller_spec.rb +++ b/spec/controllers/themes_controller_spec.rb @@ -3,12 +3,11 @@ RSpec.shared_examples "creating Theme in DB" do |key, theme_name| before(:each) { get :create_completed, key: key } subject(:theme) { Theme.find_by_name theme_name } - #let(:processing_theme_status) { ThemeStatus.find_by_name :Processing } describe "Theme with name #{theme_name}" do it { is_expected.to be } + its(:zip_file_url) { is_expected.to eq key } + it { is_expected.to be_processing } end - its(:zip_file_url) { is_expected.to eq key } - its(:theme_status) { is_expected.to eq processing_theme_status } end RSpec.describe ThemesController, type: :controller do @@ -25,9 +24,6 @@ end describe "GET #create_completed (redirect from AWS)" do - before (:all) { @processing_theme_status = create :processing_theme_status } - let (:processing_theme_status) { @processing_theme_status } - context "with valid parameters" do let(:theme_name) { "#{Faker::Lorem.sentence}zip" } let(:key) { "_#{theme_name}" } diff --git a/spec/factories/theme_statuses.rb b/spec/factories/theme_statuses.rb deleted file mode 100644 index 94a14b8..0000000 --- a/spec/factories/theme_statuses.rb +++ /dev/null @@ -1,9 +0,0 @@ -FactoryGirl.define do - factory :theme_status do - name { Faker::Lorem.characters(20) } - end - - factory :processing_theme_status, class: ThemeStatus do - name { :Processing } - end -end diff --git a/spec/factories/themes.rb b/spec/factories/themes.rb index b683b90..2c49ee1 100644 --- a/spec/factories/themes.rb +++ b/spec/factories/themes.rb @@ -2,6 +2,6 @@ factory :theme do name { Faker::Internet.slug } zip_file_url { Faker::Internet.url } - theme_status + status { :processing } end end \ No newline at end of file diff --git a/spec/models/theme_spec.rb b/spec/models/theme_spec.rb index 02ee125..7a9cc10 100644 --- a/spec/models/theme_spec.rb +++ b/spec/models/theme_spec.rb @@ -12,8 +12,7 @@ it {is_expected.to validate_length_of(:zip_file_url).is_at_most Theme::ZIP_FILE_URL_LIMIT } it {is_expected.to have_db_column(:zip_file_url).with_options limit: Theme::ZIP_FILE_URL_LIMIT, null: false } - it {is_expected.to belong_to :theme_status} - it {is_expected.to validate_presence_of :theme_status} - it {is_expected.to have_db_column(:theme_status_id).with_options null: false} + it {is_expected.to validate_presence_of :status} + it {is_expected.to have_db_column(:status).with_options null: false, default: 0} end end \ No newline at end of file diff --git a/spec/models/theme_status_spec.rb b/spec/models/theme_status_spec.rb deleted file mode 100644 index edad6ba..0000000 --- a/spec/models/theme_status_spec.rb +++ /dev/null @@ -1,10 +0,0 @@ -require 'rails_helper' - -RSpec.describe ThemeStatus, type: :model do - context "when validate" do - it {is_expected.to validate_presence_of :name} - it {expect(build(:theme_status)).to validate_uniqueness_of(:name).case_insensitive } - it {is_expected.to validate_length_of(:name).is_at_most ThemeStatus::NAME_LIMIT } - it {is_expected.to have_db_column(:name).with_options limit: ThemeStatus::NAME_LIMIT, null: false} - end -end From c6359f043d02fddc4b7d274d1a01bf39443be917 Mon Sep 17 00:00:00 2001 From: Sergey Krutous Date: Tue, 30 Jun 2015 17:17:48 +0300 Subject: [PATCH 19/37] removing theme status not covered by tests yet --- app/models/theme.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/theme.rb b/app/models/theme.rb index f6ab6a4..364ad97 100644 --- a/app/models/theme.rb +++ b/app/models/theme.rb @@ -2,7 +2,7 @@ class Theme < ActiveRecord::Base NAME_LIMIT = 100 ZIP_FILE_URL_LIMIT = 2000 - enum status: { processing: 0, uploaded: 1 } + enum status: { processing: 0 } validates :name, presence: true, length: { maximum: NAME_LIMIT }, uniqueness: { case_sensitive: false } validates :zip_file_url, presence: true, length: { maximum: ZIP_FILE_URL_LIMIT }, uniqueness: { case_sensitive: false } From 00312e11d0a68b6679f30fd9cd6fa0ef8bd30bc0 Mon Sep 17 00:00:00 2001 From: Sergey Krutous Date: Tue, 30 Jun 2015 17:24:53 +0300 Subject: [PATCH 20/37] DRY-ing theme status spec --- spec/models/theme_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/models/theme_spec.rb b/spec/models/theme_spec.rb index 7a9cc10..c6b78b3 100644 --- a/spec/models/theme_spec.rb +++ b/spec/models/theme_spec.rb @@ -13,6 +13,6 @@ it {is_expected.to have_db_column(:zip_file_url).with_options limit: Theme::ZIP_FILE_URL_LIMIT, null: false } it {is_expected.to validate_presence_of :status} - it {is_expected.to have_db_column(:status).with_options null: false, default: 0} + it {is_expected.to have_db_column(:status).with_options null: false, default: Theme.statuses[:processing]} end end \ No newline at end of file From a629634262826c05736a7392d8ea169a125aeb34 Mon Sep 17 00:00:00 2001 From: Alexander Ivanov Date: Mon, 13 Jul 2015 10:35:00 +0300 Subject: [PATCH 21/37] Moved AWS secret information into .env file --- .env | 1 + app/models/theme_upload.rb | 4 ++-- config/application.rb | 30 +++++++++++++++++------------- config/environments/test.rb | 2 -- spec/classes/theme_upload_spec.rb | 8 ++++---- 5 files changed, 24 insertions(+), 21 deletions(-) diff --git a/.env b/.env index da8ea0c..44bc43e 100644 --- a/.env +++ b/.env @@ -2,3 +2,4 @@ HOST=lvh.me:3000 GMAIL_DOMAIN=gmail.com GMAIL_USERNAME=dontreply1357@gmail.com GMAIL_PASSWORD=13579rty +AWS_SECRET_ACCESS_KEY=xbnXDiIvtqVAmoaFT5yYZWu4Wye/l+CvQt6Cp2bm diff --git a/app/models/theme_upload.rb b/app/models/theme_upload.rb index 6d89fe3..812d873 100644 --- a/app/models/theme_upload.rb +++ b/app/models/theme_upload.rb @@ -8,11 +8,11 @@ class ThemeUpload def initialize bucket_name, redirect_url @policy_conditions = [[:eq, :$bucket, bucket_name], [:eq, :$acl, :private], - [:"content-length-range", 1, Rails.configuration.max_theme_zip_file_length], + [:"content-length-range", 1, Rails.application.config.max_theme_zip_file_length], [:"starts-with", :$key, UPLOADS_FOLDER_NAME + "/"], [:eq, :$success_action_redirect, redirect_url]] @policy = { conditions: @policy_conditions, expiration: (Time.now + 10.hours).utc.iso8601 } @encoded_policy = Base64.strict_encode64(@policy.to_json) - @encoded_signature = Base64.strict_encode64(OpenSSL::HMAC.digest(OpenSSL::Digest.new('sha1'), Rails.configuration.aws_secret_access_key, @encoded_policy)) + @encoded_signature = Base64.strict_encode64(OpenSSL::HMAC.digest(OpenSSL::Digest.new('sha1'), Rails.application.config.aws_secret_access_key, @encoded_policy)) end end diff --git a/config/application.rb b/config/application.rb index 9c3e7ed..fd72053 100644 --- a/config/application.rb +++ b/config/application.rb @@ -32,18 +32,22 @@ class Application < Rails::Application # Do not swallow errors in after_commit/after_rollback callbacks. config.active_record.raise_in_transactional_callbacks = true - # ActionMailer config - config.action_mailer.default_url_options = { :host => ENV["HOST"] } - config.action_mailer.raise_delivery_errors = true - config.action_mailer.delivery_method = :smtp - config.action_mailer.perform_deliveries = true - config.action_mailer.smtp_settings = { - address: "smtp.gmail.com", - port: 587, - domain: ENV["GMAIL_DOMAIN"], - authentication: "plain", - enable_starttls_auto: true, - user_name: ENV["GMAIL_USERNAME"], - password: ENV["GMAIL_PASSWORD"]} + # ActionMailer config + config.action_mailer.default_url_options = { :host => ENV["HOST"] } + config.action_mailer.raise_delivery_errors = true + config.action_mailer.delivery_method = :smtp + config.action_mailer.perform_deliveries = true + config.action_mailer.smtp_settings = { + address: "smtp.gmail.com", + port: 587, + domain: ENV["GMAIL_DOMAIN"], + authentication: "plain", + enable_starttls_auto: true, + user_name: ENV["GMAIL_USERNAME"], + password: ENV["GMAIL_PASSWORD"]} + + # AWS bucket config + config.max_theme_zip_file_length = 1.gigabyte + config.aws_secret_access_key = ENV["AWS_SECRET_ACCESS_KEY"] end end diff --git a/config/environments/test.rb b/config/environments/test.rb index ab58f5e..513e8c6 100644 --- a/config/environments/test.rb +++ b/config/environments/test.rb @@ -40,6 +40,4 @@ # Raises error for missing translations # config.action_view.raise_on_missing_translations = true - Rails.configuration.max_theme_zip_file_length = 1.gigabyte - Rails.configuration.aws_secret_access_key = "xbnXDiIvtqVAmoaFT5yYZWu4Wye/l+CvQt6Cp2bm" end diff --git a/spec/classes/theme_upload_spec.rb b/spec/classes/theme_upload_spec.rb index 0ad28e9..71f8436 100644 --- a/spec/classes/theme_upload_spec.rb +++ b/spec/classes/theme_upload_spec.rb @@ -7,11 +7,11 @@ context "has required configuration settings" do describe "max size of theme zip file" do - it { expect(Rails.configuration.max_theme_zip_file_length).to be_between(1, 5.gigabytes - 1).inclusive } + it { expect(Rails.application.config.max_theme_zip_file_length).to be_between(1, 5.gigabytes - 1).inclusive } end describe "AWS secret key" do - it { expect(Rails.configuration.aws_secret_access_key).to be_a String } + it { expect(Rails.application.config.aws_secret_access_key).to be_a String } end end @@ -29,12 +29,12 @@ its(:policy_conditions) { is_expected.to be_an Array} its(:policy_conditions) { is_expected.to include [:eq, :$bucket, :bucket_name] } its(:policy_conditions) { is_expected.to include [:eq, :$acl, :private] } - its(:policy_conditions) { is_expected.to include [:"content-length-range", 1, Rails.configuration.max_theme_zip_file_length] } + its(:policy_conditions) { is_expected.to include [:"content-length-range", 1, Rails.application.config.max_theme_zip_file_length] } its(:policy_conditions) { is_expected.to include [:"starts-with", :$key, ThemeUpload::UPLOADS_FOLDER_NAME + "/"] } its(:policy_conditions) { is_expected.to include [:eq, :$success_action_redirect, url] } its(:encoded_policy) { is_expected.to eq Base64.strict_encode64(subject.policy.to_json) } - its(:encoded_signature) { is_expected.to eq Base64.strict_encode64(OpenSSL::HMAC.digest(OpenSSL::Digest.new('sha1'), Rails.configuration.aws_secret_access_key, subject.encoded_policy))} + its(:encoded_signature) { is_expected.to eq Base64.strict_encode64(OpenSSL::HMAC.digest(OpenSSL::Digest.new('sha1'), Rails.application.config.aws_secret_access_key, subject.encoded_policy))} end end From 77a7f4fdcdef97f44df4f9b9dceb5da0c492e032 Mon Sep 17 00:00:00 2001 From: Alexander Ivanov Date: Tue, 14 Jul 2015 10:59:54 +0300 Subject: [PATCH 22/37] Refactored to use themes_helper instead of theme_upload class, added aws_spec, updated themes_controller --- .env | 1 + app/controllers/themes_controller.rb | 9 +++- app/helpers/themes_helper.rb | 32 +++++++++++++ app/models/theme_upload.rb | 18 -------- config/application.rb | 4 +- spec/classes/theme_upload_spec.rb | 40 ---------------- spec/config/aws_spec.rb | 10 ++++ spec/controllers/themes_controller_spec.rb | 29 ++++++++++-- spec/helpers/themes_helper_spec.rb | 53 ++++++++++++++++++++++ 9 files changed, 130 insertions(+), 66 deletions(-) create mode 100644 app/helpers/themes_helper.rb delete mode 100644 app/models/theme_upload.rb delete mode 100644 spec/classes/theme_upload_spec.rb create mode 100644 spec/config/aws_spec.rb create mode 100644 spec/helpers/themes_helper_spec.rb diff --git a/.env b/.env index 44bc43e..8a3d47d 100644 --- a/.env +++ b/.env @@ -3,3 +3,4 @@ GMAIL_DOMAIN=gmail.com GMAIL_USERNAME=dontreply1357@gmail.com GMAIL_PASSWORD=13579rty AWS_SECRET_ACCESS_KEY=xbnXDiIvtqVAmoaFT5yYZWu4Wye/l+CvQt6Cp2bm +AWS_ACCESS_KEY=AKIAIQ2MGPRTWCHJKK5A diff --git a/app/controllers/themes_controller.rb b/app/controllers/themes_controller.rb index 0b925c1..4dcc62d 100644 --- a/app/controllers/themes_controller.rb +++ b/app/controllers/themes_controller.rb @@ -1,14 +1,19 @@ class ThemesController < ApplicationController + # GET#index def index end + # GET#new def new end - #redirect from AWS + # GET#create_completed redirect from AWS def create_completed key = params[:key] - theme_name = key.sub /[^_]*_/, "" #theme zip file name (theme name) is located after the first _ (underscore) + theme_file_name = key.sub(/[^_]*_/, '') # theme zip file name (theme name) is located after the first _ (underscore) + theme_name = File.basename theme_file_name, File.extname(theme_file_name) + return redirect_to action: :new unless theme_name.present? && File.extname(theme_file_name).downcase.eql?('.zip') + Theme.create! name: theme_name, zip_file_url: key, status: :processing redirect_to action: :index end diff --git a/app/helpers/themes_helper.rb b/app/helpers/themes_helper.rb new file mode 100644 index 0000000..5dbcdb5 --- /dev/null +++ b/app/helpers/themes_helper.rb @@ -0,0 +1,32 @@ +module ThemesHelper + UPLOADS_FOLDER = 'uploads/' + + def theme_upload_params + return @theme_upload_params if @theme_upload_params.present? + + bucket_name = Rails.application.config.aws_public_bucket_name + redirect_url = request.protocol + request.host_with_port + '/themes/create_completed' + + @theme_upload_params = { form_action: "https://#{bucket_name}.s3.amazonaws.com/", + form_method: 'post', + form_enclosure_type: 'multipart/form-data', + acl: 'private', + redirect_url: redirect_url, + file_id: "#{UPLOADS_FOLDER}#{SecureRandom.uuid}_${filename}", + access_key: Rails.application.config.aws_access_key } + + conditions = [[:eq, :$bucket, bucket_name], + [:eq, :$acl, :private], + [:"content-length-range", 1, Rails.application.config.aws_max_theme_zip_file_length], + [:"starts-with", :$key, UPLOADS_FOLDER], + [:eq, :$success_action_redirect, redirect_url]] + policy = { conditions: conditions, expiration: (Time.now + 10.hours).utc.iso8601 } + policy = Base64.strict_encode64(policy.to_json) + + @theme_upload_params.merge! policy: policy + @theme_upload_params.merge! signature: + Base64.strict_encode64(OpenSSL::HMAC.digest(OpenSSL::Digest.new('sha1'), + Rails.application.config.aws_secret_access_key, + policy)) + end +end diff --git a/app/models/theme_upload.rb b/app/models/theme_upload.rb deleted file mode 100644 index 812d873..0000000 --- a/app/models/theme_upload.rb +++ /dev/null @@ -1,18 +0,0 @@ -class ThemeUpload - UPLOADS_FOLDER_NAME = "uploads" - attr_reader :policy_conditions - attr_reader :policy - attr_reader :encoded_policy - attr_reader :encoded_signature - - def initialize bucket_name, redirect_url - @policy_conditions = [[:eq, :$bucket, bucket_name], - [:eq, :$acl, :private], - [:"content-length-range", 1, Rails.application.config.max_theme_zip_file_length], - [:"starts-with", :$key, UPLOADS_FOLDER_NAME + "/"], - [:eq, :$success_action_redirect, redirect_url]] - @policy = { conditions: @policy_conditions, expiration: (Time.now + 10.hours).utc.iso8601 } - @encoded_policy = Base64.strict_encode64(@policy.to_json) - @encoded_signature = Base64.strict_encode64(OpenSSL::HMAC.digest(OpenSSL::Digest.new('sha1'), Rails.application.config.aws_secret_access_key, @encoded_policy)) - end -end diff --git a/config/application.rb b/config/application.rb index fd72053..ba19aac 100644 --- a/config/application.rb +++ b/config/application.rb @@ -47,7 +47,9 @@ class Application < Rails::Application password: ENV["GMAIL_PASSWORD"]} # AWS bucket config - config.max_theme_zip_file_length = 1.gigabyte + config.aws_max_theme_zip_file_length = 1.gigabyte + config.aws_access_key = ENV["AWS_ACCESS_KEY"] config.aws_secret_access_key = ENV["AWS_SECRET_ACCESS_KEY"] + config.aws_public_bucket_name = "flexcommerce-uploadedthemes" end end diff --git a/spec/classes/theme_upload_spec.rb b/spec/classes/theme_upload_spec.rb deleted file mode 100644 index 71f8436..0000000 --- a/spec/classes/theme_upload_spec.rb +++ /dev/null @@ -1,40 +0,0 @@ -require 'rails_helper' - -RSpec.describe ThemeUpload do - let(:url){ Faker::Internet.url } - let(:bucket_name){ :bucket_name } - subject { ThemeUpload.new bucket_name, url } - - context "has required configuration settings" do - describe "max size of theme zip file" do - it { expect(Rails.application.config.max_theme_zip_file_length).to be_between(1, 5.gigabytes - 1).inclusive } - end - - describe "AWS secret key" do - it { expect(Rails.application.config.aws_secret_access_key).to be_a String } - end - end - - context "when constructing policy" do - let(:utc_now){ (Time.now + 10.hours).utc.iso8601 } - let(:utc_next_minute){ (Time.now + 10.hours + 1.minute).utc.iso8601 } - - its(:policy) { is_expected.to be_a(Hash)} - - describe "policy expiration" do - it { expect(subject.policy[:expiration]).to be_between utc_now, utc_next_minute } - end - - its(:policy) { is_expected.to include(conditions: subject.policy_conditions)} - its(:policy_conditions) { is_expected.to be_an Array} - its(:policy_conditions) { is_expected.to include [:eq, :$bucket, :bucket_name] } - its(:policy_conditions) { is_expected.to include [:eq, :$acl, :private] } - its(:policy_conditions) { is_expected.to include [:"content-length-range", 1, Rails.application.config.max_theme_zip_file_length] } - its(:policy_conditions) { is_expected.to include [:"starts-with", :$key, ThemeUpload::UPLOADS_FOLDER_NAME + "/"] } - its(:policy_conditions) { is_expected.to include [:eq, :$success_action_redirect, url] } - - its(:encoded_policy) { is_expected.to eq Base64.strict_encode64(subject.policy.to_json) } - - its(:encoded_signature) { is_expected.to eq Base64.strict_encode64(OpenSSL::HMAC.digest(OpenSSL::Digest.new('sha1'), Rails.application.config.aws_secret_access_key, subject.encoded_policy))} - end -end diff --git a/spec/config/aws_spec.rb b/spec/config/aws_spec.rb new file mode 100644 index 0000000..f588712 --- /dev/null +++ b/spec/config/aws_spec.rb @@ -0,0 +1,10 @@ +require 'rails_helper' + +RSpec.describe Rails.application.config do + describe "has set Amazon S3 options" do + it {expect(subject.aws_max_theme_zip_file_length).to be_between(1, 5.gigabytes - 1).inclusive } + it { expect(subject.aws_access_key).to be_a String } + it { expect(subject.aws_secret_access_key).to be_a String } + it { expect(subject.aws_public_bucket_name).to be_a String } + end +end \ No newline at end of file diff --git a/spec/controllers/themes_controller_spec.rb b/spec/controllers/themes_controller_spec.rb index 8eb0aa7..51b9968 100644 --- a/spec/controllers/themes_controller_spec.rb +++ b/spec/controllers/themes_controller_spec.rb @@ -25,16 +25,35 @@ describe "GET #create_completed (redirect from AWS)" do context "with valid parameters" do + {"_qwerty.zip" => "qwerty", "1234_qwerty.zip" => "qwerty", "12345_67890_qwerty.zip" => "67890_qwerty"}.each do |key, theme_name| + context "when parsing key: #{key}" do + subject { get :create_completed, key: key } + it { is_expected.to redirect_to action: :index } + it_behaves_like "creating Theme in DB", key, theme_name + end + end + end + + context "when invalid parameters" do let(:theme_name) { "#{Faker::Lorem.sentence}zip" } let(:key) { "_#{theme_name}" } subject { get :create_completed, key: key } - it { is_expected.to redirect_to action: :index } - end - {"_qwerty" => "qwerty", "1234_qwerty" => "qwerty", "12345_67890_qwerty" => "67890_qwerty"}.each do |key, theme_name| - describe "when parsing key: #{key}" do - it_behaves_like "creating Theme in DB", key, theme_name + context "when wrong file extension (not .zip)" do + let(:theme_name) { "#{Faker::Lorem.sentence}" } + it { is_expected.to redirect_to action: :new } + end + + context "when theme name blank" do + let(:theme_name) { '' } + it { is_expected.to redirect_to action: :new } + end + + context "when key missing" do end end + + context "when theme already exists" do + end end end \ No newline at end of file diff --git a/spec/helpers/themes_helper_spec.rb b/spec/helpers/themes_helper_spec.rb new file mode 100644 index 0000000..9ebf2e3 --- /dev/null +++ b/spec/helpers/themes_helper_spec.rb @@ -0,0 +1,53 @@ +require 'rails_helper' + +RSpec.describe ThemesHelper, type: :helper do + describe "#theme_upload_params" do + let(:redirect_url) {"http://test.host/themes/create_completed" } + let (:policy) { + timestamp = Time.now + allow(Time).to receive(:now).and_return(timestamp) + conditions = [[:eq, :$bucket, Rails.application.config.aws_public_bucket_name], + [:eq, :$acl, :private], + [:"content-length-range", 1, Rails.application.config.aws_max_theme_zip_file_length], + [:"starts-with", :$key, "#{ThemesHelper::UPLOADS_FOLDER}"], + [:eq, :$success_action_redirect, redirect_url]] + policy = { conditions: conditions, expiration: (Time.now + 10.hours).utc.iso8601 } + policy = Base64.strict_encode64(policy.to_json) } + + subject {helper.theme_upload_params} + + it {is_expected.to be_a Hash} + + it "has form_action key with value" do + expect(subject[:form_action]).to eq "https://#{Rails.application.config.aws_public_bucket_name}.s3.amazonaws.com/" + end + it "has form_method key with value" do + expect(subject[:form_method]).to eq "post" + end + it "has form_enclosure_type key with value" do + expect(subject[:form_enclosure_type]).to eq "multipart/form-data" + end + it "has acl key with value" do + expect(subject[:acl]).to eq "private" + end + it "has access_key key with value" do + expect(subject[:access_key]).to eq Rails.application.config.aws_access_key + end + it "has redirect_url key with value" do + expect(subject[:redirect_url]).to eq redirect_url + end + it "has file_id key with value" do + uuid= SecureRandom.uuid + allow(SecureRandom).to receive(:uuid).and_return(uuid) + expect(subject[:file_id]).to eq "#{ThemesHelper::UPLOADS_FOLDER}#{SecureRandom.uuid}_${filename}" + end + it "has policy key with value" do + expect(subject[:policy]).to eq policy + end + + it "has signature key with value" do + expect(subject[:signature]).to eq Base64.strict_encode64(OpenSSL::HMAC.digest(OpenSSL::Digest.new('sha1'), + Rails.application.config.aws_secret_access_key, policy)) + end + end +end \ No newline at end of file From 9f87632a48d4568a306e5b8683d00a91b3018199 Mon Sep 17 00:00:00 2001 From: Olga Borovaya Date: Tue, 14 Jul 2015 11:56:26 +0300 Subject: [PATCH 23/37] Updated Role: added theme permissions, updated themes_controller_spec and themes_controller --- app/controllers/themes_controller.rb | 12 ++- app/models/ability.rb | 4 + config/routes.rb | 2 +- db/migrate/20150615091157_create_roles.rb | 2 + db/schema.rb | 4 +- spec/controllers/themes_controller_spec.rb | 112 +++++++++++++++------ spec/factories/roles.rb | 10 ++ spec/models/role_spec.rb | 2 +- 8 files changed, 111 insertions(+), 37 deletions(-) diff --git a/app/controllers/themes_controller.rb b/app/controllers/themes_controller.rb index 4dcc62d..58e2ff0 100644 --- a/app/controllers/themes_controller.rb +++ b/app/controllers/themes_controller.rb @@ -1,9 +1,12 @@ class ThemesController < ApplicationController - # GET#index + before_action :authenticate_user! + load_and_authorize_resource + + #GET#index def index end - # GET#new + #GET#new def new end @@ -17,4 +20,9 @@ def create_completed Theme.create! name: theme_name, zip_file_url: key, status: :processing redirect_to action: :index end + + #DELETE#destroy + def destroy + end + end diff --git a/app/models/ability.rb b/app/models/ability.rb index a755be1..de6e7b6 100644 --- a/app/models/ability.rb +++ b/app/models/ability.rb @@ -10,5 +10,9 @@ def initialize(user) can :destroy, User if user.role.can_delete_users? can :update_users_role, User if user.role.can_update_users_role? can :update_users_password, User if user.role.can_update_users_password? + can :read, Theme if user.role.can_create_themes? || user.role.can_delete_themes? + can :create, Theme if user.role.can_create_themes? + can :create_completed, Theme if user.role.can_create_themes? + can :destroy, Theme if user.role.can_delete_themes? end end \ No newline at end of file diff --git a/config/routes.rb b/config/routes.rb index eb40544..425c23d 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -13,7 +13,7 @@ # Put here routes to tenant-resource only constraints ->(request) { request.subdomain.present? } do get 'accounts/login_with_token' => 'accounts#login_with_token' - resources :themes, only: [:index, :new] + resources :themes, only: [:index, :new, :destroy] get 'themes/create_completed' => "themes#create_completed" #redirect from AWS devise_for :users, skip: :registrations resources :users, except: :show diff --git a/db/migrate/20150615091157_create_roles.rb b/db/migrate/20150615091157_create_roles.rb index ba0ab2c..fdc768a 100644 --- a/db/migrate/20150615091157_create_roles.rb +++ b/db/migrate/20150615091157_create_roles.rb @@ -6,6 +6,8 @@ def change t.boolean :can_update_users_password, null: false, default: false t.boolean :can_update_users_role, null: false, default: false t.boolean :can_delete_users, null: false, default: false + t.boolean :can_create_themes, null: false, default: false + t.boolean :can_delete_themes, null: false, default: false t.timestamps null: false end diff --git a/db/schema.rb b/db/schema.rb index ab34f19..87a94b2 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -11,7 +11,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 20150624130545) do +ActiveRecord::Schema.define(version: 20150624111841) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -32,6 +32,8 @@ t.boolean "can_update_users_password", default: false, null: false t.boolean "can_update_users_role", default: false, null: false t.boolean "can_delete_users", default: false, null: false + t.boolean "can_create_themes", default: false, null: false + t.boolean "can_delete_themes", default: false, null: false t.datetime "created_at", null: false t.datetime "updated_at", null: false end diff --git a/spec/controllers/themes_controller_spec.rb b/spec/controllers/themes_controller_spec.rb index 51b9968..e6c00e1 100644 --- a/spec/controllers/themes_controller_spec.rb +++ b/spec/controllers/themes_controller_spec.rb @@ -11,49 +11,97 @@ end RSpec.describe ThemesController, type: :controller do - describe "GET #index" do - subject { get :index } - it { is_expected.to have_http_status(:ok) } - it { is_expected.to render_template(:index) } + let(:account) { create :account } + let(:user_under_test) { create(:user) } + before(:each) do + Apartment::Tenant.switch! account.subdomain + user_under_test.update role: create(:manage_themes_role) + sign_in user_under_test end - describe "GET #new" do - subject { get :new } - it { is_expected.to have_http_status(:ok) } - it { is_expected.to render_template(:new) } + describe "when user not logged in" do + before(:each) { sign_out user_under_test } + subject { response } + let(:params) { {} } + + { index: :get, new: :get, create_completed: :get, destroy: :delete }.each do |action, method| + describe "#{method}##{action}" do + let(:params) { {id: create(:theme).id} } if action == :destroy + before(:each) { send(method, action, params) } + it { is_expected.to have_http_status(:found) } + it { is_expected.to redirect_to(new_user_session_path) } + end + end end - describe "GET #create_completed (redirect from AWS)" do - context "with valid parameters" do - {"_qwerty.zip" => "qwerty", "1234_qwerty.zip" => "qwerty", "12345_67890_qwerty.zip" => "67890_qwerty"}.each do |key, theme_name| - context "when parsing key: #{key}" do - subject { get :create_completed, key: key } - it { is_expected.to redirect_to action: :index } - it_behaves_like "creating Theme in DB", key, theme_name + describe "when user logged in" do + let!(:theme_count) { Theme.count } + subject { response } + %w{ index new }.each do |action| + describe "GET##{action}" do + context "when user does not have permission" do + before(:each) do + case action + when 'index' then user_under_test.update role: create(:role) + when 'new' then user_under_test.role.update can_create_themes: false + end + get action + end + it { is_expected.to redirect_to(root_path) } + end + context "when user has permission" do + before(:each) do + user_under_test.update role: create(:create_themes_role) + get action + end + it { is_expected.to have_http_status(:ok) } + it { is_expected.to render_template(action) } end end end - context "when invalid parameters" do - let(:theme_name) { "#{Faker::Lorem.sentence}zip" } - let(:key) { "_#{theme_name}" } - subject { get :create_completed, key: key } - - context "when wrong file extension (not .zip)" do - let(:theme_name) { "#{Faker::Lorem.sentence}" } - it { is_expected.to redirect_to action: :new } + describe "GET#create_completed (redirect from AWS)" do + context "when user does not have permission" do + let(:key) { "1234_qwerty.zip" } + before(:each) do + user_under_test.role.update can_create_themes: false + get :create_completed, key: key + end + it { is_expected.to redirect_to(root_path) } end - context "when theme name blank" do - let(:theme_name) { '' } - it { is_expected.to redirect_to action: :new } - end + context "when user has permission" do + context "with valid parameters" do + {"_qwerty.zip" => "qwerty", "1234_qwerty.zip" => "qwerty", "12345_67890_qwerty.zip" => "67890_qwerty"}.each do |key, theme_name| + context "when parsing key: #{key}" do + subject { get :create_completed, key: key } + it { is_expected.to redirect_to action: :index } + it_behaves_like "creating Theme in DB", key, theme_name + end + end + context "when theme already exists" do + end + end - context "when key missing" do - end - end + context "when invalid parameters" do + let(:theme_name) { "#{Faker::Lorem.sentence}zip" } + let(:key) { "_#{theme_name}" } + subject { get :create_completed, key: key } + + context "when wrong file extension (not .zip)" do + let(:theme_name) { "#{Faker::Lorem.sentence}" } + it { is_expected.to redirect_to action: :new } + end - context "when theme already exists" do + context "when theme name blank" do + let(:theme_name) { '' } + it { is_expected.to redirect_to action: :new } + end + + context "when key missing" do + end + end + end end end -end \ No newline at end of file +end diff --git a/spec/factories/roles.rb b/spec/factories/roles.rb index 5ad8874..d6b56f3 100644 --- a/spec/factories/roles.rb +++ b/spec/factories/roles.rb @@ -31,6 +31,16 @@ can_update_users_role true can_delete_users true end + factory :create_themes_role do + can_create_themes true + end + factory :delete_themes_role do + can_delete_themes true + + factory :manage_themes_role do + can_create_themes true + end + end end factory :account_owner_role, parent: :manage_users_role do diff --git a/spec/models/role_spec.rb b/spec/models/role_spec.rb index a5a3d76..09bfe1c 100644 --- a/spec/models/role_spec.rb +++ b/spec/models/role_spec.rb @@ -8,7 +8,7 @@ it {expect(build(:role)).to validate_uniqueness_of(:name)} it {is_expected.to have_db_column(:name).with_options null: false, limit: Role::NAME_LIMIT_MAX } - %w{ can_create_users can_update_users_password can_update_users_role can_delete_users }.each do |column_name| + %w{ can_create_users can_update_users_password can_update_users_role can_delete_users can_create_themes can_delete_themes }.each do |column_name| it {is_expected.to have_db_column(column_name).with_options null: false, default: false } end end From 2b09f6be6dcfc96a3113a38f9ec8b4be334d787b Mon Sep 17 00:00:00 2001 From: Alexander Ivanov Date: Tue, 14 Jul 2015 17:19:56 +0300 Subject: [PATCH 24/37] Updated Theme to validate filename extension --- app/controllers/themes_controller.rb | 13 +++++++------ app/models/theme.rb | 2 +- spec/controllers/themes_controller_spec.rb | 17 +++++++++-------- spec/factories/themes.rb | 2 +- spec/models/theme_spec.rb | 7 ++++++- 5 files changed, 24 insertions(+), 17 deletions(-) diff --git a/app/controllers/themes_controller.rb b/app/controllers/themes_controller.rb index 58e2ff0..d5d0d14 100644 --- a/app/controllers/themes_controller.rb +++ b/app/controllers/themes_controller.rb @@ -12,17 +12,18 @@ def new # GET#create_completed redirect from AWS def create_completed - key = params[:key] + key = params.require(:key) theme_file_name = key.sub(/[^_]*_/, '') # theme zip file name (theme name) is located after the first _ (underscore) theme_name = File.basename theme_file_name, File.extname(theme_file_name) - return redirect_to action: :new unless theme_name.present? && File.extname(theme_file_name).downcase.eql?('.zip') - - Theme.create! name: theme_name, zip_file_url: key, status: :processing - redirect_to action: :index + theme = Theme.create name: theme_name, zip_file_url: key, status: :processing + if theme.valid? + redirect_to action: :index + else + redirect_to action: :new + end end #DELETE#destroy def destroy end - end diff --git a/app/models/theme.rb b/app/models/theme.rb index 364ad97..883edb3 100644 --- a/app/models/theme.rb +++ b/app/models/theme.rb @@ -5,6 +5,6 @@ class Theme < ActiveRecord::Base enum status: { processing: 0 } validates :name, presence: true, length: { maximum: NAME_LIMIT }, uniqueness: { case_sensitive: false } - validates :zip_file_url, presence: true, length: { maximum: ZIP_FILE_URL_LIMIT }, uniqueness: { case_sensitive: false } + validates :zip_file_url, presence: true, format: {with: /\.(zip)\z/i }, length: { maximum: ZIP_FILE_URL_LIMIT }, uniqueness: { case_sensitive: false } validates :status, presence: true end \ No newline at end of file diff --git a/spec/controllers/themes_controller_spec.rb b/spec/controllers/themes_controller_spec.rb index e6c00e1..509ce0f 100644 --- a/spec/controllers/themes_controller_spec.rb +++ b/spec/controllers/themes_controller_spec.rb @@ -69,7 +69,6 @@ end it { is_expected.to redirect_to(root_path) } end - context "when user has permission" do context "with valid parameters" do {"_qwerty.zip" => "qwerty", "1234_qwerty.zip" => "qwerty", "12345_67890_qwerty.zip" => "67890_qwerty"}.each do |key, theme_name| @@ -80,25 +79,27 @@ end end context "when theme already exists" do + let(:existing_theme) { create(:theme) } + subject { get :create_completed, key: "#{SecureRandom.uuid}_#{existing_theme.name}.zip" } + it { is_expected.to redirect_to action: :new } end end - context "when invalid parameters" do - let(:theme_name) { "#{Faker::Lorem.sentence}zip" } + let(:theme_name) { "#{Faker::Internet.url}.zip" } let(:key) { "_#{theme_name}" } subject { get :create_completed, key: key } - context "when wrong file extension (not .zip)" do - let(:theme_name) { "#{Faker::Lorem.sentence}" } + let(:theme_name) { "#{Faker::Internet.url}.rar" } it { is_expected.to redirect_to action: :new } end - context "when theme name blank" do let(:theme_name) { '' } it { is_expected.to redirect_to action: :new } end - - context "when key missing" do + context "when params missing" do + it "raises exception" do + expect { get :create_completed }.to raise_error(ActionController::ParameterMissing) + end end end end diff --git a/spec/factories/themes.rb b/spec/factories/themes.rb index 2c49ee1..4a57f3d 100644 --- a/spec/factories/themes.rb +++ b/spec/factories/themes.rb @@ -1,7 +1,7 @@ FactoryGirl.define do factory :theme do name { Faker::Internet.slug } - zip_file_url { Faker::Internet.url } + zip_file_url "#{Faker::Internet.url}.zip" status { :processing } end end \ No newline at end of file diff --git a/spec/models/theme_spec.rb b/spec/models/theme_spec.rb index c6b78b3..22addb2 100644 --- a/spec/models/theme_spec.rb +++ b/spec/models/theme_spec.rb @@ -11,7 +11,12 @@ it {expect(build(:theme)).to validate_uniqueness_of(:zip_file_url).case_insensitive} it {is_expected.to validate_length_of(:zip_file_url).is_at_most Theme::ZIP_FILE_URL_LIMIT } it {is_expected.to have_db_column(:zip_file_url).with_options limit: Theme::ZIP_FILE_URL_LIMIT, null: false } - + %w{filename.zip filename.ZIP filename.ZiP}.each do |filename| + it {is_expected.to allow_value(filename).for(:zip_file_url) } + end + %w{filename filename. filename.z filename.zi filename.abc filename.ziip}.each do |filename| + it {is_expected.to_not allow_value(filename).for(:zip_file_url) } + end it {is_expected.to validate_presence_of :status} it {is_expected.to have_db_column(:status).with_options null: false, default: Theme.statuses[:processing]} end From 01e0927c73b8c4b6a59e97c2317d266618de63bd Mon Sep 17 00:00:00 2001 From: Olga Borovaya Date: Tue, 14 Jul 2015 18:06:51 +0300 Subject: [PATCH 25/37] themes_controller:DELETE#destroy and themes_controller_spec: DELETE#destroy --- app/controllers/themes_controller.rb | 3 ++ spec/controllers/themes_controller_spec.rb | 36 ++++++++++++++++++++++ 2 files changed, 39 insertions(+) diff --git a/app/controllers/themes_controller.rb b/app/controllers/themes_controller.rb index d5d0d14..b78f475 100644 --- a/app/controllers/themes_controller.rb +++ b/app/controllers/themes_controller.rb @@ -8,6 +8,7 @@ def index #GET#new def new + @theme = Theme.new end # GET#create_completed redirect from AWS @@ -25,5 +26,7 @@ def create_completed #DELETE#destroy def destroy + @theme.destroy + redirect_to themes_path end end diff --git a/spec/controllers/themes_controller_spec.rb b/spec/controllers/themes_controller_spec.rb index 509ce0f..2ff6d18 100644 --- a/spec/controllers/themes_controller_spec.rb +++ b/spec/controllers/themes_controller_spec.rb @@ -104,5 +104,41 @@ end end end + + describe "DELETE#destroy" do + let(:theme) { create(:theme) } + let(:id) { theme.id } + context "when user does not have permission" do + before(:each) do + user_under_test.role.update can_delete_themes: false + delete :destroy, id: id + end + it { is_expected.to redirect_to(root_path) } + end + + context "when user has permission" do + context "when id wrong" do + let(:id) { theme.id+100 } + subject { delete :destroy, id: id } + it "raises exception" do + expect {subject}.to raise_error(ActiveRecord::RecordNotFound) + end + end + context "when successful" do + before(:each) do + id = theme.id + @count = Theme.count + delete :destroy, id: id + end + it "deletes theme" do + expect{theme.reload}.to raise_error(ActiveRecord::RecordNotFound) + end + it "Theme.count decreased by 1" do + expect(Theme.count).to eq(@count-1) + end + it { is_expected.to redirect_to(themes_path) } + end + end + end end end From ae26997bffb1ee304d036c847ae3226f08115fba Mon Sep 17 00:00:00 2001 From: Alexander Ivanov Date: Wed, 15 Jul 2015 13:11:21 +0300 Subject: [PATCH 26/37] DRY-ed themes_helper --- app/helpers/themes_helper.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/helpers/themes_helper.rb b/app/helpers/themes_helper.rb index 5dbcdb5..20a1ac5 100644 --- a/app/helpers/themes_helper.rb +++ b/app/helpers/themes_helper.rb @@ -2,7 +2,7 @@ module ThemesHelper UPLOADS_FOLDER = 'uploads/' def theme_upload_params - return @theme_upload_params if @theme_upload_params.present? + return @theme_upload_params if @theme_upload_params bucket_name = Rails.application.config.aws_public_bucket_name redirect_url = request.protocol + request.host_with_port + '/themes/create_completed' From 8d4e1e1fc1bea9fe9dfaeb92b82f0f930f1d753b Mon Sep 17 00:00:00 2001 From: Alexander Ivanov Date: Thu, 16 Jul 2015 19:02:20 +0300 Subject: [PATCH 27/37] Added transfer_theme_job --- Gemfile | 4 +++ Gemfile.lock | 13 +++++++++ app/controllers/themes_controller.rb | 2 ++ app/jobs/transfer_theme_job.rb | 9 +++++++ app/libs/amazon_aws_client.rb | 18 +++++++++++++ app/models/theme.rb | 2 +- config/application.rb | 2 ++ config/environments/test.rb | 2 ++ spec/config/aws_spec.rb | 2 ++ spec/controllers/themes_controller_spec.rb | 29 ++++++++++++++++---- spec/jobs/transfer_theme_job_spec.rb | 31 ++++++++++++++++++++++ spec/spec_helper.rb | 6 ++++- 12 files changed, 113 insertions(+), 7 deletions(-) create mode 100644 app/jobs/transfer_theme_job.rb create mode 100644 app/libs/amazon_aws_client.rb create mode 100644 spec/jobs/transfer_theme_job_spec.rb diff --git a/Gemfile b/Gemfile index 764b443..747b0b1 100644 --- a/Gemfile +++ b/Gemfile @@ -48,6 +48,9 @@ gem 'slim-rails', '~> 3.0.1' # Shim to load environment variables from .env into ENV in development. gem 'dotenv-rails', '~> 2.0.2', :groups => [:development, :test] +# Official Amazon AWS SDK +gem 'aws-sdk', '~> 2.1.7' + group :development, :test do # Call 'byebug' anywhere in the code to stop execution and get a debugger console gem 'byebug' @@ -70,4 +73,5 @@ group :test do gem 'factory_girl_rails', '~> 4.0' gem 'shoulda-matchers', '~> 2.8.0' gem 'faker', '~> 1.4.3' + gem 'rspec-activejob', '~> 0.4.1' end diff --git a/Gemfile.lock b/Gemfile.lock index 02bed3b..1a39ce0 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -40,6 +40,12 @@ GEM activerecord (>= 3.1.2, < 5.0) rack (>= 1.3.6) arel (6.0.0) + aws-sdk (2.1.7) + aws-sdk-resources (= 2.1.7) + aws-sdk-core (2.1.7) + jmespath (~> 1.0) + aws-sdk-resources (2.1.7) + aws-sdk-core (= 2.1.7) bcrypt (3.1.10) binding_of_caller (0.7.2) debug_inspector (>= 0.0.1) @@ -104,6 +110,8 @@ GEM jbuilder (2.3.0) activesupport (>= 3.0.0, < 5) multi_json (~> 1.2) + jmespath (1.0.2) + multi_json (~> 1.0) jquery-rails (4.0.3) rails-dom-testing (~> 1.0) railties (>= 4.2.0) @@ -153,6 +161,9 @@ GEM rdoc (4.2.0) responders (2.1.0) railties (>= 4.2.0, < 5) + rspec-activejob (0.4.1) + activejob (>= 4.2) + rspec-mocks rspec-core (3.2.3) rspec-support (~> 3.2.0) rspec-expectations (3.2.1) @@ -227,6 +238,7 @@ PLATFORMS DEPENDENCIES apartment (~> 1.0.1) + aws-sdk (~> 2.1.7) byebug cancancan (~> 1.12.0) capybara (~> 2.4.4) @@ -242,6 +254,7 @@ DEPENDENCIES jquery-rails pg (~> 0.18.2) rails (= 4.2.1) + rspec-activejob (~> 0.4.1) rspec-expectations (~> 3.2.1) rspec-its (~> 1.2.0) rspec-rails (~> 3.2.3) diff --git a/app/controllers/themes_controller.rb b/app/controllers/themes_controller.rb index b78f475..a0b4a07 100644 --- a/app/controllers/themes_controller.rb +++ b/app/controllers/themes_controller.rb @@ -18,8 +18,10 @@ def create_completed theme_name = File.basename theme_file_name, File.extname(theme_file_name) theme = Theme.create name: theme_name, zip_file_url: key, status: :processing if theme.valid? + TransferThemeJob.perform_later theme redirect_to action: :index else + AmazonAwsClient.delete_from_public_bucket key redirect_to action: :new end end diff --git a/app/jobs/transfer_theme_job.rb b/app/jobs/transfer_theme_job.rb new file mode 100644 index 0000000..16fd063 --- /dev/null +++ b/app/jobs/transfer_theme_job.rb @@ -0,0 +1,9 @@ +class TransferThemeJob < ActiveJob::Base + queue_as :default + + def perform theme + # TODO: consider Aws::S3::Errors:... handling and retry job if error is recoverable (e.g. network problem) + AmazonAwsClient.transfer_from_public_to_private_bucket theme.zip_file_url + theme.update! status: :uploaded + end +end diff --git a/app/libs/amazon_aws_client.rb b/app/libs/amazon_aws_client.rb new file mode 100644 index 0000000..be3fba0 --- /dev/null +++ b/app/libs/amazon_aws_client.rb @@ -0,0 +1,18 @@ +module AmazonAwsClient + def self.client + credentials = Aws::Credentials.new Rails.application.config.aws_access_key, Rails.application.config.aws_secret_access_key + Aws::S3::Client.new region: Rails.application.config.aws_region, credentials: credentials + end + + def self.transfer_from_public_to_private_bucket key + client = self.client + client.copy_object(bucket: Rails.application.config.aws_private_bucket_name, + key: key, + copy_source: Rails.application.config.aws_public_bucket_name + '/' + key) + client.delete_object bucket: Rails.application.config.aws_public_bucket_name, key: key + end + + def self.delete_from_public_bucket key + self.client.delete_object bucket: Rails.application.config.aws_public_bucket_name, key: key + end +end diff --git a/app/models/theme.rb b/app/models/theme.rb index 883edb3..05b2b84 100644 --- a/app/models/theme.rb +++ b/app/models/theme.rb @@ -2,7 +2,7 @@ class Theme < ActiveRecord::Base NAME_LIMIT = 100 ZIP_FILE_URL_LIMIT = 2000 - enum status: { processing: 0 } + enum status: { processing: 0, uploaded: 1 } validates :name, presence: true, length: { maximum: NAME_LIMIT }, uniqueness: { case_sensitive: false } validates :zip_file_url, presence: true, format: {with: /\.(zip)\z/i }, length: { maximum: ZIP_FILE_URL_LIMIT }, uniqueness: { case_sensitive: false } diff --git a/config/application.rb b/config/application.rb index ba19aac..fb4b56c 100644 --- a/config/application.rb +++ b/config/application.rb @@ -51,5 +51,7 @@ class Application < Rails::Application config.aws_access_key = ENV["AWS_ACCESS_KEY"] config.aws_secret_access_key = ENV["AWS_SECRET_ACCESS_KEY"] config.aws_public_bucket_name = "flexcommerce-uploadedthemes" + config.aws_private_bucket_name = "flexcommerce-productionthemes" + config.aws_region = "us-east-1" end end diff --git a/config/environments/test.rb b/config/environments/test.rb index 513e8c6..f1b7e8e 100644 --- a/config/environments/test.rb +++ b/config/environments/test.rb @@ -40,4 +40,6 @@ # Raises error for missing translations # config.action_view.raise_on_missing_translations = true + # For rspec-activejob gem + config.active_job.queue_adapter = :test end diff --git a/spec/config/aws_spec.rb b/spec/config/aws_spec.rb index f588712..2fb1ad2 100644 --- a/spec/config/aws_spec.rb +++ b/spec/config/aws_spec.rb @@ -6,5 +6,7 @@ it { expect(subject.aws_access_key).to be_a String } it { expect(subject.aws_secret_access_key).to be_a String } it { expect(subject.aws_public_bucket_name).to be_a String } + it { expect(subject.aws_private_bucket_name).to be_a String } + it { expect(subject.aws_region).to be_a String } end end \ No newline at end of file diff --git a/spec/controllers/themes_controller_spec.rb b/spec/controllers/themes_controller_spec.rb index 2ff6d18..7c74cff 100644 --- a/spec/controllers/themes_controller_spec.rb +++ b/spec/controllers/themes_controller_spec.rb @@ -10,6 +10,21 @@ end end +RSpec.shared_examples "uncussessfull create_completed" do + it "deletes theme from public bucket" do + expect(AmazonAwsClient).to have_received(:delete_from_public_bucket) + end + it { is_expected.to redirect_to action: :new } +end + +RSpec.shared_context "context for uncussessfull create_completed" do + before(:each) do + allow(AmazonAwsClient).to receive(:delete_from_public_bucket) + get :create_completed, key: key + end + subject { response } +end + RSpec.describe ThemesController, type: :controller do let(:account) { create :account } let(:user_under_test) { create(:user) } @@ -74,14 +89,16 @@ {"_qwerty.zip" => "qwerty", "1234_qwerty.zip" => "qwerty", "12345_67890_qwerty.zip" => "67890_qwerty"}.each do |key, theme_name| context "when parsing key: #{key}" do subject { get :create_completed, key: key } - it { is_expected.to redirect_to action: :index } it_behaves_like "creating Theme in DB", key, theme_name + it { expect { subject }.to enqueue_a(TransferThemeJob).with(global_id(Theme)) } + it { is_expected.to redirect_to action: :index } end end context "when theme already exists" do let(:existing_theme) { create(:theme) } - subject { get :create_completed, key: "#{SecureRandom.uuid}_#{existing_theme.name}.zip" } - it { is_expected.to redirect_to action: :new } + let(:key) {"#{SecureRandom.uuid}_#{existing_theme.name}.zip"} + include_context "context for uncussessfull create_completed" + it_behaves_like "uncussessfull create_completed" end end context "when invalid parameters" do @@ -90,11 +107,13 @@ subject { get :create_completed, key: key } context "when wrong file extension (not .zip)" do let(:theme_name) { "#{Faker::Internet.url}.rar" } - it { is_expected.to redirect_to action: :new } + include_context "context for uncussessfull create_completed" + it_behaves_like "uncussessfull create_completed" end context "when theme name blank" do let(:theme_name) { '' } - it { is_expected.to redirect_to action: :new } + include_context "context for uncussessfull create_completed" + it_behaves_like "uncussessfull create_completed" end context "when params missing" do it "raises exception" do diff --git a/spec/jobs/transfer_theme_job_spec.rb b/spec/jobs/transfer_theme_job_spec.rb new file mode 100644 index 0000000..736a2ee --- /dev/null +++ b/spec/jobs/transfer_theme_job_spec.rb @@ -0,0 +1,31 @@ +require 'rails_helper' + +RSpec.describe TransferThemeJob, type: :job do + include ActiveJob::TestHelper + + let(:theme) { create(:theme) } + subject(:job) { described_class.perform_later(theme) } + + describe "when queued" do + + it "queues the job" do + expect { job }.to change(ActiveJob::Base.queue_adapter.enqueued_jobs, :size).by(1) + end + + it "is in default queue" do + expect(TransferThemeJob.new.queue_name).to eq('default') + end + end + + describe "when executes perform" do + it "transfer theme from public to private bucket" do + expect(AmazonAwsClient).to receive(:transfer_from_public_to_private_bucket).with(theme.zip_file_url) + perform_enqueued_jobs { job } + end + it "updates theme status" do + allow(AmazonAwsClient).to receive(:transfer_from_public_to_private_bucket) + perform_enqueued_jobs { job } + expect(theme.reload.uploaded?).to be_truthy + end + end +end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 5009e7a..df8eaa6 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -1,4 +1,5 @@ require 'factory_girl' +require 'rspec/active_job' # This file was generated by the `rails generate rspec:install` command. Conventionally, all # specs live under a `spec` directory, which RSpec adds to the `$LOAD_PATH`. @@ -19,6 +20,8 @@ # # See http://rubydoc.info/gems/rspec-core/RSpec/Core/Configuration RSpec.configure do |config| + config.include(RSpec::ActiveJob) + # rspec-expectations config goes here. You can use an alternate # assertion/expectation library such as wrong or the stdlib/minitest # assertions if you prefer. @@ -109,6 +112,8 @@ config.before(:each) do Account.destroy_all Apartment::Tenant.reset + ActiveJob::Base.queue_adapter.enqueued_jobs = [] + ActiveJob::Base.queue_adapter.performed_jobs = [] end config.around(:each) do |example| @@ -116,5 +121,4 @@ example.run end end - end From a4a916252be0f0e82fe6fd9706df457fcd2cac14 Mon Sep 17 00:00:00 2001 From: Alexander Ivanov Date: Fri, 17 Jul 2015 12:20:21 +0300 Subject: [PATCH 28/37] DRY-ed themes_controller --- app/controllers/themes_controller.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/controllers/themes_controller.rb b/app/controllers/themes_controller.rb index a0b4a07..fedd765 100644 --- a/app/controllers/themes_controller.rb +++ b/app/controllers/themes_controller.rb @@ -16,8 +16,8 @@ def create_completed key = params.require(:key) theme_file_name = key.sub(/[^_]*_/, '') # theme zip file name (theme name) is located after the first _ (underscore) theme_name = File.basename theme_file_name, File.extname(theme_file_name) - theme = Theme.create name: theme_name, zip_file_url: key, status: :processing - if theme.valid? + theme = Theme.new name: theme_name, zip_file_url: key, status: :processing + if theme.save TransferThemeJob.perform_later theme redirect_to action: :index else From 8edf83bbabb5fc832c2e95b0c3b1f6b7ab8cf7b1 Mon Sep 17 00:00:00 2001 From: Alexander Ivanov Date: Mon, 20 Jul 2015 15:19:13 +0300 Subject: [PATCH 29/37] Theme: added theme file management in AWS --- app/controllers/themes_controller.rb | 14 +++------ app/jobs/delete_theme_job.rb | 12 ++++++++ app/jobs/transfer_theme_job.rb | 2 +- app/libs/amazon_aws_client.rb | 4 +++ app/models/theme.rb | 22 ++++++++++++-- spec/controllers/themes_controller_spec.rb | 27 +++++------------ spec/jobs/delete_theme_job_spec.rb | 31 +++++++++++++++++++ spec/jobs/transfer_theme_job_spec.rb | 5 ++-- spec/models/theme_spec.rb | 35 +++++++++++++++++++++- 9 files changed, 115 insertions(+), 37 deletions(-) create mode 100644 app/jobs/delete_theme_job.rb create mode 100644 spec/jobs/delete_theme_job_spec.rb diff --git a/app/controllers/themes_controller.rb b/app/controllers/themes_controller.rb index fedd765..c39a115 100644 --- a/app/controllers/themes_controller.rb +++ b/app/controllers/themes_controller.rb @@ -2,11 +2,11 @@ class ThemesController < ApplicationController before_action :authenticate_user! load_and_authorize_resource - #GET#index + # GET#index def index end - #GET#new + # GET#new def new @theme = Theme.new end @@ -17,16 +17,10 @@ def create_completed theme_file_name = key.sub(/[^_]*_/, '') # theme zip file name (theme name) is located after the first _ (underscore) theme_name = File.basename theme_file_name, File.extname(theme_file_name) theme = Theme.new name: theme_name, zip_file_url: key, status: :processing - if theme.save - TransferThemeJob.perform_later theme - redirect_to action: :index - else - AmazonAwsClient.delete_from_public_bucket key - redirect_to action: :new - end + redirect_to action: (theme.save ? 'index' : 'new') end - #DELETE#destroy + # DELETE#destroy def destroy @theme.destroy redirect_to themes_path diff --git a/app/jobs/delete_theme_job.rb b/app/jobs/delete_theme_job.rb new file mode 100644 index 0000000..22214a2 --- /dev/null +++ b/app/jobs/delete_theme_job.rb @@ -0,0 +1,12 @@ +class DeleteThemeJob < ActiveJob::Base + queue_as :default + + def perform(zip_file_url, is_public_bucket) + # TODO: consider Aws::S3::Errors:... handling and retry job if error is recoverable (e.g. network problem) + if is_public_bucket + AmazonAwsClient.delete_from_public_bucket zip_file_url + else + AmazonAwsClient.delete_from_private_bucket zip_file_url + end + end +end diff --git a/app/jobs/transfer_theme_job.rb b/app/jobs/transfer_theme_job.rb index 16fd063..6ddf492 100644 --- a/app/jobs/transfer_theme_job.rb +++ b/app/jobs/transfer_theme_job.rb @@ -1,7 +1,7 @@ class TransferThemeJob < ActiveJob::Base queue_as :default - def perform theme + def perform(theme) # TODO: consider Aws::S3::Errors:... handling and retry job if error is recoverable (e.g. network problem) AmazonAwsClient.transfer_from_public_to_private_bucket theme.zip_file_url theme.update! status: :uploaded diff --git a/app/libs/amazon_aws_client.rb b/app/libs/amazon_aws_client.rb index be3fba0..92b9c25 100644 --- a/app/libs/amazon_aws_client.rb +++ b/app/libs/amazon_aws_client.rb @@ -15,4 +15,8 @@ def self.transfer_from_public_to_private_bucket key def self.delete_from_public_bucket key self.client.delete_object bucket: Rails.application.config.aws_public_bucket_name, key: key end + + def self.delete_from_private_bucket key + self.client.delete_object bucket: Rails.application.config.aws_private_bucket_name, key: key + end end diff --git a/app/models/theme.rb b/app/models/theme.rb index 05b2b84..5689d07 100644 --- a/app/models/theme.rb +++ b/app/models/theme.rb @@ -1,10 +1,28 @@ class Theme < ActiveRecord::Base + after_validation :schedule_theme_delete_if_errors, on: :create + before_destroy :schedule_theme_delete + after_create :schedule_theme_transfer + NAME_LIMIT = 100 ZIP_FILE_URL_LIMIT = 2000 enum status: { processing: 0, uploaded: 1 } validates :name, presence: true, length: { maximum: NAME_LIMIT }, uniqueness: { case_sensitive: false } - validates :zip_file_url, presence: true, format: {with: /\.(zip)\z/i }, length: { maximum: ZIP_FILE_URL_LIMIT }, uniqueness: { case_sensitive: false } + validates :zip_file_url, presence: true, format: { with: /\.(zip)\z/i }, length: { maximum: ZIP_FILE_URL_LIMIT }, uniqueness: { case_sensitive: false } validates :status, presence: true -end \ No newline at end of file + + private + + def schedule_theme_transfer + TransferThemeJob.perform_later(self) + end + + def schedule_theme_delete + DeleteThemeJob.perform_later(zip_file_url, processing? ? true : false) + end + + def schedule_theme_delete_if_errors + DeleteThemeJob.perform_later(zip_file_url, true) if errors.present? + end +end diff --git a/spec/controllers/themes_controller_spec.rb b/spec/controllers/themes_controller_spec.rb index 7c74cff..baefa31 100644 --- a/spec/controllers/themes_controller_spec.rb +++ b/spec/controllers/themes_controller_spec.rb @@ -7,24 +7,15 @@ it { is_expected.to be } its(:zip_file_url) { is_expected.to eq key } it { is_expected.to be_processing } + it { expect(Theme.count).to eq(1) } end end RSpec.shared_examples "uncussessfull create_completed" do - it "deletes theme from public bucket" do - expect(AmazonAwsClient).to have_received(:delete_from_public_bucket) - end + it { expect(Theme.count).to be_zero } it { is_expected.to redirect_to action: :new } end -RSpec.shared_context "context for uncussessfull create_completed" do - before(:each) do - allow(AmazonAwsClient).to receive(:delete_from_public_bucket) - get :create_completed, key: key - end - subject { response } -end - RSpec.describe ThemesController, type: :controller do let(:account) { create :account } let(:user_under_test) { create(:user) } @@ -50,7 +41,6 @@ end describe "when user logged in" do - let!(:theme_count) { Theme.count } subject { response } %w{ index new }.each do |action| describe "GET##{action}" do @@ -76,6 +66,7 @@ end describe "GET#create_completed (redirect from AWS)" do + let!(:theme_count){ Theme.count } context "when user does not have permission" do let(:key) { "1234_qwerty.zip" } before(:each) do @@ -85,34 +76,30 @@ it { is_expected.to redirect_to(root_path) } end context "when user has permission" do + let(:theme_name) { "#{Faker::Internet.url}.zip" } + let(:key) {"#{SecureRandom.uuid}_#{theme_name}"} + subject { get :create_completed, key: key } context "with valid parameters" do {"_qwerty.zip" => "qwerty", "1234_qwerty.zip" => "qwerty", "12345_67890_qwerty.zip" => "67890_qwerty"}.each do |key, theme_name| context "when parsing key: #{key}" do - subject { get :create_completed, key: key } it_behaves_like "creating Theme in DB", key, theme_name - it { expect { subject }.to enqueue_a(TransferThemeJob).with(global_id(Theme)) } it { is_expected.to redirect_to action: :index } end end context "when theme already exists" do let(:existing_theme) { create(:theme) } let(:key) {"#{SecureRandom.uuid}_#{existing_theme.name}.zip"} - include_context "context for uncussessfull create_completed" it_behaves_like "uncussessfull create_completed" end end context "when invalid parameters" do - let(:theme_name) { "#{Faker::Internet.url}.zip" } - let(:key) { "_#{theme_name}" } - subject { get :create_completed, key: key } context "when wrong file extension (not .zip)" do let(:theme_name) { "#{Faker::Internet.url}.rar" } - include_context "context for uncussessfull create_completed" + subject { get :create_completed, key: key } it_behaves_like "uncussessfull create_completed" end context "when theme name blank" do let(:theme_name) { '' } - include_context "context for uncussessfull create_completed" it_behaves_like "uncussessfull create_completed" end context "when params missing" do diff --git a/spec/jobs/delete_theme_job_spec.rb b/spec/jobs/delete_theme_job_spec.rb new file mode 100644 index 0000000..df0a687 --- /dev/null +++ b/spec/jobs/delete_theme_job_spec.rb @@ -0,0 +1,31 @@ +require 'rails_helper' + +RSpec.describe DeleteThemeJob, type: :job do + include ActiveJob::TestHelper + let(:zip_file_url) { 'some_zip_file_url' } + let(:is_public_bucket) { true } + subject(:job) { described_class.perform_later(zip_file_url, is_public_bucket) } + + describe "when queued" do + it "queues the job" do + expect { job }.to change(ActiveJob::Base.queue_adapter.enqueued_jobs, :size).by(1) + end + it "is in default queue" do + expect(DeleteThemeJob.new.queue_name).to eq('default') + end + end + + describe "when executes perform" do + it "deletes theme from public bucket" do + expect(AmazonAwsClient).to receive(:delete_from_public_bucket).with(zip_file_url) + perform_enqueued_jobs { job } + end + describe "when theme in private bucket" do + let(:is_public_bucket) { false } + it "deletes theme from private bucket" do + expect(AmazonAwsClient).to receive(:delete_from_private_bucket).with(zip_file_url) + perform_enqueued_jobs { job } + end + end + end +end \ No newline at end of file diff --git a/spec/jobs/transfer_theme_job_spec.rb b/spec/jobs/transfer_theme_job_spec.rb index 736a2ee..6a2c472 100644 --- a/spec/jobs/transfer_theme_job_spec.rb +++ b/spec/jobs/transfer_theme_job_spec.rb @@ -4,12 +4,10 @@ include ActiveJob::TestHelper let(:theme) { create(:theme) } - subject(:job) { described_class.perform_later(theme) } describe "when queued" do - it "queues the job" do - expect { job }.to change(ActiveJob::Base.queue_adapter.enqueued_jobs, :size).by(1) + expect { theme }.to change(ActiveJob::Base.queue_adapter.enqueued_jobs, :size).by(1) end it "is in default queue" do @@ -18,6 +16,7 @@ end describe "when executes perform" do + subject(:job) { described_class.perform_later(theme) } it "transfer theme from public to private bucket" do expect(AmazonAwsClient).to receive(:transfer_from_public_to_private_bucket).with(theme.zip_file_url) perform_enqueued_jobs { job } diff --git a/spec/models/theme_spec.rb b/spec/models/theme_spec.rb index 22addb2..0cff823 100644 --- a/spec/models/theme_spec.rb +++ b/spec/models/theme_spec.rb @@ -1,6 +1,9 @@ require 'rails_helper' RSpec.describe Theme, type: :model do + let(:theme) { create :theme} + subject { theme } + context "when validate" do it {is_expected.to validate_presence_of :name} it {expect(build(:theme)).to validate_uniqueness_of(:name).case_insensitive } @@ -20,4 +23,34 @@ it {is_expected.to validate_presence_of :status} it {is_expected.to have_db_column(:status).with_options null: false, default: Theme.statuses[:processing]} end -end \ No newline at end of file + + context "when create" do + context "when successfull" do + it "schedules transfer theme from public to private bucket" do + expect { subject }.to enqueue_a(TransferThemeJob).with(global_id(Theme)) + end + it "does not schedules deletion theme from public bucket" do + expect { subject }.to_not enqueue_a(DeleteThemeJob).with(String, TrueClass) + end + end + context "when unsuccessfull" do + let(:theme) { build :theme, zip_file_url: "wrong_url" } + it "schedules deletion theme from public bucket" do + expect { theme.save }.to enqueue_a(DeleteThemeJob).with(String, TrueClass) + end + it "schedules transfer theme from public to private bucket" do + expect { theme.save }.to_not enqueue_a(TransferThemeJob).with(global_id(Theme)) + end + end + end + + context "when destroy" do + it "schedules deletion theme from public bucket" do + expect { theme.destroy }.to enqueue_a(DeleteThemeJob).with(String, TrueClass) + end + it "schedules deletion theme from private bucket" do + theme.update status: :uploaded + expect { theme.destroy }.to enqueue_a(DeleteThemeJob).with(String, FalseClass) + end + end +end From cd005703c3234da1341249cd403847fa24f26f45 Mon Sep 17 00:00:00 2001 From: Alexander Ivanov Date: Mon, 20 Jul 2015 15:35:07 +0300 Subject: [PATCH 30/37] DRY-ed Theme --- app/models/theme.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/theme.rb b/app/models/theme.rb index 5689d07..4e37dee 100644 --- a/app/models/theme.rb +++ b/app/models/theme.rb @@ -19,7 +19,7 @@ def schedule_theme_transfer end def schedule_theme_delete - DeleteThemeJob.perform_later(zip_file_url, processing? ? true : false) + DeleteThemeJob.perform_later(zip_file_url, processing?) end def schedule_theme_delete_if_errors From 01689982621299c8c6f809ff3923342ed70b2b8b Mon Sep 17 00:00:00 2001 From: Alexander Ivanov Date: Mon, 20 Jul 2015 16:02:12 +0300 Subject: [PATCH 31/37] Fixed code review findings --- app/jobs/delete_theme_job.rb | 6 +----- app/libs/amazon_aws_client.rb | 10 ++++++++-- app/models/theme.rb | 6 +----- 3 files changed, 10 insertions(+), 12 deletions(-) diff --git a/app/jobs/delete_theme_job.rb b/app/jobs/delete_theme_job.rb index 22214a2..8cc6faf 100644 --- a/app/jobs/delete_theme_job.rb +++ b/app/jobs/delete_theme_job.rb @@ -3,10 +3,6 @@ class DeleteThemeJob < ActiveJob::Base def perform(zip_file_url, is_public_bucket) # TODO: consider Aws::S3::Errors:... handling and retry job if error is recoverable (e.g. network problem) - if is_public_bucket - AmazonAwsClient.delete_from_public_bucket zip_file_url - else - AmazonAwsClient.delete_from_private_bucket zip_file_url - end + AmazonAwsClient.try (is_public_bucket ? :delete_from_public_bucket : :delete_from_private_bucket), zip_file_url end end diff --git a/app/libs/amazon_aws_client.rb b/app/libs/amazon_aws_client.rb index 92b9c25..2537785 100644 --- a/app/libs/amazon_aws_client.rb +++ b/app/libs/amazon_aws_client.rb @@ -13,10 +13,16 @@ def self.transfer_from_public_to_private_bucket key end def self.delete_from_public_bucket key - self.client.delete_object bucket: Rails.application.config.aws_public_bucket_name, key: key + self.delete_from_bucket Rails.application.config.aws_public_bucket_name, key end def self.delete_from_private_bucket key - self.client.delete_object bucket: Rails.application.config.aws_private_bucket_name, key: key + self.delete_from_bucket Rails.application.config.aws_private_bucket_name, key + end + + private + + def self.delete_from_bucket bucket, key + self.client.delete_object bucket: bucket, key: key end end diff --git a/app/models/theme.rb b/app/models/theme.rb index 4e37dee..18fb3c2 100644 --- a/app/models/theme.rb +++ b/app/models/theme.rb @@ -1,5 +1,5 @@ class Theme < ActiveRecord::Base - after_validation :schedule_theme_delete_if_errors, on: :create + after_validation :schedule_theme_delete, on: :create, if: "errors.present?" before_destroy :schedule_theme_delete after_create :schedule_theme_transfer @@ -21,8 +21,4 @@ def schedule_theme_transfer def schedule_theme_delete DeleteThemeJob.perform_later(zip_file_url, processing?) end - - def schedule_theme_delete_if_errors - DeleteThemeJob.perform_later(zip_file_url, true) if errors.present? - end end From 14d2908c4b012e20302f28527fea2b539a4e7bca Mon Sep 17 00:00:00 2001 From: Alexander Ivanov Date: Tue, 21 Jul 2015 12:26:37 +0300 Subject: [PATCH 32/37] Added sidekiq backend, added retry_job for AWS connection error --- Gemfile | 3 +++ Gemfile.lock | 16 ++++++++++++++++ app/jobs/delete_theme_job.rb | 7 +++++-- app/jobs/transfer_theme_job.rb | 5 ++++- config/application.rb | 2 ++ spec/jobs/delete_theme_job_spec.rb | 13 ++++++++++--- spec/jobs/transfer_theme_job_spec.rb | 7 +++++++ 7 files changed, 47 insertions(+), 6 deletions(-) diff --git a/Gemfile b/Gemfile index 747b0b1..5460539 100644 --- a/Gemfile +++ b/Gemfile @@ -36,6 +36,9 @@ gem 'sdoc', '~> 0.4.0', group: :doc gem 'slim-rails', '~> 3.0.1' +# Simple, efficient background processing +gem 'sidekiq', '~> 3.4.2' + # Use ActiveModel has_secure_password # gem 'bcrypt', '~> 3.1.7' diff --git a/Gemfile.lock b/Gemfile.lock index 1a39ce0..3afba29 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -59,6 +59,8 @@ GEM rack (>= 1.0.0) rack-test (>= 0.5.4) xpath (~> 2.0) + celluloid (0.16.0) + timers (~> 4.0.0) coffee-rails (4.1.0) coffee-script (>= 2.2.0) railties (>= 4.0.0, < 5.0) @@ -67,6 +69,7 @@ GEM execjs coffee-script-source (1.9.1.1) columnize (0.9.0) + connection_pool (2.2.0) cucumber (1.3.19) builder (>= 2.1.2) diff-lcs (>= 1.1.3) @@ -106,6 +109,7 @@ GEM multi_json (~> 1.3) globalid (0.3.5) activesupport (>= 4.1.0) + hitimes (1.2.2) i18n (0.7.0) jbuilder (2.3.0) activesupport (>= 3.0.0, < 5) @@ -159,6 +163,9 @@ GEM thor (>= 0.18.1, < 2.0) rake (10.4.2) rdoc (4.2.0) + redis (3.2.1) + redis-namespace (1.5.2) + redis (~> 3.0, >= 3.0.4) responders (2.1.0) railties (>= 4.2.0, < 5) rspec-activejob (0.4.1) @@ -196,6 +203,12 @@ GEM rdoc (~> 4.0) shoulda-matchers (2.8.0) activesupport (>= 3.0.0) + sidekiq (3.4.2) + celluloid (~> 0.16.0) + connection_pool (~> 2.2, >= 2.2.0) + json (~> 1.0) + redis (~> 3.2, >= 3.2.1) + redis-namespace (~> 1.5, >= 1.5.2) slim (3.0.6) temple (~> 0.7.3) tilt (>= 1.3.3, < 2.1) @@ -216,6 +229,8 @@ GEM thor (0.19.1) thread_safe (0.3.5) tilt (1.4.1) + timers (4.0.1) + hitimes turbolinks (2.5.3) coffee-rails tzinfo (1.2.2) @@ -261,6 +276,7 @@ DEPENDENCIES sass-rails (~> 5.0) sdoc (~> 0.4.0) shoulda-matchers (~> 2.8.0) + sidekiq (~> 3.4.2) slim-rails (~> 3.0.1) spring turbolinks diff --git a/app/jobs/delete_theme_job.rb b/app/jobs/delete_theme_job.rb index 8cc6faf..1f372ba 100644 --- a/app/jobs/delete_theme_job.rb +++ b/app/jobs/delete_theme_job.rb @@ -1,8 +1,11 @@ class DeleteThemeJob < ActiveJob::Base - queue_as :default + queue_as :low_priority + + rescue_from(Aws::S3::Errors::RequestTimeout) do + retry_job wait: 5.minutes, queue: :low_priority + end def perform(zip_file_url, is_public_bucket) - # TODO: consider Aws::S3::Errors:... handling and retry job if error is recoverable (e.g. network problem) AmazonAwsClient.try (is_public_bucket ? :delete_from_public_bucket : :delete_from_private_bucket), zip_file_url end end diff --git a/app/jobs/transfer_theme_job.rb b/app/jobs/transfer_theme_job.rb index 6ddf492..29dd699 100644 --- a/app/jobs/transfer_theme_job.rb +++ b/app/jobs/transfer_theme_job.rb @@ -1,8 +1,11 @@ class TransferThemeJob < ActiveJob::Base queue_as :default + rescue_from(Aws::S3::Errors::RequestTimeout) do + retry_job wait: 5.minutes, queue: :default + end + def perform(theme) - # TODO: consider Aws::S3::Errors:... handling and retry job if error is recoverable (e.g. network problem) AmazonAwsClient.transfer_from_public_to_private_bucket theme.zip_file_url theme.update! status: :uploaded end diff --git a/config/application.rb b/config/application.rb index fb4b56c..c62ca08 100644 --- a/config/application.rb +++ b/config/application.rb @@ -32,6 +32,8 @@ class Application < Rails::Application # Do not swallow errors in after_commit/after_rollback callbacks. config.active_record.raise_in_transactional_callbacks = true + config.active_job.queue_adapter = :sidekiq + # ActionMailer config config.action_mailer.default_url_options = { :host => ENV["HOST"] } config.action_mailer.raise_delivery_errors = true diff --git a/spec/jobs/delete_theme_job_spec.rb b/spec/jobs/delete_theme_job_spec.rb index df0a687..3ff0965 100644 --- a/spec/jobs/delete_theme_job_spec.rb +++ b/spec/jobs/delete_theme_job_spec.rb @@ -10,8 +10,8 @@ it "queues the job" do expect { job }.to change(ActiveJob::Base.queue_adapter.enqueued_jobs, :size).by(1) end - it "is in default queue" do - expect(DeleteThemeJob.new.queue_name).to eq('default') + it "is in low_priority queue" do + expect(DeleteThemeJob.new.queue_name).to eq('low_priority') end end @@ -27,5 +27,12 @@ perform_enqueued_jobs { job } end end + it "handles request timeout error" do + allow(AmazonAwsClient).to receive(:delete_from_public_bucket).and_raise(Aws::S3::Errors::RequestTimeout.new('','')) + perform_enqueued_jobs do + expect_any_instance_of(DeleteThemeJob).to receive(:retry_job).with(wait: 5.minutes, queue: :low_priority) + job + end + end end -end \ No newline at end of file +end diff --git a/spec/jobs/transfer_theme_job_spec.rb b/spec/jobs/transfer_theme_job_spec.rb index 6a2c472..edd50fa 100644 --- a/spec/jobs/transfer_theme_job_spec.rb +++ b/spec/jobs/transfer_theme_job_spec.rb @@ -26,5 +26,12 @@ perform_enqueued_jobs { job } expect(theme.reload.uploaded?).to be_truthy end + it "handles request timeout error" do + allow(AmazonAwsClient).to receive(:transfer_from_public_to_private_bucket).and_raise(Aws::S3::Errors::RequestTimeout.new('','')) + perform_enqueued_jobs do + expect_any_instance_of(TransferThemeJob).to receive(:retry_job).with(wait: 5.minutes, queue: :default) + theme + end + end end end From 076d0d66eb11f75f5803a11304332488d5b9096b Mon Sep 17 00:00:00 2001 From: Alexander Ivanov Date: Tue, 21 Jul 2015 18:07:05 +0300 Subject: [PATCH 33/37] Added usage of devise-async and apartment-sidekiq gems --- Gemfile | 2 ++ Gemfile.lock | 7 +++++++ app/models/user.rb | 2 +- config/initializers/devise_async.rb | 5 +++++ 4 files changed, 15 insertions(+), 1 deletion(-) create mode 100644 config/initializers/devise_async.rb diff --git a/Gemfile b/Gemfile index 5460539..87c6890 100644 --- a/Gemfile +++ b/Gemfile @@ -16,11 +16,13 @@ gem 'coffee-rails', '~> 4.1.0' # Use apartment for multi-tenant databases gem 'apartment', '~> 1.0.1' +gem 'apartment-sidekiq', '~> 0.2.0' #bootstrap CSS #gem 'bootstrap-sass' # Devise is a flexible authentication solution for Rails based on Warden gem 'devise', '~> 3.5.1' +gem 'devise-async', '~>0.10.1' # simple authorization solution for Rails which is decoupled from user roles gem 'cancancan', '~> 1.12.0' diff --git a/Gemfile.lock b/Gemfile.lock index 3afba29..5a74026 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -39,6 +39,9 @@ GEM apartment (1.0.1) activerecord (>= 3.1.2, < 5.0) rack (>= 1.3.6) + apartment-sidekiq (0.2.0) + apartment (~> 1.0) + sidekiq (>= 2.11) arel (6.0.0) aws-sdk (2.1.7) aws-sdk-resources (= 2.1.7) @@ -91,6 +94,8 @@ GEM responders thread_safe (~> 0.1) warden (~> 1.2.3) + devise-async (0.10.1) + devise (~> 3.2) diff-lcs (1.2.5) dotenv (2.0.2) dotenv-rails (2.0.2) @@ -253,6 +258,7 @@ PLATFORMS DEPENDENCIES apartment (~> 1.0.1) + apartment-sidekiq (~> 0.2.0) aws-sdk (~> 2.1.7) byebug cancancan (~> 1.12.0) @@ -261,6 +267,7 @@ DEPENDENCIES cucumber-rails (~> 1.4.2) database_cleaner (~> 1.4.1) devise (~> 3.5.1) + devise-async (~> 0.10.1) dotenv-rails (~> 2.0.2) factory_girl (~> 4.5.0) factory_girl_rails (~> 4.0) diff --git a/app/models/user.rb b/app/models/user.rb index 96c5366..8cf1db7 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -1,7 +1,7 @@ class User < ActiveRecord::Base # Include default devise modules. Others available are: # :confirmable, :lockable, :timeoutable and :omniauthable, :rememberable, :trackable - devise :database_authenticatable, :registerable, :recoverable, :validatable + devise :database_authenticatable, :registerable, :recoverable, :validatable, :async belongs_to :role validates_presence_of :role diff --git a/config/initializers/devise_async.rb b/config/initializers/devise_async.rb new file mode 100644 index 0000000..0084a27 --- /dev/null +++ b/config/initializers/devise_async.rb @@ -0,0 +1,5 @@ +# Supported options: :resque, :sidekiq, :delayed_job, :queue_classic, :torquebox, :backburner, :que, :sucker_punch +Devise::Async.setup do |config| + config.backend = :sidekiq + config.queue = :default +end From 3c6758f70eb54bf00e75e90f83076ef714fd45a6 Mon Sep 17 00:00:00 2001 From: Alexander Ivanov Date: Wed, 22 Jul 2015 16:32:23 +0300 Subject: [PATCH 34/37] Added Sidekiq monitoring, added Theme views --- Gemfile | 2 ++ Gemfile.lock | 7 +++++++ app/controllers/themes_controller.rb | 6 +----- app/views/themes/index.html.slim | 17 +++++++++++++++++ app/views/themes/new.html.slim | 11 +++++++++++ config/routes.rb | 5 +++++ 6 files changed, 43 insertions(+), 5 deletions(-) diff --git a/Gemfile b/Gemfile index 87c6890..4a04dbd 100644 --- a/Gemfile +++ b/Gemfile @@ -40,6 +40,8 @@ gem 'slim-rails', '~> 3.0.1' # Simple, efficient background processing gem 'sidekiq', '~> 3.4.2' +# For Sidekiq monitor +gem 'sinatra', '~> 1.4.6' # Use ActiveModel has_secure_password # gem 'bcrypt', '~> 3.1.7' diff --git a/Gemfile.lock b/Gemfile.lock index 5a74026..34df7a4 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -140,6 +140,8 @@ GEM orm_adapter (0.5.0) pg (0.18.2) rack (1.6.1) + rack-protection (1.5.3) + rack rack-test (0.6.3) rack (>= 1.0) rails (4.2.1) @@ -214,6 +216,10 @@ GEM json (~> 1.0) redis (~> 3.2, >= 3.2.1) redis-namespace (~> 1.5, >= 1.5.2) + sinatra (1.4.6) + rack (~> 1.4) + rack-protection (~> 1.4) + tilt (>= 1.3, < 3) slim (3.0.6) temple (~> 0.7.3) tilt (>= 1.3.3, < 2.1) @@ -284,6 +290,7 @@ DEPENDENCIES sdoc (~> 0.4.0) shoulda-matchers (~> 2.8.0) sidekiq (~> 3.4.2) + sinatra (~> 1.4.6) slim-rails (~> 3.0.1) spring turbolinks diff --git a/app/controllers/themes_controller.rb b/app/controllers/themes_controller.rb index c39a115..719852b 100644 --- a/app/controllers/themes_controller.rb +++ b/app/controllers/themes_controller.rb @@ -4,11 +4,7 @@ class ThemesController < ApplicationController # GET#index def index - end - - # GET#new - def new - @theme = Theme.new + @themes = Theme.all end # GET#create_completed redirect from AWS diff --git a/app/views/themes/index.html.slim b/app/views/themes/index.html.slim index e69de29..d033c23 100644 --- a/app/views/themes/index.html.slim +++ b/app/views/themes/index.html.slim @@ -0,0 +1,17 @@ +- if can? :create, Theme + = link_to "Create New", new_theme_path + +table + thead + tr + th Name + th Status + th Actions + tbody + - @themes.each do |theme| + tr + td = theme.name + td = theme.status + td + - if can? :destroy, Theme + = link_to 'Delete', theme_path(theme), method: :delete diff --git a/app/views/themes/new.html.slim b/app/views/themes/new.html.slim index e69de29..47e40d1 100644 --- a/app/views/themes/new.html.slim +++ b/app/views/themes/new.html.slim @@ -0,0 +1,11 @@ += form_tag(theme_upload_params[:form_action], multipart: true, enforce_utf8: false, authenticity_token: false) + + = hidden_field_tag('key', theme_upload_params[:file_id]) + = hidden_field_tag('AWSAccessKeyId', theme_upload_params[:access_key]) + = hidden_field_tag('acl', theme_upload_params[:acl]) + = hidden_field_tag('success_action_redirect', theme_upload_params[:redirect_url]) + = hidden_field_tag('policy', theme_upload_params[:policy]) + = hidden_field_tag('signature', theme_upload_params[:signature]) + + = file_field_tag('file', accept: '.zip') + = submit_tag('Upload Theme') diff --git a/config/routes.rb b/config/routes.rb index 425c23d..a1c4cef 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -19,6 +19,11 @@ resources :users, except: :show end + require 'sidekiq/web' + authenticate :user do + mount Sidekiq::Web => '/sidekiq' + end + # Example of regular route: # get 'products/:id' => 'catalog#view' From c18f10ddaa7faa2fc93ea2762d4dc683f4212d7b Mon Sep 17 00:00:00 2001 From: Alexander Ivanov Date: Tue, 28 Jul 2015 15:55:15 +0300 Subject: [PATCH 35/37] Added Theme management acceptance tests --- app/controllers/application_controller.rb | 12 +- app/controllers/themes_controller.rb | 6 + app/views/layouts/application.html.erb | 26 ++++ spec/acceptance_tests/common_methods.rb | 64 ++++++++ spec/acceptance_tests/strings.rb | 39 +++++ .../themes_index_page_spec.rb | 144 ++++++++++++++++++ spec/controllers/themes_controller_spec.rb | 8 + 7 files changed, 297 insertions(+), 2 deletions(-) create mode 100644 spec/acceptance_tests/common_methods.rb create mode 100644 spec/acceptance_tests/strings.rb create mode 100644 spec/acceptance_tests/themes_management/themes_index_page_spec.rb diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 618ab77..06f83ef 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -2,8 +2,16 @@ class ApplicationController < ActionController::Base # Prevent CSRF attacks by raising an exception. # For APIs, you may want to use :null_session instead. protect_from_forgery with: :exception - rescue_from CanCan::AccessDenied do - redirect_to root_path + rescue_from CanCan::AccessDenied do |exception| + redirect_to root_path, alert: exception.message end + protected + + # Helper method used in layout to render model errors, should be overriden in controllers + def errors + {} + end + helper_method :errors + end diff --git a/app/controllers/themes_controller.rb b/app/controllers/themes_controller.rb index 719852b..d9b9235 100644 --- a/app/controllers/themes_controller.rb +++ b/app/controllers/themes_controller.rb @@ -14,11 +14,17 @@ def create_completed theme_name = File.basename theme_file_name, File.extname(theme_file_name) theme = Theme.new name: theme_name, zip_file_url: key, status: :processing redirect_to action: (theme.save ? 'index' : 'new') + if theme.errors.blank? + flash.notice = "Theme '#{theme_name}' uploaded successfully" + else + flash.alert = theme.errors.full_messages + end end # DELETE#destroy def destroy @theme.destroy + flash.notice = "Theme '#{@theme.name}' successfully deleted" redirect_to themes_path end end diff --git a/app/views/layouts/application.html.erb b/app/views/layouts/application.html.erb index 5595040..fdecce5 100644 --- a/app/views/layouts/application.html.erb +++ b/app/views/layouts/application.html.erb @@ -8,6 +8,32 @@ +
+ <% if flash.notice %> +

+ <%= flash.notice %> +

+ <% end %> +
+ +
+ <% if flash.alert %> +

+ <%= flash.alert %> +

+ <% end %> + <% if errors.any? %> +
+

<%= pluralize(errors.count, "error") %> occured:

+
    + <% errors.full_messages.each do |msg| %> +
  • <%= msg %>
    • + <% end %> +
+
+ <% end %> +
+ <%= yield %> diff --git a/spec/acceptance_tests/common_methods.rb b/spec/acceptance_tests/common_methods.rb new file mode 100644 index 0000000..d8f10c0 --- /dev/null +++ b/spec/acceptance_tests/common_methods.rb @@ -0,0 +1,64 @@ +module Common_methods + def strings(str) + Strings.get[str] + end + + def visit_url(url) + Capybara.visit "http://#{url}" + end + + def expect_page_url_to_be(url) + expect(current_url).to eq("http://#{url}") + end + + def expect_error(message) + expect(page.find('#errors')).to have_content(message) + end + + def expect_no_errors + expect(page.find('#errors').text).to eq "" + end + + def expect_notification(message) + expect(page.find('#notifications')).to have_content(message) + end + + def expect_no_notifications + expect(page.find('#notifications').text).to eq "" + end + + def log_in_with(login, pass) + fill_in('E-mail', with: login) + fill_in('Password', with: pass) + click_button "Log in" + end + + def cleanup_database + Account.destroy_all +# Apartment::Tenant.drop 'MySubdomain1' rescue nil + Apartment::Tenant.reset + DatabaseCleaner.clean + end + + def create_roles + FactoryGirl.create(:account_owner_role) + FactoryGirl.create(:user_role) + FactoryGirl.create(:create_users_role) + FactoryGirl.create(:update_users_role_role) + FactoryGirl.create(:update_users_password_role) + FactoryGirl.create(:update_users_role_and_password_role) + FactoryGirl.create(:delete_users_role) + end + + def register_account(login, password, name, subdomain) + visit_url 'lvh.me:3000' + click_link (strings(:registration)) + fill_in(strings(:login), with: login) + fill_in(strings(:password), with: password) + fill_in(strings(:pass_confirm), with: password) + fill_in(strings(:name), with: name) + fill_in(strings(:subdomain), with: subdomain) + click_button(strings(:create_account_btn)) + end + +end diff --git a/spec/acceptance_tests/strings.rb b/spec/acceptance_tests/strings.rb new file mode 100644 index 0000000..d3fa546 --- /dev/null +++ b/spec/acceptance_tests/strings.rb @@ -0,0 +1,39 @@ +module Strings + def self.get + { + #UI elements + #start page + registration: 'Registration', + #create account page + login: 'Email', + password: 'Password', + pass_confirm: 'Confirm password', + subdomain: 'Subdomain', + name: 'Name', + create_account_btn: 'Create Account', + #themes index page + create_theme_btn: 'Create New', + status: 'Status', + actions: 'Actions', + delete: 'Delete', + open_file_btn: 'file', + upload_file_btn: 'Upload Theme', + + #Messages + # Common + authorization_error: 'You are not authorized to access this page.', + #Themes index page + theme_created: 'uploaded successfully', + theme_deleted: 'successfully deleted', + theme_blank_name_error: "Name can't be blank", + theme_file_ext_error: 'Zip file url is invalid', + + + #URLs + new_account_page: '/accounts/new', + themes_index_page: '/themes', + themes_new_page: '/themes/new', + themes_create_completed: '/themes/create_completed', + } + end +end diff --git a/spec/acceptance_tests/themes_management/themes_index_page_spec.rb b/spec/acceptance_tests/themes_management/themes_index_page_spec.rb new file mode 100644 index 0000000..ae0172e --- /dev/null +++ b/spec/acceptance_tests/themes_management/themes_index_page_spec.rb @@ -0,0 +1,144 @@ +require "rails_helper" +require './spec/acceptance_tests/common_methods.rb' +require './spec/acceptance_tests/strings.rb' + +RSpec.shared_context "visit page with permission" do + before(:each) do + User.last.update role: create(role) + visit_url url + end +end + +RSpec.feature "theme_management.start_page", :type => :feature do + let(:subdomain) { "testsubdomain" } + let(:host_url) { subdomain + ".lvh.me:3000" } + let(:themes_url) { host_url + Strings.get[:themes_index_page] } + let(:themes_new_url) { host_url + Strings.get[:themes_new_page] } + let(:theme) { create(:theme) } + + include Common_methods + + background do + cleanup_database + register_account('test@mail.com', 'password', 'Test Tenant', subdomain) + User.last.update role: create(:manage_themes_role) + end + + context "User without themes permissions" do + context "User without all themes permissions" do + include_context "visit page with permission" do + let(:role) { :role } + let(:url) { themes_url } + end + scenario "gets auth.error when visits themes index page" do + expect_page_url_to_be (host_url + "/") + expect_error(Strings.get[:authorization_error]) + end + end + context "User without create themes permissions" do + include_context "visit page with permission" do + let(:role) { :delete_themes_role } + let(:url) { themes_url } + end + scenario "cannot create new theme" do + expect_page_url_to_be themes_url + expect(page).to have_no_link(Strings.get[:create_theme_btn], href: new_theme_path) + end + end + context "User without delete themes permissions" do + before(:each) do + theme + User.last.update role: create(:create_themes_role) + visit_url themes_url + end + scenario "cannot delete theme" do + expect_page_url_to_be themes_url + expect(all('tr td')[2]).to have_no_link(Strings.get[:delete], href: theme_path(1)) + end + end + end + + context "User with themes permissions" do + let(:url) { host_url + Strings.get[:themes_create_completed] + param } + before(:each) do + theme + visit_url themes_url + end + context "User with all themes permissions visits themes index page" do + scenario "and page contains all fields" do + expect_page_url_to_be themes_url + + expect(page).to have_link(Strings.get[:create_theme_btn], href: new_theme_path) + + expect(page).to have_table '' + expect(page).to have_selector 'table tr', count: 2 + expect(all('tr th')[0]).to have_text Strings.get[:name] + expect(all('tr th')[1]).to have_text Strings.get[:status] + expect(all('tr th')[2]).to have_text Strings.get[:actions] + + expect(all('tr td')[0]).to have_text theme.name + expect(all('tr td')[1]).to have_text theme.status + expect(all('tr td')[2]).to have_link(Strings.get[:delete], href: theme_path(1)) + end + end + context "User clicks Create New link and opens Create Theme page" do + scenario "and page contains all fields" do + click_link(Strings.get[:create_theme_btn]) + expect_page_url_to_be themes_new_url + expect(page).to have_field(Strings.get[:open_file_btn]) + expect(page).to have_button(Strings.get[:upload_file_btn]) + end + end + context "clicks Create New link and selects theme zip file" do + let(:param) { "?key=12345_test theme.zip" } + scenario "clicks Upload Theme and successfully uploads new Theme" do + click_link(Strings.get[:create_theme_btn]) + + # click_button(Strings.get[:upload_file_btn]) + # Couldn't stub redirect from Amazon, just call action directly + visit_url url + + expect_page_url_to_be themes_url + expect_no_errors + expect_notification Strings.get[:theme_created] + expect(all('tr td')[3]).to have_text Theme.last.name + expect(all('tr td')[4]).to have_text Theme.last.status + expect(all('tr td')[5]).to have_link(Strings.get[:delete], href: theme_path(2)) + end + end + context "clicks Create New link and does not select theme file" do + let(:param) { "?key=12345_" } + scenario "clicks Upload Theme and gets an error" do + click_link(Strings.get[:create_theme_btn]) + + visit_url url + + expect_page_url_to_be themes_new_url + expect_no_notifications + expect_error Strings.get[:theme_blank_name_error] + end + end + context "clicks Create New link and select theme file not in ZIP format" do + let(:param) { "?key=12345_theme.exe" } + scenario "clicks Upload Theme and gets an error" do + click_link(Strings.get[:create_theme_btn]) + + visit_url url + + expect_page_url_to_be themes_new_url + expect_no_notifications + expect_error Strings.get[:theme_file_ext_error] + end + end + context "User with all themes permissions visits themes index page" do + scenario "clicks Delete link and successfully deletes theme" do + click_link(Strings.get[:delete]) + + expect_page_url_to_be themes_url + expect(page).to have_selector 'table tr', count: 1 + expect_no_errors + expect_notification Strings.get[:theme_deleted] + end + end + end +end diff --git a/spec/controllers/themes_controller_spec.rb b/spec/controllers/themes_controller_spec.rb index baefa31..a92dd27 100644 --- a/spec/controllers/themes_controller_spec.rb +++ b/spec/controllers/themes_controller_spec.rb @@ -14,6 +14,10 @@ RSpec.shared_examples "uncussessfull create_completed" do it { expect(Theme.count).to be_zero } it { is_expected.to redirect_to action: :new } + it "should set flash" do + subject + expect(controller).to set_flash[:alert] + end end RSpec.describe ThemesController, type: :controller do @@ -84,6 +88,10 @@ context "when parsing key: #{key}" do it_behaves_like "creating Theme in DB", key, theme_name it { is_expected.to redirect_to action: :index } + it "should set flash" do + subject + expect(controller).to set_flash[:notice] + end end end context "when theme already exists" do From 88d2ed4f02f38ce203522d055acfa969b5d9acdc Mon Sep 17 00:00:00 2001 From: Alexander Ivanov Date: Tue, 28 Jul 2015 16:20:35 +0300 Subject: [PATCH 36/37] Update sass-rails gem version --- Gemfile.lock | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/Gemfile.lock b/Gemfile.lock index 18cdca2..9559476 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -223,7 +223,7 @@ GEM rspec-support (~> 3.2.0) rspec-support (3.2.2) sass (3.4.16) - sass-rails (5.0.0) + sass-rails (5.0.3) railties (>= 4.0.0, < 5.0) sass (~> 3.1) sprockets (>= 2.8, < 4.0) @@ -335,3 +335,6 @@ DEPENDENCIES uglifier (>= 1.3.0) unicorn (~> 4.9.0) web-console (~> 2.0) + +BUNDLED WITH + 1.10.3 From d0052207f27f5b160af04e842e1fda5ef398b6c0 Mon Sep 17 00:00:00 2001 From: Alexander Ivanov Date: Wed, 29 Jul 2015 11:00:50 +0300 Subject: [PATCH 37/37] DRY-ed themes_index_page_spec --- spec/acceptance_tests/common_methods.rb | 4 +- spec/acceptance_tests/strings.rb | 4 +- .../themes_index_page_spec.rb | 49 +++++++++---------- 3 files changed, 29 insertions(+), 28 deletions(-) diff --git a/spec/acceptance_tests/common_methods.rb b/spec/acceptance_tests/common_methods.rb index d8f10c0..b692da3 100644 --- a/spec/acceptance_tests/common_methods.rb +++ b/spec/acceptance_tests/common_methods.rb @@ -1,6 +1,8 @@ module Common_methods +require './spec/acceptance_tests/strings.rb' + def strings(str) - Strings.get[str] + Strings[str] end def visit_url(url) diff --git a/spec/acceptance_tests/strings.rb b/spec/acceptance_tests/strings.rb index d3fa546..95dc16c 100644 --- a/spec/acceptance_tests/strings.rb +++ b/spec/acceptance_tests/strings.rb @@ -1,5 +1,5 @@ module Strings - def self.get + def self.[](key) { #UI elements #start page @@ -34,6 +34,6 @@ def self.get themes_index_page: '/themes', themes_new_page: '/themes/new', themes_create_completed: '/themes/create_completed', - } + }[key] end end diff --git a/spec/acceptance_tests/themes_management/themes_index_page_spec.rb b/spec/acceptance_tests/themes_management/themes_index_page_spec.rb index ae0172e..320c8f7 100644 --- a/spec/acceptance_tests/themes_management/themes_index_page_spec.rb +++ b/spec/acceptance_tests/themes_management/themes_index_page_spec.rb @@ -1,6 +1,5 @@ require "rails_helper" require './spec/acceptance_tests/common_methods.rb' -require './spec/acceptance_tests/strings.rb' RSpec.shared_context "visit page with permission" do before(:each) do @@ -12,8 +11,8 @@ RSpec.feature "theme_management.start_page", :type => :feature do let(:subdomain) { "testsubdomain" } let(:host_url) { subdomain + ".lvh.me:3000" } - let(:themes_url) { host_url + Strings.get[:themes_index_page] } - let(:themes_new_url) { host_url + Strings.get[:themes_new_page] } + let(:themes_url) { host_url + strings(:themes_index_page) } + let(:themes_new_url) { host_url + strings(:themes_new_page) } let(:theme) { create(:theme) } include Common_methods @@ -32,7 +31,7 @@ end scenario "gets auth.error when visits themes index page" do expect_page_url_to_be (host_url + "/") - expect_error(Strings.get[:authorization_error]) + expect_error(strings(:authorization_error)) end end context "User without create themes permissions" do @@ -42,7 +41,7 @@ end scenario "cannot create new theme" do expect_page_url_to_be themes_url - expect(page).to have_no_link(Strings.get[:create_theme_btn], href: new_theme_path) + expect(page).to have_no_link(strings(:create_theme_btn), href: new_theme_path) end end context "User without delete themes permissions" do @@ -53,13 +52,13 @@ end scenario "cannot delete theme" do expect_page_url_to_be themes_url - expect(all('tr td')[2]).to have_no_link(Strings.get[:delete], href: theme_path(1)) + expect(all('tr td')[2]).to have_no_link(strings(:delete), href: theme_path(1)) end end end context "User with themes permissions" do - let(:url) { host_url + Strings.get[:themes_create_completed] + param } + let(:url) { host_url + strings(:themes_create_completed) + param } before(:each) do theme visit_url themes_url @@ -68,76 +67,76 @@ scenario "and page contains all fields" do expect_page_url_to_be themes_url - expect(page).to have_link(Strings.get[:create_theme_btn], href: new_theme_path) + expect(page).to have_link(strings(:create_theme_btn), href: new_theme_path) expect(page).to have_table '' expect(page).to have_selector 'table tr', count: 2 - expect(all('tr th')[0]).to have_text Strings.get[:name] - expect(all('tr th')[1]).to have_text Strings.get[:status] - expect(all('tr th')[2]).to have_text Strings.get[:actions] + expect(all('tr th')[0]).to have_text strings(:name) + expect(all('tr th')[1]).to have_text strings(:status) + expect(all('tr th')[2]).to have_text strings(:actions) expect(all('tr td')[0]).to have_text theme.name expect(all('tr td')[1]).to have_text theme.status - expect(all('tr td')[2]).to have_link(Strings.get[:delete], href: theme_path(1)) + expect(all('tr td')[2]).to have_link(strings(:delete), href: theme_path(1)) end end context "User clicks Create New link and opens Create Theme page" do scenario "and page contains all fields" do - click_link(Strings.get[:create_theme_btn]) + click_link(strings(:create_theme_btn)) expect_page_url_to_be themes_new_url - expect(page).to have_field(Strings.get[:open_file_btn]) - expect(page).to have_button(Strings.get[:upload_file_btn]) + expect(page).to have_field(strings(:open_file_btn)) + expect(page).to have_button(strings(:upload_file_btn)) end end context "clicks Create New link and selects theme zip file" do let(:param) { "?key=12345_test theme.zip" } scenario "clicks Upload Theme and successfully uploads new Theme" do - click_link(Strings.get[:create_theme_btn]) + click_link(strings(:create_theme_btn)) - # click_button(Strings.get[:upload_file_btn]) + # click_button(strings(:upload_file_btn)) # Couldn't stub redirect from Amazon, just call action directly visit_url url expect_page_url_to_be themes_url expect_no_errors - expect_notification Strings.get[:theme_created] + expect_notification strings(:theme_created) expect(all('tr td')[3]).to have_text Theme.last.name expect(all('tr td')[4]).to have_text Theme.last.status - expect(all('tr td')[5]).to have_link(Strings.get[:delete], href: theme_path(2)) + expect(all('tr td')[5]).to have_link(strings(:delete), href: theme_path(2)) end end context "clicks Create New link and does not select theme file" do let(:param) { "?key=12345_" } scenario "clicks Upload Theme and gets an error" do - click_link(Strings.get[:create_theme_btn]) + click_link(strings(:create_theme_btn)) visit_url url expect_page_url_to_be themes_new_url expect_no_notifications - expect_error Strings.get[:theme_blank_name_error] + expect_error strings(:theme_blank_name_error) end end context "clicks Create New link and select theme file not in ZIP format" do let(:param) { "?key=12345_theme.exe" } scenario "clicks Upload Theme and gets an error" do - click_link(Strings.get[:create_theme_btn]) + click_link(strings(:create_theme_btn)) visit_url url expect_page_url_to_be themes_new_url expect_no_notifications - expect_error Strings.get[:theme_file_ext_error] + expect_error strings(:theme_file_ext_error) end end context "User with all themes permissions visits themes index page" do scenario "clicks Delete link and successfully deletes theme" do - click_link(Strings.get[:delete]) + click_link(strings(:delete)) expect_page_url_to_be themes_url expect(page).to have_selector 'table tr', count: 1 expect_no_errors - expect_notification Strings.get[:theme_deleted] + expect_notification strings(:theme_deleted) end end end