Class: RuboCop::Cop::DevDoc::Rails::NoPersistenceInService

Inherits:
Base
  • Object
show all
Defined in:
lib/rubocop/cop/dev_doc/rails/no_persistence_in_service.rb

Overview

Classes under app/services/ must not persist domain records, open transactions, or enqueue jobs/mailers.

Rationale

app/services/ is reserved for external-boundary adapters: clients for third-party APIs, storage backends, and their test doubles. An adapter talks to an external system and returns data. Anything more is domain orchestration, and its home routes by what is being orchestrated (see the orchestration best-practice doc):

  • Transactions and record writes are data-saving orchestration. Home: a domain-named ActiveModel::Model PORO under app/models/ (see the model best-practice doc) or a method on the model that owns the data.
  • Enqueues (jobs, mailer sends) are execution-flow control. Home: the calling controller or job entry point — ordering is then enforced by data flow, because the trigger runs only after the domain call returns post-commit. The one exception: an enqueue inseparable from a record state change lives in the model behind an explicitly named notify_*/send_* method with a justified per-line disable of DevDoc/Rails/NoPerformLaterInModel.

Keeping domain writes out of app/services/ prevents the "fake-generic service" failure mode: a *Service class with one caller whose generic name and location advertise reusability it does not have, and whose existence bypasses the conventions (form-object POROs, model methods, concerns) the project actually uses for domain logic.

❌
# app/services/orders/publish_service.rb
class Orders::PublishService
def call
  Order.transaction do
    @order.save!
    OrderMailer.with(order: @order).published.deliver_later
  end
end
end

✔️
# app/models/orders/publish.rb — domain PORO owns the transaction and writes,
# and RETURNS; it does not enqueue (that would trip NoPerformLaterInModel)
class Orders::Publish
include ActiveModel::Model

def save
  Order.transaction { @order.save! }
end
end

# app/controllers/orders_controller.rb — the caller triggers the send;
# it can only run after the PORO's transaction has committed
OrderMailer.with(order: publish.order).published.deliver_later if publish.save

✔️
# app/services/payment_gateway.rb — adapter; external calls only
class PaymentGateway
def charge(amount_cents)
  @client.post('/charges', amount: amount_cents)
end
end

Configurable blocklist for write wrappers

A service can hide its writes behind a model method that persists internally (e.g. an add_member_if_missing-style helper). The cop cannot see through arbitrary methods; register the known ones via KnownWriteWrappers so those sends flag too. This is a partial mitigation — reviewers must still catch unregistered wrappers.

NOTE: Two residuals the cop cannot catch — reviewers own both:

  • A service whose writes all go through wrapper methods not listed in KnownWriteWrappers.
  • Read-only domain logic parked in app/services/ (queries, catalogs, pure utilities). Nothing persists, so nothing flags — but its home is still app/models/ or lib/. Hash#delete / Array#insert collisions are why bare delete / insert are not in the tracked list; delete_all / insert_all are. Bare update IS tracked despite Hash#update (a merge! alias): ActiveRecord update is a far more common sight in a service than a Hash mutation, so the rare Hash collision is accepted (disable inline with a reason) rather than losing detection of record.update.

Examples:

# bad
ApplicationRecord.transaction { order.save! }

# bad
user.folders.find_or_create_by!(name: 'Default')

# bad
OrderMailer.with(order: order).published.deliver_later

# good — adapter work: external API call, no domain persistence
@client.post('/charges', amount: amount_cents)

Constant Summary collapse

MSG =

Two messages, routed per the orchestration taxonomy: persistence and transactions move to the model layer; enqueues move to the caller.

'`%<method>s` in app/services — services are external-boundary ' \
'adapters and must not persist domain records or enqueue jobs. ' \
'Move this into a domain object under app/models.'.freeze
ENQUEUE_MSG = 
'`%<method>s` in app/services — services are external-boundary ' \
'adapters and must not enqueue jobs or send mail. Return data and ' \
'let the calling controller or job trigger the send.'.freeze
TRANSACTION_METHODS =

with_lock opens a transaction too (see NoDeliverLaterInTransaction); an enqueue/write inside it is the same violation in a service.

%i[transaction with_lock].freeze
PERSISTENCE_METHODS =

ActiveRecord persistence. Non-bang forms and the find-or-create family are included (an observed violation used find_or_create_by!). delete and insert are deliberately ABSENT: Hash#delete / Array#insert are common enough to flood false positives; delete_all / insert_all remain.

%i[
  save save! create create! update update! update_all
  update_column update_columns update_attribute
  destroy destroy! destroy_all delete_all
  insert_all insert_all! upsert upsert_all
  find_or_create_by find_or_create_by! create_or_find_by create_or_find_by!
  first_or_create first_or_create! touch increment! decrement! toggle!
].freeze
ENQUEUE_METHODS =

Enqueues/sends — mirror NoDeliverLaterInTransaction::CORE_METHODS, plus deliver_now (an adapter must not send mail synchronously either).

%i[
  deliver_later deliver_later! deliver_now perform_later perform_all_later enqueue
].freeze
TRACKED_METHODS =

Union of the three fixed lists, pre-computed once for O(1) membership. KnownWriteWrappers (config) are matched separately as strings — see tracked_method?.

(TRANSACTION_METHODS + PERSISTENCE_METHODS + ENQUEUE_METHODS).freeze

Instance Method Summary collapse

Instance Method Details

#on_send(node) ⇒ Object Also known as: on_csend

Config-driven (KnownWriteWrappers), so do NOT use RESTRICT_ON_SEND (CONTRIBUTING documents this exact exception). Filter inside on_send, mirroring NoDeliverLaterInTransaction#tracked_method?.



141
142
143
144
145
146
147
 
# File 'lib/rubocop/cop/dev_doc/rails/no_persistence_in_service.rb', line 141

def on_send(node)
  name = node.method_name
  return unless tracked_method?(name)

  msg = ENQUEUE_METHODS.include?(name) ? ENQUEUE_MSG : MSG
  add_offense(node.loc.selector, message: format(msg, method: name))
end