fix(proxy): return implementing modules in the system's module order (PPT-2816)

spec/drivers_proxy_spec.cr

@@ -33,6 +33,39 @@ module PlaceOS::Driver::Proxy
       responses.get.should eq Array(JSON::Any).from_json("[]")
     end
 
+    it "returns implementing modules in the system's module order" do
+      cs = PlaceOS::Driver::DriverModel::ControlSystem.from_json(%(
+          {
+            "id": "sys-order-1",
+            "name": "Ordered System",
+            "capacity": 0,
+            "bookable": false,
+            "zones": ["zone-1234"],
+            "modules": ["mod-first", "mod-second", "mod-third"]
+          }
+      ))
+      system = PlaceOS::Driver::Proxy::System.new cs, "reply_id"
+
+      # the redis hash was written in a different order to the system's list
+      storage = PlaceOS::Driver::RedisStorage.new(cs.id, "system")
+      storage.clear
+      storage["Calendar/1"] = "mod-third"
+      storage["Mailer/1"] = "mod-first"
+      storage["Mailer/2"] = "mod-second"
+      storage["Extra/1"] = "mod-unlisted"
+
+      redis = PlaceOS::Driver::RedisStorage.new_redis_client
+      meta = PlaceOS::Driver::DriverModel::Metadata.new({
+        "send_mail" => {} of String => JSON::Any,
+      }, ["Mailer"])
+      {"mod-first", "mod-second", "mod-third", "mod-unlisted"}.each { |id| redis.set("interface/#{id}", meta.to_json) }
+
+      system.implementing(:Mailer).map(&.module_id).should eq(["mod-first", "mod-second", "mod-third", "mod-unlisted"])
+      system.all(:Mailer).map(&.index).should eq([1, 2])
+
+      storage.clear
+    end
+
     it "should execute functions on collections of remote drivers" do
       cs = PlaceOS::Driver::DriverModel::ControlSystem.from_json(%(
           {

src/placeos-driver/driver_model.cr

@@ -20,6 +20,8 @@ struct PlaceOS::Driver::DriverModel
     property timezone : String?
     property support_url : String?
     property zones : Array(String)
+    # module ids in the order they are listed on the system
+    property modules : Array(String) = [] of String
     property images : Array(String)?
     property security_groups : Array(String)?
   end

src/placeos-driver/proxy/system.cr

@@ -105,7 +105,7 @@ struct PlaceOS::Driver::Proxy::System
       end
     end
 
-    PlaceOS::Driver::Proxy::Drivers.new(drivers)
+    PlaceOS::Driver::Proxy::Drivers.new(drivers.sort_by!(&.index))
   end
 
   def all(module_name, *, implementing) : PlaceOS::Driver::Proxy::Drivers
@@ -125,7 +125,7 @@ struct PlaceOS::Driver::Proxy::System
       drivers << Proxy::Driver.new(@reply_id, mod_name, index.to_i, module_id, self, metadata)
     end
 
-    PlaceOS::Driver::Proxy::Drivers.new(drivers)
+    PlaceOS::Driver::Proxy::Drivers.new(drivers.sort_by!(&.index))
   end
 
   private def get_metadata(module_id : String?) : DriverModel::Metadata
@@ -140,6 +140,9 @@ struct PlaceOS::Driver::Proxy::System
   end
 
   # grabs all modules implementing(Powerable) for example
+  #
+  # Modules are returned in the order they are listed on the system, so the
+  # first result is the module an administrator placed first
   def implementing(interface) : PlaceOS::Driver::Proxy::Drivers
     interface = interface.to_s
     drivers = [] of Proxy::Driver
@@ -155,7 +158,24 @@ struct PlaceOS::Driver::Proxy::System
       drivers << Proxy::Driver.new(@reply_id, mod_name, index.to_i, module_id, self, metadata)
     end
 
-    PlaceOS::Driver::Proxy::Drivers.new(drivers)
+    PlaceOS::Driver::Proxy::Drivers.new(in_system_order(drivers))
+  end
+
+  # The redis hash holding the module mappings has no defined order, so sort by
+  # the system's module list. Modules missing from the list keep a stable
+  # name and index order after those found.
+  private def in_system_order(drivers : Array(Proxy::Driver)) : Array(Proxy::Driver)
+    return drivers if drivers.size < 2
+    order = begin
+      config.modules
+    rescue error
+      logger.warn(exception: error) { "unable to load the module order for system #{@system_id}" }
+      [] of String
+    end
+    drivers.sort_by! do |driver|
+      position = order.index(driver.module_id) || Int32::MAX
+      {position, driver.module_name, driver.index}
+    end
   end
 
   # coordination to occur on placeos core