Skip to content

Commit 6c11cbe

Browse files
committed
fix: ensure matrix is updated when pacticipant is deleted
1 parent 34afb4d commit 6c11cbe

15 files changed

+251
-51
lines changed

lib/pact_broker/api.rb

+1-1
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ module PactBroker
4141
# Pacticipants
4242
add ['pacticipants'], Api::Resources::Pacticipants, {resource_name: "pacticipants"}
4343
add ['pacticipants', 'label', :label_name], PactBroker::Api::Resources::PacticipantsForLabel, {resource_name: "pacticipants_for_label"}
44-
add ['pacticipants', :name], Api::Resources::Pacticipant, {resource_name: "pacticipant"}
44+
add ['pacticipants', :pacticipant_name], Api::Resources::Pacticipant, {resource_name: "pacticipant"}
4545
add ['pacticipants', :pacticipant_name, 'versions'], Api::Resources::Versions, {resource_name: "pacticipant_versions"}
4646
add ['pacticipants', :pacticipant_name, 'versions', :pacticipant_version_number], Api::Resources::Version, {resource_name: "pacticipant_version"}
4747
add ['pacticipants', :pacticipant_name, 'latest-version', :tag], Api::Resources::Version, {resource_name: "latest_tagged_pacticipant_version"}

lib/pact_broker/api/resources/base_resource.rb

+8-1
Original file line numberDiff line numberDiff line change
@@ -40,8 +40,12 @@ def initialize
4040
PactBroker.configuration.before_resource.call(self)
4141
end
4242

43+
def update_matrix_after_request?
44+
false
45+
end
46+
4347
def finish_request
44-
if !request.get? && !request.head?
48+
if update_matrix_after_request?
4549
matrix_service.refresh(identifier_from_path)
4650
end
4751
PactBroker.configuration.after_resource.call(self)
@@ -155,6 +159,9 @@ def contract_validation_errors? contract, params
155159
invalid
156160
end
157161

162+
def with_matrix_refresh &block
163+
matrix_service.refresh(identifier_from_path, &block)
164+
end
158165
end
159166
end
160167
end

lib/pact_broker/api/resources/pact.rb

+6-1
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,9 @@ def to_html
8787
end
8888

8989
def delete_resource
90-
pact_service.delete(pact_params)
90+
with_matrix_refresh do
91+
pact_service.delete(pact_params)
92+
end
9193
true
9294
end
9395

@@ -101,6 +103,9 @@ def pact_params
101103
@pact_params ||= PactBroker::Pacts::PactParams.from_request request, path_info
102104
end
103105

106+
def update_matrix_after_request?
107+
request.put? || request.patch?
108+
end
104109
end
105110
end
106111
end

lib/pact_broker/api/resources/pacticipant.rb

+8-2
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,9 @@ def resource_exists?
4545
end
4646

4747
def delete_resource
48-
pacticipant_service.delete pacticipant_name
48+
with_matrix_refresh do
49+
pacticipant_service.delete pacticipant_name
50+
end
4951
true
5052
end
5153

@@ -60,7 +62,11 @@ def pacticipant
6062
end
6163

6264
def pacticipant_name
63-
identifier_from_path[:name]
65+
identifier_from_path[:pacticipant_name]
66+
end
67+
68+
def update_matrix_after_request?
69+
request.patch?
6470
end
6571
end
6672
end

lib/pact_broker/api/resources/tag.rb

+4-2
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ def from_json
2222
@tag = tag_service.create identifier_from_path
2323
# Make it return a 201 by setting the Location header
2424
response.headers["Location"] = tag_url(base_url, tag)
25+
matrix_service.refresh_tags(identifier_from_path)
2526
end
2627
response.body = to_json
2728
end
@@ -39,10 +40,11 @@ def tag
3940
end
4041

4142
def delete_resource
42-
tag_service.delete identifier_from_path
43+
matrix_service.refresh_tags(identifier_from_path) do
44+
tag_service.delete identifier_from_path
45+
end
4346
true
4447
end
45-
4648
end
4749
end
4850

lib/pact_broker/api/resources/verifications.rb

+4
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,10 @@ def next_verification_number
6161
def decorator_for model
6262
PactBroker::Api::Decorators::VerificationDecorator.new(model)
6363
end
64+
65+
def update_matrix_after_request?
66+
request.post?
67+
end
6468
end
6569
end
6670
end

lib/pact_broker/api/resources/version.rb

+3-1
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,9 @@ def to_json
2525
end
2626

2727
def delete_resource
28-
version_service.delete version
28+
with_matrix_refresh do
29+
version_service.delete version
30+
end
2931
true
3032
end
3133

lib/pact_broker/matrix/repository.rb

+38-2
Original file line numberDiff line numberDiff line change
@@ -22,9 +22,45 @@ class Repository
2222
GROUP_BY_PROVIDER = [:consumer_name, :consumer_version_number, :provider_name]
2323
GROUP_BY_PACT = [:consumer_name, :provider_name]
2424

