Remove skills create logic and extend convert logic

lib/interpreters/periodic_visits.rb

@@ -182,7 +182,7 @@ def generate_services(vrp)
             first_possible_days: [service.first_possible_days[visit_index]],
             last_possible_days: [service.last_possible_days[visit_index]]
           )
-          new_service.skills += ["#{visit_index + 1}_f_#{service.visits_number}"] if !service.minimum_lapse && !service.maximum_lapse && service.visits_number > 1
+          new_service.skills += ["#{visit_index + 1}_f_#{service.visits_number}".to_sym] if !service.minimum_lapse && !service.maximum_lapse && service.visits_number > 1
 
           @expanded_services[service.id] ||= []
           @expanded_services[service.id] << new_service
@@ -397,10 +397,10 @@ def generate_rests(vehicle, day_index, vehicle_timewindow, rests_durations)
 
     def associate_skills(new_vehicle, vehicle_day_index)
       if new_vehicle.skills.empty?
-        new_vehicle.skills = [@periods.collect{ |period| "#{(vehicle_day_index * period / (@schedule_end + 1)).to_i + 1}_f_#{period}" }]
+        new_vehicle.skills = [@periods.collect{ |period| "#{(vehicle_day_index * period / (@schedule_end + 1)).to_i + 1}_f_#{period}".to_sym }]
       else
         new_vehicle.skills.collect!{ |alternative_skill|
-          alternative_skill + @periods.collect{ |period| "#{(vehicle_day_index * period / (@schedule_end + 1)).to_i + 1}_f_#{period}" }
+          alternative_skill + @periods.collect{ |period| "#{(vehicle_day_index * period / (@schedule_end + 1)).to_i + 1}_f_#{period}".to_sym }
         }
       end
     end

lib/interpreters/split_clustering.rb

@@ -54,13 +54,13 @@ def self.split_clusters(service_vrp, job = nil, &block)
             route.stops.each do |stop|
               next unless stop.service_id
 
-              stop.skills = stop.skills.to_a + ["cluster #{cluster_ref}"]
+              stop.skills = stop.skills.to_a + ["cluster #{cluster_ref}".to_sym]
             end
           }
           solution.unassigned_stops.each do |stop|
             next if stop.service_id.nil?
 
-            stop.skills = stop.skills.to_a + ["cluster #{cluster_ref}"]
+            stop.skills = stop.skills.to_a + ["cluster #{cluster_ref}".to_sym]
           end
           solution
         }
@@ -987,7 +987,7 @@ def self.collect_cluster_data(vrp, nb_clusters)
             depot: depots[simulated_vehicle],
             capacities: vehicles[simulated_vehicle][:capacities].collect{ |key, value| [key, value * vehicles.size / nb_clusters.to_f] }.to_h, #TODO: capacities needs a better way like depots...
             skills: [],
-            day_skills: ['0_day_skill', '1_day_skill', '2_day_skill', '3_day_skill', '4_day_skill', '5_day_skill', '6_day_skill'],
+            day_skills: [:'0_day_skill', :'1_day_skill', :'2_day_skill', :'3_day_skill', :'4_day_skill', :'5_day_skill', :'6_day_skill'],
             duration: 0,
             total_work_days: 1
           }
@@ -1048,11 +1048,11 @@ module ClassMethods
       def compute_day_skills(timewindows)
         if timewindows.nil? || timewindows.empty? || timewindows.any?{ |tw| tw[:day_index].nil? }
           [0, 1, 2, 3, 4, 5, 6].collect{ |avail_day|
-            "#{avail_day}_day_skill"
+            "#{avail_day}_day_skill".to_sym
           }
         else
           timewindows.collect{ |tw| tw[:day_index] }.uniq.collect{ |avail_day|
-            "#{avail_day}_day_skill"
+            "#{avail_day}_day_skill".to_sym
           }
         end
       end

models/base.rb

@@ -154,14 +154,20 @@ def self.has_many(name, options = {})
         raise 'Unknown :vrp_result option'
       end
 
