DEV Community

Petr Hlavicka
Petr Hlavicka

Posted on • Originally published at petr.codes

Business logic in Rails with operators

Having a thousand lines long controllers and/or models is not the right way to have sustainable applications or developers' sanity. Let's look at my solution for business logic in the Rails app.

Spoiler alert: yes, I will use PORO... again.

Why?

Why should you not have such long controllers/models (or even views)? There are a lot of reasons. From worse sustainability, readability, to worse testability. But mainly, they all affect the developer's happiness.

I can gladly recommend Sustainable Web Development with Ruby on Rails from David Bryant Copeland where he did a great job explaining it all.

What did I want from the solution?

I can’t say I was not influenced by other solutions. For example, I used Trailblazer before. But none of what I read about or used was the one I would like.

When I read a solution from Josef Strzibny, I realized that I should write down my approach to get some feedback.

Here is what I wanted to achieve:

  1. nothing complex
  2. naming easy as possible
  3. simple file structure
  4. OOP and its benefits (even for results)
  5. easy testability
  6. in general - as few later decisions as possible

The solution

I will demonstrate the solution on a simple Invoice model with a corresponding InvoicesController.

Naming and structure

The first thing is the naming and the corresponding file structure. I chose the Operator suffix. In our case, it will be InvoiceOperator inside the app/operators folder.

The suffix makes everything easier - the developer will always know what to use for any model, it is just a simple <ModelName>Operator.

Naming is hard, especially for non-native speakers. If you find a better name, let me know!

So, we have the class name, but what about its methods? It will be, mainly but not only, used in controllers. As Rails controllers are already breaking the Single File Responsibility principle, I will not hesitate to continue with that to have things easier.

To make it even easier, let's use the classic RESTful names for methods. For the create action in the controller, it will look like this:

# app/operators/invoice_operator.rb

class InvoiceOperator
  def create(params:)
    # ...
  end
end
Enter fullscreen mode Exit fullscreen mode
# app/controllers/invoices_controller.rb

class InvoicesController < ApplicationController
  def create
    result = InvoiceOperator.new.create(params: invoice_params)
    # ...
  end
end
Enter fullscreen mode Exit fullscreen mode

So, every model will have its operator and in every operator, we will know what methods should be used in each action. Everything is easily predictable in most cases.

Except... the new action in a controller. Having InvoiceOperator.new.new does not look cool to me. Luckily for most cases, we don't need it and we can use the simple Invoice.new.

If we will need to apply complex logic (and thus use the operator), we can use a prepare method instead of the new. It is not perfect to the previous statement, but the naming makes sense to me.

Result object

Using the result object is a common strategy. The base concept is the same for every operator, so we won’t repeat it in every operator. Let's create a BaseOperator class.

This will also help us not to think about the name of the method with our object (in our case the invoice). It will always be the result.record and not eg. result.invoice.

# app/operators/base_operator.rb

class BaseOperator
  def initialize(record: nil)
    @record = record || new_record
  end

  private

  def new_record
    raise NotImplementedError
  end

  class Result
    attr_reader :record, :meta

    def initialize(state:, record: nil, **meta)
      @state = state
      @record = record
      @meta = meta
    end

    def success?
      !!@state
    end
  end
end
Enter fullscreen mode Exit fullscreen mode

And use it for our InvoiceOperator:

# app/operators/invoice_operator.rb

class InvoiceOperator < BaseOperator
  def update(params:)
    @record.assign_attributes(params)

    # do things before updating the invoice eg:
    # update/create related records (like InvoiceItems)
    # a few more examples:
    calculate_total 
    calculate_vat

    state = @record.save

    # do your other business eg.:
    # send emails,
    # call external services and so on

    Result.new(state: state, record: @record)
  end

  def create(params:)
    @record.assign_attributes(params)

    # do things before creating the invoice eg:
    # create related records (like InvoiceItems)
    # a few more examples:
    calculate_total
    calculate_vat

    state = @record.save

    # do your other business eg.:
    # send emails,
    # call external services and so on

    Result.new(state: state, record: @record)
  end

  private

  def new_record
    Invoice.new
  end

  def calculate_total
    # you can write the logic here, 
    # or call a class that handles the calculation
  end

  def calculate_vat
    # you can write the logic here, 
    # or call a class that handles the calculation
  end
end
Enter fullscreen mode Exit fullscreen mode

The BaseOperator also introduced the initialize method. That will help us to use the operator in two ways:

  • with a new record: eg. InvoiceOperator.new.create(params: invoice_params) where it will use Invoice.new
  • with the existing record: eg. InvoiceOperator.new(record: Invoice.find(params[:id])).update(params: invoice_params)

The Result object uses a state variable. I like this way more than using two objects (one for success and one for failure). It is also much simpler for testing.

The private method new_record can be also used for setting the right "blank" object (eg. with some defaults).

And now, the example usage in the controller:

# app/controllers/invoices_controller.rb

