From 5b668b8d94def0bbc07ac810c2b1fb7afce08f2e Mon Sep 17 00:00:00 2001
From: Kevin Dew <kevindew@me.com>
Date: Wed, 14 Apr 2021 23:55:17 +0100
Subject: [PATCH] Use a mixin for shared behaviour in services

This creates a new mixin, Callable, which can be used to provide a class
with the boilerplate so that it can have a public class method of
`.call` which calls `#initialize` with the arguments and then `#call`.

This mixin replaces the inheritance approach used in services with an
ApplicationService class. These classes now all utilise this mixin
instead.

The reason for doing this was prompted by a linting violation with one
of rubocop's newer rules: `Lint/MissingSuper`, which is an offence when
a class inherits but does not call `super` in it's initialize method.
This violation seemed to only affect some of GOV.UK's newer code, such as
this service pattern, which led to some debate [1] about what the most
appropriate means to share this behaviour is, whether our
inheritance approach was actually idiomatic and how using super in every
`initialize` would be unnecessary boilerplate.

The Rubocop thread [2] regarding this rule raised an interesting point
"I'm curious of examples where there are good reasons to explicitly not
call super.", which led to me considering why we didn't think `super`
would be appropriate in these classes. My conclusion to this was that we
weren't actually intending to create a subtype with inheritance, we
actually just wanted standardisation with the interface, and that the
inherited classes have very little behavioural consistency, breaking
Liskov substitution principle [3]. When looking at the Ruby standard
library a module seems the most appropriate way to implement it,
considering similarities with modules such as Singleton and Enumerable.

Taking inspiration from the Ruby
standard library there is an in

[1]: https://github.com/alphagov/rubocop-govuk/pull/125
[2]: https://github.com/rubocop/rubocop/issues/8506
[3]: https://thoughtbot.com/blog/back-to-basics-solid#liskov-substitution-principle
---
 app/services/application_service.rb                |  7 -------
 app/services/assign_edition_status_service.rb      |  4 +++-
 app/services/create_document_service.rb            |  4 +++-
 .../create_file_attachment_blob_service.rb         |  4 +++-
 app/services/create_image_blob_service.rb          |  4 +++-
 app/services/create_next_edition_service.rb        |  4 +++-
 app/services/delete_draft_assets_service.rb        |  4 +++-
 app/services/discard_draft_edition_service.rb      |  4 +++-
 app/services/discard_path_reservations_service.rb  |  4 +++-
 app/services/edit_draft_edition_service.rb         |  4 +++-
 app/services/failsafe_draft_preview_service.rb     |  4 +++-
 app/services/generate_base_path_service.rb         |  4 +++-
 app/services/generate_unique_filename_service.rb   |  3 ++-
 app/services/preview_asset_service.rb              |  4 +++-
 app/services/preview_draft_edition_service.rb      |  4 +++-
 app/services/publish_assets_service.rb             |  4 +++-
 app/services/publish_draft_edition_service.rb      |  4 +++-
 app/services/remove_document_service.rb            |  4 +++-
 .../rescue_scheduled_publishing_service.rb         |  4 +++-
 app/services/resync_document_service.rb            |  4 +++-
 app/services/schedule_publish_service.rb           |  4 +++-
 app/services/withdraw_document_service.rb          |  4 +++-
 lib/callable.rb                                    | 14 ++++++++++++++
 23 files changed, 76 insertions(+), 28 deletions(-)
 delete mode 100644 app/services/application_service.rb
 create mode 100644 lib/callable.rb

