diff --git a/DEVELOPER_DOCUMENTATION.md b/DEVELOPER_DOCUMENTATION.md index 750801e3f..c71623161 100644 --- a/DEVELOPER_DOCUMENTATION.md +++ b/DEVELOPER_DOCUMENTATION.md @@ -106,8 +106,19 @@ pact publication resource will be created, but it will reuse the existing pact v -> versions -> latest_tagged_pact_consumer_version_orders -> latest_pact_publications_by_consumer_versions + += head_pact_publications + -> latest_pact_publications + -> latest_pact_publication_ids_for_consumer_versions + -> latest_tagged_pact_publications + -> latest_pact_publications_by_consumer_versions (optimised for pp_ids) + -> latest_tagged_pact_consumer_version_orders (optimised for pp_ids) + ``` + + + ### Useful to know stuff * The supported database types are Postgres (recommended), MySQL (sigh) and Sqlite (just for testing, not recommended for production). Check the travis.yml file for the supported database versions. diff --git a/lib/pact_broker/domain/index_item.rb b/lib/pact_broker/domain/index_item.rb index f9b39e5c8..08aa81552 100644 --- a/lib/pact_broker/domain/index_item.rb +++ b/lib/pact_broker/domain/index_item.rb @@ -60,6 +60,10 @@ def consumer_version_number @latest_pact.consumer_version_number end + def consumer_version_order + consumer_version.order + end + def consumer_version @latest_pact.consumer_version end diff --git a/lib/pact_broker/index/service.rb b/lib/pact_broker/index/service.rb index ec0a34855..07bce256d 100644 --- a/lib/pact_broker/index/service.rb +++ b/lib/pact_broker/index/service.rb @@ -3,16 +3,24 @@ require 'pact_broker/domain/index_item' require 'pact_broker/matrix/head_row' require 'pact_broker/matrix/aggregated_row' +require 'pact_broker/repositories/helpers' module PactBroker - module Index class Service - extend PactBroker::Repositories extend PactBroker::Services extend PactBroker::Logging + COLS = [:id, :consumer_name, :provider_name, :consumer_version_order] + LATEST_PPS = Sequel::Model.db[:latest_pact_publications].select(*COLS) + LATEST_TAGGED_PPS = Sequel::Model.db[:latest_tagged_pact_publications].select(*COLS) + HEAD_PP_ORDER_COLUMNS = [ + Sequel.asc(Sequel.function(:lower, :consumer_name)), + Sequel.desc(:consumer_version_order), + Sequel.asc(Sequel.function(:lower, :provider_name)) + ].freeze + # This method provides data for both the OSS server side rendered index (with and without tags) # and the Pactflow UI. It really needs to be broken into to separate methods, as it's getting too messy # supporting both @@ -46,10 +54,7 @@ def self.find_index_items_original options = {} end rows = rows.all.group_by(&:pact_publication_id).values.collect{ | rows| Matrix::AggregatedRow.new(rows) } - - rows.sort.collect do | row | - # TODO simplify. Do we really need 3 layers of abstraction? PactBroker::Domain::IndexItem.create( row.consumer, row.provider, @@ -65,31 +70,14 @@ def self.find_index_items_original options = {} end def self.find_index_items_optimised options = {} - pact_publication_ids = nil - latest_verifications_for_cv_tags = nil - - if !options[:tags] - # server side rendered index page without tags - pact_publication_ids = latest_pact_publications.select(:id) - else - # server side rendered index page with tags=true or tags[]=a&tags=[]b - if options[:tags].is_a?(Array) - # TODO test for this - pact_publication_ids = head_pact_publications_ids_for_tags(options[:tags]) - latest_verifications_for_cv_tags = PactBroker::Verifications::LatestVerificationForConsumerVersionTag - .eager(:provider_version) - .where(consumer_version_tag_name: options[:tags]).all - else - pact_publication_ids = head_pact_publications_ids - latest_verifications_for_cv_tags = PactBroker::Verifications::LatestVerificationForConsumerVersionTag.eager(:provider_version).all - end - end - + latest_verifications_for_cv_tags = latest_verifications_for_consumer_version_tags(options) latest_pact_publication_ids = latest_pact_publications.select(:id).all.collect{ |h| h[:id] } # We only need to know if a webhook exists for an integration, not what its properties are webhooks = PactBroker::Webhooks::Webhook.select(:consumer_id, :provider_id).distinct.all + pact_publication_ids = head_pact_publication_ids(options) + pact_publications = PactBroker::Pacts::PactPublication .where(id: pact_publication_ids) .select_all_qualified @@ -102,7 +90,6 @@ def self.find_index_items_optimised options = {} .eager(:head_pact_tags) pact_publications.all.collect do | pact_publication | - is_overall_latest_for_integration = latest_pact_publication_ids.include?(pact_publication.id) latest_verification = latest_verification_for_pseudo_branch(pact_publication, is_overall_latest_for_integration, latest_verifications_for_cv_tags, options[:tags]) webhook = webhooks.find{ |webhook| webhook.is_for?(pact_publication.integration) } @@ -148,47 +135,83 @@ def self.consumer_version_tags(pact_publication, tags_option) end def self.find_index_items_for_api(consumer_name: nil, provider_name: nil, **ignored) - rows = PactBroker::Matrix::HeadRow - .eager(:consumer_version_tags) - .eager(:provider_version_tags) + latest_pact_publication_ids = latest_pact_publications.select(:id).all.collect{ |h| h[:id] } + pact_publication_ids = head_pact_publication_ids(consumer_name: consumer_name, provider_name: provider_name, tags: true) + + pact_publications = PactBroker::Pacts::PactPublication + .where(id: pact_publication_ids) .select_all_qualified + .eager(:consumer) + .eager(:provider) + .eager(:pact_version) + .eager(:consumer_version) + .eager(latest_verification: { provider_version: :tags_with_latest_flag }) + .eager(:head_pact_tags) - rows = rows.consumer(consumer_name) if consumer_name - rows = rows.provider(provider_name) if provider_name - rows = rows.all.group_by(&:pact_publication_id).values.collect{ | rows| Matrix::AggregatedRow.new(rows) } + pact_publications.all.collect do | pact_publication | + + is_overall_latest_for_integration = latest_pact_publication_ids.include?(pact_publication.id) - rows.sort.collect do | row | - # TODO separate this model from IndexItem - # webhook status not currently displayed in Pactflow UI, so don't query for it. PactBroker::Domain::IndexItem.create( - row.consumer, - row.provider, - row.pact, - row.overall_latest?, - row.latest_verification_for_pact_version, + pact_publication.consumer, + pact_publication.provider, + pact_publication.to_domain_lightweight, + is_overall_latest_for_integration, + pact_publication.latest_verification, [], [], - row.consumer_head_tag_names, - row.provider_version_tags.select(&:latest?) + pact_publication.head_pact_tags.collect(&:name), + pact_publication.latest_verification ? pact_publication.latest_verification.provider_version.tags_with_latest_flag.select(&:latest?) : [] ) - end + end.sort end def self.latest_pact_publications db[:latest_pact_publications] end - def self.head_pact_publications_ids - db[:head_pact_tags].select(Sequel[:pact_publication_id].as(:id)).union(db[:latest_pact_publications].select(:id)).limit(500) + def self.db + PactBroker::Pacts::PactPublication.db end - def self.head_pact_publications_ids_for_tags(tag_names) - db[:head_pact_tags].select(Sequel[:pact_publication_id].as(:id)).where(name: tag_names).union(db[:latest_pact_publications].select(:id)).limit(500) + def self.head_pact_publication_ids(options = {}) + query = if options[:tags].is_a?(Array) + LATEST_PPS.union(LATEST_TAGGED_PPS.where(tag_name: options[:tags])) + elsif options[:tags] + LATEST_PPS.union(LATEST_TAGGED_PPS) + else + LATEST_PPS + end + + if options[:consumer_name] + query = query.where(PactBroker::Repositories::Helpers.name_like(:consumer_name, options[:consumer_name])) + end + + if options[:provider_name] + query = query.where(PactBroker::Repositories::Helpers.name_like(:provider_name, options[:provider_name])) + end + + query.order(*HEAD_PP_ORDER_COLUMNS) + .limit(options[:limit] || 50) + .offset(options[:offset] || 0) + .select(:id) end - def self.db - PactBroker::Pacts::PactPublication.db + def self.latest_verifications_for_consumer_version_tags(options) + # server side rendered index page with tags[]=a&tags=[]b + if options[:tags].is_a?(Array) + PactBroker::Verifications::LatestVerificationForConsumerVersionTag + .eager(:provider_version) + .where(consumer_version_tag_name: options[:tags]) + .all + elsif options[:tags] # server side rendered index page with tags=true + PactBroker::Verifications::LatestVerificationForConsumerVersionTag + .eager(:provider_version) + .all + else + nil # should not be used + end end end end diff --git a/lib/pact_broker/ui/controllers/index.rb b/lib/pact_broker/ui/controllers/index.rb index 8553fbea4..94ca4202d 100644 --- a/lib/pact_broker/ui/controllers/index.rb +++ b/lib/pact_broker/ui/controllers/index.rb @@ -14,7 +14,7 @@ class Index < Base if params[:tags] tags = params[:tags] == 'true' ? true : [*params[:tags]].compact end - options = { tags: tags } + options = { tags: tags, limit: params[:limit]&.to_i, offset: params[:offset]&.to_i } options[:optimised] = true if params[:optimised] == 'true' view_model = ViewDomain::IndexItems.new(index_service.find_index_items(options)) page = tags ? :'index/show-with-tags' : :'index/show' diff --git a/lib/pact_broker/ui/view_models/index_item.rb b/lib/pact_broker/ui/view_models/index_item.rb index 24bed66a2..965ab231b 100644 --- a/lib/pact_broker/ui/view_models/index_item.rb +++ b/lib/pact_broker/ui/view_models/index_item.rb @@ -27,6 +27,10 @@ def consumer_version_number PactBroker::Versions::AbbreviateNumber.call(@relationship.consumer_version_number) end + def consumer_version_order + @relationship.consumer_version_order + end + def provider_version_number PactBroker::Versions::AbbreviateNumber.call(@relationship.provider_version_number) end @@ -164,7 +168,9 @@ def verification_tooltip def <=> other comp = consumer_name.downcase <=> other.consumer_name.downcase return comp unless comp == 0 - provider_name.downcase <=> other.provider_name.downcase + comp = provider_name.downcase <=> other.provider_name.downcase + return comp unless comp == 0 + other.consumer_version_order <=> consumer_version_order end def short_version_number version_number diff --git a/lib/pact_broker/ui/views/index/show-with-tags.haml b/lib/pact_broker/ui/views/index/show-with-tags.haml index c415a79bf..563f10146 100644 --- a/lib/pact_broker/ui/views/index/show-with-tags.haml +++ b/lib/pact_broker/ui/views/index/show-with-tags.haml @@ -38,7 +38,7 @@ %td.consumer %a{:href => index_item.consumer_group_url } = escape_html(index_item.consumer_name) - %td.consumer-version-number + %td.consumer-version-number{"data-text": index_item.consumer_version_order} %div.clippable = escape_html(index_item.consumer_version_number) - if index_item.consumer_version_number diff --git a/spec/lib/pact_broker/index/service_spec.rb b/spec/lib/pact_broker/index/service_spec.rb index c7ff5c4ee..3967c70f4 100644 --- a/spec/lib/pact_broker/index/service_spec.rb +++ b/spec/lib/pact_broker/index/service_spec.rb @@ -4,7 +4,6 @@ require 'pact_broker/domain/pact' module PactBroker - module Index describe Service do let(:td) { TestDataBuilder.new } diff --git a/spec/lib/pact_broker/ui/controllers/index_spec.rb b/spec/lib/pact_broker/ui/controllers/index_spec.rb index 45feb59e1..cfa37a7fb 100644 --- a/spec/lib/pact_broker/ui/controllers/index_spec.rb +++ b/spec/lib/pact_broker/ui/controllers/index_spec.rb @@ -1,7 +1,6 @@ -require 'spec_helper' -require 'pact_broker/ui/controllers/index' - require 'rack/test' +require 'pact_broker/ui/controllers/index' +require 'pact_broker/index/service' module PactBroker module UI @@ -37,7 +36,7 @@ module Controllers end it "passes tags: true to the IndexService" do - expect(PactBroker::Index::Service).to receive(:find_index_items).with(tags: true) + expect(PactBroker::Index::Service).to receive(:find_index_items).with(tags: true, limit: nil, offset: nil) get "/", { tags: 'true' } end end @@ -48,7 +47,7 @@ module Controllers end it "passes tags: ['prod'] to the IndexService" do - expect(PactBroker::Index::Service).to receive(:find_index_items).with(tags: ["prod"]) + expect(PactBroker::Index::Service).to receive(:find_index_items).with(tags: ["prod"], limit: nil, offset: nil) get "/", { tags: ["prod"] } end end @@ -59,7 +58,7 @@ module Controllers end it "passes tags: ['prod'] to the IndexService" do - expect(PactBroker::Index::Service).to receive(:find_index_items).with(tags: ["prod"]) + expect(PactBroker::Index::Service).to receive(:find_index_items).with(tags: ["prod"], limit: nil, offset: nil) get "/", { tags: "prod" } end end diff --git a/spec/lib/pact_broker/ui/view_models/index_items_spec.rb b/spec/lib/pact_broker/ui/view_models/index_items_spec.rb index 2cd778f83..4a4d8078f 100644 --- a/spec/lib/pact_broker/ui/view_models/index_items_spec.rb +++ b/spec/lib/pact_broker/ui/view_models/index_items_spec.rb @@ -6,10 +6,10 @@ module UI module ViewDomain describe IndexItems do - let(:relationship_model_4) { double("PactBroker::Domain::IndexItem", consumer_name: "A", provider_name: "X") } - let(:relationship_model_2) { double("PactBroker::Domain::IndexItem", consumer_name: "a", provider_name: "y") } - let(:relationship_model_3) { double("PactBroker::Domain::IndexItem", consumer_name: "A", provider_name: "Z") } - let(:relationship_model_1) { double("PactBroker::Domain::IndexItem", consumer_name: "C", provider_name: "A") } + let(:relationship_model_4) { double("PactBroker::Domain::IndexItem", consumer_name: "A", provider_name: "X", consumer_version_order: 1) } + let(:relationship_model_2) { double("PactBroker::Domain::IndexItem", consumer_name: "a", provider_name: "y", consumer_version_order: 2) } + let(:relationship_model_3) { double("PactBroker::Domain::IndexItem", consumer_name: "A", provider_name: "Z", consumer_version_order: 3) } + let(:relationship_model_1) { double("PactBroker::Domain::IndexItem", consumer_name: "C", provider_name: "A", consumer_version_order: 4) } subject { IndexItems.new([relationship_model_1, relationship_model_3, relationship_model_4, relationship_model_2]) }