Hexagonal rails: Hiding the finders

This is a brief follow-up to the Hexagonal Rails sessions I did last week at the Scottish Ruby Conference with Matt and Steve. We tried to cram a 3-day course into 45 minutes, with inevitable consequences. So by way of an apology, here’s another brief foray into some of the same territory…

Today I’ve been exploring ways of improving the unit tests in one of my Rails apps. Let’s pretend it’s a blog app, and I have to add a feature to publish posts. I’ve pulled some code out of a controller to make a UseCase object:

(Here ui is a controller, and makes decisions about routing and rendering in response to the events sent by the use case object.)

I’m really sick and tired of looking at code like this. It seems that most of the classes in any Rails app know that finders return nil when they can’t find something, and so they all have to cope with that case. I want to get rid of that if, or at least hide it away in a single place so that it doesn’t contaminate the rest of the app. I want the test to be a bit simpler and more readable too. And I want the option to have richer finders, maybe coping with fuzzy edge cases and still finding the desired database record. In short, I want to wrap the finder in an Adapter.

So I experimented with a couple of alternatives to the above. First, a variant on Smalltalk’s ifTrue:ifFalse message argument style:

I don’t like this because it’s near impossible to isolate and test the interactions. For example, Blog.should_receive(:lookup_post).with( … what?

Another approach might be something like this:

Indeed Rails itself uses this style in its controllers, and so does the inherited_resources gem. But it suffers from the same testing problems as my previous attempt. So I finally settled on this:

This version uses an explicit listener object, which I can thus instantiate and test independently of the calling context. (In real code I might make it an inner class too.) And I’ve already found a couple more uses for the lookup_post method, removing dependencies and ifs along the way.

I find this approach highly readable and testable, nicely separating different responsibilities and allowing me to give them names. But that doesn’t mean I’ll stop exploring. Have you tried other approaches? How did they work out?

Update

Luke points out that I forgot to include the option I actually use the most, the Self Shunt:

This is the same as the listener option above, but the UseCase object passes itself as the listener, thus saving a class and in my opinion) improving readability a little. Can’t think why I forgot it, cos it’s the best and neatest of them all. Anyroadup, thanks Luke!

Advertisements

