From 8a3ec2712d5be8e86461ce4c7111f57fc9185831 Mon Sep 17 00:00:00 2001
From: Christian Sutter <git@doublesignal.com>
Date: Fri, 8 Dec 2023 16:30:32 +0000
Subject: [PATCH] Split `DiscoveryEngine` service module into sync/query

This helps keep these independent pieces of functionality apart a bit
better.
---
 app/controllers/searches_controller.rb                    | 2 +-
 app/models/publishing_api_document.rb                     | 4 ++--
 .../{boosts/best_bets.rb => query/best_bets_boost.rb}     | 4 ++--
 app/services/discovery_engine/{ => query}/filters.rb      | 2 +-
 .../news_recency.rb => query/news_recency_boost.rb}       | 4 ++--
 app/services/discovery_engine/{ => query}/search.rb       | 6 +++---
 app/services/discovery_engine/{ => sync}/delete.rb        | 2 +-
 app/services/discovery_engine/{ => sync}/document_name.rb | 2 +-
 app/services/discovery_engine/{ => sync}/logging.rb       | 2 +-
 app/services/discovery_engine/{ => sync}/put.rb           | 2 +-
 spec/integration/document_synchronization_spec.rb         | 8 ++++----
 spec/requests/search_request_spec.rb                      | 6 +++---
 .../services/discovery_engine/{ => query}/filters_spec.rb | 2 +-
 spec/services/discovery_engine/{ => query}/search_spec.rb | 4 ++--
 spec/services/discovery_engine/{ => sync}/delete_spec.rb  | 8 ++++----
 spec/services/discovery_engine/{ => sync}/put_spec.rb     | 6 +++---
 16 files changed, 32 insertions(+), 32 deletions(-)
 rename app/services/discovery_engine/{boosts/best_bets.rb => query/best_bets_boost.rb} (90%)
 rename app/services/discovery_engine/{ => query}/filters.rb (93%)
 rename app/services/discovery_engine/{boosts/news_recency.rb => query/news_recency_boost.rb} (93%)
 rename app/services/discovery_engine/{ => query}/search.rb (94%)
 rename app/services/discovery_engine/{ => sync}/delete.rb (96%)
 rename app/services/discovery_engine/{ => sync}/document_name.rb (84%)
 rename app/services/discovery_engine/{ => sync}/logging.rb (91%)
 rename app/services/discovery_engine/{ => sync}/put.rb (97%)
 rename spec/services/discovery_engine/{ => query}/filters_spec.rb (94%)
 rename spec/services/discovery_engine/{ => query}/search_spec.rb (97%)
 rename spec/services/discovery_engine/{ => sync}/delete_spec.rb (80%)
 rename spec/services/discovery_engine/{ => sync}/put_spec.rb (85%)

diff --git a/app/controllers/searches_controller.rb b/app/controllers/searches_controller.rb
index 59126a0..8a7ff2b 100644
--- a/app/controllers/searches_controller.rb
+++ b/app/controllers/searches_controller.rb
@@ -1,6 +1,6 @@
 class SearchesController < ApplicationController
   def show
-    render json: DiscoveryEngine::Search.new(query_params).result_set
+    render json: DiscoveryEngine::Query::Search.new(query_params).result_set
   end
 
 private
diff --git a/app/models/publishing_api_document.rb b/app/models/publishing_api_document.rb
index baa2f6e..9b0cfe3 100644
--- a/app/models/publishing_api_document.rb
+++ b/app/models/publishing_api_document.rb
@@ -7,8 +7,8 @@ class PublishingApiDocument
 
   def initialize(
     document_hash,
-    put_service: DiscoveryEngine::Put.new,
-    delete_service: DiscoveryEngine::Delete.new
+    put_service: DiscoveryEngine::Sync::Put.new,
+    delete_service: DiscoveryEngine::Sync::Delete.new
   )
     @document_hash = document_hash
     @put_service = put_service
diff --git a/app/services/discovery_engine/boosts/best_bets.rb b/app/services/discovery_engine/query/best_bets_boost.rb
similarity index 90%
rename from app/services/discovery_engine/boosts/best_bets.rb
rename to app/services/discovery_engine/query/best_bets_boost.rb
index f911e49..639b37e 100644
--- a/app/services/discovery_engine/boosts/best_bets.rb
+++ b/app/services/discovery_engine/query/best_bets_boost.rb
@@ -1,5 +1,5 @@
-module DiscoveryEngine::Boosts
-  class BestBets
+module DiscoveryEngine::Query
+  class BestBetsBoost
     def initialize(query_string)
       @query_string = query_string
     end
