-
Notifications
You must be signed in to change notification settings - Fork 0
UI/API: templates ETag + pagination (client cache, 304 handling) #16
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
8d4ab62
8cef9ac
091c8d3
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -20,6 +20,10 @@ pub struct TemplateQuery { | |||||||||
| pub group: Option<String>, | ||||||||||
| pub featured: Option<bool>, | ||||||||||
| pub since: Option<String>, // ISO8601 timestamp for incremental sync | ||||||||||
| pub page: Option<i64>, | ||||||||||
| pub per_page: Option<i64>, | ||||||||||
| // Lightweight ETag support via query param (clients may also mirror this to If-None-Match if desired) | ||||||||||
| pub etag: Option<String>, | ||||||||||
| } | ||||||||||
|
|
||||||||||
| /// 模板响应 | ||||||||||
|
|
@@ -119,14 +123,14 @@ pub async fn get_templates( | |||||||||
| _ => "name", | ||||||||||
| }; | ||||||||||
|
|
||||||||||
| let query_str = format!( | ||||||||||
| "SELECT id, {} as name, name_en, name_zh, description, classification, color, icon, | ||||||||||
| category_group, is_featured, is_active, global_usage_count, tags, version, | ||||||||||
| let base_select = format!( | ||||||||||
| "SELECT id, {} as name, name_en, name_zh, description, classification, color, icon, \ | ||||||||||
| category_group, is_featured, is_active, global_usage_count, tags, version, \ | ||||||||||
| created_at, updated_at FROM system_category_templates WHERE is_active = true", | ||||||||||
| name_field | ||||||||||
| ); | ||||||||||
|
|
||||||||||
| let mut query = sqlx::QueryBuilder::new(query_str); | ||||||||||
| let mut query = sqlx::QueryBuilder::new(base_select.clone()); | ||||||||||
|
|
||||||||||
| // 添加过滤条件 | ||||||||||
| if let Some(classification) = ¶ms.r#type { | ||||||||||
|
|
@@ -151,9 +155,54 @@ pub async fn get_templates( | |||||||||
| query.push(" AND updated_at > "); | ||||||||||
| query.push_bind(since); | ||||||||||
| } | ||||||||||
|
|
||||||||||
|
|
||||||||||
| // Stats for ETag and total (duplicate the same filters) | ||||||||||
| let mut stats_q = sqlx::QueryBuilder::new( | ||||||||||
| "SELECT COALESCE(MAX(updated_at), to_timestamp(0)) AS max_updated, COUNT(*) AS total FROM system_category_templates WHERE is_active = true" | ||||||||||
| ); | ||||||||||
| if let Some(classification) = ¶ms.r#type { | ||||||||||
| let classification = classification.to_lowercase(); | ||||||||||
| if classification != "all" { | ||||||||||
| stats_q.push(" AND classification = "); | ||||||||||
| stats_q.push_bind(classification); | ||||||||||
| } | ||||||||||
| } | ||||||||||
| if let Some(group) = ¶ms.group { | ||||||||||
| stats_q.push(" AND category_group = "); | ||||||||||
| stats_q.push_bind(group); | ||||||||||
| } | ||||||||||
| if let Some(featured) = params.featured { | ||||||||||
| stats_q.push(" AND is_featured = "); | ||||||||||
| stats_q.push_bind(featured); | ||||||||||
| } | ||||||||||
| if let Some(since) = ¶ms.since { | ||||||||||
| stats_q.push(" AND updated_at > "); | ||||||||||
| stats_q.push_bind(since); | ||||||||||
| } | ||||||||||
|
Comment on lines
+159
to
+181
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This block for building the
Comment on lines
+159
to
+181
|
||||||||||
| let stats_row = stats_q | ||||||||||
| .build() | ||||||||||
| .fetch_one(&pool) | ||||||||||
| .await | ||||||||||
| .map_err(|_| StatusCode::INTERNAL_SERVER_ERROR)?; | ||||||||||
| let max_updated: chrono::DateTime<chrono::Utc> = stats_row.try_get("max_updated").unwrap_or(chrono::DateTime::<chrono::Utc>::from_timestamp(0, 0).unwrap()); | ||||||||||
| let total_count: i64 = stats_row.try_get("total").unwrap_or(0); | ||||||||||
|
Comment on lines
+187
to
+188
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Using
Suggested change
|
||||||||||
|
|
||||||||||
| // Compute a simple ETag and return 304 if matches | ||||||||||
| let computed_etag = format!("W/\"{}:{}\"", max_updated.timestamp(), total_count); | ||||||||||
| if let Some(client_etag) = ¶ms.etag { | ||||||||||
| if *client_etag == computed_etag { | ||||||||||
| return Err(StatusCode::NOT_MODIFIED); | ||||||||||
| } | ||||||||||
| } | ||||||||||
|
|
||||||||||
| // Pagination | ||||||||||
| let per_page = params.per_page.unwrap_or(50).clamp(1, 100) as i64; | ||||||||||
| let page = params.page.unwrap_or(1).max(1) as i64; | ||||||||||
| let offset = (page - 1) * per_page; | ||||||||||
|
|
||||||||||
| query.push(" ORDER BY is_featured DESC, global_usage_count DESC, name"); | ||||||||||
|
|
||||||||||
| query.push(" LIMIT ").push_bind(per_page).push(" OFFSET ").push_bind(offset); | ||||||||||
|
|
||||||||||
| let templates = query | ||||||||||
| .build_query_as::<SystemTemplate>() | ||||||||||
| .fetch_all(&pool) | ||||||||||
|
|
@@ -162,18 +211,12 @@ pub async fn get_templates( | |||||||||
| eprintln!("Database query error: {:?}", e); | ||||||||||
| StatusCode::INTERNAL_SERVER_ERROR | ||||||||||
| })?; | ||||||||||
|
|
||||||||||
| // 获取总数 | ||||||||||
| let total: (i64,) = sqlx::query_as("SELECT COUNT(*) FROM system_category_templates WHERE is_active = true") | ||||||||||
| .fetch_one(&pool) | ||||||||||
| .await | ||||||||||
| .map_err(|_| StatusCode::INTERNAL_SERVER_ERROR)?; | ||||||||||
|
|
||||||||||
| // 使用前面统计的 total_count 和 max_updated | ||||||||||
| let response = TemplateResponse { | ||||||||||
| templates, | ||||||||||
| version: "1.0.0".to_string(), | ||||||||||
| last_updated: chrono::Utc::now().to_rfc3339(), | ||||||||||
| total: total.0, | ||||||||||
| last_updated: max_updated.to_rfc3339(), | ||||||||||
| total: total_count, | ||||||||||
|
Comment on lines
215
to
+219
|
||||||||||
| }; | ||||||||||
|
|
||||||||||
| Ok(Json(response)) | ||||||||||
|
|
@@ -432,4 +475,4 @@ pub async fn submit_usage( | |||||||||
| } | ||||||||||
|
|
||||||||||
| StatusCode::OK | ||||||||||
| } | ||||||||||
| } | ||||||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,60 +1,184 @@ | ||||||||||||||||||||||
| import 'package:flutter/material.dart'; | ||||||||||||||||||||||
| import 'package:flutter_riverpod/flutter_riverpod.dart'; | ||||||||||||||||||||||
| import '../../models/category.dart'; | ||||||||||||||||||||||
|
||||||||||||||||||||||
| import '../../models/category.dart'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This empty catch block silently swallows any errors that occur while fetching templates. This leads to a poor user experience, as the user will be shown an empty import dialog without any indication of what went wrong. The error should be caught and communicated to the user, for example, by showing a SnackBar, and the loading indicator should be correctly handled.
| templates = await CategoryServiceIntegrated().getAllTemplates(forceRefresh: true); | |
| } catch (_) {} | |
| templates = await CategoryServiceIntegrated().getAllTemplates(forceRefresh: true); | |
| } catch (e) { | |
| if (mounted) { | |
| ScaffoldMessenger.of(context).showSnackBar(SnackBar(content: Text('Error loading templates: $e'))); | |
| } | |
| setState(() { _busy = false; }); | |
| return; | |
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's correct to lowercase the
classificationhere for a case-insensitive comparison. However, the same logic is missing when applying this filter to the mainquerybuilder (around line 139). This inconsistency can cause thetotal_countto mismatch the number of returned templates, which is a critical bug. Please ensure the filtering logic is identical in both places. This issue also highlights the risk of the duplicated filtering logic.