fix: improve performance of network diagram (#614)

* fix: improve performance of network diagram

Also cap the number of pacticipants that are included in the network diagram for performance and readability reasons

PACT-1070

docs/CONFIGURATION.md

@@ -666,6 +666,16 @@ The maximum amount of time in seconds to attempt to generate the diff between tw
 **YAML configuration key name:** `pact_content_diff_timeout`<br/>
 **Default:** `15`<br/>
 
+### network_diagram_max_pacticipants
+
+The maximum number of pacticipants to include in the network diagram. When too many pacticipants are included, the diagram becomes unreadable,
+and at large numbers, the graph will not render due to browser performance issues.
+
+**Environment variable name:** `PACT_BROKER_NETWORK_DIAGRAM_MAX_PACTICIPANTS`<br/>
+**YAML configuration key name:** `network_diagram_max_pacticipants`<br/>
+**Default:** `150`<br/>
+**Allowed values:** A positive integer<br/>
+
 <br/>
 
 ## Miscellaneous

docs/configuration.yml

@@ -460,6 +460,12 @@ groups:
           The maximum amount of time in seconds to attempt to generate the diff between two pacts before aborting the request. This is required due to performance issues in the underlying diff generation code.
         default_value: 15
         supported_versions: From 2.99.0
+      network_diagram_max_pacticipants:
+        description: |-
+          The maximum number of pacticipants to include in the network diagram. When too many pacticipants are included, the diagram becomes unreadable,
+          and at large numbers, the graph will not render due to browser performance issues.
+        default_value: 150
+        allowed_values_description: A positive integer
   - title: Miscellaneous
     vars:
       features:

lib/pact_broker/api/resources/group.rb

@@ -1,10 +1,13 @@
+require "pact_broker/string_refinements"
 require "pact_broker/api/resources/base_resource"
 require "pact_broker/api/decorators/relationships_csv_decorator"
 
 module PactBroker
   module Api
     module Resources
       class Group < BaseResource
+        using PactBroker::StringRefinements
+
         def content_types_provided
           [["text/csv", :to_csv]]
         end
@@ -32,9 +35,15 @@ def policy_name
         private
 
         def group
-          @group ||= group_service.find_group_containing(pacticipant)
+          @group ||= group_service.find_group_containing(pacticipant, max_pacticipants: max_pacticipants)
+        end
+
+        def max_pacticipants
+          if request.query["maxPacticipants"]&.integer?
+            request.query["maxPacticipants"].to_i
+          end
         end
       end
     end
   end
-end
+end

lib/pact_broker/config/runtime_configuration.rb

@@ -93,6 +93,7 @@ class RuntimeConfiguration < Anyway::Config
         allow_dangerous_contract_modification: false,
         semver_formats: ["%M.%m.%p%s%d", "%M.%m", "%M"],
         seed_example_data: true,
+        network_diagram_max_pacticipants: 150,
         features: {}
       )
 
@@ -107,7 +108,10 @@ def self.getter_and_setter_method_names
         config_attributes + config_attributes.collect{ |k| "#{k}=".to_sym } + extra_methods  - [:base_url]
       end
 
-      coerce_types(features: COERCE_FEATURES)
+      coerce_types(
+        features: COERCE_FEATURES,
+        network_diagram_max_pacticipants: :integer
+      )
       sensitive_values(:database_url, :database_password)
 
       def log_level= log_level

lib/pact_broker/groups/service.rb

@@ -1,5 +1,5 @@
 require "pact_broker/repositories"
-require "pact_broker/relationships/groupify"
+require "pact_broker/domain/index_item"
 
 module PactBroker
   module Groups
@@ -8,13 +8,46 @@ module Service
       extend PactBroker::Repositories
       extend PactBroker::Services
 
-      def find_group_containing pacticipant
-        groups.find { | group | group.include_pacticipant? pacticipant }
+      # Returns a list of all the integrations (PactBroker::Domain::IndexItem) that are connected to the given pacticipant.
+      # @param pacticipant [PactBroker::Domain::Pacticipant] the pacticipant for which to return the connected pacticipants
+      # @option max_pacticipants [Integer] the maximum number of pacticipants to return, or nil for no maximum. 40 is about the most applications you can meaningfully show in the circle network diagram.
+      # @return [PactBroker::Domain::Group]
+      def find_group_containing(pacticipant, max_pacticipants: nil)
+        PactBroker::Domain::Group.new(build_index_items(integrations_connected_to(pacticipant, max_pacticipants)))
       end
 
