Skip to content

Commit

Permalink
feat(index): optimise server side rendered page
Browse files Browse the repository at this point in the history
  • Loading branch information
bethesque committed Nov 6, 2019
1 parent 979dc6b commit 1f54ccf
Show file tree
Hide file tree
Showing 14 changed files with 210 additions and 10 deletions.
10 changes: 10 additions & 0 deletions db/ddl_statements/head_pact_tags.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
def head_pact_tags_v1(connection)
connection.from(Sequel.as(:latest_pact_publication_ids_for_consumer_versions, :lp))
.join(:versions,{ Sequel[:lp][:consumer_version_id] => Sequel[:cv][:id]}, { table_alias: :cv })
.join(:latest_tagged_pact_consumer_version_orders, {
Sequel[:lp][:consumer_id] => Sequel[:o][:consumer_id],
Sequel[:lp][:provider_id] => Sequel[:o][:provider_id],
Sequel[:cv][:order] => Sequel[:o][:latest_consumer_version_order]
}, { table_alias: :o})
.select(Sequel[:o][:tag_name].as(:name), Sequel[:lp][:pact_publication_id])
end
9 changes: 9 additions & 0 deletions db/migrations/20191101_create_head_pact_tags.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
Sequel.migration do
up do
create_view(:head_pact_tags, head_pact_tags_v1(self))
end

down do
drop_view(:head_pact_tags)
end
end
26 changes: 26 additions & 0 deletions lib/pact_broker/domain/index_item.rb
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,32 @@ def <=> other
provider_name <=> other.provider_name
end

# Add logic for ignoring case
def <=> other
comparisons = [
compare_name_asc(consumer_name, other.consumer_name),
compare_number_desc(consumer_version.order, other.consumer_version.order),
compare_number_desc(latest_pact.revision_number, other.latest_pact.revision_number),
compare_name_asc(provider_name, other.provider_name)
]

comparisons.find{|c| c != 0 } || 0
end

def compare_name_asc name1, name2
name1&.downcase <=> name2&.downcase
end

def compare_number_desc number1, number2
if number1 && number2
number2 <=> number1
elsif number1
1
else
-1
end
end

def to_s
"Pact between #{consumer_name} #{consumer_version_number} and #{provider_name} #{provider_version_number}"
end
Expand Down
2 changes: 2 additions & 0 deletions lib/pact_broker/domain/version.rb
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
require 'pact_broker/db'
require 'pact_broker/domain/order_versions'
require 'pact_broker/repositories/helpers'
require 'pact_broker/tags/tag_with_latest_flag'

module PactBroker
module Domain
Expand All @@ -9,6 +10,7 @@ class Version < Sequel::Model
one_to_many :pact_publications, order: :revision_number, class: "PactBroker::Pacts::PactPublication", key: :consumer_version_id
associate(:many_to_one, :pacticipant, :class => "PactBroker::Domain::Pacticipant", :key => :pacticipant_id, :primary_key => :id)
one_to_many :tags, :reciprocal => :version, order: :created_at
one_to_many :tags_with_latest_flag, class: "PactBroker::Tags::TagWithLatestFlag", key: :version_id, primary_key: :id

dataset_module do
include PactBroker::Repositories::Helpers
Expand Down
107 changes: 107 additions & 0 deletions lib/pact_broker/index/service.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,14 @@ class Service
# supporting both

def self.find_index_items options = {}
if options[:optimised]
find_index_items_optimised(options)
else
find_index_items_original(options)
end
end

def self.find_index_items_original options = {}
rows = PactBroker::Matrix::HeadRow
.select_all_qualified
.eager(:latest_triggered_webhooks)
Expand Down Expand Up @@ -56,6 +64,89 @@ def self.find_index_items options = {}
end
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_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_publications = PactBroker::Pacts::PactPublication
.where(id: pact_publication_ids)
.select_all_qualified
.eager(:consumer)
.eager(:provider)
.eager(:pact_version)
.eager(integration: [{latest_verification: :provider_version}, :latest_triggered_webhooks])
.eager(:consumer_version)
.eager(latest_verification: { provider_version: :tags_with_latest_flag })
.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) }

PactBroker::Domain::IndexItem.create(
pact_publication.consumer,
pact_publication.provider,
pact_publication.to_domain_lightweight,
is_overall_latest_for_integration,
latest_verification,
webhook ? [webhook]: [],
pact_publication.integration.latest_triggered_webhooks,
consumer_version_tags(pact_publication, options[:tags]),
options[:tags] && latest_verification ? latest_verification.provider_version.tags_with_latest_flag.select(&:latest?) : []
)
end.sort
end

