Rework /room/{}/item and pagination query

- Now it use `skipToken` and `top` to (mostly) align to OData spec. - Its response type is now a normal struct and is documented. - Room metadata is now excluded from room item query.
2025-05-01 00:31:09 +00:00 · 2024-09-03 01:59:31 -04:00 · 2024-09-03 01:59:31 -04:00 · b05f704406
commit b05f704406
parent 77216aa0f8
6 changed files with 158 additions and 196 deletions
--- a/Cargo.lock
+++ b/Cargo.lock
@ -29,21 +29,6 @@ dependencies = [
 "zerocopy",
 ]

-[[package]]
-name = "android-tzdata"
-version = "0.1.1"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "e999941b234f3131b00bc13c22d06e8c5ff726d1b6318ac7eb276997bbb4fef0"
-
-[[package]]
-name = "android_system_properties"
-version = "0.1.5"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "819e7219dbd41043ac279b19830f2efc897156490d7fd6ea916720117ee66311"
-dependencies = [
- "libc",
-]
-
 [[package]]
 name = "anstream"
 version = "0.6.15"
@ -309,14 +294,15 @@ dependencies = [
 "rusqlite",
 "sd-notify",
 "serde",
- "serde-aux",
 "serde-inline-default",
 "serde_json",
+ "serde_urlencoded",
 "tokio",
 "tokio-stream",
 "tower-http",
 "tracing",
 "tracing-subscriber",
+ "url",
 "uuid",
 ]

@ -362,18 +348,6 @@ version = "1.0.0"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "baf1de4339761588bc0619e3cbc0120ee582ebb74b53b4efbf79117bd2da40fd"

-[[package]]
-name = "chrono"
-version = "0.4.38"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "a21f936df1771bf62b77f047b726c4625ff2e8aa607c01ec06e5a05bd8463401"
-dependencies = [
- "android-tzdata",
- "iana-time-zone",
- "num-traits",
- "windows-targets",
-]
-
 [[package]]
 name = "clap"
 version = "4.5.16"
@ -892,29 +866,6 @@ dependencies = [
 "tracing",
 ]

-[[package]]
-name = "iana-time-zone"
-version = "0.1.60"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "e7ffbb5a1b541ea2561f8c41c087286cc091e21e556a4f09a8f6cbf17b69b141"
-dependencies = [
- "android_system_properties",
- "core-foundation-sys",
- "iana-time-zone-haiku",
- "js-sys",
- "wasm-bindgen",
- "windows-core",
-]
-
-[[package]]
-name = "iana-time-zone-haiku"
-version = "0.1.2"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "f31827a206f56af32e590ba56d5d2d085f558508192593743f16b2306495269f"
-dependencies = [
- "cc",
-]
-
 [[package]]
 name = "idna"
 version = "0.5.0"
@ -1062,15 +1013,6 @@ dependencies = [
 "winapi",
 ]

-[[package]]
-name = "num-traits"
-version = "0.2.19"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "071dfc062690e90b734c0b2273ce72ad0ffa95f0c74596bc250dcfd960262841"
-dependencies = [
- "autocfg",
-]
-
 [[package]]
 name = "object"
 version = "0.36.3"
@ -1468,17 +1410,6 @@ dependencies = [
 "serde_derive",
 ]

-[[package]]
-name = "serde-aux"
-version = "4.5.0"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "0d2e8bfba469d06512e11e3311d4d051a4a387a5b42d010404fecf3200321c95"
-dependencies = [
- "chrono",
- "serde",
- "serde_json",
-]
-
 [[package]]
 name = "serde-inline-default"
 version = "0.2.0"
@ -2004,6 +1935,7 @@ dependencies = [
 "form_urlencoded",
 "idna",
 "percent-encoding",
+ "serde",
 ]

 [[package]]
@ -2166,15 +2098,6 @@ version = "0.4.0"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "712e227841d057c1ee1cd2fb22fa7e5a5461ae8e48fa2ca79ec42cfc1931183f"

