Merge pull request #314 from blackcandy-org/dev

Fix #313

app/controllers/albums_controller.rb

@@ -1,7 +1,7 @@
 # frozen_string_literal: true
 
 class AlbumsController < ApplicationController
-  include Playable
+  include PlayableConcern
 
   before_action :require_admin, only: [:update]
   before_action :find_album, except: [:index]

app/controllers/api/v1/api_controller.rb

@@ -7,14 +7,8 @@ class ApiController < ApplicationController
 
       private
 
-      # If user already has logged in then authenticate with current session,
-      # otherwise authenticate with api token.
       def find_current_user
-        Current.user = UserSession.find&.user
-
-        return if logged_in?
-
-        authenticate_with_http_token do |token, options|
+        authenticate_with_http_token do |token, _|
           user = User.find_by(api_token: token)
           return unless user.present?
 

app/controllers/api/v1/stream_controller.rb

@@ -3,45 +3,7 @@
 module Api
   module V1
     class StreamController < ApiController
-      before_action :find_stream
-
-      def new
-        send_local_file @stream.file_path
-      end
-
-      private
-
-      def set_nginx_header
-        # Let nginx can get value of media_path dynamically in the nginx config,
-        # when use X-Accel-Redirect header to send file.
-        response.headers["X-Media-Path"] = Setting.media_path
-        response.headers["X-Accel-Redirect"] = File.join("/private_media", @stream.file_path.sub(File.expand_path(Setting.media_path), ""))
-      end
-
-      def find_stream
-        song = Song.find(params[:song_id])
-        @stream = Stream.new(song)
-      end
-
-      def send_local_file(file_path)
-        if BlackCandy::Config.nginx_sendfile?
-          set_nginx_header
-
-          send_file file_path
-          return
-        end
-
-        # Use Rack::File to support HTTP range without nginx. see https://github.com/rails/rails/issues/32193
-        Rack::File.new(nil).serving(request, file_path).tap do |(status, headers, body)|
-          self.status = status
-          self.response_body = body
-
-          headers.each { |name, value| response.headers[name] = value }
-
-          response.headers["Content-Type"] = Mime[@stream.format]
-          response.headers["Content-Disposition"] = "attachment"
-        end
-      end
+      include StreamConcern
     end
   end
 end

app/controllers/api/v1/transcoded_stream_controller.rb

@@ -2,49 +2,8 @@
 
 module Api
   module V1
-    class TranscodedStreamController < StreamController
-      before_action :find_cache
-
-      # Similar to send_file in rails, but let response_body to be a stream object.
-      # The instance of Stream can respond to each() method. So the download can be streamed,
-      # instead of read whole data into memory.
-      def new
-        # Because the module ActionController::Live contains methods that can block send file.
-        # So can't simply include ActionController::Live in class, otherwise include this module only on this method.
-        self.class.send(:include, ActionController::Live)
-
-        response.headers["Content-Type"] = Mime[Stream::TRANSCODE_FORMAT]
-
-        send_stream(filename: "#{@stream.name}.mp3") do |stream_response|
-          File.open(@stream.transcode_cache_file_path, "w") do |file|
-            @stream.each do |data|
-              stream_response.write data
-              file.write data
-            end
-          end
-        end
-      end
-
-      private
-
-      def set_nginx_header
-        response.headers["X-Accel-Redirect"] = File.join("/private_cache_media", @stream.transcode_cache_file_path.sub(Stream::TRANSCODE_CACHE_DIRECTORY.to_s, ""))
-      end
-
-      def find_cache
-        return unless valid_cache?
-        send_local_file @stream.transcode_cache_file_path
-      end
-
-      def valid_cache?
-        return unless File.exist?(@stream.transcode_cache_file_path)
-
-        # Compare duration of cache file and original file to check integrity of cache file.
-        # Because the different format of the file, the duration will have a little difference,
-        # so the duration difference in two seconds are considered no problem.
-        cache_file_tag = WahWah.open(@stream.transcode_cache_file_path)
-        (@stream.duration - cache_file_tag.duration).abs <= 2
-      end
+    class TranscodedStreamController < ApiController
+      include TranscodedStreamConcern
     end
   end
 end

app/controllers/application_controller.rb

@@ -116,4 +116,24 @@ def turbo_android?
   def render_json_error(error, status)
     render json: {type: error.type, message: error.message}, status: status
   end
