Skip to content

Commit 41f9ba5

Browse files
committed
Remove concurrency helpers from repository layer
Move concurrency orchestration logic out of the repository layer. These helpers (`begin_refresh`, `begin_fetch_next_page`, `complete_sync`, `complete_sync_with_error`) belong in a service layer since they: - Orchestrate multiple repository operations - Apply business rules (checking page counts, deciding state transitions) - Return domain-specific result types The repository now provides clean primitives that a service layer can compose: - `get_or_create_and_increment_version` for atomic create/increment - `update_state_by_list_metadata_id` for state changes - `get_header`, `get_items`, etc. for reads
1 parent 48e9dd0 commit 41f9ba5

File tree

1 file changed

+0
-360
lines changed

1 file changed

+0
-360
lines changed

wp_mobile_cache/src/repository/list_metadata.rs

Lines changed: 0 additions & 360 deletions
Original file line numberDiff line numberDiff line change
@@ -486,129 +486,6 @@ impl ListMetadataRepository {
486486
})
487487
.map_err(SqliteDbError::from)
488488
}
489-
490-
// ============================================================
491-
// Concurrency Helpers
492-
// ============================================================
493-
494-
/// Begin a refresh operation (fetch first page).
495-
///
496-
/// Atomically:
497-
/// 1. Creates header if needed and increments version
498-
/// 2. Updates state to FetchingFirstPage
499-
/// 3. Returns info needed for the fetch
500-
///
501-
/// Call this before starting an API fetch for page 1.
502-
pub fn begin_refresh(
503-
executor: &impl QueryExecutor,
504-
site: &DbSite,
505-
key: &ListKey,
506-
) -> Result<RefreshInfo, SqliteDbError> {
507-
log::debug!("ListMetadataRepository::begin_refresh: key={}", key);
508-
509-
// Get or create header and increment version in a single query
510-
let header_info = Self::get_or_create_and_increment_version(executor, site, key)?;
511-
512-
// Update state to fetching
513-
Self::update_state_by_list_metadata_id(
514-
executor,
515-
header_info.list_metadata_id,
516-
ListState::FetchingFirstPage,
517-
None,
518-
)?;
519-
520-
Ok(RefreshInfo {
521-
list_metadata_id: header_info.list_metadata_id,
522-
version: header_info.version,
523-
per_page: header_info.per_page,
524-
})
525-
}
526-
527-
/// Begin a load-next-page operation.
528-
///
529-
/// Atomically:
530-
/// 1. Gets current pagination state
531-
/// 2. Checks if there are more pages to load
532-
/// 3. Updates state to FetchingNextPage
533-
/// 4. Returns info needed for the fetch (including version for later check)
534-
///
535-
/// Returns None if already at the last page or no pages loaded yet.
536-
/// Call this before starting an API fetch for page N+1.
537-
pub fn begin_fetch_next_page(
538-
executor: &impl QueryExecutor,
539-
site: &DbSite,
540-
key: &ListKey,
541-
) -> Result<Option<FetchNextPageInfo>, SqliteDbError> {
542-
log::debug!("ListMetadataRepository::begin_fetch_next_page: key={}", key);
543-
544-
let header = match Self::get_header(executor, site, key)? {
545-
Some(h) => h,
546-
None => return Ok(None), // List doesn't exist
547-
};
548-
549-
// Check if we have pages loaded and more to fetch
550-
if header.current_page == 0 {
551-
return Ok(None); // No pages loaded yet, need refresh first
552-
}
553-
554-
if let Some(total_pages) = header.total_pages
555-
&& header.current_page >= total_pages
556-
{
557-
return Ok(None); // Already at last page
558-
}
559-
560-
let next_page = header.current_page + 1;
561-
562-
// Update state to fetching
563-
Self::update_state_by_list_metadata_id(
564-
executor,
565-
header.row_id,
566-
ListState::FetchingNextPage,
567-
None,
568-
)?;
569-
570-
Ok(Some(FetchNextPageInfo {
571-
list_metadata_id: header.row_id,
572-
page: next_page,
573-
version: header.version,
574-
per_page: header.per_page,
575-
}))
576-
}
577-
578-
/// Complete a sync operation successfully.
579-
///
580-
/// Updates state to Idle and clears any error message.
581-
pub fn complete_sync(
582-
executor: &impl QueryExecutor,
583-
list_metadata_id: RowId,
584-
) -> Result<(), SqliteDbError> {
585-
log::debug!(
586-
"ListMetadataRepository::complete_sync: list_metadata_id={}",
587-
list_metadata_id.0
588-
);
589-
Self::update_state_by_list_metadata_id(executor, list_metadata_id, ListState::Idle, None)
590-
}
591-
592-
/// Complete a sync operation with an error.
593-
///
594-
/// Updates state to Error with the provided message.
595-
pub fn complete_sync_with_error(
596-
executor: &impl QueryExecutor,
597-
list_metadata_id: RowId,
598-
error_message: &str,
599-
) -> Result<(), SqliteDbError> {
600-
log::debug!(
601-
"ListMetadataRepository::complete_sync_with_error: list_metadata_id={}, error={}",
602-
list_metadata_id.0,
603-
error_message
604-
);
605-
Self::update_state_by_list_metadata_id(
606-
executor,
607-
list_metadata_id,
608-
ListState::Error,
609-
Some(error_message),
610-
)
611-
}
612489
}
613490