-      def groups
-        Relationships::Groupify.call(index_service.find_all_index_items)
+      def integrations_connected_to(pacticipant, max_pacticipants)
+        PactBroker::Integrations::Integration
+          .eager(:consumer, :provider)
+          .where(id: ids_of_integrations_connected_to(pacticipant, max_pacticipants))
+          .all
       end
+      private_class_method :integrations_connected_to
+
+      def build_index_items(integrations)
+        integrations.collect do | integration |
+          PactBroker::Domain::IndexItem.new(integration.consumer, integration.provider)
+        end
+      end
+      private_class_method :build_index_items
+
+      def ids_of_integrations_connected_to(pacticipant, max_pacticipants)
+        integrations = []
+        connected_pacticipants = Set.new([pacticipant.id])
+        new_connected_pacticipants = Set.new([pacticipant.id])
+
+        loop do
+          new_integrations = PactBroker::Integrations::Integration.including_pacticipant_id(new_connected_pacticipants.to_a).exclude(id: integrations.collect(&:id)).all
+          integrations.concat(new_integrations)
+          pacticipant_ids_for_new_integrations = Set.new(new_integrations.flat_map(&:pacticipant_ids))
+          new_connected_pacticipants = pacticipant_ids_for_new_integrations - connected_pacticipants
+          connected_pacticipants.merge(pacticipant_ids_for_new_integrations)
+          break if new_connected_pacticipants.empty? || (max_pacticipants && connected_pacticipants.size >= max_pacticipants)
+        end
+
+        integrations.collect(&:id).uniq
+      end
+      private_class_method :ids_of_integrations_connected_to
     end
   end
 end

lib/pact_broker/integrations/integration.rb

