From 3b554952a7c3f8ee6bbdfcf24abebac0368d009f Mon Sep 17 00:00:00 2001 From: halilsen Date: Mon, 12 Jul 2021 15:38:47 +0200 Subject: [PATCH] active_hash validates the IDs correctly --- Gemfile | 3 +- Gemfile.lock | 15 +++--- models/concerns/validate_data.rb | 11 ---- test/models/vrp_consistency_test.rb | 82 +++++++++-------------------- 4 files changed, 33 insertions(+), 78 deletions(-) diff --git a/Gemfile b/Gemfile index c4132b196..4f55954f8 100644 --- a/Gemfile +++ b/Gemfile @@ -16,7 +16,8 @@ gem 'grape-swagger-entity' gem 'grape_logging' gem 'actionpack' -gem 'active_hash', github: 'Mapotempo/active_hash', branch: 'mapo' +gem 'active_hash', github: 'senhalil/active_hash' # activate the next line after [PR4](https://github.com/Mapotempo/active_hash/pull/4) is merged +# gem 'active_hash', github: 'mapotempo/active_hash', branch: 'dev' # waiting for the following PRs to get merged and "released!" https://github.com/zilkey/active_hash/pull/231 and https://github.com/zilkey/active_hash/pull/233 gem 'activemodel' gem 'charlock_holmes' diff --git a/Gemfile.lock b/Gemfile.lock index 7decb8aae..92d6017e2 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -1,11 +1,3 @@ -GIT - remote: https://github.com/Mapotempo/active_hash.git - revision: 6471576562e2db78fa87f8a1cc6ec848c88ab76e - branch: mapo - specs: - active_hash (3.1.0) - activesupport (>= 5.0.0) - GIT remote: https://github.com/Mapotempo/balanced_vrp_clustering.git revision: c76cca8fe53bf802aca2f998631f7c24c59cb3c4 @@ -16,6 +8,13 @@ GIT color-generator geojson2image +GIT + remote: https://github.com/senhalil/active_hash.git + revision: 4229b74b97e50114a1bf6d69b63766cee75bea1c + specs: + active_hash (3.1.0) + activesupport (>= 5.0.0) + GIT remote: https://github.com/senhalil/rack.git revision: a23188bb32972dde155977655f57dea035eee6ea diff --git a/models/concerns/validate_data.rb b/models/concerns/validate_data.rb index 1c04c5189..584a667a6 100644 --- a/models/concerns/validate_data.rb +++ b/models/concerns/validate_data.rb @@ -27,7 +27,6 @@ def check_consistency(hash) hash[:relations] ||= [] @hash = hash - ensure_uniq_ids ensure_no_conflicting_skills configuration = @hash[:configuration] @@ -44,16 +43,6 @@ def check_consistency(hash) check_configuration(configuration, periodic_heuristic) if configuration end - def ensure_uniq_ids - # TODO: Active Hash should be checking this - [:matrices, :units, :points, :rests, :zones, :timewindows, - :vehicles, :services, :subtours].each{ |key| - next if @hash[key]&.collect{ |v| v[:id] }&.uniq!.nil? - - raise OptimizerWrapper::DiscordantProblemError.new("#{key} IDs should be unique") - } - end - def ensure_no_conflicting_skills all_skills = (@hash[:vehicles] + @hash[:services]).flat_map{ |mission| mission[:skills] diff --git a/test/models/vrp_consistency_test.rb b/test/models/vrp_consistency_test.rb index ecb3b6056..e22e8a1b1 100644 --- a/test/models/vrp_consistency_test.rb +++ b/test/models/vrp_consistency_test.rb @@ -246,64 +246,30 @@ def test_available_intervals_compatibility end def test_duplicated_ids_are_not_allowed - assert TestHelper.create(VRP.basic) # this should not produce any error - - vrp = VRP.basic - vrp[:vehicles] << vrp[:vehicles].first - assert_raises OptimizerWrapper::DiscordantProblemError do - TestHelper.create(vrp) - end - - vrp = VRP.basic - vrp[:services] << vrp[:services].first - assert_raises OptimizerWrapper::DiscordantProblemError do - TestHelper.create(vrp) - end - - vrp = VRP.basic - vrp[:matrices] << vrp[:matrices].first - assert_raises OptimizerWrapper::DiscordantProblemError do - TestHelper.create(vrp) - end - - vrp = VRP.basic - vrp[:points] << vrp[:points].first - assert_raises OptimizerWrapper::DiscordantProblemError do - TestHelper.create(vrp) - end - - vrp = VRP.basic - vrp[:rests] = Array.new(2){ |_rest| { id: 'same_id', timewindows: [{ start: 1, end: 2 }], duration: 1 } } - assert_raises OptimizerWrapper::DiscordantProblemError do - TestHelper.create(vrp) - end - - assert TestHelper.create(VRP.pud) # this should not produce any error - vrp = VRP.pud - vrp[:shipments] << vrp[:shipments].first - assert_raises OptimizerWrapper::DiscordantProblemError do - TestHelper.create(vrp) - end - - vrp = VRP.pud - vrp[:vehicles].first[:capacities] = [{ unit_id: 'unit_0', value: 10 }] - vrp[:shipments].first[:quantities] = [{ unit_id: 'unit_0', value: -5 }] - vrp[:units] << vrp[:units].first - assert_raises OptimizerWrapper::DiscordantProblemError do - TestHelper.create(vrp) - end - - vrp = VRP.pud - vrp[:zones] = Array.new(2){ |_zone| { id: 'same_zone', polygon: { type: 'Polygon', coordinates: [[[0.5, 48.5], [1.5, 48.5]]] }} } - assert_raises OptimizerWrapper::DiscordantProblemError do - TestHelper.create(vrp) - end - - vrp = VRP.pud - vrp[:subtours] = Array.new(2){ |_tour| { id: 'same_tour', time_bouds: 180 } } - assert_raises OptimizerWrapper::DiscordantProblemError do - TestHelper.create(vrp) - end + vrp_base = VRP.basic + # add missing fields so that they can be duplicated for the test + vrp_base[:rests] = [{ id: 'rest', timewindows: [{ start: 1, end: 2 }], duration: 1 }] + vrp_base[:shipments] = [{ + id: 'shipment', + pickup: { point_id: 'point_3', duration: 3 }, + delivery: { point_id: 'point_2', duration: 3 }, + quantities: [{ unit_id: 'unit', value: 3 }] + }] + vrp_base[:zones] = [{ id: 'zone', polygon: { type: 'Polygon', coordinates: [[[0.5, 48.5], [1.5, 48.5]]] }}] + vrp_base[:subtours] = [{ id: 'tour', time_bounds: 180 }] + vrp_base[:units] = [{ id: 'unit' }] + vrp_base[:vehicles].first[:capacities] = [{ unit_id: 'unit', limit: 10 }] + vrp_base = Oj.dump(vrp_base) + + assert TestHelper.create(Oj.load(vrp_base)) # this should not produce any errors + + %i[ + matrices points rests services shipments units vehicles zones + ].each{ |symbol| + vrp = Oj.load(vrp_base) + vrp[symbol] << vrp[symbol].first + assert_raises ActiveHash::IdError do TestHelper.create(vrp) end + } end def test_dates_cannot_be_mixed_with_indices