make type conversions more explicit, add unit test for edgecase of co…

…mparing nil and empty array

lib/shopify_api/webhooks/registration.rb

@@ -48,8 +48,28 @@ def mutation_name(webhook_id); end
       sig { abstract.returns(String) }
       def build_check_query; end
 
-      sig { abstract.params(body: T::Hash[String, T.untyped]).returns(T::Hash[Symbol, T.untyped]) }
-      def parse_check_result(body); end
+      sig do
+        params(body: T::Hash[String, T.untyped]).returns({
+          webhook_id: T.nilable(String),
+          current_address: T.nilable(String),
+          fields: T::Array[String],
+          metafield_namespaces: T::Array[String],
+        })
+      end
+      def parse_check_result(body)
+        edges = body.dig("data", "webhookSubscriptions", "edges") || {}
+        webhook_id = nil
+        fields = []
+        metafield_namespaces = []
+        unless edges.empty?
+          node = edges[0]["node"]
+          webhook_id = node["id"].to_s
+          fields = node["includeFields"] || []
+          metafield_namespaces = node["metafieldNamespaces"] || []
+        end
+        { webhook_id: webhook_id, current_address: nil, fields: fields,
+          metafield_namespaces: metafield_namespaces, }
+      end
 
       sig { params(webhook_id: T.nilable(String)).returns(String) }
       def build_register_query(webhook_id: nil)

lib/shopify_api/webhooks/registrations/event_bridge.rb

@@ -45,22 +45,22 @@ def build_check_query
           QUERY
         end
 
-        sig { override.params(body: T::Hash[String, T.untyped]).returns(T::Hash[Symbol, T.untyped]) }
+        sig do
+          params(body: T::Hash[String, T.untyped]).returns({
+            webhook_id: T.nilable(String),
+            current_address: T.nilable(String),
+            fields: T::Array[String],
+            metafield_namespaces: T::Array[String],
+          })
+        end
         def parse_check_result(body)
+          parse_results = super(body)
           edges = body.dig("data", "webhookSubscriptions", "edges") || {}
-          webhook_id = nil
-          current_address = nil
-          fields = nil
-          metafield_namespaces = nil
           unless edges.empty?
             node = edges[0]["node"]
-            webhook_id = node["id"].to_s
-            current_address = node["endpoint"]["arn"].to_s
-            fields = node["includeFields"]
-            metafield_namespaces = node["metafieldNamespaces"]
+            parse_results[:current_address] = node["endpoint"]["arn"].to_s
           end
-          { webhook_id: webhook_id, current_address: current_address, fields: fields,
-            metafield_namespaces: metafield_namespaces, }
+          parse_results
         end
       end
     end

lib/shopify_api/webhooks/registrations/http.rb

@@ -51,27 +51,27 @@ def build_check_query
           QUERY
         end
 
-        sig { override.params(body: T::Hash[String, T.untyped]).returns(T::Hash[Symbol, T.untyped]) }
+        sig do
+          params(body: T::Hash[String, T.untyped]).returns({
+            webhook_id: T.nilable(String),
+            current_address: T.nilable(String),
+            fields: T::Array[String],
+            metafield_namespaces: T::Array[String],
+          })
+        end
         def parse_check_result(body)
+          parse_results = super(body)
           edges = body.dig("data", "webhookSubscriptions", "edges") || {}
-          webhook_id = nil
-          current_address = nil
-          fields = nil
-          metafield_namespaces = nil
           unless edges.empty?
             node = edges[0]["node"]
-            webhook_id = node["id"].to_s
-            current_address =
+            parse_results[:current_address] =
               if node.key?("endpoint")
                 node["endpoint"]["callbackUrl"].to_s
               else
                 node["callbackUrl"].to_s
               end
-            fields = node["includeFields"]
-            metafield_namespaces = node["metafieldNamespaces"]
           end
-          { webhook_id: webhook_id, current_address: current_address, fields: fields,
-            metafield_namespaces: metafield_namespaces, }
+          parse_results
         end
       end
     end

