Prototype: Service Objects

[waiting-for-dev]

This PR is a prototype with a candidate API for introducing a service layer on Solidus. It's not intended to be merged but serves as a starting point to discuss how it can be built.

Assumptions

Introducing a service layer will benefit the platform's architecture, leading to Solidus's more straightforward evolution and extensibility.

Goals

• Provide the most extensible API so that we minimize the pain that stores have to customize/extend the default behavior in Solidus.
• Provide the maximum possible backward compatibility, so we don't disrupt existing stores.
• Find the best balance between simplicity and power.
• Be the least disruptive to new contributors.

Out of scope

• Implementation details. The code is only a PoC to demonstrate that we can build the proposed API. Much more work will be needed for a better architecture of the scaffolding.
• Business domain code. The operations in the example have some flaws as they were extracted from the current implementation. That could be refactored to fit better.

Approach

We aggregate concepts from Railway Programming, functional programming, and monadic composition and shape them into an idiomatic DSL in Ruby where the previous buzzwords are not needed to explain the API behavior in a simple way.

Description

A service object is designed as a chain of operations (a business transaction), running by default within a database transaction.

Each operation needs to return a Spree::Result instance, which may be of two sub-types: Spree::Result::Success (which wraps a returned value) or Spree::Result::Failure (which wraps an error). The chain is halted whenever a failure is returned, and subsequent operations are not run. Thus, the final result is the last operation's success or the first encountered failure.

Although each operation returns a Spree::Result instance, the transaction blocks allow us to work with them as if we only considered the happy path. The otherwise returned Spree::Result::Success instances are unwrapped into the underlying result value, while the transaction ends whenever a failure is returned. E.g.:

class CreateUserTransaction
  include Spree::Transaction

  transaction do |attributes|
    user = create_user(attributes)
    send_welcome_email(user)
  end
end

In the previous example, the create_user operation might return a #<Spree::Result::Success...@result = #<Spree::User...> or a #<Spree::Result::Failure...@error = #<ActiveModel::Errors...>. However, in the case of success, the user variable would be bound to the Spree::User instance. In case of failure, the execution flow wouldn't reach the send_welcome_email step.

Each operation may be anything responding to #call, taking any arguments, and, as said, returning a Spree::Result. E.g.:

class CreateUser
  def call(attributes)
    user = User.create(attributes)
    if user.persisted?
      Spree::Result.success(user)
    else
      Spree::Result.failure(user.errors)
    end
  end
end

The controller accesses registered services through a services helper method. Its responsibility is reduced to:

• Prepare the input for the service
• Call the service
• Prepare the HTTP response depending on the transaction result

class UsersController < Spree::BaseController
  def create
    result = services[:create_user_transaction].call(user_params)
    if result.success?
      flash[:success] = "User created"
      redirect_to user_url(result.result!)
    else
      flash[:error].now = "Please, fix the errors"
      render :new, errors: result.error!
    end
  end

  # ...
end

We could simplify the controller flow by adding pattern matching behavior to the Spree::Result object. However, we first need to remove support for Ruby v2.5 & v2.6 (which are already deprecated on Solidus):

  def create
    case services[:create_user_transaction].call(user_params)
    in Spree::Result::Success(user)
      flash[:success] = "User created"
      redirect_to user_url(user)
    in Spree::Result::Failure(errors)
      flash[:error].now = "Please, fix the errors"
      render :new, errors: errors
    end
  end

Everything registered within core/app/services is registered as a service, while all within core/app/services/operations is available as operations to be used (we probably would need to adjust this convention and not have operations nested within the services).

The interesting part for users is that they could register their own services and replace ours (also with something that is not a Spree::Transaction, as long as it returns a Spree::Result).

The most powerful help would be that they could inject individual operations without overriding the whole service. They would be able to register in config/initializers/spree.rb (API open to be revisited):

Rails.application.config.to_prepare do
  Spree::Config.operation_registry.merge(
    send_welcome_email: AmazingStore::CustomWelcomeEmail.new
  )
  # same with Spree::Config.service_registry for the whole service
end

Example in the prototype

As said offline, we've chosen the service to udpate a user from the backend for the PoC. We can also obtain good insight by studying the test file for Spree::Transaction.

As we can see running backend/spec/controllers/spree/admin/users_controller_spec.rb, backward compatibility is maintained when it comes to the HTTP request/response cycle.

Possible improvements

• Make transactions inheritable
• Instrumentation
• Automating things like publishing an event for any operation/transaction
• Adding after:, before: hooks to inject operations between the already defined in the transaction (danger of going too wild)
• Transaction composition

Open questions

• Is it ok to keep backward compatibility only on HTTP requests and responses? Otherwise, do we need to add the service layer as an optional parallel path?
• Do we want to build it as a separate and generic gem? That would help to have concerns decoupled and Solidus less of a monolithic.

[kennyadsl]