diff --git a/app/services/application_service.rb b/app/services/application_service.rb
deleted file mode 100644
index 65efdfaba..000000000
--- a/app/services/application_service.rb
+++ /dev/null
@@ -1,7 +0,0 @@
-class ApplicationService
-  private_class_method :new
-
-  def self.call(*args, **kwargs)
-    new(*args, **kwargs).call
-  end
-end
diff --git a/app/services/assign_edition_status_service.rb b/app/services/assign_edition_status_service.rb
index c38417109..c91078ab7 100644
--- a/app/services/assign_edition_status_service.rb
+++ b/app/services/assign_edition_status_service.rb
@@ -1,4 +1,6 @@
-class AssignEditionStatusService < ApplicationService
+class AssignEditionStatusService
+  include Callable
+
   def initialize(edition,
                  state:,
                  user: nil,
diff --git a/app/services/create_document_service.rb b/app/services/create_document_service.rb
index 3954b2476..52964781b 100644
--- a/app/services/create_document_service.rb
+++ b/app/services/create_document_service.rb
@@ -1,4 +1,6 @@
-class CreateDocumentService < ApplicationService
+class CreateDocumentService
+  include Callable
+
   def initialize(document_type_id:,
                  content_id: SecureRandom.uuid,
                  locale: "en",
diff --git a/app/services/create_file_attachment_blob_service.rb b/app/services/create_file_attachment_blob_service.rb
index 06be6fe33..9d0d5465a 100644
--- a/app/services/create_file_attachment_blob_service.rb
+++ b/app/services/create_file_attachment_blob_service.rb
@@ -1,4 +1,6 @@
-class CreateFileAttachmentBlobService < ApplicationService
+class CreateFileAttachmentBlobService
+  include Callable
+
   def initialize(file:, filename:, user: nil)
     @file = file
     @filename = filename
diff --git a/app/services/create_image_blob_service.rb b/app/services/create_image_blob_service.rb
index 402b1a5a2..c9f0ed22b 100644
--- a/app/services/create_image_blob_service.rb
+++ b/app/services/create_image_blob_service.rb
@@ -1,6 +1,8 @@
 require "mini_magick"
 
-class CreateImageBlobService < ApplicationService
+class CreateImageBlobService
+  include Callable
+
   def initialize(temp_image:, filename:, user: nil)
     @temp_image = temp_image
     @filename = filename
diff --git a/app/services/create_next_edition_service.rb b/app/services/create_next_edition_service.rb
index 6041015c0..fbef59d45 100644
--- a/app/services/create_next_edition_service.rb
+++ b/app/services/create_next_edition_service.rb
@@ -1,4 +1,6 @@
-class CreateNextEditionService < ApplicationService
+class CreateNextEditionService
+  include Callable
+
   def initialize(current_edition:, user:, discarded_edition: nil)
     @current_edition = current_edition
     @user = user
diff --git a/app/services/delete_draft_assets_service.rb b/app/services/delete_draft_assets_service.rb
index 863fade2c..96450b6e7 100644
--- a/app/services/delete_draft_assets_service.rb
+++ b/app/services/delete_draft_assets_service.rb
@@ -1,4 +1,6 @@
-class DeleteDraftAssetsService < ApplicationService
+class DeleteDraftAssetsService
+  include Callable
+
   def initialize(edition, **)
     @edition = edition
   end
diff --git a/app/services/discard_draft_edition_service.rb b/app/services/discard_draft_edition_service.rb
index 4f6478414..368a427e8 100644
--- a/app/services/discard_draft_edition_service.rb
+++ b/app/services/discard_draft_edition_service.rb
@@ -1,4 +1,6 @@
-class DiscardDraftEditionService < ApplicationService
+class DiscardDraftEditionService
+  include Callable
+
   def initialize(edition, user, **)
     @edition = edition
     @user = user
diff --git a/app/services/discard_path_reservations_service.rb b/app/services/discard_path_reservations_service.rb
index d77c2c88d..f71611c4c 100644
--- a/app/services/discard_path_reservations_service.rb
+++ b/app/services/discard_path_reservations_service.rb
@@ -1,4 +1,6 @@
-class DiscardPathReservationsService < ApplicationService
+class DiscardPathReservationsService
+  include Callable
+
   def initialize(edition, **)
     @edition = edition
   end
diff --git a/app/services/edit_draft_edition_service.rb b/app/services/edit_draft_edition_service.rb
index 3c642af00..927ae59c3 100644
--- a/app/services/edit_draft_edition_service.rb
+++ b/app/services/edit_draft_edition_service.rb
@@ -1,4 +1,6 @@
-class EditDraftEditionService < ApplicationService
+class EditDraftEditionService
+  include Callable
+
   def initialize(edition, user, **attributes)
     @edition = edition
     @user = user
diff --git a/app/services/failsafe_draft_preview_service.rb b/app/services/failsafe_draft_preview_service.rb
index 2647657ba..9de139cef 100644
--- a/app/services/failsafe_draft_preview_service.rb
+++ b/app/services/failsafe_draft_preview_service.rb
@@ -1,4 +1,6 @@
-class FailsafeDraftPreviewService < ApplicationService
+class FailsafeDraftPreviewService
+  include Callable
+
   def initialize(edition, **)
     @edition = edition
   end
diff --git a/app/services/generate_base_path_service.rb b/app/services/generate_base_path_service.rb
index ab3e13ebf..342fc3e97 100644
--- a/app/services/generate_base_path_service.rb
+++ b/app/services/generate_base_path_service.rb
@@ -1,4 +1,6 @@
-class GenerateBasePathService < ApplicationService
+class GenerateBasePathService
+  include Callable
+
   def initialize(edition, title:, max_repeated_titles: 1000)
     @edition = edition
     @title = title.to_s
diff --git a/app/services/generate_unique_filename_service.rb b/app/services/generate_unique_filename_service.rb
index 5c69d7d90..4ddb45b6e 100644
--- a/app/services/generate_unique_filename_service.rb
+++ b/app/services/generate_unique_filename_service.rb
@@ -1,4 +1,5 @@
-class GenerateUniqueFilenameService < ApplicationService
+class GenerateUniqueFilenameService
+  include Callable
   MAX_LENGTH = 65
 
   def initialize(filename:, existing_filenames:)
diff --git a/app/services/preview_asset_service.rb b/app/services/preview_asset_service.rb
index 165c36f8a..ea1f8f0cb 100644
--- a/app/services/preview_asset_service.rb
+++ b/app/services/preview_asset_service.rb
@@ -1,4 +1,6 @@
-class PreviewAssetService < ApplicationService
+class PreviewAssetService
+  include Callable
+
   def initialize(edition, asset, **)
     @edition = edition
     @asset = asset
diff --git a/app/services/preview_draft_edition_service.rb b/app/services/preview_draft_edition_service.rb
index f6bca04eb..0997f479b 100644
--- a/app/services/preview_draft_edition_service.rb
+++ b/app/services/preview_draft_edition_service.rb
@@ -1,4 +1,6 @@
-class PreviewDraftEditionService < ApplicationService
+class PreviewDraftEditionService
+  include Callable
+
   def initialize(edition, republish: false)
     @edition = edition
     @republish = republish
diff --git a/app/services/publish_assets_service.rb b/app/services/publish_assets_service.rb
index 26ee20376..894822d35 100644
--- a/app/services/publish_assets_service.rb
+++ b/app/services/publish_assets_service.rb
@@ -1,4 +1,6 @@
-class PublishAssetsService < ApplicationService
+class PublishAssetsService
+  include Callable
+
   def initialize(edition, superseded_edition: nil)
     @edition = edition
     @superseded_edition = superseded_edition
diff --git a/app/services/publish_draft_edition_service.rb b/app/services/publish_draft_edition_service.rb
index a3a6f83df..349ffb545 100644
--- a/app/services/publish_draft_edition_service.rb
+++ b/app/services/publish_draft_edition_service.rb
@@ -1,4 +1,6 @@
-class PublishDraftEditionService < ApplicationService
+class PublishDraftEditionService
+  include Callable
+
   def initialize(edition, user, with_review:)
     @edition = edition
     @user = user
diff --git a/app/services/remove_document_service.rb b/app/services/remove_document_service.rb
index 2a22e0f52..29c28b704 100644
--- a/app/services/remove_document_service.rb
+++ b/app/services/remove_document_service.rb
@@ -1,4 +1,6 @@
-class RemoveDocumentService < ApplicationService
+class RemoveDocumentService
+  include Callable
+
   def initialize(edition, removal, user: nil)
     @edition = edition
     @removal = removal
diff --git a/app/services/rescue_scheduled_publishing_service.rb b/app/services/rescue_scheduled_publishing_service.rb
index 7a3353e82..55a047c62 100644
--- a/app/services/rescue_scheduled_publishing_service.rb
+++ b/app/services/rescue_scheduled_publishing_service.rb
@@ -1,4 +1,6 @@
-class RescueScheduledPublishingService < ApplicationService
+class RescueScheduledPublishingService
+  include Callable
+
   def initialize(edition_id:)
     @edition_id = edition_id
   end
diff --git a/app/services/resync_document_service.rb b/app/services/resync_document_service.rb
index 3a219dcff..e3dc1ee42 100644
--- a/app/services/resync_document_service.rb
+++ b/app/services/resync_document_service.rb
@@ -1,4 +1,6 @@
-class ResyncDocumentService < ApplicationService
+class ResyncDocumentService
+  include Callable
+
   def initialize(document, **)
     @document = document
   end
diff --git a/app/services/schedule_publish_service.rb b/app/services/schedule_publish_service.rb
index c41803b75..7532e9371 100644
--- a/app/services/schedule_publish_service.rb
+++ b/app/services/schedule_publish_service.rb
@@ -1,4 +1,6 @@
-class SchedulePublishService < ApplicationService
+class SchedulePublishService
+  include Callable
+
   def initialize(edition, user, scheduling, **)
     @edition = edition
     @user = user
diff --git a/app/services/withdraw_document_service.rb b/app/services/withdraw_document_service.rb
index c96d36d58..d1c9c809f 100644
--- a/app/services/withdraw_document_service.rb
+++ b/app/services/withdraw_document_service.rb
@@ -1,4 +1,6 @@
-class WithdrawDocumentService < ApplicationService
+class WithdrawDocumentService
+  include Callable
+
   def initialize(edition, user, public_explanation:)
     @edition = edition
     @public_explanation = public_explanation
diff --git a/lib/callable.rb b/lib/callable.rb
new file mode 100644
index 000000000..8a640af30
--- /dev/null
+++ b/lib/callable.rb
@@ -0,0 +1,14 @@
+# This mixin is to provide the boilerplate for classes that implement a single
+# public class method of `.call` - typically used by objects that are entirely
+# to perform a business transaction.
+module Callable
+  extend ActiveSupport::Concern
+
+  included do
+    def self.call(*args, **kwargs, &block)
+      new(*args, **kwargs, &block).call
+    end
+
+    private_class_method :new
+  end
+end