25+
# Use a block when the refresh is caused by a resource deletion
26+
# This allows us to store the correct object ids for use afterwards
2527
def refresh params
26-
PactBroker::Matrix::Row.refresh(params)
27-
PactBroker::Matrix::HeadRow.refresh(params)
28+
criteria = find_ids_for_pacticipant_names(params)
29+
yield if block_given?
30+
PactBroker::Matrix::Row.refresh(criteria)
31+
PactBroker::Matrix::HeadRow.refresh(criteria)
32+
end
33+
34+
# Only need to update the HeadRow table when tags change
35+
# because it only changes which rows are the latest tagged ones -
36+
# it doesn't change the actual values in the underlying matrix.
37+
def refresh_tags params
38+
criteria = find_ids_for_pacticipant_names(params)
39+
yield if block_given?
40+
PactBroker::Matrix::HeadRow.refresh(criteria)
41+
end
42+
43+
def find_ids_for_pacticipant_names params
44+
criteria = {}
45+
46+
if params[:consumer_name] || params[:provider_name]
47+
if params[:consumer_name]
48+
pacticipant = PactBroker::Domain::Pacticipant.where(name_like(:name, params[:consumer_name])).single_record
49+
criteria[:consumer_id] = pacticipant.id if pacticipant
50+
end
51+
52+
if params[:provider_name]
53+
pacticipant = PactBroker::Domain::Pacticipant.where(name_like(:name, params[:provider_name])).single_record
54+
criteria[:provider_id] = pacticipant.id if pacticipant
55+
end
56+
end
57+
58+
if params[:pacticipant_name]
59+
pacticipant = PactBroker::Domain::Pacticipant.where(name_like(:name, params[:pacticipant_name])).single_record
60+
criteria[:pacticipant_id] = pacticipant.id if pacticipant
61+
end
62+
63+
criteria
2864
end
2965

3066
# Return the latest matrix row (pact/verification) for each consumer_version_number/provider_version_number

lib/pact_broker/matrix/row.rb

+14-28
Original file line numberDiff line numberDiff line change
@@ -19,42 +19,28 @@ class Row < Sequel::Model(:materialized_matrix)
1919
include PactBroker::Repositories::Helpers
2020
include PactBroker::Logging
2121

22-
def refresh params
23-
logger.debug("Refreshing #{model.table_name} for #{params}")
22+
def refresh ids
23+
logger.debug("Refreshing #{model.table_name} for #{ids}")
2424

2525
db = model.db
2626
table_name = model.table_name
2727

28-
source_view_name = model.table_name.to_s.gsub('materialized_', '').to_sym
29-
if params[:consumer_name] || params[:provider_name]
30-
criteria = {}
31-
if params[:consumer_name]
32-
pacticipant = PactBroker::Domain::Pacticipant.where(name_like(:name, params[:consumer_name])).single_record
33-
criteria[:consumer_id] = pacticipant.id if pacticipant
28+
if ids[:pacticipant_id]
29+
db.transaction do
30+
db[table_name].where(consumer_id: ids[:pacticipant_id]).or(provider_id: ids[:pacticipant_id]).delete
31+
new_rows = db[source_view_name].where(consumer_id: ids[:pacticipant_id]).or(provider_id: ids[:pacticipant_id]).distinct
32+
db[table_name].insert(new_rows)
3433
end
35-
36-
if params[:provider_name]
37-
pacticipant = PactBroker::Domain::Pacticipant.where(name_like(:name, params[:provider_name])).single_record
38-
criteria[:provider_id] = pacticipant.id if pacticipant
39-
end
40-
if criteria.any?
41-
db.transaction do
42-
db[table_name].where(criteria).delete
43-
db[table_name].insert(db[source_view_name].where(criteria).distinct)
44-
end
34+
elsif ids.any?
35+
db.transaction do
36+
db[table_name].where(ids).delete
37+
db[table_name].insert(db[source_view_name].where(ids).distinct)
4538
end
4639
end
40+
end
4741

48-
if params[:pacticipant_name]
49-
pacticipant = PactBroker::Domain::Pacticipant.where(name_like(:name, params[:pacticipant_name])).single_record
50-
if pacticipant
51-
db.transaction do
52-
db[table_name].where(consumer_id: pacticipant.id).or(provider_id: pacticipant.id).delete
53-
new_rows = db[source_view_name].where(consumer_id: pacticipant.id).or(provider_id: pacticipant.id).distinct
54-
db[table_name].insert(new_rows)
55-
end
56-
end
57-
end
42+
def source_view_name
43+
model.table_name.to_s.gsub('materialized_', '').to_sym
5844
end
5945

6046
def matching_selectors selectors

lib/pact_broker/matrix/service.rb

+6-2
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,12 @@ module Service
99
extend PactBroker::Repositories
1010
extend PactBroker::Services
1111

12-
def refresh params
13-
matrix_repository.refresh params
12+
def refresh params, &block
13+
matrix_repository.refresh(params, &block)
14+
end
15+
16+
def refresh_tags params, &block
17+
matrix_repository.refresh_tags(params, &block)
1418
end
1519

1620
def find criteria, options = {}

spec/features/publish_verification_spec.rb

-1
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@
1010

