From bb531cc98338ffae1398a1078f20b644a5c54357 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phan=20Mestach?= Date: Wed, 28 Dec 2022 16:14:52 +0100 Subject: [PATCH 1/2] WIP : handle conflicts, and response wrapper. TODO parallel publisher --- app/services/invoicing/invoice_entity.rb | 10 +- .../services/invoicing/invoice_entity_spec.rb | 152 +++++++++++------- 2 files changed, 103 insertions(+), 59 deletions(-) diff --git a/app/services/invoicing/invoice_entity.rb b/app/services/invoicing/invoice_entity.rb index 3c982ddd..df9489dd 100644 --- a/app/services/invoicing/invoice_entity.rb +++ b/app/services/invoicing/invoice_entity.rb @@ -73,7 +73,15 @@ def publish_to_dhis2 status = if Flipper[:use_parallel_publishing].enabled?(project.project_anchor) parallel_publish_to_dhis2 else - project.dhis2_connection.data_value_sets.create(@dhis2_export_values) + begin + rsp = project.dhis2_connection.data_value_sets.create(@dhis2_export_values) + + # dhis2 2.38 return status in a wrapped response + rsp.raw_status["response"] ? Dhis2::Status.new(rsp.raw_status["response"]) : rsp + rescue RestClient::Conflict => e + # dhis2 2.38 return import as conflicts + Dhis2::Status.new(JSON.parse(e.response.body)["response"]) + end end # minimize memory usage, don't log exported values but only the status diff --git a/spec/services/invoicing/invoice_entity_spec.rb b/spec/services/invoicing/invoice_entity_spec.rb index f5366e98..8ace37d2 100644 --- a/spec/services/invoicing/invoice_entity_spec.rb +++ b/spec/services/invoicing/invoice_entity_spec.rb @@ -7,6 +7,61 @@ include_context "basic_context" include Dhis2SnapshotFixture + let(:conflicts_reponse) { + { + "status": "WARNING", + "conflicts": [ + { + "value": "Data is already approved for data set: TsLR0wQJknp period: 201903 organisation unit: iA3y8AyMTG2 attribute option combo: HllvX50cXC0", + "object": "iA3y8AyMTG2" + }, + { + "value": "Data is already approved for data set: TsLR0wQJknp period: 201902 organisation unit: iA3y8AyMTG2 attribute option combo: HllvX50cXC0", + "object": "iA3y8AyMTG2" + }, + { + "value": "Data is already approved for data set: TsLR0wQJknp period: 201901 organisation unit: iA3y8AyMTG2 attribute option combo: HllvX50cXC0", + "object": "iA3y8AyMTG2" + } + ], + "description": "Import process completed successfully", + "import_count": { + "deleted": 0, + "ignored": 3, + "updated": 0, + "imported": 1 + }, + "response_type": "ImportSummary", + "import_options": { + "async": false, + "force": false, + "dry_run": false, + "sharing": false, + "id_schemes": { + }, + "merge_mode": "REPLACE", + "skip_audit": false, + "report_mode": "FULL", + "strict_periods": false, + "import_strategy": "CREATE_AND_UPDATE", + "skip_last_updated": false, + "skip_notifications": false, + "first_row_is_header": true, + "skip_existing_check": false, + "strict_data_elements": false, + "dataset_allows_periods": false, + "ignore_empty_collection": false, + "skip_pattern_validation": false, + "strict_organisation_units": false, + "require_category_option_combo": false, + "strict_category_option_combos": false, + "require_attribute_option_combo": false, + "strict_attribute_option_combos": false + }, + "data_set_complete": "false" + } + } + describe "use_parallel_publishing" do let(:project) { full_project } let(:expected_url) { "http://play.dhis2.org/demo/api/dataValueSets" } @@ -25,62 +80,7 @@ } } - let(:conflicts_reponse) { - { - "status": "WARNING", - "conflicts": [ - { - "value": "Data is already approved for data set: TsLR0wQJknp period: 201903 organisation unit: iA3y8AyMTG2 attribute option combo: HllvX50cXC0", - "object": "iA3y8AyMTG2" - }, - { - "value": "Data is already approved for data set: TsLR0wQJknp period: 201902 organisation unit: iA3y8AyMTG2 attribute option combo: HllvX50cXC0", - "object": "iA3y8AyMTG2" - }, - { - "value": "Data is already approved for data set: TsLR0wQJknp period: 201901 organisation unit: iA3y8AyMTG2 attribute option combo: HllvX50cXC0", - "object": "iA3y8AyMTG2" - } - ], - "description": "Import process completed successfully", - "import_count": { - "deleted": 0, - "ignored": 3, - "updated": 0, - "imported": 1 - }, - "response_type": "ImportSummary", - "import_options": { - "async": false, - "force": false, - "dry_run": false, - "sharing": false, - "id_schemes": { - }, - "merge_mode": "REPLACE", - "skip_audit": false, - "report_mode": "FULL", - "strict_periods": false, - "import_strategy": "CREATE_AND_UPDATE", - "skip_last_updated": false, - "skip_notifications": false, - "first_row_is_header": true, - "skip_existing_check": false, - "strict_data_elements": false, - "dataset_allows_periods": false, - "ignore_empty_collection": false, - "skip_pattern_validation": false, - "strict_organisation_units": false, - "require_category_option_combo": false, - "strict_category_option_combos": false, - "require_attribute_option_combo": false, - "strict_attribute_option_combos": false - }, - "data_set_complete": "false" - } - } - - it "sends values without feature enabled" do + it "sends values without feature enabled to a dhis2 < 2.38" do entity.instance_variable_set(:@dhis2_export_values, [{ value: 1234 }] * 1001) stub_request(:any, expected_url).to_return(body: success_response.to_json) @@ -88,11 +88,47 @@ expect(a_request(:post, expected_url)).to have_been_made.times(1) end - it "sends locked values without feature enabled" do + it "sends values without feature enabled to a dhis2 >= 2.38" do + entity.instance_variable_set(:@dhis2_export_values, [{ value: 1234 }] * 1001) + stub_request(:any, expected_url).to_return( + status: 200, + body: { + "httpStatus": "OK", + "httpStatusCode": 200, + "status": "OK", + "message": "Import was successful.", + "response": success_response + }.to_json + ) + + expect { entity.publish_to_dhis2 }.to change { Dhis2Log.count }.by(1) + expect(a_request(:post, expected_url)).to have_been_made.times(1) + end + + it "sends locked values without feature enabled to a dhis2 < 2.38" do entity.instance_variable_set(:@dhis2_export_values, [{ value: 1234 }] * 1001) stub_request(:any, expected_url).to_return(body: conflicts_reponse.to_json) - expect { entity.publish_to_dhis2 }.to raise_error Invoicing::PublishingError, "Data is already approved for data set: TsLR0wQJknp period: 201903 organisation unit: iA3y8AyMTG2 attribute option combo: HllvX50cXC0, Data is already approved for data set: TsLR0wQJknp period: 201902 organisation unit: iA3y8AyMTG2 attribute option combo: HllvX50cXC0, Data is already approved for data set: TsLR0wQJknp period: 201901 organisation unit: iA3y8AyMTG2 attribute option combo: HllvX50cXC0" + expect { entity.publish_to_dhis2 }.to raise_error( + Invoicing::PublishingError, + "Data is already approved for data set: TsLR0wQJknp period: 201903 organisation unit: iA3y8AyMTG2 attribute option combo: HllvX50cXC0, Data is already approved for data set: TsLR0wQJknp period: 201902 organisation unit: iA3y8AyMTG2 attribute option combo: HllvX50cXC0, Data is already approved for data set: TsLR0wQJknp period: 201901 organisation unit: iA3y8AyMTG2 attribute option combo: HllvX50cXC0" + ) + end + + it "sends locked values without feature enabled to a dhis2 >= 2.38" do + entity.instance_variable_set(:@dhis2_export_values, [{ value: 1234 }] * 1001) + stub_request(:any, expected_url).to_return(status: 409, body: { + "httpStatus": "Conflict", + "httpStatusCode": 409, + "status": "WARNING", + "message": "One more conflicts encountered, please check import summary.", + "response": conflicts_reponse + }.to_json) + + expect { entity.publish_to_dhis2 }.to raise_error( + Invoicing::PublishingError, + "Data is already approved for data set: TsLR0wQJknp period: 201903 organisation unit: iA3y8AyMTG2 attribute option combo: HllvX50cXC0, Data is already approved for data set: TsLR0wQJknp period: 201902 organisation unit: iA3y8AyMTG2 attribute option combo: HllvX50cXC0, Data is already approved for data set: TsLR0wQJknp period: 201901 organisation unit: iA3y8AyMTG2 attribute option combo: HllvX50cXC0" + ) end it "sends values with feature enabled" do From ee47f944198996f4601baa7178420a269681d303 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phan=20Mestach?= Date: Mon, 9 Jan 2023 10:22:54 +0100 Subject: [PATCH 2/2] Implement the same strategy for parallel publishing --- lib/parallel_dhis2.rb | 7 +++++-- spec/services/invoicing/invoice_entity_spec.rb | 18 ++++++++++++++++++ 2 files changed, 23 insertions(+), 2 deletions(-) diff --git a/lib/parallel_dhis2.rb b/lib/parallel_dhis2.rb index f2667ad1..fca22d10 100644 --- a/lib/parallel_dhis2.rb +++ b/lib/parallel_dhis2.rb @@ -272,6 +272,7 @@ def post_data_value_sets(all_values) responses = requests.map(&:response) parsed_responses = parse_responses(responses) raw_status = RollUpResponses.new(parsed_responses).call + Dhis2::Status.new(raw_status) end @@ -282,14 +283,16 @@ def parse_responses(responses) next if [nil, ""].include?(response.body) parsed_response = JSON.parse(response.body) - Dhis2::Case.deep_change(parsed_response, :underscore) + r = Dhis2::Case.deep_change(parsed_response, :underscore) + + r["response"] || r end parsed.compact end def check_for_errors!(responses) responses.each do |response| - next if response.success? + next if response.success? || response.code == 409 if response.timed_out? message = "#{response.effective_url} timed out" diff --git a/spec/services/invoicing/invoice_entity_spec.rb b/spec/services/invoicing/invoice_entity_spec.rb index 8ace37d2..95272d4a 100644 --- a/spec/services/invoicing/invoice_entity_spec.rb +++ b/spec/services/invoicing/invoice_entity_spec.rb @@ -131,6 +131,24 @@ ) end + it "sends locked values with feature enabled to a dhis2 >= 2.38" do + Flipper[:use_parallel_publishing].enable(project.project_anchor) + entity.instance_variable_set(:@dhis2_export_values, [{ value: 1234 }] * 1001) + stub_request(:any, expected_url).to_return(status: 409, body: { + "httpStatus": "Conflict", + "httpStatusCode": 409, + "status": "WARNING", + "message": "One more conflicts encountered, please check import summary.", + "response": conflicts_reponse + }.to_json) + + expect { entity.publish_to_dhis2 }.to raise_error( + Invoicing::PublishingError, + "Data is already approved for data set: TsLR0wQJknp period: 201903 organisation unit: iA3y8AyMTG2 attribute option combo: HllvX50cXC0, Data is already approved for data set: TsLR0wQJknp period: 201902 organisation unit: iA3y8AyMTG2 attribute option combo: HllvX50cXC0, Data is already approved for data set: TsLR0wQJknp period: 201901 organisation unit: iA3y8AyMTG2 attribute option combo: HllvX50cXC0" + ) + expect(a_request(:post, expected_url)).to have_been_made.times(2) + end + it "sends values with feature enabled" do Flipper[:use_parallel_publishing].enable(project.project_anchor) entity.instance_variable_set(:@dhis2_export_values, [{ value: 1234 }] * 1001)