diff --git a/Dockerfile b/Dockerfile index 249311c9..132c5afa 100644 --- a/Dockerfile +++ b/Dockerfile @@ -15,7 +15,6 @@ RUN bootsnap precompile --gemfile . # current build stage. ENV SECRET_KEY_BASE=none ENV GOVUK_NOTIFY_API_KEY=none -ENV DISCOVERY_ENGINE_DATASTORE=none ENV DISCOVERY_ENGINE_DEFAULT_COLLECTION_NAME=none RUN rails assets:precompile && rm -fr log diff --git a/app/clients/discovery_engine/control_client.rb b/app/clients/discovery_engine/control_client.rb index 3568824b..d5df3556 100644 --- a/app/clients/discovery_engine/control_client.rb +++ b/app/clients/discovery_engine/control_client.rb @@ -5,8 +5,8 @@ class ControlClient def create(control) discovery_engine_client.create_control( control: control.to_discovery_engine_control, - control_id: control.discovery_engine_id, - parent: control.parent, + control_id: control.remote_resource_id, + parent: control.parent.name, ) rescue Google::Cloud::Error => e set_record_errors(control, e) diff --git a/app/models/concerns/discovery_engine_nameable.rb b/app/models/concerns/discovery_engine_nameable.rb new file mode 100644 index 00000000..dcd9c3b6 --- /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/control.rb b/app/models/control.rb index 37207c02..a443ed0b 100644 --- a/app/models/control.rb +++ b/app/models/control.rb @@ -10,6 +10,7 @@ # see # https://cloud.google.com/ruby/docs/reference/google-cloud-discovery_engine-v1/latest/Google-Cloud-DiscoveryEngine-V1-Control class Control < ApplicationRecord + include DiscoveryEngineNameable include RemoteSynchronizable remote_synchronize with: DiscoveryEngine::ControlClient @@ -30,18 +31,13 @@ def to_discovery_engine_control } end - # The fully qualified name of the control on Discovery Engine (like a path) - def name - [parent, "controls", discovery_engine_id].join("/") - end - - # The parent of the control on Discovery Engine (always the engine) + # The parent of the control on Discovery Engine (always the default engine) def parent - Rails.configuration.discovery_engine_engine + Engine.default end # The ID of the resource on Discovery Engine - def discovery_engine_id + def remote_resource_id "search-admin-#{id}" end end diff --git a/app/models/control/boost_action.rb b/app/models/control/boost_action.rb index 10ef2ada..4ae5b800 100644 --- a/app/models/control/boost_action.rb +++ b/app/models/control/boost_action.rb @@ -23,7 +23,7 @@ def to_discovery_engine_control_action boost_action: { filter: filter_expression, boost: boost_factor, - data_store: Rails.configuration.discovery_engine_datastore, + data_store: DataStore.default.name, }, } end diff --git a/app/models/control/filter_action.rb b/app/models/control/filter_action.rb index 4956f70d..8d2da335 100644 --- a/app/models/control/filter_action.rb +++ b/app/models/control/filter_action.rb @@ -16,7 +16,7 @@ def to_discovery_engine_control_action { filter_action: { filter: filter_expression, - data_store: Rails.configuration.discovery_engine_datastore, + data_store: DataStore.default.name, }, } end diff --git a/app/models/data_store.rb b/app/models/data_store.rb new file mode 100644 index 00000000..45146058 --- /dev/null +++ b/app/models/data_store.rb @@ -0,0 +1,28 @@ +# 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 +class DataStore + include DiscoveryEngineNameable + + # The ID of the default datastore created through Terraform in `govuk_infrastructure` + DEFAULT_DATA_STORE_ID = "govuk_content".freeze + + attr_reader :remote_resource_id + + def self.default + new(DEFAULT_DATA_STORE_ID) + end + + def initialize(remote_resource_id) + @remote_resource_id = remote_resource_id + end + + def ==(other) + remote_resource_id == other.remote_resource_id + end +end diff --git a/app/models/engine.rb b/app/models/engine.rb new file mode 100644 index 00000000..0cec4cc5 --- /dev/null +++ b/app/models/engine.rb @@ -0,0 +1,30 @@ +# 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 +class Engine + include DiscoveryEngineNameable + + # The ID of the default engine created through Terraform in `govuk-infrastructure` + DEFAULT_ENGINE_ID = "govuk".freeze + + attr_reader :remote_resource_id + + def self.default + new(DEFAULT_ENGINE_ID) + end + + def initialize(remote_resource_id) + @remote_resource_id = remote_resource_id + end + + def ==(other) + remote_resource_id == other.remote_resource_id + end +end diff --git a/app/views/controls/show.html.erb b/app/views/controls/show.html.erb index a85f90f3..146c558c 100644 --- a/app/views/controls/show.html.erb +++ b/app/views/controls/show.html.erb @@ -37,6 +37,10 @@ field: t_model_attr(:display_name), value: @control.display_name, }, + { + field: t_model_attr(:name), + value: tag.code(@control.name), + } ] } %> diff --git a/config/application.rb b/config/application.rb index 86c3aea1..19dfef91 100644 --- a/config/application.rb +++ b/config/application.rb @@ -47,7 +47,6 @@ class Application < Rails::Application config.active_record.default_column_serializer = YAML # Google Discovery Engine configuration - config.discovery_engine_datastore = ENV.fetch("DISCOVERY_ENGINE_DATASTORE") config.discovery_engine_default_collection_name = ENV.fetch("DISCOVERY_ENGINE_DEFAULT_COLLECTION_NAME") end end diff --git a/config/environments/test.rb b/config/environments/test.rb index 6c68aa70..7c6a7739 100644 --- a/config/environments/test.rb +++ b/config/environments/test.rb @@ -59,7 +59,5 @@ config.action_controller.raise_on_missing_callback_actions = true # Google Discovery Engine configuration - config.discovery_engine_datastore = "[datastore]" - config.discovery_engine_engine = "[engine]" - config.discovery_engine_serving_config = "[serving_config]" + config.discovery_engine_default_collection_name = "[collection]" end diff --git a/config/locales/en.yml b/config/locales/en.yml index d7539450..24265ba3 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -78,6 +78,7 @@ en: control: display_name: Name action: Action + name: Google Cloud identifier control/boost_action: boost_factor: Boost factor filter_expression: Filter expression diff --git a/spec/clients/discovery_engine/control_client_spec.rb b/spec/clients/discovery_engine/control_client_spec.rb index 51e22b88..6126fa2f 100644 --- a/spec/clients/discovery_engine/control_client_spec.rb +++ b/spec/clients/discovery_engine/control_client_spec.rb @@ -20,7 +20,7 @@ expect(discovery_engine_client).to receive(:create_control).with( control: control.to_discovery_engine_control, control_id: "search-admin-42", - parent: "[engine]", + parent: Engine.default.name, ) subject.create(control) # rubocop:disable Rails/SaveBang (not an ActiveRecord model) @@ -80,7 +80,7 @@ describe "#delete" do it "deletes the control on Discovery Engine" do expect(discovery_engine_client).to receive(:delete_control).with( - name: "[engine]/controls/search-admin-42", + name: control.name, ) subject.delete(control) diff --git a/spec/models/control/boost_action_spec.rb b/spec/models/control/boost_action_spec.rb index 18036efc..ec44f5f3 100644 --- a/spec/models/control/boost_action_spec.rb +++ b/spec/models/control/boost_action_spec.rb @@ -59,7 +59,7 @@ boost_action: { filter: "foo = 1", boost: 0.13, - data_store: "[datastore]", + data_store: DataStore.default.name, }, }) end diff --git a/spec/models/control/filter_action_spec.rb b/spec/models/control/filter_action_spec.rb index 941dd508..261bd846 100644 --- a/spec/models/control/filter_action_spec.rb +++ b/spec/models/control/filter_action_spec.rb @@ -23,7 +23,7 @@ expect(filter.to_discovery_engine_control_action).to eq({ filter_action: { filter: "foo = 1", - data_store: "[datastore]", + data_store: DataStore.default.name, }, }) end diff --git a/spec/models/control_spec.rb b/spec/models/control_spec.rb index 1632484b..b870c764 100644 --- a/spec/models/control_spec.rb +++ b/spec/models/control_spec.rb @@ -34,28 +34,28 @@ let(:action) { build(:control_boost_action) } - describe "#discovery_engine_id" do + describe "#remote_resource_id" do it "builds an ID from the control's database ID" do - expect(control.discovery_engine_id).to eq("search-admin-42") + expect(control.remote_resource_id).to eq("search-admin-42") end end describe "#parent" do it "is the configured engine" do - expect(control.parent).to eq("[engine]") + expect(control.parent).to eq(Engine.default) end end describe "#name" do it "returns the fully qualified name of the control" do - expect(control.name).to eq("[engine]/controls/search-admin-42") + expect(control.name).to eq("#{Engine.default.name}/controls/search-admin-42") end end describe "#to_discovery_engine_control" do it "returns a representation of the control for Discovery Engine" do expect(control.to_discovery_engine_control).to include( - name: "[engine]/controls/search-admin-42", + name: "#{Engine.default.name}/controls/search-admin-42", display_name: "My boost control", # We don't care what's in the action (that's tested elsewhere), but we do care that the # key is present diff --git a/spec/models/data_store_spec.rb b/spec/models/data_store_spec.rb new file mode 100644 index 00000000..237e2822 --- /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(subject.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 00000000..c7300760 --- /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(subject.name).to eq("[collection]/engines/my-engine") + end + end +end