-[[package]]
-name = "windows-core"
-version = "0.52.0"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "33ab640c8d7e35bf8ba19b884ba838ceb4fba93a4e8c65a9059d08afcfc683d9"
-dependencies = [
- "windows-targets",
-]
-
 [[package]]
 name = "windows-registry"
 version = "0.2.0"
--- a/blahd/Cargo.toml
+++ b/blahd/Cargo.toml
@ -7,6 +7,7 @@ edition = "2021"
 anyhow = "1"
 axum = { version = "0.7", features = ["ws"] }
 axum-extra = "0.9"
+basic-toml = "0.1.9"
 clap = { version = "4", features = ["derive"] }
 ed25519-dalek = "2"
 futures-util = "0.3"
@ -15,18 +16,18 @@ humantime = "2"
 rusqlite = { version = "0.32", features = ["uuid"] }
 sd-notify = "0.4"
 serde = { version = "1", features = ["derive"] }
-serde-aux = "4"
+serde-inline-default = "0.2.0"
 serde_json = "1"
+serde_urlencoded = "0.7.1"
 tokio = { version = "1", features = ["macros", "rt-multi-thread", "sync", "time"] }
 tokio-stream = { version = "0.1", features = ["sync"] }
 tower-http = { version = "0.5", features = ["cors", "limit"] }
 tracing = "0.1"
 tracing-subscriber = "0.3"
+url = { version = "2.5.2", features = ["serde"] }
 uuid = { version = "1", features = ["v4"] }

 blah = { path = "..", features = ["rusqlite"] }
-basic-toml = "0.1.9"
-serde-inline-default = "0.2.0"

 [lints]
 workspace = true
--- a/blahd/docs/webapi.yaml
+++ b/blahd/docs/webapi.yaml
@ -16,23 +16,6 @@ paths:
        interested in (eg. chat from joined rooms).
        The message has type `Outgoing` in `blahd/src/ws.rs`.

-  /room:
-    get:
-      summary: Get room metadata
-      responses:
-        200:
-          content:
-            application/json:
-              schema:
-                $ref: '#/components/schemas/RoomMetadata'
-        404:
-          description: |
-            Room does not exist or the user does not have permission to get metadata of it.
-          content:
-            application/json:
-              schema:
-                $ref: '#/components/schemas/ApiError'
-
  /room/create:
    post:
      summary: Create a new room
@ -66,6 +49,24 @@ paths:
            application/json:
              $ref: '#/components/schemas/ApiError'

+  /room/{ruuid}:
+    get:
+      summary: Get room metadata
+      responses:
+        200:
+          content:
+            application/json:
+              schema:
+                $ref: '#/components/schemas/RoomMetadata'
+        404:
+          description: |
+            Room does not exist or the user does not have permission to get metadata of it.
+          content:
+            application/json:
+              schema:
+                $ref: '#/components/schemas/ApiError'
+
+
  /room/{ruuid}/feed.json:
    get:
      summary: JSON feed of room {ruuid}, which must be public readable
@ -95,9 +96,16 @@ paths:
          schema:
            $ret: WithSig<AuthPayload>
      parameters:
-        before_id:
-          description: Filter items before (not including) a given chat id (cid).
+        top:
          in: query
+          description: |
+            The maximum number of items to return. This is an advice and may be
+            ignored by server.
+        skipToken:
+          in: query
+          description: |
+            Retrieve the next page of items, by providing the last item's `cid`
+            from the previous response.
      responses:
        200:
          content:
@ -195,3 +203,15 @@ components:
          type: string
        attrs:
          type: int64
+
+    RoomItems:
+      type: object
+      required:
+        - items
+      properties:
+        items:
+          type: array
+          items:
+            $ref: 'WithSig<ChatPayload>'
+        skip_token:
+          type: string
--- a/blahd/src/config.rs
+++ b/blahd/src/config.rs
@ -4,6 +4,7 @@ use std::time::Duration;
 use anyhow::{ensure, Result};
 use serde::{Deserialize, Deserializer};
 use serde_inline_default::serde_inline_default;
+use url::Url;

 #[derive(Debug, Clone, Deserialize)]
 #[serde(deny_unknown_fields)]