+      if options[:type]
+        types[name] = options[:type]
+      end
+
       redefine_method(name) do
         self[name] ||= self.class.default_attributes[name]
       end
 
       redefine_method("#{name}=") do |vals|
         c = class_from_string(options[:class_name])
         self[name] = vals&.collect{ |val|
-          if val.is_a?(c)
+          if c == Symbol
+            val&.to_sym
+          elsif val.is_a?(c)
             val
           else
             c.create(val) if !val.empty?
@@ -238,19 +244,23 @@ def self.belongs_to(name, options = {})
     private
 
     def convert(key, value)
-      case self.class.types[key].to_s
+      convert_type(self.class.types[key].to_s, value)
+    end
+
+    def convert_type(type, value)
+      if type.start_with?('[')
+        return value&.map{ |v| convert_type(type[1..-2], v) }&.sort
+      end
+
+      case type
       when ''
         value
-      when '[Symbol]'
-        value = [] if value.to_a.empty?
-        value.map!(&:to_sym)
-        value.sort!
       when 'Symbol'
         value&.to_sym
       when 'Date'
         value&.to_date
       else
-        raise "Unknown type #{self.class.types[key]} for key #{key} with value #{value}"
+        raise "Unknown type #{type} with value #{value}"
       end
     end
 

models/service.rb

@@ -51,8 +51,8 @@ class Service < Base
 
     # validates_inclusion_of :type, :in => %i(service pickup delivery)
 
-    field :skills, type: Array[Symbol]
-    field :original_skills, type: Array[Symbol]
+    has_many :skills, class_name: 'Symbol', type: Array[Symbol]
+    has_many :original_skills, class_name: 'Symbol', type: Array[Symbol]
 
     field :vehicle_compatibility, as_json: :none # vehicle_compatibility[v_id] == {true -> compatible, false -> incompatible, nil -> not checked yet}
 
@@ -62,5 +62,15 @@ class Service < Base
     has_many :sticky_vehicles, class_name: 'Models::Vehicle', as_json: :ids
     has_many :quantities, class_name: 'Models::Quantity'
     has_many :relations, class_name: 'Models::Relation', as_json: :none
+
+    def self.create(hash)
+      hash[:original_id] ||= hash[:id]
+      hash[:original_skills] ||= hash[:skills] if hash[:skills]&.none?{ |skill|
+        skill.to_s.include?('vehicle_partition_') ||
+        skill.to_s.include?('work_day_partition_')
+      }
+
+      super(hash)
+    end
   end
 end

models/vehicle.rb

@@ -68,8 +68,8 @@ class Vehicle < Base
     field :unavailable_days, default: Set[] # extends unavailable_work_date and unavailable_work_day_indices
     field :global_day_index, default: nil
 
-    has_many :skills, class_name: 'Array' # Vehicles can have multiple alternative skillsets
-    has_many :original_skills, class_name: 'Array'
+    has_many :skills, class_name: 'Array', type: [[Symbol]] # Vehicles can have multiple alternative skillsets
+    has_many :original_skills, class_name: 'Array', type: [[Symbol]]
 
     field :free_approach, default: false
     field :free_return, default: false
@@ -118,11 +118,14 @@ def self.create(hash, _options = {})
         hash[:unavailable_days].delete_if{ |index| !work_day_indices.include?(index.modulo(7)) }
       end
 
-      hash[:skills] = [[]] if hash[:skills].to_a.empty? # If vehicle has no skills, it has the empty skillset
-      hash[:skills].map!{ |skill_set| skill_set.map!(&:to_sym) }
-      hash[:skills].each(&:sort!) # The order of skills of a skillset should not matter
+      hash[:original_id] ||= hash[:id]
+      hash[:original_skills] ||= hash[:skills]
 
-      super(hash)
+      vehicle = super(hash)
+      # Default skills make the empty skill_set to share the same object_id to every vehicle with no skills
+      vehicle.skills = [[]] if vehicle.skills.to_a.empty?
+      vehicle.skills.each(&:sort!)
+      vehicle
     end
 
     def need_matrix_time?

models/vrp.rb

@@ -63,7 +63,6 @@ def self.create(hash, options = {})
 
       vrp = super({})
       # moved filter here to make sure we do have configuration.schedule.indices (not date) to do work_day check with lapses
-      self.convert_expected_string_to_symbol(hash)
       self.ensure_retrocompatibility(hash)
       self.filter(hash) if options[:check] # TODO : add filters.rb here
       vrp.check_consistency(hash) if options[:check]
@@ -377,31 +376,6 @@ def self.convert_relation_lapse_into_lapses(hash)
       }
     end
 