614491
/// Header info returned from `get_or_create_and_increment_version`.
@@ -622,30 +499,6 @@ pub struct HeaderVersionInfo {
622499
pub per_page: i64,
623500
}
624501

625-
/// Information returned when starting a refresh operation.
626-
#[derive(Debug, Clone)]
627-
pub struct RefreshInfo {
628-
/// Row ID of the list_metadata record
629-
pub list_metadata_id: RowId,
630-
/// New version number (for concurrency checking)
631-
pub version: i64,
632-
/// Items per page setting
633-
pub per_page: i64,
634-
}
635-
636-
/// Information returned when starting a load-next-page operation.
637-
#[derive(Debug, Clone)]
638-
pub struct FetchNextPageInfo {
639-
/// Row ID of the list_metadata record
640-
pub list_metadata_id: RowId,
641-
/// Page number to fetch
642-
pub page: i64,
643-
/// Version at start (check before storing results)
644-
pub version: i64,
645-
/// Items per page setting
646-
pub per_page: i64,
647-
}
648-
649502
/// Input for creating a list metadata item.
650503
#[derive(Debug, Clone)]
651504
pub struct ListMetadataItemInput {
@@ -1228,217 +1081,4 @@ mod tests {
12281081
assert_eq!(item.entity_id, ((i + 1) * 100) as i64);
12291082
}
12301083
}
1231-
1232-
// ============================================================
1233-
// Concurrency Helper Tests
1234-
// ============================================================
1235-
1236-
#[rstest]
1237-
fn test_begin_refresh_creates_header_and_sets_state(test_ctx: TestContext) {
1238-
let key = ListKey::from("edit:posts:publish");
1239-
1240-
let info = ListMetadataRepository::begin_refresh(&test_ctx.conn, &test_ctx.site, &key)
1241-
.expect("should succeed");
1242-
1243-
// Verify version was incremented (from 0 to 1)
1244-
assert_eq!(info.version, 1);
1245-
assert_eq!(info.per_page, 20); // default
1246-
1247-
// Verify state is FetchingFirstPage
1248-
let state =
1249-
ListMetadataRepository::get_state_by_list_key(&test_ctx.conn, &test_ctx.site, &key)
1250-
.expect("should succeed");
1251-
assert_eq!(state, ListState::FetchingFirstPage);
1252-
}
1253-
1254-
#[rstest]
1255-
fn test_begin_refresh_increments_version_each_time(test_ctx: TestContext) {
1256-
let key = ListKey::from("edit:posts:draft");
1257-
1258-
let info1 = ListMetadataRepository::begin_refresh(&test_ctx.conn, &test_ctx.site, &key)
1259-
.expect("should succeed");
1260-
assert_eq!(info1.version, 1);
1261-
1262-
// Complete the first refresh
1263-
ListMetadataRepository::complete_sync(&test_ctx.conn, info1.list_metadata_id)
1264-
.expect("should succeed");
1265-
1266-
let info2 = ListMetadataRepository::begin_refresh(&test_ctx.conn, &test_ctx.site, &key)
1267-
.expect("should succeed");
1268-
assert_eq!(info2.version, 2);
1269-
}
1270-
1271-
#[rstest]
1272-
fn test_begin_fetch_next_page_returns_none_for_non_existent_list(test_ctx: TestContext) {
1273-
let result = ListMetadataRepository::begin_fetch_next_page(
1274-
&test_ctx.conn,
1275-
&test_ctx.site,
1276-
&ListKey::from("nonexistent"),
1277-
)
1278-
.expect("should succeed");
1279-
assert!(result.is_none());
1280-
}
1281-
1282-
#[rstest]
1283-
fn test_begin_fetch_next_page_returns_none_when_no_pages_loaded(test_ctx: TestContext) {
1284-
let key = ListKey::from("edit:posts:publish");
1285-
1286-
// Create header but don't set current_page
1287-
ListMetadataRepository::get_or_create(&test_ctx.conn, &test_ctx.site, &key)
1288-
.expect("should succeed");
1289-
1290-
let result =
1291-
ListMetadataRepository::begin_fetch_next_page(&test_ctx.conn, &test_ctx.site, &key)
1292-
.expect("should succeed");
1293-
assert!(result.is_none());
1294-
}
1295-
1296-
#[rstest]
1297-
fn test_begin_fetch_next_page_returns_none_at_last_page(test_ctx: TestContext) {
1298-
let key = ListKey::from("edit:posts:publish");
1299-
1300-
// Set up header with current_page = total_pages
1301-
let update = ListMetadataHeaderUpdate {
1302-
total_pages: Some(3),
1303-
total_items: Some(60),
1304-
current_page: 3,
1305-
per_page: 20,
1306-
};
1307-
ListMetadataRepository::update_header_by_list_key(
1308-
&test_ctx.conn,
1309-
&test_ctx.site,
1310-
&key,
1311-
&update,
1312-
)
1313-
.expect("should succeed");
1314-
1315-
let result =
1316-
ListMetadataRepository::begin_fetch_next_page(&test_ctx.conn, &test_ctx.site, &key)
1317-
.expect("should succeed");
1318-
assert!(result.is_none());
1319-
}
1320-
1321-
#[rstest]
1322-
fn test_begin_fetch_next_page_returns_info_when_more_pages(test_ctx: TestContext) {
1323-
let key = ListKey::from("edit:posts:publish");
1324-
1325-
// Set up header with more pages available
1326-
let update = ListMetadataHeaderUpdate {
1327-
total_pages: Some(5),
1328-
total_items: Some(100),
1329-
current_page: 2,
1330-
per_page: 20,
1331-
};
1332-
ListMetadataRepository::update_header_by_list_key(
1333-
&test_ctx.conn,
1334-
&test_ctx.site,
1335-
&key,
1336-
&update,
1337-
)
1338-
.expect("should succeed");
1339-
1340-
let result =
1341-
ListMetadataRepository::begin_fetch_next_page(&test_ctx.conn, &test_ctx.site, &key)
1342-
.expect("should succeed");
1343-
assert!(result.is_some());
1344-
1345-
let info = result.expect("should succeed");
1346-
assert_eq!(info.page, 3); // next page
1347-
assert_eq!(info.per_page, 20);
1348-
1349-
// Verify state changed to FetchingNextPage
1350-
let state =
1351-
ListMetadataRepository::get_state_by_list_key(&test_ctx.conn, &test_ctx.site, &key)
1352-
.expect("should succeed");
1353-
assert_eq!(state, ListState::FetchingNextPage);
1354-
}
1355-
1356-
#[rstest]
1357-
fn test_complete_sync_sets_state_to_idle(test_ctx: TestContext) {
1358-
let key = ListKey::from("edit:posts:publish");
1359-
1360-
let info = ListMetadataRepository::begin_refresh(&test_ctx.conn, &test_ctx.site, &key)
1361-
.expect("should succeed");
1362-
ListMetadataRepository::complete_sync(&test_ctx.conn, info.list_metadata_id)
1363-
.expect("should succeed");
1364-
1365-
let state =
1366-
ListMetadataRepository::get_state_by_list_key(&test_ctx.conn, &test_ctx.site, &key)
1367-
.expect("should succeed");
1368-
assert_eq!(state, ListState::Idle);
1369-
}
1370-
1371-
#[rstest]
1372-
fn test_complete_sync_with_error_sets_state_and_message(test_ctx: TestContext) {
1373-
let key = ListKey::from("edit:posts:publish");
1374-
1375-
let info = ListMetadataRepository::begin_refresh(&test_ctx.conn, &test_ctx.site, &key)
1376-
.expect("should succeed");
1377-
ListMetadataRepository::complete_sync_with_error(
1378-
&test_ctx.conn,
1379-
info.list_metadata_id,
1380-
"Network timeout",
1381-
)
1382-
.expect("should succeed");
1383-
1384-
let state_record = ListMetadataRepository::get_state_by_list_metadata_id(
1385-
&test_ctx.conn,
1386-
info.list_metadata_id,
1387-
)
1388-
.expect("query should succeed")
1389-
.expect("should succeed");
1390-
assert_eq!(state_record.state, ListState::Error);
1391-
assert_eq!(
1392-
state_record.error_message.as_deref(),
1393-
Some("Network timeout")
1394-
);
1395-
}
1396-
1397-
#[rstest]
1398-
fn test_version_check_detects_stale_operation(test_ctx: TestContext) {
1399-
let key = ListKey::from("edit:posts:publish");
1400-
1401-
// Start a refresh (version becomes 1)
1402-
let refresh_info =
1403-
ListMetadataRepository::begin_refresh(&test_ctx.conn, &test_ctx.site, &key)
1404-
.expect("should succeed");
1405-
assert_eq!(refresh_info.version, 1);
1406-
1407-
// Update header to simulate page 1 loaded
1408-
let update = ListMetadataHeaderUpdate {
1409-
total_pages: Some(5),
1410-
total_items: Some(100),
1411-
current_page: 1,
1412-
per_page: 20,
1413-
};
1414-
ListMetadataRepository::update_header_by_list_key(
1415-
&test_ctx.conn,
1416-
&test_ctx.site,
1417-
&key,
1418-
&update,
1419-
)
1420-
.expect("should succeed");
1421-
ListMetadataRepository::complete_sync(&test_ctx.conn, refresh_info.list_metadata_id)
1422-
.expect("should succeed");
1423-
1424-
// Start load-next-page (captures version = 1)
1425-
let next_page_info =
1426-
ListMetadataRepository::begin_fetch_next_page(&test_ctx.conn, &test_ctx.site, &key)
1427-
.expect("query should succeed")
1428-
.expect("should succeed");
1429-
let captured_version = next_page_info.version;
1430-
1431-
// Another refresh happens (version becomes 2)
1432-
ListMetadataRepository::begin_refresh(&test_ctx.conn, &test_ctx.site, &key)
1433-
.expect("should succeed");
1434-
1435-
// Version check should fail (stale)
1436-
let current_version =
1437-
ListMetadataRepository::get_version(&test_ctx.conn, &test_ctx.site, &key)
1438-
.expect("should succeed");
1439-
assert_ne!(
1440-
current_version, captured_version,
1441-
"Version should not match after refresh"
1442-
);
1443-
}
14441084
}

0 commit comments

Comments
 (0)