@waiting-for-dev that's fantastic, simple yet powerful. Something I would like to explore in the future is the ability to change the order of the operations (or add one at some specific position) without overriding the whole service. I think that could be interesting to chain changes to services from different places, for example, two different extensions that want to add operations to the same service.

[cesartalves]

Really like the approach taken here! Chaining of operations looks pretty powerful and I like the DSL overall :)

[cesartalves]

I wonder if the db: true parameter isn't a smell this should actually be two methods in the future - a db transaction and a block of aggregated operation within a service look different to me.

The first is atomic, ie can be treated as a single operation, while the second is not atomic in all scenarios - it would be highly dependent on the operations chained; anything with side-effects that is not the last call would break this.

Am I overthinking this? 😅 if so, feel free to ignore!

[waiting-for-dev]

Hey, @cesartalves, thanks for looking into it 🙌

I think both db: true & db: false should be considered the same in terms of atomicity. In a purely functional programming language, that would be the case. However, in Ruby (and most other languages, FWIW), we're free to do whatever we want. For instance, it's also true that you can open an ActiveRecord::Base.transaction block, create a file within, and roll it back. The file will still exist after rolling back, so there we had a side effect. The transaction block opens a business transaction, and the db parameter can make it also a database transaction.

I agree that a boolean parameter is a smell most of the time, as the caller already knows which path to take. Ideally, we could have two separate methods, like transaction & transaction_with_db. However, given how implicit database transactions are in Rails, I think it's better to default to the safe side. We could also have transaction & transaction_without_db, but it looks ugly 🙂 I'm open to discussing it further.

[cesartalves]

The magic introduced to access operations with underscore names as opposed to specifying the classes in the registry is pretty interesting 💡

[waiting-for-dev]

@waiting-for-dev that's fantastic, simple yet powerful. Something I would like to explore in the future is the ability to change the order of the operations (or add one at some specific position) without overriding the whole service. I think that could be interesting to chain changes to services from different places, for example, two different extensions that want to add operations to the same service.

Thank you, @kennyadsl!

Yeah, I agree that would be an excellent feature for Solidus extensions! I already pointed out something similar in the "Improvements section":

Adding after:, before: hooks to inject operations between the already defined in the transaction (danger of going too wild)

We'd need to find a good balance between flexibility and maintainability. IMO having before: and after: hooks into something is sometimes an anti-pattern, as it makes it very complicated to follow the execution flow. If we add the possibility to compose transactions, plus the option to inject custom operations that call super, that will go a long way toward flexibility. I'm open to discussing more before: & after: if that's not enough.

[nirebu]

@waiting-for-dev I really like the approach that has been taken for this. I feel that having services as a composition of operations right in the core of Solidus will encourage the same pattern to be used in other parts of clients' codebases. I think that this approach will help a ton the debugging when things go wrong.

[waiting-for-dev]

I think that this approach will help a ton the debugging when things go wrong.

Thanks, @nirebu! In terms of debugging, I'm thinking that it'd be much better to be explicit about what dependencies are included, instead of blindly delegating to the registry of operations. The API could look something like:

class CreateUserTransaction
  include Spree::Transaction[
    :create_user,
    :send_welcome_email
  ]

  transaction do |attributes|
    user = create_user(attributes)
    send_welcome_email(user)
  end
end

I think it won't be something difficult to achieve (famous last words 🙂 )

[jarednorman]

There's some really interesting stuff in here. Thanks for kicking of this conversation, Marc. Before I critique anything here, I just want to be clear that I'm in favour of adopting a service layer in Solidus that stores can leverage as an extension point. I think this is a great idea.

That said, I do have some concerns about much of the direction of this PR, but nothing that I think is too intrinsic. My main worry is that the metaprogramming in this PR wouldn't exactly "be the least disruptive to new contributors".

Stuff I like:

• services helper at the controller layer for accessing operations.
• The lightweight monad-style of returning Spree::Result.
• How this adds a reasonable way to access existing Solidus functionality without a ton of glue.

Stuff I'm not so sure about:

• The naming of Spree::Transaction. This prototype has us calling services to access "transactions". Seems like a disconnect. While "Service" isn't completely free of this, "Transaction" is extremely overloaded in the web/database context, so I'm hesitant to add a new kind of transaction, especially since this also involves database transactions.

• Spree::Transaction.transaction: The DSL method doesn't seem like it gives us much here, except the db argument, which we can also pass in when including Spree::Transaction. I'm not sure what this adds. I would just make this a normal method.*

• Multiple Registries: Do we really need multiple registries? What's the goal here? I feel like Solidus should provide a unified and extensible service layer, not multiple. I'd prefer something where we provide the single Solidus registry (that potentially has multiple resolvers) and supports overriding and adding additional resolvers.

