Adding a client - part three
Last time, we got as far as adding an organisation. Then we got to the stage of ensuring the other, related, objects get created and I fell down a rabbit hole of explaining why the typical Rails way of achieving these things is dangerous[1].
So let's build this thing in a different style.
The Infrastructure
The first thing to add in is the locator. This is central to everything as it provides the indirection which allows us to minimise the dependencies within our application.
Originally I was going to use dry-system for this, but dry-system locks itself down[2] and that was causing me problems[3] when I was writing my tests. So I built my own, as it's really really simple, and I put it into app/core/standard_procedure.rb.
```
class StandardProcedure
require_relative "standard_procedure/errors"
require_relative "standard_procedure/events"
def initialize
@services = {}
end
def register name, service
@services[name.to_sym] = service
end
def [] service_name
@services[service_name.to_sym]
end
class << self
def register name, service
instance.register name, service
end
def [] service_name
instance[service_name]
end
private
def instance
@instance ||= StandardProcedure.new
end
end
register "events", StandardProcedure::Events.new
end
The errors file defines a basic StandardProcedure::Error[4] class, plus a subclass called StandardProcedure::Unauthorised. The events file is the basis for our event broadcasting, using dry-events.
require "dry/events"
class StandardProcedure::Events
include Dry::Events::Publisher[:standard_procedure]
end
These are both so simple I didn't bother writing any RSpec for them.
And, with that, we've built the two most important pieces of our infrastructure.
What does this thing do?
Next up, I want to rework the Api::OrganisationsController so, instead of talking to the database directly, it uses a CreateOrganisation object[5].
In keeping with the idea that just looking at your app folder should give you an immediate, at-a-glance, overview of what the application does, I'm going to add this in to app/create_organisation/create_organisation.rb - with a spec file at spec/create_organisation/create_organisation_spec.rb
At this point we are no longer writing "feature specifications". We're writing "unit tests" and the purpose of these is slightly different. Firstly, these act as developer documentation - they provide examples of how to use the code in question. Secondly, they help drive our design and keep the code clean because we are forced to map out our dependencies.
So let's look at create_organisation_spec.rb - but be aware that this is the completed file and I actually created it clause by clause, building up the functionality gradually.
require "spec_helper"
require_relative "../../app/create_organisation/create_organisation"
RSpec.describe CreateOrganisation do
context "initialisation" do
it "requires a user that responds to `can? :create, :organisation`" do
@user = double "user"
expect { CreateOrganisation.new(user: @user, name: "My Organisation") }.to raise_error Dry::Struct::Error
end
it "requires an organisation name" do
@user = double "user"
allow(@user).to receive(:can?).with(:create, :organisation).and_return(false)
expect { CreateOrganisation.new(user: @user) }.to raise_error Dry::Struct::Error
end
end
context "without permission" do
it "fails if the user does not have `create organisation` permission" do
@user = double "user"
allow(@user).to receive(:can?).with(:create, :organisation).and_return(false)
expect { CreateOrganisation.new(user: @user, name: "My Organisation").call }.to raise_error StandardProcedure::Unauthorised
end
end
context "with permission" do
before do
@user = double "user"
allow(@user).to receive(:can?).with(:create, :organisation).and_return(true)
end
context "successfully creating an organisation" do
before do
@organisations = double "organisations"
@organisation = double "my organisation"
StandardProcedure.register("organisations", @organisations)
expect(@organisations).to receive(:create!).with(name: "My Organisation").and_return(@organisation)
end
it "notifies observers before creating the organisation" do
@result = nil
StandardProcedure["events"].subscribe "create_organisation_started" do |event|
@result = event[:user]
end
CreateOrganisation.new(user: @user, name: "My Organisation").call
expect(@result).to eq @user
end
it "tells the organisations collection to create the organisation" do
expect(CreateOrganisation.new(user: @user, name: "My Organisation").call).to eq @organisation
end
it "notifies observers after creating the organisation" do
@result = nil
StandardProcedure["events"].subscribe "create_organisation_completed" do |event|
@result = event[:organisation]
end
CreateOrganisation.new(user: @user, name: "My Organisation").call
expect(@result).to eq @organisation
end
end
context "failing to create an organisation" do
before do
@organisations = double "organisations"
@organisation = double "my organisation"
StandardProcedure.register("organisations", @organisations)
expect(@organisations).to receive(:create!).with(name: "My Organisation").and_raise(StandardProcedure::Error)
end
it "notifies observers of the failure" do
@result = nil
StandardProcedure["events"].subscribe "create_organisation_failed" do |event|
@result = event[:error]
end
CreateOrganisation.new(user: @user, name: "My Organisation").call
expect(@result).to be_kind_of(StandardProcedure::Error)
end
end
end
end
Note that I'm not requiring rails_helper, I'm requiring spec_helper. I do not want this functionality to be dependent upon Ruby on Rails. Rails is just an implementation detail. For the same reason, we're using "double objects". This class does not need to touch the database in anyway, so we enforce that isolation[6]. However, because we're not loading up the Rails framework, we do need to add in require_relative "../../app/create_organisation/create_organisation" to load our code.
The specification is then split into several parts - "initialisation", "without permission", "with permission and succeeding" and "with permission and failing".
Initialisation
For initialisation, we want our CreateOrganisation object to specify some details about the parameters it takes. For now, there are two conditions - it needs a "user" object (and that user object must have a can? method) and it needs a "name" for our organisation. The first clause creates a double[7] and passes it to our constructor, along with an organisation name parameter, expecting a Dry::Struct::Error. The second clause creates a second double but tells RSpec to pretend that it has a method called can?. However, this time it omits the organisation name parameter, and once again expects a Dry::Struct::Error.
When we run these, they fail, because we haven't even defined our CreateOrganisation class yet.
require "dry/struct"
class CreateOrganisation < Dry::Struct
module Types
include Dry::Types()
end
attribute :user, Types.Interface(:can?).optional
attribute :name, Types::Strict::String
end
So we're using another piece of dry-rb, this time a Dry::Struct. This is a simple data-structure that does some type-checking for us. In particular, we define a :user attribute as "an optional[8] object that has a method called :can?" and we define a :name attribute that is "a string that cannot be nil". Dry::Struct does the type checking for us - which is why the specification expects for Dry::Struct::Error exceptions. This is just a nice time-saver for us.
Authorisation failures
We check if the user in question does not have permission to create an organisation in the next set of clauses. We tell RSpec to build a fake user object which returns false if can?(:create, :organisation) is called. Then we create our CreateOrganisation object and call it[9]. This now fails because our object doesn't have a call method. Let's add that in.
def call
raise StandardProcedure::Unauthorised unless user&.can? :create, :organisation
end
We use the & nil operator so the same line works whether the user is nil or if the user does not have permission.
Authorised and building a new organisation
The "with permission" clause is sub-divided into two sections - "success" and "failure". The before section sets up another user double but this time it pretends to return true. Then the success clause sets up a few more doubles, fakes a few more method calls and registers one of these doubles with our locator, using the name "organisations".
Let's start with the clause that actually creates the organisation - it "tells the organisations collection to create the organisation". Notice that the CreateOrganisation class doesn't actually create the record itself - we're trying to avoid touching the database here. Instead, it tells the "organisations collection" to create it. And that's why we registered that double with the name "organisations" - it represents a collection of organisations and we need it to have a create! method. Even better, that just happens to match how we create a model using ActiveRecord[10].
We update our call method to make use of this organisations collection and now our new test passes.
def call
raise StandardProcedure::Unauthorised unless user&.can? :create, :organisation
StandardProcedure["organisations"].create! name: name
end
So far all we've done is add a load of extra layers into our code yet we've basically achieved no more than a standard Rails application. But now is the time when we start to reap the benefits.
Why are we doing it this way?
The additional clauses in the "success" section, and the clause in the "failure" section, are the first times we actually make use of this new framework. In the tests, we register a subscription to various events - create_organisation_started, create_organisation_completed and create_organisation_failed and check to see if they are called. That's why we built that Dry::Events object earlier on. Then, we update our call method, again using Dry::Events, to publish those events as required.
def call
raise StandardProcedure::Unauthorised unless user&.can? :create, :organisation
begin
StandardProcedure["events"].publish "create_organisation_started", user: user
organisation = StandardProcedure["organisations"].create! name: name
StandardProcedure["events"].publish "create_organisation_completed", user: user, organisation: organisation
organisation
rescue => error
StandardProcedure["events"].publish "create_organisation_failed", user: user, error: error
end
end
The tests all pass and suddenly we have a decoupled mechanism to trigger activities in other parts of the system.
Why are we using doubles?
Many developers frown on the use of "test doubles"[11], because they can result in brittle and fragile tests that break when something changes.
There are two reasons we're using them (and the potential fragility is a kind of feature, not a bug).
Firstly, the doubles ensure that this object stays far away from the database.
Secondly, they describe what this object expects to see when it's doing its thing.
We don't want our tests to use real ActiveRecord models because, as we add in more fields, as we add in more methods, we could inadvertently add a dependency on some of those additions. By using a double, if we need to reference an extra field or method from the database or another object, we have to make it explicit by adding it in to the double. So not only do we have to intentionally decide to do it, but our documentation is kept up-to-date as the definition of our double has stuff added to it.
In this case, our dependencies are:
- an object that has a
can?(do_something_to, target)method, which is passed in as a parameter when the object is created - an object, registered in the locator with the name "events", that has a
publish(event_name, **details)method and aregister_event(event_name)method[12] - an object, registered in the locator with the name "organisations", that has a
create!(**params)method
We don't care if they are ActiveRecord models, if they are Roda servers, if they are HTTPClient classes or something that is dynamically generated from a configuration file at run-time. As long as our dependencies are registered under the correct names and have methods that match those signatures then our CreateOrganisation object will work.
In languages that have compile-time type-checking (like Java or Typescript or Dart or C#), we need to define interfaces for each of these dependencies so the compiler knows which methods to expect. Not only is this a load of extra typing, it's also a lot of extra symbols and names for us to parse and remember. But in Ruby (and Javascript and Python and other dynamic languages), we don't need those interfaces, which is a real time-and-brain-space-saver. That doesn't mean we shouldn't be intentional about how we add things to our dependencies though - and the use of Dry::Types’ Interface definition[13] means we can specify exactly what our objects should look like without having to write new classes or modules for them.
Because of that, I’m taking a short-cut here. “Clean code” advocates will say that the “logic” of your application should never be given “database objects” - because this pollutes the abstraction and risks future schema changes or other database-related nonsense, from breaking logic that would otherwise be stable. However, I’m going to use the ActiveRecord Organisation model and it’s create! class method for my “organisations” object and if something changes (such as the Organisation model getting renamed or changing shape), I can replace it with a wrapper that registers itself as “organisations” but uses the new ActiveRecord model to do the work. That means CreateOrganisation itself doesn’t have to change - which is exactly what we want.
The point of it all
And now, finally, we reach the point of it all.
We want our application to have some mechanism that triggers automations when an organisation is created. But, because it's a configurable application, we don't know what those automations will be or how they're going to work.
However, we've now got the infrastructure to do that.
Our automations simply listen for create_organisation_completed events and, when notified, they do whatever it is they have to do.
Plus we have a pattern for implementing the rest of the system.
We add in objects, similar to CreateOrganisation, for each action that the system needs to perform. We ensure those actions use the locator to find their dependencies and fire off events at the correct times. Each new piece of functionality is small[14] and self-contained but the application as a whole is supremely extensible, with little risk of faults in one area adversely affecting another area.
One more thing
If we go back and run our original "gherkin" style specification, you'll notice it doesn't actually pass yet. Because, although we've come a long way, we've not actually finished adding the required functionality.
First up, we need to tie our different bits and pieces together. We have a load of bits and pieces that are, by design, disconnected. However, something somewhere needs to connect them or the application won’t do anything. The Standard Procedure class already registers an “events” object. But our locator knows nothing about any of the other objects we’ve created. And that’s the job for an initialiser[15]. In config/initializers/app.rb we put something like this:
require_relative "../../app/core/standard_procedure"
# Infrastructure
StandardProcedure.register "logger", Rails.logger
# Features
StandardProcedure.register "create_organisation", CreateOrganisation
This particular instance of the app knows which features it has, so it registers them with the locator. I’ve also used an “organisations” object (whose create! method just happens to match the class methods on an ActiveRecord model). Ideally we would map that in to our initialiser as well, but this is where my shortcut falls .. erm, short. The initialiser can’t see the ActiveRecord models yet (it runs before the Rails app has finished configuring itself), so I’ve just added a line into app/models/organisation.rb[16]
StandardProcedure.register "organisations", Organisation
However, the spec still fails when we check for the newly created user-group - because we haven’t actually defined anything to create that user-group. So we need the relevant automations[17].
The feature specification fails at the "And I should see that the client organisation has read-only access to the policy documents folder" step.
The step is quite long:
step "I should see that the client organisation has read-only access to the policy documents folder" do
get "/api/organisations/#{@tinyco_id}/folders", headers: @auth_headers
expect(last_response).to be_successful
data = JSON.parse(last_response.body)
folder_data = data.find { |d| d["name"] == "Policies" }
expect(folder_data).to be_present
@linked_folder_id = folder_data["id"]
get "/api/folders/#{@linked_folder_id}/documents", headers: @auth_headers
expect(last_response).to be_successful
data = JSON.parse(last_response.body)
document_data = data.find { |d| d["name"] == "Health and Safety Policy" }
expect(document_data).to be_present
get "/api/user_groups", headers: @auth_headers
expect(last_response).to be_successful
data = JSON.parse(last_response.body)
user_group_data = data.find { |d| d["name"] == "TinyCo staff" }
expect(user_group_data).to be_present
@user_group_id = user_group_data["id"]
get "/api/folders/#{@linked_folder_id}/permissions", headers: @auth_headers
expect(last_response).to be_successful
data = JSON.parse(last_response.body)
permission_data = data.find { |d| d["user_group_id"] == @user_group_id }
expect(permission_data).to be_present
end
First we try to list the folders that are attached to the organisation (looking for "Policies", which we added to our "home" organisation in "And the organisation has a folder containing policy documents"). We don't have a folders controller yet, so this obviously fails. Next, it tries to list the documents within that folder, the user groups that have been built (looking for one called "TinyCo staff") and then ensures that the user group has permission to view the folder.
So, in my case, my ResourcesController will handle the reading of data[18], so I'm not going to worry about that. Instead, we need to specify the automation that is going to set up the user group, copy (or rather sym-link the folder) and set up the permissions.
In an ideal world, we'd have some sort of configuration file that looks something like this:
StandardProcedure["automations"].on "create_organisation_completed" do |event|
user = event[:user]
created_organisation = event[:organisation]
home_organisation = StandardProcedure["organisations"].home
policies_folder = home_organisation.folders.find_by name: "Policies"
user_group = StandardProcedure["create_user_group"].new(user: user, name: "#{created_organisation} staff").call
linked_folder = StandardProcedure["link_folder"].new(user: user, source_folder: policies_folder).call
StandardProcedure["assign_permissions"].new(user: user, user_group: user_group, folder: linked_folder).call
end
We tell the "automations" object to listen for create_organisation_completed events. It extracts the user who did the work, the newly created organisation and our "home" organisation. It finds the "Policies" folder, creates a new user group, then links the folder and assigns some permissions.
So we have a few new features to implement here - automations, create_user_group, link_folder and assign_permissions. I'm also not happy about the policies_folder = home_organisation.folders.find_by name: "Policies" line - we're bypassing CanCanCan's permissions here[19], but I don't want to have to add in CanCanCan's standard accessible_by(user.ability) scope. This is because we’ll have to remember to add it in manually on every automation we write and that’s a recipe for it getting forgotten. But we'll return to that later on.
I started writing the “automations” feature, but it turns out that it’s nothing more than a very thin wrapper around the Dry::Events’ subscribe method that we saw before. It may be that we need this “automations” thing to do more work later on, but for now, let’s do the simplest thing and register a subscriber to the events directly.
StandardProcedure["events"].subscribe "create_organisation_completed" do |event|
# all the rest
end
create_user_group is very simple and similar to create_organisation. Again we write a spec for it and it needs a user_groups object registering so it can call create!, so we use the same ActiveRecord trick as with organisations. However, I’m going to put create_user_group into app/core because permissions and permissions management is so fundamental to any multi-user system. Likewise, that’s where assign_permissions is going to live.
And while working on assign_permissions[20] I discovered that:
grant_accessis a better name thanassign_permissions(as we can also have the oppositerevoke_access- you would grant access to a resource for a user-group (and I added in an array of “actions” which are the individual things you can do to that resource[21])
- in order to make this work, the
user_groupshould have a methodgrant_access_to resource, actions: [] - it doesn’t make sense to assign permissions to just anything - the resource in question should at the very least have a list of
permissions
Here’s a fragment from the assign_permission spec:
it "tells the user_group to record the new permission" do
@resource = double "resource", permissions: []
@user = double "user", can?: true
@actions = %w[do_this do_that]
@permission = double "permission"
@user_group = double "user_group"
allow(@user_group).to receive(:grant_access_to).with(@resource, actions: @actions).and_return(@permission)
expect(GrantAccess.new(user: @user, user_group: @user_group, resource: @resource, actions: @actions).call).to eq @permission
end
Just to reiterate - by writing out the spec first (and using doubles, rather than real classes), I came across ideas about how that code should look, so it reads nicely and makes sense.
Now I need to implement the grant_access_to method on the user group. This is simple database work, so I don’t really need a unit test (if ActiveRecord doesn’t do what it promises then we’re really in trouble). I create a new model, Permission, that belongs_to :user_group and belongs_to :resource.
And now, the implementation details really come into play. So far, pretty much everything we’ve done has been pure objects and the database has been an implementation detail. But now I need to decide what this resource actually looks like, how it’s going to work with folders and organisations (and anything else that will have controlled access). In terms of objects, it’s easy - a resource is something that has an array of permissinos. But in terms of the database, how we put this together matters as it has performance implications and might be difficult to change further down the line.
So we’ll work through that next time.
TL;DR; there's nothing stopping you from writing an amazing, easy to maintain, easy to change system using the Rails defaults - but it takes discipline. And, when you're under pressure, with a looming deadline or customers worried about how the system is affecting their business, it's all too easy for that discipline to slip. By putting some guard rails in first, defining patterns to follow and guiding future development down a particular path, we make it easier to write good code and harder to write bad code. ↩︎
It has a built-in plugin system for loading extra code but to make that work quickly it refuses to allow changes to its configuration after it has been set up ↩︎
More importantly, it was costing me time ↩︎
Descending from
StandardError↩︎Look back at the previous post for the justification - doing this gives us a ton of flexibility in the future for very little cost. ↩︎
Don't worry - I'll explain more in a bit ↩︎
From the rspec-mocks gem ↩︎
It's optional because we need to ensure the same "unauthorised" error is raised if someone is not logged in (
user == nil) or if they have insufficient permissions (user.can?(:what, :ever) == false) ↩︎callis a ruby convention. RubyProc,lambdas andBlocks all usecallto execute their code, so we copy that convention. In fact, ruby even has a short-cut - instead of usingmy_object.call(params)you can usemy_object.(params). ↩︎You can build for isolation and separation but it's also good to follow conventions and use names and phrases that people are familiar with ↩︎
All the names for things associated with tests is mixed up with different people calling the same things different names. You could call these mocks, you could call them doubles, I don't really care. It's a fake object that stands in for the actual implementation. ↩︎
I didn't mention this before but
Dry::Eventsrequires you to register your event names in advance ↩︎Look back at the definition of the
userattribute in theCreateOrganisationclass ↩︎Some will be much more complex than this example, spanning several related objects, each performing different aspects of a task. But we keep the related stuff together in a single folder called
app/feature_nameand we make their dependencies explicit by marking them out in the specification files. ↩︎initializer ↩︎
This is a place where my short-cut of using ActiveRecord models actually screws things up a bit ↩︎
And we finally get to see if this style of programming is really as easy as I've been promising ↩︎
Note that my ResourcesController bypasses the structure we've built above and relies on CanCanCan to access data safely. We'll address that later on. ↩︎
This is where permissions get complex. What if the user who triggered the automation does not have access rights to the Policies folder? Should the automation fail? Or should it have some sort of "privilege escalation" that allows the automation to read data that's out of scope for the user? That's application, and use-case, dependent so we need to give it some thought. ↩︎
I figured
user_groupsandassign_permissionsgo together and I would leavelink_folderstill last. ↩︎Again, experience here says to me “if they have a permissions record then they have read access, if they want to do something then we will, in addition, check for the action that they want to perform” ↩︎
