From d00c04868978b945de330146cfa50842f3baf6f8 Mon Sep 17 00:00:00 2001 From: Sean Rankine Date: Mon, 9 Dec 2024 13:45:04 +0000 Subject: [PATCH] Notify Router only on specific column updates Currently, we send a notification to the Router to update route tables whenever there is any change to the content_items (or publish_intents) tables. However, the Router only needs to update the route table when changes affect specific columns that determine the routes. This change should reduce unnecessary reloading. --- ...update_notify_trigger_for_route_changes.rb | 47 +++++++++++++++++++ db/structure.sql | 29 +++++++++++- spec/integration/notify_route_change_spec.rb | 46 ++++++++++++++++-- 3 files changed, 117 insertions(+), 5 deletions(-) create mode 100644 db/migrate/20241209132444_update_notify_trigger_for_route_changes.rb diff --git a/db/migrate/20241209132444_update_notify_trigger_for_route_changes.rb b/db/migrate/20241209132444_update_notify_trigger_for_route_changes.rb new file mode 100644 index 00000000..8fd62552 --- /dev/null +++ b/db/migrate/20241209132444_update_notify_trigger_for_route_changes.rb @@ -0,0 +1,47 @@ +class UpdateNotifyTriggerForRouteChanges < ActiveRecord::Migration[7.2] + def up + execute <<-SQL + CREATE OR REPLACE FUNCTION notify_route_change() RETURNS trigger AS $$ + BEGIN + -- Trigger on INSERT or DELETE + IF (TG_OP = 'INSERT' OR TG_OP = 'DELETE') THEN + PERFORM pg_notify('route_changes', ''); + RETURN COALESCE(NEW, OLD); + END IF; + + -- Trigger on UPDATE for specific columns + IF (TG_OP = 'UPDATE') THEN + IF TG_TABLE_NAME = 'content_items' THEN + -- Specific column checks for the content_items table + IF (NEW.routes IS DISTINCT FROM OLD.routes OR + NEW.redirects IS DISTINCT FROM OLD.redirects OR + NEW.schema_name IS DISTINCT FROM OLD.schema_name OR + NEW.rendering_app IS DISTINCT FROM OLD.rendering_app) THEN + PERFORM pg_notify('route_changes', ''); + END IF; + ELSIF TG_TABLE_NAME = 'publish_intents' THEN + -- Specific column checks for publish_intents table + IF (NEW.routes IS DISTINCT FROM OLD.routes OR + NEW.rendering_app IS DISTINCT FROM OLD.rendering_app) THEN + PERFORM pg_notify('route_changes', ''); + END IF; + END IF; + END IF; + + RETURN COALESCE(NEW, OLD); + END; + $$ LANGUAGE plpgsql; + SQL + end + + def down + execute <<-SQL + CREATE OR REPLACE FUNCTION notify_route_change() RETURNS trigger AS $$ + BEGIN + PERFORM pg_notify('route_changes', ''); + RETURN OLD; + END; + $$ LANGUAGE plpgsql; + SQL + end +end diff --git a/db/structure.sql b/db/structure.sql index 85accf46..5f68f3e1 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -52,8 +52,32 @@ CREATE FUNCTION public.notify_route_change() RETURNS trigger LANGUAGE plpgsql AS $$ BEGIN - PERFORM pg_notify('route_changes', ''); - RETURN OLD; + -- Trigger on INSERT or DELETE + IF (TG_OP = 'INSERT' OR TG_OP = 'DELETE') THEN + PERFORM pg_notify('route_changes', ''); + RETURN COALESCE(NEW, OLD); + END IF; + + -- Trigger on UPDATE for specific columns + IF (TG_OP = 'UPDATE') THEN + IF TG_TABLE_NAME = 'content_items' THEN + -- Specific column checks for the content_items table + IF (NEW.routes IS DISTINCT FROM OLD.routes OR + NEW.redirects IS DISTINCT FROM OLD.redirects OR + NEW.schema_name IS DISTINCT FROM OLD.schema_name OR + NEW.rendering_app IS DISTINCT FROM OLD.rendering_app) THEN + PERFORM pg_notify('route_changes', ''); + END IF; + ELSIF TG_TABLE_NAME = 'publish_intents' THEN + -- Specific column checks for publish_intents table + IF (NEW.routes IS DISTINCT FROM OLD.routes OR + NEW.rendering_app IS DISTINCT FROM OLD.rendering_app) THEN + PERFORM pg_notify('route_changes', ''); + END IF; + END IF; + END IF; + + RETURN COALESCE(NEW, OLD); END; $$; @@ -437,6 +461,7 @@ CREATE TRIGGER publish_intent_change_trigger AFTER INSERT OR DELETE OR UPDATE ON SET search_path TO "$user", public; INSERT INTO "schema_migrations" (version) VALUES +('20241209132444'), ('20241105135438'), ('20240312132747'), ('20240220151333'), diff --git a/spec/integration/notify_route_change_spec.rb b/spec/integration/notify_route_change_spec.rb index ecf11c04..59d7a4d4 100644 --- a/spec/integration/notify_route_change_spec.rb +++ b/spec/integration/notify_route_change_spec.rb @@ -35,15 +35,55 @@ def postgres_listener(channel) expect(listener.pop).to eq("route_changes") end - it "sends a notification when content item updated" do + it "sends a notification when content item's routes are updated" do content_item = create(:content_item) listener = postgres_listener("route_changes") - content_item.update!(base_path: "/foo") + content_item.update!( + routes: [{ "path" => content_item.base_path, "type" => "prefix" }], + ) expect(listener.pop).to eq("route_changes") end + it "sends a notification when content item's redirects are updated" do + content_item = create(:content_item) + + listener = postgres_listener("route_changes") + content_item.update!( + redirects: [{ "path" => content_item.base_path, "type" => "exact", "destination" => "/new" }], + ) + + expect(listener.pop).to eq("route_changes") + end + + it "sends a notification when content item's schema name is updated" do + content_item = create(:content_item) + + listener = postgres_listener("route_changes") + content_item.update!(schema_name: "gone") + + expect(listener.pop).to eq("route_changes") + end + + it "sends a notification when content item's rendering_app is updated" do + content_item = create(:content_item) + + listener = postgres_listener("route_changes") + content_item.update!(rendering_app: "new-frontend") + + expect(listener.pop).to eq("route_changes") + end + + it "does not send a notification when content item's details are updated" do + content_item = create(:content_item) + + listener = postgres_listener("route_changes") + content_item.update!(details: { "body" => "change" }) + + expect(listener).to be_empty + end + it "sends a notification when content item destroyed" do content_item = create(:content_item) listener = postgres_listener("route_changes") @@ -62,7 +102,7 @@ def postgres_listener(channel) it "sends a notification when publish intent updated" do publish_intent = create(:publish_intent) listener = postgres_listener("route_changes") - publish_intent.update!(publish_time: 10.minutes.from_now) + publish_intent.update!(rendering_app: "updated") expect(listener.pop).to eq("route_changes") end