diff --git a/app/services/discovery_engine/filters.rb b/app/services/discovery_engine/query/filters.rb
similarity index 93%
rename from app/services/discovery_engine/filters.rb
rename to app/services/discovery_engine/query/filters.rb
index 30f19b9..ce682cd 100644
--- a/app/services/discovery_engine/filters.rb
+++ b/app/services/discovery_engine/query/filters.rb
@@ -1,4 +1,4 @@
-module DiscoveryEngine
+module DiscoveryEngine::Query
   class Filters
     def initialize(query_params)
       @query_params = query_params
diff --git a/app/services/discovery_engine/boosts/news_recency.rb b/app/services/discovery_engine/query/news_recency_boost.rb
similarity index 93%
rename from app/services/discovery_engine/boosts/news_recency.rb
rename to app/services/discovery_engine/query/news_recency_boost.rb
index c8722b8..b84904f 100644
--- a/app/services/discovery_engine/boosts/news_recency.rb
+++ b/app/services/discovery_engine/query/news_recency_boost.rb
@@ -1,5 +1,5 @@
-module DiscoveryEngine::Boosts
-  class NewsRecency
+module DiscoveryEngine::Query
+  class NewsRecencyBoost
     FRESH_AGE = 1.week
     RECENT_AGE = 3.months
     OLD_AGE = 1.year
diff --git a/app/services/discovery_engine/search.rb b/app/services/discovery_engine/query/search.rb
similarity index 94%
rename from app/services/discovery_engine/search.rb
rename to app/services/discovery_engine/query/search.rb
index 46bd2a8..d6042eb 100644
--- a/app/services/discovery_engine/search.rb
+++ b/app/services/discovery_engine/query/search.rb
@@ -1,4 +1,4 @@
-module DiscoveryEngine
+module DiscoveryEngine::Query
   class Search
     DEFAULT_PAGE_SIZE = 10
     DEFAULT_OFFSET = 0
@@ -80,8 +80,8 @@ def serving_config
     def boost_spec
       {
         condition_boost_specs: [
-          *Boosts::NewsRecency.new.boost_specs,
-          *Boosts::BestBets.new(query).boost_specs,
+          *NewsRecencyBoost.new.boost_specs,
+          *BestBetsBoost.new(query).boost_specs,
         ],
       }
     end
diff --git a/app/services/discovery_engine/delete.rb b/app/services/discovery_engine/sync/delete.rb
similarity index 96%
rename from app/services/discovery_engine/delete.rb
rename to app/services/discovery_engine/sync/delete.rb
index ffc54ba..74ef6b8 100644
--- a/app/services/discovery_engine/delete.rb
+++ b/app/services/discovery_engine/sync/delete.rb
@@ -1,4 +1,4 @@
-module DiscoveryEngine
+module DiscoveryEngine::Sync
   class Delete
     include DocumentName
     include Logging
diff --git a/app/services/discovery_engine/document_name.rb b/app/services/discovery_engine/sync/document_name.rb
similarity index 84%
rename from app/services/discovery_engine/document_name.rb
rename to app/services/discovery_engine/sync/document_name.rb
index f302e99..e957222 100644
--- a/app/services/discovery_engine/document_name.rb
+++ b/app/services/discovery_engine/sync/document_name.rb
@@ -1,4 +1,4 @@
-module DiscoveryEngine
+module DiscoveryEngine::Sync
   module DocumentName
     def document_name(content_id)
       "#{Rails.configuration.discovery_engine_datastore_branch}/documents/#{content_id}"
