Refactor of API timeline actions (#3263)

- Increase coverage to exercise all parts of each action
- Move into namespace to share common code
- Misc refactor of each action for smaller methods, simpler code

5 files changed, 166 insertions, 61 deletions

diff --git a/app/controllers/api/v1/timelines/base_controller.rb b/app/controllers/api/v1/timelines/base_controller.rb
new file mode 100644
index 000000000..4eb29e74a
--- /dev/null
+++ b/app/controllers/api/v1/timelines/base_controller.rb
@@ -0,0 +1,30 @@
+# frozen_string_literal: true
+
+module Api::V1::Timelines
+  class BaseController < ApiController
+    respond_to :json
+    after_action :insert_pagination_headers, unless: -> { @statuses.empty? }
+
+    private
+
+    def cache_collection(raw)
+      super(raw, Status)
+    end
+
+    def pagination_params(core_params)
+      params.permit(:local, :limit).merge(core_params)
+    end
+
+    def insert_pagination_headers
+      set_pagination_headers(next_path, prev_path)
+    end
+
+    def next_path
+      raise 'Override in child controllers'
+    end
+
+    def prev_path
+      raise 'Override in child controllers'
+    end
+  end
+end
diff --git a/app/controllers/api/v1/timelines/home_controller.rb b/app/controllers/api/v1/timelines/home_controller.rb
new file mode 100644
index 000000000..33ff48b39
--- /dev/null
+++ b/app/controllers/api/v1/timelines/home_controller.rb
@@ -0,0 +1,44 @@
+# frozen_string_literal: true
+
+module Api::V1::Timelines
+  class HomeController < BaseController
+    before_action -> { doorkeeper_authorize! :read }, only: [:show]
+    before_action :require_user!, only: [:show]
+
+    def show
+      @statuses = load_statuses
+    end
+
+    private
+
+    def load_statuses
+      cached_home_statuses.tap do |statuses|
+        set_maps(statuses)
+      end
+    end
+
+    def cached_home_statuses
+      cache_collection home_statuses
+    end
+
+    def home_statuses
+      account_home_feed.get(
+        limit_param(DEFAULT_STATUSES_LIMIT),
+        params[:max_id],
+        params[:since_id]
+      )
+    end
+
+    def account_home_feed
+      Feed.new(:home, current_account)
+    end
+
+    def next_path
+      api_v1_timelines_home_url pagination_params(max_id: @statuses.last.id)
+    end
+
+    def prev_path
+      api_v1_timelines_home_url pagination_params(since_id: @statuses.first.id)
+    end
+  end
+end
diff --git a/app/controllers/api/v1/timelines/public_controller.rb b/app/controllers/api/v1/timelines/public_controller.rb
new file mode 100644
index 000000000..644c7a6f1
--- /dev/null
+++ b/app/controllers/api/v1/timelines/public_controller.rb
@@ -0,0 +1,41 @@
+# frozen_string_literal: true
+
+module Api::V1::Timelines
+  class PublicController < BaseController
+    def show
+      @statuses = load_statuses
+    end
+
+    private
+
+    def load_statuses
+      cached_public_statuses.tap do |statuses|
+        set_maps(statuses)
+      end
+    end
+
+    def cached_public_statuses
+      cache_collection public_statuses
+    end
+
+    def public_statuses
+      public_timeline_statuses.paginate_by_max_id(
+        limit_param(DEFAULT_STATUSES_LIMIT),
+        params[:max_id],
+        params[:since_id]
+      )
+    end
+
+    def public_timeline_statuses
+      Status.as_public_timeline(current_account, params[:local])
+    end
+
+    def next_path
+      api_v1_timelines_public_url pagination_params(max_id: @statuses.last.id)
+    end
+
+    def prev_path
+      api_v1_timelines_public_url pagination_params(since_id: @statuses.first.id)
+    end
+  end
+end
diff --git a/app/controllers/api/v1/timelines/tag_controller.rb b/app/controllers/api/v1/timelines/tag_controller.rb
new file mode 100644
index 000000000..818f49d3d
--- /dev/null
+++ b/app/controllers/api/v1/timelines/tag_controller.rb
@@ -0,0 +1,51 @@
+# frozen_string_literal: true
+
+module Api::V1::Timelines
+  class TagController < BaseController
+    before_action :load_tag
+
+    def show
+      @statuses = load_statuses
+    end
+
+    private
+
+    def load_tag
+      @tag = Tag.find_by(name: params[:id].downcase)
+    end
+
+    def load_statuses
+      cached_tagged_statuses.tap do |statuses|
+        set_maps(statuses)
+      end
+    end
+
+    def cached_tagged_statuses
+      cache_collection tagged_statuses
+    end
+
+    def tagged_statuses
+      if @tag.nil?
+        []
+      else
+        tag_timeline_statuses.paginate_by_max_id(
+          limit_param(DEFAULT_STATUSES_LIMIT),
+          params[:max_id],
+          params[:since_id]
+        )
+      end
+    end
+
+    def tag_timeline_statuses
+      Status.as_tag_timeline(@tag, current_account, params[:local])
+    end
+
+    def next_path
+      api_v1_timelines_tag_url params[:id], pagination_params(max_id: @statuses.last.id)
+    end
+
+    def prev_path
+      api_v1_timelines_tag_url params[:id], pagination_params(since_id: @statuses.first.id)
+    end
+  end
+end
diff --git a/app/controllers/api/v1/timelines_controller.rb b/app/controllers/api/v1/timelines_controller.rb
deleted file mode 100644
index 27dc3d0ef..000000000
--- a/app/controllers/api/v1/timelines_controller.rb
+++ /dev/null
@@ -1,61 +0,0 @@
-# frozen_string_literal: true
-
-class Api::V1::TimelinesController < ApiController
-  before_action -> { doorkeeper_authorize! :read }, only: [:home]
-  before_action :require_user!, only: [:home]
-
-  respond_to :json
-
-  def home
-    @statuses = Feed.new(:home, current_account).get(limit_param(DEFAULT_STATUSES_LIMIT), params[:max_id], params[:since_id])
-    @statuses = cache_collection(@statuses)
-
-    set_maps(@statuses)
-
-    next_path = api_v1_home_timeline_url(pagination_params(max_id: @statuses.last.id))    unless @statuses.empty?
-    prev_path = api_v1_home_timeline_url(pagination_params(since_id: @statuses.first.id)) unless @statuses.empty?
-
-    set_pagination_headers(next_path, prev_path)
-
-    render :index
-  end
-
-  def public
-    @statuses = Status.as_public_timeline(current_account, params[:local]).paginate_by_max_id(limit_param(DEFAULT_STATUSES_LIMIT), params[:max_id], params[:since_id])
-    @statuses = cache_collection(@statuses)
-
-    set_maps(@statuses)
-
-    next_path = api_v1_public_timeline_url(pagination_params(max_id: @statuses.last.id))    unless @statuses.empty?
-    prev_path = api_v1_public_timeline_url(pagination_params(since_id: @statuses.first.id)) unless @statuses.empty?
-
-    set_pagination_headers(next_path, prev_path)
-
-    render :index
-  end
-
-  def tag
-    @tag      = Tag.find_by(name: params[:id].downcase)
-    @statuses = @tag.nil? ? [] : Status.as_tag_timeline(@tag, current_account, params[:local]).paginate_by_max_id(limit_param(DEFAULT_STATUSES_LIMIT), params[:max_id], params[:since_id])
-    @statuses = cache_collection(@statuses)
-
-    set_maps(@statuses)
-
-    next_path = api_v1_hashtag_timeline_url(params[:id], pagination_params(max_id: @statuses.last.id))    unless @statuses.empty?
-    prev_path = api_v1_hashtag_timeline_url(params[:id], pagination_params(since_id: @statuses.first.id)) unless @statuses.empty?
-
-    set_pagination_headers(next_path, prev_path)
-
-    render :index
-  end
-
-  private
-
-  def cache_collection(raw)
-    super(raw, Status)
-  end
-
-  def pagination_params(core_params)
-    params.permit(:local, :limit).merge(core_params)
-  end
-end