These are my notes from a lightning talk I gave at this months LRUG.

We’ve got a Rails app which lets a user login with a username and password, nothing out of the ordinary:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24

class SessionsController < ApplicationController
  def create
    if self.current_user = User.authenticate_with_password(params)
      redirect_to root_url
    else
      render :new
    end
  end
end

class User < ActiveRecord::Base
  def self.authenticate_with_password(params)
    first(username: params[:username]).
      try(:authenticate_password, params[:password])
  end

  def authenticate_password(unencrypted_password)
    if BCrypt::Password.new(password_digest) == unencrypted_password
      self
    else
      false
    end
  end
end

SessionsController#create tries to find a User with the supplied username and password and then redirects or re-displays the login form.

A few months go by and we decide to let users login against a number of third party services, using something like OmniAuth to do the actual heavy lifting of communicating with the providers. Our controller and model have bloated a fair bit:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49

class SessionsController < ApplicationController
  def create
    if self.current_user =  if params[:facebook_id]
                              User.authenticate_with_facebook(params)
                            elsif params[:google_id]
                              User.authenticate_with_google(params)
                            elsif params[:twitter_id]
                              User.authenticate_with_twitter(params)
                            elsif params[:github_id]
                              User.authenticate_with_github(params)
                            else
                              User.authenticate_with_password(params)
                            end
      redirect_to root_url
    else
      render :new
    end
  end
end

class User < ActiveRecord::Base
  def self.authenticate_with_password(params)
    first(username: params[:username]).try(:authenticate_password, params[:password])
  end

  def self.authenticate_with_facebook(params)
    first(facebook_id: params[:facebook_id])
  end

  def self.authenticate_with_google(params)
    first(google_id: params[:google_id])
  end

  def self.authenticate_with_twitter(params)
    first(twitter_id: params[:twitter_id])
  end

  def self.authenticate_with_github(params)
    first(github_id: params[:github_id])
  end

  def authenticate_password(unencrypted_password)
    if BCrypt::Password.new(password_digest) == unencrypted_password
      self
    else
      false
    end
  end
end

There’s not much good to say about that code, but I’ve seen similar things in plenty of apps over the years. At least we’re not doing the db calls directly in the controller.

Single Responsibility Principle

The Single Responsibility Principle basically says that an object should be responsible for one thing only. Thinking about that another way, we can also say that a single change should only touch one part of the system.

With the code as it stands, if we add or remove an authentication method we’ve got to change code in both the controller and the model. Fixing that is pretty simple, let’s just move the logic for deciding which authentication method we’re using into the model:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25

class SessionsController < ApplicationController
  def create
    if self.current_user = User.authenticate(params)
      redirect_to root_url
    else
      render :new
    end
  end
end

class User < ActiveRecord::Base
  def self.authenticate(params)
    if params[:facebook_id]
      authenticate_with_facebook(params)
    elsif params[:google_id]
      authenticate_with_google(params)
    elsif params[:twitter_id]
      authenticate_with_twitter(params)
    elsif params[:github_id]
      authenticate_with_github(params)
    else
      authenticate_with_password(params)
    end
  end
end

All tidy, now if we change how authentication works we only have to make changes in one part of the codebase.

Skinny controllers, fat models

This is basically the skinny controllers, fat models principle which encourages moving your business logic out of your controllers and into your models.

This lets us keep nice clean controllers and views, but we can end up with massively bloated models. What we should aim for is not just skinny controllers, but skinny models too. In fact, we want really want everything to be skinny. Lets do some refactoring!

When all you have is a hammer…

Being a Ruby developer, our first port of call is simply to move out the authentication code into a module, using the standard include/extend pattern to bring both class and instance methods along:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27

class User < ActiveRecord::Base
  include UserAccess
end

module UserAccess
  def self.included(base)
    base.extend ClassMethods
  end

  module ClassMethods
    def authenticate(params)
      # ...
    end

    def authenticate_with_password(params)
      # ...
    end

    def authenticate_with_facebook(params)
      # ...
    end
  end

  def authenticate_password(unencrypted_password)
    # ...
  end
end

ActiveSupport::Concern can clean this up a little bit and brings a few other tricks to the table.

Have we actually done anything?

We’ve got better organised code and given this concept of “user access” a name by wrapping it up in a module. We’ve also made it easier to test as we can test this module outside of Rails making our tests faster, which is nice.