+
+  def send_local_file(file_path, format, nginx_headers: {})
+    if BlackCandy::Config.nginx_sendfile?
+      nginx_headers.each { |name, value| response.headers[name] = value }
+      send_file file_path
+
+      return
+    end
+
+    # Use Rack::File to support HTTP range without nginx. see https://github.com/rails/rails/issues/32193
+    Rack::File.new(nil).serving(request, file_path).tap do |(status, headers, body)|
+      self.status = status
+      self.response_body = body
+
+      headers.each { |name, value| response.headers[name] = value }
+
+      response.headers["Content-Type"] = Mime[format]
+      response.headers["Content-Disposition"] = "attachment"
+    end
+  end
 end

app/controllers/concerns/playable.rb → app/controllers/concerns/playable_concern.rb

@@ -1,6 +1,6 @@
 # frozen_string_literal: true
 
-module Playable
+module PlayableConcern
   extend ActiveSupport::Concern
 
   included do

app/controllers/concerns/stream_concern.rb

@@ -0,0 +1,24 @@
+# frozen_string_literal: true
+
+module StreamConcern
+  extend ActiveSupport::Concern
+
+  included do
+    before_action :find_stream
+  end
+
+  def new
+    send_local_file @stream.file_path, @stream.format, nginx_headers: {
+      # Let nginx can get value of media_path dynamically in the nginx config
+      # when use X-Accel-Redirect header to send file.
+      "X-Media-Path" => Setting.media_path,
+      "X-Accel-Redirect" => File.join("/private_media", @stream.file_path.sub(File.expand_path(Setting.media_path), ""))
+    }
+  end
+
+  private
+
+  def find_stream
+    @stream = Stream.new(Song.find(params[:song_id]))
+  end
+end

app/controllers/concerns/transcoded_stream_concern.rb

@@ -0,0 +1,53 @@
+# frozen_string_literal: true
+
+module TranscodedStreamConcern
+  extend ActiveSupport::Concern
+
+  included do
+    before_action :find_stream
+    before_action :find_cache
+  end
+
+  # Similar to send_file in rails, but let response_body to be a stream object.
+  # The instance of Stream can respond to each() method. So the download can be streamed,
+  # instead of read whole data into memory.
+  def new
+    # Because the module ActionController::Live contains methods that can block send file.
+    # So can't simply include ActionController::Live in class, otherwise include this module only on this method.
+    self.class.send(:include, ActionController::Live)
+
+    response.headers["Content-Type"] = Mime[Stream::TRANSCODE_FORMAT]
+
+    send_stream(filename: "#{@stream.name}.mp3") do |stream_response|
+      File.open(@stream.transcode_cache_file_path, "w") do |file|
+        @stream.each do |data|
+          stream_response.write data
+          file.write data
+        end
+      end
+    end
+  end
+
+  private
+
+  def find_stream
+    @stream = Stream.new(Song.find(params[:song_id]))
+  end
+
+  def find_cache
+    return unless valid_cache?
+    send_local_file @stream.transcode_cache_file_path, @stream.format, nginx_headers: {
+      "X-Accel-Redirect" => File.join("/private_cache_media", @stream.transcode_cache_file_path.sub(Stream::TRANSCODE_CACHE_DIRECTORY.to_s, ""))
+    }
+  end
+
+  def valid_cache?
+    return unless File.exist?(@stream.transcode_cache_file_path)
+
+    # Compare duration of cache file and original file to check integrity of cache file.
+    # Because the different format of the file, the duration will have a little difference,
+    # so the duration difference in two seconds are considered no problem.
+    cache_file_tag = WahWah.open(@stream.transcode_cache_file_path)
+    (@stream.duration - cache_file_tag.duration).abs <= 2
+  end
+end

app/controllers/playlists/songs_controller.rb

@@ -4,7 +4,7 @@ class Playlists::SongsController < ApplicationController
   before_action :find_playlist
   before_action :find_song, only: [:create, :destroy]
 
-  include Playable
+  include PlayableConcern
 
   def show
     @pagy, @songs = pagy(@playlist.songs.includes(:artist))

app/controllers/stream_controller.rb

@@ -0,0 +1,5 @@
+# frozen_string_literal: true
+
+class StreamController < ApplicationController
+  include StreamConcern
+end

app/controllers/transcoded_stream_controller.rb

@@ -0,0 +1,5 @@
+# frozen_string_literal: true
+
+class TranscodedStreamController < ApplicationController
+  include TranscodedStreamConcern
+end

app/helpers/song_helper.rb

@@ -1,10 +1,13 @@
 # frozen_string_literal: true
 
 module SongHelper