1111
subject { post path, verification_content, {'CONTENT_TYPE' => 'application/json' }; last_response }
1212

13-
1413
before do
1514
td.create_provider("Provider")
1615
.create_consumer("Consumer")

spec/features/update_matrix_spec.rb

+127
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,127 @@
1+
=begin
2+
3+
Things that should update the matrix (ignoring tags):
4+
* pact created
5+
* pact content updated
6+
* pact deleted
7+
* verification created
8+
* verification deleted (not yet implemented)
9+
10+
It's easier to update the matrix at the resource level, so we actually update the matrix when:
11+
* pacticipant deleted
12+
* version deleted
13+
* pact deleted
14+
15+
Things that should update the head matrix
16+
* All of the above
17+
* tag created
18+
* tag deleted
19+
20+
=end
21+
22+
describe "Deleting a resource that affects the matrix" do
23+
24+
let(:td) { TestDataBuilder.new }
25+
let(:response_body_json) { JSON.parse(subject.body) }
26+
27+
subject { delete path; last_response }
28+
29+
before do
30+
td.create_pact_with_hierarchy("Foo", "1", "Bar")
31+
.create_consumer_version_tag("prod")
32+
.create_verification(provider_version: "2")
33+
end
34+
35+
context "deleting a pact" do
36+
let(:path) { "/pacts/provider/Bar/consumer/Foo/version/1" }
37+
38+
it "deletes the relevant lines from the matrix" do
39+
expect{ subject }.to change{ PactBroker::Matrix::Row.count }.by(-1)
40+
end
41+
end
42+
43+
context "deleting a pacticipant" do
44+
let(:path) { "/pacticipants/Bar" }
45+
46+
it "deletes the relevant lines from the matrix" do
47+
expect{ subject }.to change{ PactBroker::Matrix::Row.count }.by(-1)
48+
end
49+
end
50+
51+
context "deleting a version" do
52+
let(:path) { "/pacticipants/Foo/versions/1" }
53+
54+
it "deletes the relevant lines from the matrix" do
55+
expect{ subject }.to change{ PactBroker::Matrix::Row.count }.by(-1)
56+
end
57+
end
58+
59+
context "deleting a tag" do
60+
let(:path) { "/pacticipants/Foo/versions/1/tags/prod" }
61+
62+
it "deletes the relevant lines from the matrix" do
63+
expect{ subject }.to change{ PactBroker::Matrix::HeadRow.count }.by(-1)
64+
end
65+
end
66+
end
67+
68+
describe "Creating a resource that affects the matrix" do
69+
70+
let(:td) { TestDataBuilder.new }
71+
let(:response_body_json) { JSON.parse(subject.body) }
72+
73+
subject { put(path, nil, {'CONTENT_TYPE' => 'application/json'}); last_response }
74+
75+
context "creating a tag" do
76+
before do
77+
td.create_pact_with_hierarchy("Foo", "1", "Bar")
78+
end
79+
80+
let(:path) { "/pacticipants/Foo/versions/1/tags/prod" }
81+
82+
it "adds the relevant lines to the head matrix" do
83+
expect{ subject }.to change{ PactBroker::Matrix::HeadRow.count }.by(1)
84+
end
85+
86+
it "does not add any lines to the matrix" do
87+
expect{ subject }.to change{ PactBroker::Matrix::Row.count }.by(0)
88+
end
89+
end
90+
91+
context "creating a pact" do
92+
let(:pact_content) { load_fixture('foo-bar.json') }
93+
let(:path) { "/pacts/provider/Bar/consumer/Foo/versions/1.2.3" }
94+
95+
subject { put path, pact_content, {'CONTENT_TYPE' => 'application/json'}; last_response }
96+
97+
it "adds the relevant lines to the matrix" do
98+
expect{ subject }.to change{ PactBroker::Matrix::Row.count }.by(1)
99+
end
100+
101+
it "adds the relevant lines to the head matrix" do
102+
expect{ subject }.to change{ PactBroker::Matrix::HeadRow.count }.by(1)
103+
end
104+
end
105+
106+
context "creating a verification" do
107+
let(:td) { TestDataBuilder.new }
108+
let(:path) { "/pacts/provider/Bar/consumer/Foo/pact-version/#{pact.pact_version_sha}/verification-results" }
109+
let(:verification_content) { load_fixture('verification.json') }
110+
let(:parsed_response_body) { JSON.parse(subject.body) }
111+
let(:pact) { td.pact }
112+
113+
subject { post path, verification_content, {'CONTENT_TYPE' => 'application/json' }; last_response }
114+
115+
before do
116+
td.create_pact_with_hierarchy("Foo", "1", "Bar")
117+
end
118+
119+
it "updates the relevant lines in the matrix" do
120+
expect{ subject }.to change{ PactBroker::Matrix::Row.first.provider_version_number }.from(nil).to("4.5.6")
121+
end
122+
123+
it "updates the relevant lines in the head matrix" do
124+
expect{ subject }.to change{ PactBroker::Matrix::HeadRow.first.provider_version_number }.from(nil).to("4.5.6")
125+
end
126+
end
127+
end

0 commit comments

Comments
 (0)