about summary refs log tree commit diff
diff options
context:
space:
mode:
authorEugen Rochko <eugen@zeonfederated.com>2016-11-08 23:22:44 +0100
committerEugen Rochko <eugen@zeonfederated.com>2016-11-08 23:29:08 +0100
commit9aecc0f48a046e0a05b8ca69511f8b72756fb431 (patch)
tree636bb2399ec2f853af5ebd593af6c75e311c7dd7
parent86574ea5248219fa35ecb0748920df9ff1ce6110 (diff)
Move timelines API from statuses to its own controller, add a check for
resources that require a user context vs those that don't (such as public timeline)

/api/v1/statuses/public   -> /api/v1/timelines/public
/api/v1/statuses/home     -> /api/v1/timelines/home
/api/v1/statuses/mentions -> /api/v1/timelines/mentions
/api/v1/statuses/tag/:tag -> /api/v1/timelines/tag/:tag
-rw-r--r--app/assets/javascripts/components/actions/timelines.jsx4
-rw-r--r--app/controllers/api/v1/accounts_controller.rb3
-rw-r--r--app/controllers/api/v1/follows_controller.rb2
-rw-r--r--app/controllers/api/v1/media_controller.rb2
-rw-r--r--app/controllers/api/v1/statuses_controller.rb35
-rw-r--r--app/controllers/api/v1/timelines_controller.rb37
-rw-r--r--app/controllers/api_controller.rb18
-rw-r--r--app/models/status.rb26
-rw-r--r--app/views/api/v1/timelines/index.rabl2
-rw-r--r--config/routes.rb16
-rw-r--r--spec/controllers/api/v1/statuses_controller_spec.rb32
-rw-r--r--spec/controllers/api/v1/timelines_controller_spec.rb80
12 files changed, 170 insertions, 87 deletions
diff --git a/app/assets/javascripts/components/actions/timelines.jsx b/app/assets/javascripts/components/actions/timelines.jsx
index 1dd770848..0f23ca7fc 100644
--- a/app/assets/javascripts/components/actions/timelines.jsx
+++ b/app/assets/javascripts/components/actions/timelines.jsx
@@ -73,7 +73,7 @@ export function refreshTimeline(timeline, replace = false, id = null) {
       path = `${path}/${id}`
     }
 
