From 91b5a886e078f588b22527070d2261c4a13b4060 Mon Sep 17 00:00:00 2001 From: Stephen von Takach Date: Tue, 17 Feb 2026 15:29:24 +1100 Subject: [PATCH 1/4] fix(modules): optimize queries and include live connected state --- spec/controllers/modules_spec.cr | 22 ----------- src/placeos-rest-api/controllers/modules.cr | 44 +++++++++++++++------ 2 files changed, 32 insertions(+), 34 deletions(-) diff --git a/spec/controllers/modules_spec.cr b/spec/controllers/modules_spec.cr index b7c090d3..2f8cd041 100644 --- a/spec/controllers/modules_spec.cr +++ b/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) diff --git a/src/placeos-rest-api/controllers/modules.cr b/src/placeos-rest-api/controllers/modules.cr index 222d2f02..1e3135b0 100644 --- a/src/placeos-rest-api/controllers/modules.cr +++ b/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 + 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 + d.control_system_details = ControlSystemDetails.new(sys.name, ::PlaceOS::Model::Zone.find_all(sys.zones).to_a) + end current_module else current_module From ec0b0fc9cd55add16165490ef449f75336767518 Mon Sep 17 00:00:00 2001 From: Stephen von Takach Date: Tue, 17 Feb 2026 15:57:02 +1100 Subject: [PATCH 2/4] fix build --- src/placeos-rest-api/controllers/modules.cr | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/placeos-rest-api/controllers/modules.cr b/src/placeos-rest-api/controllers/modules.cr index 1e3135b0..597690a0 100644 --- a/src/placeos-rest-api/controllers/modules.cr +++ b/src/placeos-rest-api/controllers/modules.cr @@ -249,7 +249,7 @@ module PlaceOS::Api 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 - d.control_system_details = ControlSystemDetails.new(sys.name, ::PlaceOS::Model::Zone.find_all(sys.zones).to_a) + current_module.control_system_details = ControlSystemDetails.new(sys.name, ::PlaceOS::Model::Zone.find_all(sys.zones).to_a) end current_module else From 589f24a44139638af23e390db6641cad9c5771b8 Mon Sep 17 00:00:00 2001 From: Stephen von Takach Date: Tue, 17 Feb 2026 16:26:12 +1100 Subject: [PATCH 3/4] improve system show?complete performance --- src/placeos-rest-api/controllers/modules.cr | 2 +- src/placeos-rest-api/controllers/systems.cr | 21 +++++++++++++++------ 2 files changed, 16 insertions(+), 7 deletions(-) diff --git a/src/placeos-rest-api/controllers/modules.cr b/src/placeos-rest-api/controllers/modules.cr index 597690a0..a32b4f74 100644 --- a/src/placeos-rest-api/controllers/modules.cr +++ b/src/placeos-rest-api/controllers/modules.cr @@ -202,7 +202,7 @@ module PlaceOS::Api search_results.compact_map do |d| sys_id = d.control_system_id sys = sys_id ? control_systems.find { |csys| csys.id == sys_id } : nil - d_id = d.driver_id + d_id = d.driver_id.as(String) driver = drivers.find { |drive| drive.id == d_id } next unless driver diff --git a/src/placeos-rest-api/controllers/systems.cr b/src/placeos-rest-api/controllers/systems.cr index 03cfe42e..9eb85327 100644 --- a/src/placeos-rest-api/controllers/systems.cr +++ b/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 From f57107b0f82bca6316c54cc38f26994b13e46c96 Mon Sep 17 00:00:00 2001 From: Stephen von Takach Date: Wed, 18 Feb 2026 10:24:07 +1100 Subject: [PATCH 4/4] chore(ameba): fix warnings --- .ameba.yml | 2 ++ src/placeos-rest-api/controllers/webhook.cr | 2 +- src/placeos-rest-api/utilities/current-user.cr | 1 - 3 files changed, 3 insertions(+), 2 deletions(-) diff --git a/.ameba.yml b/.ameba.yml index 39924a3a..5c6312d2 100644 --- a/.ameba.yml +++ b/.ameba.yml @@ -18,6 +18,8 @@ Lint/SpecFilename: Enabled: false Lint/UselessAssign: Enabled: false +Metrics/CyclomaticComplexity: + Enabled: false Documentation/DocumentationAdmonition: Enabled: false diff --git a/src/placeos-rest-api/controllers/webhook.cr b/src/placeos-rest-api/controllers/webhook.cr index da5d36a4..03f91afb 100644 --- a/src/placeos-rest-api/controllers/webhook.cr +++ b/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}" diff --git a/src/placeos-rest-api/utilities/current-user.cr b/src/placeos-rest-api/utilities/current-user.cr index 6dc5fcd6..03c2ac00 100644 --- a/src/placeos-rest-api/utilities/current-user.cr +++ b/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