-    def self.convert_expected_string_to_symbol(hash)
-      hash[:relations].each{ |relation|
-        relation[:type] = relation[:type]&.to_sym
-      }
-
-      hash[:services].each{ |service|
-        service[:skills] = service[:skills]&.map(&:to_sym)
-        service[:type] = service[:type]&.to_sym
-        if service[:activity] && service[:activity][:position]&.to_sym
-          service[:activity][:position] = service[:activity][:position]&.to_sym
-        end
-
-        service[:activities]&.each{ |activity|
-          activity[:position] = activity[:position]&.to_sym
-        }
-      }
-
-      hash[:vehicles].each{ |vehicle|
-        vehicle[:router_dimension] = vehicle[:router_dimension]&.to_sym
-        vehicle[:router_mode] = vehicle[:router_mode]&.to_sym
-        vehicle[:shift_preference] = vehicle[:shift_preference]&.to_sym
-        vehicle[:skills] = vehicle[:skills]&.map{ |sk_set| sk_set.map(&:to_sym) }
-      }
-    end
-
     def self.ensure_retrocompatibility(hash)
       self.convert_linked_ids_into_linked_service_ids(hash)
       self.convert_shipments_to_services(hash)

optimizer_wrapper.rb

@@ -564,7 +564,7 @@ def self.apply_zones(vrp)
       next if zone.vehicles.compact.empty?
 
       zone.vehicles.each{ |vehicle|
-        vehicle.skills.each{ |skillset| skillset << zone[:id] }
+        vehicle.skills.each{ |skillset| skillset << zone[:id].to_sym }
       }
     }
 
@@ -576,7 +576,7 @@ def self.apply_zones(vrp)
 
         next unless zone.inside(activity_loc.lat, activity_loc.lon)
 
-        service.skills += [zone[:id]]
+        service.skills += [zone[:id].to_sym]
         service.id
       }.compact
 

test/api/v01/output_test.rb

@@ -144,8 +144,8 @@ def test_returned_skills
     vrp = VRP.lat_lon_two_vehicles
     vrp[:configuration][:preprocessing] = { partitions: TestHelper.vehicle_and_days_partitions }
     vrp[:configuration][:schedule] = { range_indices: { start: 0, end: 10 }}
-    vrp[:vehicles].first[:skills] = [['skill_to_output']]
-    vrp[:services].first[:skills] = ['skill_to_output']
+    vrp[:vehicles].first[:skills] = [[:skill_to_output]]
+    vrp[:services].first[:skills] = [:skill_to_output]
 
     response = post '/0.1/vrp/submit', { api_key: 'demo', vrp: vrp }.to_json, 'CONTENT_TYPE' => 'application/json'
     result = JSON.parse(response.body)['solutions'].first
@@ -173,7 +173,7 @@ def test_skill_when_partitions
       # - exactly 1 skill corresponding to work_day entity
       assert_equal element['detail']['skills'].size - 1, (element['detail']['skills'] - %w[mon tue wed thu fri sat sun]).size
       # - exactly 1 skill corresponding to cluster number
-      assert_equal 1, (element['detail']['skills'].count{ |skill| skill.include?('cluster') })
+      assert_equal 1, (element['detail']['skills'].count{ |skill| /cluster\ [0-9]/.match?(skill) })
     }
 
     # to make it hard to find original_id back :
@@ -190,7 +190,7 @@ def test_skill_when_partitions
       # - exactly 1 skill corresponding to vehicle_id entity
       assert(element['detail']['skills'].include?('vehicle_cluster_vehicle_0') ^ element['detail']['skills'].include?('vehicle_cluster_vehicle_1'))
       # - exactly 1 skill corresponding to cluster number
-      assert_equal 1, (element['detail']['skills'].count{ |skill| skill.include?('cluster ') })
+      assert_equal 1, (element['detail']['skills'].count{ |skill| /cluster\ [0-9]/.match?(skill) })
     }
   end
 

test/lib/interpreters/interpreter_test.rb

@@ -36,8 +36,8 @@ def test_global_periodic_expand
     expanded_vrp = periodic_expand(problem)
     assert_equal number_of_days, expanded_vrp[:vehicles].size
     assert_equal problem[:services].collect{ |s| s[:visits_number] }.sum, expanded_vrp[:services].size
-    assert_equal ['1_f_2'], expanded_vrp[:services][0].skills
-    assert_equal ['2_f_2'], expanded_vrp[:services][1].skills
+    assert_equal [:"1_f_2"], expanded_vrp[:services][0].skills
+    assert_equal [:"2_f_2"], expanded_vrp[:services][1].skills
   end
 
   def test_expand_vrp_schedule_range_date
@@ -51,8 +51,8 @@ def test_expand_vrp_schedule_range_date
     expanded_vrp = periodic_expand(problem)
     assert_equal number_of_days, expanded_vrp[:vehicles].size
     assert_equal problem[:services].collect{ |s| s[:visits_number] }.sum, expanded_vrp[:services].size