-  def song_json_builder(song)
+  def song_json_builder(song, for_api: false)
+    stream_url = for_api ? new_api_v1_stream_url(song_id: song.id) : new_stream_url(song_id: song.id)
+    transcoded_stream_url = for_api ? new_api_v1_transcoded_stream_url(song_id: song.id) : new_transcoded_stream_url(song_id: song.id)
+
     Jbuilder.new do |json|
       json.call(song, :id, :name, :duration)
-      json.url need_transcode?(song) ? new_api_v1_transcoded_stream_url(song_id: song.id) : new_api_v1_stream_url(song_id: song.id)
+      json.url need_transcode?(song) ? transcoded_stream_url : stream_url
       json.album_name song.album.title
       json.artist_name song.artist.title
       json.is_favorited song.is_favorited.nil? ? Current.user.favorited?(song) : song.is_favorited

app/models/album.rb

@@ -1,10 +1,10 @@
 # frozen_string_literal: true
 
 class Album < ApplicationRecord
-  include Searchable
-  include Imageable
-  include Filterable
-  include Sortable
+  include SearchableConcern
+  include ImageableConcern
+  include FilterableConcern
+  include SortableConcern
 
   validates :name, uniqueness: {scope: :artist}
 

app/models/artist.rb

@@ -1,9 +1,9 @@
 # frozen_string_literal: true
 
 class Artist < ApplicationRecord
-  include Searchable
-  include Imageable
-  include Sortable
+  include SearchableConcern
+  include ImageableConcern
+  include SortableConcern
 
   has_many :albums, dependent: :destroy
   has_many :songs

app/models/concerns/filterable.rb → app/models/concerns/filterable_concern.rb

@@ -1,6 +1,6 @@
 # frozen_string_literal: true
 
-module Filterable
+module FilterableConcern
   extend ActiveSupport::Concern
 
   included do

app/models/concerns/global_setting.rb → app/models/concerns/global_setting_concern.rb

@@ -1,6 +1,6 @@
 # frozen_string_literal: true
 
-module GlobalSetting
+module GlobalSettingConcern
   extend ActiveSupport::Concern
 
   included do
@@ -22,12 +22,12 @@ def has_setting(setting, type: :string, default: nil)
       store :values, accessors: setting
 
       define_method("#{setting}=") do |value|
-        setting_value = ScopedSetting.convert_setting_value(type, value)
+        setting_value = ScopedSettingConcern.convert_setting_value(type, value)
         super(setting_value)
       end
 
       define_method(setting) do
-        ScopedSetting.convert_setting_value(type, super())
+        ScopedSettingConcern.convert_setting_value(type, super())
       end
 
       define_singleton_method(setting) do

app/models/concerns/imageable.rb → app/models/concerns/imageable_concern.rb

@@ -1,4 +1,4 @@
-module Imageable
+module ImageableConcern
   extend ActiveSupport::Concern
 
   included do

app/models/concerns/scoped_setting.rb → app/models/concerns/scoped_setting_concern.rb

@@ -1,6 +1,6 @@
 # frozen_string_literal: true
 
-module ScopedSetting
+module ScopedSettingConcern
   extend ActiveSupport::Concern
 
   included do
@@ -14,7 +14,7 @@ def has_setting(setting, type: :string, default: nil)
       store :settings, accessors: setting
 
       define_method(setting) do
-        setting_value = ScopedSetting.convert_setting_value(type, super())
+        setting_value = ScopedSettingConcern.convert_setting_value(type, super())
 
         setting_value || default
       end

app/models/concerns/searchable.rb → app/models/concerns/searchable_concern.rb

@@ -1,6 +1,6 @@
 # frozen_string_literal: true
 
-module Searchable
+module SearchableConcern
   extend ActiveSupport::Concern
 
   class_methods do

app/models/concerns/sortable.rb → app/models/concerns/sortable_concern.rb

@@ -1,6 +1,6 @@
 # frozen_string_literal: true
 
-module Sortable
+module SortableConcern
   extend ActiveSupport::Concern
 
   included do

app/models/playlist.rb

@@ -1,8 +1,8 @@
 # frozen_string_literal: true
 
 class Playlist < ApplicationRecord
-  include Searchable
-  include Sortable
+  include SearchableConcern
+  include SortableConcern
 
   validates :name, presence: true, if: :require_name?
 

app/models/setting.rb

@@ -1,7 +1,7 @@
 # frozen_string_literal: true
 
 class Setting < ApplicationRecord
-  include GlobalSetting
+  include GlobalSettingConcern
 
   AVAILABLE_BITRATE_OPTIONS = [128, 192, 320].freeze