Inversion of control in controller by a mediator

Question

A user has to perform a somewhat complex task on my website (submitting a rating). For the sake of example, let's consider it's a rating for a movie, done through a controller. There are several possible "paths" the logic could follow

• The user wants to cancel his rating submission (eg he didn't go to the movie)
  • He provides all the required parameters OR not
• The user submits the rating submission
  • Providing all the required parameters OR not
• The user had already submitted his rating ( and is denied the action)
• Other reasons (eg movie deleted, etc.)

I am trying to encapsulate all the logic in a Service/Mediator, so I can also reuse the logic outside a controller (eg. in an operational console).

I decided to make the controller an argument during this RatingService instanciation, and this is turning into an inversion of control/the service does everything. My service knows what state it is currently in, and triggers various callbacks (notification, tracking, etc.) including rendering "flashes" for the user through the controller.

This starts smelling weird since my service is slowly taking possession of the controller for itself.

I am showing some sample code (Ruby on Rails) just to illustrate my point

class ReviewController
  def review
     ReviewService.review(reviewable, current_user, 
                          controller: self, **review_params)
  end
end

class ReviewService
  def review
     ...
  end

  private

  def cancelled_submission?
    ...
    controller.flash(I18n.t(:cancelled)) if controller.present?
  end

  def submit_rating
    ...
    controller.flash(I18n.t(:rating_submitted)) if controller.present?
  end

  def already_submitted
    ...
    controller.flash(I18n.t(:already_submitted)) if controller.present?
  end
end

I am wondering if it would be a better practice to consider a full inversion of control, and only declare methods in my controller that can be used in the service.

That is, turn

class ReviewController
  def review
     ...
  end
end

class ReviewService
  def review
     ...
  end

  private

  def cancelled_submission?
    ...
    controller.flash(I18n.t(:cancelled)) if controller.present?
  end
end

Into

class ReviewController
  def review
     ...
  end

  # Public method, meant to be called back from the service in an inversion of control fashion
  def notify_cancelled
    flash(I18n.t(:cancelled)
  end
end

class ReviewService
  def review
     ...
  end

  private

  def cancelled_submission?
    ...
    controller.notify_cancelled if controller.present?
  end
end

..or if there are better practices out there

Answer 1

5 years later my view on this question

The service object

• should handle error generation on its side (like populate an "errors" ivar, or adding ActiveModel errors directly on object)
• should not have a strong dependency on the controller (although it's often convenient, for retrieving current_user, etc.)

The controller

should handle how these errors are sent as a response.

• A json controller could return the error directly in the error payload, or add it as a meta if the error is more like a warning.
• For a controller rendering an html view, the controller can read the service errors and populate its Rails "flash" array after the service is done doing its business.

What we use(d) in practice and thoughts

We use a proxy pattern to add controllers as a soft dependency in services. It is helpful mainly regarding the current_user, request object, and error context propagation to bug handler :when a service raises an error, we can automatically fetch the service.controller if it is available to add extra context, like the request, IP, etc. Furthermore, our proxy handles passing the controller to a subservice called from the main service.

# some concern/utility
def controller_proxy(controller)
  @controller = if Kernel.instance_method(:class).bind(controller).call <= 
    ServiceControllerProxy
      controller
    else
      ServiceControllerProxy.new(controller)
    end
  end

# service
class MyService
  def initialize(controller: nil)
    controller_proxy(controller)
  end

# proxy implementation
class ServiceControllerProxy < BasicObject
  def initialize(controller = nil)
    @controller = controller
  end 

  def method_missing(method_name, *args, &block)
    if @controller.present?
      return if controller_should_not_handle?(method_name)

      @controller.public_send(method_name, *args, &block)
    elsif cannot_be_used_without_controller?(method_name)
      report_use_of_proxy_and_missing_controller(method_name, args, block)
    else
      super
    end
  end

This was fine when the context back then, was using the service through a controller or through the rails console.

The implementation led to some problems when we started to replace our JSON controllers by GraphQL controllers. Stuff like "request" made less sense because the graphql endpoint is always /graphql and the "path" of the request needs to be determined differently.

Also, using this proxy is akin to doing black magic, especially for new guys who do not know about this ; and using basic objects require weird precautions (see this use of Kernel.instance_method.bind). As a more experienced developer, I would avoid using this and keep the code simpler, even if it leads to less dry code.