From 08268d798bbda8b8d00a5b2c7fdf3e83d2e0f306 Mon Sep 17 00:00:00 2001 From: Frank Elsinga Date: Sat, 2 Nov 2024 23:20:17 +0100 Subject: [PATCH] made sure that the rust based deserialiser is implemented (#1611) * made sure that the rust based deserialiser is implemented * added fixes for bugs which the new testcase revealed * added more fixes discovered in a second testing run * fixed things encountered in the third pass * added the final round of fixes# * Removed the shaddow api * removed a few now unsused structs * fixed comments being partially not getitign rendered * removed the undocumented `tumonline_room_nr` from the public api * formatting --- data/output/openapi.yaml | 2 +- data/sources/02_rooms-extended.yaml | 21 +- server/src/locations/details.rs | 420 +++++++++++++++++++++++++--- server/src/setup/tests.rs | 26 +- webclient/app/api_types/index.ts | 2 +- 5 files changed, 417 insertions(+), 54 deletions(-) diff --git a/data/output/openapi.yaml b/data/output/openapi.yaml index 096fe2a3a..29e782abe 100644 --- a/data/output/openapi.yaml +++ b/data/output/openapi.yaml @@ -1636,7 +1636,7 @@ components: comment: description: | A comment to show to an entry. - It is used in the rare cases, where some aspect about the rooom/.. or its translation are misleading. + It is used in the rare cases, where some aspect about the room/.. or its translation are misleading. An example of a room with a comment is MW1801. type: string calendar_url: diff --git a/data/sources/02_rooms-extended.yaml b/data/sources/02_rooms-extended.yaml index 5a68f28c0..491a27345 100644 --- a/data/sources/02_rooms-extended.yaml +++ b/data/sources/02_rooms-extended.yaml @@ -118,9 +118,10 @@ "0501.05.131": name: Vorhoelzer-Forum Dachterasse "0502.EG.221": - comment: - de: Unter dieser Treppe existiert ein WC, dessen Raumnummer und designierung (men/women/unisex) unklar ist - en: Under this staircase there is a toilet whose room number and designation (men/women/unisex) is unclear + props: + comment: + de: Unter dieser Treppe existiert ein WC, dessen Raumnummer und designierung (men/women/unisex) unklar ist + en: Under this staircase there is a toilet whose room number and designation (men/women/unisex) is unclear "0503.EG.347": # previously: Seminarraum name: Archiv usage: @@ -206,13 +207,15 @@ "0509.02.953A": name: Think Tank / Teeküche arch_name: 1951 - comment: - de: Dieser Raum hat auch die Architektennummer 1953A - en: This room also has the architect number 1953A + props: + comment: + de: Dieser Raum hat auch die Architektennummer 1953A + en: This room also has the architect number 1953A "0509.02.952": - comment: - de: Dieser Raum hat zwei Türen 1954 und 1952 - en: This room has two doors 1954 and 1952 + props: + comment: + de: Dieser Raum hat zwei Türen 1954 und 1952 + en: This room has two doors 1954 and 1952 "4213.01.318": # baywa coworking space props: links: diff --git a/server/src/locations/details.rs b/server/src/locations/details.rs index 848ebf151..dfb253b50 100644 --- a/server/src/locations/details.rs +++ b/server/src/locations/details.rs @@ -1,5 +1,6 @@ use actix_web::http::header::{CacheControl, CacheDirective}; use actix_web::{get, web, HttpResponse}; +use serde::{Deserialize, Serialize}; use sqlx::Error::RowNotFound; use sqlx::PgPool; use tracing::error; @@ -29,23 +30,30 @@ pub async fn get_handler( .await }; match result { - Ok(d) => match d { - None => HttpResponse::NotFound().body("Not found"), - Some(d) => { - let mut response_json = serde_json::to_string(&d).unwrap(); - // We don't want to serialise this data at any point in the server. - // This just flows through the server, but adding redirect_url to the response is necessary - response_json.pop(); // remove last } - response_json.push_str(&format!(",\"redirect_url\":\"{redirect_url}\"}}",)); - HttpResponse::Ok() - .insert_header(CacheControl(vec![ - CacheDirective::MaxAge(24 * 60 * 60), // valid for 1d - CacheDirective::Public, - ])) - .content_type("application/json") - .body(response_json) + Ok(d) => { + if let Some(d) = d { + let res = serde_json::from_value::(d); + match res { + Err(e) => { + error!("cannot serialise {id} because {e:?}"); + HttpResponse::InternalServerError() + .content_type("text/plain") + .body("Internal Server Error") + } + Ok(mut res) => { + res.redirect_url = Some(redirect_url); + HttpResponse::Ok() + .insert_header(CacheControl(vec![ + CacheDirective::MaxAge(24 * 60 * 60), // valid for 1d + CacheDirective::Public, + ])) + .json(res) + } + } + } else { + HttpResponse::NotFound().body("Not found") } - }, + } Err(e) => { error!("Error requesting details for {probable_id}: {e:?}"); HttpResponse::InternalServerError() @@ -55,6 +63,362 @@ pub async fn get_handler( } } +#[derive(Serialize, Deserialize, Debug, Clone)] +struct Operator { + id: u32, + url: String, + code: String, + name: String, +} + +#[derive(Serialize, Deserialize, Debug, Default)] +#[serde(rename_all = "snake_case")] +enum LocationType { + #[default] + Room, + Building, + JoinedBuilding, + Area, + Site, + Campus, + Poi, +} + +#[derive(Deserialize, Serialize, Debug, Default)] +struct LocationDetailsResponse { + /// The id, that was requested + id: String, + /// The type of the entry + r#type: LocationType, + /// The type of the entry in a human-readable form + type_common_name: String, + /// The name of the entry in a human-readable form + name: String, + /// A list of alternative ids for this entry. + /// + /// Not to be confused with + /// - [`id`] which is the unique identifier or + /// - [`visual-id`] which is an alternative identifier for the entry (only displayed in the URL). + aliases: Vec, + /// The ids of the parents. + /// They are ordered as they would appear in a Breadcrumb menu. + /// See [`parent_names`] for their human names. + parents: Vec, + /// The ids of the parents. They are ordered as they would appear in a Breadcrumb menu. + /// See [`parents`] for their actual ids. + parent_names: Vec, + /// Data for the info-card table + props: Props, + /// The information you need to request Images from the /cdn/{size}/{id}_{counter}.webp endpoint + /// TODO: Sometimes missing, sometimes not.. so weird.. + #[serde(skip_serializing_if = "Option::is_none")] + imgs: Option>, + ranking_factors: RankingFactors, + /// Where we got our data from, should be displayed at the bottom of any page containing this data + sources: Sources, + /// The url, this item should be displayed at. Present on both redirects and normal entries, to allow for the common /view/:id path + redirect_url: Option, + coords: Coordinate, + maps: Maps, + #[serde(skip_serializing_if = "Option::is_none")] + sections: Option, +} + +#[derive(Deserialize, Serialize, Debug, Default)] +struct Sections { + #[serde(skip_serializing_if = "Option::is_none")] + buildings_overview: Option, + #[serde(skip_serializing_if = "Option::is_none")] + rooms_overview: Option, + #[serde(skip_serializing_if = "Option::is_none")] + featured_overview: Option, +} + +#[derive(Deserialize, Serialize, Debug, Default)] +struct BuildingsOverviewItem { + /// The id of the entry + id: String, + /// Human display name + name: String, + /// What should be displayed below this Building + subtext: String, + /// The thumbnail for the building + thumb: Option, +} + +#[derive(Deserialize, Serialize, Debug, Default)] +struct FeaturedOverviewItem { + /// The id of the entry + id: String, + /// Human display name + name: String, + /// What should be displayed below this Building + subtext: String, + /// The thumbnail for the building + image_url: String, +} + +#[derive(Deserialize, Serialize, Debug, Default)] +struct BuildingsOverview { + entries: Vec, + n_visible: u32, +} + +#[derive(Deserialize, Serialize, Debug, Default)] +struct RoomsOverviewUsageChild { + id: String, + name: String, +} +#[derive(Deserialize, Serialize, Debug, Default)] +struct RoomsOverviewUsage { + name: String, + count: u32, + children: Vec, +} + +#[derive(Deserialize, Serialize, Debug, Default)] +struct RoomsOverview { + usages: Vec, +} + +#[derive(Deserialize, Serialize, Debug, Default)] +struct FeaturedOverview { + entries: Vec, +} + +#[derive(Deserialize, Serialize, Debug, Default)] +struct Maps { + default: DefaultMaps, + #[serde(skip_serializing_if = "Option::is_none")] + roomfinder: Option, + /// [`None`] would mean no overlay maps are displayed by default. + /// For rooms, you should add a warning that no floor map is available for this room + #[serde(skip_serializing_if = "Option::is_none")] + overlays: Option, +} + +#[derive(Deserialize, Serialize, Debug, Default)] +struct RoomfinderMap { + /// The id of the map, that should be shown as a default + /// Example: `rf142` + default: String, + available: Vec, +} +#[derive(Deserialize, Serialize, Debug, Default)] +struct RoomfinderMapEntry { + /// human-readable name of the map + name: String, + /// machine-readable name of the map + id: String, + /// Scale of the map. 2000 means 1:2000 + scale: String, + /// Map image y dimensions + height: i32, + /// Map image y dimensions + width: i32, + /// x Position on map image + x: i32, + /// y Position on map image + y: i32, + /// Where the map was imported from + source: String, + /// Where the map is stored + file: String, +} +#[derive(Deserialize, Serialize, Debug, Default)] +struct OverlayMaps { + /// The floor-id of the map, that should be shown as a default. + /// null means: + /// - We suggest, you don't show a map by default. + /// - This is only the case for buildings or other such entities and not for rooms, if we know where they are and a map exists + default: Option, + available: Vec, +} + +#[derive(Deserialize, Serialize, Debug, Default)] +struct OverlayMapEntry { + /// machine-readable floor-id of the map. + /// Should start with 0 for the ground level (defined by the main entrance) and increase or decrease. + /// It is not guaranteed that numbers are consecutive or that `1` corresponds to level `01`, because buildings sometimes have more complicated layouts. They are however always in the correct (physical) order. + id: i32, + /// Floor of the Map. + /// Should be used for display to the user in selectors. + /// Matches the floor part of the TUMonline roomcode. + floor: String, + /// human-readable name of the map + name: String, + /// filename of the map + file: String, + /// Coordinates are four `[lon, lat]` pairs, for the top left, top right, bottom right, bottom left image corners. + coordinates: [(f64, f64); 4], +} + +#[derive(Deserialize, Serialize, Debug, Default)] +#[serde(rename_all = "snake_case")] +enum DefaultMaps { + #[default] + Interactive, + Roomfinder, +} + +#[derive(Deserialize, Serialize, Debug, Default)] +struct ExtraComputedProp { + /// example: `Genauere Angaben` + #[serde(skip_serializing_if = "Option::is_none")] + header: Option, + /// example: `for exams: 102 in tight, 71 in wide, 49 in corona` + body: String, + /// example: `data based on a Survey of chimneysweeps` + #[serde(skip_serializing_if = "Option::is_none")] + footer: Option, +} + +#[derive(Deserialize, Serialize, Debug, Default)] +struct ComputedProp { + /// example: `Raumkennung` + name: String, + /// example: `5602.EG.001` + text: String, + #[serde(skip_serializing_if = "Option::is_none")] + extra: Option, +} +#[derive(Deserialize, Serialize, Debug, Default)] +struct Props { + /// The operator of the room + #[serde(skip_serializing_if = "Option::is_none")] + operator: Option, + computed: Vec, + #[serde(skip_serializing_if = "Vec::is_empty", default = "Vec::new")] + links: Vec, + /// A comment to show to an entry. + /// It is used in the rare cases, where some aspect about the room/.. or its translation are misleading. + /// An example of a room with a comment is `MW1801`. + #[serde(skip_serializing_if = "String::is_empty", default = "String::new")] + comment: String, + /// link to the calendar of the room + /// examples: + /// - 'https://campus.tum.de/tumonline/tvKalender.wSicht?cOrg=19691&cRes=12543&cReadonly=J' + /// - 'https://campus.tum.de/tumonline/tvKalender.wSicht?cOrg=19691&cRes=12559&cReadonly=J' + #[serde(skip_serializing_if = "Option::is_none")] + calendar_url: Option, +} + +#[derive(Serialize, Deserialize, Debug)] +struct Source { + /// name of the provider + name: String, + /// url of the provider + #[serde(skip_serializing_if = "Option::is_none")] + url: Option, +} +#[derive(Deserialize, Serialize, Debug, Default)] +struct Sources { + /// Was this entry patched by us? (e.g. to fix a typo in the name/...) + /// If so, we should not display the source, as it is not the original source. + #[serde(skip_serializing_if = "Option::is_none")] + patched: Option, // default = false + /// What is the basis of the data we have + base: Vec, +} + +#[derive(Deserialize, Serialize, Debug, Default)] +struct ImageInfo { + /// The name of the image file. consists of {building_id}_{image_id}.webp, where image_id is a counter starting at 0 + name: String, + author: URLRef, + source: PossibleURLRef, + license: PossibleURLRef, + #[serde(skip_serializing_if = "Option::is_none")] + meta: Option, +} + +#[derive(Deserialize, Serialize, Debug, Default)] +struct PossibleURLRef { + text: String, + #[serde(skip_serializing_if = "Option::is_none")] + url: Option, +} + +#[derive(Deserialize, Serialize, Debug, Default)] +struct URLRef { + text: String, + url: Option, +} + +/// Additional data about the images. +/// Does not have to be displayed. +/// All fields are optional. +#[derive(Deserialize, Serialize, Debug, Default)] +struct ImageMetadata { + ///optional date description + #[serde(skip_serializing_if = "Option::is_none")] + date: Option, + ///optional location description + #[serde(skip_serializing_if = "Option::is_none")] + location: Option, + ///optional coordinates in lat,lon + #[serde(skip_serializing_if = "Option::is_none")] + geo: Option, + /// optional in contrast to source this points to the image itself. + /// You should not use this to request the images, as they are not scaled. + #[serde(skip_serializing_if = "Option::is_none")] + image_url: Option, + /// optional caption + #[serde(skip_serializing_if = "Option::is_none")] + caption: Option, + /// optional headline + #[serde(skip_serializing_if = "Option::is_none")] + headline: Option, + /// optional the event this image was taken at + #[serde(skip_serializing_if = "Option::is_none")] + event: Option, + /// optional the event this image is about + #[serde(skip_serializing_if = "Option::is_none")] + faculty: Option, + ///optional the building this image is about + #[serde(skip_serializing_if = "Option::is_none")] + building: Option, + /// optional the department this image is about + #[serde(skip_serializing_if = "Option::is_none")] + department: Option, +} + +#[derive(Deserialize, Serialize, Debug, Default)] +struct RankingFactors { + rank_combined: u32, + rank_type: u32, + rank_usage: u32, + #[serde(skip_serializing_if = "Option::is_none")] + rank_boost: Option, + #[serde(skip_serializing_if = "Option::is_none")] + rank_custom: Option, +} + +#[derive(Deserialize, Serialize, Debug, Default)] +struct Coordinate { + lat: f64, + lon: f64, + source: CoordinateSource, + #[serde(skip_serializing_if = "Option::is_none")] + accuracy: Option, +} + +#[derive(Deserialize, Serialize, Debug, Default)] +#[serde(rename_all = "snake_case")] +enum CoordinateAccuracy { + #[default] + Building, +} + +#[derive(Serialize, Deserialize, Debug, Default)] +#[serde(rename_all = "snake_case")] +enum CoordinateSource { + #[default] + Navigatum, + Roomfinder, + Inferred, +} + #[tracing::instrument(skip(pool))] async fn get_alias_and_redirect(pool: &PgPool, query: &str) -> Option<(String, String)> { let result = sqlx::query_as!( @@ -112,36 +476,26 @@ mod tests { use super::*; use crate::{setup::tests::PostgresTestContainer, AppData}; - /// Allows tesing if a modification has changed the output of the details API + /// Allows testing if a modification has changed the output of the details API /// /// The testcase can be executed via running the following command on main /// ```bash - /// INSTA_OUTPUT=none INSTA_UPDATE=always DATABASE_URL=postgres://postgres:CHANGE_ME@localhost:5432 cargo test -p navigatum-server test_get_handler_unchanged -- --nocapture --include-ignored + /// INSTA_OUTPUT=none INSTA_UPDATE=always DATABASE_URL=postgres://postgres:CHANGE_ME@localhost:5432 cargo test --package navigatum-server test_get_handler_unchanged -- --nocapture --include-ignored /// ``` /// /// And then running this command on the change /// ```bash - /// DATABASE_URL=postgres://postgres:CHANGE_ME@localhost:5432 cargo insta --review -- -p navigatum-server test_get_handler_unchanged --nocapture --include-ignored + /// DATABASE_URL=postgres://postgres:CHANGE_ME@localhost:5432 cargo insta test --review --package navigatum-server -- test_get_handler_unchanged --nocapture --include-ignored /// ``` /// - /// This is a ..bit.. slow, due to using a [`tokio::task::LocalSet`]. + /// This is a *bit* slow, due to using a [`tokio::task::LocalSet`]. /// Using multiple cores for this might be possible, but optimising this testcase from 10m is currently not worth it #[ignore] #[actix_web::test] #[tracing_test::traced_test] async fn test_get_handler_unchanged() { - // setup + load data into postgis let pg = PostgresTestContainer::new().await; - for i in 0..20 { - let res = crate::setup::database::load_data(&pg.pool).await; - if let Err(e) = res { - error!("failed to load db because {e:?}. Retrying for 20s"); - tokio::time::sleep(std::time::Duration::from_secs(1)).await; - } else { - info!("successfully initalised the db in try {i}"); - break; - } - } + pg.load_data_retrying().await; let keys: Vec = sqlx::query_scalar!("SELECT key FROM de") .fetch_all(&pg.pool) @@ -164,7 +518,7 @@ mod tests { info!( "processed {resolved_cnt}/{all_keys_len} <=> {percentage}%", percentage = 100_f32 * (resolved_cnt as f32) / (all_keys_len as f32) - ) + ); } } @@ -193,7 +547,7 @@ mod tests { settings.set_sort_maps(true); settings.set_snapshot_path("location_get_handler"); settings.bind(|| { - insta::assert_json_snapshot!(key.clone(), body_value); + insta::assert_json_snapshot!(key.clone(), body_value, {".hash" => 0}); }); } } diff --git a/server/src/setup/tests.rs b/server/src/setup/tests.rs index 1decd930e..7177ca442 100644 --- a/server/src/setup/tests.rs +++ b/server/src/setup/tests.rs @@ -1,6 +1,7 @@ use meilisearch_sdk::client::Client; use testcontainers_modules::testcontainers::{ContainerAsync, ImageExt}; use testcontainers_modules::{meilisearch, testcontainers::runners::AsyncRunner}; +use tracing::{error, info}; pub struct PostgresTestContainer { _container: ContainerAsync, @@ -31,6 +32,20 @@ impl PostgresTestContainer { pool, } } + pub async fn load_data_retrying(&self) { + for i in 0..20 { + let res = crate::setup::database::load_data(&self.pool).await; + if let Err(e) = res { + error!("failed to load db because {e:?}. Retrying for 20s"); + tokio::time::sleep(std::time::Duration::from_secs(1)).await; + } else { + info!("successfully initalised the db in try {i}"); + return; + } + } + + panic!("could not initialise db after 20s") + } } pub struct MeiliSearchTestContainer { @@ -65,16 +80,7 @@ impl MeiliSearchTestContainer { #[tracing_test::traced_test] async fn test_db_setup() { let pg = PostgresTestContainer::new().await; - for i in 0..20 { - let res = crate::setup::database::load_data(&pg.pool).await; - if let Err(e) = res { - tracing::error!("failed to load db because {e:?}. Retrying for 20s"); - tokio::time::sleep(std::time::Duration::from_secs(1)).await; - } else { - tracing::info!("successfully initalised the db in try {i}"); - break; - } - } + pg.load_data_retrying().await; } #[tokio::test] diff --git a/webclient/app/api_types/index.ts b/webclient/app/api_types/index.ts index 6432a036f..d80b911ec 100644 --- a/webclient/app/api_types/index.ts +++ b/webclient/app/api_types/index.ts @@ -280,7 +280,7 @@ export type components = { readonly links?: readonly components["schemas"]["LinkProp"][]; /** * @description A comment to show to an entry. - * It is used in the rare cases, where some aspect about the rooom/.. or its translation are misleading. + * It is used in the rare cases, where some aspect about the room/.. or its translation are misleading. * An example of a room with a comment is MW1801. */ readonly comment?: string;