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 a register_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_access is a better name than assign_permissions (as we can also have the opposite revoke_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_group should have a method grant_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.