active_hash validates the IDs correctly

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'

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

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]

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