-    api(getState).get(`/api/v1/statuses/${path}${params}`).then(function (response) {
+    api(getState).get(`/api/v1/timelines/${path}${params}`).then(function (response) {
       dispatch(refreshTimelineSuccess(timeline, response.data, replace));
     }).catch(function (error) {
       dispatch(refreshTimelineFail(timeline, error));
@@ -101,7 +101,7 @@ export function expandTimeline(timeline, id = null) {
       path = `${path}/${id}`
     }
 
-    api(getState).get(`/api/v1/statuses/${path}?max_id=${lastId}`).then(response => {
+    api(getState).get(`/api/v1/timelines/${path}?max_id=${lastId}`).then(response => {
       dispatch(expandTimelineSuccess(timeline, response.data));
     }).catch(error => {
       dispatch(expandTimelineFail(timeline, error));
diff --git a/app/controllers/api/v1/accounts_controller.rb b/app/controllers/api/v1/accounts_controller.rb
index bb06ddac9..4140439a8 100644
--- a/app/controllers/api/v1/accounts_controller.rb
+++ b/app/controllers/api/v1/accounts_controller.rb
@@ -1,8 +1,9 @@
 class Api::V1::AccountsController < ApiController
   before_action -> { doorkeeper_authorize! :read }, except: [:follow, :unfollow, :block, :unblock]
   before_action -> { doorkeeper_authorize! :follow }, only: [:follow, :unfollow, :block, :unblock]
-
+  before_action :require_user!, except: [:show, :following, :followers, :statuses]
   before_action :set_account, except: [:verify_credentials, :suggestions]
+
   respond_to    :json
 
   def show
diff --git a/app/controllers/api/v1/follows_controller.rb b/app/controllers/api/v1/follows_controller.rb
index 526316531..80a5aedf2 100644
--- a/app/controllers/api/v1/follows_controller.rb
+++ b/app/controllers/api/v1/follows_controller.rb
@@ -1,5 +1,7 @@
 class Api::V1::FollowsController < ApiController
   before_action -> { doorkeeper_authorize! :follow }
+  before_action :require_user!
+
   respond_to    :json
 
   def create
diff --git a/app/controllers/api/v1/media_controller.rb b/app/controllers/api/v1/media_controller.rb
index dffc797fe..ab216f9c9 100644
--- a/app/controllers/api/v1/media_controller.rb
+++ b/app/controllers/api/v1/media_controller.rb
@@ -1,5 +1,7 @@
 class Api::V1::MediaController < ApiController
   before_action -> { doorkeeper_authorize! :write }
+  before_action :require_user!
+
   respond_to    :json
 
   def create
diff --git a/app/controllers/api/v1/statuses_controller.rb b/app/controllers/api/v1/statuses_controller.rb
index 0a823e3e6..51a044a6c 100644
--- a/app/controllers/api/v1/statuses_controller.rb
+++ b/app/controllers/api/v1/statuses_controller.rb
@@ -1,8 +1,8 @@
 class Api::V1::StatusesController < ApiController
   before_action -> { doorkeeper_authorize! :read }, except: [:create, :destroy, :reblog, :unreblog, :favourite, :unfavourite]
   before_action -> { doorkeeper_authorize! :write }, only:  [:create, :destroy, :reblog, :unreblog, :favourite, :unfavourite]
-
-  before_action :set_status, only: [:show, :context, :reblogged_by, :favourited_by]
+  before_action :require_user!, except: [:show, :context, :reblogged_by, :favourited_by]
+  before_action :set_status, only:      [:show, :context, :reblogged_by, :favourited_by]
 
   respond_to :json
 
@@ -56,37 +56,6 @@ class Api::V1::StatusesController < ApiController
     render action: :show
   end
 
-  def home
-    @statuses = Feed.new(:home, current_user.account).get(20, params[:max_id], params[:since_id]).to_a
-    set_maps(@statuses)
-    render action: :index
-  end
-
-  def mentions
-    @statuses = Feed.new(:mentions, current_user.account).get(20, params[:max_id], params[:since_id]).to_a
-    set_maps(@statuses)
-    render action: :index
-  end
-
-  def public
-    @statuses = Status.as_public_timeline(current_user.account).paginate_by_max_id(20, params[:max_id], params[:since_id]).to_a
-    set_maps(@statuses)
-    render action: :index
-  end
-
-  def tag
-    @tag = Tag.find_by(name: params[:id].downcase)
-
-    if @tag.nil?
-      @statuses = []
-    else
-      @statuses = Status.as_tag_timeline(@tag, current_user.account).paginate_by_max_id(20, params[:max_id], params[:since_id]).to_a
-      set_maps(@statuses)
-    end
-
-    render action: :index
-  end
-
   private
 
   def set_status
diff --git a/app/controllers/api/v1/timelines_controller.rb b/app/controllers/api/v1/timelines_controller.rb
new file mode 100644
index 000000000..e5176dd4b
--- /dev/null
+++ b/app/controllers/api/v1/timelines_controller.rb
@@ -0,0 +1,37 @@
+class Api::V1::TimelinesController < ApiController
+  before_action -> { doorkeeper_authorize! :read }
+  before_action :require_user!, only: [:home, :mentions]
+
+  respond_to :json
+
+  def home
+    @statuses = Feed.new(:home, current_account).get(20, params[:max_id], params[:since_id]).to_a
+    set_maps(@statuses)
+    render action: :index
+  end
+
+  def mentions
+    @statuses = Feed.new(:mentions, current_account).get(20, params[:max_id], params[:since_id]).to_a
+    set_maps(@statuses)
+    render action: :index
+  end
+
+  def public
+    @statuses = Status.as_public_timeline(current_account).paginate_by_max_id(20, params[:max_id], params[:since_id]).to_a
+    set_maps(@statuses)
+    render action: :index
+  end
+
+  def tag
+    @tag = Tag.find_by(name: params[:id].downcase)
+
+    if @tag.nil?
+      @statuses = []
+    else
+      @statuses = Status.as_tag_timeline(@tag, current_account).paginate_by_max_id(20, params[:max_id], params[:since_id]).to_a
+      set_maps(@statuses)
+    end
+
+    render action: :index
+  end
+end
diff --git a/app/controllers/api_controller.rb b/app/controllers/api_controller.rb
index 273aaff85..db4035a96 100644
--- a/app/controllers/api_controller.rb
+++ b/app/controllers/api_controller.rb
@@ -60,6 +60,14 @@ class ApiController < ApplicationController
 
   def current_user
     super || current_resource_owner
+  rescue ActiveRecord::RecordNotFound
+    nil
+  end
+
+  def require_user!
+    current_resource_owner
+  rescue ActiveRecord::RecordNotFound
+    render json: { error: 'This method requires an authenticated user' }, status: 422
   end
 
   def render_empty
@@ -67,8 +75,14 @@ class ApiController < ApplicationController
   end
 
   def set_maps(statuses)
+    if current_account.nil?
+      @reblogs_map    = {}
+      @favourites_map = {}
+      return
+    end
+
     status_ids      = statuses.flat_map { |s| [s.id, s.reblog_of_id] }.compact.uniq
-    @reblogs_map    = Status.reblogs_map(status_ids, current_user.account)
-    @favourites_map = Status.favourites_map(status_ids, current_user.account)
+    @reblogs_map    = Status.reblogs_map(status_ids, current_account)
+    @favourites_map = Status.favourites_map(status_ids, current_account)
   end
 end
diff --git a/app/models/status.rb b/app/models/status.rb
index d68b7afa6..07aef26ee 100644
--- a/app/models/status.rb
+++ b/app/models/status.rb
@@ -95,23 +95,29 @@ class Status < ApplicationRecord
       where(id: Mention.where(account: account).pluck(:status_id)).with_includes.with_counters
     end
 
-    def as_public_timeline(account)
-      joins('LEFT OUTER JOIN statuses AS reblogs ON statuses.reblog_of_id = reblogs.id')
+    def as_public_timeline(account = nil)
+      query = joins('LEFT OUTER JOIN statuses AS reblogs ON statuses.reblog_of_id = reblogs.id')
         .joins('LEFT OUTER JOIN accounts ON statuses.account_id = accounts.id')
         .where('accounts.silenced = FALSE')
-        .where('(reblogs.account_id IS NULL OR reblogs.account_id NOT IN (SELECT target_account_id FROM blocks WHERE account_id = ?)) AND statuses.account_id NOT IN (SELECT target_account_id FROM blocks WHERE account_id = ?)', account.id, account.id)
-        .with_includes
-        .with_counters
+
+      unless account.nil?
+        query = query.where('(reblogs.account_id IS NULL OR reblogs.account_id NOT IN (SELECT target_account_id FROM blocks WHERE account_id = ?)) AND statuses.account_id NOT IN (SELECT target_account_id FROM blocks WHERE account_id = ?)', account.id, account.id)
+      end
+
+      query.with_includes.with_counters
     end
 
-    def as_tag_timeline(tag, account)
-      tag.statuses
+    def as_tag_timeline(tag, account = nil)
+      query = tag.statuses
         .joins('LEFT OUTER JOIN statuses AS reblogs ON statuses.reblog_of_id = reblogs.id')
         .joins('LEFT OUTER JOIN accounts ON statuses.account_id = accounts.id')
         .where('accounts.silenced = FALSE')
-        .where('(reblogs.account_id IS NULL OR reblogs.account_id NOT IN (SELECT target_account_id FROM blocks WHERE account_id = ?)) AND statuses.account_id NOT IN (SELECT target_account_id FROM blocks WHERE account_id = ?)', account.id, account.id)
-        .with_includes
-        .with_counters
+
+      unless account.nil?
+        query = query.where('(reblogs.account_id IS NULL OR reblogs.account_id NOT IN (SELECT target_account_id FROM blocks WHERE account_id = ?)) AND statuses.account_id NOT IN (SELECT target_account_id FROM blocks WHERE account_id = ?)', account.id, account.id)
+      end
+
+      query.with_includes.with_counters
     end
 
     def favourites_map(status_ids, account_id)
diff --git a/app/views/api/v1/timelines/index.rabl b/app/views/api/v1/timelines/index.rabl
new file mode 100644
index 000000000..0a0ed13c5
--- /dev/null
+++ b/app/views/api/v1/timelines/index.rabl
@@ -0,0 +1,2 @@
+collection @statuses
+extends('api/v1/statuses/show')
diff --git a/config/routes.rb b/config/routes.rb
index 0a20d1655..26b61b8e1 100644
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -55,13 +55,6 @@ Rails.application.routes.draw do
     # JSON / REST API
     namespace :v1 do
       resources :statuses, only: [:create, :show, :destroy] do
-        collection do
-          get :home
-          get :mentions
-          get :public
-          get '/tag/:id', action: :tag
-        end
-
         member do
           get :context
           get :reblogged_by
@@ -74,6 +67,15 @@ Rails.application.routes.draw do
         end
       end
 
+      resources :timelines, only: [] do
+        collection do
+          get :home
+          get :mentions
+          get :public
+          get '/tag/:id', action: :tag
+        end
+      end
+
       resources :follows,  only: [:create]
       resources :media,    only: [:create]
       resources :apps,     only: [:create]
diff --git a/spec/controllers/api/v1/statuses_controller_spec.rb b/spec/controllers/api/v1/statuses_controller_spec.rb
index 9f9bb0c4f..9b027daf8 100644
--- a/spec/controllers/api/v1/statuses_controller_spec.rb
+++ b/spec/controllers/api/v1/statuses_controller_spec.rb
@@ -59,38 +59,6 @@ RSpec.describe Api::V1::StatusesController, type: :controller do
     end
   end
 
-  describe 'GET #home' do
-    it 'returns http success' do
-      get :home
-      expect(response).to have_http_status(:success)
-    end
-  end
-
-  describe 'GET #mentions' do
-    it 'returns http success' do
-      get :mentions
-      expect(response).to have_http_status(:success)
-    end
-  end
-
-  describe 'GET #public' do
-    it 'returns http success' do
-      get :public
-      expect(response).to have_http_status(:success)
-    end
-  end
-
-  describe 'GET #tag' do
-    before do
-      post :create, params: { status: 'It is a #test' }
-    end
-
-    it 'returns http success' do
-      get :tag, params: { id: 'test' }
-      expect(response).to have_http_status(:success)
-    end
-  end
-
   describe 'POST #create' do
     before do
       post :create, params: { status: 'Hello world' }
diff --git a/spec/controllers/api/v1/timelines_controller_spec.rb b/spec/controllers/api/v1/timelines_controller_spec.rb
new file mode 100644
index 000000000..c94519ac5
--- /dev/null
+++ b/spec/controllers/api/v1/timelines_controller_spec.rb
@@ -0,0 +1,80 @@
+require 'rails_helper'
+
+RSpec.describe Api::V1::TimelinesController, type: :controller do
+  render_views
+
+  let(:user)  { Fabricate(:user, account: Fabricate(:account, username: 'alice')) }
+
+  before do
+    stub_request(:post, "https://pubsubhubbub.superfeedr.com/").to_return(:status => 200, :body => "", :headers => {})
+    allow(controller).to receive(:doorkeeper_token) { token }
+  end
+
+  context 'with a user context' do
+    let(:token) { double acceptable?: true, resource_owner_id: user.id }
+
+    describe 'GET #home' do
+      it 'returns http success' do
+        get :home
+        expect(response).to have_http_status(:success)
+      end
+    end
+
+    describe 'GET #mentions' do
+      it 'returns http success' do
+        get :mentions
+        expect(response).to have_http_status(:success)
+      end
+    end
+
+    describe 'GET #public' do
+      it 'returns http success' do
+        get :public
+        expect(response).to have_http_status(:success)
+      end
+    end
+
+    describe 'GET #tag' do
+      before do
+        PostStatusService.new.call(user.account, 'It is a #test')
+      end
+
+      it 'returns http success' do
+        get :tag, params: { id: 'test' }
+        expect(response).to have_http_status(:success)
+      end
+    end
+  end
+
+  context 'without a user context' do
+    let(:token) { double acceptable?: true, resource_owner_id: nil }
+
+    describe 'GET #home' do
+      it 'returns http unprocessable entity' do
+        get :home
+        expect(response).to have_http_status(:unprocessable_entity)
+      end
+    end
+
+    describe 'GET #mentions' do
+      it 'returns http unprocessable entity' do
+        get :mentions
+        expect(response).to have_http_status(:unprocessable_entity)
+      end
+    end
+
+    describe 'GET #public' do
+      it 'returns http success' do
+        get :public
+        expect(response).to have_http_status(:success)
+      end
+    end
+
+    describe 'GET #tag' do
+      it 'returns http success' do
+        get :tag, params: { id: 'test' }
+        expect(response).to have_http_status(:success)
+      end
+    end
+  end
+end