Merge pull request #73 from alphagov/vertex-delete

Implement `GoogleDiscoveryEngine::Repository#delete`

lib/repositories/google_discovery_engine/repository.rb

@@ -4,6 +4,9 @@ module Repositories
   module GoogleDiscoveryEngine
     # A repository integrating with Google Discovery Engine
     class Repository
+      # We only ever use the default branch (for now)
+      BRANCH = "/branches/default_branch".freeze
+
       def initialize(
         client: ::Google::Cloud::DiscoveryEngine.document_service(version: :v1),
         logger: Logger.new($stdout, progname: self.class.name)
@@ -24,12 +27,34 @@ def put(content_id, metadata, content: nil, payload_version: nil)
       end
 
       def delete(content_id, payload_version: nil)
-        logger.info(sprintf("[DELETE %s@v%s]", content_id, payload_version))
+        client.delete_document(name: document_name(content_id))
+
+        logger.info(sprintf("[GCDE][DELETE %s@v%s]", content_id, payload_version))
+      rescue Google::Cloud::NotFoundError => e
+        # TODO: Should we eventually send this to Sentry? A document not existing is a relatively
+        # common error initially as an unpublish request may come through before we've imported the
+        # document to begin with.
+        logger.warn(sprintf("[GCDE][DELETE %s@v%s] %s", content_id, payload_version, e.message))
+      rescue Google::Cloud::Error => e
+        logger.error(sprintf("[GCDE][DELETE %s@v%s] %s", content_id, payload_version, e.message))
+        GovukError.notify(e)
       end
 
     private
 
       attr_reader :client, :logger
+
+      def datastore_path
+        ENV.fetch("DISCOVERY_ENGINE_DATASTORE")
+      end
+
+      def branch_path
+        datastore_path + BRANCH
+      end
+
+      def document_name(content_id)
+        "#{branch_path}/documents/#{content_id}"
+      end
     end
   end
 end

spec/lib/repositories/google_discovery_engine/repository_spec.rb

@@ -6,7 +6,12 @@
 RSpec.describe Repositories::GoogleDiscoveryEngine::Repository do
   let(:repository) { described_class.new(client:, logger:) }
   let(:client) { instance_double(Google::Cloud::DiscoveryEngine::V1::DocumentService::Client) }
-  let(:logger) { instance_double(Logger, info: nil) }
+  let(:logger) { instance_double(Logger, info: nil, warn: nil, error: nil) }
+
+  before do
+    allow(ENV).to receive(:fetch).with("DISCOVERY_ENGINE_DATASTORE").and_return("datastore-path")
+    allow(GovukError).to receive(:notify)
+  end
 
   describe "#put" do
     it "logs the put operation" do
@@ -25,10 +30,57 @@
   end
 
   describe "#delete" do
-    it "logs the delete operation" do
-      repository.delete("some_content_id", payload_version: "1")
+    context "when the delete succeeds" do
+      before do
+        allow(client).to receive(:delete_document)
+
+        repository.delete("some_content_id", payload_version: "1")
+      end
+
+      it "deletes the document" do
+        expect(client).to have_received(:delete_document)
+          .with(name: "datastore-path/branches/default_branch/documents/some_content_id")
+      end
+
+      it "logs the delete operation" do
+        expect(logger).to have_received(:info).with("[GCDE][DELETE some_content_id@v1]")
+      end
+    end
+
+    context "when the delete fails because the document doesn't exist" do
+      let(:err) { Google::Cloud::NotFoundError.new("It got lost") }
+
+      before do
+        allow(client).to receive(:delete_document).and_raise(err)
+
+        repository.delete("some_content_id", payload_version: "1")
+      end
+
+      it "logs the failure" do
+        expect(logger).to have_received(:warn).with("[GCDE][DELETE some_content_id@v1] It got lost")
+      end
+
+      it "does not send the error to Sentry" do
+        expect(GovukError).not_to have_received(:notify)
+      end
+    end
+
+    context "when the delete fails for another reason" do
+      let(:err) { Google::Cloud::Error.new("Something went wrong") }
+
+      before do
+        allow(client).to receive(:delete_document).and_raise(err)
+
+        repository.delete("some_content_id", payload_version: "1")
+      end
+
+      it "logs the failure" do
+        expect(logger).to have_received(:error).with("[GCDE][DELETE some_content_id@v1] Something went wrong")
+      end
 
-      expect(logger).to have_received(:info).with("[DELETE some_content_id@v1]")
+      it "send the error to Sentry" do
+        expect(GovukError).to have_received(:notify).with(err)
+      end
     end
   end
 end