diff --git a/drivers/place/at_capacity_mailer_spec.cr b/drivers/place/at_capacity_mailer_spec.cr index 44aa9de858..6600325d18 100644 --- a/drivers/place/at_capacity_mailer_spec.cr +++ b/drivers/place/at_capacity_mailer_spec.cr @@ -2,6 +2,50 @@ require "placeos-driver/spec" require "placeos-driver/interface/mailer" class StaffAPI < DriverSpecs::MockDriver + getter metadata_shape : String = "legacy" + getter failing_type : String? = nil + getter asset_calls = 0 + getter type_calls = 0 + + def configure(metadata_shape : String, failing_type : String? = nil) + @metadata_shape = metadata_shape + @failing_type = failing_type + end + + def asset_categories(hidden : Bool? = nil) + JSON.parse([ + {id: "category-desk", name: "_DESKS_", hidden: true}, + {id: "category-parking", name: "_PARKING_", hidden: true}, + {id: "category-parking-legacy", name: "_PARKING_SPACES_", hidden: true}, + ].to_json) + end + + def asset_types(category_id : String? = nil, zone_id : String? = nil, brand : String? = nil, model_number : String? = nil) + @type_calls += 1 + JSON.parse([ + {id: "type-desk", name: "_DESKS_", category_id: "category-desk"}, + {id: "type-parking", name: "_PARKING_SPACES_", category_id: "category-parking"}, + {id: "type-parking-duplicate", name: "_PARKING_SPACES_", category_id: "category-parking"}, + {id: "type-parking-legacy", name: "_PARKING_SPACES_", category_id: "category-parking-legacy"}, + {id: "type-parking-users", name: "_PARKING_USERS_", category_id: "category-parking"}, + {id: "type-locker", name: "_LOCKERS_", category_id: "category-desk"}, + ].select { |type| category_id.nil? || type[:category_id] == category_id }.to_json) + end + + def assets(type_id : String? = nil, zone_id : String? = nil) + @asset_calls += 1 + raise "assets unavailable" if type_id == failing_type + ids = case type_id + when "type-desk" then ["asset-desk-1", "asset-desk-2"] + when "type-parking" then ["asset-park-1"] + when "type-parking-duplicate" then ["asset-park-2"] + when "type-parking-legacy" then ["asset-park-3"] + else ["unrelated-asset"] + end + ids = [] of String unless zone_id == "level-1" || zone_id == "level-2" + JSON.parse(ids.map { |id| {id: id, identifier: "Name #{id}", name: nil, zone_id: zone_id, zones: [] of String} }.to_json) + end + ZONES = [ { created_at: 1660537814, @@ -54,6 +98,19 @@ class StaffAPI < DriverSpecs::MockDriver zone = ZONES.find! { |z| z["id"] == id } key = key.not_nil! + return JSON.parse("{}") if metadata_shape == "missing" + raise "metadata unavailable" if metadata_shape == "error" + unless metadata_shape == "legacy" + details = case metadata_shape + when "migrated" then JSON.parse(%({"migrated":true,"migrated_at":1765438509624})) + when "empty" then JSON.parse("[]") + when "null" then JSON.parse("null") + when "object" then JSON.parse("{}") + else JSON.parse(%("")) + end + return JSON.parse({key => {name: key, parent_id: id, details: details}}.to_json) + end + details = case key when "desks" [ @@ -140,6 +197,9 @@ class StaffAPI < DriverSpecs::MockDriver when "parking" ["park-1"] end + if metadata_shape == "migrated" + assets = type == "desk" ? ["asset-desk-1", "asset-desk-2"] : ["asset-park-1", "asset-park-2", "asset-park-3"] + end JSON.parse(assets.to_json) end end @@ -244,4 +304,71 @@ DriverSpecs.mock_driver "Place::AtCapacityMailer" do ##################################### # End of tests for: #check_capacity + + api = system(:StaffAPI_1).as(StaffAPI) + + it "keeps legacy capacity lists metadata-only when Asset records also exist" do + settings({booking_type: "desk", zones: ["level-1"]}) + exec(:get_asset_ids).get.should eq({"level-1" => ["desk-1", "desk-2"]}) + api.asset_calls.should eq 0 + api.type_calls.should eq 0 + end + + it "sends at-capacity mail for a migrated desk zone" do + api.configure("migrated") + settings({booking_type: "desk", zones: ["level-2"]}) + exec(:get_asset_ids).get.should eq({"level-2" => ["asset-desk-1", "asset-desk-2"]}) + exec(:check_capacity).get + system(:Mailer_1)[:sent].should eq 2 + end + + it "includes duplicate parking types from current and legacy categories" do + api.configure("migrated") + settings({booking_type: "parking", zones: ["level-1"]}) + exec(:get_asset_ids).get.should eq({"level-1" => ["asset-park-1", "asset-park-2", "asset-park-3"]}) + end + + ["missing", "null", "object", "string", "error"].each do |shape| + it "uses Asset records for #{shape} metadata" do + api.configure(shape) + settings({booking_type: "desk", zones: ["level-1"]}) + exec(:get_asset_ids).get.should eq({"level-1" => ["asset-desk-1", "asset-desk-2"]}) + end + end + + it "keeps an empty metadata array as an empty capacity list" do + api.configure("empty") + settings({booking_type: "desk", zones: ["level-1"]}) + calls = api.asset_calls + exec(:get_asset_ids).get.should eq({"level-1" => [] of String}) + api.asset_calls.should eq calls + end + + it "caches Asset lists per zone and shares type discovery between zones" do + api.configure("migrated") + settings({booking_type: "parking", zones: ["level-1", "level-2"]}) + asset_calls = api.asset_calls + type_calls = api.type_calls + exec(:get_asset_ids).get + exec(:get_asset_ids).get + api.asset_calls.should eq asset_calls + 6 + api.type_calls.should eq type_calls + 1 + end + + it "expires the Asset list and type caches using asset_cache_timeout" do + api.configure("migrated") + settings({booking_type: "desk", zones: ["level-1"], asset_cache_timeout: 0}) + asset_calls = api.asset_calls + type_calls = api.type_calls + 2.times { exec(:get_asset_ids).get } + api.asset_calls.should eq asset_calls + 2 + api.type_calls.should eq type_calls + 2 + end + + it "retains available assets when one duplicate type cannot be queried" do + api.configure("migrated", "type-parking-duplicate") + settings({booking_type: "parking", zones: ["level-1"]}) + exec(:get_asset_ids).get.should eq({"level-1" => ["asset-park-1", "asset-park-3"]}) + api.configure("migrated") + end end diff --git a/drivers/place/auto_release_spec.cr b/drivers/place/auto_release_spec.cr index 25375d0d76..d5c0f0a26a 100644 --- a/drivers/place/auto_release_spec.cr +++ b/drivers/place/auto_release_spec.cr @@ -2,6 +2,46 @@ require "placeos-driver/spec" require "placeos-driver/interface/mailer" class StaffAPI < DriverSpecs::MockDriver + getter metadata_shape : String = "migrated" + getter asset_identifier : String? = "Desk from Asset" + getter asset_name : String? = "Fallback name" + getter assets_fail : Bool = false + getter asset_calls = 0 + + def configure(metadata_shape : String, identifier : String? = "Desk from Asset", name : String? = "Fallback name", assets_fail : Bool = false) + @metadata_shape = metadata_shape + @asset_identifier = identifier + @asset_name = name + @assets_fail = assets_fail + end + + def metadata(id : String, key : String? = nil) + details = if metadata_shape == "migrated" + JSON.parse(%({"migrated":true,"migrated_at":1765438509624})) + else + JSON.parse([{id: "legacy-desk", name: "Legacy desk"}].to_json) + end + JSON.parse({key.not_nil! => {name: key, parent_id: id, details: details}}.to_json) + end + + def asset_categories(hidden : Bool? = nil) + JSON.parse([{id: "category-desk", name: "_DESKS_", hidden: true}].to_json) + end + + def asset_types(category_id : String? = nil, zone_id : String? = nil, brand : String? = nil, model_number : String? = nil) + JSON.parse([{id: "type-desk", name: "_DESKS_", category_id: "category-desk"}].to_json) + end + + def assets(type_id : String? = nil, zone_id : String? = nil) + @asset_calls += 1 + raise "assets unavailable" if assets_fail + return JSON.parse("[]") unless type_id == "type-desk" && zone_id == "zone-1234" + JSON.parse([ + {id: "asset-desk", identifier: asset_identifier, name: asset_name, zone_id: zone_id, zones: [] of String}, + {id: "legacy-desk", identifier: "Asset name for legacy desk", name: nil, zone_id: zone_id, zones: [] of String}, + ].to_json) + end + def on_load self[:rejected] = 0 end @@ -799,6 +839,7 @@ class Mailer < DriverSpecs::MockDriver ) self[:sent] = self[:sent].as_i + 1 self[:reply_to] = reply_to + self[:last_args] = args end def send_mail( @@ -1306,4 +1347,43 @@ DriverSpecs.mock_driver "Place::AutoRelease" do ############################# # End of tests for: #enabled? + + api = system(:StaffAPI_1).as(StaffAPI) + [ + {"migrated", "Asset identifier", "Asset name", "asset-desk", "Asset identifier"}, + {"migrated", "", "Asset name", "asset-desk", "Asset name"}, + {"migrated", nil, nil, "asset-desk", "asset-desk"}, + {"migrated", "", "", "asset-desk", "asset-desk"}, + {"legacy", "Asset identifier", "Asset name", "asset-desk", "Asset identifier"}, + {"legacy", "Asset identifier", "Asset name", "legacy-desk", "Legacy desk"}, + {"legacy", "Asset identifier", "Asset name", "unknown-desk", "unknown-desk"}, + ].each do |shape, identifier, name, asset_id, expected| + it "resolves #{shape} #{asset_id} with identifier #{identifier.inspect} and name #{name.inspect}" do + api.configure(shape, identifier, name) + settings({auto_release: {time_before: 10, time_after: 10, resources: ["desk"]}}) + status[:pending_release] = [StaffAPI::BOOKINGS[1].merge({asset_id: asset_id})] + status[:released_booking_ids] = [] of Int64 + status[:emailed_booking_ids] = [] of Int64 + calls = api.asset_calls + exec(:send_release_emails).get.should eq [2] + system(:Mailer_1)[:last_args]["asset_name"].should eq expected + api.asset_calls.should eq calls if asset_id == "legacy-desk" + + calls = api.asset_calls + status[:emailed_booking_ids] = [] of Int64 + exec(:send_release_emails).get.should eq [2] + system(:Mailer_1)[:last_args]["asset_name"].should eq expected + api.asset_calls.should eq calls + end + end + + it "still sends the email with the raw id when the Asset request fails" do + api.configure("migrated", assets_fail: true) + settings({auto_release: {time_before: 10, time_after: 10, resources: ["desk"]}}) + status[:pending_release] = [StaffAPI::BOOKINGS[1].merge({asset_id: "asset-desk"})] + status[:released_booking_ids] = [] of Int64 + status[:emailed_booking_ids] = [] of Int64 + exec(:send_release_emails).get.should eq [2] + system(:Mailer_1)[:last_args]["asset_name"].should eq "asset-desk" + end end diff --git a/drivers/place/bookings/asset_name_resolver.cr b/drivers/place/bookings/asset_name_resolver.cr index 717e85463c..35b32d8781 100644 --- a/drivers/place/bookings/asset_name_resolver.cr +++ b/drivers/place/bookings/asset_name_resolver.cr @@ -5,12 +5,16 @@ module Place::AssetNameResolver include Place::LockerMetadataParser @asset_cache : AssetCache = AssetCache.new + @asset_record_cache : AssetCache = AssetCache.new + @asset_type_cache = {} of String => Tuple(Int64, Array(String)) @asset_cache_timeout : Int64 = 3600_i64 # 1 hour private getter asset_cache : AssetCache private def clear_asset_cache @asset_cache = AssetCache.new + @asset_record_cache = AssetCache.new + @asset_type_cache.clear end private def lookup_asset(asset_id : String, type : String, zones : Array(String) = [building_id]) : String @@ -19,14 +23,10 @@ module Place::AssetNameResolver return locker.name if locker else zones.each do |zone_id| - asset = if (cache = asset_cache[{zone_id, type}]?) && cache[0] > Time.utc.to_unix - cache[1].find { |asset| asset.id == asset_id } - else - assets = lookup_assets(zone_id, type) - @asset_cache[{zone_id, type}] = {Time.utc.to_unix + @asset_cache_timeout, assets} - assets.find { |asset| asset.id == asset_id } - end + asset = lookup_assets(zone_id, type).find { |asset| asset.id == asset_id } + return asset.name if asset + asset = lookup_asset_records(zone_id, type).find { |asset| asset.id == asset_id } return asset.name if asset end end @@ -46,8 +46,24 @@ module Place::AssetNameResolver end if metadata_field - metadata = Metadata.from_json staff_api.metadata(zone_id, metadata_field).get[metadata_field].to_json - assets = metadata.details.as_a.map { |asset| Asset.from_json asset.to_json } + if (cache = asset_cache[{zone_id, type}]?) && cache[0] > Time.utc.to_unix + return cache[1] + end + + details = begin + metadata = Metadata.from_json staff_api.metadata(zone_id, metadata_field).get[metadata_field].to_json + metadata.details.as_a? + rescue error + logger.debug { "unable to get #{metadata_field} from zone #{zone_id} metadata" } + nil + end + + assets = if details + details.map { |asset| Asset.from_json asset.to_json } + else + lookup_asset_records(zone_id, type) + end + @asset_cache[{zone_id, type}] = {Time.utc.to_unix + @asset_cache_timeout, assets} elsif type == "locker" assets = locker_details.map { |id, locker| Asset.new(id, locker.name) } end @@ -58,6 +74,48 @@ module Place::AssetNameResolver [] of Asset end + private def lookup_asset_records(zone_id : String, type : String) : Array(Asset) + assets = [] of Asset + type_name = case type + when "desk" then "_DESKS_" + when "parking" then "_PARKING_SPACES_" + end + return assets unless type_name + + if (cache = @asset_record_cache[{zone_id, type}]?) && cache[0] > Time.utc.to_unix + return cache[1] + end + + begin + type_ids = if (cache = @asset_type_cache[type]?) && cache[0] > Time.utc.to_unix + cache[1] + else + ids = staff_api.asset_types.get.as_a.select { |asset_type| asset_type["name"]?.try(&.as_s?) == type_name } + .map { |asset_type| asset_type["id"].as_s }.uniq! + @asset_type_cache[type] = {Time.utc.to_unix + @asset_cache_timeout, ids} + ids + end + + type_ids.each do |type_id| + begin + staff_api.assets(type_id: type_id, zone_id: zone_id).get.as_a.each do |asset| + id = asset["id"].as_s + name = asset["identifier"]?.try(&.as_s?).presence || asset["name"]?.try(&.as_s?).presence || id + assets << Asset.new(id, name) + end + rescue error + logger.warn(exception: error) { "unable to get #{type_id} assets from zone #{zone_id}" } + end + end + rescue error + logger.warn(exception: error) { "unable to get #{type} asset types for zone #{zone_id}" } + end + + assets.uniq!(&.id) + @asset_record_cache[{zone_id, type}] = {Time.utc.to_unix + @asset_cache_timeout, assets} + assets + end + # zone_id, type timeout, assets alias AssetCache = Hash(Tuple(String, String), Tuple(Int64, Array(Asset)))