diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 91f75f6..b27528e 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -45,9 +45,7 @@ jobs: RAILS_ENV: test # All Google client library calls are mocked, but the application needs this set to boot GOOGLE_CLOUD_PROJECT_ID: not-used - DISCOVERY_ENGINE_SERVING_CONFIG: not-used - DISCOVERY_ENGINE_DATASTORE: not-used - DISCOVERY_ENGINE_DATASTORE_BRANCH: not-used + DISCOVERY_ENGINE_DEFAULT_COLLECTION_NAME: not-used # Redis running through govuk-infrastructure action REDIS_URL: redis://localhost:6379 steps: diff --git a/app/models/branch.rb b/app/models/branch.rb new file mode 100644 index 0000000..40944ba --- /dev/null +++ b/app/models/branch.rb @@ -0,0 +1,21 @@ +# Represents a branch on a Discovery Engine data store. +# +# Currently, every data store on Discovery Engine has exactly *one* branch (the default branch), and +# we are not able to make any changes to that resource. However, we still need to model it here +# because documents are children of the branch, not the datastore itself. +# +# see https://cloud.google.com/ruby/docs/reference/google-cloud-discovery_engine-v1/latest/Google-Cloud-DiscoveryEngine-V1-DocumentService-Client +# (there is no documentation specific to branches) +Branch = Data.define(:remote_resource_id) do + include DiscoveryEngineNameable + + # The default branch automatically available on a data store + def self.default + new("default_branch") + end + + def parent + # We only use a single data store in our architecture, so we can hardcode it here. + DataStore.default + end +end diff --git a/app/models/concerns/discovery_engine_nameable.rb b/app/models/concerns/discovery_engine_nameable.rb new file mode 100644 index 0000000..dcd9c3b --- /dev/null +++ b/app/models/concerns/discovery_engine_nameable.rb @@ -0,0 +1,31 @@ +# Enhances models with a `#name` method returning their fully qualified Google Cloud Platform +# resource name (like a path). +# +# For example, for a `Control`, this would be: +# projects/{project}/locations/{location}/collections/{collection_id}/engines/ +# {engine_id}/controls/{control_id} +# +# Requires the including class to implement `#remote_resource_id`, and optionally `#parent` if the +# parent resource is not the default collection. +module DiscoveryEngineNameable + # The name (fully qualified path) of this Discovery Engine resource on GCP + def name + [parent_name, resource_path_fragment, remote_resource_id].join("/") + end + +private + + def resource_path_fragment + # For example: `DataStore` -> `dataStores` + self.class.name.downcase_first.pluralize + end + + def parent_name + if respond_to?(:parent) + parent.name + else + # The default collection is the parent of all root-level resources + Rails.configuration.discovery_engine_default_collection_name + end + end +end diff --git a/app/models/data_store.rb b/app/models/data_store.rb new file mode 100644 index 0000000..9461818 --- /dev/null +++ b/app/models/data_store.rb @@ -0,0 +1,15 @@ +# Represents a data store on Discovery Engine. +# +# A data store contains the indexed documents that can be searched through an engine. +# +# Our architecture currently only has a single data store, so we do not need the ability to manage +# data stores through Search Admin. +# +# see https://cloud.google.com/ruby/docs/reference/google-cloud-discovery_engine-v1/latest/Google-Cloud-DiscoveryEngine-V1-DataStore +DataStore = Data.define(:remote_resource_id) do + include DiscoveryEngineNameable + + def self.default + new("govuk_content") + end +end diff --git a/app/models/engine.rb b/app/models/engine.rb new file mode 100644 index 0000000..cdf826a --- /dev/null +++ b/app/models/engine.rb @@ -0,0 +1,18 @@ +# Represents an engine on Discovery Engine. +# +# An engine (called "app" in the Google Cloud UI) is an abstraction over the data stores that +# contain our searchable documents, and is used for querying. It is the parent resource of several +# other resources such as controls and serving configs. +# +# Our architecture currently only has a single engine, so we do not need the ability to manage +# engines through Search Admin. +# +# see https://cloud.google.com/ruby/docs/reference/google-cloud-discovery_engine-v1/latest/Google-Cloud-DiscoveryEngine-V1-Engine +Engine = Data.define(:remote_resource_id) do + include DiscoveryEngineNameable + + # The default engine created through Terraform in `govuk-infrastructure` + def self.default + new("govuk") + end +end diff --git a/app/models/serving_config.rb b/app/models/serving_config.rb new file mode 100644 index 0000000..9f87704 --- /dev/null +++ b/app/models/serving_config.rb @@ -0,0 +1,21 @@ +# Represents a serving config on Discovery Engine. +# +# A serving config is an endpoint on an engine that can be used for +# querying. Each serving config can have different configuration (in particular, different sets of +# active controls), which allows us to test out new configuration changes outside of the default +# serving config. +# +# see https://cloud.google.com/ruby/docs/reference/google-cloud-discovery_engine-v1beta/latest/Google-Cloud-DiscoveryEngine-V1beta-ServingConfig +ServingConfig = Data.define(:remote_resource_id) do + include DiscoveryEngineNameable + + # The default serving config automatically available on an engine + def self.default + new("default_search") + end + + def parent + # We only use a single engine in our architecture, so we can hardcode it here. + Engine.default + end +end diff --git a/app/services/discovery_engine/autocomplete/complete.rb b/app/services/discovery_engine/autocomplete/complete.rb index 5309b67..94dd6ca 100644 --- a/app/services/discovery_engine/autocomplete/complete.rb +++ b/app/services/discovery_engine/autocomplete/complete.rb @@ -32,14 +32,10 @@ def suggestions def complete_query_request { - data_store:, + data_store: DataStore.default.name, query:, query_model: QUERY_MODEL, } end - - def data_store - Rails.configuration.discovery_engine_datastore - end end end diff --git a/app/services/discovery_engine/autocomplete/update_denylist.rb b/app/services/discovery_engine/autocomplete/update_denylist.rb index f428943..cfe7263 100644 --- a/app/services/discovery_engine/autocomplete/update_denylist.rb +++ b/app/services/discovery_engine/autocomplete/update_denylist.rb @@ -54,7 +54,7 @@ def bucket_name end def parent - Rails.configuration.discovery_engine_datastore + DataStore.default.name end end end diff --git a/app/services/discovery_engine/query/search.rb b/app/services/discovery_engine/query/search.rb index 0985405..94d4853 100644 --- a/app/services/discovery_engine/query/search.rb +++ b/app/services/discovery_engine/query/search.rb @@ -56,7 +56,7 @@ def query end def serving_config - Rails.configuration.discovery_engine_serving_config + ServingConfig.default.name end def page_size diff --git a/app/services/discovery_engine/sync/operation.rb b/app/services/discovery_engine/sync/operation.rb index f9d182f..7a851f8 100644 --- a/app/services/discovery_engine/sync/operation.rb +++ b/app/services/discovery_engine/sync/operation.rb @@ -60,7 +60,7 @@ def version_cache end def document_name - "#{Rails.configuration.discovery_engine_datastore_branch}/documents/#{content_id}" + [Branch.default.name, "documents", content_id].join("/") end def log(level, message) diff --git a/app/services/discovery_engine/user_events/import.rb b/app/services/discovery_engine/user_events/import.rb index a2de75d..66aa725 100644 --- a/app/services/discovery_engine/user_events/import.rb +++ b/app/services/discovery_engine/user_events/import.rb @@ -53,7 +53,7 @@ def call table_id:, partition_date:, }, - parent: Rails.configuration.discovery_engine_datastore, + parent: DataStore.default.name, ) logger.info("Waiting for import_user_events operation to finish remotely") diff --git a/config/application.rb b/config/application.rb index e08e72e..67ba14e 100644 --- a/config/application.rb +++ b/config/application.rb @@ -17,9 +17,7 @@ class Application < Rails::Application config.api_only = true # Google Discovery Engine configuration - config.discovery_engine_serving_config = ENV.fetch("DISCOVERY_ENGINE_SERVING_CONFIG") - config.discovery_engine_datastore = ENV.fetch("DISCOVERY_ENGINE_DATASTORE") - config.discovery_engine_datastore_branch = ENV.fetch("DISCOVERY_ENGINE_DATASTORE_BRANCH") + config.discovery_engine_default_collection_name = ENV.fetch("DISCOVERY_ENGINE_DEFAULT_COLLECTION_NAME") config.google_cloud_project_id = ENV.fetch("GOOGLE_CLOUD_PROJECT_ID") # Document sync configuration diff --git a/config/environments/test.rb b/config/environments/test.rb index 7710e32..757333b 100644 --- a/config/environments/test.rb +++ b/config/environments/test.rb @@ -39,6 +39,9 @@ # Raise error when a before_action's only/except options reference missing actions. config.action_controller.raise_on_missing_callback_actions = true + + # Google Discovery Engine configuration + config.discovery_engine_default_collection_name = "[collection]" end # TODO: remove this workaround once GovukPrometheusExporter initialisation is fixed in govuk_app_config. diff --git a/spec/models/branch_spec.rb b/spec/models/branch_spec.rb new file mode 100644 index 0000000..9585794 --- /dev/null +++ b/spec/models/branch_spec.rb @@ -0,0 +1,15 @@ +RSpec.describe Branch do + subject(:branch) { described_class.new("my-branch") } + + describe ".default" do + it "returns the default branch" do + expect(described_class.default).to eq(described_class.new("default_branch")) + end + end + + describe "#name" do + it "returns the fully qualified name of the branch" do + expect(branch.name).to eq("[collection]/dataStores/govuk_content/branches/my-branch") + end + end +end diff --git a/spec/models/data_store_spec.rb b/spec/models/data_store_spec.rb new file mode 100644 index 0000000..80a1537 --- /dev/null +++ b/spec/models/data_store_spec.rb @@ -0,0 +1,15 @@ +RSpec.describe DataStore do + subject(:data_store) { described_class.new("my-data-store") } + + describe ".default" do + it "returns the default data store" do + expect(described_class.default).to eq(described_class.new("govuk_content")) + end + end + + describe "#name" do + it "returns the fully qualified name of the data store" do + expect(data_store.name).to eq("[collection]/dataStores/my-data-store") + end + end +end diff --git a/spec/models/engine_spec.rb b/spec/models/engine_spec.rb new file mode 100644 index 0000000..9ef4543 --- /dev/null +++ b/spec/models/engine_spec.rb @@ -0,0 +1,15 @@ +RSpec.describe Engine do + subject(:engine) { described_class.new("my-engine") } + + describe ".default" do + it "returns the default engine" do + expect(described_class.default).to eq(described_class.new("govuk")) + end + end + + describe "#name" do + it "returns the fully qualified name of the engine" do + expect(engine.name).to eq("[collection]/engines/my-engine") + end + end +end diff --git a/spec/models/serving_config_spec.rb b/spec/models/serving_config_spec.rb new file mode 100644 index 0000000..b93afb7 --- /dev/null +++ b/spec/models/serving_config_spec.rb @@ -0,0 +1,15 @@ +RSpec.describe ServingConfig do + subject(:serving_config) { described_class.new("my-serving-config") } + + describe ".default" do + it "returns the default serving config" do + expect(described_class.default).to eq(described_class.new("default_search")) + end + end + + describe "#name" do + it "returns the fully qualified name of the serving config" do + expect(serving_config.name).to eq("[collection]/engines/govuk/servingConfigs/my-serving-config") + end + end +end diff --git a/spec/services/discovery_engine/autocomplete/complete_spec.rb b/spec/services/discovery_engine/autocomplete/complete_spec.rb index 4d21db4..095bc38 100644 --- a/spec/services/discovery_engine/autocomplete/complete_spec.rb +++ b/spec/services/discovery_engine/autocomplete/complete_spec.rb @@ -3,10 +3,6 @@ let(:client) { double("completion service", complete_query:) } - before do - allow(Rails.configuration).to receive(:discovery_engine_datastore).and_return("/the/datastore") - end - describe "#completion_result" do subject(:completion_result) { completion.completion_result } @@ -22,7 +18,7 @@ completion_result expect(client).to have_received(:complete_query).with( - data_store: "/the/datastore", + data_store: DataStore.default.name, query:, query_model: "user-event", ) diff --git a/spec/services/discovery_engine/autocomplete/update_denylist_spec.rb b/spec/services/discovery_engine/autocomplete/update_denylist_spec.rb index 8540a04..7c0af44 100644 --- a/spec/services/discovery_engine/autocomplete/update_denylist_spec.rb +++ b/spec/services/discovery_engine/autocomplete/update_denylist_spec.rb @@ -15,7 +15,6 @@ before do allow(Rails.configuration).to receive_messages( - discovery_engine_datastore: "data/store", google_cloud_project_id: "my-fancy-project", ) end @@ -25,7 +24,7 @@ update_denylist.call expect(client).to have_received(:purge_suggestion_deny_list_entries) - .with(parent: "data/store") + .with(parent: DataStore.default.name) expect(purge_operation).to have_received(:wait_until_done!) end @@ -37,7 +36,7 @@ data_schema: "suggestion_deny_list", input_uris: ["gs://my-fancy-project_vais_artifacts/denylist.jsonl"], }, - parent: "data/store", + parent: DataStore.default.name, ) expect(import_operation).to have_received(:wait_until_done!) end diff --git a/spec/services/discovery_engine/query/search_spec.rb b/spec/services/discovery_engine/query/search_spec.rb index 324e4c3..ec3353e 100644 --- a/spec/services/discovery_engine/query/search_spec.rb +++ b/spec/services/discovery_engine/query/search_spec.rb @@ -16,8 +16,6 @@ end before do - allow(Rails.configuration).to receive(:discovery_engine_serving_config) - .and_return("serving-config-path") allow(DiscoveryEngine::Query::Filters).to receive(:new).and_return(filters) end @@ -52,7 +50,7 @@ it "calls the client with the expected parameters" do expect(client).to have_received(:search).with( - serving_config: "serving-config-path", + serving_config: ServingConfig.default.name, query: "garden centres", offset: 0, page_size: 10, @@ -76,7 +74,7 @@ it "calls the client with the expected parameters" do expect(client).to have_received(:search).with( - serving_config: "serving-config-path", + serving_config: ServingConfig.default.name, query: "garden centres", offset: 11, page_size: 22, diff --git a/spec/services/discovery_engine/sync/delete_spec.rb b/spec/services/discovery_engine/sync/delete_spec.rb index 8d3222a..a0664cf 100644 --- a/spec/services/discovery_engine/sync/delete_spec.rb +++ b/spec/services/discovery_engine/sync/delete_spec.rb @@ -14,7 +14,7 @@ it "deletes the document" do expect(client).to have_received(:delete_document) - .with(name: "branch/documents/some_content_id") + .with(name: "#{Branch.default.name}/documents/some_content_id") end it_behaves_like "a successful sync operation", "delete" diff --git a/spec/services/discovery_engine/sync/put_spec.rb b/spec/services/discovery_engine/sync/put_spec.rb index 8119434..4ce79bc 100644 --- a/spec/services/discovery_engine/sync/put_spec.rb +++ b/spec/services/discovery_engine/sync/put_spec.rb @@ -26,7 +26,7 @@ expect(client).to have_received(:update_document).with( document: { id: "some_content_id", - name: "branch/documents/some_content_id", + name: "#{Branch.default.name}/documents/some_content_id", json_data: "{\"foo\":\"bar\",\"payload_version\":\"1\"}", content: { mime_type: "text/html", diff --git a/spec/services/discovery_engine/sync/shared_examples.rb b/spec/services/discovery_engine/sync/shared_examples.rb index ea3d351..e6ad61c 100644 --- a/spec/services/discovery_engine/sync/shared_examples.rb +++ b/spec/services/discovery_engine/sync/shared_examples.rb @@ -8,7 +8,6 @@ before do allow(Kernel).to receive(:sleep).and_return(nil) allow(Rails).to receive(:logger).and_return(logger) - allow(Rails.configuration).to receive(:discovery_engine_datastore_branch).and_return("branch") allow(GovukError).to receive(:notify) allow(Coordination::DocumentLock).to receive(:new).with("some_content_id").and_return(lock) diff --git a/spec/services/discovery_engine/user_events/import_spec.rb b/spec/services/discovery_engine/user_events/import_spec.rb index 451da0c..2ea5681 100644 --- a/spec/services/discovery_engine/user_events/import_spec.rb +++ b/spec/services/discovery_engine/user_events/import_spec.rb @@ -14,7 +14,6 @@ before do allow(Rails.configuration).to receive_messages( - discovery_engine_datastore: "data/store", google_cloud_project_id: "my-fancy-project", ) end @@ -55,7 +54,7 @@ table_id: "search-event", partition_date: Google::Type::Date.new(year: 2000, month: 1, day: 1), }, - parent: "data/store", + parent: DataStore.default.name, ) end end @@ -71,7 +70,7 @@ table_id: "search-intraday-event", partition_date: Google::Type::Date.new(year: 1989, month: 12, day: 13), }, - parent: "data/store", + parent: DataStore.default.name, ) end end