diff --git a/docs/CONFIGURATION.md b/docs/CONFIGURATION.md index c8c2a023c..741546a3d 100644 --- a/docs/CONFIGURATION.md +++ b/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`
**Default:** `15`
+### 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`
+**YAML configuration key name:** `network_diagram_max_pacticipants`
+**Default:** `150`
+**Allowed values:** A positive integer
+
## Miscellaneous diff --git a/docs/configuration.yml b/docs/configuration.yml index 130135bce..622c02ab2 100644 --- a/docs/configuration.yml +++ b/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: diff --git a/lib/pact_broker/api/resources/group.rb b/lib/pact_broker/api/resources/group.rb index b4dcd61de..d45a99c71 100644 --- a/lib/pact_broker/api/resources/group.rb +++ b/lib/pact_broker/api/resources/group.rb @@ -1,3 +1,4 @@ +require "pact_broker/string_refinements" require "pact_broker/api/resources/base_resource" require "pact_broker/api/decorators/relationships_csv_decorator" @@ -5,6 +6,8 @@ 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 \ No newline at end of file +end diff --git a/lib/pact_broker/config/runtime_configuration.rb b/lib/pact_broker/config/runtime_configuration.rb index 0f4cbfadd..f28d3365e 100644 --- a/lib/pact_broker/config/runtime_configuration.rb +++ b/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 diff --git a/lib/pact_broker/groups/service.rb b/lib/pact_broker/groups/service.rb index f033382fd..92aed6db8 100644 --- a/lib/pact_broker/groups/service.rb +++ b/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 diff --git a/lib/pact_broker/integrations/integration.rb b/lib/pact_broker/integrations/integration.rb index 37086009c..9e22e8f2e 100644 --- a/lib/pact_broker/integrations/integration.rb +++ b/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 diff --git a/lib/pact_broker/relationships/groupify.rb b/lib/pact_broker/relationships/groupify.rb deleted file mode 100644 index 0f2257698..000000000 --- a/lib/pact_broker/relationships/groupify.rb +++ /dev/null @@ -1,45 +0,0 @@ -require "pact_broker/domain/group" - -=begin - Splits all index_items up into groups of non-connecting index_items. -=end - -module PactBroker - - module Relationships - - class Groupify - - def self.call index_items - recurse_groups([], index_items.dup).collect { |group| Domain::Group.new(group) } - end - - def self.recurse_groups groups, index_item_pool - if index_item_pool.empty? - groups - else - first, *rest = index_item_pool - group = [first] - new_connections = true - while new_connections - new_connections = false - group = rest.inject(group) do |connected, candidate| - if connected.select { |index_item| index_item.connected?(candidate) }.any? - new_connections = true - connected + [candidate] - else - connected - end - end - - rest = rest - group - group.uniq - end - - recurse_groups(groups + [group], index_item_pool - group) - end - end - end - - end -end diff --git a/lib/pact_broker/string_refinements.rb b/lib/pact_broker/string_refinements.rb index 1fa549c46..cd27c81c8 100644 --- a/lib/pact_broker/string_refinements.rb +++ b/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 diff --git a/lib/pact_broker/ui/controllers/groups.rb b/lib/pact_broker/ui/controllers/groups.rb index 6675d231f..13634fcef 100644 --- a/lib/pact_broker/ui/controllers/groups.rb +++ b/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 diff --git a/lib/pact_broker/ui/views/groups/show.html.erb b/lib/pact_broker/ui/views/groups/show.html.erb index bf8729cad..51964d70b 100644 --- a/lib/pact_broker/ui/views/groups/show.html.erb +++ b/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); diff --git a/spec/lib/pact_broker/api/resources/group_spec.rb b/spec/lib/pact_broker/api/resources/group_spec.rb index 743f2d99c..ac429fe74 100644 --- a/spec/lib/pact_broker/api/resources/group_spec.rb +++ b/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 diff --git a/spec/lib/pact_broker/groups/service_spec.rb b/spec/lib/pact_broker/groups/service_spec.rb index b3bd10aa0..d14b6c097 100644 --- a/spec/lib/pact_broker/groups/service_spec.rb +++ b/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 \ No newline at end of file diff --git a/spec/lib/pact_broker/relationships/groupify_spec.rb b/spec/lib/pact_broker/relationships/groupify_spec.rb deleted file mode 100644 index 83450e87c..000000000 --- a/spec/lib/pact_broker/relationships/groupify_spec.rb +++ /dev/null @@ -1,43 +0,0 @@ -require "spec_helper" -require "pact_broker/relationships/groupify" -require "pact_broker/domain/index_item" - -module PactBroker - module Relationships - describe Groupify do - describe ".call" do - - let(:consumer_a) { double("consumer a", id: 1, name: "consumer a") } - let(:consumer_b) { double("consumer b", id: 2, name: "consumer b") } - let(:consumer_c) { double("consumer c", id: 3, name: "consumer c") } - - let(:consumer_l) { double("consumer l", id: 4, name: "consumer l") } - let(:consumer_m) { double("consumer m", id: 5, name: "consumer m") } - - let(:provider_p) { double("provider p", id: 6, name: "provider p") } - - let(:provider_x) { double("provider x", id: 7, name: "provider x") } - let(:provider_y) { double("provider y", id: 8, name: "provider y") } - let(:provider_z) { double("provider z", id: 9, name: "provider z") } - - let(:relationship_1) { Domain::IndexItem.new(consumer_a, provider_x) } - let(:relationship_4) { Domain::IndexItem.new(consumer_a, provider_y) } - let(:relationship_2) { Domain::IndexItem.new(consumer_b, provider_y) } - - let(:relationship_3) { Domain::IndexItem.new(consumer_c, provider_z) } - - let(:relationship_5) { Domain::IndexItem.new(consumer_l, provider_p) } - let(:relationship_6) { Domain::IndexItem.new(consumer_m, provider_p) } - - let(:relationships) { [relationship_1, relationship_2, relationship_3, relationship_4, relationship_5, relationship_6] } - - it "separates the relationships into isolated groups" do - groups = Groupify.call(relationships) - expect(groups[0]).to eq(Domain::Group.new(relationship_1, relationship_4, relationship_2)) - expect(groups[1]).to eq(Domain::Group.new(relationship_3)) - expect(groups[2]).to eq(Domain::Group.new(relationship_5, relationship_6)) - end - end - end - end -end