In effect all we’re doing is cleaning up the source so it’s easier to find things, we’re not modelling the problem any better. We’re still treating the User as a bucket of methods without giving any real thought as to where these things belong.

Over time we include more and more functionality into the one model, hardly a “single responsibility”:

1
2
3
4
5
6
7
8
9
10
11

class User < ActiveRecord::Base
  include UserAccess
  include NameAccessors
  include LoginAuditing
  include UserSerialising
  include RecencyLogging
  include UserInvites
  include UserAvatar
  include UserPermissions
  include Kitchen::Sink
  ...

What should the User care about?

All the User class should really care about is persistence - storing and retreiving the attributes from the database.

Anything other than that is really outside of its scope, I’d argue that even observers, validation & callbacks don’t belong in the model most of the time.

If we look back at the UserAccess module, it’s pretty self contained and would quite happily exist outside of the User model. Other than the #authenticate_password method it’s all just class methods which go off and try and find a User.

With very few changes we can make this a stand-alone module:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16

module UserAccess
  def self.authenticate(params)
    # ...
  end

  def self.authenticate_with_password(params)
    user = User.first(username: params[:username])
    if BCrypt::Password.new(user.try(:password_digest)) == params[:password]
      user
    end
  end

  def self.authenticate_with_facebook(params)
    # ...
  end
end

I’ve inlined the instance method, it could just as easily be another class method which takes the user and password, it doesn’t really matter at this point. The important thing is that it has allowed us to remove the mixin from the User model, leaving it doing just one thing - handling persistence.

The controller needs changing to point to our new stand-alone module, but that’s a trivial change:

1
2
3
4
5
6
7
8
9

class SessionsController < ApplicationController
  def create
    if self.current_user = UserAccess.authenticate(params)
      redirect_to root_url
    else
      render :new
    end
  end
end

Skinny controller, skinny model, we’re done. Right?

So we’ve got a nice clean model and a nice clean controller, but what about the UserAccess module? It’s a bit of a mess, but at least it’s swept into one self contained part of the system so we can refactor this without affecting anything else.

Back to the Single Responsibility Principle, lets split up the module into a sub-module per authentication type, that way each is nicely self contained and responsible purely for the one authentication scheme:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27

module UserAccess
  def self.authenticate(params)
    if params[:facebook_id]
      Facebook.authenticate(params)
    elsif params[:google_id]
      Google.authenticate(params)
    elsif params[:twitter_id]
      Twitter.authenticate(params)
    elsif params[:github_id]
      GitHub.authenticate(params)
    else
      Password.authenticate(params)
    end
  end

  module FaceBook
    def self.authenticate(params)
      # ...
    end
  end

  module Password
    def self.authenticate(params)
      # ...
    end
  end
end

That’s a bit cleaner, but we’ve still got that nasty .authenticate method and we’re back with the problem that if we add or remove an authentication method we’re going to have to change code in more than one place. Should the top level UserAccess module really know about the logic which determines which sub-module to use?

Chain of Responsibility

What we really want to do is move the logic from the .authenticate method down into the sub-modules. This is where something like the Chain of Responsibility pattern comes in handy.

Instead of choosing which authentication type to use up front, we ask each module one at a time whether it can handle the submitted parameters:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32

module UserAccess
  AUTH_TYPES = [
    FaceBook,
    Twitter,
    GitHub,
    Password
  ]

  def self.authenticate(params)
    AUTH_TYPES.detect{|auth| auth.can_handle?(params) }.try(:authenticate, params)
  end

  module FaceBook
    def self.can_handle?(params)
      params.has_key?(:facebook_id)
    end

    def self.authenticate(params)
      # ...
    end
  end

  module Password
    def self.can_handle?(params)
      params.has_key?(:username) && params.has_key?(:password)
    end

    def self.authenticate(params)
      # ...
    end
  end
end

Now we just loop through the authentication modules and find the first one which can handle the parameters we have and then calls authenticate on it.

Each module is completely responsible for its logic and if we add or remove an authentication method we only have to change one thing.

I’m using an array of types here to loop through, but you could also just loop through the sub-modules of the UserAccess module. You could also get rid of the .can_handle? method by just calling .authenticate and returning the User for success, nil for a failure and false if it doesn’t handle the params, but I prefer to be explicit and having a return nil vs false can lead to much confusion.

Here’s a high level overview of what we’ve ended up with, skinny controller, skinny model and a skinny set of modules all responsible for one thing only:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28

class SessionsController < ApplicationController
  def create
    UserAccess.authenticate(params)
  end
