Adding a new client - part two
So we started building our first feature in tiny little steps. And we've not achieved very much so far - we know we can't do anything if not logged in, nor can we do anything if we're not an admin.
But today we're going to put some big foundations in place. As I mentioned earlier, I've implemented a version of this before so I have a good idea of how I think it's going to end up. However, we're going to let the specifications drive us forward - it might turn out that my ideas weren't the best (or simplest) way of building this.
Creating the organisation
Our final scenario is when our admin logs in, creates a new organisation which results in a new user-group being created and some pre-existing folders being made available, read-only, to this new organisation.
So let's get started with something simple - creating the organisation:
context "when I am logged in as an admin" do
it "creates the organisation" do
user = a_user status: "admin"
access_token = an_access_token_for user: user
post "/api/organisations", params: {name: "My organisation"}, headers: auth_headers_for(access_token)
expect(response).to be_created
get "/api/organisations", headers: auth_headers_for(access_token)
expect(response).to be_successful
data = JSON.parse(response.body)
expect(data.size).to eq 1
expect(data.first["name"]).to eq "My organisation"
end
end
As expected, when we run this, it fails. And we don't even have an organisation model yet. So let's create one - using bin/rails g model Organisation
. At the moment, it only needs a primary key, a name field (t.string :name, null: false, default: "", index: true
) and I'm going to leap ahead and add in a status field (t.integer :status, null: false, default: 0, index: true
). We don't strictly need it yet, but I know from experience that deleting records when you have an audit trail can cause problems - so, at the very least, we can use it to implement a "soft-delete" function.
Our organisation model looks like this:
class Organisation < ApplicationRecord
validates :name, presence: true
enum status: {active: 0, inactive: -1}
end
Take note that the status
field in the database has a default value of 0 and the status
enum's active
is represented by 0[1] - so every organisation is active unless we specify otherwise[2].
And our organisations controller becomes:
class Api::OrganisationsController < ApplicationController
def create
raise NotAuthorised unless current_user.admin?
render json: Organisation.create!(organisation_params), status: :created
rescue NotAuthorised
head :unauthorized
end
class NotAuthorised < StandardError
end
private
def organisation_params
params.permit(:name)
end
end
Firstly, if we're not doing concurrent programming[3], I like to use exceptions. Rather than using lots of nested if
statements, it keeps the "happy path" and the "error path" separate. The first line checks for permissions, then we have the normal case, followed by a separate section dealing with when things go wrong.
Secondly, to create our organisation, we sanitise the incoming parameters, build a record and return a status of :created
. We render the results as :json
- as this is purely an API call - which uses ActiveRecord's default JSON serialisation. It's not perfect, but it does the job.
Thirdly, we define our NotAuthorised
exception. This means that there's a lot going on here - it's a lot of stuff for one small controller file - and I can say for a fact that we're going to move things around later. But, for the moment, it's the simplest thing that works.
Now when we run the spec, it fails with at the next stage - when we read the organisations back via the API. Which makes sense as we have no read API for organisations.
Reading organisations
You can implement a simple index
action on the organisations controller. To get started, we add the index
action into our config/routes.rb
file.
namespace :api do
resources :organisations, only: [:create, :index]
end
But before we go any further, we'll need to go off on a slight tangent.
In my Gemfile I have included CanCanCan
. This gem allows you to define a set of access rules for your various models, it adds scopes to your models to govern read access and a helper method to your controllers that trigger those rules.
The ability.rb
file is the traditional way to define our CanCanCan rules. I like to add it to my user model like so:
class User < ApplicationRecord
delegate :can?, to: :ability
delegate :cannot?, to: :ability
def ability
@ability ||= User::Ability.new(self)
end
end
We'll come back to the delegate
calls later. But for now, we'll define a very basic set of permissions, in app/models/user/ability.rb
like so:
class User::Ability
include CanCan::Ability
def initialize user
if user.admin?
can :create, Organisation
can :read, Organisation, status: "active"
end
end
end
It's a bit messy, but it enough for now. With that in place, we can return to our organisations controller. We add in an index
action and simplify the create
call.
class Api::OrganisationsController < ApplicationController
def index
render json: Organisation.accessible_by(current_ability)
end
def create
authorize! :create, Organisation
render json: Organisation.create!(organisation_params), status: :created
rescue CanCan::AccessDenied
head :unauthorized
end
private
def organisation_params
params.permit(:name)
end
end
Our controller is looking much simpler already[4] and the specs all pass.
CanCanCan does the heavy lifting for us.
In our index
action, we use the accessible_by
scope, passing in current_ability
. These are supplied by CanCanCan. current_ability
builds an ability object for the current user and accessible_by
adds a extra where
clauses to our database queries, which filter out any organisations this user is not allowed to see. In this case, the ability class's can :read, Organisation, status: "active"
rule filters on the status field. Later on, we'll add some much more sophisticated clauses into the ability class, and vary those rules based on the access levels that our user has. But, for now, remember that, whenever you read from the database, add in the accessible_by
scope and you will never leak data to the outside world.
In our create
action, we use the authorize! :create, Organisation
call to block access. If the current user does not have the :create
permission defined in the ability file, a CanCan::AccessDenied
exception is raised, which we trap. This means we can drop the NotAuthorised
exception from our controller.
Time to come clean
I have to admit that the code I have in front of me does not look like this. The organisations controller does not have an index
action and my routes.rb
only includes the :create
action on the resources :organisations
line.
As I mentioned before, I've implemented much of this before. And one of the things I have written is a "generic resources API" - inspired by Supabase. The code is pretty complex (as is the spec file) but it means that I do not need to write a controller per model - resulting in a smaller routes file and fewer controllers. I'll go into how this works later on (it also depends heavily on actions[5] which we'll come to soon).
As a sneak preview, here's some (but not all) of the generic API in my routes file:
namespace :api do
resources :organisations, only: [:create]
# generic API
get ":class_name", to: "resources#index", as: :resources
get ":class_name/:id", to: "resources#show", as: :resource
# more generic API comes here but we'll come to that later
end
The idea here is that you can call GET /api/organisations
which Rails will route to the Api::ResourcesController#index
action - and this will then use CanCanCan to return a JSON list of all the organisations you are allowed to see. Or you can call GET /api/things
which will route to the same controller but return all the "things" you are allowed to see. And if you want to see get an individual resource you can call GET /api/organisations/123
or GET /api/things456
- again, you will only see the resources you're allowed to see.
User groups
The next step is to add a new clause to our specification. If we create a new organisation, we want to trigger an automation that creates a user-group specific to that organisation. At this point in time, I don't really know what a user-group looks like, but that's the entire point of this process. We're going to do the bare minimum to make the specification pass and revisit it if and when more requirements become apparent.
So let's add a new clause to the specification. Remember, this is our opportunity to define the "perfect" API - the one we wish we had when interacting with this system. It may turn out to be impossible, but at this point in time, with the wide open future in front of us, we might as well do things the best way we can.
First thing is to add the new clause - and move some of the setup code into RSpec's let
statements[6]. This means we're sharing some of the basic record creation stuff when it's needed across multiple tests.
context "when I am logged in as an admin" do
let(:user) { a_user status: "admin" }
let(:access_token) { an_access_token_for user: user }
it "creates the organisation" do
post "/api/organisations", params: {name: "My organisation"}, headers: auth_headers_for(access_token)
# (snip) all the stuff we saw before
end
it "automatically creates a user group for the new organisation" do
end
end
Next, we write out the stuff we're actually testing. We're going to create an organisation, but this time, instead of testing that the organisation was created[7], we're going to test that a user-group was created.
it "automatically creates a user group for the new organisation" do
post "/api/organisations", params: {name: "Megacorp"}, headers: auth_headers_for(access_token)
get "/api/user_groups", headers: auth_headers_for(access_token)
expect(response).to be_successful
data = JSON.parse(response.body)
expect(data.size).to eq 1
expect(data.first["name"]).to eq "Megacorp staff"
end
Obviously this is going to fail. Firstly, we don't have a user-group model, or controller. Secondly, we haven't defined this automation.
Let's start with the user-group.
Generate a new model (called UserGroup
in a table user_groups
). Keep it simple and make it identical to the organisation with a name and a status (active/inactive). Add clauses to the user's ability class - can :read UserGroup
and can :create, UserGroup
. I don't need to add in a controller, as my generic resources stuff will handle it, but if you're following along, you will just need to add the route for user_groups#index
with a controller that looks much like the organisations one we defined earlier.
Next up is the automation. But, first, a quick digression.
Ensuring our code does not end up a tangled mess
The absolute simplest thing we can do to make the automations work is simply attach an after_create
callback to our Organisation model. In a small application, we can do that with few repercussions. But this is going to be the heart of my clients' systems, probably for at least a decade[8]. So I'm going to avoid the standard Rails way of doing things - for a number of reasons.
Firstly, I know that this system will have multiple automations and they will vary based upon the configuration. The health and safety company we're building rules for will work differently to the retailer we'll be looking at next. So whatever the callback does will need to have some configuration options. More importantly, it's likely that there will be cascades of automations, where automations perform actions which trigger further automations. I know, from experience, that this can create an untraceable mess, where it's impossible to debug or improve the code once a certain level of complexity is reached.
Secondly, ActiveRecord callbacks are, necessarily linked to ActiveRecord models. Which means they are intrinsically tied to the database and the database schema. As the system grows and changes, the "surface area" of each model will increase (as more and more fields and behaviours thrown together) and it will become harder to isolate individual behaviours to debug or change them. It's not inevitable that our code will become a mess but if we follow the standard Rails patterns it takes discipline in order to keep things clean[9]. Instead, I want to make it easy to do the right thing and hard to do the wrong thing - so the code naturally stays separated and isolated. Ultimately, the goal here is, no matter how large the codebase grows, it remains easy to understand and easy to change.
Thirdly, I've just finished listening to "Clean Architecture" and "Clean Agile" by Bob Martin[10]. There are many things within these books that I take issue with[11] but one idea that really stuck with me was that your source code should reflect what your application does. If someone looks at your top-level "app" folder for the very first time, all they will see is that "this is a Ruby on Rails project". It provides no insight into what the project actually does and how it works. What if your top-level "app" folder actually described the functionality? With folders like create_organisation
, start_shift
, receive_incoming_order
and publish_job
. When a new developer joins your team and you ask them to fix a bug with receiving incoming orders, they know exactly where to start - it's right there in front of them in the app/receive_incoming_order
folder.
Fourthly, and this is related to the above, Rails does a lot of work to autoload your code. If you learn only ever build Rails projects and never write "pure" ruby you will never have to use a require
or require_relative
statement. Whilst this is convenient, it hides something really important. Dependencies. Keeping track of your dependencies, and more importantly, keeping track of the direction of your dependencies, is essential for ensuring that your code remains easy to change.
Event Handlers and Locators
What we're trying to avoid is "coupling" - that is tying big chunks of code together. Coupled code is difficult to understand and difficult to change because so much disparate stuff is tied together[12]. Or, to repeat the point above, the first bit of code depends on the second bit of code and the second bit of code depends on the first bit of code. A double-sided dependency - often hard to spot and which can lead to unexpected bugs.
How do we prevent this?
By forcing a separation between the chunks of code and making the dependencies explicit, so we can easily see when we end up tying things together that shouldn't be tied together.
How do we separate our code?
By broadcasting events and locating objects.
In our feature, we want a user-group to be created and folders made available when an organisation is created. So, when an organisation is created, we trigger an event that notifies any registered observers[13]. Actually, ActiveRecord callbacks are an event notification mechanism. They just happen to be a very tightly coupled mechanism and we're looking for something a bit looser.
The other cause of coupling is class names. We could easily write an Organisation
class that creates a UserGroup
and Folder
when needed. But what if, at some point in the future, it's not enough to just create a UserGroup
but you also have to create several other inter-connected objects at the same time?
To deal with this, we use locators. The locator is a level of indirection - instead of referring to organisations or folders or user-groups by their class, we tell an intermediary what we are looking for and it locates the relevant class for us. Couple that with the "factory pattern" and we can build objects without a direct dependency on the classes that do the work. All we need is a working knowledge of which methods to call and which parameters to pass.
Currently, our OrganisationsController has the line render json: Organisation.create!(organisation_params), status: :created
- it is tied directly to the Organisation
ActiveRecord model. Which means if we want to add extra behaviours to the creation of an organisation, we either need to add them into the controller or add them into the model. Adding them to the controller means if we want to create an organisation from anywhere else[14] we need to repeat this logic again and again - a recipe for inconsistencies and bugs. Adding them to the model then ties them to the database (because ActiveRecord does not work without some form of storage), which will make our tests slower and more brittle, plus mean we cannot use this same logic outside of this particular application[15].
So, instead, we create a factory for creating organisations - I'm going to call it CreateOrganisation
and get that to do the work. We also load it into a locator, so our controller tells the locator it wants to create an organisation, the locator returns the factory, then the controller tells the factory to do the work.
A pre-built implementation
I started writing some classes to handle all of this, then realised I had seen it before. The dry-rb collection of gems has a whole collection of prebuilt classes and modules that do exactly this (plus a whole lot more). It took me a little while to get my head around how they are structured, but I'm going to start with this.
- We'll have a locator, that is an instance of
Dry::System
. This will be calledStandardProcedure
and we'll use a Rails initialiser to fill it with the classes and factories that the application needs. - We'll have an event broadcaster, that is responsible for notifying any observers that something has happened elsewhere in the application. This will be using
Dry::Events
and will be installed in the locator asevents
so every part of the system can access it. - We'll implement our factories using
Dry::Transaction
. We've already noticed that creating an organisation is a multi-step process, involving authorisation, creating the object and then notifying interested parties.Dry::Transaction
is designed to handle multi-step processes; you give it an input, it passes that input along a "pipeline" of steps, eventually resulting in your desired output, or an error condition.
The plan is to reimplement our controller so it looks something like this[16]
class Api::OrganisationsController < ApplicationController
def index
render json: StandardProcedure["fetch_organisations"].for(current_user)
end
def create
render json: StandardProcedure["create_organisation"].with(organisation_params.merge(user: current_user)), status: :created
rescue StandardProcedure::Unauthorised
head :unauthorized
# Probably more stuff will have to go here
end
private
def organisation_params
params.permit(:name)
end
end
Now, the controller is isolated from both the database models and our authorisation framework, CanCanCan[17]. So if we change how the models are accessed[18] or we decide that CanCanCan isn't working out, we just change the relevant parts and the controller is unaffected. At first glance, all this indirection looks like we've made a load of work for ourselves, but, in reality, it's very simple[19] and the implementation work will be isolated, with very few dependencies.
This gives us flexibility and options as to how we build the thing - not just today, but for evermore.
However, this has been a lot of words just to get here, so we'll run through the real work next time.
Always specify the values for your enums - it is a field in your database and that data will probably live for much longer than this bit of code. So you want to make sure you've got control of the values and they are explicitly defined. ↩︎
I also like to make the "not normal" statuses negative numbers - keeping an obvious distinction between the stuff you see in day to day use and the stuff you only see under special circumstances. ↩︎
Concurrent programming is a whole different topic which requires a different mindset ↩︎
And, rest assured, will become even simpler ↩︎
In Collabor8Online I called these
Command
s but I think I prefer the nameAction
s. However, it's a dangerous game as Rails uses the wordaction
internally, so I might have to revisit it. ↩︎When you use
let
statements with ActiveRecord models you can get some weird side-effects. Records are only created on-demand, so you might need to explicitly create the record before your spec runs. And you may need to callmodel.reload
within your spec to make sure you read the updated values from the database. ↩︎No need to repeat ourselves, we've already tested that organisation creation works ↩︎
Code always lives for longer than you expect. And, for most projects, old age is painful for everyone involved ↩︎
and, if you're working in a team, that also means extra agreement on what shape that discipline takes as well as code reviews to ensure no-one steps off the ideal path. ↩︎
If you have an Audible subscription, they are available for free, without costing you your monthly credit ↩︎
Not least, I've inherited badly built projects that tried to use these principles but gave up half-way and they can be infuriating to navigate. ↩︎
Have you ever changed a bit of code over here and found a bug appears over there. That's because those bits of code are coupled and changes ripple outwards in unforeseen ways. ↩︎
In Javascript, they call them listeners. In Ruby, the tradition is to call them observers - which in turn comes from the Smalltalk language. Smalltalk actually called their observation mechanism the "dependency protocol", explaining exactly what it was for. ↩︎
Another controller, a command-line utility or even just our database seeds file ↩︎
Again, a command-line interface is the obvious example.
But one thing I really want to try is putting the application logic into a ruby gem, then loading it as WASM into a web-browser application. The browser will then contain all the important business rules as close to the end-user as possible. But, instead of talking directly to a database, it fires off HTTP requests to store and fetch data. ↩︎
Remember, we're trying to design our perfect API here, so let's start with how we want it to look, then discover if that is feasible. ↩︎
We removed the
CanCan::AccessDenied
exception and replaced it with an application-specificStandardProcedure::Unauthorised
exception. ↩︎Maybe we switch to a database that has offline and syncing capabilities such as Firebase or CouchDB. Or maybe this application becomes just one of a suite of applications and it needs to call a JSON API in order to read its data from elsewhere. ↩︎
Thanks to dry-rb ↩︎