class InvoicesController < ApplicationController
  def create
    result = InvoiceOperator.new.create(params: invoice_params)

    if result.success?
      redirect_to result.record, notice: "Created!"
    else
      render :new, locals: {
        invoice: result.record
      }, status: :unprocessable_entity
    end
  end

  def update
    result = InvoiceOperator.new(record: Invoice.find(params[:id]))
      .update(params: invoice_params)

    if result.success?
      redirect_to result.record, notice: "Updated!"
    else
      render :edit, locals: {
        invoice: result.record
      }, status: :unprocessable_entity
    end
  end
end
Enter fullscreen mode Exit fullscreen mode

Custom actions in controllers

If you are using custom actions in controllers, you can continue to have the same method name in the operator.

If you don't and you are using only RESTful actions, you can end up with this:

module Invoices
  class DuplicatesController < ApplicationController
    def create
      original_invoice = Invoice.find(params[:id])
      result = InvoiceOperator.new(record: original_invoice).duplicate

      if result.success?
        redirect_to result.record, notice: "Duplicated!"
      else
        redirect_back fallback_location: original_invoice, allow_other_host: false
      end
    end
  end
end
Enter fullscreen mode Exit fullscreen mode

In this case, the action create does not correspond with the operator's duplicate method, but at least, the controller name is related to it. That should help with a decision on what name should be used.

Other possible solution could be to use a new operator (eg. InvoiceDuplicateOperator) that would inherit from InvoiceOperator and has the right create action.

Testing

I mentioned testing several times. Here is a simplified example for testing the operator.

# spec/operators/invoice_operator_spec.rb

RSpec.describe InvoiceOperator, type: :operator do
  let(:invoice) {}
  let(:company) { create(:company) }
  let(:operator) { described_class.new(record: invoice) }

  describe "create" do
    let(:params) do
      ActionController::Parameters.new({
        "company_id" => company.id,
        "date_from" => "2021-01-01",
        "date_to" => "2021-01-31",
        "due_at" => "2021-01-16"
      })
    end

    it "creates a record" do
      result = operator.create(params: params)

      expect(result).to be_success
      expect(result.record.persisted?).to be_truthy
    end
  end

  describe "update" do
    let(:invoice) { create(:invoice, paid_at: nil) }
    let(:params) do
      ActionController::Parameters.new({
        "paid_at" => "2021-01-18"
      })
    end

    it "updates a record" do
      result = operator.update(params: params)

      expect(result).to be_success
      expect(result.record.paid_at).not_to be_nil
    end
  end
end
Enter fullscreen mode Exit fullscreen mode

And here is a simplified spec for the create action:

# spec/requests/invoices_spec.rb

RSpec.describe "Invoices", type: :request, signed_in: true do
  let(:current_user) { create(:user) }
  let(:invoice) { create(:invoice, date_from: "2021-01-01", date_to: "2021-01-31") }

  describe "create" do
    before do
      allow(InvoiceOperator).to receive_message_chain(:new, :create).and_return(
        instance_double("BaseOperator::Result", success?: success, record: invoice)
      )
      post invoices_path, params: {invoice: {title: "Just Invoice"}}
    end

    context "with successful result" do
      let(:success) { true }

      it { expect(response).to have_http_status(:found) }
    end

    context "without successful result" do
      let(:success) { false }

      it { expect(response).to have_http_status(:unprocessable_entity) } 
    end
  end 
end
Enter fullscreen mode Exit fullscreen mode

Summary

This solution was not battle-tested in a large Rails application for a long period of time. But I think it is a simple, readable, predictable and extendable solution.

It solved a lot of what I wanted from it. I am already using it in one application and I, obviously, like it.

I would really welcome any feedback and I hope we can together find an even better solution.

Oldest comments (8)

Collapse
 
citronak profile image
Petr Hlavicka

Thanks for the feedback. The reason is, that I don't want to have the business logic inside the model. I want to have models as a place for things mostly related to the database (like validations and associations and so).

Models with business logic inside have too many responsibilities.

Collapse
 
choltz profile image
Chris Holtz

I completely agree.

In smaller rails applications, one can probably get away with mixing models with business logic, but after a certain size, it becomes a tangled mess.

Separation of concerns is important. If models are dedicated to query and data storage, and operator classes are dedicated to business logic (and presentation logic is in helper/decorator classes and configuration logic is in initializer classes, etc.), then you know exactly where to look for different types of code.

Further, pulling business logic into a PORO opens opportunities for much faster test runs, since the test suite can bypass the DB altogether for some business tests. Similarly, the operator objects in your article, for instance, allow allow one to run tests on code that would otherwise have required the slow overhead of a controller test.

While the database (and therefore model) structures inform the business operations, one does not need to dogmatically adhere to that structure in the business layer. This is particularly the case when your app also relies of data sources beyond the database or when you have multiple apps that need to read information from the same database.

 
citronak profile image
Petr Hlavicka

Thanks, Jared, I will definitely listen the podcast.