# Worst. Code. Ever.
#
def self.latest_verification_for_pseudo_branch(pact_publication, is_overall_latest, latest_verifications_for_cv_tags, tags_option)
if tags_option == true
latest_verifications_for_cv_tags
.select{ | v | v.consumer_id == pact_publication.consumer_id && v.provider_id == pact_publication.provider_id && pact_publication.head_pact_tags.collect(&:name).include?(v.consumer_version_tag_name) }
.sort{ |v1, v2| v1.id <=> v2.id }.last || (is_overall_latest && pact_publication.integration.latest_verification)
elsif tags_option.is_a?(Array)
latest_verifications_for_cv_tags
.select{ | v | v.consumer_id == pact_publication.consumer_id && v.provider_id == pact_publication.provider_id && pact_publication.head_pact_tags.collect(&:name).include?(v.consumer_version_tag_name) && tags_option.include?(v.consumer_version_tag_name) }
.sort{ |v1, v2| v1.id <=> v2.id }.last || (is_overall_latest && pact_publication.integration.latest_verification)
else
pact_publication.integration.latest_verification
end
end

def self.consumer_version_tags(pact_publication, tags_option)
if tags_option == true
pact_publication.head_pact_tags.collect(&:name)
elsif tags_option.is_a?(Array)
pact_publication.head_pact_tags.collect(&:name) & tags_option
else
[]
end
end

def self.find_index_items_for_api(consumer_name: nil, provider_name: nil, **ignored)
rows = PactBroker::Matrix::HeadRow
.eager(:consumer_version_tags)
Expand Down Expand Up @@ -83,6 +174,22 @@ def self.find_index_items_for_api(consumer_name: nil, provider_name: nil, **igno
)
end
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)
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)
end

def self.db
PactBroker::Pacts::PactPublication.db
end
end
end
end
3 changes: 2 additions & 1 deletion lib/pact_broker/integrations/integration.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
require 'pact_broker/verifications/pseudo_branch_status'
require 'pact_broker/domain/verification'
require 'pact_broker/webhooks/latest_triggered_webhook'
require 'pact_broker/webhooks/webhook'