diff --git a/app/services/discovery_engine/logging.rb b/app/services/discovery_engine/sync/logging.rb
similarity index 91%
rename from app/services/discovery_engine/logging.rb
rename to app/services/discovery_engine/sync/logging.rb
index 8d19be0..b0e96af 100644
--- a/app/services/discovery_engine/logging.rb
+++ b/app/services/discovery_engine/sync/logging.rb
@@ -1,4 +1,4 @@
-module DiscoveryEngine
+module DiscoveryEngine::Sync
   module Logging
     def log(level, message, content_id:, payload_version:)
       combined_message = sprintf(
diff --git a/app/services/discovery_engine/put.rb b/app/services/discovery_engine/sync/put.rb
similarity index 97%
rename from app/services/discovery_engine/put.rb
rename to app/services/discovery_engine/sync/put.rb
index 28a9eee..3e8a26d 100644
--- a/app/services/discovery_engine/put.rb
+++ b/app/services/discovery_engine/sync/put.rb
@@ -1,4 +1,4 @@
-module DiscoveryEngine
+module DiscoveryEngine::Sync
   class Put
     MIME_TYPE = "text/html".freeze
 
diff --git a/spec/integration/document_synchronization_spec.rb b/spec/integration/document_synchronization_spec.rb
index ee6e76a..ee49885 100644
--- a/spec/integration/document_synchronization_spec.rb
+++ b/spec/integration/document_synchronization_spec.rb
@@ -3,12 +3,12 @@
 RSpec.describe "Document synchronization" do
   let(:message) { GovukMessageQueueConsumer::MockMessage.new(payload) }
 
-  let(:put_service) { instance_double(DiscoveryEngine::Put, call: nil) }
-  let(:delete_service) { instance_double(DiscoveryEngine::Delete, call: nil) }
+  let(:put_service) { instance_double(DiscoveryEngine::Sync::Put, call: nil) }
+  let(:delete_service) { instance_double(DiscoveryEngine::Sync::Delete, call: nil) }
 
   before do
-    allow(DiscoveryEngine::Put).to receive(:new).and_return(put_service)
-    allow(DiscoveryEngine::Delete).to receive(:new).and_return(delete_service)
+    allow(DiscoveryEngine::Sync::Put).to receive(:new).and_return(put_service)
+    allow(DiscoveryEngine::Sync::Delete).to receive(:new).and_return(delete_service)
 
     PublishingApiMessageProcessor.new.process(message)
   end
diff --git a/spec/requests/search_request_spec.rb b/spec/requests/search_request_spec.rb
index e02afce..99e9ff9 100644
--- a/spec/requests/search_request_spec.rb
+++ b/spec/requests/search_request_spec.rb
@@ -1,10 +1,10 @@
 RSpec.describe "Making a search request" do
-  let(:search_service) { instance_double(DiscoveryEngine::Search, result_set:) }
+  let(:search_service) { instance_double(DiscoveryEngine::Query::Search, result_set:) }
   let(:result_set) { ResultSet.new(results:, total: 42, start: 21) }
   let(:results) { [Result.new(content_id: "123"), Result.new(content_id: "456")] }
 
   before do
-    allow(DiscoveryEngine::Search).to receive(:new).and_return(search_service)
+    allow(DiscoveryEngine::Query::Search).to receive(:new).and_return(search_service)
   end
 
   describe "GET /search.json" do
@@ -25,7 +25,7 @@
     it "passes any query parameters to the search service in the expected format" do
       get "/search.json?q=garden+centres&start=11&count=22"
 
-      expect(DiscoveryEngine::Search).to have_received(:new).with(
+      expect(DiscoveryEngine::Query::Search).to have_received(:new).with(
         hash_including(q: "garden centres", start: "11", count: "22"),
       )
     end
diff --git a/spec/services/discovery_engine/filters_spec.rb b/spec/services/discovery_engine/query/filters_spec.rb
similarity index 94%
rename from spec/services/discovery_engine/filters_spec.rb
rename to spec/services/discovery_engine/query/filters_spec.rb
index ce8ee15..4595931 100644
--- a/spec/services/discovery_engine/filters_spec.rb
+++ b/spec/services/discovery_engine/query/filters_spec.rb
@@ -1,4 +1,4 @@
-RSpec.describe DiscoveryEngine::Filters do
+RSpec.describe DiscoveryEngine::Query::Filters do
   describe "#filter_expression" do
     subject(:filter_expression) { described_class.new(query_params).filter_expression }
 
diff --git a/spec/services/discovery_engine/search_spec.rb b/spec/services/discovery_engine/query/search_spec.rb
similarity index 97%
rename from spec/services/discovery_engine/search_spec.rb
rename to spec/services/discovery_engine/query/search_spec.rb
index e4c3952..efbe6a4 100644
--- a/spec/services/discovery_engine/search_spec.rb
+++ b/spec/services/discovery_engine/query/search_spec.rb
@@ -1,4 +1,4 @@
-RSpec.describe DiscoveryEngine::Search do
+RSpec.describe DiscoveryEngine::Query::Search do
   subject(:search) { described_class.new(query_params, client:) }
 
   let(:client) { double("SearchService::Client", search: search_return_value) }
@@ -18,7 +18,7 @@
   before do
     allow(Rails.configuration).to receive(:discovery_engine_serving_config)
       .and_return("serving-config-path")
-    allow(DiscoveryEngine::Filters).to receive(:new).and_return(filters)
+    allow(DiscoveryEngine::Query::Filters).to receive(:new).and_return(filters)
   end
 
   around do |example|
diff --git a/spec/services/discovery_engine/delete_spec.rb b/spec/services/discovery_engine/sync/delete_spec.rb
similarity index 80%
rename from spec/services/discovery_engine/delete_spec.rb
rename to spec/services/discovery_engine/sync/delete_spec.rb
index b84ffd5..d18fe54 100644
--- a/spec/services/discovery_engine/delete_spec.rb
+++ b/spec/services/discovery_engine/sync/delete_spec.rb
@@ -1,4 +1,4 @@
-RSpec.describe DiscoveryEngine::Delete do
+RSpec.describe DiscoveryEngine::Sync::Delete do
   subject(:delete) { described_class.new(client:) }
 
   let(:client) { double("DocumentService::Client", delete_document: nil) }
@@ -25,7 +25,7 @@
     it "logs the delete operation" do
       expect(logger).to have_received(:add).with(
         Logger::Severity::INFO,
-        "[DiscoveryEngine::Delete] Successfully deleted content_id:some_content_id payload_version:1",
+        "[DiscoveryEngine::Sync::Delete] Successfully deleted content_id:some_content_id payload_version:1",
       )
     end
   end
@@ -42,7 +42,7 @@
     it "logs the failure" do
       expect(logger).to have_received(:add).with(
         Logger::Severity::INFO,
-        "[DiscoveryEngine::Delete] Did not delete document as it doesn't exist remotely (It got lost). content_id:some_content_id payload_version:1",
+        "[DiscoveryEngine::Sync::Delete] Did not delete document as it doesn't exist remotely (It got lost). content_id:some_content_id payload_version:1",
       )
     end
 
@@ -63,7 +63,7 @@
     it "logs the failure" do
       expect(logger).to have_received(:add).with(
         Logger::Severity::ERROR,
-        "[DiscoveryEngine::Delete] Failed to delete document due to an error (Something went wrong) content_id:some_content_id payload_version:1",
+        "[DiscoveryEngine::Sync::Delete] Failed to delete document due to an error (Something went wrong) content_id:some_content_id payload_version:1",
       )
     end
 
diff --git a/spec/services/discovery_engine/put_spec.rb b/spec/services/discovery_engine/sync/put_spec.rb
similarity index 85%
rename from spec/services/discovery_engine/put_spec.rb
rename to spec/services/discovery_engine/sync/put_spec.rb
index cc8b73b..712d956 100644
--- a/spec/services/discovery_engine/put_spec.rb
+++ b/spec/services/discovery_engine/sync/put_spec.rb
@@ -1,4 +1,4 @@
-RSpec.describe DiscoveryEngine::Put do
+RSpec.describe DiscoveryEngine::Sync::Put do
   subject(:put) { described_class.new(client:) }
 
   let(:client) { double("DocumentService::Client", update_document: nil) }
@@ -42,7 +42,7 @@
     it "logs the put operation" do
       expect(logger).to have_received(:add).with(
         Logger::Severity::INFO,
-        "[DiscoveryEngine::Put] Successfully added/updated content_id:some_content_id payload_version:1",
+        "[DiscoveryEngine::Sync::Put] Successfully added/updated content_id:some_content_id payload_version:1",
       )
     end
   end
@@ -59,7 +59,7 @@
     it "logs the failure" do
       expect(logger).to have_received(:add).with(
         Logger::Severity::ERROR,
-        "[DiscoveryEngine::Put] Failed to add/update document due to an error (Something went wrong) content_id:some_content_id payload_version:1",
+        "[DiscoveryEngine::Sync::Put] Failed to add/update document due to an error (Something went wrong) content_id:some_content_id payload_version:1",
       )
     end