-    assert_equal ['1_f_2'], expanded_vrp[:services][0].skills
-    assert_equal ['2_f_2'], expanded_vrp[:services][1].skills
+    assert_equal [:"1_f_2"], expanded_vrp[:services][0].skills
+    assert_equal [:"2_f_2"], expanded_vrp[:services][1].skills
   end
 
   def test_generated_service_timewindows_after_periodic_expand

test/models/service_test.rb

@@ -0,0 +1,56 @@
+# Copyright © Mapotempo, 2022
+#
+# This file is part of Mapotempo.
+#
+# Mapotempo is free software. You can redistribute it and/or
+# modify since you respect the terms of the GNU Affero General
+# Public License as published by the Free Software Foundation,
+# either version 3 of the License, or (at your option) any later version.
+#
+# Mapotempo is distributed in the hope that it will be useful, but WITHOUT
+# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY
+# or FITNESS FOR A PARTICULAR PURPOSE.  See the Licenses for more details.
+#
+# You should have received a copy of the GNU Affero General Public License
+# along with Mapotempo. If not, see:
+# <http://www.gnu.org/licenses/agpl.html>
+#
+require './test/test_helper'
+
+module Models
+  class ServiceTest < Minitest::Test
+    def test_skills
+      Models.delete_all
+      service1 = { id: 'service_1' }
+      service2 = { id: 'service_2' }
+
+      s1 = Models::Service.create(service1)
+      s2 = Models::Service.create(service2)
+
+      refute_equal s1.skills.object_id, s2.skills.object_id
+
+      problem = { services: [service1, service2] }
+
+      vrp = Models::Vrp.create(problem)
+
+      refute_equal vrp.services.first.skills.object_id,
+                   vrp.services.last.skills.object_id
+
+      Models.delete_all
+      service1[:skills] = nil
+      service2[:skills] = nil
+
+      s1 = Models::Service.create(service1)
+      s2 = Models::Service.create(service2)
+
+      refute_equal s1.skills.object_id, s2.skills.object_id
+
+      problem = { services: [service1, service2] }
+
+      vrp = Models::Vrp.create(problem)
+
+      refute_equal vrp.services.first.skills.object_id,
+                   vrp.services.last.skills.object_id
+    end
+  end
+end

test/models/vehicle_test.rb

@@ -72,5 +72,56 @@ def test_total_work_time_in_range
       vrp.vehicles.first.sequence_timewindows << Models::Timewindow.create({ start: 5, end: 6, day_index: 0 })
       assert_equal 13, vrp.vehicles.first.total_work_time_in_range(0, 3)
     end
+
+    def test_skills
+      Models.delete_all
+      vehicle1 = { id: 'vehicle_1' }
+      vehicle2 = { id: 'vehicle_2' }
+
+      v1 = Models::Vehicle.create(vehicle1)
+      v2 = Models::Vehicle.create(vehicle2)
+
+      refute_equal v1.skills.object_id, v2.skills.object_id
+      refute_equal v1.skills.first.object_id, v2.skills.first.object_id
+
+      problem = { vehicles: [vehicle1, vehicle2] }
+
+      vrp = Models::Vrp.create(problem)
+
+      refute_equal vrp.vehicles.first.skills.object_id,
+                   vrp.vehicles.last.skills.object_id
+
+      refute_equal vrp.vehicles.first.skills.first.object_id,
+                   vrp.vehicles.last.skills.first.object_id
+
+      Models.delete_all
+      vehicle1[:skills] = nil
+      vehicle2[:skills] = nil
+
+      v1 = Models::Vehicle.create(vehicle1)
+      v2 = Models::Vehicle.create(vehicle2)
+
+      refute_equal v1.skills.object_id, v2.skills.object_id
+      refute_equal v1.skills.first.object_id, v2.skills.first.object_id
+
+      problem = { vehicles: [vehicle1, vehicle2] }
+
+      vrp = Models::Vrp.create(problem)
+
+      refute_equal vrp.vehicles.first.skills.object_id,
+                   vrp.vehicles.last.skills.object_id
+
+      refute_equal vrp.vehicles.first.skills.first.object_id,
+                   vrp.vehicles.last.skills.first.object_id
+    end
+
+    def test_symbol_skills
+      vehicle1 = { id: 'vehicle_1', skills: [['string']] }
+
+      v1 = Models::Vehicle.create(vehicle1)
+
+      assert v1.skills.first.first.is_a?(Symbol)
+      assert v1.as_json[:skills].first.first.is_a?(Symbol)
+    end
   end
 end