From 086f8a85ac2457de3595665ae3814b41e4bdffde Mon Sep 17 00:00:00 2001 From: aidewoode Date: Tue, 22 Nov 2022 16:38:30 +0800 Subject: [PATCH 1/4] Remove devcontainer --- .devcontainer/Dockerfile | 24 -------------- .devcontainer/devcontainer.json | 26 --------------- .devcontainer/docker-compose.yml | 56 -------------------------------- 3 files changed, 106 deletions(-) delete mode 100644 .devcontainer/Dockerfile delete mode 100644 .devcontainer/devcontainer.json delete mode 100644 .devcontainer/docker-compose.yml diff --git a/.devcontainer/Dockerfile b/.devcontainer/Dockerfile deleted file mode 100644 index 6c8e8f33..00000000 --- a/.devcontainer/Dockerfile +++ /dev/null @@ -1,24 +0,0 @@ -# [Choice] Ruby version (use -bullseye variants on local arm64/Apple Silicon): 3, 3.1, 3.0, 2, 2.7, 2.6, 3-bullseye, 3.1-bullseye, 3.0-bullseye, 2-bullseye, 2.7-bullseye, 2.6-bullseye, 3-buster, 3.1-buster, 3.0-buster, 2-buster, 2.7-buster, 2.6-buster -ARG VARIANT=3.1-bullseye -FROM mcr.microsoft.com/vscode/devcontainers/ruby:0-${VARIANT} - -# Install Rails -# RUN gem install rails webdrivers - -# Default value to allow debug server to serve content over GitHub Codespace's port forwarding service -# The value is a comma-separated list of allowed domains -ENV RAILS_DEVELOPMENT_HOSTS=".githubpreview.dev" - -# [Choice] Node.js version: lts/*, 16, 14, 12, 10 -ARG NODE_VERSION="lts/*" -RUN su vscode -c "source /usr/local/share/nvm/nvm.sh && nvm install ${NODE_VERSION} 2>&1" - -# [Optional] Uncomment this section to install additional OS packages. -RUN apt-get update && export DEBIAN_FRONTEND=noninteractive \ - && apt-get -y install --no-install-recommends imagemagick ffmpeg chromium - -# [Optional] Uncomment this line to install additional gems. -RUN gem install foreman - -# [Optional] Uncomment this line to install global node packages. -RUN su vscode -c "source /usr/local/share/nvm/nvm.sh && npm install -g yarn" 2>&1 \ No newline at end of file diff --git a/.devcontainer/devcontainer.json b/.devcontainer/devcontainer.json deleted file mode 100644 index f460a101..00000000 --- a/.devcontainer/devcontainer.json +++ /dev/null @@ -1,26 +0,0 @@ -// For format details, see https://aka.ms/devcontainer.json. For config options, see the README at: -// https://github.com/microsoft/vscode-dev-containers/tree/v0.241.1/containers/ruby-rails-postgres -// Update the VARIANT arg in docker-compose.yml to pick a Ruby version -{ - "name": "Black Candy", - "dockerComposeFile": "docker-compose.yml", - "service": "app", - "workspaceFolder": "/workspace", - // Configure tool-specific properties. - "customizations": { - // Configure properties specific to VS Code. - "vscode": { - // Add the IDs of extensions you want installed when the container is created. - "extensions": ["rebornix.Ruby"] - } - }, - // Use 'forwardPorts' to make a list of ports inside the container available locally. - // This can be used to network with other containers or the host. - "forwardPorts": [ - 3000 - ], - // Use 'postCreateCommand' to run commands after the container is created. - "postCreateCommand": "bundle install && yarn install && rake db:prepare", - // Comment out to connect as root instead. More info: https://aka.ms/vscode-remote/containers/non-root. - "remoteUser": "vscode" -} diff --git a/.devcontainer/docker-compose.yml b/.devcontainer/docker-compose.yml deleted file mode 100644 index bbd46361..00000000 --- a/.devcontainer/docker-compose.yml +++ /dev/null @@ -1,56 +0,0 @@ -version: '3' - -services: - app: - build: - context: .. - dockerfile: .devcontainer/Dockerfile - args: - # Update 'VARIANT' to pick a version of Ruby: 3, 3.1, 3.0, 2, 2.7, 2.6 - # Append -bullseye or -buster to pin to an OS version. - # Use -bullseye variants on local arm64/Apple Silicon. - VARIANT: "3.1-bullseye" - # Optional Node.js version to install - NODE_VERSION: "14" - - volumes: - - ..:/workspace:cached - - ~/media_data:/media_data - - bundler-data:/usr/local/bundle - - environment: - DB_HOST: postgres - DB_USER: postgres - MEDIA_PATH: /media_data - - # Overrides default command so things don't shut down after the process ends. - command: sleep infinity - - # Runs app on the same network as the database container, allows "forwardPorts" in devcontainer.json function. - networks: - - external_network - - internal_network - # Uncomment the next line to use a non-root user for all processes. - user: vscode - - # Use "forwardPorts" in **devcontainer.json** to forward an app port locally. - # (Adding the "ports" property to this file will not forward from a Codespace.) - - postgres: - image: postgres:11.1-alpine - restart: unless-stopped - volumes: - - postgres-data:/var/lib/postgresql/data - networks: - - internal_network - # Add "forwardPorts": ["5432"] to **devcontainer.json** to forward PostgreSQL locally. - # (Adding the "ports" property to this file will not forward from a Codespace.) - -volumes: - postgres-data: - bundler-data: - -networks: - external_network: - internal_network: - internal: true From 8d1fbccb1e35a594503b7f9203121c5b5a2a3bbf Mon Sep 17 00:00:00 2001 From: aidewoode Date: Wed, 23 Nov 2022 10:52:37 +0800 Subject: [PATCH 2/4] Remove dotenv --- .env.template | 4 ---- Gemfile | 1 - Gemfile.lock | 5 ----- 3 files changed, 10 deletions(-) delete mode 100644 .env.template diff --git a/.env.template b/.env.template deleted file mode 100644 index 9e602831..00000000 --- a/.env.template +++ /dev/null @@ -1,4 +0,0 @@ -DB_USER=DB_USER -DB_HOST=DB_HOST -REDIS_SIDEKIQ_URL=REDIS_SIDEKIQ_URL -REDIS_CABLE_URL=REDIS_CABLE_URL diff --git a/Gemfile b/Gemfile index 54992b59..d2f595db 100644 --- a/Gemfile +++ b/Gemfile @@ -73,7 +73,6 @@ gem "bootsnap", "~> 1.10.3", require: false group :development, :test do gem "standard", "~> 1.7" - gem "dotenv-rails", "~> 2.8.1" # See https://guides.rubyonrails.org/debugging_rails_applications.html#debugging-with-the-debug-gem gem "debug", platforms: %i[mri mingw x64_mingw] end diff --git a/Gemfile.lock b/Gemfile.lock index 36abf3ba..1d8c845f 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -116,10 +116,6 @@ GEM irb (>= 1.3.6) reline (>= 0.3.1) docile (1.4.0) - dotenv (2.8.1) - dotenv-rails (2.8.1) - dotenv (= 2.8.1) - railties (>= 3.2) erubi (1.11.0) ferrum (0.13) addressable (~> 2.5) @@ -321,7 +317,6 @@ DEPENDENCIES cssbundling-rails (~> 1.1.0) cuprite (~> 0.14.3) debug - dotenv-rails (~> 2.8.1) httparty (~> 0.17.0) jbuilder (~> 2.11.5) jsbundling-rails (~> 1.0.0) From 2efc6fb8337e702ec01c9a316da64fa00e56b292 Mon Sep 17 00:00:00 2001 From: aidewoode Date: Fri, 25 Nov 2022 11:24:45 +0800 Subject: [PATCH 3/4] Add black candy config to set app config easily --- Gemfile | 13 +- Gemfile.lock | 2 + app/controllers/api/v1/stream_controller.rb | 6 +- app/controllers/application_controller.rb | 4 +- app/controllers/concerns/playable.rb | 2 +- app/controllers/settings_controller.rb | 2 +- app/controllers/users/settings_controller.rb | 2 +- app/controllers/users_controller.rb | 4 +- app/lib/black_candy_error.rb | 9 -- app/models/concerns/global_setting.rb | 4 +- app/models/setting.rb | 2 +- config/application.rb | 3 + config/cable.yml | 4 +- config/database.yml | 4 +- config/environments/production.rb | 6 +- config/initializers/sidekiq.rb | 6 +- config/puma.rb | 15 +-- lib/black_candy/config.rb | 87 +++++++++++++ lib/black_candy/error.rb | 11 ++ .../api/v1/stream_controller_test.rb | 4 +- .../v1/transcoded_stream_controller_test.rb | 2 +- test/lib/black_candy/config_test.rb | 123 ++++++++++++++++++ test/models/setting_test.rb | 8 +- test/test_helper.rb | 15 ++- 24 files changed, 274 insertions(+), 64 deletions(-) delete mode 100644 app/lib/black_candy_error.rb create mode 100644 lib/black_candy/config.rb create mode 100644 lib/black_candy/error.rb create mode 100644 test/lib/black_candy/config_test.rb diff --git a/Gemfile b/Gemfile index d2f595db..b510fc7a 100644 --- a/Gemfile +++ b/Gemfile @@ -19,12 +19,6 @@ gem "cssbundling-rails", "~> 1.1.0" # Bundle and transpile JavaScript in Rails gem "jsbundling-rails", "~> 1.0.0" -if ENV.fetch("DATABASE_ADAPTER", "sqlite") == "postgresql" - gem "pg", "~> 1.3.2" -else - gem "sqlite3", "~> 1.5.0" -end - # Use Puma as the app server gem "puma", "~> 6.0.0" @@ -65,6 +59,12 @@ gem "bcrypt", "~> 3.1.11" # For sync on library changes gem "listen", "~> 3.7.1" +# For postgresql database adapter +gem "pg", "~> 1.3.2" + +# For sqlite database adapter +gem "sqlite3", "~> 1.5.0" + # Use Capistrano for deployment # gem 'capistrano-rails', group: :development @@ -87,7 +87,6 @@ group :development do end group :test do - # Adds support for Capybara system testing and selenium driver gem "capybara", "~> 3.38.0" gem "cuprite", "~> 0.14.3" gem "webmock", "~> 3.14.0", require: false diff --git a/Gemfile.lock b/Gemfile.lock index 1d8c845f..09421a6d 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -180,6 +180,7 @@ GEM parallel (1.22.1) parser (3.1.2.1) ast (~> 2.4.1) + pg (1.3.5) propshaft (0.6.4) actionpack (>= 7.0.0) activesupport (>= 7.0.0) @@ -323,6 +324,7 @@ DEPENDENCIES listen (~> 3.7.1) memory_profiler (~> 0.9.13) pagy (~> 5.6.6) + pg (~> 1.3.2) propshaft (~> 0.6.4) puma (~> 6.0.0) rails (~> 7.0.0) diff --git a/app/controllers/api/v1/stream_controller.rb b/app/controllers/api/v1/stream_controller.rb index 725d11c7..e4dab5dd 100644 --- a/app/controllers/api/v1/stream_controller.rb +++ b/app/controllers/api/v1/stream_controller.rb @@ -23,12 +23,8 @@ def find_stream @stream = Stream.new(song) end - def nginx_sendfile? - ENV.fetch("NGINX_SENDFILE", "false") == "true" - end - def send_local_file(file_path) - if nginx_sendfile? + if BlackCandy::Config.nginx_sendfile? set_nginx_header send_file file_path diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 4820cde3..215debc1 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -9,7 +9,7 @@ class ApplicationController < ActionController::Base before_action :find_current_user before_action :require_login - rescue_from BlackCandyError::Forbidden do + rescue_from BlackCandy::Error::Forbidden do respond_to do |format| format.js { head :forbidden } format.json { head :forbidden } @@ -75,7 +75,7 @@ def require_login end def require_admin - raise BlackCandyError::Forbidden unless is_admin? + raise BlackCandy::Error::Forbidden unless is_admin? end def logout_current_user diff --git a/app/controllers/concerns/playable.rb b/app/controllers/concerns/playable.rb index 17810efc..5d266a47 100644 --- a/app/controllers/concerns/playable.rb +++ b/app/controllers/concerns/playable.rb @@ -9,7 +9,7 @@ module Playable end def play - raise BlackCandyError::Forbidden if @song_ids.blank? + raise BlackCandy::Error::Forbidden if @song_ids.blank? @playlist = Current.user.current_playlist @playlist.replace(@song_ids) diff --git a/app/controllers/settings_controller.rb b/app/controllers/settings_controller.rb index 45000008..db21b25f 100644 --- a/app/controllers/settings_controller.rb +++ b/app/controllers/settings_controller.rb @@ -6,7 +6,7 @@ def show end def update - raise BlackCandyError::Forbidden unless is_admin? + raise BlackCandy::Error::Forbidden unless is_admin? setting = Setting.instance if setting.update(setting_params) diff --git a/app/controllers/users/settings_controller.rb b/app/controllers/users/settings_controller.rb index 0148699d..1cb24508 100644 --- a/app/controllers/users/settings_controller.rb +++ b/app/controllers/users/settings_controller.rb @@ -25,6 +25,6 @@ def find_user end def auth_user - raise BlackCandyError::Forbidden unless @user == Current.user + raise BlackCandy::Error::Forbidden unless @user == Current.user end end diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index 2d377ff1..67e5a0b8 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -37,7 +37,7 @@ def update def destroy # Avoit user delete self - raise BlackCandyError::Forbidden if @user == Current.user + raise BlackCandy::Error::Forbidden if @user == Current.user @user.destroy flash.now[:success] = t("success.delete") @@ -50,7 +50,7 @@ def find_user end def auth_user - raise BlackCandyError::Forbidden unless @user == Current.user || is_admin? + raise BlackCandy::Error::Forbidden unless @user == Current.user || is_admin? end def user_params diff --git a/app/lib/black_candy_error.rb b/app/lib/black_candy_error.rb deleted file mode 100644 index d1734f47..00000000 --- a/app/lib/black_candy_error.rb +++ /dev/null @@ -1,9 +0,0 @@ -# frozen_string_literal: true - -module BlackCandyError - class Forbidden < StandardError; end - - class NotFound < StandardError; end - - class InvalidFilePath < StandardError; end -end diff --git a/app/models/concerns/global_setting.rb b/app/models/concerns/global_setting.rb index bf56f129..2ed83ad2 100644 --- a/app/models/concerns/global_setting.rb +++ b/app/models/concerns/global_setting.rb @@ -32,7 +32,9 @@ def has_setting(setting, type: :string, default: nil) define_singleton_method(setting) do setting_value = instance.send(setting) - setting_value || default || ENV[setting.to_s.upcase] + default_value = default.is_a?(Proc) ? default.call : default + + setting_value || default_value end end end diff --git a/app/models/setting.rb b/app/models/setting.rb index ef64481a..b61e7f28 100644 --- a/app/models/setting.rb +++ b/app/models/setting.rb @@ -5,7 +5,7 @@ class Setting < ApplicationRecord AVAILABLE_BITRATE_OPTIONS = [128, 192, 320].freeze - has_setting :media_path + has_setting :media_path, default: proc { BlackCandy::Config.media_path } has_setting :discogs_token has_setting :transcode_bitrate, type: :integer, default: 128 has_setting :allow_transcode_lossless, type: :boolean, default: false diff --git a/config/application.rb b/config/application.rb index 5b9bc320..cd113f80 100644 --- a/config/application.rb +++ b/config/application.rb @@ -14,6 +14,9 @@ require "action_cable/engine" require "rails/test_unit/railtie" +require_relative "../lib/black_candy/config" +require_relative "../lib/black_candy/error" + # Require the gems listed in Gemfile, including any gems # you've limited to :test, :development, or :production. Bundler.require(*Rails.groups) diff --git a/config/cable.yml b/config/cable.yml index 7551cd92..bc8f1894 100644 --- a/config/cable.yml +++ b/config/cable.yml @@ -4,10 +4,10 @@ development: test: adapter: test -<% if ENV["REDIS_URL"].present? || ENV["REDIS_CABLE_URL"].present? %> +<% if BlackCandy::Config.redis_cable_url.present? %> production: adapter: redis - url: <%= ENV.fetch("REDIS_CABLE_URL", ENV["REDIS_URL"]) %> + url: <%= BlackCandy::Config.redis_cable_url %> channel_prefix: black_candy_production <% else %> production: diff --git a/config/database.yml b/config/database.yml index 3eaad4f5..d494ffc0 100644 --- a/config/database.yml +++ b/config/database.yml @@ -12,12 +12,12 @@ test: database: db/test.sqlite3 -<% if ENV.fetch("DATABASE_ADAPTER", "sqlite") == "postgresql" %> +<% if BlackCandy::Config.database_adapter == "postgresql" %> production: adapter: postgresql encoding: unicode pool: <%= ENV.fetch("RAILS_MAX_THREADS") { 5 } %> - url: <%= ENV['DATABASE_URL'] %> + url: <%= BlackCandy::Config.database_url %> <% else %> production: <<: *default diff --git a/config/environments/production.rb b/config/environments/production.rb index 4ca81017..07dc1c90 100644 --- a/config/environments/production.rb +++ b/config/environments/production.rb @@ -42,12 +42,10 @@ config.log_tags = [:request_id] # Use a different cache store in production. - config.cache_store = (ENV["REDIS_URL"].present? || ENV["REDIS_CACHE_URL"].present?) ? - [:redis_cache_store, {url: ENV.fetch("REDIS_CACHE_URL", ENV["REDIS_URL"])}] : - [:file_store, "#{root}/tmp/cache/"] + config.cache_store = BlackCandy::Config.redis_cache_url.present? ? [:redis_cache_store, {url: BlackCandy::Config.redis_cache_url}] : [:file_store, "#{root}/tmp/cache/"] # Use a real queuing backend for Active Job (and separate queues per environment). - config.active_job.queue_adapter = (ENV["REDIS_URL"].present? || ENV["REDIS_SIDEKIQ_URL"].present?) ? :sidekiq : :async + config.active_job.queue_adapter = BlackCandy::Config.redis_sidekiq_url.present? ? :sidekiq : :async config.active_job.queue_name_prefix = "black_candy_production" diff --git a/config/initializers/sidekiq.rb b/config/initializers/sidekiq.rb index dd7bdb80..c37c97b5 100644 --- a/config/initializers/sidekiq.rb +++ b/config/initializers/sidekiq.rb @@ -1,9 +1,9 @@ -if Rails.configuration.active_job.queue_adapter == :sidekiq +if BlackCandy::Config.redis_sidekiq_url.present? Sidekiq.configure_server do |config| - config.redis = {url: ENV.fetch("REDIS_SIDEKIQ_URL", ENV["REDIS_URL"])} + config.redis = {url: BlackCandy::Config.redis_sidekiq_url} end Sidekiq.configure_client do |config| - config.redis = {url: ENV.fetch("REDIS_SIDEKIQ_URL", ENV["REDIS_URL"])} + config.redis = {url: BlackCandy::Config.redis_sidekiq_url} end end diff --git a/config/puma.rb b/config/puma.rb index c7c6f563..9bacad24 100644 --- a/config/puma.rb +++ b/config/puma.rb @@ -1,20 +1,11 @@ -embedded_sidekiq = ENV.fetch("EMBEDDED_SIDEKIQ", "false") == "true" - -# Accroding to the documentation, we should keep embedded sidekiq concurrency very low, i.e. 1-2 -embedded_sidekiq_concurrency = [ENV.fetch("EMBEDDED_SIDEKIQ_CONCURRENCY", 2), 2].min -# A good rule of thumb is that your puma threads + sidekiq concurrency should never be greater than 5 -max_threads_for_embedded_sidekiq = 5 +require_relative "../lib/black_candy/config" # Puma can serve each request in a thread from an internal thread pool. # The `threads` method setting takes two numbers: a minimum and maximum. # Any libraries that use thread pools should be configured to match # the maximum value specified for Puma. Default is set to 5 threads for minimum # and maximum; this matches the default thread size of Active Record. -# -max_threads_count = embedded_sidekiq ? (max_threads_for_embedded_sidekiq - embedded_sidekiq_concurrency) : ENV.fetch("RAILS_MAX_THREADS", 5) -min_threads_count = embedded_sidekiq ? 1 : ENV.fetch("RAILS_MIN_THREADS") { max_threads_count } - -threads min_threads_count, max_threads_count +threads BlackCandy::Config.puma_min_threads_count, BlackCandy::Config.puma_max_threads_count # Specifies the `worker_timeout` threshold that Puma will use to wait before # terminating a worker in development environments. @@ -49,7 +40,7 @@ # Allow puma to be restarted by `rails restart` command. plugin :tmp_restart -if embedded_sidekiq +if BlackCandy::Config.embedded_sidekiq? workers 2 # Preloading the application is necessary to ensure diff --git a/lib/black_candy/config.rb b/lib/black_candy/config.rb new file mode 100644 index 00000000..b14c32c5 --- /dev/null +++ b/lib/black_candy/config.rb @@ -0,0 +1,87 @@ +# frozen_string_literal: true + +module BlackCandy + class Config + # Accroding to the documentation, a good rule of thumb is that your puma threads + sidekiq concurrency should never be greater than 5 + MAX_THREADS_FOR_EMBEDDED_SIDEKIQ = 5 + SUPPORTED_DATABASE_ADAPTERS = %w[sqlite postgresql] + + class ValidationError < StandardError; end + + class << self + private + + def has_config(config_name, default: nil) + define_singleton_method(config_name) do + env_value = ENV[config_name.to_s.upcase] + default_value = default.is_a?(Proc) ? default.call : default + + return default_value if env_value.nil? || env_value.empty? + + config_value = case default_value.class.name + when "TrueClass", "FalseClass" + ["true", "yes"].include?(env_value&.downcase) + when "Integer" + env_value.to_i + else + env_value.to_s + end + + validate_block = @validate_blocks&.fetch(config_name, nil) + validate_block.call(config_value) if validate_block.respond_to?(:call) + + config_value + end + + if [true, false].include?(default) + singleton_class.send(:alias_method, "#{config_name}?", config_name) + end + end + + def validate(config_name, &block) + @validate_blocks ||= {} + @validate_blocks[config_name] = block + end + + def raise_validation_error(message) + raise ValidationError.new(message) + end + end + + has_config :redis_url + has_config :redis_cache_url, default: proc { redis_url } + has_config :redis_sidekiq_url, default: proc { redis_url } + has_config :redis_cable_url, default: proc { redis_url } + has_config :database_url + has_config :media_path + has_config :database_adapter, default: "sqlite" + has_config :nginx_sendfile, default: false + has_config :embedded_sidekiq, default: false + has_config :embedded_sidekiq_concurrency, default: 2 + + # Accroding to the documentation, we should keep embedded sidekiq concurrency very low, i.e. 1-2 + validate :embedded_sidekiq_concurrency do |value| + if value > 2 + raise_validation_error "The concurrency of embedded sidekiq is too high." + end + end + + validate :database_adapter do |value| + unless SUPPORTED_DATABASE_ADAPTERS.include?(value) + raise_validation_error "Unsupported database adapter." + end + + if value == "postgresql" && database_url.blank? + raise_validation_error "DATABASE_URL is required if database adapter is postgresql" + end + end + + def self.puma_max_threads_count + embedded_sidekiq ? (MAX_THREADS_FOR_EMBEDDED_SIDEKIQ - embedded_sidekiq_concurrency) : ENV.fetch("RAILS_MAX_THREADS", 5) + end + + def self.puma_min_threads_count + embedded_sidekiq ? (MAX_THREADS_FOR_EMBEDDED_SIDEKIQ - embedded_sidekiq_concurrency) : ENV.fetch("RAILS_MIN_THREADS") { puma_max_threads_count } + end + end +end diff --git a/lib/black_candy/error.rb b/lib/black_candy/error.rb new file mode 100644 index 00000000..ae57dd3f --- /dev/null +++ b/lib/black_candy/error.rb @@ -0,0 +1,11 @@ +# frozen_string_literal: true + +module BlackCandy + module Error + class Forbidden < StandardError; end + + class NotFound < StandardError; end + + class InvalidFilePath < StandardError; end + end +end diff --git a/test/controllers/api/v1/stream_controller_test.rb b/test/controllers/api/v1/stream_controller_test.rb index 22e771c1..4c1e112d 100644 --- a/test/controllers/api/v1/stream_controller_test.rb +++ b/test/controllers/api/v1/stream_controller_test.rb @@ -14,7 +14,7 @@ class Api::V1::StreamControllerTest < ActionDispatch::IntegrationTest end test "should set header for nginx send file" do - stub_env("NGINX_SENDFILE", "true") do + with_env("NGINX_SENDFILE" => "true") do get new_api_v1_stream_url(song_id: songs(:mp3_sample).id) assert_equal Setting.media_path, @response.get_header("X-Media-Path") @@ -28,7 +28,7 @@ class Api::V1::StreamControllerTest < ActionDispatch::IntegrationTest end test "should respond file data when set nginx send file header" do - stub_env("NGINX_SENDFILE", "true") do + with_env("NGINX_SENDFILE" => "true") do get new_api_v1_stream_url(song_id: songs(:mp3_sample).id) assert_equal binary_data(file_fixture("artist1_album2.mp3")), response.body end diff --git a/test/controllers/api/v1/transcoded_stream_controller_test.rb b/test/controllers/api/v1/transcoded_stream_controller_test.rb index e0b2a708..29f6ec8d 100644 --- a/test/controllers/api/v1/transcoded_stream_controller_test.rb +++ b/test/controllers/api/v1/transcoded_stream_controller_test.rb @@ -51,7 +51,7 @@ class Api::V1::TranscodedStreamControllerTest < ActionDispatch::IntegrationTest get new_api_v1_transcoded_stream_url(song_id: songs(:flac_sample).id) assert_response :success - stub_env("NGINX_SENDFILE", "true") do + with_env("NGINX_SENDFILE" => "true") do get new_api_v1_transcoded_stream_url(song_id: songs(:flac_sample).id) assert_equal "/private_cache_media/2/ZmxhY19zYW1wbGVfbWQ1X2hhc2g=_128.mp3", @response.get_header("X-Accel-Redirect") assert_equal "audio/mpeg", @response.get_header("Content-Type") diff --git a/test/lib/black_candy/config_test.rb b/test/lib/black_candy/config_test.rb new file mode 100644 index 00000000..8d1f2d75 --- /dev/null +++ b/test/lib/black_candy/config_test.rb @@ -0,0 +1,123 @@ +# frozen_string_literal: true + +require "test_helper" + +class BlackCandy::ConfigTest < ActiveSupport::TestCase + test "should get value from ENV" do + assert_nil BlackCandy::Config.redis_url + + with_env("REDIS_URL" => "test_redis_url") do + assert_equal "test_redis_url", BlackCandy::Config.redis_url + end + end + + test "should get default value when can not find value from ENV" do + assert_nil ENV["DATABASE_ADAPTER"] + assert_equal "sqlite", BlackCandy::Config.database_adapter + end + + test "should get nginx_sendfile value as a boolean" do + assert_nil ENV["NGINX_SENDFILE"] + assert_not BlackCandy::Config.nginx_sendfile? + + with_env("NGINX_SENDFILE" => "true") do + assert BlackCandy::Config.nginx_sendfile? + end + end + + test "should get embedded_sidekiq_concurrency value as an integer" do + assert_nil ENV["EMBEDDED_SIDEKIQ_CONCURRENCY"] + assert_equal 2, BlackCandy::Config.embedded_sidekiq_concurrency + + with_env("EMBEDDED_SIDEKIQ_CONCURRENCY" => "1") do + assert_equal 1, BlackCandy::Config.embedded_sidekiq_concurrency + end + end + + test "should use redis_url as default value when redis_cache_url did not set" do + assert_nil ENV["REDIS_URL"] + assert_nil ENV["REDIS_CACHE_URL"] + assert_nil BlackCandy::Config.redis_cache_url + + with_env("REDIS_URL" => "test_redis_url") do + assert_equal "test_redis_url", BlackCandy::Config.redis_cache_url + end + + with_env("REDIS_URL" => "test_redis_url", "REDIS_CACHE_URL" => "test_redis_cache_url") do + assert_equal "test_redis_cache_url", BlackCandy::Config.redis_cache_url + end + end + + test "should use redis_url as default value when redis_cable_url did not set" do + assert_nil ENV["REDIS_URL"] + assert_nil ENV["REDIS_CABLE_UR"] + assert_nil BlackCandy::Config.redis_cable_url + + with_env("REDIS_URL" => "test_redis_url") do + assert_equal "test_redis_url", BlackCandy::Config.redis_cable_url + end + + with_env("REDIS_URL" => "test_redis_url", "REDIS_CABLE_URL" => "test_redis_cable_url") do + assert_equal "test_redis_cable_url", BlackCandy::Config.redis_cable_url + end + end + + test "should use redis_url as default value when redis_sidekiq_url did not set" do + assert_nil ENV["REDIS_URL"] + assert_nil ENV["REDIS_SIDEKIQ_URL"] + assert_nil BlackCandy::Config.redis_sidekiq_url + + with_env("REDIS_URL" => "test_redis_url") do + assert_equal "test_redis_url", BlackCandy::Config.redis_sidekiq_url + end + + with_env("REDIS_URL" => "test_redis_url", "REDIS_SIDEKIQ_URL" => "test_redis_sidekiq_url") do + assert_equal "test_redis_sidekiq_url", BlackCandy::Config.redis_sidekiq_url + end + end + + test "should raise error when embedded_sidekiq_concurrency is more than 2" do + with_env("EMBEDDED_SIDEKIQ_CONCURRENCY" => "3") do + assert_raises(BlackCandy::Config::ValidationError) do + BlackCandy::Config.embedded_sidekiq_concurrency + end + end + end + + test "should raise error when database_adapter is not supported" do + with_env("DATABASE_ADAPTER" => "invalid_adapter") do + assert_raises(BlackCandy::Config::ValidationError) do + BlackCandy::Config.database_adapter + end + end + end + + test "should raise error when database_adapter is postgresql but database_url is not set" do + assert_nil ENV["DATABASE_URL"] + + with_env("DATABASE_ADAPTER" => "postgresql") do + assert_raises(BlackCandy::Config::ValidationError) do + BlackCandy::Config.database_adapter + end + end + + with_env("DATABASE_ADAPTER" => "postgresql", "DATABASE_URL" => "database_url") do + assert_equal "postgresql", BlackCandy::Config.database_adapter + end + end + + test "when embedded sidekiq is enabled puma threads plus sidekiq concurrency should never be greater than 5" do + assert_not BlackCandy::Config.embedded_sidekiq? + assert_equal 5, BlackCandy::Config.puma_max_threads_count + + with_env("EMBEDDED_SIDEKIQ" => "true", "EMBEDDED_SIDEKIQ_CONCURRENCY" => "2") do + assert_equal 3, BlackCandy::Config.puma_max_threads_count + assert_equal 3, BlackCandy::Config.puma_min_threads_count + end + + with_env("EMBEDDED_SIDEKIQ" => "true", "EMBEDDED_SIDEKIQ_CONCURRENCY" => "1") do + assert_equal 4, BlackCandy::Config.puma_max_threads_count + assert_equal 4, BlackCandy::Config.puma_min_threads_count + end + end +end diff --git a/test/models/setting_test.rb b/test/models/setting_test.rb index 3dc30b61..d584f336 100644 --- a/test/models/setting_test.rb +++ b/test/models/setting_test.rb @@ -8,10 +8,10 @@ class SettingTest < ActiveSupport::TestCase end test "should get env default value when setting value did not set" do - ENV["MEDIA_PATH"] = "/test_media_path" - - assert_nil Setting.instance.values&.[]("media_path") - assert_equal "/test_media_path", Setting.media_path + with_env("MEDIA_PATH" => "/test_media_path") do + assert_nil Setting.instance.values&.[]("media_path") + assert_equal "/test_media_path", Setting.media_path + end end test "should get singleton global setting" do diff --git a/test/test_helper.rb b/test/test_helper.rb index cb926dd0..9f55af30 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -98,11 +98,18 @@ def with_forgery_protection ActionController::Base.allow_forgery_protection = old end - def stub_env(name, value) - old_value = ENV[name] - ENV[name] = value + def with_env(envs = {}) + old_value = {} + + envs.each do |key, value| + old_value[key] = ENV[key] + ENV[key] = value + end + yield ensure - ENV[name] = old_value + old_value.each do |key, value| + ENV[key] = value + end end end From 5d3e48b1e4eb34581778a23f4916ca8ff4729466 Mon Sep 17 00:00:00 2001 From: aidewoode Date: Fri, 25 Nov 2022 13:50:09 +0800 Subject: [PATCH 4/4] Fix some fragile system tests --- test/system/albums_test.rb | 4 ++-- test/system/artists_test.rb | 4 ++-- test/system/setting_test.rb | 4 ---- test/system/songs_test.rb | 4 ++-- 4 files changed, 6 insertions(+), 10 deletions(-) diff --git a/test/system/albums_test.rb b/test/system/albums_test.rb index a6e015be..e183c523 100644 --- a/test/system/albums_test.rb +++ b/test/system/albums_test.rb @@ -15,13 +15,13 @@ class AlbumsSystemTest < ApplicationSystemTestCase test "show albums" do visit albums_url - assert_selector(:test_id, "album_card", count: Pagy::DEFAULT[:items]) + assert_selector(:test_id, "album_card", count: Pagy::DEFAULT[:items], visible: :all) end test "show next page albums when scroll to the bottom" do visit albums_url scroll_to :bottom - assert_selector(:test_id, "album_card", count: Pagy::DEFAULT[:items] * 2) + assert_selector(:test_id, "album_card", count: Pagy::DEFAULT[:items] * 2, visible: :all) end end diff --git a/test/system/artists_test.rb b/test/system/artists_test.rb index 4c1d8df1..4a23f98e 100644 --- a/test/system/artists_test.rb +++ b/test/system/artists_test.rb @@ -15,13 +15,13 @@ class ArtistsSystemTest < ApplicationSystemTestCase test "show artists" do visit artists_url - assert_selector(:test_id, "artist_card", count: Pagy::DEFAULT[:items]) + assert_selector(:test_id, "artist_card", count: Pagy::DEFAULT[:items], visible: :all) end test "show next page artists when scroll to the bottom" do visit artists_url scroll_to :bottom - assert_selector(:test_id, "artist_card", count: Pagy::DEFAULT[:items] * 2) + assert_selector(:test_id, "artist_card", count: Pagy::DEFAULT[:items] * 2, visible: :all) end end diff --git a/test/system/setting_test.rb b/test/system/setting_test.rb index cc9b0879..c4a19b92 100644 --- a/test/system/setting_test.rb +++ b/test/system/setting_test.rb @@ -17,16 +17,12 @@ class SettingSystemTest < ApplicationSystemTestCase end test "update media path" do - Media.syncing = false - visit setting_url fill_in("setting_media_path", with: Rails.root.join("test/fixtures/files").to_s) click_on("Sync") - assert_equal "Syncing...", find(:test_id, "media_sync_button").value assert_text("Media sync completed") - assert_equal "Sync", find(:test_id, "media_sync_button").value end test "update discogs token" do diff --git a/test/system/songs_test.rb b/test/system/songs_test.rb index e00be6bf..df374e24 100644 --- a/test/system/songs_test.rb +++ b/test/system/songs_test.rb @@ -22,14 +22,14 @@ class SongsSystemTest < ApplicationSystemTestCase test "show songs" do visit songs_url - assert_selector(:test_id, "song_item", count: Pagy::DEFAULT[:items]) + assert_selector(:test_id, "song_item", count: Pagy::DEFAULT[:items], visible: :all) end test "show next page songs when scroll to the bottom" do visit songs_url scroll_to :bottom - assert_selector(:test_id, "song_item", count: Pagy::DEFAULT[:items] * 2) + assert_selector(:test_id, "song_item", count: Pagy::DEFAULT[:items] * 2, visible: :all) end test "play song from songs list" do