From 7a6b33c60d06b242c4b03a5753f5b373c3d9ca31 Mon Sep 17 00:00:00 2001
From: Christian Sutter <git@doublesignal.com>
Date: Mon, 11 Dec 2023 10:18:29 +0000
Subject: [PATCH] Refactor filters further

- Tweak filter logic to be more readable
- Add escaping for characters that need escaping in Discovery Engine
  filter syntax
---
 .../discovery_engine/query/filters.rb         | 37 +++++++++++++------
 .../discovery_engine/query/filters_spec.rb    |  6 +++
 2 files changed, 31 insertions(+), 12 deletions(-)

diff --git a/app/services/discovery_engine/query/filters.rb b/app/services/discovery_engine/query/filters.rb
index 4763cf0..a91b6a4 100644
--- a/app/services/discovery_engine/query/filters.rb
+++ b/app/services/discovery_engine/query/filters.rb
@@ -5,10 +5,16 @@ def initialize(query_params)
     end
 
     def filter_expression
-      surround_and_join([
+      expressions = [
         reject_links_filter,
         content_purpose_supergroup_filter,
-      ], between: " AND ", surround: "(", surround_end: ")")
+      ]
+
+      expressions
+        .compact
+        .map { surround(_1, delimiter: "(", delimiter_end: ")") }
+        .join(" AND ")
+        .presence
     end
 
   private
@@ -18,25 +24,32 @@ def filter_expression
     def reject_links_filter
       return nil if query_params[:reject_link].blank?
 
-      values = surround_and_join(query_params[:reject_link], between: ",", surround: '"')
+      values = Array(query_params[:reject_link])
+        .map { filter_string_value(_1) }
+        .join(",")
+
       "NOT link: ANY(#{values})"
     end
 
     def content_purpose_supergroup_filter
       return nil if query_params[:filter_content_purpose_supergroup].blank?
 
-      values = surround_and_join(
-        query_params[:filter_content_purpose_supergroup], between: ",", surround: '"'
-      )
+      values = Array(query_params[:filter_content_purpose_supergroup])
+        .map { filter_string_value(_1) }
+        .join(",")
+
       "content_purpose_supergroup: ANY(#{values})"
     end
 
-    def surround_and_join(string_or_strings, between:, surround:, surround_end: surround)
-      Array(string_or_strings)
-        .compact
-        .map { "#{surround}#{_1}#{surround_end}" }
-        .join(between)
-        .presence
+    # Input strings need to be wrapped in double quotes and have double quotes or backslashes
+    # escaped for Discovery Engine's filter syntax
+    def filter_string_value(str)
+      escaped_str = str.gsub(/(["\\])/, '\\\\\1')
+      surround(escaped_str, delimiter: '"')
+    end
+
+    def surround(str, delimiter:, delimiter_end: delimiter)
+      "#{delimiter}#{str}#{delimiter_end}"
     end
   end
 end
diff --git a/spec/services/discovery_engine/query/filters_spec.rb b/spec/services/discovery_engine/query/filters_spec.rb
index 99c543b..68d1b39 100644
--- a/spec/services/discovery_engine/query/filters_spec.rb
+++ b/spec/services/discovery_engine/query/filters_spec.rb
@@ -53,5 +53,11 @@
 
       it { is_expected.to eq('(NOT link: ANY("/foo")) AND (content_purpose_supergroup: ANY("services"))') }
     end
+
+    context "with filters containing escapable characters" do
+      let(:query_params) { { q: "garden centres", filter_content_purpose_supergroup: "foo\"\\bar" } }
+
+      it { is_expected.to eq('(content_purpose_supergroup: ANY("foo\\"\\\\bar"))') }
+    end
   end
 end