lib/shopify_api/webhooks/registrations/pub_sub.rb

@@ -50,22 +50,23 @@ def build_check_query
           QUERY
         end
 
-        sig { override.params(body: T::Hash[String, T.untyped]).returns(T::Hash[Symbol, T.untyped]) }
+        sig do
+          params(body: T::Hash[String, T.untyped]).returns({
+            webhook_id: T.nilable(String),
+            current_address: T.nilable(String),
+            fields: T::Array[String],
+            metafield_namespaces: T::Array[String],
+          })
+        end
         def parse_check_result(body)
+          parse_results = super(body)
           edges = body.dig("data", "webhookSubscriptions", "edges") || {}
-          webhook_id = nil
-          current_address = nil
-          fields = nil
-          metafield_namespaces = nil
           unless edges.empty?
             node = edges[0]["node"]
-            webhook_id = node["id"].to_s
-            current_address = "pubsub://#{node["endpoint"]["pubSubProject"]}:#{node["endpoint"]["pubSubTopic"]}"
-            fields = node["includeFields"]
-            metafield_namespaces = node["metafieldNamespaces"]
+            parse_results[:current_address] =
+              "pubsub://#{node["endpoint"]["pubSubProject"]}:#{node["endpoint"]["pubSubTopic"]}"
           end
-          { webhook_id: webhook_id, current_address: current_address, fields: fields,
-            metafield_namespaces: metafield_namespaces, }
+          parse_results
         end
       end
     end

lib/shopify_api/webhooks/registry.rb

@@ -219,10 +219,14 @@ def webhook_registration_needed?(client, registration)
           check_response = client.query(query: registration.build_check_query, response_as_struct: false)
           raise Errors::WebhookRegistrationError,
             "Failed to check if webhook was already registered" unless check_response.ok?
+
           parsed_check_result = registration.parse_check_result(T.cast(check_response.body, T::Hash[String, T.untyped]))
+          registration_fields = registration.fields || []
+          registration_metafield_namespaces = registration.metafield_namespaces || []
+
           must_register = parsed_check_result[:current_address] != registration.callback_address ||
-            parsed_check_result[:fields]&.sort != registration.fields&.sort ||
-            parsed_check_result[:metafield_namespaces]&.sort != registration.metafield_namespaces&.sort
+            parsed_check_result[:fields].sort != registration_fields.sort ||
+            parsed_check_result[:metafield_namespaces].sort != registration_metafield_namespaces.sort
 
           { webhook_id: parsed_check_result[:webhook_id], must_register: must_register }
         end

test/webhooks/registry_test.rb

@@ -133,6 +133,12 @@ def test_http_registraion_is_not_needed_if_identical_webhook_exists
           fields: "field1, field2",
           metafield_namespaces: ["namespace1", "namespace2"],
         )
+
+        do_no_registration_needed_test(
+          queries[:http][:check_existing_response],
+          :http,
+          "test-webhooks",
+        )
       end
 
       def test_http_registration_add_and_update
@@ -273,6 +279,12 @@ def test_pubsub_registration_is_not_needed_if_identical_webhook_exists
           fields: "field1, field2",
           metafield_namespaces: ["namespace1", "namespace2"],
         )
+
+        do_no_registration_needed_test(
+          queries[:pub_sub][:check_existing_response],
+          :pub_sub,
+          "pubsub://my-project-id:my-topic-id",
+        )
       end
 
       def test_pubsub_registration_add_and_update
@@ -373,6 +385,12 @@ def test_eventbridge_registration_is_not_needed_if_identical_webhook_exists
           fields: "field1, field2",
           metafield_namespaces: ["namespace1", "namespace2"],
         )
+
+        do_no_registration_needed_test(
+          queries[:event_bridge][:check_existing_response],
+          :event_bridge,
+          "test-webhooks",
+        )
       end
 
       def test_eventbridge_registration_add_and_update