module PactBroker
module Integrations
Expand All @@ -17,7 +18,7 @@ class Integration < Sequel::Model
# This will only work when using eager loading. The keys are just blanked out to avoid errors.
# I don't understand how they work at all.
# It would be nice to do this declaratively.
many_to_many :webhooks, :left_key => [], left_primary_key: [], :eager_loader=>(proc do |eo_opts|
many_to_many :webhooks, class: "PactBroker::Webhooks::Webhook", :left_key => [], left_primary_key: [], :eager_loader=>(proc do |eo_opts|
eo_opts[:rows].each do |row|
row.associations[:webhooks] = []
end
Expand Down
2 changes: 2 additions & 0 deletions lib/pact_broker/matrix/head_row.rb
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,9 @@ class HeadRow < Row
# the query down.
# This relation relies on consumer_version_tags already being loaded
one_to_one :latest_verification_for_consumer_version_tag, :class => "PactBroker::Verifications::LatestVerificationForConsumerVersionTag", primary_keys: [], key: [], :eager_loader=>(proc do |eo_opts|
# create an index of provider_id/consumer_id/consumer_version_tag_name => row
tag_to_row = eo_opts[:rows].each_with_object({}) { | row, map | map[[row.provider_id, row.consumer_id, row.consumer_version_tag_name]] = row }
# Initialise the association with nil - not sure why?
eo_opts[:rows].each{|row| row.associations[:latest_verification_for_consumer_version_tag] = nil}

# Need the all then the each to ensure the eager loading works
Expand Down
31 changes: 27 additions & 4 deletions lib/pact_broker/pacts/pact_publication.rb
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
require 'pact_broker/domain/pact'
require 'pact_broker/pacts/pact_version'
require 'pact_broker/repositories/helpers'
require 'pact_broker/integrations/integration'
require 'pact_broker/tags/head_pact_tags'

module PactBroker
module Pacts
Expand All @@ -16,6 +18,9 @@ class PactPublication < Sequel::Model(:pact_publications)
associate(:many_to_one, :consumer, :class => "PactBroker::Domain::Pacticipant", :key => :consumer_id, :primary_key => :id)
associate(:many_to_one, :consumer_version, :class => "PactBroker::Domain::Version", :key => :consumer_version_id, :primary_key => :id)
associate(:many_to_one, :pact_version, class: "PactBroker::Pacts::PactVersion", :key => :pact_version_id, :primary_key => :id)
associate(:many_to_one, :integration, class: "PactBroker::Integrations::Integration", key: [:consumer_id, :provider_id], primary_key: [:consumer_id, :provider_id])
one_to_one(:latest_verification, class: "PactBroker::Verifications::LatestVerificationForPactVersion", key: :pact_version_id, primary_key: :pact_version_id)
one_to_many(:head_pact_tags, class: "PactBroker::Tags::HeadPactTag", primary_key: :id, key: :pact_publication_id)

dataset_module do
include PactBroker::Repositories::Helpers
Expand Down Expand Up @@ -49,21 +54,39 @@ def to_domain
PactBroker::Domain::Pact.new(
id: id,
provider: provider,
consumer: consumer_version.pacticipant,
consumer: consumer,
consumer_version_number: consumer_version.number,
consumer_version: to_version_domain,
revision_number: revision_number,
json_content: pact_version.content,
pact_version_sha: pact_version.sha,
latest_verification: latest_verification,
latest_verification: nil,
created_at: created_at,
head_tag_names: [],
db_model: self
)
end

def to_domain_lightweight
PactBroker::Domain::Pact.new(
id: id,
provider: provider,
consumer: consumer,
consumer_version_number: consumer_version.number,
consumer_version: to_version_domain_lightweight,
revision_number: revision_number,
pact_version_sha: pact_version.sha,
created_at: created_at,
head_tag_names: head_tag_names,
db_model: self
)
end

def to_version_domain
OpenStruct.new(number: consumer_version.number, pacticipant: consumer_version.pacticipant, tags: consumer_version.tags, order: consumer_version.order)
OpenStruct.new(number: consumer_version.number, pacticipant: consumer, tags: consumer_version.tags, order: consumer_version.order)
end

def to_version_domain_lightweight
OpenStruct.new(number: consumer_version.number, pacticipant: consumer, order: consumer_version.order)
end

def upsert
Expand Down
12 changes: 12 additions & 0 deletions lib/pact_broker/tags/head_pact_tags.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
require 'pact_broker/db'
require 'pact_broker/repositories/helpers'

module PactBroker
module Tags
class HeadPactTag < Sequel::Model
dataset_module do
include PactBroker::Repositories::Helpers
end
end
end
end
1 change: 0 additions & 1 deletion lib/pact_broker/tags/tag_with_latest_flag.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ module PactBroker
module Tags
# The tag associated with the latest verification for a given tag
class TagWithLatestFlag < Sequel::Model(:tags_with_latest_flag)

dataset_module do
include PactBroker::Repositories::Helpers
end
Expand Down
4 changes: 3 additions & 1 deletion lib/pact_broker/ui/controllers/index.rb
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,9 @@ class Index < Base
if params[:tags]
tags = params[:tags] == 'true' ? true : [*params[:tags]].compact
end
view_model = ViewDomain::IndexItems.new(index_service.find_index_items(tags: tags))
options = { tags: tags }
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'
haml page, {locals: {index_items: view_model, title: "Pacts"}, layout: :'layouts/main'}
end
Expand Down
4 changes: 2 additions & 2 deletions lib/pact_broker/webhooks/webhook.rb
Original file line number Diff line number Diff line change
Expand Up @@ -68,8 +68,8 @@ def parsed_body
end
end

def is_for? relationship
(consumer_id == relationship.consumer_id || !consumer_id) && (provider_id == relationship.provider_id || !provider_id)
def is_for? integration
(consumer_id == integration.consumer_id || !consumer_id) && (provider_id == integration.provider_id || !provider_id)
end

private
Expand Down
7 changes: 6 additions & 1 deletion spec/lib/pact_broker/index/service_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,13 @@ module Index
describe Service do
let(:td) { TestDataBuilder.new }
let(:tags) { ['prod', 'production'] }
let(:options) { { tags: tags } }
let(:options) { { tags: tags, optimised: optimised } }
let(:rows) { subject.find_index_items(options) }
let(:optimised) { true }

before do
td.create_global_webhook
end

subject{ Service }

Expand Down
2 changes: 2 additions & 0 deletions spec/lib/pact_broker/pacts/pact_publication_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -99,12 +99,14 @@ module Pacts
context "when the pact is the latest for a tag" do
it "returns the relevant tag names" do
expect(pact_publication.head_tag_names).to eq ["yes"]
expect(pact_publication.head_pact_tags.collect(&:name)).to eq ["yes"]
end
end

context "when the pact is not the latest for a tag" do
it "returns the relevant tag names" do
expect(pact_publication.head_tag_names).to eq ["yes"]
expect(pact_publication.head_pact_tags.collect(&:name)).to eq ["yes"]
end
end
end
Expand Down

0 comments on commit 1f54ccf

Please sign in to comment.