15 thoughts on “Hexagonal rails: Hiding the finders

  1. Just something to point out … post = Post.find(post_id) will throw an ActiveRecord::RecordNotFound error is no post exists with that id.

    So the spec with Post.should_receive(:find).with(post_id).and_return(nil) is actually hiding a defect in your code.

    You would use Post.find_by_id to return nil if no record exists.

    • @Shane, thanks for pointing that out. I’ve corrected the first gist now.

      And that raises an interesting point. The bug was there because I was trying to write fast tests, so that the feedback loop would be quick and I could get on with writing the post. Under “normal” circumstances I want to write integration tests for every object that touches the framework. But those tests are always slow to run, and there are usually many of them. By wrapping the finder in an object, I can limit the uses of the framework down to a single point. I still need integration tests for the Blog class, to check that it uses the framework correctly, but now I can write fasts tests for everything else by mocking out their interactions with Blog.

      So by hiding the finder I get rid of an ‘if’ and I get faster tests. Win-win.

  2. I created a set of abstractions to help me deal with this in the projects I work on. The bottom line is that I’d rather encapsulate the find -> do something, else -> do something else, and make that reusable. That way you only need to worry about the listener behavior. I also prefer to use an event/callback based approach rather than pass a specific listener (although, it may make sense in a few cases–I need to think about adding that to the event lib. An example of how this works (need to open the code for those modules): https://gist.github.com/3065187

  3. Ah, I still use old-style rails tests, so I never gave much thought to how to do this using mocks. However, I’m trying to change into a more mock-oriented style, so I figured this would be a nice challence. I updated that gist above with the results.

    I tried to find a more general solution to the problem: the Demux class can be used with any kind of collection. TBH, I think this would probably be better modeled using a Demux module that can be mixed into a collection (I added that alternative API to the gist).

    I didn’t test any of this, so I’m not sure if it works as it should. In any case, something to consider. I find using a callback-based pattern without actually passing in the colaborator makes for a more decoupled design.

    Thanks for the opportunity to think about this. I thoroughly enjoyed it, and your article.

  4. Okay, I’m slow… All the solutions you provided seem good ones to me. Can you explain further how your final solution with listeners is the better approach? Also, the hexagonal rails link points to nginx with a bad gateway error.

    • @Justin, The first version causes knowledge of the finder’s API — particularly the magic nil special case value — to be littered throughout my code. The versions with lambdas solve that, but make it pretty difficult to isolate the unit tests so they don’t hit Rails (and therefore also the database). The listener approaches solve all of those problems.

      As for the link, part of the conference site seems to have disappeared. I’ve asked the question…

      • Thanks for explaining that Kevin, that helped a lot. I’ll be looking for the conference site to be updated, I think that will help fill in any additional gaps.

  5. I don’t like the variable name ui. It is not the ui, but an observer which
    receives notifications about post publishing results. This role can be played by the ui, but the publisher should not know about it. In most dynamic languages, the only way to express and differentiate interfaces and concrete implementations, is to use good names.
    I have bad feelings about the instantiation of the PublishesPosts, and I think this problem can traced back to the fack that rspec makes very easy to mock class methods. If the PublishesAPost is a hidden implementation detail, then the test should not know about it. If its not then it should be a normal collaborator which can be passed from the outside. If I would do it this way, I would integrate my test with the PublishesAPost and have only tests for the PublishesPosts.

  6. Thank you for these articles. I hope it’s just the beginning.

    Hexagonal Rails is a very interesting topic and seems to be a pretty new idea in the community. Therefore, it is hard to find a lot of practical examples of how the existing code should be decoupled from the Rails application.

    One of my biggest concerns is models refactoring. As I understand, in the hexagonal architecture ORM (ActiveRecord / DataMapper) acts as an adapter, and all the code, that calls ORM dependent methods, should be kept inside the model class: attribute definitions, validations, associations, and finders.

    My two questions are:

    1. As the Rails model cannot be tested without invoking Rails with all of its dependencies, does it mean, that my model validations do not belong to business logic, and should not be tested by unit tests? It doesn’t seem to make much sense to test validations at integration level either, since some of the models might be called from UI, others as background tasks, and etc. Then how do I test it?

    2. If ORM acts as an adapter, it means, that there cannot be any Model.find(id) (ActiveRecord) or Model.get(id) (DataMapper) calls outside of the Model class (app/models/model.rb), since that would create a dependency on a specific ORM for the entire application. Nevertheless, if I keep all the model related database calls in the same Model class, doesn’t that break the principles of Single Responsibility (it will contain queries for different behaviors of, for example, User model) and Open/Closed (which will cause endless modifications instead of extensions of the same model)?

    Again, I am very interested in this topic, but such dilemmas as these make it hard move to Hexagonal Rails camp and I would be grateful for any possible solutions or examples.

    • @Steve, You’re right, separating/hiding the ORM is the next big challenge. Hiding the finders is only a tiny step towards that. The rest is big and hairy, and a few folks at the Scottish Ruby Conf did begin scratching the surface of the problem.

      It’s too big to discuss in length here, and I will blog more about it soon. Some of my thoughts are:

      * don’t forget you can have models that don’t persist
      * look at the Memento pattern for ideas on how to separate the persisted state from the business object
      * push validations “up” towards the use case objects (https://silkandspinach.net/2012/07/11/hexagonal-rails-rake-tasks-are-adapters/)
      * think about how CQRS chooses what to persist

      This is a huge topic, and I suspect many many people will explore it in the next 12 months…

  7. To test a listener, initiate a connection from a client to any active database controlled by that listener, as described in “Testing Configuration on the Server” . If the only clients available to access the listener are on a different protocol, you must use an Oracle Connection Manager to access the listener.

  8. @steve, regarding the ORM separation, I’ve found it is better not to think of DataMapper or ActiveRecord as the adapter between the business logic and the database, but to create an adapter between the business logic and DataMapper or ActiveRecord.

    Core –> | –> Adapter –> ActiveRecord, DataMapper, or whatever –> Db Server

    I was recently participated in the development of code for loading contractor records from CSV into a table, replacing any/all existing records, I saw that I would need to to replace all existing records with the new ones, and leave everything as it was if any error occurs (i.e. run in a transaction). That operation becomes the responsibility of a single #set_contractors(data) method of our contractor persistence class, which then interacts with the Contractor AR model to perform the operation. An instance of the contractor persistence class is passed to ContractorImporter.new, and the CSV data is passed to ContractorImporter#import_csv.

    All of our tests of the adapter actually exercise the database, since the #set_contractors implementation is trivial, and consists almost entirely of interactions with Contractor. Any unit test would have to basically mock the entire implementation. The tests of ContractorImporter all mock the adapter and do not touch the database. Finally, there are 3 integration tests of a higher level method that passes realistic CSV data to an instance of ContractorImporter that is using the Rails contractor persistence adapter: correct number of rows created, one of the rows is correct, and runs in under 4 seconds.

    We are not testing validation yet, but my thinking is that we’ll introduce a contractor entity class that decorates a contractor data object and uses validations from ActiveModel. Although ActiveModel does come from the Rails project, from the point of view of the core application, that is irrelevant, and does not indicate a dependency on Rails in general — It just happens to be the library we’ll use to assist with performing validation.

Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out / Change )

Twitter picture

You are commenting using your Twitter account. Log Out / Change )

Facebook photo

You are commenting using your Facebook account. Log Out / Change )

Google+ photo

You are commenting using your Google+ account. Log Out / Change )

Connecting to %s