Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Good, idiomatic way to refactor out business logic from controllers

I am new to Scala and Play; and I wrote a "do all" controller that contains both business and presentation logic. I want to refactor the business logic out of the controller.

Here's what my Scala/Play looks like. What is a good/idiomatic way to refactor out the business logic out of this controller, with a clean interface?

object NodeRender extends Controller {
...
def deleteNode(nodeId: Long) = Action { request =>
    //business logic
    val commitDocument = Json.toJson(
    Map(
        "delete" -> Seq( Map( "id" -> toJson( nodeId)))  
    ))
    val commitSend   = Json.stringify( commitDocument)
    val commitParams = Map( "commit" -> "true", "wt" -> "json")
    val headers = Map( "Content-type" -> "application/json")

    val sol = host( "127.0.0.1", 8080)
    val updateReq  = sol / "solr-store" / "collection1" / "update" / "json" <<?
        commitParams <:< headers << commitSend

    val commitResponse = Http( updateReq)()

    //presentation logic
    Redirect( routes.NodeRender.listNodes)
}

In Python/Django I write two classes XApiHandler and XBackend and use a clean interface between them.

xb = XBackend( user).do_stuff()
if not xb:
  return a_404_error
else:
  return the_right_stuff( xb.content) #please dont assume its a view!
like image 995
Jesvin Jose Avatar asked Mar 05 '13 18:03

Jesvin Jose


People also ask

CAN controller have business logic?

The controller is responsible for most of your business logic. If your application is a web application, the controller will handle the requests coming into your application. If your application is a mobile application, the controller will handle the state of your application for your views.

Should a controller have business logic?

Let me tell you this, controllers shouldn't do anything remotely related to business logic, and directly access data stores. The controller's only purpose is to receive a request and return a response. Everything that goes in between is not its responsibility.


2 Answers

A few assumptions:

1) The HTTP call on your second last line blocks

2) You don't say whether the redirect needs to wait for the response from the Http call, but I assume it does.

Blocking call should be moved to another thread so that you aren't blocking threads that handle requests. Play docs are quite specific about this. The Akka.future function combined with Asynchelps.

Controller code:

1 def deleteNode(nodeId: Long) = Action { request =>
2     Async{
3         val response = Akka.future( BusinessService.businessLogic(nodeId) )
4 
5         response.map { result =>
6             result map {
7                 Redirect( routes.NodeRender.listNodes)
8             } recover {
9                 InternalServerError("Failed due to ...")
10            } get 
11        }
12    }
13}

This is a bit more than your PHP, but it is multi-threaded.

The code passed to Akka.future on line 3 will be called at some time in the future using a different thread. But the call to Akka.future returns immediately with a Future[Try] (see below for the return type of the business method). That means the variable response has the type Future[Try]. The call to the map method on line 5 doesn't call the code inside the map block, rather it registers that code (lines 6-10) as a callback. The thread doesn't block on line 5 and returns the Future to the Async block. The Async block returns a AsyncResult to Play and that tells Play to register itself for a callback when the future is completed.

In the mean time, some other thread will make the call to the BusinessService from line 3 and once the HTTP call that you make to the back end system returns, the response variable on line 3 is "completed" meaning that the callback on lines 6-10 gets called. result has the type Try which is abstract and has just two subclasses: Success and Failure. If result is a sucess, then the map method calls line 7 and wraps it in a new Success. If result is a failure, then the map method returns the failure. The recover method on line 8 does the opposite. If the result of the map method is a success, then it returns the success, otherwise it calls line 9 and wraps it in a Success (not a Failure!). The call to the get method on line 10 takes the redirect or the error out of the Success and that value is used to complete the AsyncResult which Play is holding on to. Play then gets a callback that the response is ready and can be rendered and sent.

Using this solution, no threads which service incoming requests get blocked. That's important because for example on a 4 core machine, Play only has 8 threads capable of handling incoming requests. It won't spawn any new ones, at least not when using the default configuration.

Here is the code from the Business Service object (pretty much copied your code):

def businessLogic(nodeId: Long): Future[Try] {

    val commitDocument = Json.toJson(
    Map(
       "delete" -> Seq( Map( "id" -> toJson( nodeId)))  
    ))
    val commitSend   = Json.stringify( commitDocument)
    val commitParams = Map( "commit" -> "true", "wt" -> "json")
    val headers = Map( "Content-type" -> "application/json")

    val sol = host( "127.0.0.1", 8080)
    val updateReq  = sol / "solr-store" / "collection1" / "update" / "json" <<?
        commitParams <:< headers << commitSend

    val commitResponse = Http( updateReq)()

    Success(commitResponse) //return the response or null, doesnt really matter so long as its wrapped in a successful Try 
}

The presentation logic and the business logic are now totally decoupled.

See https://speakerdeck.com/heathermiller/futures-and-promises-in-scala-2-dot-10 and http://docs.scala-lang.org/overviews/core/futures.html for more information.

like image 154
Ant Kutschera Avatar answered Nov 15 '22 19:11

Ant Kutschera


I would probably do it like this

object NodeRenderer extends Controller {

  def listNodes = Action { request =>
    Ok("list")
  }

  def deleteNode(nodeId: Long)(
    implicit nodeService: NodeService = NodeService) = Action { request =>

    Async {
      Future {
        val response = nodeService.deleteNode(nodeId)

        response.apply.fold(
          error => BadRequest(error.message),
          success => Redirect(routes.NodeRenderer.listNodes))
      }
    }
  }
}

The node service file would look something like this

trait NodeService {
  def deleteNode(nodeId: Long): Promise[Either[Error, Success]]
}

object NodeService extends NodeService {

  val deleteDocument =
    (__ \ "delete").write(
      Writes.seq(
        (__ \ "id").write[Long]))

  val commitParams = Map("commit" -> "true", "wt" -> "json")
  val headers = Map("Content-type" -> "application/json")

  def sol = host("127.0.0.1", 8080)
  def baseReq = sol / "solr-store" / "collection1" / "update" / "json" <<?
    commitParams <:< headers

  def deleteNode(nodeId: Long): Promise[Either[Error, Success]] = {

    //business logic
    val commitDocument =
      deleteDocument
        .writes(Seq(nodeId))
        .toString

    val updateReq = baseReq << commitDocument

    Http(updateReq).either.map(
      _.left.map(e => Error(e.getMessage))
        .right.map(r => Success))
  }
}

Where I defined Error and Success like this

case class Error(message: String)
trait Success
case object Success extends Success

This separates your http part and business logic, allowing you to create other types of front-ends for the same service. At the same time it allows you to test your http handling while supplying a mock of the NodeService.

If you need to have different types of NodeService bound to the same controller you might convert the NodeRenderer to a class and pass it in using the constructor. This example shows you how to do that.

like image 38
EECOLOR Avatar answered Nov 15 '22 18:11

EECOLOR