end

class User < ActiveRecord::Base
end

module UserAccess
  def self.authenticate(params)
  end

  module FaceBook
  end

  module Google
  end

  module GitHub
  end

  module Twitter
  end

  module Password
  end
end

There’s plenty of other approaches to refactoring code like this, but I hope this gives a few ideas beyond “I know, I’ll move it into a module”.

I’m currently working on a project which has an API backend and a JS frontend which consumes that API. Both parts are built with Rails and must be served from the same domain and port because of the same origin policy.

The API will be served from a sub-directory like so:

• http://example.com - serves the JS app
• http://example.com/api - serves the API

It’s pretty trivial to set this up with nginx, but developing locally is a bit trickier. Running both apps with rails server will put them on different ports and the JS app won’t be able to communicate with the API.

We could setup a local nginx config on our development machines, but this makes it harder to setup breakpoints in ruby-debug amongst other things.

Rails apps are just Rack apps, so my first thought was to create a config.ru which mounts both Rails apps:

1
2
3
4
5
6
7
8
9
10
11
12

require File.expand_path('../api/config/environment',  __FILE__)
require File.expand_path('../frontend/config/environment',  __FILE__)

run Rack::Builder.new {
 map "/api" do
   run API::Application
 end

 map "/" do
   run Frontend::Application
 end
}

This raises an error saying You cannot have more than one Rails::Application so that’s that idea out the window.

We could turn the API into a Rails Engine and mount that inside the other app, but we really want these two apps to be completely separate and not have to know about each other outside of the documented API.

The obvious solution is to use a proxy to let us run each Rails app independently and have the proxy forward requests to each one depending on the URL.

The simplest thing I could think to setup a proxy server was to use Rack::Proxy and about 5 minutes later I had a working solution:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15

require 'rack-proxy'

class AppProxy < Rack::Proxy
  def rewrite_env(env)
    request = Rack::Request.new(env)
    if request.path =~ %r{^/api}
      env["HTTP_HOST"] = "localhost:3001"
    else
      env["HTTP_HOST"] = "localhost:3000"
    end
    env
  end
end

run AppProxy.new

Pretty simple, we just rewrite the HTTP_HOST depending on whether or not the requested path starts with “/api”.

Now we fire up the frontend and backend Rails apps on port 3000 and 3001 respectively, run the proxy on another port and point the browser there.

Using rackup config.ru worked fine, but when I tried to run the proxy using passenger-standalone I got the following error:

1
2
3
4
5
6
7
8
9
10
11

=============== Phusion Passenger Standalone web server started ===============
PID file: /Users/rlivsey/Sites/multi-rails-experiment/tmp/pids/passenger.9999.pid
Log file: /Users/rlivsey/Sites/multi-rails-experiment/log/passenger.9999.log
Environment: development
Accessible via: http://0.0.0.0:9999/

You can stop Phusion Passenger Standalone by pressing Ctrl-C.
===============================================================================
2012/02/23 14:11:52 [error] 9691#0: *4 "/Users/rlivsey/Sites/multi-rails-experiment/public/index.html" is not found (2: No such file or directory), client: 127.0.0.1, server: _, request: "HEAD / HTTP/1.1", host: "0.0.0.0"
2012/02/23 14:12:07 [error] 9691#0: *5 "/Users/rlivsey/Sites/multi-rails-experiment/public/index.html" is not found (2: No such file or directory), client: 127.0.0.1, server: _, request: "GET / HTTP/1.1", host: "localhost:9999"
2012/02/23 14:12:07 [error] 9691#0: *5 open() "/Users/rlivsey/Sites/multi-rails-experiment/public/favicon.ico" failed (2: No such file or directory), client: 127.0.0.1, server: _, request: "GET /favicon.ico HTTP/1.1", host: "localhost:9999"

This is because passenger-standalone sets up the nginx config expecting there to be a public directory, so I just created an empty one and everything worked fine.

With this setup we can also trivially switch the API host to point to production, letting us develop the frontend against the production API should we want to test the UI with live data.

It’s been a few days since we turned on payment and, in my eyes at least, officially shipped MinuteBase. We’ve been in beta for quite some time, but having paying customers is the major milestone which transforms it from a “project” into an actual business. I’ve learned a huge amount and thought that now is a good time to look back at our progress.

What went well

We based MinuteBase on a problem we actually experienced

