fix: module connected state

.ameba.yml

@@ -18,6 +18,8 @@ Lint/SpecFilename:
   Enabled: false
 Lint/UselessAssign:
   Enabled: false
+Metrics/CyclomaticComplexity:
+  Enabled: false
 
 Documentation/DocumentationAdmonition:
   Enabled: false

spec/controllers/modules_spec.cr

@@ -245,28 +245,6 @@ module PlaceOS::Api
           found.should be_true
         end
 
-        it "connected" do
-          mod = Model::Generator.module
-          mod.ignore_connected = false
-          mod.connected = true
-          mod.save!
-          mod.persisted?.should be_true
-
-          params = HTTP::Params.encode({"connected" => "true"})
-          path = "#{Modules.base_route}?#{params}"
-
-          found = until_expected("GET", path, Spec::Authentication.headers) do |response|
-            results = Array(Hash(String, JSON::Any)).from_json(response.body)
-
-            all_connected = results.all? { |r| r["connected"].as_bool == true }
-            contains_created = results.any? { |r| r["id"].as_s == mod.id }
-
-            !results.empty? && all_connected && contains_created
-          end
-
-          found.should be_true
-        end
-
         it "no_logic" do
           driver = Model::Generator.driver(role: Model::Driver::Role::Service).save!
           mod = Model::Generator.module(driver: driver)

src/placeos-rest-api/controllers/modules.cr

@@ -107,8 +107,6 @@ module PlaceOS::Api
       as_of : Int64? = nil,
       @[AC::Param::Info(description: "only return modules running in this system (query params are ignored if this is provided)", example: "sys-1234")]
       control_system_id : String? = nil,
-      @[AC::Param::Info(description: "only return modules with a particular connected state", example: "true")]
-      connected : Bool? = nil,
       @[AC::Param::Info(description: "only return instances of this driver", example: "driver-1234")]
       driver_id : String? = nil,
       @[AC::Param::Info(description: "do not return logic modules (return only modules that can exist in multiple systems)", example: "true")]
@@ -177,13 +175,6 @@ module PlaceOS::Api
         query.filter({"driver_id" => [driver_id]})
       end
 
-      unless connected.nil?
-        query.filter({
-          "ignore_connected" => [false],
-          "connected"        => [connected],
-        })
-      end
-
       unless running.nil?
         query.should({"running" => [running]})
       end
@@ -201,21 +192,39 @@ module PlaceOS::Api
       search_results = paginate_results(elastic, query)
 
       # Include subset of association data with results
+      # avoid n+1 requests
+      control_system_ids = search_results.compact_map(&.control_system_id).uniq!
+      drivers = Model::Driver.find_all search_results.map(&.driver_id.as(String)).uniq!
+
+      control_systems = Model::ControlSystem.find_all(control_system_ids).to_a
+      zones = Model::Zone.find_all(control_systems.flat_map(&.zones)).to_a
+
       search_results.compact_map do |d|
-        sys = d.control_system
-        driver = d.driver
+        sys_id = d.control_system_id
+        sys = sys_id ? control_systems.find { |csys| csys.id == sys_id } : nil
+        d_id = d.driver_id.as(String)
+        driver = drivers.find { |drive| drive.id == d_id }
         next unless driver
 
         # Include control system on Logic modules so it is possible
         # to display the inherited settings
         sys_field = if sys
-                      ControlSystemDetails.new(sys.name, ::PlaceOS::Model::Zone.find_all(sys.zones).to_a)
+                      ControlSystemDetails.new(sys.name, sys.zones.compact_map { |zid| zones.find { |zone| zone.id == zid } })
                     else
                       nil
                     end
 
         d.control_system_details = sys_field
         d.driver_details = DriverDetails.new(driver.name, driver.description, driver.module_name)
+
+        # grab connected state from redis
+        d.connected = if d.running
+                        storage = Driver::RedisStorage.new(d.id.as(String))
+                        storage["connected"]? != "false"
+                      else
+                        true
+                      end
+
         d
       end
     end
@@ -229,8 +238,19 @@ module PlaceOS::Api
       running_on = self.class.locate_module(current_module.id.as(String)) rescue nil
       current_module.core_node = running_on
 
+      # grab connected state from redis
+      current_module.connected = if current_module.running
+                                   storage = Driver::RedisStorage.new(current_module.id.as(String))
+                                   storage["connected"]? != "false"
+                                 else
+                                   true
+                                 end
+
       if complete && (driver = current_module.driver)
         current_module.driver_details = DriverDetails.new(driver.name, driver.description, driver.module_name)
+        if sys = current_module.control_system
+          current_module.control_system_details = ControlSystemDetails.new(sys.name, ::PlaceOS::Model::Zone.find_all(sys.zones).to_a)
+        end
         current_module
       else
         current_module

src/placeos-rest-api/controllers/systems.cr

@@ -267,12 +267,21 @@ module PlaceOS::Api
         sys.zone_data_details = ::PlaceOS::Model::Zone.find_all(current_control_system.zones).to_a
 
         # extend the module details with the driver details
-        modules = ::PlaceOS::Model::Module.find_all(current_control_system.modules).to_a.map do |mod|
-          # Pick off driver name, and module_name from associated driver
-          mod.driver_details = mod.driver.try do |driver|
-            Api::Modules::DriverDetails.new(driver.name, driver.description, driver.module_name)
-          end
-          mod
+        modules = ::PlaceOS::Model::Module.find_all(current_control_system.modules).to_a
+        drivers = Model::Driver.find_all modules.map(&.driver_id.as(String)).uniq!
+        modules.select! do |mod|
+          d_id = mod.driver_id.as(String)
+          driver = drivers.find { |drive| drive.id == d_id }
+          next unless driver
+
+          mod.connected = if mod.running
+                            storage = Driver::RedisStorage.new(mod.id.as(String))
+                            storage["connected"]? != "false"
+                          else
+                            true
+                          end
+
+          mod.driver_details = Api::Modules::DriverDetails.new(driver.name, driver.description, driver.module_name)
         end
         sys.module_data_details = modules
 

src/placeos-rest-api/controllers/webhook.cr

@@ -82,7 +82,7 @@ module PlaceOS::Api
     alias RemoteDriver = ::PlaceOS::Driver::Proxy::RemoteDriver
 
     # Triggers the webhook
-    def notify(method_type : String) # ameba:disable Metrics/CyclomaticComplexity
+    def notify(method_type : String)
       # Notify the trigger service
       trigger_uri = TRIGGERS_URI.dup
       trigger_uri.path = "/api/triggers/v2/webhook/#{current_trigger_instance.id}?secret=#{current_trigger_instance.webhook_secret}"

src/placeos-rest-api/utilities/current-user.cr

@@ -14,7 +14,6 @@ module PlaceOS::Api
     # Parses, and validates JWT if present.
     # Throws Error::MissingBearer and JWT::Error.
 
-    # ameba:disable Metrics/CyclomaticComplexity
     def authorize! : ::PlaceOS::Model::UserJWT
       if token = @user_token
         return token