• The metaprogramming in Spree::Transaction: Including it calls a separate method that generates a new module that contains its own own included hook that sets everything up and includes that. 🥴

  It took me a second to understand, but I get why you did this. It's not exactly intuitive code and I really hope there's some less obtuse way of accomplishing the same thing... though I sadly am not currently sure what it would be.

---

*Upon further discussion with @adammathys, I've realized what Spree::Transaction.transaction stuff gives us.

[jarednorman]

To summarize: I really like most of what this gets us, but the file containing Spree::Transaction contains some really gnarly and unapproachable code that I think will be hard for people to understand and work with.

[tvdeyen]

Thanks. I like where this is going a lot. I second everything what Jared already said (really a great written comment, Jared).

Have you thought about adopting dry-rb libraries (especially dry-monads)?

We have huge success with it in our custom frontend we wrote for Solidus at CandleScience and are happy to share some of the ideas and commands we wrote.

The benefit would be that we do not need to maintain that crazy meta programming that is necessary to make this work in Solidus and rely on the great community that stands behind dry-rb.

[waiting-for-dev]

Many thanks for looking into it, @jarednorman & @tvdeyen! ❤️

The naming of Spree::Transaction. This prototype has us calling services to access "transactions". Seems like a disconnect. While "Service" isn't completely free of this, "Transaction" is extremely overloaded in the web/database context, so I'm hesitant to add a new kind of transaction, especially since this also involves database transactions.

I highly agree here. I'll come up with a better name. We can probably go with Spree::Service.

Multiple Registries: Do we really need multiple registries? What's the goal here? I feel like Solidus should provide a unified and extensible service layer, not multiple. I'd prefer something where we provide the single Solidus registry (that potentially has multiple resolvers) and supports overriding and adding additional resolvers.

Makes sense. I want to revisit that stuff, regardless. We want to be explicit about what is included as a dependency. That means a single registry will be enough, and there'll be no need to separate between registries for operations vs. services.

The metaprogramming in Spree::Transaction: Including it calls a separate method that generates a new module that contains its own own included hook that sets everything up and includes that. 🥴

I wanted to leave implementation details out of scope. The code is far from being production ready. It's just a POC to show that API can be build.

Have you thought about adopting dry-rb libraries (especially dry-monads)?

Yes, I've thought about it a lot 🙂 However, notice that at this point, there's no dry-rb library for that. It used to be dry-transaction, but it has been deprecated. dry-monad's do notation would fit part of it, but it'd fall short of allowing the extensibility we need for operations and to provide instrumentation. Also, I think it'd be good to depend on a monad interface instead of an implementation. But I do want to make it compatible with dry-rb.

[jarednorman]

I wanted to leave implementation details out of scope. The code is far from being production ready. It's just a POC to show that API can be build.

Gotcha. I think my main concern is how tricky it might be to accomplish something that's gets us all this great stuff without the implementation being too spooky.

Yes, I've thought about it a lot 🙂 However, notice that at this point, there's no dry-rb library for that. It used to be dry-transaction, but it has been deprecated. dry-monad's do notation would fit part of it, but it'd fall short of allowing the extensibility we need for operations and to provide instrumentation. Also, I think it'd be good to depend on a monad interface instead of an implementation. But I do want to make it compatible with dry-rb.

I think I agree with maintaining our own implementation here, give this. That side of things is relatively straightforward and if we can come up with an implementation that's easier to grok, what we have here seems both simple to use and pretty powerful.

[waiting-for-dev]

I'm in the process of creating the actual implementation for the prototype. In the process, I've realized that it's possible to transform the proposed API:

class CreateUserTransaction
  include Spree::Transaction

  transaction do |attributes|
    user = create_user(attributes)
    send_welcome_email(user)
  end
end

into

class CreateUserService < Spree::Service
  def call(attributes)
    service do
      user = create_user(attributes)
      send_welcome_email(user)
    end
  end
end

The caller wouldn't need to do anything else than before:

services[:create_user_service].call(user_params)

That would address @jarednorman feedback:

Spree::Transaction.transaction: The DSL method doesn't seem like it gives us much here, except the db argument, which we can also pass in when including Spree::Transaction. I'm not sure what this adds. I would just make this a normal method.*

And would allow using any method in the class as regular, so probably it's preferable.

[waiting-for-dev]

BTW, progress can be followed at https://github.com/nebulab/kwork/

[jarednorman]

@waiting-for-dev I like that!

I was thinking yesterday about how this all might help us improve the architecture and more so extensibility of the order updater. This could be a really big improvement here.

[waiting-for-dev]

I just added a new commit adopting the external library. Everything is still WIP, but the current interface looks like this:

class CreateUserTransaction < Spree::Transaction
  operations :create_user,
             :send_welcome_email

  def call(attributes)
    transaction do  
      user = create_user(attributes)
      send_welcome_email(user)
    end
  end
end

[waiting-for-dev]

Closing, as the prototype is now completed.