From b17b2f25acc4d0cd4284835f28364451cb2fcd88 Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Sat, 5 Jan 2019 11:27:58 +0100 Subject: Follow new account before unfollowing the old one during move (#9719) --- app/workers/unfollow_follow_worker.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'app/workers') diff --git a/app/workers/unfollow_follow_worker.rb b/app/workers/unfollow_follow_worker.rb index a2133bb8c..50d3bf034 100644 --- a/app/workers/unfollow_follow_worker.rb +++ b/app/workers/unfollow_follow_worker.rb @@ -10,9 +10,9 @@ class UnfollowFollowWorker old_target_account = Account.find(old_target_account_id) new_target_account = Account.find(new_target_account_id) - UnfollowService.new.call(follower_account, old_target_account) FollowService.new.call(follower_account, new_target_account) - rescue ActiveRecord::RecordNotFound + UnfollowService.new.call(follower_account, old_target_account) + rescue ActiveRecord::RecordNotFound, Mastodon::NotPermittedError true end end -- cgit From a49d43d1121ac10f96d5a9cbf78112c707e7a59e Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Sat, 5 Jan 2019 12:43:28 +0100 Subject: Add scheduled statuses (#9706) Fix #340 --- .../api/v1/scheduled_statuses_controller.rb | 77 ++++++++++ app/controllers/api/v1/statuses_controller.rb | 9 +- app/models/concerns/account_associations.rb | 1 + app/models/media_attachment.rb | 38 ++--- app/models/scheduled_status.rb | 39 +++++ .../rest/scheduled_status_serializer.rb | 11 ++ app/services/post_status_service.rb | 169 +++++++++++++++------ app/services/suspend_account_service.rb | 1 + app/workers/publish_scheduled_status_worker.rb | 24 +++ .../scheduler/scheduled_statuses_scheduler.rb | 19 +++ config/locales/en.yml | 4 + config/routes.rb | 1 + config/sidekiq.yml | 3 + .../20190103124649_create_scheduled_statuses.rb | 9 ++ ...add_scheduled_status_id_to_media_attachments.rb | 8 + db/schema.rb | 14 +- .../api/v1/conversations_controller_spec.rb | 2 +- .../api/v1/notifications_controller_spec.rb | 4 +- .../api/v1/timelines/home_controller_spec.rb | 2 +- .../api/v1/timelines/list_controller_spec.rb | 2 +- .../api/v1/timelines/public_controller_spec.rb | 4 +- .../api/v1/timelines/tag_controller_spec.rb | 2 +- spec/fabricators/scheduled_status_fabricator.rb | 4 + spec/lib/feed_manager_spec.rb | 6 +- spec/models/scheduled_status_spec.rb | 4 + .../services/batched_remove_status_service_spec.rb | 4 +- spec/services/post_status_service_spec.rb | 44 +++--- spec/services/remove_status_service_spec.rb | 2 +- .../publish_scheduled_status_worker_spec.rb | 23 +++ 29 files changed, 432 insertions(+), 98 deletions(-) create mode 100644 app/controllers/api/v1/scheduled_statuses_controller.rb create mode 100644 app/models/scheduled_status.rb create mode 100644 app/serializers/rest/scheduled_status_serializer.rb create mode 100644 app/workers/publish_scheduled_status_worker.rb create mode 100644 app/workers/scheduler/scheduled_statuses_scheduler.rb create mode 100644 db/migrate/20190103124649_create_scheduled_statuses.rb create mode 100644 db/migrate/20190103124754_add_scheduled_status_id_to_media_attachments.rb create mode 100644 spec/fabricators/scheduled_status_fabricator.rb create mode 100644 spec/models/scheduled_status_spec.rb create mode 100644 spec/workers/publish_scheduled_status_worker_spec.rb (limited to 'app/workers') diff --git a/app/controllers/api/v1/scheduled_statuses_controller.rb b/app/controllers/api/v1/scheduled_statuses_controller.rb new file mode 100644 index 000000000..9950296f3 --- /dev/null +++ b/app/controllers/api/v1/scheduled_statuses_controller.rb @@ -0,0 +1,77 @@ +# frozen_string_literal: true + +class Api::V1::ScheduledStatusesController < Api::BaseController + include Authorization + + before_action -> { doorkeeper_authorize! :read, :'read:statuses' }, except: [:update, :destroy] + before_action -> { doorkeeper_authorize! :write, :'write:statuses' }, only: [:update, :destroy] + + before_action :set_statuses, only: :index + before_action :set_status, except: :index + + after_action :insert_pagination_headers, only: :index + + def index + render json: @statuses, each_serializer: REST::ScheduledStatusSerializer + end + + def show + render json: @status, serializer: REST::ScheduledStatusSerializer + end + + def update + @status.update!(scheduled_status_params) + render json: @status, serializer: REST::ScheduledStatusSerializer + end + + def destroy + @status.destroy! + render_empty + end + + private + + def set_statuses + @statuses = current_account.scheduled_statuses.paginate_by_id(limit_param(DEFAULT_STATUSES_LIMIT), params_slice(:max_id, :since_id, :min_id)) + end + + def set_status + @status = current_account.scheduled_statuses.find(params[:id]) + end + + def scheduled_status_params + params.permit(:scheduled_at) + end + + def pagination_params(core_params) + params.slice(:limit).permit(:limit).merge(core_params) + end + + def insert_pagination_headers + set_pagination_headers(next_path, prev_path) + end + + def next_path + if records_continue? + api_v1_scheduled_statuses_url pagination_params(max_id: pagination_max_id) + end + end + + def prev_path + unless @statuses.empty? + api_v1_scheduled_statuses_url pagination_params(min_id: pagination_since_id) + end + end + + def records_continue? + @statuses.size == limit_param(DEFAULT_STATUSES_LIMIT) + end + + def pagination_max_id + @statuses.last.id + end + + def pagination_since_id + @statuses.first.id + end +end diff --git a/app/controllers/api/v1/statuses_controller.rb b/app/controllers/api/v1/statuses_controller.rb index 49a52f7a6..29b420c67 100644 --- a/app/controllers/api/v1/statuses_controller.rb +++ b/app/controllers/api/v1/statuses_controller.rb @@ -45,16 +45,17 @@ class Api::V1::StatusesController < Api::BaseController def create @status = PostStatusService.new.call(current_user.account, - status_params[:status], - status_params[:in_reply_to_id].blank? ? nil : Status.find(status_params[:in_reply_to_id]), + text: status_params[:status], + thread: status_params[:in_reply_to_id].blank? ? nil : Status.find(status_params[:in_reply_to_id]), media_ids: status_params[:media_ids], sensitive: status_params[:sensitive], spoiler_text: status_params[:spoiler_text], visibility: status_params[:visibility], + scheduled_at: status_params[:scheduled_at], application: doorkeeper_token.application, idempotency: request.headers['Idempotency-Key']) - render json: @status, serializer: REST::StatusSerializer + render json: @status, serializer: @status.is_a?(ScheduledStatus) ? REST::ScheduledStatusSerializer : REST::StatusSerializer end def destroy @@ -77,7 +78,7 @@ class Api::V1::StatusesController < Api::BaseController end def status_params - params.permit(:status, :in_reply_to_id, :sensitive, :spoiler_text, :visibility, media_ids: []) + params.permit(:status, :in_reply_to_id, :sensitive, :spoiler_text, :visibility, :scheduled_at, media_ids: []) end def pagination_params(core_params) diff --git a/app/models/concerns/account_associations.rb b/app/models/concerns/account_associations.rb index a894b5eed..7dafeee34 100644 --- a/app/models/concerns/account_associations.rb +++ b/app/models/concerns/account_associations.rb @@ -14,6 +14,7 @@ module AccountAssociations has_many :mentions, inverse_of: :account, dependent: :destroy has_many :notifications, inverse_of: :account, dependent: :destroy has_many :conversations, class_name: 'AccountConversation', dependent: :destroy, inverse_of: :account + has_many :scheduled_statuses, inverse_of: :account, dependent: :destroy # Pinned statuses has_many :status_pins, inverse_of: :account, dependent: :destroy diff --git a/app/models/media_attachment.rb b/app/models/media_attachment.rb index 62a11185a..6b939124f 100644 --- a/app/models/media_attachment.rb +++ b/app/models/media_attachment.rb @@ -3,20 +3,21 @@ # # Table name: media_attachments # -# id :bigint(8) not null, primary key -# status_id :bigint(8) -# file_file_name :string -# file_content_type :string -# file_file_size :integer -# file_updated_at :datetime -# remote_url :string default(""), not null -# created_at :datetime not null -# updated_at :datetime not null -# shortcode :string -# type :integer default("image"), not null -# file_meta :json -# account_id :bigint(8) -# description :text +# id :bigint(8) not null, primary key +# status_id :bigint(8) +# file_file_name :string +# file_content_type :string +# file_file_size :integer +# file_updated_at :datetime +# remote_url :string default(""), not null +# created_at :datetime not null +# updated_at :datetime not null +# shortcode :string +# type :integer default("image"), not null +# file_meta :json +# account_id :bigint(8) +# description :text +# scheduled_status_id :bigint(8) # class MediaAttachment < ApplicationRecord @@ -76,8 +77,9 @@ class MediaAttachment < ApplicationRecord IMAGE_LIMIT = 8.megabytes VIDEO_LIMIT = 40.megabytes - belongs_to :account, inverse_of: :media_attachments, optional: true - belongs_to :status, inverse_of: :media_attachments, optional: true + belongs_to :account, inverse_of: :media_attachments, optional: true + belongs_to :status, inverse_of: :media_attachments, optional: true + belongs_to :scheduled_status, inverse_of: :media_attachments, optional: true has_attached_file :file, styles: ->(f) { file_styles f }, @@ -94,8 +96,8 @@ class MediaAttachment < ApplicationRecord validates :account, presence: true validates :description, length: { maximum: 420 }, if: :local? - scope :attached, -> { where.not(status_id: nil) } - scope :unattached, -> { where(status_id: nil) } + scope :attached, -> { where.not(status_id: nil).or(where.not(scheduled_status_id: nil)) } + scope :unattached, -> { where(status_id: nil, scheduled_status_id: nil) } scope :local, -> { where(remote_url: '') } scope :remote, -> { where.not(remote_url: '') } diff --git a/app/models/scheduled_status.rb b/app/models/scheduled_status.rb new file mode 100644 index 000000000..c95470fc8 --- /dev/null +++ b/app/models/scheduled_status.rb @@ -0,0 +1,39 @@ +# frozen_string_literal: true + +# == Schema Information +# +# Table name: scheduled_statuses +# +# id :bigint(8) not null, primary key +# account_id :bigint(8) +# scheduled_at :datetime +# params :jsonb +# + +class ScheduledStatus < ApplicationRecord + include Paginable + + TOTAL_LIMIT = 300 + DAILY_LIMIT = 25 + + belongs_to :account, inverse_of: :scheduled_statuses + has_many :media_attachments, inverse_of: :scheduled_status, dependent: :destroy + + validate :validate_future_date + validate :validate_total_limit + validate :validate_daily_limit + + private + + def validate_future_date + errors.add(:scheduled_at, I18n.t('scheduled_statuses.too_soon')) if scheduled_at.present? && scheduled_at <= Time.now.utc + PostStatusService::MIN_SCHEDULE_OFFSET + end + + def validate_total_limit + errors.add(:base, I18n.t('scheduled_statuses.over_total_limit', limit: TOTAL_LIMIT)) if account.scheduled_statuses.count >= TOTAL_LIMIT + end + + def validate_daily_limit + errors.add(:base, I18n.t('scheduled_statuses.over_daily_limit', limit: DAILY_LIMIT)) if account.scheduled_statuses.where('scheduled_at::date = ?::date', scheduled_at).count >= DAILY_LIMIT + end +end diff --git a/app/serializers/rest/scheduled_status_serializer.rb b/app/serializers/rest/scheduled_status_serializer.rb new file mode 100644 index 000000000..522991bcf --- /dev/null +++ b/app/serializers/rest/scheduled_status_serializer.rb @@ -0,0 +1,11 @@ +# frozen_string_literal: true + +class REST::ScheduledStatusSerializer < ActiveModel::Serializer + attributes :id, :scheduled_at + + has_many :media_attachments, serializer: REST::MediaAttachmentSerializer + + def id + object.id.to_s + end +end diff --git a/app/services/post_status_service.rb b/app/services/post_status_service.rb index eff1b1461..07fd969e5 100644 --- a/app/services/post_status_service.rb +++ b/app/services/post_status_service.rb @@ -1,71 +1,96 @@ # frozen_string_literal: true class PostStatusService < BaseService + MIN_SCHEDULE_OFFSET = 5.minutes.freeze + # Post a text status update, fetch and notify remote users mentioned # @param [Account] account Account from which to post - # @param [String] text Message - # @param [Status] in_reply_to Optional status to reply to # @param [Hash] options + # @option [String] :text Message + # @option [Status] :thread Optional status to reply to # @option [Boolean] :sensitive # @option [String] :visibility # @option [String] :spoiler_text + # @option [String] :language + # @option [String] :scheduled_at # @option [Enumerable] :media_ids Optional array of media IDs to attach # @option [Doorkeeper::Application] :application # @option [String] :idempotency Optional idempotency key # @return [Status] - def call(account, text, in_reply_to = nil, **options) - if options[:idempotency].present? - existing_id = redis.get("idempotency:status:#{account.id}:#{options[:idempotency]}") - return Status.find(existing_id) if existing_id + def call(account, options = {}) + @account = account + @options = options + @text = @options[:text] || '' + @in_reply_to = @options[:thread] + + return idempotency_duplicate if idempotency_given? && idempotency_duplicate? + + validate_media! + preprocess_attributes! + + if scheduled? + schedule_status! + else + process_status! + postprocess_status! + bump_potential_friendship! end - media = validate_media!(options[:media_ids]) - status = nil - text = options.delete(:spoiler_text) if text.blank? && options[:spoiler_text].present? + redis.setex(idempotency_key, 3_600, @status.id) if idempotency_given? - visibility = options[:visibility] || account.user&.setting_default_privacy - visibility = :unlisted if visibility == :public && account.silenced + @status + end - ApplicationRecord.transaction do - status = account.statuses.create!(text: text, - media_attachments: media || [], - thread: in_reply_to, - sensitive: (options[:sensitive].nil? ? account.user&.setting_default_sensitive : options[:sensitive]) || options[:spoiler_text].present?, - spoiler_text: options[:spoiler_text] || '', - visibility: visibility, - language: language_from_option(options[:language]) || account.user&.setting_default_language&.presence || LanguageDetector.instance.detect(text, account), - application: options[:application]) - end + private - process_hashtags_service.call(status) - process_mentions_service.call(status) + def preprocess_attributes! + @text = @options.delete(:spoiler_text) if @text.blank? && @options[:spoiler_text].present? + @visibility = @options[:visibility] || @account.user&.setting_default_privacy + @visibility = :unlisted if @visibility == :public && @account.silenced + @scheduled_at = @options[:scheduled_at]&.to_datetime + @scheduled_at = nil if scheduled_in_the_past? + end - LinkCrawlWorker.perform_async(status.id) unless status.spoiler_text? - DistributionWorker.perform_async(status.id) - Pubsubhubbub::DistributionWorker.perform_async(status.stream_entry.id) - ActivityPub::DistributionWorker.perform_async(status.id) + def process_status! + # The following transaction block is needed to wrap the UPDATEs to + # the media attachments when the status is created - if options[:idempotency].present? - redis.setex("idempotency:status:#{account.id}:#{options[:idempotency]}", 3_600, status.id) + ApplicationRecord.transaction do + @status = @account.statuses.create!(status_attributes) end - bump_potential_friendship(account, status) - - status + process_hashtags_service.call(@status) + process_mentions_service.call(@status) end - private + def schedule_status! + if @account.statuses.build(status_attributes).valid? + # The following transaction block is needed to wrap the UPDATEs to + # the media attachments when the scheduled status is created - def validate_media!(media_ids) - return if media_ids.blank? || !media_ids.is_a?(Enumerable) + ApplicationRecord.transaction do + @status = @account.scheduled_statuses.create!(scheduled_status_attributes) + end + else + raise ActiveRecord::RecordInvalid + end + end + + def postprocess_status! + LinkCrawlWorker.perform_async(@status.id) unless @status.spoiler_text? + DistributionWorker.perform_async(@status.id) + Pubsubhubbub::DistributionWorker.perform_async(@status.stream_entry.id) + ActivityPub::DistributionWorker.perform_async(@status.id) + end - raise Mastodon::ValidationError, I18n.t('media_attachments.validations.too_many') if media_ids.size > 4 + def validate_media! + return if @options[:media_ids].blank? || !@options[:media_ids].is_a?(Enumerable) - media = MediaAttachment.where(status_id: nil).where(id: media_ids.take(4).map(&:to_i)) + raise Mastodon::ValidationError, I18n.t('media_attachments.validations.too_many') if @options[:media_ids].size > 4 - raise Mastodon::ValidationError, I18n.t('media_attachments.validations.images_and_video') if media.size > 1 && media.find(&:video?) + @media = MediaAttachment.where(status_id: nil).where(id: @options[:media_ids].take(4).map(&:to_i)) - media + raise Mastodon::ValidationError, I18n.t('media_attachments.validations.images_and_video') if @media.size > 1 && @media.find(&:video?) end def language_from_option(str) @@ -84,10 +109,68 @@ class PostStatusService < BaseService Redis.current end - def bump_potential_friendship(account, status) - return if !status.reply? || account.id == status.in_reply_to_account_id + def scheduled? + @scheduled_at.present? + end + + def idempotency_key + "idempotency:status:#{@account.id}:#{@options[:idempotency]}" + end + + def idempotency_given? + @options[:idempotency].present? + end + + def idempotency_duplicate + if scheduled? + @account.schedule_statuses.find(@idempotency_duplicate) + else + @account.statuses.find(@idempotency_duplicate) + end + end + + def idempotency_duplicate? + @idempotency_duplicate = redis.get(idempotency_key) + end + + def scheduled_in_the_past? + @scheduled_at.present? && @scheduled_at <= Time.now.utc + MIN_SCHEDULE_OFFSET + end + + def bump_potential_friendship! + return if !@status.reply? || @account.id == @status.in_reply_to_account_id ActivityTracker.increment('activity:interactions') - return if account.following?(status.in_reply_to_account_id) - PotentialFriendshipTracker.record(account.id, status.in_reply_to_account_id, :reply) + return if @account.following?(@status.in_reply_to_account_id) + PotentialFriendshipTracker.record(@account.id, @status.in_reply_to_account_id, :reply) + end + + def status_attributes + { + text: @text, + media_attachments: @media || [], + thread: @in_reply_to, + sensitive: (@options[:sensitive].nil? ? @account.user&.setting_default_sensitive : @options[:sensitive]) || @options[:spoiler_text].present?, + spoiler_text: @options[:spoiler_text] || '', + visibility: @visibility, + language: language_from_option(@options[:language]) || @account.user&.setting_default_language&.presence || LanguageDetector.instance.detect(@text, @account), + application: @options[:application], + } + end + + def scheduled_status_attributes + { + scheduled_at: @scheduled_at, + media_attachments: @media || [], + params: scheduled_options, + } + end + + def scheduled_options + @options.tap do |options_hash| + options_hash[:in_reply_to_status_id] = options_hash.delete(:thread)&.id + options_hash[:application_id] = options_hash.delete(:application)&.id + options_hash[:scheduled_at] = nil + options_hash[:idempotency] = nil + end end end diff --git a/app/services/suspend_account_service.rb b/app/services/suspend_account_service.rb index 6ab6b2901..1bc2314de 100644 --- a/app/services/suspend_account_service.rb +++ b/app/services/suspend_account_service.rb @@ -20,6 +20,7 @@ class SuspendAccountService < BaseService owned_lists passive_relationships report_notes + scheduled_statuses status_pins stream_entries subscriptions diff --git a/app/workers/publish_scheduled_status_worker.rb b/app/workers/publish_scheduled_status_worker.rb new file mode 100644 index 000000000..298a13001 --- /dev/null +++ b/app/workers/publish_scheduled_status_worker.rb @@ -0,0 +1,24 @@ +# frozen_string_literal: true + +class PublishScheduledStatusWorker + include Sidekiq::Worker + + def perform(scheduled_status_id) + scheduled_status = ScheduledStatus.find(scheduled_status_id) + scheduled_status.destroy! + + PostStatusService.new.call( + scheduled_status.account, + options_with_objects(scheduled_status.params.with_indifferent_access) + ) + rescue ActiveRecord::RecordNotFound, ActiveRecord::RecordInvalid + true + end + + def options_with_objects(options) + options.tap do |options_hash| + options_hash[:application] = Doorkeeper::Application.find(options_hash.delete(:application_id)) if options[:application_id] + options_hash[:thread] = Status.find(options_hash.delete(:in_reply_to_status_id)) if options_hash[:in_reply_to_status_id] + end + end +end diff --git a/app/workers/scheduler/scheduled_statuses_scheduler.rb b/app/workers/scheduler/scheduled_statuses_scheduler.rb new file mode 100644 index 000000000..70a45846b --- /dev/null +++ b/app/workers/scheduler/scheduled_statuses_scheduler.rb @@ -0,0 +1,19 @@ +# frozen_string_literal: true + +class Scheduler::ScheduledStatusesScheduler + include Sidekiq::Worker + + sidekiq_options unique: :until_executed, retry: 0 + + def perform + due_statuses.find_each do |scheduled_status| + PublishScheduledStatusWorker.perform_at(scheduled_status.scheduled_at) + end + end + + private + + def due_statuses + ScheduledStatus.where('scheduled_at <= ?', Time.now.utc + PostStatusService::MIN_SCHEDULE_OFFSET) + end +end diff --git a/config/locales/en.yml b/config/locales/en.yml index 6c78b9fc9..7e05568f1 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -728,6 +728,10 @@ en: error: Error title: Title unfollowed: Unfollowed + scheduled_statuses: + over_daily_limit: You have exceeded the limit of %{limit} scheduled toots for that day + over_total_limit: You have exceeded the limit of %{limit} scheduled toots + too_soon: The scheduled date must be in the future sessions: activity: Last activity browser: Browser diff --git a/config/routes.rb b/config/routes.rb index 5bdd1986c..3ae2735d1 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -283,6 +283,7 @@ Rails.application.routes.draw do resources :streaming, only: [:index] resources :custom_emojis, only: [:index] resources :suggestions, only: [:index, :destroy] + resources :scheduled_statuses, only: [:index, :show, :update, :destroy] resources :conversations, only: [:index, :destroy] do member do diff --git a/config/sidekiq.yml b/config/sidekiq.yml index c44af5b6c..0ec1742ab 100644 --- a/config/sidekiq.yml +++ b/config/sidekiq.yml @@ -6,6 +6,9 @@ - [mailers, 2] - [pull] :schedule: + scheduled_statuses_scheduler: + every: '5m' + class: Scheduler::ScheduledStatusesScheduler subscriptions_scheduler: cron: '<%= Random.rand(0..59) %> <%= Random.rand(4..6) %> * * *' class: Scheduler::SubscriptionsScheduler diff --git a/db/migrate/20190103124649_create_scheduled_statuses.rb b/db/migrate/20190103124649_create_scheduled_statuses.rb new file mode 100644 index 000000000..2b78073b8 --- /dev/null +++ b/db/migrate/20190103124649_create_scheduled_statuses.rb @@ -0,0 +1,9 @@ +class CreateScheduledStatuses < ActiveRecord::Migration[5.2] + def change + create_table :scheduled_statuses do |t| + t.belongs_to :account, foreign_key: { on_delete: :cascade } + t.datetime :scheduled_at, index: true + t.jsonb :params + end + end +end diff --git a/db/migrate/20190103124754_add_scheduled_status_id_to_media_attachments.rb b/db/migrate/20190103124754_add_scheduled_status_id_to_media_attachments.rb new file mode 100644 index 000000000..6f6cf2351 --- /dev/null +++ b/db/migrate/20190103124754_add_scheduled_status_id_to_media_attachments.rb @@ -0,0 +1,8 @@ +class AddScheduledStatusIdToMediaAttachments < ActiveRecord::Migration[5.2] + disable_ddl_transaction! + + def change + add_reference :media_attachments, :scheduled_status, foreign_key: { on_delete: :nullify }, index: false + add_index :media_attachments, :scheduled_status_id, algorithm: :concurrently + end +end diff --git a/db/schema.rb b/db/schema.rb index 066a90b52..9380362e1 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 2018_12_26_021420) do +ActiveRecord::Schema.define(version: 2019_01_03_124754) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -336,7 +336,9 @@ ActiveRecord::Schema.define(version: 2018_12_26_021420) do t.json "file_meta" t.bigint "account_id" t.text "description" + t.bigint "scheduled_status_id" t.index ["account_id"], name: "index_media_attachments_on_account_id" + t.index ["scheduled_status_id"], name: "index_media_attachments_on_scheduled_status_id" t.index ["shortcode"], name: "index_media_attachments_on_shortcode", unique: true t.index ["status_id"], name: "index_media_attachments_on_status_id" end @@ -487,6 +489,14 @@ ActiveRecord::Schema.define(version: 2018_12_26_021420) do t.index ["target_account_id"], name: "index_reports_on_target_account_id" end + create_table "scheduled_statuses", force: :cascade do |t| + t.bigint "account_id" + t.datetime "scheduled_at" + t.jsonb "params" + t.index ["account_id"], name: "index_scheduled_statuses_on_account_id" + t.index ["scheduled_at"], name: "index_scheduled_statuses_on_scheduled_at" + end + create_table "session_activations", force: :cascade do |t| t.string "session_id", null: false t.datetime "created_at", null: false @@ -700,6 +710,7 @@ ActiveRecord::Schema.define(version: 2018_12_26_021420) do add_foreign_key "list_accounts", "lists", on_delete: :cascade add_foreign_key "lists", "accounts", on_delete: :cascade add_foreign_key "media_attachments", "accounts", name: "fk_96dd81e81b", on_delete: :nullify + add_foreign_key "media_attachments", "scheduled_statuses", on_delete: :nullify add_foreign_key "media_attachments", "statuses", on_delete: :nullify add_foreign_key "mentions", "accounts", name: "fk_970d43f9d1", on_delete: :cascade add_foreign_key "mentions", "statuses", on_delete: :cascade @@ -718,6 +729,7 @@ ActiveRecord::Schema.define(version: 2018_12_26_021420) do add_foreign_key "reports", "accounts", column: "assigned_account_id", on_delete: :nullify add_foreign_key "reports", "accounts", column: "target_account_id", name: "fk_eb37af34f0", on_delete: :cascade add_foreign_key "reports", "accounts", name: "fk_4b81f7522c", on_delete: :cascade + add_foreign_key "scheduled_statuses", "accounts", on_delete: :cascade add_foreign_key "session_activations", "oauth_access_tokens", column: "access_token_id", name: "fk_957e5bda89", on_delete: :cascade add_foreign_key "session_activations", "users", name: "fk_e5fda67334", on_delete: :cascade add_foreign_key "status_pins", "accounts", name: "fk_d4cb435b62", on_delete: :cascade diff --git a/spec/controllers/api/v1/conversations_controller_spec.rb b/spec/controllers/api/v1/conversations_controller_spec.rb index 2e9525855..070f65061 100644 --- a/spec/controllers/api/v1/conversations_controller_spec.rb +++ b/spec/controllers/api/v1/conversations_controller_spec.rb @@ -15,7 +15,7 @@ RSpec.describe Api::V1::ConversationsController, type: :controller do let(:scopes) { 'read:statuses' } before do - PostStatusService.new.call(other.account, 'Hey @alice', nil, visibility: 'direct') + PostStatusService.new.call(other.account, text: 'Hey @alice', visibility: 'direct') end it 'returns http success' do diff --git a/spec/controllers/api/v1/notifications_controller_spec.rb b/spec/controllers/api/v1/notifications_controller_spec.rb index 9f679cb8a..d0f82e79f 100644 --- a/spec/controllers/api/v1/notifications_controller_spec.rb +++ b/spec/controllers/api/v1/notifications_controller_spec.rb @@ -50,9 +50,9 @@ RSpec.describe Api::V1::NotificationsController, type: :controller do let(:scopes) { 'read:notifications' } before do - first_status = PostStatusService.new.call(user.account, 'Test') + first_status = PostStatusService.new.call(user.account, text: 'Test') @reblog_of_first_status = ReblogService.new.call(other.account, first_status) - mentioning_status = PostStatusService.new.call(other.account, 'Hello @alice') + mentioning_status = PostStatusService.new.call(other.account, text: 'Hello @alice') @mention_from_status = mentioning_status.mentions.first @favourite = FavouriteService.new.call(other.account, first_status) @follow = FollowService.new.call(other.account, 'alice') diff --git a/spec/controllers/api/v1/timelines/home_controller_spec.rb b/spec/controllers/api/v1/timelines/home_controller_spec.rb index 63d624c35..e953e4649 100644 --- a/spec/controllers/api/v1/timelines/home_controller_spec.rb +++ b/spec/controllers/api/v1/timelines/home_controller_spec.rb @@ -17,7 +17,7 @@ describe Api::V1::Timelines::HomeController do describe 'GET #show' do before do follow = Fabricate(:follow, account: user.account) - PostStatusService.new.call(follow.target_account, 'New status for user home timeline.') + PostStatusService.new.call(follow.target_account, text: 'New status for user home timeline.') end it 'returns http success' do diff --git a/spec/controllers/api/v1/timelines/list_controller_spec.rb b/spec/controllers/api/v1/timelines/list_controller_spec.rb index 93a2be6e6..45e4bf34c 100644 --- a/spec/controllers/api/v1/timelines/list_controller_spec.rb +++ b/spec/controllers/api/v1/timelines/list_controller_spec.rb @@ -19,7 +19,7 @@ describe Api::V1::Timelines::ListController do before do follow = Fabricate(:follow, account: user.account) list.accounts << follow.target_account - PostStatusService.new.call(follow.target_account, 'New status for user home timeline.') + PostStatusService.new.call(follow.target_account, text: 'New status for user home timeline.') end it 'returns http success' do diff --git a/spec/controllers/api/v1/timelines/public_controller_spec.rb b/spec/controllers/api/v1/timelines/public_controller_spec.rb index a0f778cdc..737aedba6 100644 --- a/spec/controllers/api/v1/timelines/public_controller_spec.rb +++ b/spec/controllers/api/v1/timelines/public_controller_spec.rb @@ -16,7 +16,7 @@ describe Api::V1::Timelines::PublicController do describe 'GET #show' do before do - PostStatusService.new.call(user.account, 'New status from user for federated public timeline.') + PostStatusService.new.call(user.account, text: 'New status from user for federated public timeline.') end it 'returns http success' do @@ -29,7 +29,7 @@ describe Api::V1::Timelines::PublicController do describe 'GET #show with local only' do before do - PostStatusService.new.call(user.account, 'New status from user for local public timeline.') + PostStatusService.new.call(user.account, text: 'New status from user for local public timeline.') end it 'returns http success' do diff --git a/spec/controllers/api/v1/timelines/tag_controller_spec.rb b/spec/controllers/api/v1/timelines/tag_controller_spec.rb index 472779f54..f71ca2a39 100644 --- a/spec/controllers/api/v1/timelines/tag_controller_spec.rb +++ b/spec/controllers/api/v1/timelines/tag_controller_spec.rb @@ -16,7 +16,7 @@ describe Api::V1::Timelines::TagController do describe 'GET #show' do before do - PostStatusService.new.call(user.account, 'It is a #test') + PostStatusService.new.call(user.account, text: 'It is a #test') end it 'returns http success' do diff --git a/spec/fabricators/scheduled_status_fabricator.rb b/spec/fabricators/scheduled_status_fabricator.rb new file mode 100644 index 000000000..52384d137 --- /dev/null +++ b/spec/fabricators/scheduled_status_fabricator.rb @@ -0,0 +1,4 @@ +Fabricator(:scheduled_status) do + account + scheduled_at { 20.hours.from_now } +end diff --git a/spec/lib/feed_manager_spec.rb b/spec/lib/feed_manager_spec.rb index 64e109aec..c506cd87f 100644 --- a/spec/lib/feed_manager_spec.rb +++ b/spec/lib/feed_manager_spec.rb @@ -108,14 +108,14 @@ RSpec.describe FeedManager do it 'returns false for status by followee mentioning another account' do bob.follow!(alice) - status = PostStatusService.new.call(alice, 'Hey @jeff') + status = PostStatusService.new.call(alice, text: 'Hey @jeff') expect(FeedManager.instance.filter?(:home, status, bob.id)).to be false end it 'returns true for status by followee mentioning blocked account' do bob.block!(jeff) bob.follow!(alice) - status = PostStatusService.new.call(alice, 'Hey @jeff') + status = PostStatusService.new.call(alice, text: 'Hey @jeff') expect(FeedManager.instance.filter?(:home, status, bob.id)).to be true end @@ -155,7 +155,7 @@ RSpec.describe FeedManager do context 'for mentions feed' do it 'returns true for status that mentions blocked account' do bob.block!(jeff) - status = PostStatusService.new.call(alice, 'Hey @jeff') + status = PostStatusService.new.call(alice, text: 'Hey @jeff') expect(FeedManager.instance.filter?(:mentions, status, bob.id)).to be true end diff --git a/spec/models/scheduled_status_spec.rb b/spec/models/scheduled_status_spec.rb new file mode 100644 index 000000000..f8c9d8b81 --- /dev/null +++ b/spec/models/scheduled_status_spec.rb @@ -0,0 +1,4 @@ +require 'rails_helper' + +RSpec.describe ScheduledStatus, type: :model do +end diff --git a/spec/services/batched_remove_status_service_spec.rb b/spec/services/batched_remove_status_service_spec.rb index c66214555..e53623449 100644 --- a/spec/services/batched_remove_status_service_spec.rb +++ b/spec/services/batched_remove_status_service_spec.rb @@ -8,8 +8,8 @@ RSpec.describe BatchedRemoveStatusService, type: :service do let!(:jeff) { Fabricate(:user).account } let!(:hank) { Fabricate(:account, username: 'hank', protocol: :activitypub, domain: 'example.com', inbox_url: 'http://example.com/inbox') } - let(:status1) { PostStatusService.new.call(alice, 'Hello @bob@example.com') } - let(:status2) { PostStatusService.new.call(alice, 'Another status') } + let(:status1) { PostStatusService.new.call(alice, text: 'Hello @bob@example.com') } + let(:status2) { PostStatusService.new.call(alice, text: 'Another status') } before do allow(Redis.current).to receive_messages(publish: nil) diff --git a/spec/services/post_status_service_spec.rb b/spec/services/post_status_service_spec.rb index 8f3552224..3774fed6f 100644 --- a/spec/services/post_status_service_spec.rb +++ b/spec/services/post_status_service_spec.rb @@ -7,7 +7,7 @@ RSpec.describe PostStatusService, type: :service do account = Fabricate(:account) text = "test status update" - status = subject.call(account, text) + status = subject.call(account, text: text) expect(status).to be_persisted expect(status.text).to eq text @@ -18,20 +18,31 @@ RSpec.describe PostStatusService, type: :service do account = Fabricate(:account) text = "test status update" - status = subject.call(account, text, in_reply_to_status) + status = subject.call(account, text: text, thread: in_reply_to_status) expect(status).to be_persisted expect(status.text).to eq text expect(status.thread).to eq in_reply_to_status end + it 'schedules a status' do + account = Fabricate(:account) + future = Time.now.utc + 2.hours + + status = subject.call(account, text: 'Hi future!', scheduled_at: future) + + expect(status).to be_a ScheduledStatus + expect(status.scheduled_at).to eq future + expect(status.params['text']).to eq 'Hi future!' + end + it 'creates response to the original status of boost' do boosted_status = Fabricate(:status) in_reply_to_status = Fabricate(:status, reblog: boosted_status) account = Fabricate(:account) text = "test status update" - status = subject.call(account, text, in_reply_to_status) + status = subject.call(account, text: text, thread: in_reply_to_status) expect(status).to be_persisted expect(status.text).to eq text @@ -69,7 +80,7 @@ RSpec.describe PostStatusService, type: :service do end it 'creates a status with limited visibility for silenced users' do - status = subject.call(Fabricate(:account, silenced: true), 'test', nil, visibility: :public) + status = subject.call(Fabricate(:account, silenced: true), text: 'test', visibility: :public) expect(status).to be_persisted expect(status.visibility).to eq "unlisted" @@ -88,7 +99,7 @@ RSpec.describe PostStatusService, type: :service do account = Fabricate(:account) text = 'This is an English text.' - status = subject.call(account, text) + status = subject.call(account, text: text) expect(status.language).to eq 'en' end @@ -99,7 +110,7 @@ RSpec.describe PostStatusService, type: :service do allow(ProcessMentionsService).to receive(:new).and_return(mention_service) account = Fabricate(:account) - status = subject.call(account, "test status update") + status = subject.call(account, text: "test status update") expect(ProcessMentionsService).to have_received(:new) expect(mention_service).to have_received(:call).with(status) @@ -111,7 +122,7 @@ RSpec.describe PostStatusService, type: :service do allow(ProcessHashtagsService).to receive(:new).and_return(hashtags_service) account = Fabricate(:account) - status = subject.call(account, "test status update") + status = subject.call(account, text: "test status update") expect(ProcessHashtagsService).to have_received(:new) expect(hashtags_service).to have_received(:call).with(status) @@ -124,7 +135,7 @@ RSpec.describe PostStatusService, type: :service do account = Fabricate(:account) - status = subject.call(account, "test status update") + status = subject.call(account, text: "test status update") expect(DistributionWorker).to have_received(:perform_async).with(status.id) expect(Pubsubhubbub::DistributionWorker).to have_received(:perform_async).with(status.stream_entry.id) @@ -135,7 +146,7 @@ RSpec.describe PostStatusService, type: :service do allow(LinkCrawlWorker).to receive(:perform_async) account = Fabricate(:account) - status = subject.call(account, "test status update") + status = subject.call(account, text: "test status update") expect(LinkCrawlWorker).to have_received(:perform_async).with(status.id) end @@ -146,8 +157,7 @@ RSpec.describe PostStatusService, type: :service do status = subject.call( account, - "test status update", - nil, + text: "test status update", media_ids: [media.id], ) @@ -160,8 +170,7 @@ RSpec.describe PostStatusService, type: :service do expect do subject.call( account, - "test status update", - nil, + text: "test status update", media_ids: [ Fabricate(:media_attachment, account: account), Fabricate(:media_attachment, account: account), @@ -182,8 +191,7 @@ RSpec.describe PostStatusService, type: :service do expect do subject.call( account, - "test status update", - nil, + text: "test status update", media_ids: [ Fabricate(:media_attachment, type: :video, account: account), Fabricate(:media_attachment, type: :image, account: account), @@ -197,12 +205,12 @@ RSpec.describe PostStatusService, type: :service do it 'returns existing status when used twice with idempotency key' do account = Fabricate(:account) - status1 = subject.call(account, 'test', nil, idempotency: 'meepmeep') - status2 = subject.call(account, 'test', nil, idempotency: 'meepmeep') + status1 = subject.call(account, text: 'test', idempotency: 'meepmeep') + status2 = subject.call(account, text: 'test', idempotency: 'meepmeep') expect(status2.id).to eq status1.id end def create_status_with_options(**options) - subject.call(Fabricate(:account), 'test', nil, options) + subject.call(Fabricate(:account), options.merge(text: 'test')) end end diff --git a/spec/services/remove_status_service_spec.rb b/spec/services/remove_status_service_spec.rb index 2134f51fd..7bba83a60 100644 --- a/spec/services/remove_status_service_spec.rb +++ b/spec/services/remove_status_service_spec.rb @@ -19,7 +19,7 @@ RSpec.describe RemoveStatusService, type: :service do jeff.follow!(alice) hank.follow!(alice) - @status = PostStatusService.new.call(alice, 'Hello @bob@example.com') + @status = PostStatusService.new.call(alice, text: 'Hello @bob@example.com') Fabricate(:status, account: bill, reblog: @status, uri: 'hoge') subject.call(@status) end diff --git a/spec/workers/publish_scheduled_status_worker_spec.rb b/spec/workers/publish_scheduled_status_worker_spec.rb new file mode 100644 index 000000000..f8547e6fe --- /dev/null +++ b/spec/workers/publish_scheduled_status_worker_spec.rb @@ -0,0 +1,23 @@ +# frozen_string_literal: true + +require 'rails_helper' + +describe PublishScheduledStatusWorker do + subject { described_class.new } + + let(:scheduled_status) { Fabricate(:scheduled_status, params: { text: 'Hello world, future!' }) } + + describe 'perform' do + before do + subject.perform(scheduled_status.id) + end + + it 'creates a status' do + expect(scheduled_status.account.statuses.first.text).to eq 'Hello world, future!' + end + + it 'removes the scheduled status' do + expect(ScheduledStatus.find_by(id: scheduled_status.id)).to be_nil + end + end +end -- cgit From 8ad57a0426402831373f9f45705774f875b5b65d Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Sun, 6 Jan 2019 09:21:16 +0100 Subject: Fix PublishScheduledStatusWorker being queued with no arguments (#9724) --- app/workers/scheduler/scheduled_statuses_scheduler.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'app/workers') diff --git a/app/workers/scheduler/scheduled_statuses_scheduler.rb b/app/workers/scheduler/scheduled_statuses_scheduler.rb index 70a45846b..1772a246b 100644 --- a/app/workers/scheduler/scheduled_statuses_scheduler.rb +++ b/app/workers/scheduler/scheduled_statuses_scheduler.rb @@ -7,7 +7,7 @@ class Scheduler::ScheduledStatusesScheduler def perform due_statuses.find_each do |scheduled_status| - PublishScheduledStatusWorker.perform_at(scheduled_status.scheduled_at) + PublishScheduledStatusWorker.perform_at(scheduled_status.scheduled_at, scheduled_status.id) end end -- cgit From 1cbdf8d218dcd07889d14e4f5f22a4339bddb879 Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Sun, 6 Jan 2019 12:03:27 +0100 Subject: Fix wrong param name in scheduled statuses and return params in API (#9725) The database column and API param are called in_reply_to_id, not in_reply_to_status_id, so it makes no sense to encode it that way --- app/serializers/rest/scheduled_status_serializer.rb | 6 +++++- app/services/post_status_service.rb | 8 ++++---- app/workers/publish_scheduled_status_worker.rb | 2 +- 3 files changed, 10 insertions(+), 6 deletions(-) (limited to 'app/workers') diff --git a/app/serializers/rest/scheduled_status_serializer.rb b/app/serializers/rest/scheduled_status_serializer.rb index 522991bcf..5d6311b87 100644 --- a/app/serializers/rest/scheduled_status_serializer.rb +++ b/app/serializers/rest/scheduled_status_serializer.rb @@ -1,11 +1,15 @@ # frozen_string_literal: true class REST::ScheduledStatusSerializer < ActiveModel::Serializer - attributes :id, :scheduled_at + attributes :id, :scheduled_at, :params has_many :media_attachments, serializer: REST::MediaAttachmentSerializer def id object.id.to_s end + + def params + object.params.without(:application_id) + end end diff --git a/app/services/post_status_service.rb b/app/services/post_status_service.rb index 07fd969e5..260765edf 100644 --- a/app/services/post_status_service.rb +++ b/app/services/post_status_service.rb @@ -167,10 +167,10 @@ class PostStatusService < BaseService def scheduled_options @options.tap do |options_hash| - options_hash[:in_reply_to_status_id] = options_hash.delete(:thread)&.id - options_hash[:application_id] = options_hash.delete(:application)&.id - options_hash[:scheduled_at] = nil - options_hash[:idempotency] = nil + options_hash[:in_reply_to_id] = options_hash.delete(:thread)&.id + options_hash[:application_id] = options_hash.delete(:application)&.id + options_hash[:scheduled_at] = nil + options_hash[:idempotency] = nil end end end diff --git a/app/workers/publish_scheduled_status_worker.rb b/app/workers/publish_scheduled_status_worker.rb index 298a13001..641fcc61c 100644 --- a/app/workers/publish_scheduled_status_worker.rb +++ b/app/workers/publish_scheduled_status_worker.rb @@ -18,7 +18,7 @@ class PublishScheduledStatusWorker def options_with_objects(options) options.tap do |options_hash| options_hash[:application] = Doorkeeper::Application.find(options_hash.delete(:application_id)) if options[:application_id] - options_hash[:thread] = Status.find(options_hash.delete(:in_reply_to_status_id)) if options_hash[:in_reply_to_status_id] + options_hash[:thread] = Status.find(options_hash.delete(:in_reply_to_id)) if options_hash[:in_reply_to_id] end end end -- cgit