@@ -75,6 +75,12 @@ class Integration < Sequel::Model(Sequel::Model.db[:integrations].select(:id, :c
         end
       end)
 
+      dataset_module do
+        def including_pacticipant_id(pacticipant_id)
+          where(consumer_id: pacticipant_id).or(provider_id: pacticipant_id)
+        end
+      end
+
       def self.compare_by_last_action_date a, b
         if b.latest_pact_or_verification_publication_date && a.latest_pact_or_verification_publication_date
           b.latest_pact_or_verification_publication_date <=> a.latest_pact_or_verification_publication_date
@@ -111,6 +117,10 @@ def consumer_name
       def provider_name
         provider.name
       end
+
+      def pacticipant_ids
+        [consumer_id, provider_id]
+      end
     end
   end
 end

lib/pact_broker/string_refinements.rb

@@ -37,6 +37,10 @@ def blank?
     end
 
     refine String do
+      def integer?
+        self =~ /^\d+$/
+      end
+
       def present?
         !blank?
       end

lib/pact_broker/ui/controllers/groups.rb

@@ -30,12 +30,13 @@ def locals(overrides)
           pacticipant = pacticipant_service.find_pacticipant_by_name(params[:name])
           {
             csv_path: "#{base_url}/groups/#{ERB::Util.url_encode(params[:name])}.csv",
+            max_pacticipants: PactBroker.configuration.network_diagram_max_pacticipants,
             pacticipant_name: params[:name],
             repository_url: pacticipant&.repository_url,
             base_url: base_url,
             pacticipant: pacticipant,
             details_url: "#{base_url}/pacticipants/#{ERB::Util.url_encode(params[:name])}",
-            network_url: "#{base_url}/pacticipants/#{ERB::Util.url_encode(params[:name])}/network"
+            network_url: "#{base_url}/pacticipants/#{ERB::Util.url_encode(params[:name])}/network?maxPacticipants=#{PactBroker.configuration.network_diagram_max_pacticipants}"
           }.merge(overrides)
         end
       end

lib/pact_broker/ui/views/groups/show.html.erb

@@ -329,8 +329,9 @@ window.onload = function() {
         .attr("d","M 10 0 L 10 10 L 0 5 z")
         .attr("fill", "#A0A0A0");
 
+    const maxPacticipants = new URL(location).searchParams.get("maxPacticipants") || <%= max_pacticipants %>;
 
-    d3.text("<%= csv_path %>", "text/csv", function(unparsedData) {
+    d3.text(`<%= csv_path %>?maxPacticipants=${maxPacticipants}`, "text/csv", function(unparsedData) {
         var data=d3.csv.parseRows(unparsedData);
         pacticipants = parseCSV(data);
         pacticipantNameArray = getPacticipantNames(pacticipants);

spec/lib/pact_broker/api/resources/group_spec.rb

@@ -23,7 +23,7 @@ module Resources
           allow(decorator).to receive(:to_csv).and_return(csv)
         end
 
-        subject { get "/groups/Some%20Service", "", {"HTTP_X_My_App_Version" => "2"} }
+        subject { get "/groups/Some%20Service", "", { "HTTP_X_My_App_Version" => "2" } }
 
         context "when the pacticipant exists" do
 
@@ -33,7 +33,7 @@ module Resources
           end
 
           it "finds the group containing the pacticipant" do
-            expect(PactBroker::Groups::Service).to receive(:find_group_containing).with(pacticipant)
+            expect(PactBroker::Groups::Service).to receive(:find_group_containing).with(pacticipant, max_pacticipants: nil)
             subject
           end
 
@@ -56,6 +56,15 @@ module Resources
             subject
             expect(last_response.body).to eq csv
           end
+
+          context "when maxPacticipants is specified" do
+            subject { get "/groups/Some%20Service", { "maxPacticipants" => "30" }, { "HTTP_X_My_App_Version" => "2" } }
+
+            it "finds the group containing the pacticipant" do
+              expect(PactBroker::Groups::Service).to receive(:find_group_containing).with(pacticipant, max_pacticipants: 30)
+              subject
+            end
+          end
         end
 
         context "when the pacticipant does not exist" do

spec/lib/pact_broker/groups/service_spec.rb

@@ -1,52 +1,57 @@
-require "spec_helper"
 require "pact_broker/groups/service"
-require "pact_broker/index/service"
 
 module PactBroker
-
   module Groups
     describe Service do
-
       describe "#find_group_containing" do
+        before do
+          td.create_consumer("app a")
+            .create_provider("app x")
+            .create_integration
+            .create_consumer("app b")
+            .create_provider("app y")
+            .create_integration
+            .use_consumer("app y")
+            .create_provider("app z")
+            .create_integration
+            .use_consumer("app z")
+            .use_provider("app y")
+            .create_integration
+        end
 
-        let(:consumer_a) { double("consumer a", name: "consumer a", id: 1)}
-        let(:consumer_b) { double("consumer b", name: "consumer b", id: 2)}
+        let(:app_a) { td.find_pacticipant("app a") }
+        let(:app_b) { td.find_pacticipant("app b") }
 
-        let(:provider_x) { double("provider x", name: "provider x", id: 3)}
-        let(:provider_y) { double("provider y", name: "provider y", id: 4)}
+        let(:app_x) { td.find_pacticipant("app x") }
+        let(:app_y) { td.find_pacticipant("app y") }
+        let(:app_z) { td.find_pacticipant("app z") }
 
-        let(:relationship_1) { Domain::IndexItem.new(consumer_a, provider_x) }
-        let(:relationship_2) { Domain::IndexItem.new(consumer_b, provider_y) }
+        let(:relationship_1) { Domain::IndexItem.new(app_a, app_x) }
+        let(:relationship_2) { Domain::IndexItem.new(app_b, app_y) }
+        let(:relationship_3) { Domain::IndexItem.new(app_y, app_z) }
+        let(:relationship_3) { Domain::IndexItem.new(app_z, app_y) }
 
         let(:group_1) { Domain::Group.new(relationship_1) }
-        let(:group_2) { Domain::Group.new(relationship_2) }
+        let(:group_2) { Domain::Group.new(relationship_2, relationship_3) }
 
-        let(:relationship_list) { double("relationship list") }
-        let(:groups) { [group_1, group_2]}
+        subject  { Service.find_group_containing(app_b) }
 
-        subject  { Service.find_group_containing(consumer_b) }
-
-        before do
-          allow(PactBroker::Index::Service).to receive(:find_index_items).and_return(relationship_list)
-          allow(Relationships::Groupify).to receive(:call).and_return(groups)
-        end
-
-        it "retrieves a list of the relationships" do
-          allow(Index::Service).to receive(:find_index_items)
-          subject
+        it "returns the Group containing the given pacticipant" do
+          expect(subject.size).to eq 3
+          expect(subject).to include(have_attributes(consumer_name: "app b", provider_name: "app y"))
+          expect(subject).to include(have_attributes(consumer_name: "app y", provider_name: "app z"))
+          expect(subject).to include(have_attributes(consumer_name: "app z", provider_name: "app y"))
         end
 
-        it "turns the relationships into groups" do
-          expect(Relationships::Groupify).to receive(:call).with(relationship_list)
-          subject
-        end
+        context "when a max_pacticipants is specified" do
+          subject  { Service.find_group_containing(app_b, max_pacticipants: 2) }
 
-        it "returns the Group containing the given pacticipant" do
-          expect(subject).to be group_2
+          it "stops searching before reaching the end of the group" do
+            expect(subject.size).to eq 1
+            expect(subject).to include(have_attributes(consumer_name: "app b", provider_name: "app y"))
+          end
         end
-
       end
-
     end
   end
 end