@ -25,7 +26,7 @@ pub struct DatabaseConfig {
 #[serde(deny_unknown_fields)]
 pub struct ServerConfig {
    pub listen: String,
-    pub base_url: String,
+    pub base_url: Url,

    #[serde_inline_default(1024)]
    pub max_page_len: usize,
@ -52,8 +53,8 @@ fn de_duration_sec<'de, D: Deserializer<'de>>(de: D) -> Result<Duration, D::Erro
 impl Config {
    pub fn validate(&self) -> Result<()> {
        ensure!(
-            !self.server.base_url.ends_with("/"),
-            "base_url must not have trailing slash",
+            !self.server.base_url.cannot_be_a_base(),
+            "base_url must be able to be a base",
        );
        Ok(())
    }
--- a/blahd/src/main.rs
+++ b/blahd/src/main.rs
@ -1,3 +1,4 @@
+use std::num::NonZeroUsize;
 use std::path::PathBuf;
 use std::sync::{Arc, Mutex};
 use std::time::{Duration, SystemTime};
@ -17,8 +18,9 @@ use blah::types::{
 use config::Config;
 use ed25519_dalek::SIGNATURE_LENGTH;
 use middleware::{ApiError, OptionalAuth, SignedJson};
-use rusqlite::{named_params, params, OptionalExtension, Row};
+use rusqlite::{named_params, params, Connection, OptionalExtension, Row};
 use serde::{Deserialize, Serialize};
+use url::Url;
 use utils::ExpiringSet;
 use uuid::Uuid;

@ -91,9 +93,6 @@ impl AppState {
    fn init(config: Config, conn: rusqlite::Connection) -> Result<Self> {
        static INIT_SQL: &str = include_str!("../init.sql");

-        // Should be validated by `Config`.
-        assert!(!config.server.base_url.ends_with('/'));
-
        conn.execute_batch(INIT_SQL)
            .context("failed to initialize database")?;
        Ok(Self {
@ -278,28 +277,42 @@ async fn room_create(
    Ok(Json(ruuid))
 }

-// NB. `next_url` generation depends on this structure.
-#[derive(Debug, Deserialize)]
-struct GetRoomItemParams {
-    #[serde(
-        default,
-        deserialize_with = "serde_aux::field_attributes::deserialize_number_from_string"
-    )]
-    before_id: u64,
-    page_len: Option<usize>,
+/// Pagination query parameters.
+///
+/// Field names are inspired by Microsoft's design, which is an extension to OData spec.
+/// See: <https://learn.microsoft.com/en-us/graph/query-parameters#odata-system-query-options>
+#[derive(Debug, Clone, Copy, Default, Serialize, Deserialize)]
+#[serde(deny_unknown_fields, rename_all = "camelCase")]
+struct Pagination {
+    /// A opaque token from previous response to fetch the next page.
+    skip_token: Option<u64>,
+    /// Maximum page size.
+    top: Option<NonZeroUsize>,
+}
+
+#[derive(Debug, Serialize)]
+struct RoomItems {
+    items: Vec<ChatItem>,
+    #[serde(skip_serializing_if = "Option::is_none")]
+    skip_token: Option<String>,
 }

 async fn room_get_item(
    st: ArcState,
    WithRejection(Path(ruuid), _): WithRejection<Path<Uuid>, ApiError>,
-    WithRejection(params, _): WithRejection<Query<GetRoomItemParams>, ApiError>,
+    WithRejection(Query(pagination), _): WithRejection<Query<Pagination>, ApiError>,
    OptionalAuth(user): OptionalAuth,
-) -> Result<impl IntoResponse, ApiError> {
-    let (room_meta, items) = query_room_items(&st, ruuid, user.as_ref(), &params)?;
-
-    // TODO: This format is to-be-decided. Or do we even need this interface other than
-    // `feed.json`?
-    Ok(Json((room_meta, items)))
+) -> Result<Json<RoomItems>, ApiError> {
+    let (items, skip_token) = {
+        let conn = st.conn.lock().unwrap();
+        get_room_if_readable(&conn, ruuid, user.as_ref(), |_row| Ok(()))?;
+        query_room_items(&st, &conn, ruuid, pagination)?
+    };
+    let items = items.into_iter().map(|(_, item)| item).collect();
+    Ok(Json(RoomItems {
+        items,
+        skip_token: skip_token.map(|x| x.to_string()),
+    }))
 }

 async fn room_get_metadata(
@ -307,24 +320,28 @@ async fn room_get_metadata(
    WithRejection(Path(ruuid), _): WithRejection<Path<Uuid>, ApiError>,
    OptionalAuth(user): OptionalAuth,
 ) -> Result<Json<RoomMetadata>, ApiError> {
-    let (room_meta, _) = query_room_items(
-        &st,
-        ruuid,
-        user.as_ref(),
-        &GetRoomItemParams {
-            before_id: 0,
-            page_len: Some(0),
-        },
-    )?;
-    Ok(Json(room_meta))
+    let (title, attrs) =
+        get_room_if_readable(&st.conn.lock().unwrap(), ruuid, user.as_ref(), |row| {
+            Ok((
+                row.get::<_, String>("title")?,
+                row.get::<_, RoomAttrs>("attrs")?,
+            ))
+        })?;
+
+    Ok(Json(RoomMetadata { title, attrs }))
 }

 async fn room_get_feed(
    st: ArcState,
    WithRejection(Path(ruuid), _): WithRejection<Path<Uuid>, ApiError>,
-    params: Query<GetRoomItemParams>,
+    Query(pagination): Query<Pagination>,
 ) -> Result<impl IntoResponse, ApiError> {
-    let (room_meta, items) = query_room_items(&st, ruuid, None, &params)?;
+    let title;
+    let (items, skip_token) = {
+        let conn = st.conn.lock().unwrap();
+        title = get_room_if_readable(&conn, ruuid, None, |row| row.get::<_, String>("title"))?;
+        query_room_items(&st, &conn, ruuid, pagination)?
+    };

    let items = items
        .into_iter()
@ -347,19 +364,28 @@ async fn room_get_feed(
        })
        .collect::<Vec<_>>();

-    let page_len = params
-        .page_len
-        .unwrap_or(st.config.server.max_page_len)
-        .min(st.config.server.max_page_len);
-
-    let base_url = &st.config.server.base_url;
-    let feed_url = format!("{base_url}/room/{ruuid}/feed.json");
-    let next_url = (items.len() == page_len).then(|| {
-        let last_id = &items.last().expect("page size is not 0").id;
-        format!("{feed_url}?before_id={last_id}")
+    let feed_url = st
+        .config
+        .server
+        .base_url
+        .join(&format!("/room/{ruuid}/feed.json"))
+        .expect("base_url must be valid");
+    let next_url = skip_token.map(|skip_token| {
+        let next_params = Pagination {
+            skip_token: Some(skip_token),
+            top: pagination.top,
+        };
+        let mut next_url = feed_url.clone();
+        {
+            let mut query = next_url.query_pairs_mut();
+            let ser = serde_urlencoded::Serializer::new(&mut query);
+            next_params.serialize(ser).unwrap();
+            query.finish();
+        }
+        next_url
    });
    let feed = FeedRoom {
-        title: room_meta.title,
+        title,
        items,
        next_url,
        feed_url,
@ -376,9 +402,9 @@ async fn room_get_feed(
 #[serde(tag = "version", rename = "https://jsonfeed.org/version/1.1")]
 struct FeedRoom {
    title: String,
-    feed_url: String,
+    feed_url: Url,
    #[serde(skip_serializing_if = "Option::is_none")]
-    next_url: Option<String>,
+    next_url: Option<Url>,
    items: Vec<FeedItem>,
 }

@ -405,7 +431,7 @@ struct FeedItemExtra {
    sig: [u8; SIGNATURE_LENGTH],
 }

-#[derive(Debug, Serialize, Deserialize)]
+#[derive(Debug, Serialize)]
 pub struct RoomMetadata {
    pub title: String,
    pub attrs: RoomAttrs,
@ -440,40 +466,28 @@ fn get_room_if_readable<T>(
    .ok_or_else(|| error_response!(StatusCode::NOT_FOUND, "not_found", "room not found"))
 }

+type ChatItemWithId = (u64, ChatItem);
+
+/// Get room items with pagination parameters,
+/// return a page of items and the next skip_token if this is not the last page.
 fn query_room_items(
    st: &AppState,
+    conn: &Connection,
    ruuid: Uuid,
-    user: Option<&UserKey>,
-    params: &GetRoomItemParams,
-) -> Result<(RoomMetadata, Vec<(u64, ChatItem)>), ApiError> {
-    let conn = st.conn.lock().unwrap();
-
-    let (rid, title, attrs) = get_room_if_readable(&conn, ruuid, user, |row| {
-        Ok((
-            row.get::<_, u64>("rid")?,
-            row.get::<_, String>("title")?,
-            row.get::<_, RoomAttrs>("attrs")?,
-        ))
-    })?;
-
-    let room_meta = RoomMetadata { title, attrs };
-
-    if params.page_len == Some(0) {
-        return Ok((room_meta, Vec::new()));
-    }
-
-    let page_len = params
-        .page_len
-        .unwrap_or(st.config.server.max_page_len)
+    pagination: Pagination,
+) -> Result<(Vec<ChatItemWithId>, Option<u64>), ApiError> {
+    let page_len = pagination
+        .top
+        .map_or(usize::MAX, |n| n.get())
        .min(st.config.server.max_page_len);
-
    let mut stmt = conn.prepare(
        r"
        SELECT `cid`, `timestamp`, `nonce`, `sig`, `userkey`, `sig`, `rich_text`
-        FROM `room_item`
+        FROM `room`
+        JOIN `room_item` USING (`rid`)
        JOIN `user` USING (`uid`)
-        WHERE `rid` = :rid AND
-            (:before_cid = 0 OR `cid` < :before_cid)
+        WHERE `ruuid` = :ruuid AND
+            (:before_cid IS NULL OR `cid` < :before_cid)
        ORDER BY `cid` DESC
        LIMIT :limit
        ",
@ -481,8 +495,8 @@ fn query_room_items(
    let items = stmt
        .query_and_then(
            named_params! {
-                ":rid": rid,
-                ":before_cid": params.before_id,
+                ":ruuid": ruuid,
+                ":before_cid": pagination.skip_token,
                ":limit": page_len,
            },
            |row| {
@ -503,8 +517,10 @@ fn query_room_items(
            },
        )?
        .collect::<rusqlite::Result<Vec<_>>>()?;
+    let skip_token =
+        (items.len() == page_len).then(|| items.last().expect("page must not be empty").0);

-    Ok((room_meta, items))
+    Ok((items, skip_token))
 }

 async fn room_post_item(
--- a/pages/main.js
+++ b/pages/main.js
@ -178,25 +178,26 @@ async function connectRoom(url) {

    log(`fetching room: ${url}`);

-    const auth = await signData({ typ: 'auth' });
-    fetch(
-        `${url}/item`,
-        {
-            headers: {
-                'Authorization': auth,
-            },
-        },
-    )
-    .then(async (resp) => {
-        return [resp.status, await resp.json()];
-    })
-    // TODO: This response format is to-be-decided.
+    const genFetchOpts = async () => ({ headers: { 'Authorization': await signData({ typ: 'auth' }) } });
+    genFetchOpts()
+    .then(opts => fetch(url, opts))
+    .then(async (resp) => { return [resp.status, await resp.json()]; })
    .then(async ([status, json]) => {
        if (status !== 200) throw new Error(`status ${status}: ${json.error.message}`);
-        const [{ title }, items] = json
-        document.title = `room: ${title}`
+        document.title = `room: ${json.title}`
+    })
+    .catch((e) => {
+        log(`failed to get room metadata: ${e}`);
+    });
+
+    genFetchOpts()
+    .then(opts => fetch(`${url}/item`, opts))
+    .then(async (resp) => { return [resp.status, await resp.json()]; })
+    .then(async ([status, json]) => {
+        if (status !== 200) throw new Error(`status ${status}: ${json.error.message}`);
+        const { items } = json
        items.reverse();
-        for (const [_cid, chat] of items) {
+        for (const chat of items) {
            await showChatMsg(chat);
        }
        log('---history---');