Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Split DiscoveryEngine service module into sync/query #145

Merged
merged 1 commit into from
Dec 11, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion app/controllers/searches_controller.rb
Original file line number Diff line number Diff line change
@@ -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
Expand Down
4 changes: 2 additions & 2 deletions app/models/publishing_api_document.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
module DiscoveryEngine::Boosts
class BestBets
module DiscoveryEngine::Query
class BestBetsBoost
def initialize(query_string)
@query_string = query_string
end
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
module DiscoveryEngine
module DiscoveryEngine::Query
class Filters
def initialize(query_params)
@query_params = query_params
Expand Down
Original file line number Diff line number Diff line change
@@ -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
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
module DiscoveryEngine
module DiscoveryEngine::Query
class Search
DEFAULT_PAGE_SIZE = 10
DEFAULT_OFFSET = 0
Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
module DiscoveryEngine
module DiscoveryEngine::Sync
class Delete
include DocumentName
include Logging
Expand Down
Original file line number Diff line number Diff line change
@@ -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}"
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
module DiscoveryEngine
module DiscoveryEngine::Sync
module Logging
def log(level, message, content_id:, payload_version:)
combined_message = sprintf(
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
module DiscoveryEngine
module DiscoveryEngine::Sync
class Put
MIME_TYPE = "text/html".freeze

Expand Down
8 changes: 4 additions & 4 deletions spec/integration/document_synchronization_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 3 additions & 3 deletions spec/requests/search_request_spec.rb
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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
Expand Down
Original file line number Diff line number Diff line change
@@ -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 }

Expand Down
Original file line number Diff line number Diff line change
@@ -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) }
Expand All @@ -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|
Expand Down
Original file line number Diff line number Diff line change
@@ -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) }
Expand All @@ -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
Expand All @@ -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

Expand All @@ -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

Expand Down
Original file line number Diff line number Diff line change
@@ -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) }
Expand Down Expand Up @@ -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
Expand All @@ -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

Expand Down