Collapse
 
alg profile image
Aleksey Gureiev

I have to second what Petr says. Business logic should never go inside a model. It often involves juggling with different models, external services, jobs and whatnot. Are you suggesting to push it all into models so that a post (for example) had the knowledge of some email sending functionality that it had to invoke when it's created? Absolutely not. Models are mostly DTOs providing their data manipulation and transformation as additional methods.

Then where does the logic go if it's not in the models? Who can have this knowledge of everything business-involved in your system? Controllers? No. And here's why. Controllers are there for processing your HTTP requests -- converting inputs, invoking operations and rendering outputs. Assume that you have other channels your requests for operations come in -- CLI, Websockets, queues... Are you going to duplicate your business logic across all of them? The sane architectural decision is to move all your business logic into a separate layer between your input channels (Rails controllers, for example) and your data layer. Here's where PORO, service objects, interactors, commands (however you call them) come into play.

Of course, everyone is entitled to do whatever they like with their apps. Unfortunately with modern languages and everyone jumping on the cool coding train without classical software engineering education we have a lot of programmers with no idea how to properly architecture their products. My suggestion to everyone looking to make themselves a better programmer is to take time to read works by Martin Fowler, Kent Beck, Robert Martin and others. Time well spent. Peace!

 
alg profile image
Aleksey Gureiev • Edited

Good discussion. I replied to your comment without making any assumptions, so there's nothing personal. Made a specific effort not to rely anything said to anyone. And I never suggested to you specifically to read these sources, but to anyone willing to improve their coding-fu. I'm glad we share the library ticket. Now back to the subject.

As for database records sending emails, where did you get that? I was referring to the models. They don't do that. It's not their responsibility (SRP principle). In my systems, their responsibility lies in working with own data. You can hardly call Rails ActiveRecords and ActiveModels anemic as they provide a ton of functionality on top of plain DTOs.

The term "business logic" is so broad that we must be specific about what we call it. When I disagree with placing it into the models I mainly think about business operations. We got used to putting these into app/actions (named by controller "actions", the meat of which was extracted into their own classes). We also have app/services for general-purpose single-responsibility services for low-level operations.

Controllers in our apps look mostly like:

class ChatsController < ...
  def create
    authorize! Chat, to: :create?

    input = convert_input.(Chats::CreateInput, params)
    chat = create_chat.(input, user: current_user)

    respond_with chat
  end
end
Enter fullscreen mode Exit fullscreen mode

Here we shift non-HTTP stuff out of controller into business operations layer where Chats::CreateChatAction belongs. The added benefit of all this is if you ever thought of cutting your monolithic app into now-modern microservices, you already have your code nicely separated into contexts, but that is a different story.

Hope I made my point clear. Just to reiterate, there's no such thing as ideal and true architecture. We attempt to make our systems as manageable, extensible and decoupled as we can. Moving out business operations into a separate layer helps us a lot.

Collapse
 
epigene profile image
Augusts Bautra • Edited

Thanks for sharing, Petr, it's valuable to keep thinking about how to code better.

I think most everyone agrees that pushing for skinny controllers, where actions generally delegate to some PORO or even the model and then check the result object, is the way to go.

However, in projects that I have worked on we have settled for strongly preferring SRP and thus instead of a single Operator, we define a class for every meaningful business operation. Doing so provides a number of benefits:

  1. Helps avoid helper method hell (calculate_total and calculate_vat in your example)
  2. Supports class-level .call that wraps initialization and execution of the initialized operation. Access to operation instances is not public, only the result object is.
  3. Easy to test - in controllers merely assert that the PORO.call occured with correct arguments and that the result was correctly used to determine the response. No need to use the deprecated receive_message_chain and *_any_instance_of.

I've also dabbled in Trailblazer, and learned a lot. I particularly like the idea of Concepts as being separate from models, a superset that decouples domain entities from the underlying RDB, so we can have concepts that have no underlying table (ad-hoc, read-only concepts), one table (the usual 1-to-1 model), or n number of tables (the fabled composite concept).
From this point of view it may be helpful for developers to forget about Rails models and controller actions for a bit, and focus on their domain, to come up with appropriate domain objects (Models-> Concepts) and Operations, and only then busying themselves with implementing them in the structures Ruby and Rails provides.

Collapse
 
citronak profile image
Petr Hlavicka

Hi Augusts, thank you for your feedback and information 🙂

Collapse
 
apotonick profile image
Nick Sutterer

I'd love to learn what Trailblazer version you were using back then! From what you describe here, it sounds like you were trying out operations in Trailblazer 1.1? As the creator, I can tell you, the #process method (in your case it's #create, #update, etc) was a huge mistake. Those methods quickly got super messy, that is why we came up with Trailblazer 2.0 that, besides the new file structure, focuses on streamlining business logic flow and automatic error handling with the step DSL (described here: trailblazer.to/2.1/docs/activity.h...).
Nice post, and thanks for the link! 🥰