diff --git a/shard.lock b/shard.lock index 562ae699..10d8ba63 100644 --- a/shard.lock +++ b/shard.lock @@ -223,7 +223,7 @@ shards: placeos-models: git: https://github.com/placeos/models.git - version: 9.118.1 + version: 9.119.0 placeos-resource: git: https://github.com/place-labs/resource.git diff --git a/spec/controllers/asset_categories_spec.cr b/spec/controllers/asset_categories_spec.cr index 7e905c71..ac8aa1e6 100644 --- a/spec/controllers/asset_categories_spec.cr +++ b/spec/controllers/asset_categories_spec.cr @@ -134,8 +134,6 @@ module PlaceOS::Api end describe "support-subsystem permissions" do - ::Spec.before_each { clear_group_tables } - it "allows POST for a support user with Create on the org zone (both sides)" do authority = Model::Authority.find_by_domain("localhost").not_nil! user, headers = Spec::Authentication.authentication(sys_admin: false, support: false) diff --git a/spec/controllers/asset_purchase_orders_spec.cr b/spec/controllers/asset_purchase_orders_spec.cr index f8ad5fe2..c9bd5904 100644 --- a/spec/controllers/asset_purchase_orders_spec.cr +++ b/spec/controllers/asset_purchase_orders_spec.cr @@ -74,8 +74,6 @@ module PlaceOS::Api end describe "support-subsystem permissions" do - ::Spec.before_each { clear_group_tables } - it "allows POST for a support user with Create on the org zone (both sides)" do authority = Model::Authority.find_by_domain("localhost").not_nil! user, headers = Spec::Authentication.authentication(sys_admin: false, support: false) diff --git a/spec/controllers/asset_types_spec.cr b/spec/controllers/asset_types_spec.cr index c5de356d..b84ed38d 100644 --- a/spec/controllers/asset_types_spec.cr +++ b/spec/controllers/asset_types_spec.cr @@ -91,8 +91,6 @@ module PlaceOS::Api end describe "support-subsystem permissions" do - ::Spec.before_each { clear_group_tables } - it "allows POST for a support user with Create on the org zone (both sides)" do authority = Model::Authority.find_by_domain("localhost").not_nil! user, headers = Spec::Authentication.authentication(sys_admin: false, support: false) diff --git a/spec/controllers/assets_spec.cr b/spec/controllers/assets_spec.cr index 51c64eb0..24710474 100644 --- a/spec/controllers/assets_spec.cr +++ b/spec/controllers/assets_spec.cr @@ -247,8 +247,6 @@ module PlaceOS::Api end describe "support-subsystem permissions" do - ::Spec.before_each { clear_group_tables } - # Build an Asset (unsaved) whose `zone_id` is the supplied zone. build_asset = ->(zone : Model::Zone) { asset_type = Model::Generator.asset_type.save! diff --git a/spec/controllers/chat_gpt_plugin_spec.cr b/spec/controllers/chat_gpt_plugin_spec.cr index 392a0f19..b2359558 100644 --- a/spec/controllers/chat_gpt_plugin_spec.cr +++ b/spec/controllers/chat_gpt_plugin_spec.cr @@ -33,7 +33,9 @@ module PlaceOS::Api driver = Model::Generator.driver(role: Model::Driver::Role::Logic) driver.module_name = module_name driver.save! + # no custom name, so core maps the module by its driver's module_name mod = Model::Generator.module(driver: driver) + mod.custom_name = nil mod.running = true mod.save! @@ -55,14 +57,8 @@ module PlaceOS::Api system.save! sys_id = system.id.as(String) - # the lookups core maintains. Core rebuilds them asynchronously when systems change, - # which can replace seeded entries, so they're seeded before each call - seed_lookups = -> { - lookup = ::PlaceOS::Driver::RedisStorage.new(sys_id, "system") - lookup["LLM/1"] = llm_id - lookup["Meet/1"] = meet_ids[0] - lookup["Meet/2"] = meet_ids[1] - } + # the module lookups the plugin resolves capabilities through, as core maps them + wait_for_module_lookups(sys_id, {"LLM/1" => llm_id, "Meet/1" => meet_ids[0], "Meet/2" => meet_ids[1]}) ::PlaceOS::Driver::RedisStorage.new(llm_id)["prompt"] = { prompt: "You help people use the boardroom", @@ -74,7 +70,6 @@ module PlaceOS::Api ].to_json _, credentials = Spec::Authentication.authentication - seed_lookups.call headers, result = open_session.call(sys_id, credentials) result["serverInfo"]["name"].should eq "chat_gpt_plugin" @@ -92,13 +87,11 @@ module PlaceOS::Api # every tool is hinted read only, so clients don't ask for confirmation tools.all?(&.["annotations"]["readOnlyHint"].as_bool).should be_true - seed_lookups.call capabilities = call_tool.call(sys_id, headers, "capabilities", {} of String => JSON::Any) capabilities["isError"].should be_false capabilities["structuredContent"]["body"]["prompt"].should eq "You help people use the boardroom" capabilities["structuredContent"]["body"]["system_id"].should eq sys_id - seed_lookups.call schema = call_tool.call(sys_id, headers, "function_schema", {"capability_id" => JSON::Any.new("Meet_2")}) schema["structuredContent"]["body"][0]["function"].should eq "set_power" @@ -108,7 +101,6 @@ module PlaceOS::Api body: "true", ) [JSON.parse(%({"state": true})), JSON::Any.new(%({"state": true}))].each do |function_params| - seed_lookups.call called = call_tool.call(sys_id, headers, "call_function", { "capability_id" => JSON::Any.new("Meet_2"), "function_name" => JSON::Any.new("set_power"), diff --git a/spec/controllers/edges_spec.cr b/spec/controllers/edges_spec.cr index d6b7fbba..2822477d 100644 --- a/spec/controllers/edges_spec.cr +++ b/spec/controllers/edges_spec.cr @@ -42,7 +42,7 @@ module PlaceOS::Api end describe "CRUD operations", tags: "crud" do - ::Spec.before_each do + before_each do PlaceOS::Model::Edge.clear end Spec.test_crd(Model::Edge, Edges) @@ -68,7 +68,7 @@ module PlaceOS::Api end describe "monitoring endpoints" do - ::Spec.before_each do + before_each do PlaceOS::Model::Edge.clear end diff --git a/spec/controllers/groups/history_spec.cr b/spec/controllers/groups/history_spec.cr index 7b05abbf..82f913ae 100644 --- a/spec/controllers/groups/history_spec.cr +++ b/spec/controllers/groups/history_spec.cr @@ -4,8 +4,6 @@ module PlaceOS::Api describe Groups::History do base = Groups::History.base_route - ::Spec.before_each { clear_group_tables } - it "sys_admin can list all history; non-admin must scope to a managed group" do authority = Model::Authority.find_by_domain("localhost").not_nil! admin = Spec::Authentication.user(sys_admin: true) diff --git a/spec/controllers/groups/invitation_spec.cr b/spec/controllers/groups/invitation_spec.cr index 176d1052..14150bc1 100644 --- a/spec/controllers/groups/invitation_spec.cr +++ b/spec/controllers/groups/invitation_spec.cr @@ -4,8 +4,6 @@ module PlaceOS::Api describe Groups::Invitations do base = Groups::Invitations.base_route - ::Spec.before_each { clear_group_tables } - it "manager can create and destroy invitations; response includes plaintext_secret once" do authority = Model::Authority.find_by_domain("localhost").not_nil! manager, manager_headers = Spec::Authentication.authentication(sys_admin: false, support: false) diff --git a/spec/controllers/groups/user_spec.cr b/spec/controllers/groups/user_spec.cr index 41241b9e..445cd3ba 100644 --- a/spec/controllers/groups/user_spec.cr +++ b/spec/controllers/groups/user_spec.cr @@ -4,8 +4,6 @@ module PlaceOS::Api describe Groups::Users do base = Groups::Users.base_route - ::Spec.before_each { clear_group_tables } - it "manager can add and remove a user on their group" do authority = Model::Authority.find_by_domain("localhost").not_nil! manager, manager_headers = Spec::Authentication.authentication(sys_admin: false, support: false) diff --git a/spec/controllers/groups/zone_spec.cr b/spec/controllers/groups/zone_spec.cr index 9951ae6d..5722528a 100644 --- a/spec/controllers/groups/zone_spec.cr +++ b/spec/controllers/groups/zone_spec.cr @@ -4,8 +4,6 @@ module PlaceOS::Api describe Groups::Zones do base = Groups::Zones.base_route - ::Spec.before_each { clear_group_tables } - it "manager can delegate a zone they already have coverage on" do authority = Model::Authority.find_by_domain("localhost").not_nil! manager, manager_headers = Spec::Authentication.authentication(sys_admin: false, support: false) diff --git a/spec/controllers/groups_spec.cr b/spec/controllers/groups_spec.cr index 8fc02498..4e5c7488 100644 --- a/spec/controllers/groups_spec.cr +++ b/spec/controllers/groups_spec.cr @@ -4,8 +4,6 @@ module PlaceOS::Api describe Groups do base = Groups.base_route - ::Spec.before_each { clear_group_tables } - it "sys_admin can create a root group; non-admin cannot" do authority = Model::Authority.find_by_domain("localhost").not_nil! payload = Model::Generator.group(authority: authority).to_json diff --git a/spec/controllers/metadata_spec.cr b/spec/controllers/metadata_spec.cr index 9e0113e7..b31398e4 100644 --- a/spec/controllers/metadata_spec.cr +++ b/spec/controllers/metadata_spec.cr @@ -478,8 +478,6 @@ module PlaceOS::Api end describe "support subsystem permissions" do - ::Spec.before_each { clear_group_tables } - # ---------------------------------------------------------------- # 1. zone parent: PATCH needs Update on both sides # ---------------------------------------------------------------- diff --git a/spec/controllers/modules_spec.cr b/spec/controllers/modules_spec.cr index cbaab281..f69f44c7 100644 --- a/spec/controllers/modules_spec.cr +++ b/spec/controllers/modules_spec.cr @@ -3,7 +3,7 @@ require "timecop" module PlaceOS::Api describe Modules do - ::Spec.before_each do + before_each do Model::Module.clear Model::ControlSystem.clear end @@ -467,8 +467,6 @@ module PlaceOS::Api end describe "support-subsystem permissions" do - ::Spec.before_each { clear_group_tables } - # Build a logic module attached to a control system whose only zone is # `zone`. The module derives its zones from that system, so granting on # `zone` (via a "support" GroupZone) gates access to the module. diff --git a/spec/controllers/oauth_applications_spec.cr b/spec/controllers/oauth_applications_spec.cr index 3e46373e..946eaa03 100644 --- a/spec/controllers/oauth_applications_spec.cr +++ b/spec/controllers/oauth_applications_spec.cr @@ -12,7 +12,7 @@ module PlaceOS::Api describe OAuthApplications do base = OAuthApplications.base_route - ::Spec.before_each do + before_each do clear_group_tables Model::DoorkeeperApplication.clear end diff --git a/spec/controllers/pending_mails_spec.cr b/spec/controllers/pending_mails_spec.cr index 76f8d7df..ebb87c89 100644 --- a/spec/controllers/pending_mails_spec.cr +++ b/spec/controllers/pending_mails_spec.cr @@ -403,8 +403,6 @@ module PlaceOS::Api end describe "subsystem-based permissions" do - ::Spec.before_each { clear_group_tables } - it "allows a support-subsystem user with Delete reach on a mail's zone to destroy it" do authority = Model::Authority.find_by_domain("localhost").not_nil! user, headers = Spec::Authentication.authentication(sys_admin: false, support: false) diff --git a/spec/controllers/public_events_spec.cr b/spec/controllers/public_events_spec.cr index 7a0756a0..b9304849 100644 --- a/spec/controllers/public_events_spec.cr +++ b/spec/controllers/public_events_spec.cr @@ -10,7 +10,7 @@ module PlaceOS::Api end describe PublicEvents, tags: "public_events" do - ::Spec.before_each do + before_each do Model::ControlSystem.clear Model::Driver.clear Model::Module.clear @@ -255,24 +255,26 @@ module PlaceOS::Api end it "delegates to the driver and returns its result when the module is present" do - system = PlaceOS::Api.public_control_system - driver = Model::Generator.driver(role: Model::Driver::Role::Logic) driver.module_name = "PublicEvents" driver.save! + # no custom name, so core maps the module as "PublicEvents/1" mod = Model::Generator.module(driver: driver) + mod.custom_name = nil mod.running = true mod.save! - system.modules = [mod.id.as(String)] - system.save! - module_id = mod.id.as(String) + + # saved once with its module, so core maps the system from a single event + system = Model::Generator.control_system + system.public = true + system.modules = [module_id] + system.save! sys_id = system.id.as(String) - # Seed the system-to-module lookup so Proxy::System.module_id? resolves. - system_storage = ::PlaceOS::Driver::RedisStorage.new(sys_id, "system") - system_storage["PublicEvents/1"] = module_id + # Proxy::System.module_id? resolves through the lookup core maintains + wait_for_module_lookups(sys_id, {"PublicEvents/1" => module_id}) # Seed the driver interface metadata so RemoteDriver.metadata? resolves # and function_present?("register_attendee") returns true. @@ -304,7 +306,6 @@ module PlaceOS::Api result.status_code.should eq 200 result.body.should eq "true" - system_storage.delete("PublicEvents/1") ::PlaceOS::Driver::RedisStorage.with_redis(&.del("interface/#{module_id}")) end end diff --git a/spec/controllers/settings_spec.cr b/spec/controllers/settings_spec.cr index 48219ea3..ce348b97 100644 --- a/spec/controllers/settings_spec.cr +++ b/spec/controllers/settings_spec.cr @@ -223,8 +223,6 @@ module PlaceOS::Api end describe "support subsystem permissions" do - ::Spec.before_each { clear_group_tables } - # ---------------------------------------------------------------- # 1. show / index(?parent_id=zone-…) gated on Read # ---------------------------------------------------------------- diff --git a/spec/controllers/signage/playlist_media_spec.cr b/spec/controllers/signage/playlist_media_spec.cr index 4da3b664..e5aee8b6 100644 --- a/spec/controllers/signage/playlist_media_spec.cr +++ b/spec/controllers/signage/playlist_media_spec.cr @@ -24,7 +24,7 @@ module PlaceOS::Api describe PlaylistMedia do base = PlaylistMedia.base_route - ::Spec.before_each do + before_each do Model::Playlist::Item.clear Model::Playlist.clear clear_group_tables diff --git a/spec/controllers/signage/playlists_spec.cr b/spec/controllers/signage/playlists_spec.cr index 004cfe20..523035d1 100644 --- a/spec/controllers/signage/playlists_spec.cr +++ b/spec/controllers/signage/playlists_spec.cr @@ -24,7 +24,7 @@ module PlaceOS::Api describe Playlist do base = Playlist.base_route - ::Spec.before_each do + before_each do Model::Playlist::ItemSchedule.clear Model::Playlist::Revision.clear Model::Playlist::Item.clear @@ -772,7 +772,7 @@ module PlaceOS::Api end describe "approval requests" do - ::Spec.before_each { Model::PendingMail.clear } + before_each { Model::PendingMail.clear } describe "GET /approvers" do it "returns approve and manage users (not read-only members)" do diff --git a/spec/controllers/signage/signage_ai_providers_spec.cr b/spec/controllers/signage/signage_ai_providers_spec.cr index cbaef3b9..9dfbc2eb 100644 --- a/spec/controllers/signage/signage_ai_providers_spec.cr +++ b/spec/controllers/signage/signage_ai_providers_spec.cr @@ -10,6 +10,11 @@ module PlaceOS::Api before_each do Model::SignageAIJob.clear Model::SignageAIProvider.clear + # `setup_signage_ai` creates the domain's storage, which has to be the + # only one (a leftover can collide with it, or be picked as the default) + Model::Playlist::Item.clear + Model::Upload.clear + Model::Storage.clear WebMock.allow_net_connect = false end diff --git a/spec/controllers/signage/template_mappings_spec.cr b/spec/controllers/signage/template_mappings_spec.cr index 8f6c74f0..eb4fcff2 100644 --- a/spec/controllers/signage/template_mappings_spec.cr +++ b/spec/controllers/signage/template_mappings_spec.cr @@ -7,7 +7,7 @@ module PlaceOS::Api # NOTE: don't clear Zone here — the shared auth helper's org zone # ("zone-perm-org") must survive between examples (clearing it orphans # its permissions Metadata and later re-creation collides). - ::Spec.before_each do + before_each do Model::SignageTemplate::SystemTemplate.clear Model::SignageTemplate.clear Model::Playlist::Item.clear diff --git a/spec/controllers/signage/templates_spec.cr b/spec/controllers/signage/templates_spec.cr index b9807882..357391fe 100644 --- a/spec/controllers/signage/templates_spec.cr +++ b/spec/controllers/signage/templates_spec.cr @@ -75,7 +75,7 @@ module PlaceOS::Api describe SignageTemplates do base = SignageTemplates.base_route - ::Spec.before_each do + before_each do Model::SignageTemplate::SystemTemplate.clear Model::SignageTemplate.clear clear_group_tables @@ -1041,7 +1041,7 @@ module PlaceOS::Api end describe "approval requests" do - ::Spec.before_each { Model::PendingMail.clear } + before_each { Model::PendingMail.clear } describe "GET /approvers" do it "returns approve and manage users (not read-only members)" do diff --git a/spec/controllers/signage_plugins_spec.cr b/spec/controllers/signage_plugins_spec.cr index 7bb5123c..c93c5115 100644 --- a/spec/controllers/signage_plugins_spec.cr +++ b/spec/controllers/signage_plugins_spec.cr @@ -2,7 +2,7 @@ require "../helper" module PlaceOS::Api describe SignagePlugins do - ::Spec.before_each do + before_each do Model::SignagePlugin.clear end diff --git a/spec/controllers/signage_spec.cr b/spec/controllers/signage_spec.cr index a1005620..09ba8439 100644 --- a/spec/controllers/signage_spec.cr +++ b/spec/controllers/signage_spec.cr @@ -3,7 +3,7 @@ require "timecop" module PlaceOS::Api describe Signage do - ::Spec.before_each do + before_each do Model::SignageTemplate::SystemTemplate.clear Model::SignageTemplate.clear Model::Playlist::ItemSchedule.clear diff --git a/spec/controllers/system-triggers_spec.cr b/spec/controllers/system-triggers_spec.cr index 1a3b4310..a5a21e7e 100644 --- a/spec/controllers/system-triggers_spec.cr +++ b/spec/controllers/system-triggers_spec.cr @@ -213,8 +213,6 @@ module PlaceOS::Api end describe "support-subsystem permissions" do - ::Spec.before_each { clear_group_tables } - # A control system scoped to `zone`, plus a persisted trigger instance # attached to it. The instance inherits the system's zones, so granting # on `zone` (via a "support" GroupZone) gates index/show/mutations. diff --git a/spec/controllers/systems_spec.cr b/spec/controllers/systems_spec.cr index 59e7afa8..e89aba11 100644 --- a/spec/controllers/systems_spec.cr +++ b/spec/controllers/systems_spec.cr @@ -3,10 +3,22 @@ require "http/web_socket" require "../helper" module PlaceOS::Api - ::Spec.before_each do - PlaceOS::Model::Module.clear - PlaceOS::Model::Driver.clear - PlaceOS::Model::ControlSystem.clear + # A system holding one module, named `custom_name`, once core has mapped it + # as `/1`. The system is saved once with its module so core + # sees a single event, then core's lookup is awaited rather than seeded, + # which core would overwrite (see `wait_for_module_lookups`). + def self.mapped_module_system(custom_name : String) : {Model::ControlSystem, Model::Module} + mod = Model::Generator.module + mod.custom_name = custom_name + mod.save! + module_id = mod.id.as(String) + + system = Model::Generator.control_system + system.modules = [module_id] + system.save! + wait_for_module_lookups(system.id.as(String), {"#{custom_name}/1" => module_id}) + + {system, mod} end def self.spec_add_module(system, mod, headers) @@ -93,6 +105,12 @@ module PlaceOS::Api end describe Systems do + before_each do + PlaceOS::Model::Module.clear + PlaceOS::Model::Driver.clear + PlaceOS::Model::ControlSystem.clear + end + Spec.test_404(Systems.base_route, model_name: Model::ControlSystem.table_name, headers: Spec::Authentication.headers) describe "index", tags: "search" do @@ -891,13 +909,9 @@ module PlaceOS::Api end it "GET /systems/:sys_id/functions/:module_slug" do - cs = PlaceOS::Model::Generator.control_system.save! - mod = PlaceOS::Model::Generator.module(control_system: cs).save! - module_slug = mod.id.as(String) - - sys_lookup = PlaceOS::Driver::RedisStorage.new(cs.id.as(String), "system") - lookup_key = "#{module_slug}/1" - sys_lookup[lookup_key] = module_slug + cs, mod = PlaceOS::Api.mapped_module_system("Functoids") + module_id = mod.id.as(String) + module_slug = "Functoids_1" PlaceOS::Driver::RedisStorage.with_redis do |redis| meta = PlaceOS::Driver::DriverModel::Metadata.new({ @@ -906,7 +920,7 @@ module PlaceOS::Api "function3" => {"arg1" => JSON.parse(%({"type":"integer"})), "arg2" => JSON.parse(%({"type":"integer","default":200}))}, }, ["Functoids"]) - redis.set("interface/#{module_slug}", meta.to_json) + redis.set("interface/#{module_id}", meta.to_json) end path = Systems.base_route + "#{cs.id}/functions/#{module_slug}" @@ -958,11 +972,10 @@ module PlaceOS::Api end context "with core" do - mod, cs = get_sys - # "fetches the state for `key` in module defined by `module_slug` it "GET /systems/:sys_id/:module_slug/:key" do - module_slug = cs.modules.first + cs, mod = PlaceOS::Api.mapped_module_system("Display") + module_slug = "Display_1" # Create a storage proxy driver_proxy = PlaceOS::Driver::RedisStorage.new mod.id.as(String) @@ -970,10 +983,6 @@ module PlaceOS::Api status_name = "orange" driver_proxy[status_name] = 1 - sys_lookup = PlaceOS::Driver::RedisStorage.new(cs.id.as(String), "system") - lookup_key = "#{module_slug}/1" - sys_lookup[lookup_key] = mod.id.as(String) - path = Systems.base_route + "#{cs.id}/#{module_slug}/orange" response = client.get( @@ -985,7 +994,8 @@ module PlaceOS::Api end it "GET /systems/:sys_id/:module_slug" do - module_slug = cs.modules.first + cs, mod = PlaceOS::Api.mapped_module_system("Display") + module_slug = "Display_1" # Create a storage proxy driver_proxy = PlaceOS::Driver::RedisStorage.new mod.id.as(String) @@ -993,10 +1003,6 @@ module PlaceOS::Api status_name = "nugget" driver_proxy[status_name] = 1 - sys_lookup = PlaceOS::Driver::RedisStorage.new(cs.id.as(String), "system") - lookup_key = "#{module_slug}/1" - sys_lookup[lookup_key] = mod.id.as(String) - path = Systems.base_route + "#{cs.id}/#{module_slug}" response = client.get( @@ -1258,8 +1264,6 @@ module PlaceOS::Api end describe "subsystem-based permissions" do - ::Spec.before_each { clear_group_tables } - it "PATCH allowed for 'signage' subsystem with Update perm on a signage system" do cs, _zone, _group, headers = setup_subsystem_cs("signage", Model::Permissions::Update, signage: true) diff --git a/spec/controllers/uploads_spec.cr b/spec/controllers/uploads_spec.cr index 3385afe5..266c3347 100644 --- a/spec/controllers/uploads_spec.cr +++ b/spec/controllers/uploads_spec.cr @@ -2,7 +2,7 @@ require "../helper" module PlaceOS::Api describe Uploads do - ::Spec.before_each do + before_each do Model::Storage.clear end @@ -332,11 +332,11 @@ module PlaceOS::Api authority = Model::Authority.find_by_domain("localhost").not_nil! # Create two storages for the same authority - storage1 = Model::Generator.storage(authority_id: authority.id) + storage1 = Model::Generator.storage(bucket: "bucket-#{random_name}", authority_id: authority.id) storage1.is_default = true storage1.save! - storage2 = Model::Generator.storage(authority_id: authority.id) + storage2 = Model::Generator.storage(bucket: "bucket-#{random_name}", authority_id: authority.id) storage2.is_default = false storage2.save! @@ -364,11 +364,11 @@ module PlaceOS::Api authority = Model::Authority.find_by_domain("localhost").not_nil! # Create two storages - storage1 = Model::Generator.storage(authority_id: authority.id) + storage1 = Model::Generator.storage(bucket: "bucket-#{random_name}", authority_id: authority.id) storage1.is_default = true storage1.save! - storage2 = Model::Generator.storage(authority_id: authority.id) + storage2 = Model::Generator.storage(bucket: "bucket-#{random_name}", authority_id: authority.id) storage2.is_default = false storage2.save! @@ -429,7 +429,7 @@ module PlaceOS::Api # only for DENY cases, where the gate raises before any storage call, # so these tests never touch S3/minio. Cleanup is a table truncate # (no per-row `destroy`, hence no S3) via `.clear`. - ::Spec.before_each do + before_each do clear_group_tables Model::Upload.clear Model::Storage.clear diff --git a/spec/controllers/users_spec.cr b/spec/controllers/users_spec.cr index 130167b6..d2e0dc40 100644 --- a/spec/controllers/users_spec.cr +++ b/spec/controllers/users_spec.cr @@ -271,6 +271,43 @@ module PlaceOS::Api end end + describe "POST /users/resource_token" do + it "returns the current user's SSO resource token" do + user = Model::User.find!(Spec::Authentication.user.id.as(String)) + user.access_token = "stored-access" + user.expires = false + user.save! + + begin + result = client.post( + path: File.join(Users.base_route, "/resource_token"), + headers: Spec::Authentication.headers, + ) + + result.status_code.should eq 200 + token = Users::AccessToken.from_json(result.body) + token.token.should eq "stored-access" + token.expires.should be_nil + ensure + user.access_token = nil + user.save! + end + end + + it "returns 404 when no resource token is available" do + user = Model::User.find!(Spec::Authentication.user.id.as(String)) + user.access_token = nil + user.refresh_token = nil + user.save! + + result = client.post( + path: File.join(Users.base_route, "/resource_token"), + headers: Spec::Authentication.headers, + ) + result.status_code.should eq 404 + end + end + describe "GET /users/:id/metadata" do it "shows user metadata" do user = Model::Generator.user.save! @@ -297,8 +334,6 @@ module PlaceOS::Api end describe "support subsystem permissions" do - ::Spec.before_each { clear_group_tables } - it "allows a support-subsystem user with Create grant to POST /users" do authority = Model::Authority.find_by_domain("localhost").not_nil! user, headers = Spec::Authentication.authentication(sys_admin: false, support: false) diff --git a/spec/controllers/zones_spec.cr b/spec/controllers/zones_spec.cr index b18c2baa..616e6b3c 100644 --- a/spec/controllers/zones_spec.cr +++ b/spec/controllers/zones_spec.cr @@ -561,8 +561,6 @@ module PlaceOS::Api end describe "subsystem-based permissions" do - ::Spec.before_each { clear_group_tables } - it "allows update (PATCH playlists) for a user with Update in a 'signage' group" do authority = Model::Authority.find_by_domain("localhost").not_nil! user, headers = Spec::Authentication.authentication(sys_admin: false, support: false) diff --git a/spec/helper.cr b/spec/helper.cr index 489ddfb0..a9539305 100644 --- a/spec/helper.cr +++ b/spec/helper.cr @@ -1,7 +1,6 @@ require "action-controller/spec_helper" require "http" require "mutex" -require "promise" require "random" require "pg-orm" require "spec" @@ -36,6 +35,11 @@ Spec.after_suite { clear_tables } Spec.before_each do PlaceOS::Api::HttpMocks.reset + + # Group grants apply to the shared, cached spec users, so one left over by an + # earlier example would hand a "regular" user permissions it shouldn't have + # (and break the one-root-per-authority rule). Clear them before every example. + clear_group_tables end # Application config @@ -67,33 +71,29 @@ def clear_tables PlaceOS::Model::Group, ].each(&.clear) - {% begin %} - Promise.all( - {% for t in { - PlaceOS::Model::ApiKey, - PlaceOS::Model::AssetCategory, - PlaceOS::Model::AssetType, - PlaceOS::Model::Asset, - PlaceOS::Model::AssetPurchaseOrder, - PlaceOS::Model::Authority, - PlaceOS::Model::ControlSystem, - PlaceOS::Model::Driver, - PlaceOS::Model::Module, - PlaceOS::Model::Repository, - PlaceOS::Model::Settings, - PlaceOS::Model::Trigger, - PlaceOS::Model::TriggerInstance, - PlaceOS::Model::User, - PlaceOS::Model::Zone, - PlaceOS::Model::Metadata, - PlaceOS::Model::Upload, - PlaceOS::Model::Storage, - PlaceOS::Model::DoorkeeperApplication, - } %} - Promise.defer { {{t.id}}.clear }, - {% end %} - ).get - {% end %} + # Sequentially: these tables cascade into one another, so deleting them + # concurrently can deadlock (seen in CI after every example had passed). + [ + PlaceOS::Model::ApiKey, + PlaceOS::Model::AssetCategory, + PlaceOS::Model::AssetType, + PlaceOS::Model::Asset, + PlaceOS::Model::AssetPurchaseOrder, + PlaceOS::Model::Authority, + PlaceOS::Model::ControlSystem, + PlaceOS::Model::Driver, + PlaceOS::Model::Module, + PlaceOS::Model::Repository, + PlaceOS::Model::Settings, + PlaceOS::Model::Trigger, + PlaceOS::Model::TriggerInstance, + PlaceOS::Model::User, + PlaceOS::Model::Zone, + PlaceOS::Model::Metadata, + PlaceOS::Model::Upload, + PlaceOS::Model::Storage, + PlaceOS::Model::DoorkeeperApplication, + ].each(&.clear) end def random_name diff --git a/spec/openai/chatgpt_spec.cr b/spec/openai/chatgpt_spec.cr index 566ffc85..4745efee 100644 --- a/spec/openai/chatgpt_spec.cr +++ b/spec/openai/chatgpt_spec.cr @@ -2,7 +2,7 @@ require "../helper" module PlaceOS::Api describe ChatGPT do - ::Spec.before_each do + before_each do Model::ChatMessage.clear Model::Chat.clear end diff --git a/spec/spec_helpers/core_helper.cr b/spec/spec_helpers/core_helper.cr index f55bd7e5..9ff7f28d 100644 --- a/spec/spec_helpers/core_helper.cr +++ b/spec/spec_helpers/core_helper.cr @@ -1,68 +1,22 @@ require "placeos-compiler" -PRIVATE_DRIVER_ID = "driver-#{random_id}" - def random_id UUID.random.to_s.split('-').first end -def get_driver - driver = PlaceOS::Model::Driver.where(name: "spec_helper").first? - driver, _, _ = setup_system if driver.nil? - driver -end - -def get_sys - driver = PlaceOS::Model::Driver.where(name: "spec_helper").first? - if driver.nil? - _, _, mod, cs = setup_system - else - mod = PlaceOS::Model::Module.where(name: driver.module_name).first - cs = PlaceOS::Model::ControlSystem.where(id: mod.control_system_id).first +# Waits for core to publish a system's module lookups (`system/` in redis). +# +# The core service running alongside the specs owns these lookups: on each +# ControlSystem change it clears the hash and rebuilds it from the system's +# modules (`/`), asynchronously and with each changefeed +# event processed concurrently. A lookup seeded by hand is therefore wiped +# whenever core catches up, so instead persist the system once, with its final +# modules, and wait here until core's mapping matches what the spec expects. +def wait_for_module_lookups(system_id : String, expected : Hash(String, String), timeout : Time::Span = 10.seconds) : Nil + lookup = PlaceOS::Driver::RedisStorage.new(system_id, "system") + deadline = Time.instant + timeout + until (current = lookup.to_h) == expected + raise "timed out waiting for core to map the modules of #{system_id}: expected #{expected}, got #{current}" if Time.instant > deadline + sleep 10.milliseconds end - {mod, cs} -end - -def setup_system(repository_folder_name = "private-drivers") - # Repository metadata - repository_uri = "https://github.com/placeos/private-drivers" - repository_name = "Private Drivers" - - # Driver metadata - driver_file_name = "drivers/place/private_helper.cr" - driver_module_name = "PrivateHelper" - driver_name = "spec_helper" - driver_commit = "HEAD" - driver_role = PlaceOS::Model::Driver::Role::Logic - - repository = PlaceOS::Model::Generator.repository(type: PlaceOS::Model::Repository::Type::Driver) - repository.uri = repository_uri - repository.name = repository_name - repository.folder_name = repository_folder_name + random_id - repository.save! - - driver = PlaceOS::Model::Driver.new( - name: driver_name, - role: driver_role, - commit: driver_commit, - module_name: driver_module_name, - file_name: driver_file_name, - ) - # driver._new_flag = true - driver.id = PRIVATE_DRIVER_ID - driver.repository = repository - driver.save! - - control_system = PlaceOS::Model::Generator.control_system.save! - - mod = PlaceOS::Model::Generator.module(driver: driver) - mod.running = true - mod.save! - - mod.control_system = control_system - - control_system.modules = [mod.id.as(String)] - control_system.save - - {driver, repository, mod, control_system} end diff --git a/spec/webrtc/signaller_spec.cr b/spec/webrtc/signaller_spec.cr index e023b3ca..10fdef0d 100644 --- a/spec/webrtc/signaller_spec.cr +++ b/spec/webrtc/signaller_spec.cr @@ -6,7 +6,10 @@ module PlaceOS::Api updates = [] of SessionSignal on_message = ->(message : String) { - updates << SessionSignal.from_json message + signal = SessionSignal.from_json message + # keep-alive pings are sent to every socket on a 30s timer, so one can + # land at any point during the example + updates << signal unless signal.type.ping? } session_id = UUID.random.to_s @@ -23,8 +26,7 @@ module PlaceOS::Api to_user: "server", value: nil ).to_json) - sleep 500.milliseconds - updates.size.should eq 2 + wait_for_signals(updates, 2) sessions = CallDetails::SESSIONS sessions.user_list(session_id).should eq [user_id] @@ -37,8 +39,7 @@ module PlaceOS::Api body: transfer, headers: Spec::Authentication.headers, ) - sleep 500.milliseconds - updates.size.should eq 3 + wait_for_signals(updates, 3) updates[-1].value.should eq transfer # test kick user @@ -48,14 +49,33 @@ module PlaceOS::Api body: {reason: "bad user"}.to_json, headers: Spec::Authentication.headers, ) - sleep 500.milliseconds - updates.size.should eq 4 - ws.closed?.should be_true + wait_for_signals(updates, 4) + wait_until_closed(ws) end end end end +# Waits until the expected number of signals arrive, then checks no more did +def wait_for_signals(updates : Array, count : Int32, timeout : Time::Span = 5.seconds) : Nil + deadline = Time.instant + timeout + until updates.size >= count + raise "timed out waiting for #{count} signals, got #{updates.size}" if Time.instant > deadline + sleep 10.milliseconds + end + # give an unexpected extra signal the chance to arrive before asserting the count + sleep 100.milliseconds + updates.size.should eq count +end + +def wait_until_closed(ws : HTTP::WebSocket, timeout : Time::Span = 5.seconds) : Nil + deadline = Time.instant + timeout + until ws.closed? + raise "timed out waiting for the websocket to close" if Time.instant > deadline + sleep 10.milliseconds + end +end + def signaller(on_message : Proc(String, _) = ->(_msg : String) { }, &) host = "localhost" bearer = PlaceOS::Api::Spec::Authentication.headers["Authorization"].split(' ').last diff --git a/spec/websocket/session_spec.cr b/spec/websocket/session_spec.cr index 476f179b..62d252dd 100644 --- a/spec/websocket/session_spec.cr +++ b/spec/websocket/session_spec.cr @@ -122,10 +122,11 @@ module PlaceOS::Api::WebSocket id = rand(10).to_i64 updates, _, _ = test_websocket_api(Systems.base_route, Spec::Authentication.headers) do |ws, control_system, mod, ws_updates| + # debug sessions are addressed by module id, not name (see Session#debug) request = { id: id, system_id: control_system.id.as(String), - module_name: mod.resolved_name, + module_name: mod.id.as(String), name: status_name, command: Session::Request::Command::Debug, } @@ -144,10 +145,11 @@ module PlaceOS::Api::WebSocket id = rand(10).to_i64 updates, _, _ = test_websocket_api(Systems.base_route, Spec::Authentication.headers) do |ws, control_system, mod, ws_updates| + # debug sessions are addressed by module id, not name (see Session#debug) request = { id: id, system_id: control_system.id.as(String), - module_name: mod.resolved_name, + module_name: mod.id.as(String), name: status_name, command: Session::Request::Command::Ignore, } @@ -214,26 +216,35 @@ def bind(base, auth, on_message : Proc(String, _) = ->(_msg : String) { }, &) socket.close end +# A ControlSystem containing a single Module, mapped as `/1`. +# +# The system is saved once with its module and the spec waits for core's +# lookup, which bindings resolve the module through. A lookup seeded by hand +# races core's asynchronous rebuild: core wipes it (publishing `lookup-change`, +# which remaps the binding onto nothing) and updates stop arriving. +def websocket_spec_system : {PlaceOS::Model::ControlSystem, PlaceOS::Model::Module} + mod = PlaceOS::Model::Generator.module.save! + module_id = mod.id.as(String) + + control_system = PlaceOS::Model::Generator.control_system + control_system.modules = [module_id] + control_system.save! + + wait_for_module_lookups(control_system.id.as(String), {"#{mod.resolved_name}/1" => module_id}) + {control_system, mod} +end + # Binds to the websocket API # Yields API websocket, and a control system + module # Cleans up the websocket and models def test_websocket_api(base, headers, &) - # Create a System - control_system = PlaceOS::Model::Generator.control_system.save! - - # Create a Module - mod = PlaceOS::Model::Generator.module(control_system: control_system).save! + control_system, mod = websocket_spec_system updates = [] of PlaceOS::Api::WebSocket::Session::Response on_message = ->(message : String) { updates << PlaceOS::Api::WebSocket::Session::Response.from_json message } - # Set metadata in redis to allow binding to module - sys_lookup = PlaceOS::Driver::RedisStorage.new(control_system.id.as(String), "system") - lookup_key = "#{mod.custom_name}/1" - sys_lookup[lookup_key] = mod.id.as(String) - bind(base, headers, on_message) do |ws| # `updates` is exposed so tests can gate on messages arriving (vs fixed sleeps). # Blocks that don't need it can simply take fewer params. @@ -248,15 +259,9 @@ def test_websocket_api(base, headers, &) end def test_websocket_exec(base, headers, &) - control_system = PlaceOS::Model::Generator.control_system.save! - mod = PlaceOS::Model::Generator.module(control_system: control_system).save! - + control_system, mod = websocket_spec_system module_slug = mod.id.as(String) - sys_lookup = PlaceOS::Driver::RedisStorage.new(control_system.id.as(String), "system") - lookup_key = "#{mod.custom_name}/1" - sys_lookup[lookup_key] = module_slug - PlaceOS::Driver::RedisStorage.with_redis do |redis| meta = PlaceOS::Driver::DriverModel::Metadata.new({ "nugget" => {} of String => JSON::Any, @@ -281,7 +286,5 @@ def test_websocket_exec(base, headers, &) control_system.destroy mod.destroy - sys_lookup.clear - {updates, control_system, mod} end diff --git a/src/placeos-rest-api/controllers/users.cr b/src/placeos-rest-api/controllers/users.cr index 488abeb5..b9aad311 100644 --- a/src/placeos-rest-api/controllers/users.cr +++ b/src/placeos-rest-api/controllers/users.cr @@ -96,7 +96,7 @@ module PlaceOS::Api raise Error::Unauthorized.new("user not found") end - record AccessToken, token : String, expires : Int64? { include JSON::Serializable } + alias AccessToken = ::PlaceOS::Model::User::ResourceToken # Obtain a token to the current users SSO resources # this token is used for delegated access to things like MS Graph API or Google API in the context of the user @@ -125,68 +125,13 @@ module PlaceOS::Api user.save! end - protected def get_user_token(current_user) : AccessToken + protected def get_user_token(user : Model::User) : AccessToken authority = current_authority raise Error::NotFound.new("no valid authority") unless authority - self.class.get_user_token(current_user, authority) - end - - def self.get_user_token(current_user : Model::User, authority : Model::Authority) : AccessToken - expired = true - - if access_token = current_user.access_token.presence - if current_user.expires - expires_at = Time.unix(current_user.expires_at.not_nil!) - if 5.minutes.from_now < expires_at - return AccessToken.new(access_token.as(String), expires_at.to_unix) - end - - # Allow for clock drift - expired = 15.seconds.from_now > expires_at - else - return AccessToken.new(access_token.as(String), nil) - end - end - - raise Error::NotFound.new("no refresh token available") unless current_user.refresh_token.presence - - begin - internals = authority.internals - sso_strat = if sso_strat_id = internals["oauth-strategy"]?.try(&.as_s?) # (i.e. oauth_strat-FNsaSj6bp-M) - ::PlaceOS::Model::OAuthAuthentication.find(sso_strat_id) - else - ::PlaceOS::Model::OAuthAuthentication.where(authority_id: authority.id).first? - # ::PlaceOS::Model::OAuthAuthentication.collection_query do |table| - # table.get_all(authority.id, index: :authority_id) - # end.first? - end - - raise Error::NotFound.new("no oauth configuration found") unless sso_strat - - client_id = sso_strat.client_id - client_secret = sso_strat.client_secret - token_uri = URI.parse(sso_strat.token_url) - token_host = token_uri.hostname.not_nil! - token_path = token_uri.request_target - - oauth2_client = OAuth2::Client.new(token_host, client_id, client_secret, token_uri: token_path) - token = oauth2_client.get_access_token_using_refresh_token(current_user.refresh_token, sso_strat.scope) - - current_user.access_token = token.access_token - current_user.refresh_token = token.refresh_token if token.refresh_token - current_user.expires_at = Time.utc.to_unix + token.expires_in.not_nil! - current_user.save! - - AccessToken.new(current_user.access_token.as(String), current_user.expires_at) - rescue error - Log.warn(exception: error) { "failed refresh access token" } - if !expired - AccessToken.new(current_user.access_token.as(String), current_user.expires_at) - else - raise error - end - end + user.resource_token(authority) + rescue error : Model::Error::NoResourceToken + raise Error::NotFound.new(error.message || "no resource token available") end # CRUD diff --git a/src/placeos-rest-api/utilities/ms-token-exchange.cr b/src/placeos-rest-api/utilities/ms-token-exchange.cr index b9e400ea..331e593a 100644 --- a/src/placeos-rest-api/utilities/ms-token-exchange.cr +++ b/src/placeos-rest-api/utilities/ms-token-exchange.cr @@ -149,7 +149,7 @@ module PlaceOS::Api def ensure_valid_token(oauth : Model::OAuthAuthentication, user : Model::User, token : String, token_info : PeekInfo) # return if there is an existing token and valid - existing = Api::Users.get_user_token(user, oauth.authority.as(Model::Authority)) rescue nil + existing = user.resource_token(oauth.authority.as(Model::Authority)) rescue nil return if existing # if not existing or refresh failed, get a token using this token and on behalf of