I’ve worked on a number of projects over the years which I thought were a good idea, but weren’t solving a problem that I actually faced. This lead to either solving the wrong problem, or just getting bored once the rush of building something new wore off. Just because you experience a problem doesn’t necessarily mean that it’s a viable business, but it certainly helps.

I had a co-founder

This has been essential for getting through the inevitable slumps in motivation which occur. It’s much easier to quit when there’s no one else involved. We’ve not always agreed, far from it, but the many hours of discussion and arguments have lead to a much better product.

Having a co-founder with complementary skills is also essential. I’m a pretty good developer but I’m never going to be mistaken for a designer. I like to think I have a reasonably good eye, but put me in front of PhotoShop for a year and it’s unlikely that I’ll produce anything as beautiful as MinuteBase.

We launched early

Our first version wasn’t quite an MVP, we probably could have launched earlier, but looking back it’s amazing what features we didn’t have. Having actual users other than ourselves tell us what they are missing has been fantastic. It also means we’ve been able to focus on things which are actual problems, instead of imagined “essential” features which in the end don’t matter all that much.

We dog-fooded right from the beginning

Because MinuteBase was built to solve a problem we actually had, we were able to use it right from the earliest stages in the companies we worked for and to collaborate with our clients. We also use MinuteBase to build MinuteBase, by writing up all our meetings and discussions, sharing documents and tracking the tasks and actions as we go.

Using it every day means we have a better idea of where we need to improve than having to wait for other people to tell us. I’m even using it right now to write up this blog post, you can see it at MinuteBase here.

What could be improved

Iterations were too long

MinuteBase 2 initially started off as some small improvements to the prototype app, there’s nothing there which we couldn’t have added in iteratively as we went. Instead we put too many changes and into one release and ended up taking far too long to get changes in-front of our users where we could get feedback.

We changed technology stack mid-stream

The first version was built using Merb, MySQL, DataMapper and Prototype. Our version 2 is Rails 3, MongoDB, MongoMapper, ElasticSearch and jQuery. Very little code survives from the initial prototype.

This meant that far too much time was spent re-building things which already worked instead of on improvements. It also meant that it took us much longer to get in front of our users as we couldn’t run both versions side by side sharing the same database.

However, building one to throw away meant that when we were building “version 2” we had a much better idea of what worked and what didn’t. We were able to make more fundamental changes to the way the app worked than if we were iteratively changing the prototype.

We didn’t turn on payment early enough

There’s no reason why we couldn’t have enabled payment 6 months ago, in version 1 of the app. Instead we convinced ourselves that it wasn’t ready, and that we’d turn on payment after “this one next feature” or bug fix. Of course because our iterations were too large, that “one next feature” ended up taking months, during which time we could have been bringing in money and proving the business model. We even had people asking how they could pay us!

We didn’t have a “business guy”

After going through this process I think the ultimate founding team is a designer, a developer and a business-person. While we’re building the product there’s no one focused on sales & marketing or just getting out there and talking to people.

That’s not to say that we shouldn’t or couldn’t be doing more of that side of things ourselves, but it’s easy to put off going out & talking to people, or drumming up press until after you’ve “finished” building the app. And you never really finish.

We’ve been too quiet

If you look at the MinuteBase blog or Twitter stream, you’ll see there’s not a lot there. All the posts are about changes to the app and new features.

We need to get much better at producing original content and linking to interesting material so that the blog itself can work as a marketing channel. As it stands, unless you’re a MinuteBase user, there’s not much point subscribing to our blog or following us on Twitter.

This has to change and we’re going to be spending much more time on this in future.

MinuteBase might be too specific a name

Our original focus was to build the best tool to take meeting minutes and we chose our name based on that. Call it scope creep, or pivoting, but the MinuteBase of today does far more than just meeting minutes.

With the introduction of workspaces, MinuteBase has turned into a great project management tool but our name is still focused on one part of the app.

Time will tell how much of a problem this is, but having a more generic name or something focused on meetings instead of minutes could have been a better idea.

In Closing

So many of these lessons are things should have already known. In my day job managing projects over the years I preached agile development, small iterations, test driven development. Even things I didn’t have first hand experience in I should have known via talking to others or from Hacker News over the years.

For some reason when you’re building it for yourself and don’t have anyone to report to these things go out of the window.

This process has made me a much better developer, a much better manager and no matter what happens to MinuteBase I’ve no doubt that it makes me a far more capable person than I was before.

If you go to meetings, or manage projects, why not give MinuteBase a try.