From c1c5a36a24976eb72d177041e02dbe9aaf3ea9dd Mon Sep 17 00:00:00 2001 From: omprakash Date: Thu, 14 May 2026 13:14:30 +0530 Subject: [PATCH 1/9] refactor(sdk): extract shared helpers to dedupe field/screen/row/util code MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Consolidate repeated patterns into single sources of truth so future changes are made in one place instead of 10-20. New helpers: - utils/date_helpers.dart — parseDateTime, parseTime, formatDurationSeconds shared by date/datetime/duration fields and form_builder patch normaliser - utils/frappe_json_utils.dart — parseBool: canonical 0/1/'true'/bool coercion for every model fromJson - utils/sql_row_utils.dart — retryCountFrom, lastAttemptAtFrom, utcMillisFrom, parseEnumByName for sqflite row factories - ui/widgets/fields/field_helpers.dart — requiredValidator, baseFieldDecoration, fieldErrorText shared by every Form*Field - ui/widgets/screen_helpers.dart — showStatusSnackBar + showConfirmDialog replace ad-hoc SnackBar/Dialog construction across screens Schema/query additions: - system_columns: systemSyncMetadataColumnNames + linkCompanionColumnDDL consumed by PayloadAssembler, PayloadSerializer, parent/child schema builders and runtime migration so strip rules cannot diverge - table_name: stripDocsPrefix inverse of normalizeDoctypeTableName - query_result: QueryResult.ofRows derives hasMore + returnedCount for online/offline parity in UnifiedResolver No behaviour change. flutter analyze + dart format clean across the package. --- lib/src/api/attachment_service.dart | 6 +- lib/src/api/auth.dart | 5 +- lib/src/api/doctype_service.dart | 13 +- lib/src/api/document_service.dart | 16 +- lib/src/api/oauth2_helper.dart | 94 +++---- lib/src/api/utils.dart | 28 +- lib/src/database/app_database.dart | 39 +-- lib/src/database/daos/doctype_dao.dart | 39 +-- lib/src/database/daos/doctype_meta_dao.dart | 76 ++---- lib/src/database/daos/outbox_dao.dart | 125 ++++----- lib/src/database/schema/child_schema.dart | 5 +- lib/src/database/schema/parent_schema.dart | 7 +- lib/src/database/schema/system_columns.dart | 34 +++ lib/src/database/table_name.dart | 17 +- lib/src/models/app_config.dart | 85 +++--- lib/src/models/doc_field.dart | 13 +- lib/src/models/doc_type_meta.dart | 22 +- lib/src/models/outbox_row.dart | 25 +- lib/src/models/pending_attachment.dart | 23 +- lib/src/models/workflow_transition.dart | 5 +- lib/src/query/filter_errors.dart | 19 +- lib/src/query/filter_parser.dart | 130 ++++++--- lib/src/query/query_result.dart | 18 ++ lib/src/query/unified_resolver.dart | 59 ++-- lib/src/screens/mobile_home_screen.dart | 95 ++----- lib/src/sdk/frappe_sdk.dart | 120 +++++---- lib/src/services/auth_service.dart | 57 ++-- lib/src/services/link_option_service.dart | 42 +-- lib/src/services/meta_differ.dart | 6 +- lib/src/services/meta_migration.dart | 3 +- lib/src/services/meta_service.dart | 70 ++--- lib/src/services/offline_repository.dart | 50 ++-- lib/src/services/sync_controller.dart | 26 +- lib/src/services/sync_service.dart | 98 +++---- lib/src/sync/attachment_pipeline.dart | 16 +- lib/src/sync/payload_assembler.dart | 30 +-- lib/src/sync/payload_serializer.dart | 25 +- lib/src/sync/pull_engine.dart | 5 +- lib/src/sync/push_engine.dart | 24 +- lib/src/ui/document_list_screen.dart | 21 +- lib/src/ui/form_screen.dart | 147 ++++------ lib/src/ui/login_screen.dart | 25 +- lib/src/ui/offline_transition_screen.dart | 23 +- lib/src/ui/screens/sync_errors_screen.dart | 33 +-- lib/src/ui/sync_status_screen.dart | 59 ++-- lib/src/ui/widgets/fields/attach_field.dart | 27 +- .../ui/widgets/fields/child_table_field.dart | 23 +- lib/src/ui/widgets/fields/data_field.dart | 8 +- lib/src/ui/widgets/fields/date_field.dart | 20 +- lib/src/ui/widgets/fields/datetime_field.dart | 20 +- lib/src/ui/widgets/fields/duration_field.dart | 20 +- lib/src/ui/widgets/fields/field_helpers.dart | 51 ++++ lib/src/ui/widgets/fields/image_field.dart | 25 +- lib/src/ui/widgets/fields/link_field.dart | 55 ++-- lib/src/ui/widgets/fields/numeric_field.dart | 10 +- lib/src/ui/widgets/fields/password_field.dart | 8 +- lib/src/ui/widgets/fields/rating_field.dart | 25 +- lib/src/ui/widgets/fields/select_field.dart | 15 +- lib/src/ui/widgets/fields/text_field.dart | 17 +- lib/src/ui/widgets/fields/time_field.dart | 8 +- lib/src/ui/widgets/form_builder.dart | 101 ++----- lib/src/ui/widgets/screen_helpers.dart | 254 ++++++++++++++++++ lib/src/utils/api_tracer.dart | 26 +- lib/src/utils/date_helpers.dart | 62 +++++ lib/src/utils/frappe_json_utils.dart | 16 ++ lib/src/utils/sql_row_utils.dart | 43 +++ 66 files changed, 1461 insertions(+), 1251 deletions(-) create mode 100644 lib/src/ui/widgets/fields/field_helpers.dart create mode 100644 lib/src/ui/widgets/screen_helpers.dart create mode 100644 lib/src/utils/date_helpers.dart create mode 100644 lib/src/utils/frappe_json_utils.dart create mode 100644 lib/src/utils/sql_row_utils.dart diff --git a/lib/src/api/attachment_service.dart b/lib/src/api/attachment_service.dart index 3adce47..c023ddd 100644 --- a/lib/src/api/attachment_service.dart +++ b/lib/src/api/attachment_service.dart @@ -3,6 +3,7 @@ import 'dart:io'; import 'rest_helper.dart'; +import 'utils.dart'; class AttachmentService { final RestHelper _restHelper; @@ -37,9 +38,6 @@ class AttachmentService { fields: fields, ); - if (response is Map && response.containsKey('message')) { - return response['message'] as Map; - } - return response as Map; + return unwrapMessage>(response); } } diff --git a/lib/src/api/auth.dart b/lib/src/api/auth.dart index d379417..541e88e 100644 --- a/lib/src/api/auth.dart +++ b/lib/src/api/auth.dart @@ -1,6 +1,8 @@ // Copyright (c) 2026, Bhushan Barbuddhe and contributors // For license information, please see license.txt +import 'package:flutter/foundation.dart' show debugPrint; + import 'rest_helper.dart'; abstract class SessionStorage { @@ -41,8 +43,7 @@ class AuthService { try { await _restHelper.post('/api/method/mobile_auth.logout'); } catch (e, st) { - // ignore: avoid_print - print( + debugPrint( 'AuthApi: server logout failed (continuing local cleanup) — $e\n$st', ); } finally { diff --git a/lib/src/api/doctype_service.dart b/lib/src/api/doctype_service.dart index 8368fda..f0582e8 100644 --- a/lib/src/api/doctype_service.dart +++ b/lib/src/api/doctype_service.dart @@ -8,6 +8,7 @@ import 'package:flutter/foundation.dart'; import 'exceptions.dart'; import 'rest_helper.dart'; +import 'utils.dart'; class DoctypeService { final RestHelper _restHelper; @@ -148,10 +149,7 @@ class DoctypeService { Future> getByName(String doctype, String name) async { final response = await _restHelper.get('/api/resource/$doctype/$name'); - if (response is Map && response.containsKey('data')) { - return response['data'] as Map; - } - return response as Map; + return unwrapData>(response); } /// Bulk-fetch full parent docs (with embedded child rows) via the @@ -207,10 +205,13 @@ class DoctypeService { ); if (nameList.isEmpty) return []; + // Use `?.toString()` (matches listChildDocs) so int-valued `name` + // fields from Frappe's autoname-by-numeric-series — which historically + // dropped silently under the `is String` check — round-trip correctly. final names = [ for (final n in nameList) - if (n is Map && n['name'] is String) - (n['name'] as String), + if (n is Map) + if (n['name']?.toString() case final String s when s.isNotEmpty) s, ]; if (names.isEmpty) return []; diff --git a/lib/src/api/document_service.dart b/lib/src/api/document_service.dart index ec3ed3a..d68e343 100644 --- a/lib/src/api/document_service.dart +++ b/lib/src/api/document_service.dart @@ -3,6 +3,7 @@ import 'dart:convert'; import 'rest_helper.dart'; +import 'utils.dart'; class DocumentService { final RestHelper _restHelper; @@ -19,19 +20,13 @@ class DocumentService { '/api/method/frappe.client.insert', body: {'doc': jsonEncode(data..['doctype'] = doctype)}, ); - if (response is Map && response.containsKey('message')) { - return response['message'] as Map; - } - return response as Map; + return unwrapMessage>(response); } else { final response = await _restHelper.post( '/api/resource/$doctype', body: data, ); - if (response is Map && response.containsKey('data')) { - return response['data'] as Map; - } - return response as Map; + return unwrapData>(response); } } @@ -44,10 +39,7 @@ class DocumentService { '/api/resource/$doctype/$name', body: data, ); - if (response is Map && response.containsKey('data')) { - return response['data'] as Map; - } - return response as Map; + return unwrapData>(response); } Future deleteDocument(String doctype, String name) async { diff --git a/lib/src/api/oauth2_helper.dart b/lib/src/api/oauth2_helper.dart index b4c7245..b17b001 100644 --- a/lib/src/api/oauth2_helper.dart +++ b/lib/src/api/oauth2_helper.dart @@ -4,6 +4,41 @@ import 'dart:math'; import 'package:crypto/crypto.dart'; import 'package:http/http.dart' as http; +/// Builds a Frappe `api/method/` URI from [baseUrl], normalising +/// the trailing slash so `baseUrl` with or without a trailing `/` +/// produces the same URI. Single source of truth for OAuth endpoint URL +/// construction. +Uri _oauthUri(String baseUrl, String path) { + final root = baseUrl.endsWith('/') ? baseUrl : '$baseUrl/'; + return Uri.parse('${root}api/method/$path'); +} + +/// Performs a form-encoded `application/x-www-form-urlencoded` POST to +/// [uri], throws `Exception('$errorLabel failed: ...')` on non-200, and +/// returns the decoded JSON map. Shared by [OAuth2Helper.exchangeCodeForToken] +/// and [OAuth2Helper.refreshToken] which previously had byte-for-byte +/// identical http.post calls. +Future> _postFormEncoded( + Uri uri, + Map body, + String errorLabel, +) async { + final response = await http.post( + uri, + headers: { + 'Content-Type': 'application/x-www-form-urlencoded', + 'Accept': 'application/json', + }, + body: body.keys.map((k) => '$k=${Uri.encodeComponent(body[k]!)}').join('&'), + ); + if (response.statusCode != 200) { + throw Exception( + '$errorLabel failed: ${response.statusCode} ${response.body}', + ); + } + return jsonDecode(response.body) as Map; +} + /// PKCE code verifier and challenge pair (RFC 7636). class PkcePair { final String codeVerifier; @@ -81,10 +116,7 @@ class OAuth2Helper { String codeChallengeMethod = 'S256', String responseType = 'code', }) { - final path = baseUrl.endsWith('/') ? baseUrl : '$baseUrl/'; - final uri = Uri.parse( - '${path}api/method/frappe.integrations.oauth2.authorize', - ); + final uri = _oauthUri(baseUrl, 'frappe.integrations.oauth2.authorize'); final q = { 'client_id': clientId, 'redirect_uri': redirectUri, @@ -111,10 +143,7 @@ class OAuth2Helper { String? codeVerifier, String? clientSecret, }) async { - final path = baseUrl.endsWith('/') ? baseUrl : '$baseUrl/'; - final uri = Uri.parse( - '${path}api/method/frappe.integrations.oauth2.get_token', - ); + final uri = _oauthUri(baseUrl, 'frappe.integrations.oauth2.get_token'); final body = { 'grant_type': 'authorization_code', 'code': code, @@ -127,22 +156,7 @@ class OAuth2Helper { if (clientSecret != null && clientSecret.isNotEmpty) { body['client_secret'] = clientSecret; } - final response = await http.post( - uri, - headers: { - 'Content-Type': 'application/x-www-form-urlencoded', - 'Accept': 'application/json', - }, - body: body.keys - .map((k) => '$k=${Uri.encodeComponent(body[k]!)}') - .join('&'), - ); - if (response.statusCode != 200) { - throw Exception( - 'OAuth token exchange failed: ${response.statusCode} ${response.body}', - ); - } - final json = jsonDecode(response.body) as Map; + final json = await _postFormEncoded(uri, body, 'OAuth token exchange'); if (json['error'] != null) { throw Exception( 'OAuth error: ${json['error']} - ${json['error_description'] ?? ''}', @@ -161,10 +175,7 @@ class OAuth2Helper { required String refreshToken, String? clientSecret, }) async { - final path = baseUrl.endsWith('/') ? baseUrl : '$baseUrl/'; - final uri = Uri.parse( - '${path}api/method/frappe.integrations.oauth2.get_token', - ); + final uri = _oauthUri(baseUrl, 'frappe.integrations.oauth2.get_token'); final body = { 'grant_type': 'refresh_token', 'refresh_token': refreshToken, @@ -173,22 +184,7 @@ class OAuth2Helper { if (clientSecret != null && clientSecret.isNotEmpty) { body['client_secret'] = clientSecret; } - final response = await http.post( - uri, - headers: { - 'Content-Type': 'application/x-www-form-urlencoded', - 'Accept': 'application/json', - }, - body: body.keys - .map((k) => '$k=${Uri.encodeComponent(body[k]!)}') - .join('&'), - ); - if (response.statusCode != 200) { - throw Exception( - 'OAuth refresh failed: ${response.statusCode} ${response.body}', - ); - } - final json = jsonDecode(response.body) as Map; + final json = await _postFormEncoded(uri, body, 'OAuth refresh'); return OAuth2TokenResponse.fromJson(json); } @@ -197,10 +193,7 @@ class OAuth2Helper { required String baseUrl, required String accessToken, }) async { - final path = baseUrl.endsWith('/') ? baseUrl : '$baseUrl/'; - final uri = Uri.parse( - '${path}api/method/frappe.integrations.oauth2.openid_profile', - ); + final uri = _oauthUri(baseUrl, 'frappe.integrations.oauth2.openid_profile'); final response = await http.get( uri, headers: {'Authorization': 'Bearer $accessToken'}, @@ -218,10 +211,7 @@ class OAuth2Helper { required String baseUrl, required String token, }) async { - final path = baseUrl.endsWith('/') ? baseUrl : '$baseUrl/'; - final uri = Uri.parse( - '${path}api/method/frappe.integrations.oauth2.revoke_token', - ); + final uri = _oauthUri(baseUrl, 'frappe.integrations.oauth2.revoke_token'); await http.post( uri, headers: {'Content-Type': 'application/x-www-form-urlencoded'}, diff --git a/lib/src/api/utils.dart b/lib/src/api/utils.dart index 154b19c..3bb760e 100644 --- a/lib/src/api/utils.dart +++ b/lib/src/api/utils.dart @@ -2,6 +2,28 @@ // For license information, please see license.txt import 'dart:convert'; +import 'package:flutter/foundation.dart' show debugPrint; + +/// Frappe wraps API responses inconsistently: most `frappe.client.*` and +/// `frappe.*.method` calls return `{"message": ...}` while REST resource +/// endpoints return `{"data": ...}`. Service callers that know which +/// envelope to expect use [unwrapMessage] / [unwrapData] to strip it, +/// keeping the two strip-decisions in one place. The bare-response +/// fallback (no envelope) is returned unchanged so non-Frappe / direct- +/// payload endpoints continue to work. +T unwrapMessage(dynamic response) { + if (response is Map && response.containsKey('message')) { + return response['message'] as T; + } + return response as T; +} + +T unwrapData(dynamic response) { + if (response is Map && response.containsKey('data')) { + return response['data'] as T; + } + return response as T; +} Map parseSetCookie(String setCookieValue) { final cookies = {}; @@ -34,7 +56,9 @@ String extractErrorMessage(dynamic body) { } } catch (e, st) { // ignore: avoid_print - print('extractErrorMessage: _server_messages parse failed — $e\n$st'); + debugPrint( + 'extractErrorMessage: _server_messages parse failed — $e\n$st', + ); } } @@ -83,7 +107,7 @@ String? _extractServerMessage(dynamic body) { } } catch (e, st) { // ignore: avoid_print - print('_extractServerMessage: jsonDecode failed — $e\n$st'); + debugPrint('_extractServerMessage: jsonDecode failed — $e\n$st'); } } return null; diff --git a/lib/src/database/app_database.dart b/lib/src/database/app_database.dart index 92798ba..bc8a154 100644 --- a/lib/src/database/app_database.dart +++ b/lib/src/database/app_database.dart @@ -282,20 +282,33 @@ class AppDatabase { } } + /// Selects table names from `sqlite_master` matching [whereClause] (with + /// optional bind [args]) and runs `DROP TABLE IF EXISTS ""` for + /// each. Shared by [wipeOfflineDocumentTables] (drops `docs__*` mirrors + /// only) and [_clearAllDataInternal] (drops everything except SQLite + /// internals) so the predicate is the only thing that varies. + static Future _dropTablesWhere( + DatabaseExecutor txn, + String whereClause, [ + List? args, + ]) async { + final tables = await txn.rawQuery( + "SELECT name FROM sqlite_master WHERE type='table' AND $whereClause", + args, + ); + for (final r in tables) { + final name = r['name'] as String; + await txn.execute('DROP TABLE IF EXISTS "$name"'); + } + } + /// Drops every `docs__` table and clears `outbox`, /// `pending_attachments`, `link_options`. Preserves `doctype_meta`, /// `auth_tokens`, `doctype_permission`, `sdk_meta`. Used by the /// offline → online transition (Spec §7.5). Future wipeOfflineDocumentTables() async { await _db.transaction((txn) async { - final tables = await txn.rawQuery( - "SELECT name FROM sqlite_master WHERE type='table' " - "AND name LIKE 'docs\\_\\_%' ESCAPE '\\'", - ); - for (final r in tables) { - final name = r['name'] as String; - await txn.execute('DROP TABLE IF EXISTS "$name"'); - } + await _dropTablesWhere(txn, r"name LIKE 'docs\_\_%' ESCAPE '\'"); await txn.delete('outbox'); await txn.delete('pending_attachments'); await txn.delete('link_options'); @@ -330,15 +343,7 @@ class AppDatabase { /// 'sqlite_%'`: if it's not a SQLite internal, it gets dropped. static Future _clearAllDataInternal(Database db) async { await db.transaction((txn) async { - final tables = await txn.rawQuery( - "SELECT name FROM sqlite_master " - "WHERE type='table' " - "AND name NOT LIKE 'sqlite_%'", - ); - for (final r in tables) { - final tableName = r['name'] as String; - await txn.execute('DROP TABLE IF EXISTS "$tableName"'); - } + await _dropTablesWhere(txn, "name NOT LIKE 'sqlite_%'"); }); // Recreate the base schema. _onCreate brings back: doctype_meta diff --git a/lib/src/database/daos/doctype_dao.dart b/lib/src/database/daos/doctype_dao.dart index 2cfd3e1..3c0e049 100644 --- a/lib/src/database/daos/doctype_dao.dart +++ b/lib/src/database/daos/doctype_dao.dart @@ -7,35 +7,42 @@ class DoctypeDao { DoctypeDao(this._db, {required String tableName}) : _table = tableName; - Future insert(Map row) async { + /// Stamps a new (or upsert-new) row with `mobile_uuid` and + /// `local_modified` if the caller didn't already provide them. + /// Shared by [insert] and the insert branch of [upsertByServerName] + /// so identity stamping cannot diverge between the two write paths. + void _stamp(Map row) { row.putIfAbsent('mobile_uuid', () => const Uuid().v4()); row.putIfAbsent( 'local_modified', () => DateTime.now().toUtc().millisecondsSinceEpoch, ); - await _db.insert(_table, row, conflictAlgorithm: ConflictAlgorithm.abort); } - Future?> findByMobileUuid(String uuid) async { + /// Returns the first matching row or null. Used by [findByMobileUuid] + /// and [findByServerName] — keeps the query shape (limit, copy-out) + /// in a single place. + Future?> _findOneWhere(String col, Object value) async { final rows = await _db.query( _table, - where: 'mobile_uuid = ?', - whereArgs: [uuid], + where: '$col = ?', + whereArgs: [value], limit: 1, ); return rows.isEmpty ? null : Map.from(rows.first); } - Future?> findByServerName(String name) async { - final rows = await _db.query( - _table, - where: 'server_name = ?', - whereArgs: [name], - limit: 1, - ); - return rows.isEmpty ? null : Map.from(rows.first); + Future insert(Map row) async { + _stamp(row); + await _db.insert(_table, row, conflictAlgorithm: ConflictAlgorithm.abort); } + Future?> findByMobileUuid(String uuid) => + _findOneWhere('mobile_uuid', uuid); + + Future?> findByServerName(String name) => + _findOneWhere('server_name', name); + Future>> findByStatus(String status) async { final rows = await _db.query( _table, @@ -80,11 +87,7 @@ class DoctypeDao { } else { final row = Map.from(fields); row['server_name'] = serverName; - row.putIfAbsent('mobile_uuid', () => const Uuid().v4()); - row.putIfAbsent( - 'local_modified', - () => DateTime.now().toUtc().millisecondsSinceEpoch, - ); + _stamp(row); await _db.insert(_table, row); } } diff --git a/lib/src/database/daos/doctype_meta_dao.dart b/lib/src/database/daos/doctype_meta_dao.dart index bc7caa5..0807a7e 100644 --- a/lib/src/database/daos/doctype_meta_dao.dart +++ b/lib/src/database/daos/doctype_meta_dao.dart @@ -1,11 +1,22 @@ import 'package:sqflite/sqflite.dart'; import '../entities/doctype_meta_entity.dart'; +import '../table_name.dart'; class DoctypeMetaDao { final Database _database; DoctypeMetaDao(this._database); + /// Resolves the persisted table_name override for [doctype], falling back + /// to [normalizeDoctypeTableName] if no override row exists. Replaces the + /// `metaDao.getTableName(doctype) ?? normalizeDoctypeTableName(doctype)` + /// idiom previously inlined in `UnifiedResolver`, `PullEngine`, and + /// `PushEngine` so a fallback-strategy change (cache, logging, alternate + /// naming) is made in exactly one place. + Future tableNameFor(String doctype) async { + return await getTableName(doctype) ?? normalizeDoctypeTableName(doctype); + } + Future findByDoctype(String doctype) async { final maps = await _database.query( 'doctype_meta', @@ -114,18 +125,25 @@ class DoctypeMetaDao { ); } - Future getTableName(String doctype) async { + /// Reads a single text column from `doctype_meta` for the given doctype, + /// returning null when the row is absent or the column is null. Shared + /// implementation backing every single-column getter on this DAO so + /// schema- or query-shape changes only need editing here. + Future _readStringCol(String doctype, String column) async { final rows = await _database.query( 'doctype_meta', - columns: ['table_name'], + columns: [column], where: 'doctype = ?', whereArgs: [doctype], limit: 1, ); if (rows.isEmpty) return null; - return rows.first['table_name'] as String?; + return rows.first[column] as String?; } + Future getTableName(String doctype) => + _readStringCol(doctype, 'table_name'); + Future setMetaWatermark(String doctype, String watermark) async { await _database.update( 'doctype_meta', @@ -135,17 +153,8 @@ class DoctypeMetaDao { ); } - Future getMetaWatermark(String doctype) async { - final rows = await _database.query( - 'doctype_meta', - columns: ['meta_watermark'], - where: 'doctype = ?', - whereArgs: [doctype], - limit: 1, - ); - if (rows.isEmpty) return null; - return rows.first['meta_watermark'] as String?; - } + Future getMetaWatermark(String doctype) => + _readStringCol(doctype, 'meta_watermark'); Future setDepGraphJson(String doctype, String depGraphJson) async { await _database.update( @@ -156,17 +165,8 @@ class DoctypeMetaDao { ); } - Future getDepGraphJson(String doctype) async { - final rows = await _database.query( - 'doctype_meta', - columns: ['dep_graph_json'], - where: 'doctype = ?', - whereArgs: [doctype], - limit: 1, - ); - if (rows.isEmpty) return null; - return rows.first['dep_graph_json'] as String?; - } + Future getDepGraphJson(String doctype) => + _readStringCol(doctype, 'dep_graph_json'); Future setLastOkCursor(String doctype, String cursorJson) async { await _database.update( @@ -189,17 +189,8 @@ class DoctypeMetaDao { ); } - Future getLastOkCursor(String doctype) async { - final rows = await _database.query( - 'doctype_meta', - columns: ['last_ok_cursor'], - where: 'doctype = ?', - whereArgs: [doctype], - limit: 1, - ); - if (rows.isEmpty) return null; - return rows.first['last_ok_cursor'] as String?; - } + Future getLastOkCursor(String doctype) => + _readStringCol(doctype, 'last_ok_cursor'); Future markEntryPoint(String doctype, bool isEntryPoint) async { await _database.update( @@ -221,17 +212,8 @@ class DoctypeMetaDao { /// Lighter accessors for offline-first meta sync — avoid round-tripping /// through [DoctypeMetaEntity] when only the JSON blob is needed. - Future getMetaJson(String doctype) async { - final rows = await _database.query( - 'doctype_meta', - columns: ['metaJson'], - where: 'doctype = ?', - whereArgs: [doctype], - limit: 1, - ); - if (rows.isEmpty) return null; - return rows.first['metaJson'] as String?; - } + Future getMetaJson(String doctype) => + _readStringCol(doctype, 'metaJson'); Future upsertMetaJson(String doctype, String metaJson) async { final updated = await _database.update( diff --git a/lib/src/database/daos/outbox_dao.dart b/lib/src/database/daos/outbox_dao.dart index ba37de8..234ed34 100644 --- a/lib/src/database/daos/outbox_dao.dart +++ b/lib/src/database/daos/outbox_dao.dart @@ -7,6 +7,30 @@ import '../../models/outbox_row.dart'; /// knew about the doc, so there is nothing to push). enum RecordSaveResult { enqueued, cancelledLocally } +/// The four outbox states that can be collapsed into a newer save: +/// `pending`, `failed`, `blocked`, `conflict`. `in_flight` and `done` +/// are deliberately excluded (a save during an in-flight push always +/// inserts a fresh follow-up row). +final List _collapsableStateWireNames = [ + OutboxState.pending.wireName, + OutboxState.failed.wireName, + OutboxState.blocked.wireName, + OutboxState.conflict.wireName, +]; + +/// SQL WHERE clause that matches outbox rows in any collapsable state +/// for a given (doctype, mobileUuid). Used by [OutboxDao.recordSave] +/// (for both the collapse-candidate lookup and the DELETE-cancels-prior +/// branch) and [OutboxDao.cancelPendingFor]. The four `?` placeholders +/// after `state IN` are filled from [_collapsableStateWireNames]. +const String _collapsableWhereClause = + 'doctype = ? AND mobile_uuid = ? AND state IN (?, ?, ?, ?)'; + +/// Builds the whereArgs list to go with [_collapsableWhereClause]: +/// `[doctype, mobileUuid, pending, failed, blocked, conflict]`. +List _collapsableWhereArgs(String doctype, String mobileUuid) => + [doctype, mobileUuid, ..._collapsableStateWireNames]; + class OutboxDao { final DatabaseExecutor _db; @@ -42,18 +66,11 @@ class OutboxDao { required OutboxOperation operation, DateTime? createdAt, }) async { - final ts = (createdAt ?? DateTime.now().toUtc()).millisecondsSinceEpoch; + final stampedAt = createdAt ?? DateTime.now().toUtc(); final existing = await _db.query( 'outbox', - where: 'doctype = ? AND mobile_uuid = ? AND state IN (?, ?, ?, ?)', - whereArgs: [ - doctype, - mobileUuid, - OutboxState.pending.wireName, - OutboxState.failed.wireName, - OutboxState.blocked.wireName, - OutboxState.conflict.wireName, - ], + where: _collapsableWhereClause, + whereArgs: _collapsableWhereArgs(doctype, mobileUuid), orderBy: 'created_at ASC, id ASC', ); @@ -64,15 +81,8 @@ class OutboxDao { if (hasInsert) { await _db.delete( 'outbox', - where: 'doctype = ? AND mobile_uuid = ? AND state IN (?, ?, ?, ?)', - whereArgs: [ - doctype, - mobileUuid, - OutboxState.pending.wireName, - OutboxState.failed.wireName, - OutboxState.blocked.wireName, - OutboxState.conflict.wireName, - ], + where: _collapsableWhereClause, + whereArgs: _collapsableWhereArgs(doctype, mobileUuid), ); return RecordSaveResult.cancelledLocally; } @@ -85,19 +95,15 @@ class OutboxDao { doctype, mobileUuid, 'UPDATE', - OutboxState.pending.wireName, - OutboxState.failed.wireName, - OutboxState.blocked.wireName, - OutboxState.conflict.wireName, + ..._collapsableStateWireNames, ], ); - await _db.insert('outbox', { - 'doctype': doctype, - 'mobile_uuid': mobileUuid, - 'operation': operation.wireName, - 'state': OutboxState.pending.wireName, - 'created_at': ts, - }); + await insertPending( + doctype: doctype, + mobileUuid: mobileUuid, + operation: operation, + createdAt: stampedAt, + ); return RecordSaveResult.enqueued; } @@ -105,13 +111,12 @@ class OutboxDao { operation == OutboxOperation.cancel) { // SUBMIT/CANCEL never collapse — they're docstatus transitions // distinct from prior INSERT/UPDATE rows. - await _db.insert('outbox', { - 'doctype': doctype, - 'mobile_uuid': mobileUuid, - 'operation': operation.wireName, - 'state': OutboxState.pending.wireName, - 'created_at': ts, - }); + await insertPending( + doctype: doctype, + mobileUuid: mobileUuid, + operation: operation, + createdAt: stampedAt, + ); return RecordSaveResult.enqueued; } @@ -121,16 +126,7 @@ class OutboxDao { orElse: () => const {}, ); if (existingInsert.isNotEmpty) { - await _db.update( - 'outbox', - { - 'state': OutboxState.pending.wireName, - 'error_code': null, - 'error_message': null, - }, - where: 'id = ?', - whereArgs: [existingInsert['id']], - ); + await resetToPending(existingInsert['id'] as int); return RecordSaveResult.enqueued; } final existingUpdate = existing.firstWhere( @@ -138,25 +134,15 @@ class OutboxDao { orElse: () => const {}, ); if (existingUpdate.isNotEmpty) { - await _db.update( - 'outbox', - { - 'state': OutboxState.pending.wireName, - 'error_code': null, - 'error_message': null, - }, - where: 'id = ?', - whereArgs: [existingUpdate['id']], - ); + await resetToPending(existingUpdate['id'] as int); return RecordSaveResult.enqueued; } - await _db.insert('outbox', { - 'doctype': doctype, - 'mobile_uuid': mobileUuid, - 'operation': operation.wireName, - 'state': OutboxState.pending.wireName, - 'created_at': ts, - }); + await insertPending( + doctype: doctype, + mobileUuid: mobileUuid, + operation: operation, + createdAt: stampedAt, + ); return RecordSaveResult.enqueued; } @@ -169,15 +155,8 @@ class OutboxDao { }) async { await _db.delete( 'outbox', - where: 'doctype = ? AND mobile_uuid = ? AND state IN (?, ?, ?, ?)', - whereArgs: [ - doctype, - mobileUuid, - OutboxState.pending.wireName, - OutboxState.failed.wireName, - OutboxState.blocked.wireName, - OutboxState.conflict.wireName, - ], + where: _collapsableWhereClause, + whereArgs: _collapsableWhereArgs(doctype, mobileUuid), ); } diff --git a/lib/src/database/schema/child_schema.dart b/lib/src/database/schema/child_schema.dart index f10a590..0d59bec 100644 --- a/lib/src/database/schema/child_schema.dart +++ b/lib/src/database/schema/child_schema.dart @@ -1,5 +1,6 @@ import '../../models/doc_type_meta.dart'; import '../field_type_mapping.dart'; +import '../table_name.dart'; import 'system_columns.dart'; /// Column names emitted by the child system block. A meta field that uses @@ -34,11 +35,11 @@ List buildChildSchemaDDL( if (sqlType == null) continue; cols.add('$name $sqlType'); if (isLinkFieldType(type)) { - cols.add('${name}__is_local INTEGER'); + cols.add(linkCompanionColumnDDL(name)); } } - final suffix = tableName.replaceFirst('docs__', ''); + final suffix = stripDocsPrefix(tableName); return [ 'CREATE TABLE $tableName (\n ${cols.join(',\n ')}\n)', 'CREATE UNIQUE INDEX ix_${suffix}_server_name ' diff --git a/lib/src/database/schema/parent_schema.dart b/lib/src/database/schema/parent_schema.dart index c8d0f55..4ee8e1d 100644 --- a/lib/src/database/schema/parent_schema.dart +++ b/lib/src/database/schema/parent_schema.dart @@ -1,5 +1,6 @@ import '../../models/doc_type_meta.dart'; import '../field_type_mapping.dart'; +import '../table_name.dart'; import 'index_policy.dart'; import 'system_columns.dart'; @@ -48,7 +49,7 @@ List buildParentSchemaDDL( cols.add('$name $sqlType'); if (isLinkFieldType(type)) { - cols.add('${name}__is_local INTEGER'); + cols.add(linkCompanionColumnDDL(name)); } if (normFields.contains(name) && sqlType == 'TEXT') { @@ -58,7 +59,7 @@ List buildParentSchemaDDL( final ddl = ['CREATE TABLE $tableName (\n ${cols.join(',\n ')}\n)']; - final suffix = _indexSuffix(tableName); + final suffix = stripDocsPrefix(tableName); ddl.add( 'CREATE UNIQUE INDEX ix_${suffix}_server_name ' 'ON $tableName(server_name) WHERE server_name IS NOT NULL', @@ -81,6 +82,4 @@ List buildParentSchemaDDL( return ddl; } -String _indexSuffix(String tableName) => tableName.replaceFirst('docs__', ''); - String _sanitizeColName(String col) => col.replaceAll('__', '_'); diff --git a/lib/src/database/schema/system_columns.dart b/lib/src/database/schema/system_columns.dart index a79d16c..e6a6a3c 100644 --- a/lib/src/database/schema/system_columns.dart +++ b/lib/src/database/schema/system_columns.dart @@ -34,3 +34,37 @@ const systemChildColumnNames = { 'idx', 'modified', }; + +/// SDK-internal sync metadata column names — the subset of system columns +/// that must be stripped from any payload going to Frappe. Distinct from +/// [systemParentColumnNames] because Frappe still expects `docstatus` and +/// `modified` on the wire, so this set EXCLUDES those. +/// +/// Single source of truth — [PayloadAssembler] (assembles outbound +/// payloads) and [PayloadSerializer] (builds `ThreeWayMerge` base +/// snapshots) both consume this set so the two strip-decisions cannot +/// diverge silently and leak sync metadata into the wire or into a merge +/// base. +const systemSyncMetadataColumnNames = { + // Identity / link columns — emitted explicitly by the caller. + 'mobile_uuid', + 'server_name', + // Per-doc sync state. + 'sync_status', + 'sync_error', + 'error_code', + 'sync_attempts', + 'last_attempt_at', + 'sync_op', + 'push_base_payload', + // Local bookkeeping. + 'local_modified', + 'pulled_at', +}; + +/// Canonical column-definition fragment for a Link field's `__is_local` +/// companion column. Used by parent and child schema builders and by the +/// runtime ALTER TABLE migration so a column-name or type change is made +/// in exactly one place. +String linkCompanionColumnDDL(String fieldName) => + '${fieldName}__is_local INTEGER'; diff --git a/lib/src/database/table_name.dart b/lib/src/database/table_name.dart index 9016fc2..06ca615 100644 --- a/lib/src/database/table_name.dart +++ b/lib/src/database/table_name.dart @@ -26,11 +26,18 @@ String normalizeDoctypeTableName(String doctype, {String prefix = 'docs__'}) { .replaceAll(RegExp(r'_+'), '_') .replaceAll(RegExp(r'^_|_$'), ''); if (body.isEmpty) { - throw ArgumentError.value( - doctype, - 'doctype', - 'normalized to empty string', - ); + throw ArgumentError.value(doctype, 'doctype', 'normalized to empty string'); } return '$prefix$body'; } + +/// Returns the doctype suffix portion of a `docs__` table name — +/// the inverse of the `docs__` prefix added by [normalizeDoctypeTableName]. +/// If [tableName] is not prefixed (passes through other code paths), the +/// input is returned unchanged. +/// +/// Single source of truth — `parent_schema.dart` (index naming), +/// `child_schema.dart` (index naming), and `meta_differ.dart` (diff suffix) +/// all call this so a prefix change is made in exactly one place. +String stripDocsPrefix(String tableName) => + tableName.replaceFirst('docs__', ''); diff --git a/lib/src/models/app_config.dart b/lib/src/models/app_config.dart index b3b349a..677d736 100644 --- a/lib/src/models/app_config.dart +++ b/lib/src/models/app_config.dart @@ -65,6 +65,49 @@ class LoginConfig { ); } + /// Builds [LoginConfig] from a JSON map supporting both snake_case and + /// camelCase keys (the local config-file format vs the API JSON format). + /// Delegated to from [AppConfig.fromJsonFile] so adding a new field to + /// [LoginConfig] only needs one factory edit instead of two. + factory LoginConfig.fromJsonFile(Map json) { + final socialProvidersJson = + (json['social_providers'] ?? json['socialProviders']) + as List? ?? + const []; + return LoginConfig( + enablePasswordLogin: + json['enable_password_login'] as bool? ?? + json['enablePasswordLogin'] as bool? ?? + true, + enableOAuth: + json['enable_oauth'] as bool? ?? + json['enableOAuth'] as bool? ?? + false, + enableSocialLogin: + json['enable_social_login'] as bool? ?? + json['enableSocialLogin'] as bool? ?? + false, + enableMobileLogin: + json['enable_mobile_login'] as bool? ?? + json['enableMobileLogin'] as bool? ?? + false, + oauthClientId: + json['oauth_client_id'] as String? ?? + json['oauthClientId'] as String?, + oauthClientSecret: + json['oauth_client_secret'] as String? ?? + json['oauthClientSecret'] as String?, + socialProviders: socialProvidersJson + .whereType>() + .map(SocialProviderConfig.fromJson) + .toList(), + autoDiscoverSocialProviders: + json['auto_discover_social_providers'] as bool? ?? + json['autoDiscoverSocialProviders'] as bool? ?? + true, + ); + } + Map toJson() { return { 'enablePasswordLogin': enablePasswordLogin, @@ -112,45 +155,9 @@ class AppConfig { /// Builds [AppConfig] from a JSON map (supports snake_case and camelCase). static AppConfig fromJsonFile(Map json) { final login = json['login_config'] ?? json['loginConfig']; - LoginConfig? loginConfig; - if (login is Map) { - final socialProvidersJson = - (login['social_providers'] ?? login['socialProviders']) - as List? ?? - const []; - loginConfig = LoginConfig( - enablePasswordLogin: - login['enable_password_login'] as bool? ?? - login['enablePasswordLogin'] as bool? ?? - true, - enableOAuth: - login['enable_oauth'] as bool? ?? - login['enableOAuth'] as bool? ?? - false, - enableSocialLogin: - login['enable_social_login'] as bool? ?? - login['enableSocialLogin'] as bool? ?? - false, - enableMobileLogin: - login['enable_mobile_login'] as bool? ?? - login['enableMobileLogin'] as bool? ?? - false, - oauthClientId: - login['oauth_client_id'] as String? ?? - login['oauthClientId'] as String?, - oauthClientSecret: - login['oauth_client_secret'] as String? ?? - login['oauthClientSecret'] as String?, - socialProviders: socialProvidersJson - .whereType>() - .map(SocialProviderConfig.fromJson) - .toList(), - autoDiscoverSocialProviders: - login['auto_discover_social_providers'] as bool? ?? - login['autoDiscoverSocialProviders'] as bool? ?? - true, - ); - } + final loginConfig = login is Map + ? LoginConfig.fromJsonFile(login) + : null; return AppConfig( baseUrl: json['base_url'] as String? ?? json['baseUrl'] as String? ?? '', doctypes: diff --git a/lib/src/models/doc_field.dart b/lib/src/models/doc_field.dart index 103c0f3..8912156 100644 --- a/lib/src/models/doc_field.dart +++ b/lib/src/models/doc_field.dart @@ -1,5 +1,7 @@ import 'dart:convert'; +import '../utils/frappe_json_utils.dart' as frappe_json; + /// Represents a Frappe DocField (field definition in metadata) class DocField { final String? fieldname; @@ -54,14 +56,9 @@ class DocField { }); factory DocField.fromJson(Map json) { - // Handle Frappe's JSON format - can be int (0/1) or bool - bool parseBool(dynamic value, {bool defaultValue = false}) { - if (value == null) return defaultValue; - if (value is bool) return value; - if (value is int) return value == 1; - if (value is String) return value == '1' || value.toLowerCase() == 'true'; - return defaultValue; - } + // Frappe's JSON encodes booleans inconsistently — see frappe_json_utils. + bool parseBool(dynamic v, {bool defaultValue = false}) => + frappe_json.parseBool(v, defaultValue: defaultValue); int? parseInt(dynamic value) { if (value == null) return null; diff --git a/lib/src/models/doc_type_meta.dart b/lib/src/models/doc_type_meta.dart index f5c020a..86f1fa0 100644 --- a/lib/src/models/doc_type_meta.dart +++ b/lib/src/models/doc_type_meta.dart @@ -1,5 +1,6 @@ import 'package:flutter/foundation.dart'; +import '../utils/frappe_json_utils.dart'; import 'doc_field.dart'; /// Represents Frappe DocType metadata @@ -65,21 +66,8 @@ class DocTypeMeta { } }).toList(); - // Handle isTable - can be int (0/1) or bool - bool isTableValue = false; - if (json['istable'] != null) { - if (json['istable'] is bool) { - isTableValue = json['istable'] as bool; - } else if (json['istable'] is int) { - isTableValue = (json['istable'] as int) == 1; - } - } else if (json['isTable'] != null) { - if (json['isTable'] is bool) { - isTableValue = json['isTable'] as bool; - } else if (json['isTable'] is int) { - isTableValue = (json['isTable'] as int) == 1; - } - } + // Handle isTable - can be int (0/1) or bool. See frappe_json_utils. + final isTableValue = parseBool(json['istable'] ?? json['isTable']); final titleField = json['title_field'] as String?; final sortField = json['sort_field'] as String?; @@ -151,9 +139,7 @@ class DocTypeMeta { final permLevel = raw['permlevel'] ?? raw['perm_level'] ?? 0; if (permLevel is num && permLevel != 0) continue; - final flag = raw[action]; - final allowed = flag == 1 || flag == true; - if (!allowed) continue; + if (!parseBool(raw[action])) continue; final role = raw['role']?.toString(); if (userRoles == null || userRoles.isEmpty) { diff --git a/lib/src/models/outbox_row.dart b/lib/src/models/outbox_row.dart index 8c6e5cc..448daf8 100644 --- a/lib/src/models/outbox_row.dart +++ b/lib/src/models/outbox_row.dart @@ -1,5 +1,7 @@ // ignore_for_file: constant_identifier_names +import '../utils/sql_row_utils.dart'; + enum OutboxOperation { insert, update, submit, cancel, delete } enum OutboxState { pending, inFlight, done, failed, conflict, blocked } @@ -17,13 +19,8 @@ enum ErrorCode { extension ErrorCodeHelpers on ErrorCode { String get wireName => name; - static ErrorCode? parse(String? raw) { - if (raw == null) return null; - for (final c in ErrorCode.values) { - if (c.name == raw) return c; - } - return ErrorCode.UNKNOWN; - } + static ErrorCode? parse(String? raw) => + parseEnumByName(ErrorCode.values, raw, fallback: ErrorCode.UNKNOWN); } extension OutboxOperationHelpers on OutboxOperation { @@ -120,19 +117,11 @@ class OutboxRow { operation: OutboxOperationHelpers.parse(row['operation'] as String), payload: row['payload'] as String?, state: OutboxStateHelpers.parse(row['state'] as String), - retryCount: (row['retry_count'] as int?) ?? 0, - lastAttemptAt: row['last_attempt_at'] == null - ? null - : DateTime.fromMillisecondsSinceEpoch( - row['last_attempt_at'] as int, - isUtc: true, - ), + retryCount: retryCountFrom(row), + lastAttemptAt: lastAttemptAtFrom(row), errorMessage: row['error_message'] as String?, errorCode: ErrorCodeHelpers.parse(row['error_code'] as String?), - createdAt: DateTime.fromMillisecondsSinceEpoch( - row['created_at'] as int, - isUtc: true, - ), + createdAt: utcMillisFrom(row, 'created_at'), ); } } diff --git a/lib/src/models/pending_attachment.dart b/lib/src/models/pending_attachment.dart index c3d86f2..865b0b1 100644 --- a/lib/src/models/pending_attachment.dart +++ b/lib/src/models/pending_attachment.dart @@ -1,12 +1,13 @@ +import '../utils/sql_row_utils.dart'; + enum AttachmentState { pending, uploading, done, failed } extension AttachmentStateHelpers on AttachmentState { String get wireName => name; static AttachmentState parse(String raw) { - for (final s in AttachmentState.values) { - if (s.name == raw) return s; - } - throw ArgumentError.value(raw, 'attachment_state'); + final value = parseEnumByName(AttachmentState.values, raw); + if (value == null) throw ArgumentError.value(raw, 'attachment_state'); + return value; } } @@ -65,20 +66,12 @@ class PendingAttachment { isPrivate: (row['is_private'] as int? ?? 1) == 1, sizeBytes: row['size_bytes'] as int?, state: AttachmentStateHelpers.parse(row['state'] as String), - retryCount: (row['retry_count'] as int?) ?? 0, - lastAttemptAt: row['last_attempt_at'] == null - ? null - : DateTime.fromMillisecondsSinceEpoch( - row['last_attempt_at'] as int, - isUtc: true, - ), + retryCount: retryCountFrom(row), + lastAttemptAt: lastAttemptAtFrom(row), errorMessage: row['error_message'] as String?, serverFileName: row['server_file_name'] as String?, serverFileUrl: row['server_file_url'] as String?, - createdAt: DateTime.fromMillisecondsSinceEpoch( - row['created_at'] as int, - isUtc: true, - ), + createdAt: utcMillisFrom(row, 'created_at'), ); } } diff --git a/lib/src/models/workflow_transition.dart b/lib/src/models/workflow_transition.dart index fe63f82..6734351 100644 --- a/lib/src/models/workflow_transition.dart +++ b/lib/src/models/workflow_transition.dart @@ -1,3 +1,5 @@ +import '../utils/frappe_json_utils.dart'; + /// Represents a possible Frappe workflow transition for the current document state. /// Returned by [WorkflowService.getTransitions]. class WorkflowTransition { @@ -25,13 +27,12 @@ class WorkflowTransition { }); factory WorkflowTransition.fromJson(Map json) { - final allow = json['allow_self_approval']; return WorkflowTransition( action: json['action'] as String? ?? '', nextState: json['next_state'] as String? ?? '', state: json['state'] as String? ?? '', allowed: json['allowed'] as String?, - allowSelfApproval: allow == 1 || allow == true, + allowSelfApproval: parseBool(json['allow_self_approval']), ); } } diff --git a/lib/src/query/filter_errors.dart b/lib/src/query/filter_errors.dart index 46116c2..4d31cf0 100644 --- a/lib/src/query/filter_errors.dart +++ b/lib/src/query/filter_errors.dart @@ -1,8 +1,16 @@ +/// Shared base for the two filter-related exception types so the +/// `final String message; toString() => '$ClassName: $message';` pattern +/// is declared once. Subclasses override only the class name in +/// [toString] (kept as the class name so existing log greps still work). +abstract class FilterException implements Exception { + final String message; + const FilterException(this.message); +} + /// The filter is malformed, references unknown columns, or uses an /// operator that isn't part of the supported set. Caller bug. -class FilterParseError implements Exception { - final String message; - const FilterParseError(this.message); +class FilterParseError extends FilterException { + const FilterParseError(super.message); @override String toString() => 'FilterParseError: $message'; } @@ -10,9 +18,8 @@ class FilterParseError implements Exception { /// The filter shape is well-formed but uses a feature this version of /// the SDK doesn't translate offline (e.g. cross-doctype child filters, /// tree operators, `user.department`-style dynamic values). Spec §6.5. -class UnsupportedFilterError implements Exception { - final String message; - const UnsupportedFilterError(this.message); +class UnsupportedFilterError extends FilterException { + const UnsupportedFilterError(super.message); @override String toString() => 'UnsupportedFilterError: $message'; } diff --git a/lib/src/query/filter_parser.dart b/lib/src/query/filter_parser.dart index adf26e3..4f3e57e 100644 --- a/lib/src/query/filter_parser.dart +++ b/lib/src/query/filter_parser.dart @@ -73,38 +73,36 @@ class FilterParser { final whereParts = []; final params = []; - for (final f in filters) { - if (f.length == 4) { - throw const UnsupportedFilterError( - 'Cross-doctype child-table filters ' - '[["Doctype","field","op","value"]] are not supported in v1; ' - 'flatten to parent-field filters.', - ); - } - if (f.length != 3) { - throw FilterParseError( + // Canonical messages — both filter and or_filter loops use the same + // wording (including the "flatten to parent-field filters" hint that + // the or_filter copy used to omit). Keeps caller-facing error text + // consistent across the two unsupported-feature paths. + const length4 = + 'Cross-doctype child-table filters ' + '[["Doctype","field","op","value"]] are not supported in v1; ' + 'flatten to parent-field filters.'; + _parseAndCollect( + filters, + colTypes: colTypes, + normFields: normFields, + sqlParts: whereParts, + params: params, + malformedMessage: (f) => 'Malformed filter: $f (expected [col, op, value])', - ); - } - final parsed = _parseOne(f, colTypes, normFields); - whereParts.add(parsed.sql); - params.addAll(parsed.params); - } + length4Message: length4, + ); final orParts = []; - for (final f in orFilters) { - if (f.length == 4) { - throw const UnsupportedFilterError( - 'Cross-doctype child-table or_filters not supported in v1.', - ); - } - if (f.length != 3) { - throw FilterParseError('Malformed or_filter: $f'); - } - final parsed = _parseOne(f, colTypes, normFields); - orParts.add(parsed.sql); - params.addAll(parsed.params); - } + _parseAndCollect( + orFilters, + colTypes: colTypes, + normFields: normFields, + sqlParts: orParts, + params: params, + malformedMessage: (f) => + 'Malformed or_filter: $f (expected [col, op, value])', + length4Message: length4, + ); final where = []; where.addAll(whereParts); @@ -131,6 +129,47 @@ class FilterParser { return ParsedQuery(sql: sb.toString(), params: params); } + /// Validates each filter row in [inputs] (rejecting 4-tuples with + /// [length4Message], non-3-tuples via [malformedMessage]), dispatches + /// each to [_parseOne], and appends the resulting SQL fragment to + /// [sqlParts] and bind values to [params]. Shared by the `filters` and + /// `orFilters` loops so a new tuple shape only requires editing this + /// method instead of two parallel loops. + static void _parseAndCollect( + List> inputs, { + required Map colTypes, + required Set normFields, + required List sqlParts, + required List params, + required String Function(List) malformedMessage, + required String length4Message, + }) { + for (final f in inputs) { + if (f.length == 4) { + throw UnsupportedFilterError(length4Message); + } + if (f.length != 3) { + throw FilterParseError(malformedMessage(f)); + } + final parsed = _parseOne(f, colTypes, normFields); + sqlParts.add(parsed.sql); + params.addAll(parsed.params); + } + } + + /// Returns the canonical SQL `IFNULL($col, )` wrapper used to + /// coalesce null column values before equality / LIKE comparisons. + /// Picks `0` for numeric columns and `''` for text — sourced once so + /// a future switch (e.g. to `COALESCE` or collation suffixes) applies + /// to every text / numeric comparison site at once. + static String _ifnullExpr(String col, bool isNumeric) => + isNumeric ? 'IFNULL($col, 0)' : "IFNULL($col, '')"; + + /// Returns the canonical `col >= ? AND col <= ?` SQL fragment used by + /// both the `between` and `timespan` operators. + static ParsedQuery _rangeQuery(String col, Object? start, Object? end) => + ParsedQuery(sql: '$col >= ? AND $col <= ?', params: [start, end]); + static ParsedQuery _parseOne( List f, Map colTypes, @@ -153,10 +192,10 @@ class FilterParser { switch (op) { case '=': case '!=': - if (isNumeric) { - return ParsedQuery(sql: 'IFNULL($col, 0) $op ?', params: [value]); - } - return ParsedQuery(sql: "IFNULL($col, '') $op ?", params: [value]); + return ParsedQuery( + sql: '${_ifnullExpr(col, isNumeric)} $op ?', + params: [value], + ); case '<': case '<=': case '>': @@ -185,7 +224,10 @@ class FilterParser { params: [normalizeForSearch(value?.toString())], ); } - return ParsedQuery(sql: "IFNULL($col, '') $sqlOp ?", params: [value]); + return ParsedQuery( + sql: '${_ifnullExpr(col, false)} $sqlOp ?', + params: [value], + ); case 'between': if (value is! List || value.length != 2) { throw const FilterParseError('"between" needs a 2-element list'); @@ -200,25 +242,25 @@ class FilterParser { isStart: false, type: type, ); - return ParsedQuery( - sql: '$col >= ? AND $col <= ?', - params: [start, end], - ); + return _rangeQuery(col, start, end); case 'timespan': if (value == null) { throw const FilterParseError('"timespan" requires a keyword value'); } final range = FrappeTimespan.resolve(value.toString()); - return ParsedQuery( - sql: '$col >= ? AND $col <= ?', - params: [range.start, range.end], - ); + return _rangeQuery(col, range.start, range.end); case 'is': if (value == 'set') { - return ParsedQuery(sql: "IFNULL($col, '') != ''", params: const []); + return ParsedQuery( + sql: "${_ifnullExpr(col, false)} != ''", + params: const [], + ); } if (value == 'not set') { - return ParsedQuery(sql: "IFNULL($col, '') = ''", params: const []); + return ParsedQuery( + sql: "${_ifnullExpr(col, false)} = ''", + params: const [], + ); } if (value == null) { return ParsedQuery(sql: '$col IS NULL', params: const []); diff --git a/lib/src/query/query_result.dart b/lib/src/query/query_result.dart index a811106..c4dc948 100644 --- a/lib/src/query/query_result.dart +++ b/lib/src/query/query_result.dart @@ -29,4 +29,22 @@ class QueryResult { returnedCount: 0, originBreakdown: {}, ); + + /// Convenience factory that derives `hasMore` from + /// `rows.length == pageSize` and `returnedCount` from `rows.length`. + /// Used by [UnifiedResolver] to keep the offline-path and online-passthrough + /// constructions in sync — a future change to the `hasMore` heuristic + /// (e.g. trusting a server-supplied flag) applies to both at once. + factory QueryResult.ofRows( + List rows, + int pageSize, + Map originBreakdown, + ) { + return QueryResult( + rows: rows, + hasMore: rows.length == pageSize, + returnedCount: rows.length, + originBreakdown: originBreakdown, + ); + } } diff --git a/lib/src/query/unified_resolver.dart b/lib/src/query/unified_resolver.dart index 833f8c5..ca73ab2 100644 --- a/lib/src/query/unified_resolver.dart +++ b/lib/src/query/unified_resolver.dart @@ -8,6 +8,7 @@ import 'package:sqflite/sqflite.dart'; import '../api/client.dart'; import '../database/daos/doctype_meta_dao.dart'; import '../database/table_name.dart'; +import '../models/doc_type_meta.dart'; import '../models/meta_resolver.dart'; import '../models/offline_mode.dart'; import '../models/offline_mode_notifier.dart'; @@ -101,21 +102,23 @@ class UnifiedResolver { ); } final meta = await metaResolver(doctype); - final tableName = - await metaDao.getTableName(doctype) ?? - normalizeDoctypeTableName(doctype); + final tableName = await metaDao.tableNameFor(doctype); // For child tables, Frappe link_filters reference Frappe's virtual // `parent` field (server_name of parent). The local schema stores // the parent's mobile_uuid in `parent_uuid` instead. Translate // before handing off to FilterParser so the column whitelist check // passes and the lookup finds both offline and synced child rows. - final effectiveFilters = meta.isTable - ? await _translateParentFilters(filters, tableName) - : [...filters]; - final effectiveOrFilters = meta.isTable - ? await _translateParentFilters(orFilters, tableName) - : [...orFilters]; + final effectiveFilters = await _maybeTranslateFilters( + filters, + meta, + tableName, + ); + final effectiveOrFilters = await _maybeTranslateFilters( + orFilters, + meta, + tableName, + ); // Child tables (isTable=true) have no sync_status column — skip. if (!includeFailed && !meta.isTable) { @@ -165,12 +168,7 @@ class UnifiedResolver { breakdown[origin] = (breakdown[origin] ?? 0) + 1; } - return QueryResult>( - rows: rows, - hasMore: rows.length == pageSize, - returnedCount: rows.length, - originBreakdown: breakdown, - ); + return QueryResult.ofRows(rows, pageSize, breakdown); } /// Counts rows for [doctype] in the active mode. @@ -201,9 +199,7 @@ class UnifiedResolver { } } - final tableName = - await metaDao.getTableName(doctype) ?? - normalizeDoctypeTableName(doctype); + final tableName = await metaDao.tableNameFor(doctype); final whereClause = dirtyOnly ? "sync_status IN ('dirty','sync_error','sync_blocked')" : "sync_status != 'failed'"; @@ -255,13 +251,10 @@ class UnifiedResolver { .whereType>() .map((r) => Map.from(r)) .toList(); - return QueryResult>( - rows: rows, - hasMore: rows.length == pageSize, - returnedCount: rows.length, - originBreakdown: rows.isEmpty - ? const {} - : {RowOrigin.server: rows.length}, + return QueryResult.ofRows( + rows, + pageSize, + rows.isEmpty ? const {} : {RowOrigin.server: rows.length}, ); } @@ -307,6 +300,22 @@ class UnifiedResolver { /// Translates Frappe's virtual `parent` column references into /// `parent_uuid` lookups for child-table queries. /// + /// Translates [filters] through [_translateParentFilters] for child-table + /// doctypes, or returns a shallow copy otherwise. Lifts the two + /// ternaries (one for `filters`, one for `orFilters`) out of [resolve] + /// so adding a third filter list (e.g. future `sort_by` filters) doesn't + /// require copy-pasting the conditional. + Future> _maybeTranslateFilters( + List input, + DocTypeMeta meta, + String tableName, + ) async { + if (meta.isTable) { + return _translateParentFilters(input, tableName); + } + return [...input]; + } + /// Frappe link_filters use `parent` (the server_name of the parent row), /// but the local child schema stores the parent's mobile_uuid in /// `parent_uuid`. For offline records the Link picker returns mobile_uuid diff --git a/lib/src/screens/mobile_home_screen.dart b/lib/src/screens/mobile_home_screen.dart index 41e23b3..4ea086b 100644 --- a/lib/src/screens/mobile_home_screen.dart +++ b/lib/src/screens/mobile_home_screen.dart @@ -6,6 +6,7 @@ import '../models/link_filter_result.dart'; import '../sdk/frappe_sdk.dart'; import '../ui/document_list_screen.dart'; import '../ui/widgets/form_builder.dart' show FieldChangeHandler; +import '../ui/widgets/screen_helpers.dart'; /// Generic home screen that renders doctype groups from the SDK's Mobile Configuration. /// @@ -491,34 +492,17 @@ class _MobileHomeScreenState extends State } Future _handleLogout() async { - final confirmed = await showDialog( - context: context, - builder: (ctx) => AlertDialog( - title: const Text('Logout'), - content: const Text('Are you sure you want to logout?'), - actions: [ - TextButton( - onPressed: () => Navigator.pop(ctx, false), - child: const Text('Cancel'), - ), - TextButton( - onPressed: () => Navigator.pop(ctx, true), - child: const Text( - 'Logout', - style: TextStyle(color: Color(0xFFD32F2F)), - ), - ), - ], - ), + final confirmed = await showConfirmDialog( + context, + title: 'Logout', + content: 'Are you sure you want to logout?', + confirmLabel: 'Logout', + confirmColor: const Color(0xFFD32F2F), ); if (confirmed != true || !mounted) return; - showDialog( - context: context, - barrierDismissible: false, - builder: (_) => const Center(child: CircularProgressIndicator()), - ); + final dismissLoading = showLoadingDialog(context); try { await widget.sdk.logout(); @@ -526,18 +510,15 @@ class _MobileHomeScreenState extends State // ignore: avoid_print print('MobileHomeScreen: logout failed — $e\n$st'); if (mounted) { - ScaffoldMessenger.of(context).showSnackBar( - SnackBar( - content: Text( - 'Logout warning: ${e.toString().split(':').last.trim()}', - ), - backgroundColor: const Color(0xFFE65100), - ), + showStatusSnackBar( + context, + 'Logout warning: ${e.toString().split(':').last.trim()}', + backgroundColor: const Color(0xFFE65100), ); } } finally { if (mounted) { - Navigator.of(context).pop(); // dismiss loading + dismissLoading(); } if (widget.onLogout != null) { await widget.onLogout!(); @@ -546,35 +527,19 @@ class _MobileHomeScreenState extends State } Future _handleForcePullAll() async { - final confirmed = await showDialog( - context: context, - builder: (ctx) => AlertDialog( - title: const Text('Force Re-Sync All?'), - content: const Text( + final confirmed = await showConfirmDialog( + context, + title: 'Force Re-Sync All?', + content: 'This re-downloads all reference and master data from scratch. ' 'Your unsynced form entries are not affected.', - ), - actions: [ - TextButton( - onPressed: () => Navigator.pop(ctx, false), - child: const Text('Cancel'), - ), - TextButton( - onPressed: () => Navigator.pop(ctx, true), - child: const Text('Re-Sync'), - ), - ], - ), + confirmLabel: 'Re-Sync', ); if (confirmed != true || !mounted) return; setState(() => _isForceSyncing = true); - showDialog( - context: context, - barrierDismissible: false, - builder: (_) => const Center(child: CircularProgressIndicator()), - ); + final dismissLoading = showLoadingDialog(context); try { await widget.sdk.forcePullAll(); @@ -582,18 +547,15 @@ class _MobileHomeScreenState extends State // ignore: avoid_print print('MobileHomeScreen: forcePullAll failed — $e\n$st'); if (mounted) { - ScaffoldMessenger.of(context).showSnackBar( - SnackBar( - content: Text( - 'Re-sync failed: ${e.toString().split(':').last.trim()}', - ), - backgroundColor: const Color(0xFFE65100), - ), + showStatusSnackBar( + context, + 'Re-sync failed: ${e.toString().split(':').last.trim()}', + backgroundColor: const Color(0xFFE65100), ); } } finally { if (mounted) { - Navigator.of(context).pop(); // dismiss loading + dismissLoading(); setState(() => _isForceSyncing = false); } } @@ -646,11 +608,10 @@ class _MobileHomeScreenState extends State print('MobileHomeScreen: _navigateToDoctype failed — $e\n$st'); if (mounted) { ScaffoldMessenger.of(context).hideCurrentSnackBar(); - ScaffoldMessenger.of(context).showSnackBar( - SnackBar( - content: Text('Error: ${e.toString().split(':').last.trim()}'), - backgroundColor: Colors.red, - ), + showStatusSnackBar( + context, + 'Error: ${e.toString().split(':').last.trim()}', + severity: SnackBarSeverity.error, ); } } diff --git a/lib/src/sdk/frappe_sdk.dart b/lib/src/sdk/frappe_sdk.dart index 22aaf08..2c41356 100644 --- a/lib/src/sdk/frappe_sdk.dart +++ b/lib/src/sdk/frappe_sdk.dart @@ -11,6 +11,7 @@ import '../concurrency/connectivity_watcher.dart'; import '../database/app_database.dart'; import '../database/daos/doctype_meta_dao.dart'; import '../database/daos/sdk_meta_dao.dart'; +import '../models/closure_result.dart'; import '../models/offline_mode.dart'; import '../models/offline_mode_notifier.dart'; import '../models/session_user.dart'; @@ -636,6 +637,15 @@ class FrappeSDK { } } + /// Throws if [initialize] hasn't run. Called as the first line of every + /// public service getter so the (12+) "if (!_initialized) throw ..." + /// blocks all share one canonical message and exception type. + void _assertInitialized() { + if (!_initialized) { + throw Exception('SDK not initialized. Call initialize() first.'); + } + } + /// Get Frappe API client (for direct API calls) /// /// Example: @@ -644,50 +654,38 @@ class FrappeSDK { /// await client.document.createDocument('Customer', {'name': 'Test'}); /// ``` FrappeClient get api { - if (!_initialized) { - throw Exception('SDK not initialized. Call initialize() first.'); - } + _assertInitialized(); return _client!; } /// Get Auth Service AuthService get auth { - if (!_initialized) { - throw Exception('SDK not initialized. Call initialize() first.'); - } + _assertInitialized(); return _authService!; } /// Get Meta Service MetaService get meta { - if (!_initialized) { - throw Exception('SDK not initialized. Call initialize() first.'); - } + _assertInitialized(); return _metaService!; } /// Get Permission Service (doctype read/write/create/delete from login or mobile_auth.permissions) PermissionService get permissions { - if (!_initialized) { - throw Exception('SDK not initialized. Call initialize() first.'); - } + _assertInitialized(); return _permissionService!; } /// Get Translation Service (Frappe translations via mobile_auth.get_translations). /// Use [TranslationService.loadTranslations] then [TranslationService.translate] or [TranslationService.call]. TranslationService get translations { - if (!_initialized) { - throw Exception('SDK not initialized. Call initialize() first.'); - } + _assertInitialized(); return _translationService!; } /// Get Sync Service SyncService get sync { - if (!_initialized) { - throw Exception('SDK not initialized. Call initialize() first.'); - } + _assertInitialized(); return _syncService!; } @@ -698,9 +696,7 @@ class FrappeSDK { /// after a successful auth call. Logout flows call `.clear()` before /// running [AtomicWipe.wipe] to drop the persisted JSON. SessionUserService get sessionUserService { - if (!_initialized) { - throw Exception('SDK not initialized. Call initialize() first.'); - } + _assertInitialized(); return _sessionUserService!; } @@ -715,9 +711,7 @@ class FrappeSDK { /// Get Repository (for offline operations) OfflineRepository get repository { - if (!_initialized) { - throw Exception('SDK not initialized. Call initialize() first.'); - } + _assertInitialized(); return _repository!; } @@ -726,9 +720,7 @@ class FrappeSDK { /// - offline → per-doctype `docs__` tables via FilterParser /// - online → `frappe.client.get_list` / `frappe.client.get_count` UnifiedResolver get resolver { - if (!_initialized) { - throw Exception('SDK not initialized. Call initialize() first.'); - } + _assertInitialized(); return _resolver!; } @@ -739,25 +731,19 @@ class FrappeSDK { /// apps mount [OfflineTransitionScreen] above their router driven by /// this stream. Spec §7. OfflineTransitionService get offlineTransition { - if (!_initialized) { - throw Exception('SDK not initialized. Call initialize() first.'); - } + _assertInitialized(); return _offlineTransitionService!; } /// Get Link Option Service LinkOptionService get linkOptions { - if (!_initialized) { - throw Exception('SDK not initialized. Call initialize() first.'); - } + _assertInitialized(); return _linkOptionService!; } /// Get Database instance AppDatabase get database { - if (!_initialized) { - throw Exception('SDK not initialized. Call initialize() first.'); - } + _assertInitialized(); return _database!; } @@ -1066,21 +1052,16 @@ class FrappeSDK { if (!await onlineCheck()) return const {}; final entryPoints = await _metaService!.getMobileFormDoctypeNames(); final closure = await _metaService!.closure(entryPoints); - final pullable = {}; - for (final doctype in closure.doctypes) { - if (closure.childDoctypes.contains(doctype)) continue; - if (_permissionService != null && - !await _permissionService!.canRead(doctype)) { - continue; - } - pullable.add(doctype); - } + final pullable = await _buildPullableDoctypes( + entryPoints: entryPoints, + closure: closure, + ); if (pullable.isEmpty) return const {}; // Hold the SyncMutex for the entire closure pull so concurrent // single-doctype callers (pullSync, pullSyncWaiting) serialise // behind it — same contract as the former pullSyncMany call path. return await _syncService!.protect( - () => _pullEngine!.run(closure, allowedDoctypes: pullable), + () => _pullEngine!.run(closure, allowedDoctypes: pullable.toSet()), ); } catch (e, st) { // ignore: avoid_print @@ -1091,6 +1072,37 @@ class FrappeSDK { } } + /// Builds the set of doctypes eligible for a closure pull, given the + /// already-resolved [closure] from `MetaService.closure(entryPoints)`. + /// Always excludes child-table doctypes (those ride inside parents) + /// and any doctype the user lacks read permission for. When + /// [excludeEntryPoints] is true, entry-point mobile-form doctypes are + /// also excluded — `forcePullAll` opts in because entry points are + /// managed by the normal sync cycle there; `_runUpgradeClosurePull` + /// opts out because the upgrade pull is its only sync trigger. + /// + /// Returns a `List` rather than a `Set` because callers want + /// the cursor-clear pre-pass (`forcePullAll`) to iterate deterministically + /// and the closure pull (`_runUpgradeClosurePull`) accepts any iterable. + Future> _buildPullableDoctypes({ + required Iterable entryPoints, + required ClosureResult closure, + bool excludeEntryPoints = false, + }) async { + final entryPointSet = excludeEntryPoints ? entryPoints.toSet() : null; + final pullable = []; + for (final doctype in closure.doctypes) { + if (entryPointSet != null && entryPointSet.contains(doctype)) continue; + if (closure.childDoctypes.contains(doctype)) continue; + if (_permissionService != null && + !await _permissionService!.canRead(doctype)) { + continue; + } + pullable.add(doctype); + } + return pullable; + } + /// Adapter for `SyncController.runPull`. Propagates the SIG-2 deferred set /// from PullEngine so SyncController can re-pull those doctypes after push. Future> _runPullForController() => _runUpgradeClosurePull(); @@ -1132,18 +1144,12 @@ class FrappeSDK { } try { final entryPoints = await _metaService!.getMobileFormDoctypeNames(); - final entryPointSet = entryPoints.toSet(); final closure = await _metaService!.closure(entryPoints); - final pullable = []; - for (final doctype in closure.doctypes) { - if (entryPointSet.contains(doctype)) continue; - if (closure.childDoctypes.contains(doctype)) continue; - if (_permissionService != null && - !await _permissionService!.canRead(doctype)) { - continue; - } - pullable.add(doctype); - } + final pullable = await _buildPullableDoctypes( + entryPoints: entryPoints, + closure: closure, + excludeEntryPoints: true, + ); if (pullable.isEmpty) { _syncCompleteController?.add(null); return; diff --git a/lib/src/services/auth_service.dart b/lib/src/services/auth_service.dart index 9ccffc3..1e5570c 100644 --- a/lib/src/services/auth_service.dart +++ b/lib/src/services/auth_service.dart @@ -28,6 +28,21 @@ class AuthService { static const Uuid _uuid = Uuid(); FrappeClient? _client; bool _isAuthenticated = false; + + /// Maps Frappe's `roles` JSON list (returned by `/api/method/login`, + /// `verify_login_otp`, and the post-login `frappe.auth.get_logged_user` + /// response) into a clean `List`. Drops null and empty entries, + /// returns an empty list when the input is null. Shared by [login], + /// [verifyLoginOtp], and [fetchUserInfo] so role-extraction stays uniform + /// across auth paths. + static List _parseRoles(dynamic rolesJson) { + if (rolesJson is! List) return []; + return rolesJson + .map((r) => r.toString()) + .where((r) => r.isNotEmpty) + .toList(); + } + bool _isRefreshingToken = false; AppDatabase? _database; List _roles = []; @@ -119,13 +134,7 @@ class AuthService { final mobileFormNamesJson = response['mobile_form_names'] as List?; - final rolesJson = response['roles'] as List?; - _roles = - rolesJson - ?.map((r) => r.toString()) - .where((r) => r.isNotEmpty) - .toList() ?? - []; + _roles = _parseRoles(response['roles']); _language = response['language'] as String?; @@ -210,13 +219,7 @@ class AuthService { } dev.log('access_token: $accessToken', name: 'Auth'); - final rolesJson = response['roles'] as List?; - _roles = - rolesJson - ?.map((r) => r.toString()) - .where((r) => r.isNotEmpty) - .toList() ?? - []; + _roles = _parseRoles(response['roles']); _language = response['language'] as String?; await _processLoginResponse( @@ -249,13 +252,7 @@ class AuthService { ? result['data'] as Map : result; - final rolesJson = message['roles'] as List?; - _roles = - rolesJson - ?.map((r) => r.toString()) - .where((r) => r.isNotEmpty) - .toList() ?? - []; + _roles = _parseRoles(message['roles']); _language = message['language'] as String?; return message; } catch (e, st) { @@ -292,6 +289,14 @@ class AuthService { .map((json) => MobileFormName.fromJson(json as Map)) .toList(); + // Mirror MetaService._updateMobileFormDoctypes field set: carry + // `groupName` and `sortOrder` through the unset / upsert loops so a + // login here doesn't silently drop those columns relative to a + // later `resyncMobileConfiguration()` call. (The timestamp-newer + // check that MetaService also runs is intentionally NOT replicated + // — AuthService's login path is the entry point that establishes + // server-side mobile form list as authoritative; no comparison is + // needed here.) final allMetas = await _database!.doctypeMetaDao.findAll(); for (final meta in allMetas) { if (meta.isMobileForm) { @@ -301,13 +306,17 @@ class AuthService { serverModifiedAt: meta.serverModifiedAt, isMobileForm: false, metaJson: meta.metaJson, + groupName: meta.groupName, + sortOrder: meta.sortOrder, ); await _database!.doctypeMetaDao.updateDoctypeMeta(updatedMeta); } } - for (final mfn in mobileFormNames) { + for (int i = 0; i < mobileFormNames.length; i++) { + final mfn = mobileFormNames[i]; final doctype = mfn.mobileDoctype; + if (doctype.isEmpty) continue; final existingMeta = await _database!.doctypeMetaDao.findByDoctype( doctype, ); @@ -319,6 +328,8 @@ class AuthService { serverModifiedAt: mfn.doctypeMetaModifiedAt, isMobileForm: true, metaJson: existingMeta.metaJson, + groupName: mfn.groupName, + sortOrder: i, ); await _database!.doctypeMetaDao.updateDoctypeMeta(updatedMeta); } else { @@ -328,6 +339,8 @@ class AuthService { serverModifiedAt: mfn.doctypeMetaModifiedAt, isMobileForm: true, metaJson: '{}', + groupName: mfn.groupName, + sortOrder: i, ); await _database!.doctypeMetaDao.insertDoctypeMeta(newMeta); } diff --git a/lib/src/services/link_option_service.dart b/lib/src/services/link_option_service.dart index 7b663c0..bcbdeab 100644 --- a/lib/src/services/link_option_service.dart +++ b/lib/src/services/link_option_service.dart @@ -34,6 +34,26 @@ class LinkOptionService { _metaResolver = metaResolver, _syncCompleteStream = syncComplete$; + /// Converts Frappe-shaped filter rows to the 3-tuple `[field, op, value]` + /// shape that [UnifiedResolver.resolve] consumes. Frappe APIs hand back + /// either 4-tuples (`[doctype, field, op, value]`) or already-3-tuples; + /// rows of any other length are silently dropped (consistent with the + /// pre-refactor inline behavior). The output type is always + /// `List>` so both callers (`getLinkOptions` and + /// `getLinkOptionsOffline`) get the same typed collection. + static List> _toThreeTuples(List? filters) { + final out = >[]; + if (filters == null) return out; + for (final f in filters) { + if (f.length == 4) { + out.add([f[1], f[2], f[3]]); + } else if (f.length == 3) { + out.add(List.from(f)); + } + } + return out; + } + /// Test / subclass constructor. Use when all methods are overridden and no /// resolver is needed (e.g. recording mocks in widget tests). LinkOptionService.withoutResolver() @@ -55,16 +75,7 @@ class LinkOptionService { final titleField = meta.titleField; // Convert Frappe 4-tuple filters [doctype, field, op, value] → 3-tuples. - final threeTuples = >[]; - if (normalizedFilters != null) { - for (final f in normalizedFilters) { - if (f.length == 4) { - threeTuples.add([f[1], f[2], f[3]]); - } else if (f.length == 3) { - threeTuples.add(List.from(f)); - } - } - } + final threeTuples = _toThreeTuples(normalizedFilters); final result = await resolver.resolve( doctype: doctype, @@ -333,16 +344,7 @@ class LinkOptionService { final meta = await metaResolver(doctype); final titleField = meta.titleField; - final threeTuples = []; - if (filters != null) { - for (final f in filters) { - if (f.length == 4) { - threeTuples.add([f[1], f[2], f[3]]); - } else if (f.length == 3) { - threeTuples.add(List.from(f)); - } - } - } + final threeTuples = _toThreeTuples(filters); if (query != null && query.isNotEmpty && titleField != null) { threeTuples.add([titleField, 'like', '%$query%']); } diff --git a/lib/src/services/meta_differ.dart b/lib/src/services/meta_differ.dart index 1d38d55..d8bf76f 100644 --- a/lib/src/services/meta_differ.dart +++ b/lib/src/services/meta_differ.dart @@ -36,9 +36,9 @@ class MetaDiffer { } } - final newTableSuffix = normalizeDoctypeTableName( - newMeta.name, - ).replaceFirst('docs__', ''); + final newTableSuffix = stripDocsPrefix( + normalizeDoctypeTableName(newMeta.name), + ); for (final name in oldByName.keys) { final oldFt = oldByName[name]!.fieldtype; if (sqliteColumnTypeFor(oldFt) == null) continue; diff --git a/lib/src/services/meta_migration.dart b/lib/src/services/meta_migration.dart index 82a7f5e..e7accbd 100644 --- a/lib/src/services/meta_migration.dart +++ b/lib/src/services/meta_migration.dart @@ -1,6 +1,7 @@ import 'package:flutter/foundation.dart'; import 'package:sqflite/sqflite.dart'; import '../database/normalize_for_search.dart'; +import '../database/schema/system_columns.dart'; import '../models/meta_diff.dart'; class MetaMigration { @@ -51,7 +52,7 @@ class MetaMigration { final exists = await _columnExists(txn, tableName, '${ln}__is_local'); if (!exists) { await txn.execute( - 'ALTER TABLE $tableName ADD COLUMN ${ln}__is_local INTEGER', + 'ALTER TABLE $tableName ADD COLUMN ${linkCompanionColumnDDL(ln)}', ); debugPrint('MetaMigration[$tableName] added ${ln}__is_local'); } diff --git a/lib/src/services/meta_service.dart b/lib/src/services/meta_service.dart index d020b4e..3a8cbe9 100644 --- a/lib/src/services/meta_service.dart +++ b/lib/src/services/meta_service.dart @@ -53,10 +53,15 @@ class MetaService { _metaCacheOrder.add(doctype); } - /// Fetches from server and saves to DB only (no in-memory cache). Use for prefetch. - Future fetchAndStoreInDb(String doctype) async { - final metaData = await _fetchMetaFromServer(doctype); - // Preserve existing serverModifiedAt and isMobileForm if exists + /// Upserts the server's meta JSON for [doctype], preserving the existing + /// row's `serverModifiedAt`, `isMobileForm`, `groupName`, and `sortOrder` + /// values. Shared by [fetchAndStoreInDb] (prefetch path) and [getMeta]'s + /// network-refresh branch so the four preserved fields cannot drift + /// between the two write paths. + Future _upsertMetaJson( + String doctype, + Map metaData, + ) async { final existing = await _database.doctypeMetaDao.findByDoctype(doctype); final entity = DoctypeMetaEntity( doctype: doctype, @@ -67,12 +72,17 @@ class MetaService { groupName: existing?.groupName, sortOrder: existing?.sortOrder, ); - // Check if exists, update if present, insert if new if (existing != null) { await _database.doctypeMetaDao.updateDoctypeMeta(entity); } else { await _database.doctypeMetaDao.insertDoctypeMeta(entity); } + } + + /// Fetches from server and saves to DB only (no in-memory cache). Use for prefetch. + Future fetchAndStoreInDb(String doctype) async { + final metaData = await _fetchMetaFromServer(doctype); + await _upsertMetaJson(doctype, metaData); // Invalidate the in-memory LRU so the next getMeta() reload reads the // freshly-persisted row instead of serving the previous cached copy. // Without this, schema changes synced via prefetchToDb / @@ -139,22 +149,7 @@ class MetaService { final metaData = await _fetchMetaFromServer(doctype); final meta = DocTypeMeta.fromJson(metaData); - // Preserve existing serverModifiedAt and isMobileForm if exists - final existing = await _database.doctypeMetaDao.findByDoctype(doctype); - final entity = DoctypeMetaEntity( - doctype: doctype, - modified: metaData['modified']?.toString(), - serverModifiedAt: existing?.serverModifiedAt, - isMobileForm: existing?.isMobileForm ?? false, - metaJson: jsonEncode(metaData), - groupName: existing?.groupName, - sortOrder: existing?.sortOrder, - ); - if (existing != null) { - await _database.doctypeMetaDao.updateDoctypeMeta(entity); - } else { - await _database.doctypeMetaDao.insertDoctypeMeta(entity); - } + await _upsertMetaJson(doctype, metaData); _putInCache(doctype, meta); return meta; } @@ -303,32 +298,13 @@ class MetaService { } } - /// Sync all mobile form doctypes. - Future syncAllMobileFormDoctypes() async { - try { - final mobileFormMetas = await _database.doctypeMetaDao - .findMobileFormDoctypes(); - final mobileFormDoctypes = mobileFormMetas - .map((meta) => meta.doctype) - .toList(); - - if (mobileFormDoctypes.isNotEmpty) { - for (final doctype in mobileFormDoctypes) { - try { - await fetchAndStoreInDb(doctype); - } catch (e, st) { - debugPrint( - 'MetaService.syncAllMobileFormDoctypes($doctype) failed — $e\n$st', - ); - continue; - } - } - } - } catch (e, st) { - debugPrint('MetaService.syncAllMobileFormDoctypes failed — $e\n$st'); - return; - } - } + /// Alias for [prefetchMobileFormDoctypes]. Retained so the existing + /// `FrappeSDK.syncAll()` facade keeps its semantic name; the underlying + /// behavior (fetch + DB-store for every mobile form doctype, per-doctype + /// debugPrint on failure) is identical because [prefetchToDb] — invoked + /// by [prefetchMobileFormDoctypes] — loops `fetchAndStoreInDb` with the + /// same per-doctype error guard. + Future syncAllMobileFormDoctypes() => prefetchMobileFormDoctypes(); /// Updates mobile form doctypes in database from mobile form names list. /// diff --git a/lib/src/services/offline_repository.dart b/lib/src/services/offline_repository.dart index f697154..36c3fee 100644 --- a/lib/src/services/offline_repository.dart +++ b/lib/src/services/offline_repository.dart @@ -137,11 +137,7 @@ class OfflineRepository { final ddls = isChild ? buildChildSchemaDDL(meta, tableName: tableName) : buildParentSchemaDDL(meta, tableName: tableName); - await db.transaction((txn) async { - for (final stmt in ddls) { - await txn.execute(stmt); - } - }); + await _executeDDL(ddls); try { await _database.doctypeMetaDao.setTableName(doctype, tableName); } catch (e, st) { @@ -418,7 +414,19 @@ class OfflineRepository { opt != null && opt.isNotEmpty) { final cm = await _loadMeta(opt); - if (cm != null) childMetasByDoctype[opt] = cm; + if (cm != null) { + childMetasByDoctype[opt] = cm; + } else { + // Match LocalWriter.writeParent's loud-on-failure pattern — + // a missing child meta means the save will silently drop the + // child rows, and visibility into that is critical when + // debugging "child data went missing" reports. + developer.log( + 'OfflineRepository.saveDocument: child meta missing for ' + '$opt (skipping child rows for this fieldtype)', + name: 'OfflineRepository', + ); + } } } } @@ -629,6 +637,20 @@ class OfflineRepository { ); } + /// Runs a list of DDL statements inside a single SQLite transaction. + /// Shared by [ensureSchemaForClosure], [_resolveChildMetas] (child-table + /// fallback create), and [_ensurePerDoctypeTable] so any future change + /// to DDL execution (retry, logging, savepoint wrapping) only needs to + /// happen in one place. + Future _executeDDL(List ddls) async { + final db = _database.rawDatabase; + await db.transaction((txn) async { + for (final stmt in ddls) { + await txn.execute(stmt); + } + }); + } + Future _loadMeta(String doctype) async { final cached = _metaCache[doctype]; if (cached != null) return cached; @@ -694,12 +716,9 @@ class OfflineRepository { if (!_ensuredTables.contains(childTable)) { final db = _database.rawDatabase; if (!await sqliteTableExists(db, childTable)) { - final ddls = buildChildSchemaDDL(childMeta, tableName: childTable); - await db.transaction((txn) async { - for (final stmt in ddls) { - await txn.execute(stmt); - } - }); + await _executeDDL( + buildChildSchemaDDL(childMeta, tableName: childTable), + ); } _ensuredTables.add(childTable); } @@ -719,12 +738,7 @@ class OfflineRepository { if (_ensuredTables.contains(tableName)) return; final db = _database.rawDatabase; if (!await sqliteTableExists(db, tableName)) { - final ddls = buildParentSchemaDDL(meta, tableName: tableName); - await db.transaction((txn) async { - for (final stmt in ddls) { - await txn.execute(stmt); - } - }); + await _executeDDL(buildParentSchemaDDL(meta, tableName: tableName)); // Persist the table-name mapping so future code (UnifiedResolver // etc.) can route through DoctypeMetaDao.getTableName(...). try { diff --git a/lib/src/services/sync_controller.dart b/lib/src/services/sync_controller.dart index d1e7350..43c25e2 100644 --- a/lib/src/services/sync_controller.dart +++ b/lib/src/services/sync_controller.dart @@ -152,16 +152,24 @@ class SyncController { await runPush(); } + /// All outbox rows in any "actionable" terminal state — failed, + /// conflict, or blocked. These are the buckets surfaced to the user as + /// errors and the rows that [retryAll] re-queues. Shared by `retryAll` + /// and `pendingErrors` so the set of "error" states stays uniform. + Future> _allActionableRows() async { + return [ + ...await outboxDao.findByState(OutboxState.failed), + ...await outboxDao.findByState(OutboxState.conflict), + ...await outboxDao.findByState(OutboxState.blocked), + ]; + } + /// Re-queue every error/blocked/conflict row sorted by Spec §7.4 /// priority, then run a single push drain. [filterDoctypes] limits /// the operation to a doctype subset (used by the per-doctype /// `Retry` action in SyncErrorsScreen). Future retryAll({List? filterDoctypes}) async { - final all = [ - ...await outboxDao.findByState(OutboxState.failed), - ...await outboxDao.findByState(OutboxState.conflict), - ...await outboxDao.findByState(OutboxState.blocked), - ]; + final all = await _allActionableRows(); final filtered = filterDoctypes == null ? all : all.where((r) => filterDoctypes.contains(r.doctype)).toList(); @@ -174,13 +182,7 @@ class SyncController { /// All rows currently in failed / conflict / blocked. Used by /// SyncErrorsScreen and the `Retry all` button's progress display. - Future> pendingErrors() async { - final rows = []; - rows.addAll(await outboxDao.findByState(OutboxState.failed)); - rows.addAll(await outboxDao.findByState(OutboxState.conflict)); - rows.addAll(await outboxDao.findByState(OutboxState.blocked)); - return rows; - } + Future> pendingErrors() => _allActionableRows(); /// Resolves a conflicted row. /// - [pullAndOverwriteLocal]: fetches the current server snapshot, applies diff --git a/lib/src/services/sync_service.dart b/lib/src/services/sync_service.dart index e29de0e..a54cef3 100644 --- a/lib/src/services/sync_service.dart +++ b/lib/src/services/sync_service.dart @@ -106,14 +106,7 @@ class SyncService { print('SyncService.pushSync: isOnline() threw — $e\n$st'); } if (!online) { - return SyncResult( - 0, - 0, - 0, - 'No internet connection', - errors: const [], - status: SyncStatus.noConnectivity, - ); + return SyncResult.noConnectivity(); } final runner = _pushRunner; if (runner == null) { @@ -183,14 +176,7 @@ class SyncService { return SyncResult.empty(status: SyncStatus.offlineModeDisabled); } if (!await isOnline()) { - return SyncResult( - 0, - 0, - 0, - 'No internet connection', - errors: const [], - status: SyncStatus.noConnectivity, - ); + return SyncResult.noConnectivity(); } final result = await _syncMutex.tryProtect( () => _pullOneInternal(doctype: doctype, since: since), @@ -220,14 +206,7 @@ class SyncService { return SyncResult.empty(status: SyncStatus.offlineModeDisabled); } if (!await isOnline()) { - return SyncResult( - 0, - 0, - 0, - 'No internet connection', - errors: const [], - status: SyncStatus.noConnectivity, - ); + return SyncResult.noConnectivity(); } return _syncMutex.protect( () => _pullOneInternal(doctype: doctype, since: since), @@ -262,17 +241,7 @@ class SyncService { }; } if (!await isOnline()) { - return { - for (final dt in doctypes) - dt: SyncResult( - 0, - 0, - 0, - 'No internet connection', - errors: const [], - status: SyncStatus.noConnectivity, - ), - }; + return {for (final dt in doctypes) dt: SyncResult.noConnectivity()}; } final results = await _syncMutex.tryProtect(() async { final out = {}; @@ -384,20 +353,32 @@ class SyncService { @visibleForTesting Future isChildTableForTest(String doctype) => _isChildTable(doctype); - Future _isChildTable(String doctype) async { + /// Reads and parses the persisted meta JSON for [doctype], returning + /// null when the row is absent, empty, or fails to decode. The empty + /// sentinel `'{}'` is treated as absent (legacy rows from earlier + /// schema bumps). Shared by [_isChildTable] and [_doctypeHasChildTables] + /// so the null-guard and parse pattern lives in exactly one place + /// within this class. Caller is responsible for logging the [where] + /// label so the stack trace on a parse failure remains diagnosable. + Future _loadMetaFromDao(String doctype, String where) async { final raw = await _database.doctypeMetaDao.getMetaJson(doctype); - if (raw == null || raw.isEmpty || raw == '{}') return false; + if (raw == null || raw.isEmpty || raw == '{}') return null; try { final parsed = jsonDecode(raw) as Map; - if (parsed.isEmpty) return false; - return DocTypeMeta.fromJson(parsed).isTable; + if (parsed.isEmpty) return null; + return DocTypeMeta.fromJson(parsed); } catch (e, st) { // ignore: avoid_print - print('SyncService._isChildTable($doctype) parse failed — $e\n$st'); - return false; + print('SyncService.$where($doctype) parse failed — $e\n$st'); + return null; } } + Future _isChildTable(String doctype) async { + final meta = await _loadMetaFromDao(doctype, '_isChildTable'); + return meta?.isTable ?? false; + } + Future _pullOneInternal({ required String doctype, int? since, @@ -622,24 +603,14 @@ class SyncService { // `SyncEngineBuilder._resolveServerNameFor`. Future _doctypeHasChildTables(String doctype) async { - final raw = await _database.doctypeMetaDao.getMetaJson(doctype); - if (raw == null || raw.isEmpty || raw == '{}') return false; - try { - final parsed = jsonDecode(raw) as Map; - final meta = DocTypeMeta.fromJson(parsed); - for (final f in meta.fields) { - if (f.fieldtype == 'Table' || f.fieldtype == 'Table MultiSelect') { - return true; - } + final meta = await _loadMetaFromDao(doctype, '_doctypeHasChildTables'); + if (meta == null) return false; + for (final f in meta.fields) { + if (f.fieldtype == 'Table' || f.fieldtype == 'Table MultiSelect') { + return true; } - return false; - } catch (e, st) { - // ignore: avoid_print - print( - 'SyncService._doctypeHasChildTables($doctype) parse failed — $e\n$st', - ); - return false; } + return false; } // `syncDoctype` was deleted in retirement Phase 6 — no production @@ -715,6 +686,19 @@ class SyncResult { /// "tried, no work". factory SyncResult.empty({SyncStatus status = SyncStatus.ran}) => SyncResult(0, 0, 0, null, status: status); + + /// Canonical no-connectivity result used by every sync entry point + /// (`pushSync`, `pullSync`, `pullSyncWaiting`, `pullSyncMany`) when the + /// pre-flight connectivity check fails. Defined once so a wording or + /// status change applies to all four callers atomically. + factory SyncResult.noConnectivity() => SyncResult( + 0, + 0, + 0, + 'No internet connection', + errors: const [], + status: SyncStatus.noConnectivity, + ); } /// Individual sync error details diff --git a/lib/src/sync/attachment_pipeline.dart b/lib/src/sync/attachment_pipeline.dart index 41f7425..5a27823 100644 --- a/lib/src/sync/attachment_pipeline.dart +++ b/lib/src/sync/attachment_pipeline.dart @@ -17,6 +17,16 @@ typedef AttachmentUploadFn = typedef FileFromPathFn = File Function(String path); +/// Default retry-backoff schedule used by [AttachmentPipeline] and +/// [PushEngine]'s `attachmentBackoff` / `networkBackoff` parameters. +/// 3 attempts at 2s, 5s, 10s. Defined once so a product-side schedule +/// change doesn't have to update three default-parameter sites. +const List kDefaultSyncBackoff = [ + Duration(seconds: 2), + Duration(seconds: 5), + Duration(seconds: 10), +]; + class AttachmentUploadResult { final String fileName; final String fileUrl; @@ -43,11 +53,7 @@ class AttachmentPipeline { AttachmentPipeline({ required this.dao, required this.uploader, - this.backoff = const [ - Duration(seconds: 2), - Duration(seconds: 5), - Duration(seconds: 10), - ], + this.backoff = kDefaultSyncBackoff, this.fileFromPath = _defaultFileFromPath, }); diff --git a/lib/src/sync/payload_assembler.dart b/lib/src/sync/payload_assembler.dart index 4326dbb..00462bc 100644 --- a/lib/src/sync/payload_assembler.dart +++ b/lib/src/sync/payload_assembler.dart @@ -1,5 +1,6 @@ import 'package:sqflite/sqflite.dart'; +import '../database/schema/system_columns.dart'; import '../models/doc_type_meta.dart'; import '../models/outbox_row.dart'; import 'child_table_info.dart'; @@ -11,26 +12,15 @@ import 'uuid_rewriter.dart'; /// of the struct collapse onto the canonical class. typedef ChildInfo = ChildTableInfo; -// Columns dropped before the assembled payload reaches the wire. This -// is intentionally NARROWER than `PayloadSerializer.serializeForBase` — -// `__is_local` companion columns are kept here so [UuidRewriter] can -// see which Link values are local UUIDs that need rewriting. UuidRewriter -// strips `__is_local` itself before returning. -const _systemColumns = { - 'mobile_uuid', - 'server_name', - 'sync_status', - 'sync_error', - 'error_code', - 'sync_attempts', - 'last_attempt_at', - 'sync_op', - 'push_base_payload', - 'local_modified', - 'pulled_at', - // `modified` is handled explicitly: included for UPDATE/SUBMIT (Frappe's - // check_if_latest), excluded for INSERT. -}; +// Columns dropped before the assembled payload reaches the wire. Shared +// with [PayloadSerializer] via [systemSyncMetadataColumnNames] so the two +// strip-decisions cannot drift apart. Intentionally NARROWER than +// `PayloadSerializer.serializeForBase` — `__is_local` companion columns +// are kept here so [UuidRewriter] can see which Link values are local +// UUIDs that need rewriting. UuidRewriter strips `__is_local` itself +// before returning. `modified` is handled explicitly: included for +// UPDATE/SUBMIT (Frappe's check_if_latest), excluded for INSERT. +const _systemColumns = systemSyncMetadataColumnNames; class PayloadAssembler { /// Reads the authoritative DB snapshot for [row], builds a payload dict diff --git a/lib/src/sync/payload_serializer.dart b/lib/src/sync/payload_serializer.dart index 6ef1c22..0b167ac 100644 --- a/lib/src/sync/payload_serializer.dart +++ b/lib/src/sync/payload_serializer.dart @@ -1,3 +1,4 @@ +import '../database/schema/system_columns.dart'; import '../models/doc_type_meta.dart'; /// Serializes a `docs__` row into the canonical user-fields-only @@ -13,29 +14,13 @@ import '../models/doc_type_meta.dart'; /// and as the parent-field portion of a Frappe payload, so the merge /// base and the eventual push body are byte-comparable. class PayloadSerializer { - /// Columns excluded from outbound payloads. Mirrors the (private) - /// allow-list previously inlined in `PayloadAssembler._systemColumns`, - /// extended with the new metadata columns added by the legacy-retirement - /// schema bump (`error_code`, `last_attempt_at`, `push_base_payload`). + /// Columns excluded from outbound payloads. Sourced from the shared + /// [systemSyncMetadataColumnNames] so this strip-decision and the one + /// in `PayloadAssembler` cannot drift apart. /// /// Note: `docstatus` and `modified` are NOT in this set — they are /// genuine Frappe doc fields that the server expects on the wire. - static const _excludedColumns = { - // Identity / link columns — emitted explicitly by the caller. - 'mobile_uuid', - 'server_name', - // Per-doc sync state. - 'sync_status', - 'sync_error', - 'error_code', - 'sync_attempts', - 'last_attempt_at', - 'sync_op', - 'push_base_payload', - // Local bookkeeping. - 'local_modified', - 'pulled_at', - }; + static const _excludedColumns = systemSyncMetadataColumnNames; static Map serializeForBase( Map row, diff --git a/lib/src/sync/pull_engine.dart b/lib/src/sync/pull_engine.dart index fab19af..b3dd221 100644 --- a/lib/src/sync/pull_engine.dart +++ b/lib/src/sync/pull_engine.dart @@ -7,7 +7,6 @@ import '../concurrency/concurrency_pool.dart'; import '../concurrency/write_queue.dart'; import '../database/daos/doctype_meta_dao.dart'; import '../database/daos/outbox_dao.dart'; -import '../database/table_name.dart'; import '../models/closure_result.dart'; import '../models/dep_graph.dart'; import '../models/meta_resolver.dart'; @@ -139,9 +138,7 @@ class PullEngine { DoctypeSyncState(startedAt: startedAt), ); - final parentTable = - await metaDao.getTableName(doctype) ?? - normalizeDoctypeTableName(doctype); + final parentTable = await metaDao.tableNameFor(doctype); // Resolve child metas for every Table / Table MultiSelect outgoing edge. final childInfo = {}; diff --git a/lib/src/sync/push_engine.dart b/lib/src/sync/push_engine.dart index b254af2..17431e0 100644 --- a/lib/src/sync/push_engine.dart +++ b/lib/src/sync/push_engine.dart @@ -121,16 +121,8 @@ class PushEngine { required this.attachmentUploader, DependenciesForRowFn? dependencyScanner, this.writeQueueResolver, - this.attachmentBackoff = const [ - Duration(seconds: 2), - Duration(seconds: 5), - Duration(seconds: 10), - ], - this.networkBackoff = const [ - Duration(seconds: 2), - Duration(seconds: 5), - Duration(seconds: 10), - ], + this.attachmentBackoff = kDefaultSyncBackoff, + this.networkBackoff = kDefaultSyncBackoff, }) : dependencyScanner = dependencyScanner ?? _defaultDependencyScanner; /// Drains the outbox once. Call this on user save (debounced), on @@ -273,9 +265,7 @@ class PushEngine { ); final childMetas = await _childMetasFor(meta); - final parentTable = - await metaDao.getTableName(row.doctype) ?? - normalizeDoctypeTableName(row.doctype); + final parentTable = await metaDao.tableNameFor(row.doctype); // Read the per-doc snapshot — it's the canonical source of truth for // server_name and retry counters. The slim outbox no longer carries @@ -385,9 +375,7 @@ class PushEngine { entry.value.doctype, ); } - final parentTable = - await metaDao.getTableName(row.doctype) ?? - normalizeDoctypeTableName(row.doctype); + final parentTable = await metaDao.tableNameFor(row.doctype); if (writeQueueResolver != null) { final wq = _writeQueues.putIfAbsent( row.doctype, @@ -417,9 +405,7 @@ class PushEngine { OutboxRow row, TimestampMismatchError err, ) async { - final parentTable = - await metaDao.getTableName(row.doctype) ?? - normalizeDoctypeTableName(row.doctype); + final parentTable = await metaDao.tableNameFor(row.doctype); final currentRows = await db.query( parentTable, where: 'mobile_uuid = ?', diff --git a/lib/src/ui/document_list_screen.dart b/lib/src/ui/document_list_screen.dart index 66701c1..fca3c12 100644 --- a/lib/src/ui/document_list_screen.dart +++ b/lib/src/ui/document_list_screen.dart @@ -18,6 +18,7 @@ import '../services/sync_service.dart'; import 'form_screen.dart'; import 'widgets/form_builder.dart' show FrappeFormStyle, OnButtonPressedCallback, FieldChangeHandler; +import 'widgets/screen_helpers.dart'; import 'widgets/sync_error_banner.dart' show humanizeOutboxError; /// Layout variants for [DocumentListScreen]. @@ -376,21 +377,7 @@ class _DocumentListScreenState extends State { ]; }, ), - if (_isSyncing) - const Padding( - padding: EdgeInsets.all(16.0), - child: SizedBox( - width: 20, - height: 20, - child: CircularProgressIndicator(strokeWidth: 2), - ), - ) - else - IconButton( - icon: const Icon(Icons.refresh), - onPressed: _pullDocuments, - tooltip: 'Refresh', - ), + refreshOrSpinnerAction(isBusy: _isSyncing, onRefresh: _pullDocuments), ], ), body: _isLoading @@ -435,6 +422,10 @@ class _DocumentListScreenState extends State { Widget _buildEmptyState() { final listStyle = widget.style ?? const DocumentListStyle(); + // Original structure: `Center(Column(...))` with no surrounding + // Padding and a `bodySmall` subtitle — different enough from the + // canonical EmptyStateWidget (which adds Padding(24) and uses + // `bodyMedium`) that switching would visibly shift spacing. return Center( child: Column( mainAxisAlignment: MainAxisAlignment.center, diff --git a/lib/src/ui/form_screen.dart b/lib/src/ui/form_screen.dart index 0b9fee0..af4fdff 100644 --- a/lib/src/ui/form_screen.dart +++ b/lib/src/ui/form_screen.dart @@ -16,6 +16,7 @@ import '../services/sync_controller.dart'; import '../services/sync_service.dart'; import '../services/workflow_service.dart'; import '../utils/uuid_pattern.dart'; +import 'widgets/screen_helpers.dart'; import 'widgets/sync_error_banner.dart'; import 'widgets/form_builder.dart' show @@ -296,11 +297,10 @@ class _FormScreenState extends State with WidgetsBindingObserver { // ignore: avoid_print print('FormScreen: pushSync failed — $e\n$st'); if (mounted) { - ScaffoldMessenger.of(context).showSnackBar( - SnackBar( - content: Text('Push failed: ${toUserFriendlyMessage(e)}'), - backgroundColor: Colors.red, - ), + showStatusSnackBar( + context, + 'Push failed: ${toUserFriendlyMessage(e)}', + severity: SnackBarSeverity.error, ); } return; @@ -310,12 +310,11 @@ class _FormScreenState extends State with WidgetsBindingObserver { await _loadSyncErrors(); if (!mounted) return; final stuck = _syncErrorRows.isNotEmpty; - ScaffoldMessenger.of(context).showSnackBar( - SnackBar( - content: Text(stuck ? 'Push completed with errors' : 'Pushed'), - backgroundColor: stuck ? Colors.orange : Colors.green, - duration: const Duration(seconds: 2), - ), + showStatusSnackBar( + context, + stuck ? 'Push completed with errors' : 'Pushed', + severity: stuck ? SnackBarSeverity.warning : SnackBarSeverity.success, + duration: const Duration(seconds: 2), ); } @@ -368,22 +367,20 @@ class _FormScreenState extends State with WidgetsBindingObserver { _isFormDirty.value = false; await _loadWorkflowTransitions(); if (mounted) { - ScaffoldMessenger.of(context).showSnackBar( - SnackBar( - content: Text('Workflow: $action'), - backgroundColor: Colors.green, - ), + showStatusSnackBar( + context, + 'Workflow: $action', + severity: SnackBarSeverity.success, ); } } } catch (e, st) { debugPrint('FormScreen._applyWorkflowAction($action) failed — $e\n$st'); if (mounted) { - ScaffoldMessenger.of(context).showSnackBar( - SnackBar( - content: Text(toUserFriendlyMessage(e)), - backgroundColor: Colors.red, - ), + showStatusSnackBar( + context, + toUserFriendlyMessage(e), + severity: SnackBarSeverity.error, ); } } @@ -457,15 +454,11 @@ class _FormScreenState extends State with WidgetsBindingObserver { final method = field.options?.trim(); if (method == null || method.isEmpty) { if (mounted) { - ScaffoldMessenger.of(context).showSnackBar( - SnackBar( - content: Text( - '${field.displayLabel}: Action not configured for mobile. ' - 'This button may use client-side logic only available on web.', - ), - backgroundColor: Colors.orange, - duration: const Duration(seconds: 4), - ), + showStatusSnackBar( + context, + '${field.displayLabel}: Action not configured for mobile. ' + 'This button may use client-side logic only available on web.', + severity: SnackBarSeverity.warning, ); } return; @@ -473,11 +466,10 @@ class _FormScreenState extends State with WidgetsBindingObserver { if (widget.api == null) { if (mounted) { - ScaffoldMessenger.of(context).showSnackBar( - const SnackBar( - content: Text('Action unavailable offline'), - backgroundColor: Colors.orange, - ), + showStatusSnackBar( + context, + 'Action unavailable offline', + severity: SnackBarSeverity.warning, ); } return; @@ -486,21 +478,19 @@ class _FormScreenState extends State with WidgetsBindingObserver { try { await widget.api!.call(method, args: {'doc': formData}); if (mounted) { - ScaffoldMessenger.of(context).showSnackBar( - const SnackBar( - content: Text('Action completed'), - backgroundColor: Colors.green, - ), + showStatusSnackBar( + context, + 'Action completed', + severity: SnackBarSeverity.success, ); } } catch (e, st) { debugPrint('FormScreen.action($method) failed — $e\n$st'); if (mounted) { - ScaffoldMessenger.of(context).showSnackBar( - SnackBar( - content: Text(toUserFriendlyMessage(e)), - backgroundColor: Colors.red, - ), + showStatusSnackBar( + context, + toUserFriendlyMessage(e), + severity: SnackBarSeverity.error, ); } } @@ -699,11 +689,10 @@ class _FormScreenState extends State with WidgetsBindingObserver { _baselineFormData = savedData!; }); _isFormDirty.value = false; - ScaffoldMessenger.of(context).showSnackBar( - const SnackBar( - content: Text('Saved successfully'), - backgroundColor: Colors.green, - ), + showStatusSnackBar( + context, + 'Saved successfully', + severity: SnackBarSeverity.success, ); widget.onSaveSuccess?.call(); } @@ -741,11 +730,10 @@ class _FormScreenState extends State with WidgetsBindingObserver { _baselineFormData = savedData; }); _isFormDirty.value = false; - ScaffoldMessenger.of(context).showSnackBar( - const SnackBar( - content: Text('Document saved successfully'), - backgroundColor: Colors.green, - ), + showStatusSnackBar( + context, + 'Document saved successfully', + severity: SnackBarSeverity.success, ); widget.onSaveSuccess?.call(); } @@ -769,22 +757,12 @@ class _FormScreenState extends State with WidgetsBindingObserver { Future _handleDelete() async { if (widget.document == null) return; - final confirmed = await showDialog( - context: context, - builder: (context) => AlertDialog( - title: const Text('Delete Document'), - content: const Text('Are you sure you want to delete this document?'), - actions: [ - TextButton( - onPressed: () => Navigator.pop(context, false), - child: const Text('Cancel'), - ), - TextButton( - onPressed: () => Navigator.pop(context, true), - child: const Text('Delete', style: TextStyle(color: Colors.red)), - ), - ], - ), + final confirmed = await showConfirmDialog( + context, + title: 'Delete Document', + content: 'Are you sure you want to delete this document?', + confirmLabel: 'Delete', + confirmColor: Colors.red, ); if (confirmed != true) return; @@ -810,11 +788,10 @@ class _FormScreenState extends State with WidgetsBindingObserver { } if (mounted) { - ScaffoldMessenger.of(context).showSnackBar( - const SnackBar( - content: Text('Document deleted'), - backgroundColor: Colors.orange, - ), + showStatusSnackBar( + context, + 'Document deleted', + severity: SnackBarSeverity.warning, ); Navigator.pop(context); widget.onSaveSuccess?.call(); @@ -922,23 +899,7 @@ class _FormScreenState extends State with WidgetsBindingObserver { onRetry: widget.syncController == null ? null : _retrySyncRow, ), if (_errorMessage != null) - Container( - width: double.infinity, - padding: const EdgeInsets.all(12), - color: Colors.red[50], - child: Row( - children: [ - const Icon(Icons.error, color: Colors.red), - const SizedBox(width: 8), - Expanded( - child: Text( - _errorMessage!, - style: const TextStyle(color: Colors.red), - ), - ), - ], - ), - ), + ErrorMessageBanner(message: _errorMessage!), if (widget.meta.hasWorkflow && widget.document != null && widget.api != null) diff --git a/lib/src/ui/login_screen.dart b/lib/src/ui/login_screen.dart index 3bae852..cdb6fd0 100644 --- a/lib/src/ui/login_screen.dart +++ b/lib/src/ui/login_screen.dart @@ -334,6 +334,10 @@ class _LoginScreenState extends State { }); final uri = Uri.parse(authorizeUrl); final canLaunch = await canLaunchUrl(uri); + // Match _startOAuth: bail if the widget unmounted while we awaited + // canLaunchUrl/launchUrl so the subsequent setState doesn't fire on + // a deactivated state. + if (!mounted) return; if (canLaunch) { await launchUrl(uri, mode: LaunchMode.externalApplication); } else { @@ -347,6 +351,7 @@ class _LoginScreenState extends State { } catch (e, st) { debugPrint('LoginScreen._startSocialOAuth failed — $e\n$st'); _cancelOAuthListener(); + if (!mounted) return; setState(() { _errorMessage = e.toString().replaceAll('Exception: ', ''); _isLoading = false; @@ -385,10 +390,12 @@ class _LoginScreenState extends State { } } catch (e, st) { debugPrint('LoginScreen._handleLogin failed — $e\n$st'); - setState(() { - _errorMessage = e.toString().replaceAll('Exception: ', ''); - _isLoading = false; - }); + if (mounted) { + setState(() { + _errorMessage = e.toString().replaceAll('Exception: ', ''); + _isLoading = false; + }); + } } } @@ -451,10 +458,12 @@ class _LoginScreenState extends State { } } catch (e, st) { debugPrint('LoginScreen._handleVerifyOtp failed — $e\n$st'); - setState(() { - _errorMessage = e.toString().replaceAll('Exception: ', ''); - _isLoading = false; - }); + if (mounted) { + setState(() { + _errorMessage = e.toString().replaceAll('Exception: ', ''); + _isLoading = false; + }); + } } } diff --git a/lib/src/ui/offline_transition_screen.dart b/lib/src/ui/offline_transition_screen.dart index 2629f86..8f67944 100644 --- a/lib/src/ui/offline_transition_screen.dart +++ b/lib/src/ui/offline_transition_screen.dart @@ -1,6 +1,7 @@ import 'package:flutter/material.dart'; import '../services/offline_transition_service.dart'; +import 'widgets/screen_helpers.dart'; /// Full-screen UI for the offline → online transition flow. /// @@ -87,25 +88,13 @@ class _Failed extends StatelessWidget { const _Failed({required this.state, required this.service}); Future _confirmForceExit(BuildContext context) async { - final confirm = await showDialog( - context: context, - builder: (ctx) => AlertDialog( - title: const Text('Force exit?'), - content: Text( + final confirm = await showConfirmDialog( + context, + title: 'Force exit?', + content: 'Discarding ${state.remainingDirty} pending record(s). ' 'This cannot be undone.', - ), - actions: [ - TextButton( - onPressed: () => Navigator.pop(ctx, false), - child: const Text('Cancel'), - ), - TextButton( - onPressed: () => Navigator.pop(ctx, true), - child: const Text('Discard'), - ), - ], - ), + confirmLabel: 'Discard', ); if (confirm == true) await service.forceExit(); } diff --git a/lib/src/ui/screens/sync_errors_screen.dart b/lib/src/ui/screens/sync_errors_screen.dart index b5f09c6..3e0c351 100644 --- a/lib/src/ui/screens/sync_errors_screen.dart +++ b/lib/src/ui/screens/sync_errors_screen.dart @@ -1,6 +1,7 @@ import 'package:flutter/material.dart'; import '../../models/outbox_row.dart'; +import '../widgets/screen_helpers.dart'; /// List of currently-erroring outbox rows, grouped by doctype with /// per-row Retry / View error / Open actions and a header `Retry all` @@ -95,29 +96,15 @@ class SyncErrorsScreen extends StatelessWidget { Widget _buildEmptyState(BuildContext context) { final theme = Theme.of(context); - return Center( - child: Padding( - padding: const EdgeInsets.all(24), - child: Column( - mainAxisSize: MainAxisSize.min, - children: [ - Icon( - Icons.check_circle_outline, - size: 64, - color: theme.colorScheme.primary, - ), - const SizedBox(height: 16), - Text('No sync errors', style: theme.textTheme.titleMedium), - const SizedBox(height: 8), - Text( - 'All queued changes have synced successfully.', - textAlign: TextAlign.center, - style: theme.textTheme.bodyMedium?.copyWith( - color: theme.colorScheme.onSurfaceVariant, - ), - ), - ], - ), + return EmptyStateWidget( + icon: Icons.check_circle_outline, + iconColor: theme.colorScheme.primary, + title: 'No sync errors', + subtitle: 'All queued changes have synced successfully.', + // Original used onSurfaceVariant (theme-adaptive in dark mode); + // preserve exactly instead of the helper's grey-600 default. + subtitleStyle: theme.textTheme.bodyMedium?.copyWith( + color: theme.colorScheme.onSurfaceVariant, ), ); } diff --git a/lib/src/ui/sync_status_screen.dart b/lib/src/ui/sync_status_screen.dart index f8d00b8..c5821c1 100644 --- a/lib/src/ui/sync_status_screen.dart +++ b/lib/src/ui/sync_status_screen.dart @@ -2,6 +2,7 @@ import 'package:flutter/material.dart'; import '../services/sync_service.dart'; import '../services/offline_repository.dart'; import '../models/document.dart'; +import 'widgets/screen_helpers.dart'; /// Screen to display sync status and errors class SyncStatusScreen extends StatefulWidget { @@ -77,11 +78,10 @@ class _SyncStatusScreenState extends State { _showErrorDialog(result.errors); } } else if (result.success > 0 && mounted) { - ScaffoldMessenger.of(context).showSnackBar( - SnackBar( - content: Text('Successfully synced ${result.success} document(s)'), - backgroundColor: Colors.green, - ), + showStatusSnackBar( + context, + 'Successfully synced ${result.success} document(s)', + severity: SnackBarSeverity.success, ); } } catch (e, st) { @@ -92,11 +92,10 @@ class _SyncStatusScreenState extends State { }); if (mounted) { - ScaffoldMessenger.of(context).showSnackBar( - SnackBar( - content: Text('Sync error: $e'), - backgroundColor: Colors.red, - ), + showStatusSnackBar( + context, + 'Sync error: $e', + severity: SnackBarSeverity.error, ); } } @@ -128,11 +127,10 @@ class _SyncStatusScreenState extends State { _showErrorDialog(result.errors); } } else if (result.success > 0 && mounted) { - ScaffoldMessenger.of(context).showSnackBar( - SnackBar( - content: Text('Successfully synced ${result.success} document(s)'), - backgroundColor: Colors.green, - ), + showStatusSnackBar( + context, + 'Successfully synced ${result.success} document(s)', + severity: SnackBarSeverity.success, ); } } catch (e, st) { @@ -143,11 +141,10 @@ class _SyncStatusScreenState extends State { }); if (mounted) { - ScaffoldMessenger.of(context).showSnackBar( - SnackBar( - content: Text('Sync error: $e'), - backgroundColor: Colors.red, - ), + showStatusSnackBar( + context, + 'Sync error: $e', + severity: SnackBarSeverity.error, ); } } @@ -217,21 +214,10 @@ class _SyncStatusScreenState extends State { appBar: AppBar( title: const Text('Sync Status'), actions: [ - if (_isSyncing) - const Padding( - padding: EdgeInsets.all(16.0), - child: SizedBox( - width: 20, - height: 20, - child: CircularProgressIndicator(strokeWidth: 2), - ), - ) - else - IconButton( - icon: const Icon(Icons.refresh), - onPressed: _loadDirtyDocuments, - tooltip: 'Refresh', - ), + refreshOrSpinnerAction( + isBusy: _isSyncing, + onRefresh: _loadDirtyDocuments, + ), ], ), body: Column( @@ -285,6 +271,9 @@ class _SyncStatusScreenState extends State { child: _isLoading ? const Center(child: CircularProgressIndicator()) : _dirtyDocuments.isEmpty + // Original used `Center(Column(...))` with no surrounding + // Padding and a bold title — EmptyStateWidget's canonical + // Padding(24) + titleMedium would visibly shift this. ? Center( child: Column( mainAxisAlignment: MainAxisAlignment.center, diff --git a/lib/src/ui/widgets/fields/attach_field.dart b/lib/src/ui/widgets/fields/attach_field.dart index a8f6a2d..3a21780 100644 --- a/lib/src/ui/widgets/fields/attach_field.dart +++ b/lib/src/ui/widgets/fields/attach_field.dart @@ -6,6 +6,7 @@ import 'package:flutter/material.dart'; import 'package:flutter_form_builder/flutter_form_builder.dart'; import 'package:file_picker/file_picker.dart'; import 'base_field.dart'; +import 'field_helpers.dart'; /// Widget for Attach field type. /// When [uploadFile] is set, picks upload to server first and store file_url; otherwise stores local path. @@ -32,22 +33,17 @@ class AttachField extends BaseField { initialValue: filePath, enabled: enabled && !field.readOnly, validator: field.reqd - ? (value) { - if (value == null || value.isEmpty) { - return '${field.displayLabel} is required'; - } - return null; - } + ? (value) => requiredValidator(value, field.displayLabel) : null, builder: (FormFieldState fieldState) { + // BaseField.build (the enclosing widget) already renders the + // external label with required-asterisk + translation. The inline + // Padding(Text(field.label)) that used to live here was a + // second copy that skipped the asterisk — removed for visual + // consistency with text/numeric/etc field widgets. return Column( crossAxisAlignment: CrossAxisAlignment.start, children: [ - if (field.label != null) - Padding( - padding: const EdgeInsets.only(bottom: 8.0), - child: Text(field.label!, style: style?.labelStyle), - ), OutlinedButton.icon( onPressed: enabled && !field.readOnly ? () async { @@ -90,14 +86,7 @@ class AttachField extends BaseField { overflow: TextOverflow.ellipsis, ), ), - if (fieldState.hasError) - Padding( - padding: const EdgeInsets.only(top: 4.0), - child: Text( - fieldState.errorText!, - style: const TextStyle(color: Colors.red, fontSize: 12), - ), - ), + fieldErrorText(fieldState), ], ); }, diff --git a/lib/src/ui/widgets/fields/child_table_field.dart b/lib/src/ui/widgets/fields/child_table_field.dart index 8426859..4e40370 100644 --- a/lib/src/ui/widgets/fields/child_table_field.dart +++ b/lib/src/ui/widgets/fields/child_table_field.dart @@ -1,6 +1,7 @@ import 'package:flutter/material.dart'; import '../../../models/doc_field.dart'; import '../../../models/doc_type_meta.dart'; +import '../screen_helpers.dart'; /// Builds the form widget for a child table row (add/edit dialog or bottom sheet). /// [registerSubmit] is called with the form's submit handler so the host can show Save/Cancel. @@ -164,7 +165,15 @@ class ChildTableField extends StatelessWidget { BuildContext context, List listValue, ) async { - if (getMeta == null || field.options == null || formBuilder == null) return; + // The same `onChanged == null` guard the edit-row dialog has at + // line 213. Without it, the add dialog appears successfully and then + // crashes when invoking `onChanged!(newList)` after the user submits. + if (getMeta == null || + field.options == null || + onChanged == null || + formBuilder == null) { + return; + } DocTypeMeta? childMeta; try { @@ -174,9 +183,9 @@ class ChildTableField extends StatelessWidget { 'ChildTableField._showAddRowDialog: getMeta(${field.options}) failed — $e\n$st', ); if (context.mounted) { - ScaffoldMessenger.of( - context, - ).showSnackBar(SnackBar(content: Text('Error loading form: $e'))); + // Original called the bare `SnackBar(content: Text(...))` with no + // backgroundColor — preserve that neutral styling explicitly. + showStatusSnackBar(context, 'Error loading form: $e'); } return; } @@ -223,9 +232,9 @@ class ChildTableField extends StatelessWidget { 'ChildTableField._showEditRowDialog: getMeta(${field.options}) failed — $e\n$st', ); if (context.mounted) { - ScaffoldMessenger.of( - context, - ).showSnackBar(SnackBar(content: Text('Error loading form: $e'))); + // Original called the bare `SnackBar(content: Text(...))` with no + // backgroundColor — preserve that neutral styling explicitly. + showStatusSnackBar(context, 'Error loading form: $e'); } return; } diff --git a/lib/src/ui/widgets/fields/data_field.dart b/lib/src/ui/widgets/fields/data_field.dart index 3428867..7f27dc2 100644 --- a/lib/src/ui/widgets/fields/data_field.dart +++ b/lib/src/ui/widgets/fields/data_field.dart @@ -1,6 +1,7 @@ import 'package:flutter/material.dart'; import 'package:flutter_form_builder/flutter_form_builder.dart'; import 'base_field.dart'; +import 'field_helpers.dart'; /// Widget for Data field type class DataField extends BaseField { @@ -52,11 +53,10 @@ class DataField extends BaseField { : null, validator: field.reqd ? (value) { - if (value == null || value.toString().isEmpty) { - return '${field.displayLabel} is required'; - } + final required = requiredValidator(value, field.displayLabel); + if (required != null) return required; // Phone validation - must start with + and country code - if (isPhone && value.isNotEmpty) { + if (isPhone && value!.isNotEmpty) { final trimmed = value.trim(); // Check if it starts with + (required by Frappe) if (!trimmed.startsWith('+')) { diff --git a/lib/src/ui/widgets/fields/date_field.dart b/lib/src/ui/widgets/fields/date_field.dart index 8c8e506..ba47879 100644 --- a/lib/src/ui/widgets/fields/date_field.dart +++ b/lib/src/ui/widgets/fields/date_field.dart @@ -1,7 +1,9 @@ import 'package:flutter/material.dart'; import 'package:flutter_form_builder/flutter_form_builder.dart'; import 'package:intl/intl.dart'; +import '../../../utils/date_helpers.dart'; import 'base_field.dart'; +import 'field_helpers.dart'; /// Widget for Date field type class DateField extends BaseField { @@ -16,19 +18,10 @@ class DateField extends BaseField { @override Widget buildField(BuildContext context) { - DateTime? initialDate; - if (value != null) { - if (value is DateTime) { - initialDate = value; - } else if (value is String) { - initialDate = DateTime.tryParse(value); - } - } - return FormBuilderDateTimePicker( key: ValueKey('date_${field.fieldname}'), name: field.fieldname ?? '', - initialValue: initialDate, + initialValue: parseDateTime(value), enabled: enabled && !field.readOnly, inputType: InputType.date, format: DateFormat('yyyy-MM-dd'), @@ -42,12 +35,7 @@ class DateField extends BaseField { suffixIcon: const Icon(Icons.calendar_today), ), validator: field.reqd - ? (value) { - if (value == null) { - return '${field.displayLabel} is required'; - } - return null; - } + ? (value) => requiredValidator(value, field.displayLabel) : null, onChanged: (val) => onChanged?.call(val?.toIso8601String()), ); diff --git a/lib/src/ui/widgets/fields/datetime_field.dart b/lib/src/ui/widgets/fields/datetime_field.dart index 355fce5..a8d987b 100644 --- a/lib/src/ui/widgets/fields/datetime_field.dart +++ b/lib/src/ui/widgets/fields/datetime_field.dart @@ -4,7 +4,9 @@ import 'package:flutter/material.dart'; import 'package:flutter_form_builder/flutter_form_builder.dart'; import 'package:intl/intl.dart'; +import '../../../utils/date_helpers.dart'; import 'base_field.dart'; +import 'field_helpers.dart'; /// Widget for Datetime field type class DatetimeField extends BaseField { @@ -19,19 +21,10 @@ class DatetimeField extends BaseField { @override Widget buildField(BuildContext context) { - DateTime? initialDateTime; - if (value != null) { - if (value is DateTime) { - initialDateTime = value; - } else if (value is String) { - initialDateTime = DateTime.tryParse(value); - } - } - return FormBuilderDateTimePicker( key: ValueKey('datetime_${field.fieldname}'), name: field.fieldname ?? '', - initialValue: initialDateTime, + initialValue: parseDateTime(value), enabled: enabled && !field.readOnly, inputType: InputType.both, format: DateFormat('yyyy-MM-dd HH:mm:ss'), @@ -45,12 +38,7 @@ class DatetimeField extends BaseField { suffixIcon: const Icon(Icons.access_time), ), validator: field.reqd - ? (value) { - if (value == null) { - return '${field.displayLabel} is required'; - } - return null; - } + ? (value) => requiredValidator(value, field.displayLabel) : null, onChanged: (val) => onChanged?.call(val?.toIso8601String()), ); diff --git a/lib/src/ui/widgets/fields/duration_field.dart b/lib/src/ui/widgets/fields/duration_field.dart index 6a4d3da..07ea817 100644 --- a/lib/src/ui/widgets/fields/duration_field.dart +++ b/lib/src/ui/widgets/fields/duration_field.dart @@ -3,7 +3,9 @@ import 'package:flutter/material.dart'; import 'package:flutter_form_builder/flutter_form_builder.dart'; +import '../../../utils/date_helpers.dart'; import 'base_field.dart'; +import 'field_helpers.dart'; /// Widget for Duration field type (in seconds) class DurationField extends BaseField { @@ -16,17 +18,6 @@ class DurationField extends BaseField { super.style, }); - String _formatDuration(int seconds) { - final hours = seconds ~/ 3600; - final minutes = (seconds % 3600) ~/ 60; - final secs = seconds % 60; - - if (hours > 0) { - return '${hours.toString().padLeft(2, '0')}:${minutes.toString().padLeft(2, '0')}:${secs.toString().padLeft(2, '0')}'; - } - return '${minutes.toString().padLeft(2, '0')}:${secs.toString().padLeft(2, '0')}'; - } - int? _parseDuration(String? value) { if (value == null || value.isEmpty) return null; @@ -69,7 +60,7 @@ class DurationField extends BaseField { key: ValueKey('duration_${field.fieldname}'), name: field.fieldname ?? '', initialValue: initialSeconds != null - ? _formatDuration(initialSeconds) + ? formatDurationSeconds(initialSeconds) : null, enabled: enabled && !field.readOnly, keyboardType: TextInputType.number, @@ -84,9 +75,8 @@ class DurationField extends BaseField { ), validator: field.reqd ? (value) { - if (value == null || value.isEmpty) { - return '${field.displayLabel} is required'; - } + final required = requiredValidator(value, field.displayLabel); + if (required != null) return required; if (_parseDuration(value) == null) { return 'Invalid duration format'; } diff --git a/lib/src/ui/widgets/fields/field_helpers.dart b/lib/src/ui/widgets/fields/field_helpers.dart new file mode 100644 index 0000000..62a05aa --- /dev/null +++ b/lib/src/ui/widgets/fields/field_helpers.dart @@ -0,0 +1,51 @@ +import 'package:flutter/material.dart'; + +import '../../../models/doc_field.dart'; +import 'base_field.dart'; + +/// `validator` for a required field. Returns the standard +/// `'$label is required'` message when [value] is null OR its string form +/// is empty; null otherwise (valid). Shared by every field widget that +/// wires a non-null `validator:` callback so the message text and the +/// null-or-empty check do not drift between widgets. Use +/// `(value) => requiredValidator(value, field.displayLabel)` (or fold +/// translation through [BaseField.style]) at the call site. +String? requiredValidator(dynamic value, String label) { + if (value == null || value.toString().isEmpty) { + return '$label is required'; + } + return null; +} + +/// Returns an `InputDecoration` that applies the SDK's read-only fill +/// (grey-200 background when `field.readOnly`) on top of any caller-provided +/// [style] decoration. Used by every text-shaped field widget so a change +/// to the read-only theming applies everywhere at once. +InputDecoration baseFieldDecoration( + DocField field, { + String? hint, + FieldStyle? style, +}) { + return style?.decoration ?? + InputDecoration( + hintText: hint ?? field.placeholder, + border: const OutlineInputBorder(), + filled: field.readOnly, + fillColor: field.readOnly ? Colors.grey[200] : null, + ); +} + +/// Renders the field's validation error in the standard red 12px style +/// underneath the input. Used by the custom `FormBuilderField.builder` +/// field widgets (attach, image, rating) that build their own column and +/// need to surface `fieldState.errorText` manually. +Widget fieldErrorText(FormFieldState fieldState) { + if (!fieldState.hasError) return const SizedBox.shrink(); + return Padding( + padding: const EdgeInsets.only(top: 4.0), + child: Text( + fieldState.errorText!, + style: const TextStyle(color: Colors.red, fontSize: 12), + ), + ); +} diff --git a/lib/src/ui/widgets/fields/image_field.dart b/lib/src/ui/widgets/fields/image_field.dart index c972c04..586fa93 100644 --- a/lib/src/ui/widgets/fields/image_field.dart +++ b/lib/src/ui/widgets/fields/image_field.dart @@ -6,6 +6,7 @@ import 'package:flutter/material.dart'; import 'package:flutter_form_builder/flutter_form_builder.dart'; import 'package:image_picker/image_picker.dart'; import 'base_field.dart'; +import 'field_helpers.dart'; /// Widget for Image/Attach Image field type. /// When [uploadFile] is set, picks upload to server first and store file_url; otherwise stores local path. @@ -101,12 +102,7 @@ class ImageField extends BaseField { initialValue: imagePath, enabled: enabled && !field.readOnly, validator: field.reqd - ? (value) { - if (value == null || value.isEmpty) { - return '${field.displayLabel} is required'; - } - return null; - } + ? (value) => requiredValidator(value, field.displayLabel) : null, builder: (FormFieldState fieldState) { final raw = fieldState.value ?? imagePath; @@ -114,14 +110,12 @@ class ImageField extends BaseField { final isUrl = _isServerUrl(currentValue); final displayUrl = isUrl ? _fullImageUrl(currentValue) : null; + // BaseField.build (the enclosing widget) already renders the + // external label with required-asterisk; the inline label that + // used to live here is gone for parity with text/numeric/etc. return Column( crossAxisAlignment: CrossAxisAlignment.start, children: [ - if (field.label != null) - Padding( - padding: const EdgeInsets.only(bottom: 8.0), - child: Text(field.label!, style: style?.labelStyle), - ), if (currentValue != null && currentValue.isNotEmpty) Padding( padding: const EdgeInsets.only(bottom: 8.0), @@ -200,14 +194,7 @@ class ImageField extends BaseField { ), ], ), - if (fieldState.hasError) - Padding( - padding: const EdgeInsets.only(top: 4.0), - child: Text( - fieldState.errorText!, - style: const TextStyle(color: Colors.red, fontSize: 12), - ), - ), + fieldErrorText(fieldState), ], ); }, diff --git a/lib/src/ui/widgets/fields/link_field.dart b/lib/src/ui/widgets/fields/link_field.dart index 61855b4..090efa8 100644 --- a/lib/src/ui/widgets/fields/link_field.dart +++ b/lib/src/ui/widgets/fields/link_field.dart @@ -8,6 +8,8 @@ import '../../../models/link_filter_result.dart'; import '../../../services/link_option_service.dart'; import '../../../services/link_field_coordinator.dart'; import '../../../database/entities/link_option_entity.dart'; +import '../../../utils/uuid_pattern.dart'; +import 'field_helpers.dart'; import 'searchable_select.dart'; /// Widget for Link field type with cached options @@ -57,12 +59,16 @@ class LinkField extends BaseField { } } - // Auto-select when exactly one option and no valid selection + // Auto-select when exactly one option and no valid selection. + // Propagate `onIsLocalChanged` so an auto-picked UUID-shaped + // option (offline mobile_uuid) flips `__is_local` for + // UuidRewriter at push time — matches `_applyOptionsAndAutoSelect`. if (options!.length == 1 && (validInitialValue == null || validInitialValue.isEmpty)) { validInitialValue = options!.first; WidgetsBinding.instance.addPostFrameCallback((_) { onChanged?.call(options!.first); + onIsLocalChanged?.call(looksLikeMobileUuid(options!.first)); }); } @@ -85,12 +91,7 @@ class LinkField extends BaseField { ) .toList(), validator: field.reqd - ? (value) { - if (value == null || value.toString().isEmpty) { - return '${field.displayLabel} is required'; - } - return null; - } + ? (value) => requiredValidator(value, field.displayLabel) : null, onChanged: (val) => onChanged?.call(val), ); @@ -133,12 +134,7 @@ class LinkField extends BaseField { suffixIcon: const Icon(Icons.search), ), validator: field.reqd - ? (value) { - if (value == null || value.toString().isEmpty) { - return '${field.displayLabel} is required'; - } - return null; - } + ? (value) => requiredValidator(value, field.displayLabel) : null, onChanged: (val) => onChanged?.call(val), ); @@ -275,14 +271,7 @@ class _LinkFieldDropdownState extends State<_LinkFieldDropdown> { final dependentNames = LinkOptionService.getDependentFieldNames( widget.linkFilters, ); - setState(() { - _options = []; - _isLoading = false; - _waitingForDependent = true; - _dependentFieldName = dependentNames.isNotEmpty - ? dependentNames.first - : ''; - }); + _setWaitingForDependent(dependentNames); return; } setState(() => _isLoading = true); @@ -293,6 +282,23 @@ class _LinkFieldDropdownState extends State<_LinkFieldDropdown> { ); } + /// Puts the dropdown into the "waiting for parent field" state. Always + /// guards `.first` against an empty list (defensive — the coordinator + /// path historically did this with a ternary, the `_loadOptions` path + /// relied on an outer `.isNotEmpty` if-condition; consolidating into + /// one helper means a future code edit can't accidentally call `.first` + /// unguarded). + void _setWaitingForDependent(List dependentNames) { + setState(() { + _options = []; + _isLoading = false; + _waitingForDependent = true; + _dependentFieldName = dependentNames.isNotEmpty + ? dependentNames.first + : ''; + }); + } + DocField? _docFieldFromDynamic(dynamic f) { if (f == null) return null; if (f is DocField) return f; @@ -335,12 +341,7 @@ class _LinkFieldDropdownState extends State<_LinkFieldDropdown> { widget.linkFilters!.isNotEmpty && filters == null && dependentNames.isNotEmpty) { - setState(() { - _options = []; - _isLoading = false; - _waitingForDependent = true; - _dependentFieldName = dependentNames.first; - }); + _setWaitingForDependent(dependentNames); return; } try { diff --git a/lib/src/ui/widgets/fields/numeric_field.dart b/lib/src/ui/widgets/fields/numeric_field.dart index 84fa69b..d6bb750 100644 --- a/lib/src/ui/widgets/fields/numeric_field.dart +++ b/lib/src/ui/widgets/fields/numeric_field.dart @@ -1,6 +1,7 @@ import 'package:flutter/material.dart'; import 'package:flutter_form_builder/flutter_form_builder.dart'; import 'base_field.dart'; +import 'field_helpers.dart'; /// Widget for numeric field types (Int, Float, Currency, Percent) class NumericField extends BaseField { @@ -37,12 +38,11 @@ class NumericField extends BaseField { ), validator: field.reqd ? (value) { - if (value == null || value.toString().isEmpty) { - return '${field.displayLabel} is required'; - } + final required = requiredValidator(value, field.displayLabel); + if (required != null) return required; final numValue = isInt - ? int.tryParse(value) - : double.tryParse(value); + ? int.tryParse(value!) + : double.tryParse(value!); if (numValue == null) { return 'Please enter a valid number'; } diff --git a/lib/src/ui/widgets/fields/password_field.dart b/lib/src/ui/widgets/fields/password_field.dart index 03de125..09f3550 100644 --- a/lib/src/ui/widgets/fields/password_field.dart +++ b/lib/src/ui/widgets/fields/password_field.dart @@ -4,6 +4,7 @@ import 'package:flutter/material.dart'; import 'package:flutter_form_builder/flutter_form_builder.dart'; import 'base_field.dart'; +import 'field_helpers.dart'; /// Widget for Password field type class PasswordField extends BaseField { @@ -34,12 +35,7 @@ class PasswordField extends BaseField { suffixIcon: const Icon(Icons.lock), ), validator: field.reqd - ? (value) { - if (value == null || value.isEmpty) { - return '${field.displayLabel} is required'; - } - return null; - } + ? (value) => requiredValidator(value, field.displayLabel) : null, onChanged: (val) => onChanged?.call(val), ); diff --git a/lib/src/ui/widgets/fields/rating_field.dart b/lib/src/ui/widgets/fields/rating_field.dart index 2213411..ec5c5f2 100644 --- a/lib/src/ui/widgets/fields/rating_field.dart +++ b/lib/src/ui/widgets/fields/rating_field.dart @@ -4,6 +4,7 @@ import 'package:flutter/material.dart'; import 'package:flutter_form_builder/flutter_form_builder.dart'; import 'base_field.dart'; +import 'field_helpers.dart'; /// Widget for Rating field type class RatingField extends BaseField { @@ -44,22 +45,15 @@ class RatingField extends BaseField { initialValue: initialRating, enabled: enabled && !field.readOnly, validator: field.reqd - ? (value) { - if (value == null) { - return '${field.displayLabel} is required'; - } - return null; - } + ? (value) => requiredValidator(value, field.displayLabel) : null, builder: (FormFieldState fieldState) { + // BaseField.build already renders the external label with + // required-asterisk; the inline label that used to live here is + // gone for parity with text/numeric/etc field widgets. return Column( crossAxisAlignment: CrossAxisAlignment.start, children: [ - if (field.label != null) - Padding( - padding: const EdgeInsets.only(bottom: 8.0), - child: Text(field.label!, style: style?.labelStyle), - ), Row( children: List.generate(maxRating, (index) { final rating = index + 1; @@ -80,14 +74,7 @@ class RatingField extends BaseField { ); }), ), - if (fieldState.hasError) - Padding( - padding: const EdgeInsets.only(top: 4.0), - child: Text( - fieldState.errorText!, - style: const TextStyle(color: Colors.red, fontSize: 12), - ), - ), + fieldErrorText(fieldState), ], ); }, diff --git a/lib/src/ui/widgets/fields/select_field.dart b/lib/src/ui/widgets/fields/select_field.dart index 9b69c60..3c64756 100644 --- a/lib/src/ui/widgets/fields/select_field.dart +++ b/lib/src/ui/widgets/fields/select_field.dart @@ -1,6 +1,7 @@ import 'package:flutter/material.dart'; import 'package:flutter_form_builder/flutter_form_builder.dart'; import 'base_field.dart'; +import 'field_helpers.dart'; /// Widget for Select field type. Supports single and multi-select (when field.allowMultiple). class SelectField extends BaseField { @@ -90,12 +91,7 @@ class SelectField extends BaseField { .map((opt) => FormBuilderFieldOption(value: opt, child: Text(opt))) .toList(), validator: field.reqd - ? (value) { - if (value == null || value.isEmpty) { - return '${field.displayLabel} is required'; - } - return null; - } + ? (value) => requiredValidator(value, field.displayLabel) : null, onChanged: (val) => onChanged?.call(_listToValue(val)), ); @@ -137,12 +133,7 @@ class SelectField extends BaseField { return DropdownMenuItem(value: option, child: Text(option)); }).toList(), validator: field.reqd - ? (value) { - if (value == null || value.toString().isEmpty) { - return '${field.displayLabel} is required'; - } - return null; - } + ? (value) => requiredValidator(value, field.displayLabel) : null, onChanged: (val) => onChanged?.call(val), ); diff --git a/lib/src/ui/widgets/fields/text_field.dart b/lib/src/ui/widgets/fields/text_field.dart index 05a1263..ce2d7a8 100644 --- a/lib/src/ui/widgets/fields/text_field.dart +++ b/lib/src/ui/widgets/fields/text_field.dart @@ -1,6 +1,7 @@ import 'package:flutter/material.dart'; import 'package:flutter_form_builder/flutter_form_builder.dart'; import 'base_field.dart'; +import 'field_helpers.dart'; /// Widget for Text, Long Text, and Small Text field types class TextFieldWidget extends BaseField { @@ -24,25 +25,13 @@ class TextFieldWidget extends BaseField { name: field.fieldname ?? '', initialValue: value?.toString() ?? field.defaultValue ?? '', enabled: enabled && !field.readOnly, - decoration: - style?.decoration ?? - InputDecoration( - hintText: field.placeholder, - border: const OutlineInputBorder(), - filled: field.readOnly, - fillColor: field.readOnly ? Colors.grey[200] : null, - ), + decoration: baseFieldDecoration(field, style: style), maxLines: maxLines, maxLength: (field.length != null && field.length! > 0) ? field.length : null, validator: field.reqd - ? (value) { - if (value == null || value.toString().isEmpty) { - return '${field.displayLabel} is required'; - } - return null; - } + ? (value) => requiredValidator(value, field.displayLabel) : null, onChanged: (val) => onChanged?.call(val), ); diff --git a/lib/src/ui/widgets/fields/time_field.dart b/lib/src/ui/widgets/fields/time_field.dart index cb40652..a514b5a 100644 --- a/lib/src/ui/widgets/fields/time_field.dart +++ b/lib/src/ui/widgets/fields/time_field.dart @@ -5,6 +5,7 @@ import 'package:flutter/material.dart'; import 'package:flutter_form_builder/flutter_form_builder.dart'; import 'package:intl/intl.dart'; import 'base_field.dart'; +import 'field_helpers.dart'; /// Widget for Time field type class TimeField extends BaseField { @@ -54,12 +55,7 @@ class TimeField extends BaseField { suffixIcon: const Icon(Icons.access_time), ), validator: field.reqd - ? (value) { - if (value == null) { - return '${field.displayLabel} is required'; - } - return null; - } + ? (value) => requiredValidator(value, field.displayLabel) : null, onChanged: (val) { if (val != null) { diff --git a/lib/src/ui/widgets/form_builder.dart b/lib/src/ui/widgets/form_builder.dart index 0caa85a..786c98b 100644 --- a/lib/src/ui/widgets/form_builder.dart +++ b/lib/src/ui/widgets/form_builder.dart @@ -8,6 +8,7 @@ import '../../models/link_filter_result.dart'; import '../../constants/field_types.dart'; import '../../services/link_option_service.dart'; import '../../services/link_field_coordinator.dart'; +import '../../utils/date_helpers.dart'; import '../../utils/depends_on_evaluator.dart'; import 'fields/field_factory.dart'; import 'fields/base_field.dart'; @@ -342,17 +343,6 @@ class _FrappeFormBuilderState extends State }); } - String _formatDurationPatchedValue(int seconds) { - final hours = seconds ~/ 3600; - final minutes = (seconds % 3600) ~/ 60; - final secs = seconds % 60; - - if (hours > 0) { - return '${hours.toString().padLeft(2, '0')}:${minutes.toString().padLeft(2, '0')}:${secs.toString().padLeft(2, '0')}'; - } - return '${minutes.toString().padLeft(2, '0')}:${secs.toString().padLeft(2, '0')}'; - } - List _normalizeMultiSelectPatchedValue(dynamic value) { if (value == null) return []; if (value is List) { @@ -441,7 +431,7 @@ class _FrappeFormBuilderState extends State case FieldTypes.duration: if (value == null || value == '') return ''; - if (value is int) return _formatDurationPatchedValue(value); + if (value is int) return formatDurationSeconds(value); return value.toString(); case 'Table': @@ -568,28 +558,24 @@ class _FrappeFormBuilderState extends State } } - bool _shouldShowField(DocField field) { - if (field.dependsOn == null || field.dependsOn!.isEmpty) { - return true; - } - return DependsOnEvaluator.evaluate(field.dependsOn, _formData); + /// Evaluates a `depends_on` expression against the current form data, + /// returning [defaultValue] when [expr] is null or empty. Shared by + /// [_shouldShowField], [_isFieldRequired], and [_isFieldReadOnly] so a + /// future change to `DependsOnEvaluator.evaluate` (e.g. adding a parent + /// context parameter) applies to all three guards at once. + bool _evaluateDepends(String? expr, bool defaultValue) { + if (expr == null || expr.isEmpty) return defaultValue; + return DependsOnEvaluator.evaluate(expr, _formData); } - bool _isFieldRequired(DocField field) { - if (field.reqd) return true; - if (field.mandatoryDependsOn == null || field.mandatoryDependsOn!.isEmpty) { - return false; - } - return DependsOnEvaluator.evaluate(field.mandatoryDependsOn, _formData); - } + bool _shouldShowField(DocField field) => + _evaluateDepends(field.dependsOn, true); - bool _isFieldReadOnly(DocField field) { - if (field.readOnly) return true; - if (field.readOnlyDependsOn == null || field.readOnlyDependsOn!.isEmpty) { - return false; - } - return DependsOnEvaluator.evaluate(field.readOnlyDependsOn, _formData); - } + bool _isFieldRequired(DocField field) => + field.reqd || _evaluateDepends(field.mandatoryDependsOn, false); + + bool _isFieldReadOnly(DocField field) => + field.readOnly || _evaluateDepends(field.readOnlyDependsOn, false); /// Handles fetch_from: when a Link field changes, fetch the linked document /// and patch target fields (format: "link_field_name.source_field_name"). @@ -758,6 +744,7 @@ class _FrappeFormBuilderState extends State idx: field.idx, inListView: field.inListView, allowMultiple: field.allowMultiple, + searchIndex: field.searchIndex, ); final initialValue = @@ -1255,49 +1242,16 @@ class _FrappeFormBuilderState extends State // Save all form fields first to ensure FormBuilder captures all values state.save(); - // Get all form values from FormBuilder (includes all fields) - final formValues = Map.from(state.value); - - // Merge with _formData (fields that were changed via onChanged) - formValues.addAll(_formData); - - // Build complete form data with ALL fields from metadata - // This ensures we save complete data, not just changed fields - final completeFormData = {}; - - // First, initialize all fields from metadata with their default/initial values - // Skip non-data fields (Button, HTML, Image, etc.) - they hold no form value - for (final field in widget.meta.fields) { - if (field.fieldname != null && !field.hidden && field.isDataField) { - // Priority: formValues > initialData > defaultValue > empty value - completeFormData[field.fieldname!] = - formValues[field.fieldname] ?? - widget.initialData?[field.fieldname] ?? - field.defaultValue ?? - (field.fieldtype == 'Check' - ? 0 - : (field.fieldtype == 'Table' || - field.fieldtype == 'Table MultiSelect') - ? [] - : ''); - } - } - - // Then override with any form values (user input takes precedence) - // But skip null values for Table fields — ChildTableField is not a - // FormBuilderField, so state.value returns null for Table fields even - // when _formData has the actual child row data. - for (final entry in formValues.entries) { - if (entry.value != null) { - completeFormData[entry.key] = entry.value; - } - } - - widget.onSubmit?.call(completeFormData); + widget.onSubmit?.call(_buildCompleteFormData()); } - /// Builds current form data (same structure as submit). Used for dirty detection. - Map _getCurrentFormData() { + /// Assembles the full form data map: every non-hidden data field with + /// its current value (user input takes precedence), filling in + /// `initialData` / `defaultValue` / per-fieldtype empties for fields + /// the user hasn't touched. Shared by [_handleSubmit] (post-validate) + /// and [_getCurrentFormData] (dirty detection) so the default-value + /// fallback logic stays consistent between submit and dirty-check. + Map _buildCompleteFormData() { final state = _formKey.currentState; final formValues = state != null ? Map.from(state.value) @@ -1326,6 +1280,9 @@ class _FrappeFormBuilderState extends State return complete; } + /// Builds current form data (same structure as submit). Used for dirty detection. + Map _getCurrentFormData() => _buildCompleteFormData(); + void _emitFormDataChanged() { widget.onFormDataChanged?.call(_getCurrentFormData()); } diff --git a/lib/src/ui/widgets/screen_helpers.dart b/lib/src/ui/widgets/screen_helpers.dart new file mode 100644 index 0000000..7ae3d93 --- /dev/null +++ b/lib/src/ui/widgets/screen_helpers.dart @@ -0,0 +1,254 @@ +import 'package:flutter/material.dart'; + +/// Severity buckets for [showStatusSnackBar]. Maps to background colors +/// used historically across the SDK's screens — success (green), error +/// (red), warning (orange), info (blue/default). Centralised so a future +/// theme switch (e.g. routing through `Theme.of(context).colorScheme`) +/// updates every snackbar at once. +enum SnackBarSeverity { success, error, warning, info } + +/// Single canonical SnackBar emit used by every SDK screen / dialog +/// (mobile_home_screen, form_screen, sync_status_screen, child_table +/// error path, etc.). Replaces 17+ hand-written +/// `showSnackBar(SnackBar(content: Text(...), backgroundColor: ...))` +/// call sites. Pass [severity] to pick a color from the SDK palette, or +/// [backgroundColor] to override with a specific value (used by sites +/// that historically used non-Material colors like deep-orange `#E65100` +/// — passing the literal preserves the original visual exactly). +/// +/// Every snackbar emitted through this helper carries a `DISMISS` action +/// so the user can clear it without waiting for the duration to elapse +/// (Flutter's default swipe-down dismissal isn't discoverable). Pass +/// [showDismissAction] = false to suppress, e.g. for purely transient +/// notifications where the duration is short and a button is noise. +void showStatusSnackBar( + BuildContext context, + String message, { + SnackBarSeverity severity = SnackBarSeverity.info, + Color? backgroundColor, + Duration? duration, + bool showDismissAction = true, +}) { + Color? bg = backgroundColor; + if (bg == null) { + switch (severity) { + case SnackBarSeverity.success: + bg = Colors.green; + case SnackBarSeverity.error: + bg = Colors.red; + case SnackBarSeverity.warning: + bg = Colors.orange; + case SnackBarSeverity.info: + bg = null; + } + } + // Tapping a SnackBarAction auto-dismisses the snackbar before the + // onPressed callback runs (Flutter framework contract), so an empty + // onPressed is the canonical "dismiss-only" wiring. `textColor: white` + // ensures the label is readable on colored backgrounds; with neutral + // (null bg) Flutter's default action color picks up the theme accent. + final action = showDismissAction + ? SnackBarAction( + label: 'DISMISS', + textColor: bg != null ? Colors.white : null, + onPressed: () {}, + ) + : null; + ScaffoldMessenger.of(context).showSnackBar( + SnackBar( + content: Text(message), + backgroundColor: bg, + duration: duration ?? const Duration(seconds: 4), + action: action, + ), + ); +} + +/// Confirmation dialog with Cancel + destructive-action buttons. Returns +/// true when the user picks the destructive action, false on Cancel, null +/// if dismissed via the OS back gesture. Shared by every destructive flow +/// (logout, force re-sync, delete document, force-exit-offline) so the +/// dialog scaffolding lives in one place. +Future showConfirmDialog( + BuildContext context, { + required String title, + required String content, + String confirmLabel = 'Confirm', + String cancelLabel = 'Cancel', + Color? confirmColor, +}) { + return showDialog( + context: context, + builder: (ctx) => AlertDialog( + title: Text(title), + content: Text(content), + actions: [ + TextButton( + onPressed: () => Navigator.pop(ctx, false), + child: Text(cancelLabel), + ), + TextButton( + onPressed: () => Navigator.pop(ctx, true), + child: Text( + confirmLabel, + style: confirmColor != null ? TextStyle(color: confirmColor) : null, + ), + ), + ], + ), + ); +} + +/// AppBar action that renders a refresh icon when idle and a centered +/// spinner when busy. Shared by `document_list_screen.dart` and +/// `sync_status_screen.dart`. +Widget refreshOrSpinnerAction({ + required bool isBusy, + required VoidCallback onRefresh, + String tooltip = 'Refresh', +}) { + if (isBusy) { + return const Padding( + padding: EdgeInsets.all(16.0), + child: SizedBox( + width: 20, + height: 20, + child: CircularProgressIndicator(strokeWidth: 2), + ), + ); + } + return IconButton( + icon: const Icon(Icons.refresh), + onPressed: onRefresh, + tooltip: tooltip, + ); +} + +/// Standard empty-state widget used by every SDK list screen: centered +/// column with an icon, a title, an optional subtitle, and an optional +/// trailing action. Shared by `mobile_home_screen`, `document_list_screen`, +/// `sync_status_screen`, `sync_errors_screen`. +class EmptyStateWidget extends StatelessWidget { + final IconData icon; + final String title; + final String? subtitle; + final Widget? action; + final Color? iconColor; + + /// Optional style for the title. Defaults to `theme.textTheme.titleMedium`. + final TextStyle? titleStyle; + + /// Optional style for the subtitle. Defaults to + /// `theme.textTheme.bodyMedium?.copyWith(color: Colors.grey[600])`. + /// Override at the call site when the historical inline style differed + /// (e.g. `bodySmall` for compact lists, `onSurfaceVariant` for theme + /// adaptiveness in dark mode). + final TextStyle? subtitleStyle; + + const EmptyStateWidget({ + super.key, + required this.icon, + required this.title, + this.subtitle, + this.action, + this.iconColor, + this.titleStyle, + this.subtitleStyle, + }); + + @override + Widget build(BuildContext context) { + final theme = Theme.of(context); + return Center( + child: Padding( + padding: const EdgeInsets.all(24), + child: Column( + mainAxisAlignment: MainAxisAlignment.center, + mainAxisSize: MainAxisSize.min, + children: [ + Icon(icon, size: 64, color: iconColor ?? Colors.grey), + const SizedBox(height: 16), + Text(title, style: titleStyle ?? theme.textTheme.titleMedium), + if (subtitle != null) ...[ + const SizedBox(height: 8), + Text( + subtitle!, + textAlign: TextAlign.center, + style: + subtitleStyle ?? + theme.textTheme.bodyMedium?.copyWith( + color: Colors.grey[600], + ), + ), + ], + if (action != null) ...[const SizedBox(height: 16), action!], + ], + ), + ), + ); + } +} + +/// Modal loading overlay. Pushes a non-dismissible dialog showing a +/// centered spinner; returns a `VoidCallback` the caller invokes to +/// dismiss. Replaces the inline `showDialog(barrierDismissible: false, +/// ProgressIndicator)` pattern used at `mobile_home_screen.dart:517` +/// and `:573` for logout / force-resync flows. +VoidCallback showLoadingDialog(BuildContext context) { + bool dismissed = false; + showDialog( + context: context, + barrierDismissible: false, + builder: (_) => const Center(child: CircularProgressIndicator()), + ); + return () { + if (dismissed) return; + dismissed = true; + // Match the historical inline call (`Navigator.of(context).pop()`) so + // existing nested-navigator semantics are preserved exactly. + Navigator.of(context).pop(); + }; +} + +/// Inline error banner — red Container + icon + message — used by +/// `login_screen.dart` and `form_screen.dart`. Login adds rounded corners +/// + border; form-screen does not. Pass `bordered: true` for the login +/// style, false (default) for the form-screen style. +class ErrorMessageBanner extends StatelessWidget { + final String message; + final bool bordered; + final Color? backgroundColor; + + const ErrorMessageBanner({ + super.key, + required this.message, + this.bordered = false, + this.backgroundColor, + }); + + @override + Widget build(BuildContext context) { + final bg = backgroundColor ?? Colors.red[50]; + return Container( + width: bordered ? null : double.infinity, + padding: const EdgeInsets.all(12), + decoration: bordered + ? BoxDecoration( + color: bg, + borderRadius: BorderRadius.circular(8), + border: Border.all(color: Colors.red), + ) + : null, + color: bordered ? null : bg, + child: Row( + children: [ + const Icon(Icons.error, color: Colors.red), + const SizedBox(width: 8), + Expanded( + child: Text(message, style: const TextStyle(color: Colors.red)), + ), + ], + ), + ); + } +} diff --git a/lib/src/utils/api_tracer.dart b/lib/src/utils/api_tracer.dart index f937dee..181e231 100644 --- a/lib/src/utils/api_tracer.dart +++ b/lib/src/utils/api_tracer.dart @@ -8,6 +8,18 @@ import 'package:flutter/foundation.dart'; class ApiTracer { static const String _tag = '[Frappe API]'; + /// Caps a body / Map / List string at 500 chars for log readability. + /// Non-collection bodies pass through as `toString()`. Shared by + /// [traceRequest] and [traceResponse] so a threshold change applies to + /// both at once. + static String _preview(dynamic body) { + if (body is Map || body is List) { + final s = body.toString(); + return s.length > 500 ? '${s.substring(0, 500)}...' : s; + } + return body.toString(); + } + /// Log request (method, url, body). No-op in release. static void traceRequest({ required String method, @@ -22,12 +34,7 @@ class ApiTracer { buffer.writeln(' query: $queryParams'); } if (body != null) { - final preview = body is Map || body is List - ? body.toString().length > 500 - ? '${body.toString().substring(0, 500)}...' - : body.toString() - : body.toString(); - buffer.writeln(' body: $preview'); + buffer.writeln(' body: ${_preview(body)}'); } debugPrint(buffer.toString()); } @@ -45,12 +52,7 @@ class ApiTracer { if (error != null) { buffer.writeln(' error: $error'); } else if (body != null) { - final preview = body is Map || body is List - ? body.toString().length > 500 - ? '${body.toString().substring(0, 500)}...' - : body.toString() - : body.toString(); - buffer.writeln(' body: $preview'); + buffer.writeln(' body: ${_preview(body)}'); } debugPrint(buffer.toString()); } diff --git a/lib/src/utils/date_helpers.dart b/lib/src/utils/date_helpers.dart new file mode 100644 index 0000000..5a51a86 --- /dev/null +++ b/lib/src/utils/date_helpers.dart @@ -0,0 +1,62 @@ +import 'package:flutter/material.dart' show TimeOfDay; + +/// Coerces a dynamic value into a [DateTime] or null. +/// - `null` → null +/// - `DateTime` → returned as-is +/// - `String` → parsed via [DateTime.tryParse]; null on parse failure +/// - any other type → null +/// +/// Shared by the date and datetime field widgets and by the form builder's +/// patched-value normalizer so the coercion logic lives in one place. +DateTime? parseDateTime(dynamic value) { + if (value == null) return null; + if (value is DateTime) return value; + if (value is String) return DateTime.tryParse(value); + return null; +} + +/// Coerces a dynamic value into a "time of day" expressed as a [DateTime] +/// on the current calendar day. Accepts the same shapes as [parseDateTime] +/// plus `TimeOfDay` (returns today @ hh:mm) and a manual `HH:MM[:SS]` +/// string split that [DateTime.tryParse] would reject (because Frappe +/// persists Time fields as bare `HH:MM:SS` without a date prefix). +DateTime? parseTime(dynamic value) { + if (value == null) return null; + if (value is DateTime) return value; + if (value is TimeOfDay) { + final now = DateTime.now(); + return DateTime(now.year, now.month, now.day, value.hour, value.minute); + } + if (value is String) { + final parsed = DateTime.tryParse(value); + if (parsed != null) return parsed; + // Fall back to manual `HH:MM[:SS]` split. + final parts = value.split(':'); + if (parts.length >= 2) { + final h = int.tryParse(parts[0]); + final m = int.tryParse(parts[1]); + final s = parts.length > 2 ? int.tryParse(parts[2]) ?? 0 : 0; + if (h != null && m != null) { + final now = DateTime.now(); + return DateTime(now.year, now.month, now.day, h, m, s); + } + } + } + return null; +} + +/// Formats a duration expressed in seconds as `HH:MM:SS` (when hours > 0) +/// or `MM:SS`. Each component is zero-padded to two digits. Shared by the +/// Duration field widget and the form builder's patched-value formatter. +String formatDurationSeconds(int seconds) { + final hours = seconds ~/ 3600; + final minutes = (seconds % 3600) ~/ 60; + final secs = seconds % 60; + if (hours > 0) { + return '${hours.toString().padLeft(2, '0')}:' + '${minutes.toString().padLeft(2, '0')}:' + '${secs.toString().padLeft(2, '0')}'; + } + return '${minutes.toString().padLeft(2, '0')}:' + '${secs.toString().padLeft(2, '0')}'; +} diff --git a/lib/src/utils/frappe_json_utils.dart b/lib/src/utils/frappe_json_utils.dart new file mode 100644 index 0000000..af9658c --- /dev/null +++ b/lib/src/utils/frappe_json_utils.dart @@ -0,0 +1,16 @@ +/// Frappe's REST API returns boolean fields as `int` (0/1), `bool`, or +/// `String` ('1'/'true'/'0') depending on the endpoint and DocType +/// definition. [parseBool] is the canonical coercion used by every model +/// `fromJson` factory that needs to interpret a Frappe-shaped boolean. +/// +/// Returns [defaultValue] when [value] is null or any non-recognised type. +bool parseBool(dynamic value, {bool defaultValue = false}) { + if (value == null) return defaultValue; + if (value is bool) return value; + if (value is int) return value == 1; + if (value is String) { + final lower = value.toLowerCase(); + return value == '1' || lower == 'true'; + } + return defaultValue; +} diff --git a/lib/src/utils/sql_row_utils.dart b/lib/src/utils/sql_row_utils.dart new file mode 100644 index 0000000..3e4ef26 --- /dev/null +++ b/lib/src/utils/sql_row_utils.dart @@ -0,0 +1,43 @@ +/// Small helpers shared by every `fromMap(Map row)` factory +/// on a model that mirrors a SQLite table. Extracted because the retry / +/// last-attempt / error_message block in `OutboxRow` and `PendingAttachment` +/// was byte-for-byte identical; consolidating here means a change to how +/// retry timestamps are stored (e.g. moving to TEXT) is made in one place +/// across every outbox-shaped model. +library; + +/// Reads `retry_count` from a SQLite row, defaulting to 0 when absent or +/// null. Matches the historical inline `(row['retry_count'] as int?) ?? 0`. +int retryCountFrom(Map row) => + (row['retry_count'] as int?) ?? 0; + +/// Reads `last_attempt_at` from a SQLite row as a UTC [DateTime], returning +/// null when the column is null. The column is persisted as +/// `millisecondsSinceEpoch` so it reconstructs with `isUtc: true`. +DateTime? lastAttemptAtFrom(Map row) { + final raw = row['last_attempt_at']; + if (raw == null) return null; + return DateTime.fromMillisecondsSinceEpoch(raw as int, isUtc: true); +} + +/// Reads a required UTC [DateTime] millisecond-epoch column from a SQLite +/// row. Used for columns like `created_at` that always exist by schema +/// invariant. Throws if the column is null — that should be impossible +/// for required-by-schema columns. +DateTime utcMillisFrom(Map row, String key) => + DateTime.fromMillisecondsSinceEpoch(row[key] as int, isUtc: true); + +/// Resolves an enum value from its `.name` string. Returns null for a +/// null input (preserving callers that treat `parse(null) == null` as +/// "no value supplied"); returns [fallback] for a non-null input that +/// matches no enum value. Used by the `Helpers.parse(...)` +/// extensions that previously each rolled their own loop. Type [T] must +/// be the enum type; pass [values] explicitly so the helper stays +/// callable without reflection. +T? parseEnumByName(List values, String? raw, {T? fallback}) { + if (raw == null) return null; + for (final v in values) { + if (v.name == raw) return v; + } + return fallback; +} From 2705b56ff6dbe07d64686eea0fbf26d049a485fe Mon Sep 17 00:00:00 2001 From: omprakash Date: Fri, 15 May 2026 16:13:20 +0530 Subject: [PATCH 2/9] fix(sync): initial-sync offset pagination, fast-fail boot probes, and UX polish MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - DoctypeService.list: guard non-List Frappe message shape → treat as empty page - RestHelper.get/getPublic: expose per-call timeout and maxRetries overrides so boot/splash callers can fast-fail instead of burning the default 30s × 3-retry budget - PermissionService.syncFromApi: accept timeout param; remove silent catch so callers own error handling - MobileHomeScreen: suppress full-screen spinner on background syncComplete$ reloads — only show it on first paint when the list is empty - FieldStyle: default showLabel/showDescription to false - Tests: Cursor.start round-trip, initial-sync limit_start advance vs incremental modified-filter path, stall guard cursor pre-seeding --- lib/src/api/doctype_service.dart | 10 +- lib/src/api/rest_helper.dart | 41 +++++-- lib/src/api/utils.dart | 2 - lib/src/screens/mobile_home_screen.dart | 10 +- lib/src/services/permission_service.dart | 30 ++--- lib/src/ui/widgets/fields/base_field.dart | 4 +- test/link_field_test.dart | 1 - test/sync/cursor_test.dart | 39 +++++++ test/sync/pull_engine_test.dart | 15 ++- test/sync/pull_page_fetcher_test.dart | 131 +++++++++++++++++----- 10 files changed, 223 insertions(+), 60 deletions(-) diff --git a/lib/src/api/doctype_service.dart b/lib/src/api/doctype_service.dart index f0582e8..8e5ec3a 100644 --- a/lib/src/api/doctype_service.dart +++ b/lib/src/api/doctype_service.dart @@ -85,7 +85,15 @@ class DoctypeService { ); if (response is Map && response.containsKey('message')) { - return response['message'] as List; + final msg = response['message']; + if (msg is List) return msg; + // Frappe returned a non-List message (null, error string, etc.). + // Treat as empty page — callers see no records and pull continues. + debugPrint( + 'DoctypeService.list: unexpected message shape for $doctype ' + '(${msg?.runtimeType ?? "null"}) — treating as empty page', + ); + return []; } return []; } diff --git a/lib/src/api/rest_helper.dart b/lib/src/api/rest_helper.dart index 8eec317..9e5fc7e 100644 --- a/lib/src/api/rest_helper.dart +++ b/lib/src/api/rest_helper.dart @@ -93,23 +93,41 @@ class RestHelper { Map.from(_getHeaders()); /// Performs a GET request. + /// + /// [timeout] overrides [requestTimeout] for this single call — use a + /// short value (e.g. 10s) for splash-blocking calls where the default + /// 30s × 3-retry budget (~93s worst case) would freeze the UI. + /// [maxRetries] overrides the default GET retry budget (2 retries on + /// transient network errors). Pass `0` for fast-fail boot probes. Future get( String endpoint, { Map? queryParams, + Duration? timeout, + int? maxRetries, }) async { - return _request('GET', endpoint, queryParams: queryParams); + return _request( + 'GET', + endpoint, + queryParams: queryParams, + timeout: timeout, + maxRetries: maxRetries, + ); } /// Performs a GET request without auth headers. Future getPublic( String endpoint, { Map? queryParams, + Duration? timeout, + int? maxRetries, }) async { return _request( 'GET', endpoint, queryParams: queryParams, includeAuth: false, + timeout: timeout, + maxRetries: maxRetries, ); } @@ -139,6 +157,8 @@ class RestHelper { Map? queryParams, dynamic body, bool includeAuth = true, + Duration? timeout, + int? maxRetries, }) async { var uri = Uri.parse('$baseUrl$endpoint'); if (queryParams != null) { @@ -154,8 +174,13 @@ class RestHelper { body: method != 'GET' ? body : null, ); + final effectiveTimeout = timeout ?? requestTimeout; + // Default budget: 1 initial attempt + 2 retries = 3 total attempts. + // Pass `maxRetries: 0` from boot/probe call sites to fail fast. + final effectiveMaxAttempts = (maxRetries ?? 2) + 1; + int attempts = 0; - while (attempts < 3) { + while (attempts < effectiveMaxAttempts) { try { final headers = _getHeaders(includeAuth: includeAuth); if (method != 'GET' && body != null) { @@ -167,7 +192,7 @@ class RestHelper { case 'GET': response = await _client .get(uri, headers: headers) - .timeout(requestTimeout); + .timeout(effectiveTimeout); break; case 'POST': response = await _client @@ -176,7 +201,7 @@ class RestHelper { headers: headers, body: body != null ? jsonEncode(body) : null, ) - .timeout(requestTimeout); + .timeout(effectiveTimeout); break; case 'PUT': response = await _client @@ -185,12 +210,12 @@ class RestHelper { headers: headers, body: body != null ? jsonEncode(body) : null, ) - .timeout(requestTimeout); + .timeout(effectiveTimeout); break; case 'DELETE': response = await _client .delete(uri, headers: headers) - .timeout(requestTimeout); + .timeout(effectiveTimeout); break; default: throw ApiException('Unsupported HTTP method: $method'); @@ -216,7 +241,7 @@ class RestHelper { } rethrow; } on SocketException catch (e) { - if (method == 'GET' && attempts < 2) { + if (method == 'GET' && attempts < effectiveMaxAttempts - 1) { attempts++; await Future.delayed(Duration(milliseconds: 500 * (1 << attempts))); continue; @@ -229,7 +254,7 @@ class RestHelper { } throw NetworkException('No internet connection'); } on TimeoutException { - if (method == 'GET' && attempts < 2) { + if (method == 'GET' && attempts < effectiveMaxAttempts - 1) { attempts++; await Future.delayed(Duration(milliseconds: 500 * (1 << attempts))); continue; diff --git a/lib/src/api/utils.dart b/lib/src/api/utils.dart index 3bb760e..600dc95 100644 --- a/lib/src/api/utils.dart +++ b/lib/src/api/utils.dart @@ -55,7 +55,6 @@ String extractErrorMessage(dynamic body) { raw = serverMsg; } } catch (e, st) { - // ignore: avoid_print debugPrint( 'extractErrorMessage: _server_messages parse failed — $e\n$st', ); @@ -106,7 +105,6 @@ String? _extractServerMessage(dynamic body) { } } } catch (e, st) { - // ignore: avoid_print debugPrint('_extractServerMessage: jsonDecode failed — $e\n$st'); } } diff --git a/lib/src/screens/mobile_home_screen.dart b/lib/src/screens/mobile_home_screen.dart index 4ea086b..7a1352d 100644 --- a/lib/src/screens/mobile_home_screen.dart +++ b/lib/src/screens/mobile_home_screen.dart @@ -114,7 +114,15 @@ class _MobileHomeScreenState extends State int get _totalErrors => _errorCounts.values.fold(0, (a, b) => a + b); Future _load() async { - setState(() => _loading = true); + // Only flash the full-screen spinner when there's nothing on screen yet + // (first paint, or after a wipe). Background reloads triggered by + // `syncComplete$` keep the existing list visible while counts refresh — + // otherwise every completed sync wipes the screen to a spinner before + // re-rendering, which reads as a jarring "whole screen refresh once". + final isFirstPaint = _groups.isEmpty; + if (isFirstPaint) { + setState(() => _loading = true); + } try { // Fire connectivity check without blocking — update UI when it resolves widget.sdk.sync diff --git a/lib/src/services/permission_service.dart b/lib/src/services/permission_service.dart index c4bbea0..6632037 100644 --- a/lib/src/services/permission_service.dart +++ b/lib/src/services/permission_service.dart @@ -1,5 +1,3 @@ -import 'package:flutter/foundation.dart'; - import '../api/client.dart'; import '../database/app_database.dart'; import '../database/entities/doctype_permission_entity.dart'; @@ -39,19 +37,21 @@ class PermissionService { /// Call mobile_auth.permissions API and refresh local cache. /// Accepts permissions as list or map (same as [saveFromLoginResponse]). - Future?> syncFromApi() async { - try { - final result = await _client.rest.get( - '/api/v2/method/mobile_auth.permissions', - ); - if (result is! Map) return null; - final data = result['data'] as Map? ?? result; - await saveFromLoginResponse(data['permissions']); - return data; - } catch (e, st) { - debugPrint('PermissionService.syncFromApi failed — $e\n$st'); - return null; - } + /// + /// [timeout], when supplied, fast-fails the call instead of waiting on + /// the default 30s × 3-retry budget — pass a short value (e.g. 10s) + /// when this is gating a splash/boot path so a wrong-server or down + /// server surfaces a [NetworkException] quickly. + Future?> syncFromApi({Duration? timeout}) async { + final result = await _client.rest.get( + '/api/v2/method/mobile_auth.permissions', + timeout: timeout, + maxRetries: timeout != null ? 0 : null, + ); + if (result is! Map) return null; + final data = result['data'] as Map? ?? result; + await saveFromLoginResponse(data['permissions']); + return data; } Future _savePermissionMap(Map map) async { diff --git a/lib/src/ui/widgets/fields/base_field.dart b/lib/src/ui/widgets/fields/base_field.dart index 30292b8..94d999f 100644 --- a/lib/src/ui/widgets/fields/base_field.dart +++ b/lib/src/ui/widgets/fields/base_field.dart @@ -21,8 +21,8 @@ class FieldStyle { this.descriptionStyle, this.decoration, this.translate, - this.showLabel = true, - this.showDescription = true, + this.showLabel = false, + this.showDescription = false, }); } diff --git a/test/link_field_test.dart b/test/link_field_test.dart index db110df..c55a901 100644 --- a/test/link_field_test.dart +++ b/test/link_field_test.dart @@ -27,7 +27,6 @@ class _FakeLinkOptionService extends LinkOptionService { @override Future> getLinkOptions( String doctype, { - bool forceRefresh = false, List>? filters, }) => _completer.future; diff --git a/test/sync/cursor_test.dart b/test/sync/cursor_test.dart index 3c167b5..8cbb0d6 100644 --- a/test/sync/cursor_test.dart +++ b/test/sync/cursor_test.dart @@ -14,6 +14,38 @@ void main() { final back = Cursor.fromJson(jsonDecode(json) as Map?); expect(back.modified, '2026-01-01 00:00:00'); expect(back.name, 'SO-1'); + expect(back.start, 0); + }); + + test('start field round-trips through JSON', () { + const c = Cursor(modified: '2026-01-01', name: 'X', start: 300); + final json = c.toJson()!; + expect(json['start'], 300); + final back = Cursor.fromJson(json); + expect(back.start, 300); + }); + + test('start=0 is omitted from toJson to keep JSON clean', () { + const c = Cursor(modified: '2026-01-01', name: 'X', start: 0); + final json = c.toJson()!; + expect(json.containsKey('start'), isFalse); + }); + + test('fromJson missing "start" defaults to 0', () { + final c = Cursor.fromJson({'modified': '2026-01-01', 'name': 'X'}); + expect(c.start, 0); + }); + + test('markComplete resets start to 0', () { + const c = Cursor( + modified: '2026-01-01', + name: 'X', + complete: false, + start: 500, + ); + final done = c.markComplete(); + expect(done.complete, isTrue); + expect(done.start, 0); }); test('fromJson(null) → empty', () { @@ -56,8 +88,15 @@ void main() { const a = Cursor(modified: '2026-01-01', name: 'SO-1', complete: true); const b = Cursor(modified: '2026-01-01', name: 'SO-1', complete: true); const c = Cursor(modified: '2026-01-01', name: 'SO-2', complete: true); + const d = Cursor( + modified: '2026-01-01', + name: 'SO-1', + complete: true, + start: 100, + ); expect(a, equals(b)); expect(a, isNot(equals(c))); + expect(a, isNot(equals(d)), reason: 'differing start breaks equality'); expect(Cursor.empty, equals(const Cursor())); }); diff --git a/test/sync/pull_engine_test.dart b/test/sync/pull_engine_test.dart index 370d273..8f6ff7f 100644 --- a/test/sync/pull_engine_test.dart +++ b/test/sync/pull_engine_test.dart @@ -521,11 +521,11 @@ void main() { test( 'resumes from pre-existing cursor (_decodeJsonOrNull with non-empty string)', () async { - // Pre-seed a cursor so getLastOkCursor returns a non-empty JSON string → - // _decodeJsonOrNull parses it and Cursor.fromJson uses the values. + // Pre-seed a complete cursor (complete=true → incremental mode) so the + // engine resumes with a modified >= filter rather than a fresh offset pull. await metaDao.setLastOkCursor( 'Customer', - '{"modified":"2026-01-01 00:00:00","name":"C-0","complete":false}', + '{"modified":"2026-01-01 00:00:00","name":"C-0","complete":true}', ); var calls = 0; final fetcher = PullPageFetcher( @@ -632,9 +632,14 @@ void main() { test( 'stall guard: terminates and persists cursor when all page rows share same modified', () async { - // Simulates a doctype where every row has the same `modified` timestamp - // (e.g. bulk-imported reference data). Without the stall guard, + // Simulates INCREMENTAL sync where every row has the same `modified` + // timestamp (e.g. bulk-imported reference data). Without the stall guard, // `modified >= cursor.modified` returns the same page forever. + // The stall guard only fires for complete=true cursors; seed one first. + await metaDao.setLastOkCursor( + 'Customer', + '{"modified":"2025-12-31","name":"C-0","complete":true}', + ); var calls = 0; final fetcher = PullPageFetcher( listHttp: (doctype, params) async { diff --git a/test/sync/pull_page_fetcher_test.dart b/test/sync/pull_page_fetcher_test.dart index 1340857..038b1c0 100644 --- a/test/sync/pull_page_fetcher_test.dart +++ b/test/sync/pull_page_fetcher_test.dart @@ -10,7 +10,7 @@ class _Captured { } void main() { - test('first page (null cursor): no cursor predicate', () async { + test('first page (null cursor): no filter, limit_start=0', () async { final cap = _Captured(); final fetcher = PullPageFetcher( listHttp: (doctype, params) async { @@ -38,6 +38,7 @@ void main() { expect(orFilters == null || orFilters.isEmpty, isTrue); expect(cap.params!['order_by'], 'modified asc, name asc'); expect(cap.params!['limit_page_length'], 500); + expect(cap.params!['limit_start'], 0); expect( (cap.params!['fields'] as List), containsAll(['name', 'modified', 'customer_name']), @@ -45,7 +46,51 @@ void main() { }); test( - 'non-empty cursor → filters has modified >= cursor.modified (and ONLY that)', + 'initial sync: limit_start advances by pageSize, no modified filter', + () async { + final capturedParams = >[]; + var call = 0; + final fetcher = PullPageFetcher( + listHttp: (doctype, params) async { + capturedParams.add(Map.of(params)); + call++; + if (call == 1) { + return List.generate( + 3, + (i) => {'name': 'X-${i + 1}', 'modified': '2026-01-0${i + 1}'}, + ); + } + return const []; + }, + ); + final meta = DocTypeMeta(name: 'X', fields: const []); + + // Page 1 — cursor.start = 0 + final r1 = await fetcher.fetch( + doctype: 'X', + meta: meta, + cursor: Cursor.empty, + pageSize: 3, + ); + expect(capturedParams[0]['limit_start'], 0); + expect(capturedParams[0]['filters'], isNull); + expect(r1.advancedCursor.complete, isFalse); + expect(r1.advancedCursor.start, 3); + + // Page 2 — cursor.start = 3 + await fetcher.fetch( + doctype: 'X', + meta: meta, + cursor: r1.advancedCursor, + pageSize: 3, + ); + expect(capturedParams[1]['limit_start'], 3); + expect(capturedParams[1]['filters'], isNull); + }, + ); + + test( + 'incremental cursor (complete=true): modified filter, limit_start=0', () async { final cap = _Captured(); final fetcher = PullPageFetcher( @@ -58,10 +103,15 @@ void main() { await fetcher.fetch( doctype: 'X', meta: meta, - cursor: const Cursor(modified: '2026-01-01 00:00:00', name: 'A'), + cursor: const Cursor( + modified: '2026-01-01 00:00:00', + name: 'A', + complete: true, + ), pageSize: 500, ); + expect(cap.params!['limit_start'], 0); final filters = cap.params!['filters'] as List?; expect(filters, isNotNull); expect( @@ -79,39 +129,69 @@ void main() { 'by PullApply UPSERT idempotency', ); - // The earlier buggy `or_filters: [['modified', '>', X]]` shape must - // NOT be present — its presence + a separate `name > Y` filter is - // exactly what caused row exclusion. final orf = cap.params!['or_filters'] as List?; expect(orf == null || orf.isEmpty, isTrue); }, ); - test('returned rows + advancedCursor derived from last row', () async { - final fetcher = PullPageFetcher( - listHttp: (doctype, params) async => [ - {'name': 'X-1', 'modified': '2026-01-01 00:00:00'}, - {'name': 'X-2', 'modified': '2026-01-02 00:00:00'}, - ], - ); - final meta = DocTypeMeta(name: 'X', fields: const []); - final result = await fetcher.fetch( - doctype: 'X', - meta: meta, - cursor: Cursor.empty, - pageSize: 500, - ); - expect(result.rows.length, 2); - expect(result.advancedCursor.name, 'X-2'); - expect(result.advancedCursor.modified, '2026-01-02 00:00:00'); - }); + test( + 'initial sync: advancedCursor tracks modified/name + advances start', + () async { + final fetcher = PullPageFetcher( + listHttp: (doctype, params) async => [ + {'name': 'X-1', 'modified': '2026-01-01 00:00:00'}, + {'name': 'X-2', 'modified': '2026-01-02 00:00:00'}, + ], + ); + final meta = DocTypeMeta(name: 'X', fields: const []); + final result = await fetcher.fetch( + doctype: 'X', + meta: meta, + cursor: Cursor.empty, + pageSize: 500, + ); + expect(result.rows.length, 2); + expect(result.advancedCursor.name, 'X-2'); + expect(result.advancedCursor.modified, '2026-01-02 00:00:00'); + expect(result.advancedCursor.complete, isFalse); + expect(result.advancedCursor.start, 2); + }, + ); + + test( + 'incremental: advancedCursor uses last row modified/name, complete=true', + () async { + final fetcher = PullPageFetcher( + listHttp: (doctype, params) async => [ + {'name': 'X-1', 'modified': '2026-01-01 00:00:00'}, + {'name': 'X-2', 'modified': '2026-01-02 00:00:00'}, + ], + ); + final meta = DocTypeMeta(name: 'X', fields: const []); + final result = await fetcher.fetch( + doctype: 'X', + meta: meta, + cursor: const Cursor( + modified: '2026-01-01 00:00:00', + name: 'A', + complete: true, + ), + pageSize: 500, + ); + expect(result.rows.length, 2); + expect(result.advancedCursor.name, 'X-2'); + expect(result.advancedCursor.modified, '2026-01-02 00:00:00'); + expect(result.advancedCursor.complete, isTrue); + expect(result.advancedCursor.start, 0); + }, + ); test('empty result → advancedCursor stays unchanged', () async { final fetcher = PullPageFetcher( listHttp: (doctype, params) async => const [], ); final meta = DocTypeMeta(name: 'X', fields: const []); - final start = const Cursor(modified: '2026-01-01', name: 'A'); + const start = Cursor(modified: '2026-01-01', name: 'A', complete: true); final result = await fetcher.fetch( doctype: 'X', meta: meta, @@ -120,6 +200,7 @@ void main() { ); expect(result.rows, isEmpty); expect(result.advancedCursor.name, 'A'); + expect(result.advancedCursor.modified, '2026-01-01'); }); test('skips child-table fieldtypes from requested fields', () async { From 811710de4dc85d84e5cfbcbbd737995638b13e97 Mon Sep 17 00:00:00 2001 From: omprakash Date: Sun, 17 May 2026 16:09:57 +0530 Subject: [PATCH 3/9] fix(sdk): close initial-sync schema gap; expose push/pull/form hooks - FrappeSDK._runUpgradeClosurePull: call ensureSchemaForClosure before the closure pull so per-doctype mirror tables exist (closes the "no such table" hole that previously required SNF's runSnfPostSdkSync to re-pull) - FrappeSDK._initialMetaAndDataSync: bracket the boot pull with isInitialSync=true so host UI subscribed to state$ can render a syncing indicator; reset on every exit path - FrappeSDK: surface boot failures via SyncStateNotifier.lastError (code: 'network'|'permissions'), expose retryInitialMetaAndDataSync(), clear lastError on logout - parent/child schema DDL: CREATE TABLE/INDEX IF NOT EXISTS so SNF's belt-and-suspenders ensureSchemaForClosure and the SDK's eager pull can re-enter safely after a partial failure - PullEngine: optional SchemaReconcilerFn wired to OfflineRepository.reconcileParentTableForMeta so ALTER ADD COLUMN runs before applying a page (closes the meta-cache race) - PullEngine stall guard now scoped to incremental (complete=true); initial-sync uses limit_start offset which always advances - PushEngine: optional PayloadTransformerFn between assembly and HTTP dispatch (e.g. host auto-submit on sync); exception-safe - FrappeFormBuilder: onValidationFailed callback so parents can stop loading indicators when submit is rejected - FrappeFormBuilder: ??=-inject linkOptionService / linkFieldCoordinator on host-supplied customFieldFactory so Link pickers aren't half-configured - SyncStateNotifier: recordLastError / clearLastError bypass copyWith (cannot express "set to null") - SDK barrel: export PayloadTransformerFn + ChildTableFormBuilder - Tests: PayloadTransformerFn smoke; onValidationFailed fires on invalid submit and not on valid --- lib/frappe_mobile_sdk.dart | 3 + lib/src/database/schema/child_schema.dart | 11 +- lib/src/database/schema/parent_schema.dart | 23 ++- lib/src/sdk/frappe_sdk.dart | 143 +++++++++++++++++- lib/src/services/offline_repository.dart | 24 +++ lib/src/services/sync_engine_builder.dart | 12 +- lib/src/sync/cursor.dart | 33 +++- lib/src/sync/pull_engine.dart | 52 ++++++- lib/src/sync/pull_page_fetcher.dart | 73 +++++---- lib/src/sync/push_engine.dart | 30 ++++ lib/src/sync/sync_state_notifier.dart | 44 ++++++ lib/src/ui/widgets/form_builder.dart | 18 +++ test/sync/push_payload_transform_test.dart | 40 +++++ .../form_builder_validation_failed_test.dart | 97 ++++++++++++ 14 files changed, 544 insertions(+), 59 deletions(-) create mode 100644 test/sync/push_payload_transform_test.dart create mode 100644 test/ui/widgets/form_builder_validation_failed_test.dart diff --git a/lib/frappe_mobile_sdk.dart b/lib/frappe_mobile_sdk.dart index 3d8deac..d4c8a9f 100644 --- a/lib/frappe_mobile_sdk.dart +++ b/lib/frappe_mobile_sdk.dart @@ -94,6 +94,8 @@ export 'src/ui/widgets/fields/button_field.dart'; export 'src/ui/widgets/fields/numeric_field.dart'; export 'src/ui/widgets/fields/link_field.dart'; export 'src/ui/widgets/fields/phone_field.dart'; +export 'src/ui/widgets/fields/child_table_field.dart' + show ChildTableFormBuilder; // Constants export 'src/constants/field_types.dart'; @@ -120,6 +122,7 @@ export 'src/services/sync_controller.dart' export 'src/sync/sync_state.dart' show SyncState, DoctypeSyncState, QueueSummary, SyncErrorSummary; export 'src/sync/sync_state_notifier.dart' show SyncStateNotifier; +export 'src/sync/push_engine.dart' show PayloadTransformerFn; export 'src/ui/widgets/sync_status_bar.dart' show SyncStatusBar; export 'src/ui/widgets/document_list_filter_chip.dart' show DocumentListFilterChip, DocumentListFilter, DocumentListFilterCounts; diff --git a/lib/src/database/schema/child_schema.dart b/lib/src/database/schema/child_schema.dart index 0d59bec..9f80c4c 100644 --- a/lib/src/database/schema/child_schema.dart +++ b/lib/src/database/schema/child_schema.dart @@ -39,12 +39,17 @@ List buildChildSchemaDDL( } } + // `IF NOT EXISTS` on every CREATE makes the DDL idempotent so a re-entry + // after a partial prior failure (or a concurrent caller racing against + // SNF's `runSnfPostSdkSync.ensureSchemaForClosure`) cannot abort the + // schema-create loop on "table/index already exists". See parent_schema.dart + // for the full rationale. final suffix = stripDocsPrefix(tableName); return [ - 'CREATE TABLE $tableName (\n ${cols.join(',\n ')}\n)', - 'CREATE UNIQUE INDEX ix_${suffix}_server_name ' + 'CREATE TABLE IF NOT EXISTS $tableName (\n ${cols.join(',\n ')}\n)', + 'CREATE UNIQUE INDEX IF NOT EXISTS ix_${suffix}_server_name ' 'ON $tableName(server_name) WHERE server_name IS NOT NULL', - 'CREATE UNIQUE INDEX ux_${suffix}_parent_slot ' + 'CREATE UNIQUE INDEX IF NOT EXISTS ux_${suffix}_parent_slot ' 'ON $tableName(parent_uuid, parentfield, idx)', ]; } diff --git a/lib/src/database/schema/parent_schema.dart b/lib/src/database/schema/parent_schema.dart index 4ee8e1d..d3a2fe2 100644 --- a/lib/src/database/schema/parent_schema.dart +++ b/lib/src/database/schema/parent_schema.dart @@ -57,15 +57,28 @@ List buildParentSchemaDDL( } } - final ddl = ['CREATE TABLE $tableName (\n ${cols.join(',\n ')}\n)']; + // `IF NOT EXISTS` on every CREATE makes the DDL idempotent so a second + // caller (e.g. SDK's schema-eager `_runUpgradeClosurePull` racing with + // SNF's `runSnfPostSdkSync.ensureSchemaForClosure`, or a re-entry after + // a partial prior failure) cannot abort the loop on "table already + // exists" / "index already exists" and leave later doctypes — especially + // child tables — uncreated. Aligns with `feedback_migration_safety_pattern`: + // idempotent CREATEs should be safe to re-run without explicit guards. + final ddl = [ + 'CREATE TABLE IF NOT EXISTS $tableName (\n ${cols.join(',\n ')}\n)', + ]; final suffix = stripDocsPrefix(tableName); ddl.add( - 'CREATE UNIQUE INDEX ix_${suffix}_server_name ' + 'CREATE UNIQUE INDEX IF NOT EXISTS ix_${suffix}_server_name ' 'ON $tableName(server_name) WHERE server_name IS NOT NULL', ); - ddl.add('CREATE INDEX ix_${suffix}_status ON $tableName(sync_status)'); - ddl.add('CREATE INDEX ix_${suffix}_modified ON $tableName(modified)'); + ddl.add( + 'CREATE INDEX IF NOT EXISTS ix_${suffix}_status ON $tableName(sync_status)', + ); + ddl.add( + 'CREATE INDEX IF NOT EXISTS ix_${suffix}_modified ON $tableName(modified)', + ); final additional = chooseIndexes( meta, @@ -74,7 +87,7 @@ List buildParentSchemaDDL( ).where((c) => c != 'server_name' && c != 'sync_status' && c != 'modified'); for (final col in additional) { ddl.add( - 'CREATE INDEX ix_${suffix}_${_sanitizeColName(col)} ' + 'CREATE INDEX IF NOT EXISTS ix_${suffix}_${_sanitizeColName(col)} ' 'ON $tableName($col)', ); } diff --git a/lib/src/sdk/frappe_sdk.dart b/lib/src/sdk/frappe_sdk.dart index 2c41356..5d820de 100644 --- a/lib/src/sdk/frappe_sdk.dart +++ b/lib/src/sdk/frappe_sdk.dart @@ -6,12 +6,14 @@ import 'dart:async'; import 'package:flutter/foundation.dart'; import '../api/client.dart'; +import '../api/exceptions.dart'; import '../concurrency/concurrency_pool.dart'; import '../concurrency/connectivity_watcher.dart'; import '../database/app_database.dart'; import '../database/daos/doctype_meta_dao.dart'; import '../database/daos/sdk_meta_dao.dart'; import '../models/closure_result.dart'; +import '../models/doc_type_meta.dart'; import '../models/offline_mode.dart'; import '../models/offline_mode_notifier.dart'; import '../models/session_user.dart'; @@ -30,6 +32,7 @@ import '../services/link_option_service.dart'; import '../services/translation_service.dart'; import '../sync/pull_engine.dart'; import '../sync/push_engine.dart'; +import '../sync/sync_state.dart'; import '../sync/sync_state_notifier.dart'; /// Main SDK initialization class for easy setup @@ -37,6 +40,12 @@ class FrappeSDK { final String baseUrl; final String? databaseAppName; + /// Optional payload transformer; passed to the push pipeline. See + /// [PayloadTransformerFn] in `push_engine.dart`. Apps use this to + /// override push payloads (e.g. SNF auto-submits submittable doctypes + /// to `docstatus=1` on sync). + final PayloadTransformerFn? payloadTransformer; + FrappeClient? _client; AppDatabase? _database; AuthService? _authService; @@ -137,7 +146,11 @@ class FrappeSDK { _modeNotifier?.value = next; } - FrappeSDK({required this.baseUrl, this.databaseAppName}); + FrappeSDK({ + required this.baseUrl, + this.databaseAppName, + this.payloadTransformer, + }); /// Test-only constructor: accepts a pre-built [AppDatabase] (e.g. in-memory). /// Wires all services directly without calling [initialize()]. @@ -155,7 +168,8 @@ class FrappeSDK { enabled: true, isPersisted: true, ), - }) : databaseAppName = null { + }) : databaseAppName = null, + payloadTransformer = null { _database = database; _modeNotifier = OfflineModeNotifier(offlineMode); _syncCompleteController = StreamController.broadcast(); @@ -300,6 +314,11 @@ class FrappeSDK { // INSERTs after a ghost-success retry instead of falling back to L3 // (extra GET-by-mobile_uuid before each retry). serverHasDedupHook: true, + // Closes the SDK meta-cache race: PullEngine reconciles the + // on-disk schema against the meta it's about to apply, just before + // each doctype's pull loop runs. + schemaReconciler: _repository!.reconcileParentTableForMeta, + payloadTransformer: payloadTransformer, ); _syncStateNotifier = pack.notifier; _pushPool = pack.pushPool; @@ -418,6 +437,15 @@ class FrappeSDK { .whenComplete(() => _pendingDrain = null); unawaited(_pendingDrain!); } + // Awaited intentionally — keeps the boot pipeline sequential + // with the host app's own post-init sync, avoiding two + // concurrent invocations of `checkAndSyncDoctypes` / + // `resyncMobileConfiguration` that would otherwise race. + // + // The splash-hang concern (wrong / unresponsive server) is + // addressed inside [_initialMetaAndDataSync] by a short-timeout + // permissions probe that bails fast and surfaces the failure + // on [syncStateNotifier.value.lastError]. await _initialMetaAndDataSync(); } } @@ -441,6 +469,20 @@ class FrappeSDK { } } + /// Re-run the boot-time metadata + data sync. Host apps wire this to a + /// "Retry" action on the UI banner they show when + /// [syncStateNotifier]'s `lastError` populates with `code == 'network'` + /// — the only sanctioned way to retry a failed initial sync without + /// going through full logout/login. + /// + /// Returns the future that completes when the boot sync settles (or + /// fails fast on the short-timeout probe). The future never throws — + /// failures are reported via [syncStateNotifier]. + Future retryInitialMetaAndDataSync() async { + if (!_initialized) return; + await _initialMetaAndDataSync(); + } + /// Resolves the session-bound offline mode from the persisted record. /// /// - Persisted value present → use it verbatim. The @@ -635,6 +677,10 @@ class FrappeSDK { isPersisted: false, ); } + // The notifier outlives session boundaries; drop a stale boot-sync + // banner so the next login doesn't inherit the previous session's + // unreachable-server state. + _syncStateNotifier?.clearLastError(); } /// Throws if [initialize] hasn't run. Called as the first line of every @@ -981,11 +1027,50 @@ class FrappeSDK { // race-free. if (!_cachedOnline) return; + // Clear any prior boot error so the host UI can drop a stale banner + // before we probe again. + _syncStateNotifier?.clearLastError(); + + // Flip `isInitialSync: true` for the whole boot/login data-pull so + // any host UI subscribed to `state$` (SyncStatusBar, custom banners) + // can render a "Syncing…" indicator while the closure pull runs. + // The flag is reset by [_setInitialSyncFlag] at every exit point + // below so it never sticks across early returns or thrown errors. + final initialSyncNotifier = _syncStateNotifier; + _setInitialSyncFlag(initialSyncNotifier, true); + try { - await _permissionService?.syncFromApi(); + // Short-timeout probe: if the configured base URL is wrong or the + // server is down, this fails in ~10s instead of ~93s. Bailing here + // also skips the long-timeout downstream calls (meta, config, + // closure pull) that would chain another ~90s wait each. + await _permissionService?.syncFromApi( + timeout: const Duration(seconds: 10), + ); + } on NetworkException catch (e, st) { + _syncStateNotifier?.recordLastError( + SyncErrorSummary( + code: 'network', + message: e.message, + at: DateTime.now(), + ), + ); + // ignore: avoid_print + print('FrappeSDK: boot sync — server unreachable: $e\n$st'); + _setInitialSyncFlag(initialSyncNotifier, false); + return; } catch (e, st) { + _syncStateNotifier?.recordLastError( + SyncErrorSummary( + code: 'permissions', + message: e.toString(), + at: DateTime.now(), + ), + ); // ignore: avoid_print print('FrappeSDK: permissions.syncFromApi failed — $e\n$st'); + // Non-network failure: continue with other steps so a partial + // outage (e.g. one method 500) doesn't block the rest of boot. } try { @@ -1010,9 +1095,25 @@ class FrappeSDK { } // Online mode stops here — closure pull is offline-only. - if (!_offlineMode.enabled) return; + if (!_offlineMode.enabled) { + _setInitialSyncFlag(initialSyncNotifier, false); + return; + } - await _runUpgradeClosurePull(); + try { + await _runUpgradeClosurePull(); + } finally { + _setInitialSyncFlag(initialSyncNotifier, false); + } + } + + /// Toggles [SyncState.isInitialSync] when [notifier] is non-null. Used + /// by [_initialMetaAndDataSync] to bracket the first-pull window so any + /// host UI subscribed to `state$` can render a "Syncing…" indicator + /// without leaking the flag across early-return paths. + void _setInitialSyncFlag(SyncStateNotifier? notifier, bool value) { + if (notifier == null) return; + notifier.value = notifier.value.copyWith(isInitialSync: value); } /// Pulls every doctype in the closure of the user's mobile-form entry @@ -1052,6 +1153,38 @@ class FrappeSDK { if (!await onlineCheck()) return const {}; final entryPoints = await _metaService!.getMobileFormDoctypeNames(); final closure = await _metaService!.closure(entryPoints); + + // Create per-doctype mirror tables BEFORE the pull writes into them. + // Without this, PullEngine.applyPageInTxn crashes with + // `no such table: docs__` for any doctype whose schema + // hasn't been created yet (the per-doctype table create path is + // owned by saveDocument / ensureSchemaForClosure — neither of which + // ran during the SDK's auto-fired initial sync before this fix). + // SNF's own runSnfPostSdkSync also calls ensureSchemaForClosure as + // belt-and-suspenders; this call is idempotent. + if (_repository != null) { + final metasByDoctype = {}; + for (final dt in closure.doctypes) { + try { + metasByDoctype[dt] = await _metaService!.getMeta(dt); + } catch (e, st) { + // ignore: avoid_print + print('FrappeSDK: closure pull — getMeta($dt) failed — $e\n$st'); + } + } + try { + await _repository!.ensureSchemaForClosure( + metas: metasByDoctype, + childDoctypes: closure.childDoctypes, + ); + } catch (e, st) { + // ignore: avoid_print + print( + 'FrappeSDK: closure pull — ensureSchemaForClosure failed — $e\n$st', + ); + } + } + final pullable = await _buildPullableDoctypes( entryPoints: entryPoints, closure: closure, diff --git a/lib/src/services/offline_repository.dart b/lib/src/services/offline_repository.dart index 36c3fee..dbae699 100644 --- a/lib/src/services/offline_repository.dart +++ b/lib/src/services/offline_repository.dart @@ -775,6 +775,30 @@ class OfflineRepository { /// (SQLite's `DROP COLUMN` story is finicky and stale columns are /// harmless extras). The diff is funneled through [MetaMigration.apply] /// so existing-row backfill of new `__norm` columns happens for free. + /// Public entrypoint for schema reconcile — called by [PullEngine] + /// right before applying a pull page so the table's columns match the + /// meta the apply step is about to use. Addresses the race where + /// SNF's `runSnfPostSdkSync.ensureSchemaForClosure` and the SDK's + /// concurrent `checkAndSyncDoctypes` read the meta cache at slightly + /// different moments — SNF builds the table with meta-T1, PullApply + /// then iterates meta-T2 (now refreshed) and crashes on + /// `no such column`. + /// + /// No-op if the table doesn't exist yet — the create path runs + /// elsewhere ([ensureSchemaForClosure] / [_ensurePerDoctypeTable]). + /// Bypasses [_ensuredTables] on purpose so re-entrant callers (e.g. + /// PullEngine running after [ensureSchemaForClosure] already added + /// the table to the set) still get their migrations applied. + Future reconcileParentTableForMeta( + String doctype, + String tableName, + DocTypeMeta meta, + ) async { + final db = _database.rawDatabase; + if (!await sqliteTableExists(db, tableName)) return; + await _reconcileParentTableSchema(doctype, tableName, meta); + } + Future _reconcileParentTableSchema( String doctype, String tableName, diff --git a/lib/src/services/sync_engine_builder.dart b/lib/src/services/sync_engine_builder.dart index 2d015b5..b66080b 100644 --- a/lib/src/services/sync_engine_builder.dart +++ b/lib/src/services/sync_engine_builder.dart @@ -20,6 +20,9 @@ import '../sync/push_engine.dart'; import '../sync/sync_state_notifier.dart'; import 'sync_controller.dart'; +// Re-export so callers building the SDK pack can name the typedef. +export '../sync/push_engine.dart' show PayloadTransformerFn; + /// Bundle of the engines + façade that `FrappeSDK` stashes after wiring. class SyncEnginePack { final SyncStateNotifier notifier; @@ -53,6 +56,8 @@ class SyncEngineBuilder { bool serverHasDedupHook = false, int? concurrencyOverride, SyncStateNotifier? sharedNotifier, + SchemaReconcilerFn? schemaReconciler, + PayloadTransformerFn? payloadTransformer, }) async { final notifier = sharedNotifier ?? SyncStateNotifier(); final tier = await DeviceTier.detect(override: concurrencyOverride); @@ -184,6 +189,7 @@ class SyncEngineBuilder { resolveServerName: resolveServerName, attachmentUploader: attachmentUploader, writeQueueResolver: writeQueueResolver, + payloadTransformer: payloadTransformer, ); // PullEngine is built but not auto-invoked. The list-http callback @@ -198,7 +204,8 @@ class SyncEngineBuilder { filters: (params['filters'] as List?)?.cast>(), fields: (params['fields'] as List?)?.cast(), orderBy: params['order_by'] as String?, - limitPageLength: params['limit_page_length'] as int? ?? 100, + limitPageLength: params['limit_page_length'] as int? ?? 500, + limitStart: params['limit_start'] as int? ?? 0, ); return result .whereType() @@ -212,10 +219,11 @@ class SyncEngineBuilder { outboxDao: outboxDao, pool: pullPool, fetcher: PullPageFetcher(listHttp: listHttp), - pageSize: 100, + pageSize: 500, notifier: notifier, metaResolver: metaResolver, writeQueueResolver: writeQueueResolver, + schemaReconciler: schemaReconciler, ); final controller = SyncController( diff --git a/lib/src/sync/cursor.dart b/lib/src/sync/cursor.dart index b612811..e16c2ec 100644 --- a/lib/src/sync/cursor.dart +++ b/lib/src/sync/cursor.dart @@ -9,12 +9,24 @@ /// INCREMENTAL transition). Mirrors the format already written by /// `SyncService._pullOneInternal` so both pull paths interoperate without /// silently dropping the field on read/write roundtrips. +/// +/// `start` is the row offset used during initial (complete=false) pulls. +/// [PullPageFetcher] sends `limit_start=start` and advances it by pageSize +/// each page, avoiding `modified >=` filtering until the full dataset is +/// fetched once. After initial drain, [markComplete] resets start to 0 and +/// incremental pulls use `modified >=` with `limit_start=0` from that point. class Cursor { final String? modified; final String? name; final bool complete; + final int start; - const Cursor({this.modified, this.name, this.complete = false}); + const Cursor({ + this.modified, + this.name, + this.complete = false, + this.start = 0, + }); static const Cursor empty = Cursor(); @@ -26,15 +38,20 @@ class Cursor { bool complete = false, }) => Cursor(modified: modified, name: name, complete: complete); - /// Returns a copy with `complete: true` — used by the pull engine when - /// a doctype drains to mark the cursor as eligible for INCREMENTAL + /// Returns a copy with `complete: true` and `start: 0` — used by the pull + /// engine when a doctype drains to mark it as eligible for INCREMENTAL /// (delta) pulls on the next call. Cursor markComplete() => - Cursor(modified: modified, name: name, complete: true); + Cursor(modified: modified, name: name, complete: true, start: 0); Map? toJson() { if (isNull) return null; - return {'modified': modified, 'name': name, 'complete': complete}; + return { + 'modified': modified, + 'name': name, + 'complete': complete, + if (start != 0) 'start': start, + }; } factory Cursor.fromJson(Map? json) { @@ -43,6 +60,7 @@ class Cursor { modified: json['modified'] as String?, name: json['name'] as String?, complete: json['complete'] == true, + start: json['start'] as int? ?? 0, ); } @@ -52,8 +70,9 @@ class Cursor { other is Cursor && modified == other.modified && name == other.name && - complete == other.complete; + complete == other.complete && + start == other.start; @override - int get hashCode => Object.hash(modified, name, complete); + int get hashCode => Object.hash(modified, name, complete, start); } diff --git a/lib/src/sync/pull_engine.dart b/lib/src/sync/pull_engine.dart index b3dd221..dee9881 100644 --- a/lib/src/sync/pull_engine.dart +++ b/lib/src/sync/pull_engine.dart @@ -9,6 +9,7 @@ import '../database/daos/doctype_meta_dao.dart'; import '../database/daos/outbox_dao.dart'; import '../models/closure_result.dart'; import '../models/dep_graph.dart'; +import '../models/doc_type_meta.dart'; import '../models/meta_resolver.dart'; import 'cursor.dart'; import 'pull_apply.dart'; @@ -19,6 +20,18 @@ import 'sync_state_notifier.dart'; /// Back-compat alias. Use [MetaResolverFn] directly in new code. typedef MetaResolver = MetaResolverFn; +/// Per-doctype schema reconcile hook. Called at the start of each +/// `_runDoctype` with the meta the engine is about to apply, so the +/// receiver can `ALTER TABLE ADD COLUMN` any fields that were added to +/// meta after the table was originally created. +/// +/// In production this is wired to +/// `OfflineRepository.reconcileParentTableForMeta`. Optional — when +/// null, no reconcile happens and writes assume schema parity (fine for +/// tests). +typedef SchemaReconcilerFn = + Future Function(String doctype, String tableName, DocTypeMeta meta); + /// Drives the pull side of the sync engine. Spec §5.1 + §5.4. /// /// For each (non-child) doctype in the closure: @@ -57,6 +70,12 @@ class PullEngine { /// when [writeQueueResolver] is non-null. final Map _writeQueues = {}; + /// Optional schema-reconcile callback invoked at the start of each + /// `_runDoctype`. See [SchemaReconcilerFn] for rationale. Failure is + /// caught and logged — pull continues with whatever columns currently + /// exist on the table. + final SchemaReconcilerFn? schemaReconciler; + PullEngine({ required this.db, required this.metaDao, @@ -67,6 +86,7 @@ class PullEngine { required this.notifier, required this.metaResolver, this.writeQueueResolver, + this.schemaReconciler, }); /// Returns the set of doctypes that were deferred (skipped because a @@ -126,6 +146,23 @@ class PullEngine { } final meta = await metaResolver(doctype); + + // Reconcile the on-disk schema against THIS meta snapshot before + // applying any pages. Closes the SNF/SDK race where the table was + // created from a slightly older meta and PullApply now wants to + // UPDATE columns that don't exist yet. See [SchemaReconcilerFn]. + final reconciler = schemaReconciler; + if (reconciler != null) { + try { + final parentTableForReconcile = await metaDao.tableNameFor(doctype); + await reconciler(doctype, parentTableForReconcile, meta); + } catch (e, st) { + debugPrint( + 'PullEngine._runDoctype($doctype): schemaReconciler failed — $e\n$st', + ); + } + } + var scratch = Cursor.fromJson( _decodeJsonOrNull(await metaDao.getLastOkCursor(doctype)), ); @@ -211,12 +248,15 @@ class PullEngine { // confirmation" case (network error on the next request) is what // protects the cursor from advancing prematurely. - // Stall guard: when `modified >= cursor.modified` returns a non-empty - // page where every row shares the same modified timestamp as the - // cursor, the advanced cursor equals the input cursor and the next - // request returns the same page — infinite loop. Break here; PullApply's - // UPSERT-by-server_name idempotency keeps the applied rows correct. - if (scratch.modified == priorModified && scratch.name == priorName) { + // Stall guard (incremental only): when `modified >= cursor.modified` + // returns a non-empty page where every row shares the same modified + // timestamp, the advanced cursor equals the input cursor and the next + // request returns the same page — infinite loop. Not applicable to + // initial sync (complete=false) because that path uses limit_start + // offset pagination, which always advances. + if (scratch.complete && + scratch.modified == priorModified && + scratch.name == priorName) { break; } } diff --git a/lib/src/sync/pull_page_fetcher.dart b/lib/src/sync/pull_page_fetcher.dart index c9b58db..997eca1 100644 --- a/lib/src/sync/pull_page_fetcher.dart +++ b/lib/src/sync/pull_page_fetcher.dart @@ -18,26 +18,26 @@ class PullPageResult { const PullPageResult({required this.rows, required this.advancedCursor}); } -/// One-page list fetch with cursor pagination. Spec §5.1. +/// One-page list fetch with dual-mode pagination. Spec §5.1. /// -/// Builds the Frappe `frappe.client.get_list` query with the cursor -/// predicate. The spec's true predicate is -/// `(modified > cursor.modified) OR (modified == cursor.modified AND -/// name > cursor.name)` — Frappe's REST `or_filters` cannot express the -/// nested AND-within-OR in a single request, so we approximate with -/// `modified >= cursor.modified` and rely on PullApply's -/// UPSERT-by-`server_name` idempotency to absorb the seam row(s). +/// **Initial sync** (`cursor.complete == false`): uses `limit_start` offset +/// pagination — no `modified` filter, `limit_start` advances by `pageSize` +/// each page. This guarantees the full dataset is fetched before the cursor +/// is committed, avoiding the seam-skip risk of applying `modified >=` while +/// new records can still land behind the advancing watermark. /// -/// **Stall hazard:** when many rows share the same `modified` second (e.g. -/// a bulk import on the server), `modified >= cursor.modified` keeps -/// returning the same page — `advancedCursor` equals the input cursor and -/// the loop never terminates. [PullEngine] owns a stall guard: it detects -/// when `advancedCursor.modified == scratch.modified && advancedCursor.name -/// == scratch.name` and breaks. Callers that drive this fetcher directly -/// must apply the same guard. Idempotent UPSERT keeps applied rows correct. -/// For the `>pageSize` same-second case, rows past the first page are -/// missed until the cursor moves forward; the spec's two-request variant -/// (one per OR branch) can close that gap as a follow-up. +/// **Incremental sync** (`cursor.complete == true`): uses the classic +/// `modified >= cursor.modified` predicate with `limit_start = 0`. Combined +/// with `order_by modified asc, name asc` this returns: +/// - the seam row(s) at `modified == cursor.modified` — idempotently +/// re-applied by PullApply's UPSERT-by-server_name +/// - all rows with `modified > cursor.modified` +/// +/// **Stall hazard (incremental only):** when many rows share the same +/// `modified` second, `modified >= cursor.modified` keeps returning the same +/// page. [PullEngine] owns the stall guard for this case (it only fires for +/// `complete == true` pages). For initial sync the loop terminates on an +/// empty page, so no stall guard is needed. class PullPageFetcher { final ListHttpFn listHttp; @@ -54,31 +54,42 @@ class PullPageFetcher { 'fields': fields, 'order_by': 'modified asc, name asc', 'limit_page_length': pageSize, + 'limit_start': cursor.complete ? 0 : cursor.start, }; - if (!cursor.isNull && cursor.modified != null) { - // Plan-compliant single-predicate form: `modified >= cursor.modified`. - // Combined with `order_by modified asc, name asc`, this returns: - // - the seam row(s) at modified == cursor.modified — idempotently - // re-applied by PullApply's UPSERT-by-server_name - // - all rows with modified > cursor.modified - // No row is silently skipped (unlike the earlier `name > X AND - // modified > X` form, which excluded later-modified earlier-named - // rows). + if (cursor.complete && cursor.modified != null) { + // Incremental: single-predicate form `modified >= cursor.modified`. + // Seam row at cursor.modified is re-applied idempotently by PullApply. params['filters'] = >[ ['modified', '>=', cursor.modified], ]; } + // Initial sync (complete=false): no filter, offset advances via limit_start. final rows = await listHttp(doctype, params); if (rows.isEmpty) { return PullPageResult(rows: rows, advancedCursor: cursor); } final last = rows.last; - final next = Cursor( - modified: last['modified'] as String?, - name: last['name'] as String?, - ); + + final Cursor next; + if (cursor.complete) { + // Incremental: advance by last row's modified/name timestamp. + next = Cursor( + modified: last['modified'] as String?, + name: last['name'] as String?, + complete: true, + ); + } else { + // Initial sync: advance offset; track modified/name for the final cursor + // that markComplete() will persist after the full drain. + next = Cursor( + modified: last['modified'] as String?, + name: last['name'] as String?, + complete: false, + start: cursor.start + rows.length, + ); + } return PullPageResult(rows: rows, advancedCursor: next); } diff --git a/lib/src/sync/push_engine.dart b/lib/src/sync/push_engine.dart index 17431e0..d0b9114 100644 --- a/lib/src/sync/push_engine.dart +++ b/lib/src/sync/push_engine.dart @@ -53,6 +53,20 @@ typedef PushServerFetchFn = typedef PushServerLookupByUuidFn = Future?> Function(String doctype, String mobileUuid); +/// Optional transformer invoked just before the SDK pushes a row to the +/// server. Receives the doctype, the assembled payload, and the parent +/// meta; returns a (possibly modified) payload that gets sent. +/// +/// Use case: consumers can override `docstatus` (e.g. auto-submit +/// submittable doctypes on sync) or strip/inject metadata without +/// changing the local row's representation. +typedef PayloadTransformerFn = + Map Function( + String doctype, + Map payload, + DocTypeMeta meta, + ); + /// Top-level orchestrator for the offline-first push path. Spec §5.2. /// /// Pipeline per outbox row: @@ -104,6 +118,10 @@ class PushEngine { /// when [writeQueueResolver] is non-null. final Map _writeQueues = {}; + /// Optional payload transformer; runs after [PayloadAssembler.assemble] + /// and before HTTP dispatch. See [PayloadTransformerFn]. + final PayloadTransformerFn? payloadTransformer; + PushEngine({ required this.db, required this.outboxDao, @@ -121,6 +139,7 @@ class PushEngine { required this.attachmentUploader, DependenciesForRowFn? dependencyScanner, this.writeQueueResolver, + this.payloadTransformer, this.attachmentBackoff = kDefaultSyncBackoff, this.networkBackoff = kDefaultSyncBackoff, }) : dependencyScanner = dependencyScanner ?? _defaultDependencyScanner; @@ -302,6 +321,17 @@ class PushEngine { ); payload = AttachmentPipeline.inlinePayload(payload, resolved: uploaded); + final transformer = payloadTransformer; + if (transformer != null) { + try { + payload = transformer(row.doctype, payload, meta); + } catch (e, st) { + // ignore: avoid_print + print('PushEngine._dispatchOnce: payloadTransformer threw — $e\n$st'); + // Fall through with un-transformed payload — never block the push. + } + } + final method = _methodFor(row.operation); final isInsert = row.operation == OutboxOperation.insert; diff --git a/lib/src/sync/sync_state_notifier.dart b/lib/src/sync/sync_state_notifier.dart index dde9b4a..5acaa42 100644 --- a/lib/src/sync/sync_state_notifier.dart +++ b/lib/src/sync/sync_state_notifier.dart @@ -47,5 +47,49 @@ class SyncStateNotifier { value = _value.copyWith(failedMetaSyncs: next); } + /// Records [error] on [SyncState.lastError]. Used by the SDK to surface + /// boot-time / sync-pipeline failures the host app can subscribe to and + /// turn into UI (e.g. a "couldn't reach server" banner). + /// + /// Bypasses [SyncState.copyWith] because the copyWith helper coalesces + /// `null` to the prior value — it cannot express "set to this error", + /// only "leave unchanged." + void recordLastError(SyncErrorSummary error) { + if (_value.lastError == error) return; + value = SyncState( + isOnline: _value.isOnline, + isInitialSync: _value.isInitialSync, + isPulling: _value.isPulling, + isPushing: _value.isPushing, + isUploading: _value.isUploading, + isPaused: _value.isPaused, + perDoctype: _value.perDoctype, + queue: _value.queue, + lastError: error, + lastSyncAt: _value.lastSyncAt, + failedMetaSyncs: _value.failedMetaSyncs, + ); + } + + /// Clears [SyncState.lastError]. Called by the SDK at the start of a + /// fresh sync attempt so the UI banner doesn't linger after the user + /// retries. + void clearLastError() { + if (_value.lastError == null) return; + value = SyncState( + isOnline: _value.isOnline, + isInitialSync: _value.isInitialSync, + isPulling: _value.isPulling, + isPushing: _value.isPushing, + isUploading: _value.isUploading, + isPaused: _value.isPaused, + perDoctype: _value.perDoctype, + queue: _value.queue, + lastError: null, + lastSyncAt: _value.lastSyncAt, + failedMetaSyncs: _value.failedMetaSyncs, + ); + } + Future close() => _controller.close(); } diff --git a/lib/src/ui/widgets/form_builder.dart b/lib/src/ui/widgets/form_builder.dart index 786c98b..7935929 100644 --- a/lib/src/ui/widgets/form_builder.dart +++ b/lib/src/ui/widgets/form_builder.dart @@ -173,6 +173,12 @@ class FrappeFormBuilder extends StatefulWidget { /// Called once with the form's submit handler so the parent (e.g. FormScreen) can trigger save from AppBar. final void Function(void Function() submit)? registerSubmit; + /// Fires when [registerSubmit]'s callback was invoked but form + /// validation rejected the submit attempt. Use this to stop a + /// parent-managed loading indicator that was started before triggering + /// submit. Does not fire on successful submit ([onSubmit] does). + final VoidCallback? onValidationFailed; + /// If set, field labels, section titles and tab labels are passed through this (e.g. sdk.translations.translate). final String Function(String)? translate; @@ -211,6 +217,7 @@ class FrappeFormBuilder extends StatefulWidget { this.fetchLinkedDocument, this.getMeta, this.registerSubmit, + this.onValidationFailed, this.translate, this.onButtonPressed, this.onFormDataChanged, @@ -315,6 +322,16 @@ class _FrappeFormBuilderState extends State linkOptionService: widget.linkOptionService, linkFieldCoordinator: _linkFieldCoordinator, ); + // Custom factories supplied by the host won't have these wired in + // their own constructor — they're host-internal services exposed + // here so override factories (e.g. SNF's SnfFieldFactory) can + // still produce Link/etc. fields without their pickers being half- + // configured on first build. Default-constructed FieldFactory above + // sets both directly; this branch handles the custom case. + if (widget.customFieldFactory != null) { + _fieldFactory.linkOptionService ??= widget.linkOptionService; + _fieldFactory.linkFieldCoordinator ??= _linkFieldCoordinator; + } _buildFormStructure(); _tabController = TabController( @@ -1236,6 +1253,7 @@ class _FrappeFormBuilderState extends State break; } } + widget.onValidationFailed?.call(); return; } diff --git a/test/sync/push_payload_transform_test.dart b/test/sync/push_payload_transform_test.dart new file mode 100644 index 0000000..a1ef550 --- /dev/null +++ b/test/sync/push_payload_transform_test.dart @@ -0,0 +1,40 @@ +import 'package:flutter_test/flutter_test.dart'; +import 'package:frappe_mobile_sdk/frappe_mobile_sdk.dart'; + +void main() { + test('PayloadTransformerFn typedef is exported from the SDK barrel', () { + // Smoke test: typedef must be importable from package consumer. + Map identity( + String doctype, + Map payload, + DocTypeMeta meta, + ) => payload; + final PayloadTransformerFn f = identity; + expect(f, isNotNull); + }); + + test('transformer fn receives doctype, payload, meta and returns map', () { + Map? captured; + Map fn( + String doctype, + Map payload, + DocTypeMeta meta, + ) { + captured = payload; + return {...payload, 'docstatus': 1}; + } + + final meta = DocTypeMeta( + name: 'X', + label: 'X', + isTable: false, + titleField: null, + searchFields: null, + fields: const [], + ); + final out = fn('X', {'foo': 'bar'}, meta); + + expect(captured, {'foo': 'bar'}); + expect(out, {'foo': 'bar', 'docstatus': 1}); + }); +} diff --git a/test/ui/widgets/form_builder_validation_failed_test.dart b/test/ui/widgets/form_builder_validation_failed_test.dart new file mode 100644 index 0000000..dc78fbc --- /dev/null +++ b/test/ui/widgets/form_builder_validation_failed_test.dart @@ -0,0 +1,97 @@ +import 'package:flutter/material.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:frappe_mobile_sdk/frappe_mobile_sdk.dart'; + +void main() { + testWidgets('onValidationFailed fires when a mandatory field is empty', ( + tester, + ) async { + var failedCount = 0; + var submitCount = 0; + void Function()? captured; + + final meta = DocTypeMeta( + name: 'Test', + label: 'Test', + isTable: false, + titleField: null, + searchFields: null, + fields: [ + DocField( + fieldname: 'student_name', + fieldtype: 'Data', + idx: 1, + label: 'Student Name', + reqd: true, + ), + ], + ); + + await tester.pumpWidget( + MaterialApp( + home: Scaffold( + body: FrappeFormBuilder( + meta: meta, + initialData: const {}, + onSubmit: (_) => submitCount++, + onValidationFailed: () => failedCount++, + registerSubmit: (cb) => captured = cb, + ), + ), + ), + ); + await tester.pumpAndSettle(); + + captured!.call(); // attempt submit with empty mandatory + await tester.pumpAndSettle(); + + expect(failedCount, 1); + expect(submitCount, 0); + }); + + testWidgets('onValidationFailed does not fire on valid submit', ( + tester, + ) async { + var failedCount = 0; + var submitCount = 0; + void Function()? captured; + + final meta = DocTypeMeta( + name: 'Test', + label: 'Test', + isTable: false, + titleField: null, + searchFields: null, + fields: [ + DocField( + fieldname: 'note', + fieldtype: 'Data', + idx: 1, + label: 'Note', + reqd: false, + ), + ], + ); + + await tester.pumpWidget( + MaterialApp( + home: Scaffold( + body: FrappeFormBuilder( + meta: meta, + initialData: const {'note': 'hello'}, + onSubmit: (_) => submitCount++, + onValidationFailed: () => failedCount++, + registerSubmit: (cb) => captured = cb, + ), + ), + ), + ); + await tester.pumpAndSettle(); + + captured!.call(); // valid submit + await tester.pumpAndSettle(); + + expect(failedCount, 0); + expect(submitCount, 1); + }); +} From 4e09e508ed1789ec6e4863449947082dbaf2ce07 Mon Sep 17 00:00:00 2001 From: omprakash Date: Fri, 29 May 2026 14:54:26 +0530 Subject: [PATCH 4/9] chore: untrack local dev tooling (.mcp.json, devtools_options.yaml, scripts/) These were swept in by `git add -A` during the merges; they are local developer tooling, not part of the package. Untracked and gitignored. --- .gitignore | 6 +++- .mcp.json | 10 ------- devtools_options.yaml | 3 -- scripts/check_coverage.sh | 20 ------------- scripts/extract_doctypes.py | 43 -------------------------- scripts/find_uncovered.py | 26 ---------------- scripts/parse_coverage.py | 60 ------------------------------------- 7 files changed, 5 insertions(+), 163 deletions(-) delete mode 100644 .mcp.json delete mode 100644 devtools_options.yaml delete mode 100755 scripts/check_coverage.sh delete mode 100644 scripts/extract_doctypes.py delete mode 100644 scripts/find_uncovered.py delete mode 100644 scripts/parse_coverage.py diff --git a/.gitignore b/.gitignore index 0aa0c40..e4a4e28 100644 --- a/.gitignore +++ b/.gitignore @@ -89,4 +89,8 @@ test/.test_coverage.dart # FVM Version Cache .fvm/ -/docs \ No newline at end of file +/docs +# Local dev tooling (not part of the package) +.mcp.json +devtools_options.yaml +/scripts/ diff --git a/.mcp.json b/.mcp.json deleted file mode 100644 index fc59994..0000000 --- a/.mcp.json +++ /dev/null @@ -1,10 +0,0 @@ -{ - "mcpServers": { - "flutter-skill": { - "command": "flutter-skill", - "args": [ - "server" - ] - } - } -} diff --git a/devtools_options.yaml b/devtools_options.yaml deleted file mode 100644 index fa0b357..0000000 --- a/devtools_options.yaml +++ /dev/null @@ -1,3 +0,0 @@ -description: This file stores settings for Dart & Flutter DevTools. -documentation: https://docs.flutter.dev/tools/devtools/extensions#configure-extension-enablement-states -extensions: diff --git a/scripts/check_coverage.sh b/scripts/check_coverage.sh deleted file mode 100755 index 333172b..0000000 --- a/scripts/check_coverage.sh +++ /dev/null @@ -1,20 +0,0 @@ -#!/bin/bash - -# Get the directory where the script is located -SCRIPT_DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd )" -PROJECT_ROOT="$( dirname "$SCRIPT_DIR" )" - -cd "$PROJECT_ROOT" || exit 1 - -# Run tests and generate coverage -echo "Running tests with coverage in $PROJECT_ROOT..." -flutter test --coverage - -# Check if coverage was generated -if [ -f "coverage/lcov.info" ]; then - echo "Parsing coverage report..." - python3 scripts/parse_coverage.py -else - echo "Error: coverage/lcov.info not found. Did tests run successfully?" - exit 1 -fi diff --git a/scripts/extract_doctypes.py b/scripts/extract_doctypes.py deleted file mode 100644 index 912fe12..0000000 --- a/scripts/extract_doctypes.py +++ /dev/null @@ -1,43 +0,0 @@ -import os -import json -import re - -def sanitize_filename(name): - # Convert spaces to underscores and remove non-alphanumeric chars - name = name.lower() - name = re.sub(r'[^a-z0-9]+', '_', name) - return name.strip('_') - -def extract_doctypes(input_file, output_dir): - if not os.path.exists(output_dir): - os.makedirs(output_dir) - print(f"Created directory: {output_dir}") - - count = 0 - with open(input_file, 'r') as f: - for line in f: - parts = line.strip().split('\t') - if len(parts) < 3: - continue - - doctype_name = parts[1].strip() - json_content = parts[2].strip() - - if not doctype_name or json_content == '{}': - continue - - filename = sanitize_filename(doctype_name) + '.json' - filepath = os.path.join(output_dir, filename) - - try: - data = json.loads(json_content) - with open(filepath, 'w') as out: - json.dump(data, out, indent=2) - count += 1 - except json.JSONDecodeError: - print(f"Error decoding JSON for doctype: {doctype_name}") - - print(f"Extracted {count} doctypes to {output_dir}") - -if __name__ == "__main__": - extract_doctypes('AllDoctypesDump.md', 'assets/doctypes') diff --git a/scripts/find_uncovered.py b/scripts/find_uncovered.py deleted file mode 100644 index 4102b0e..0000000 --- a/scripts/find_uncovered.py +++ /dev/null @@ -1,26 +0,0 @@ -import sys - -def find_uncovered_lines(lcov_path, target_file): - with open(lcov_path, 'r') as f: - in_target = False - uncovered = [] - for line in f: - line = line.strip() - if line.startswith('SF:') and target_file in line: - in_target = True - elif line == 'end_of_record': - in_target = False - elif in_target and line.startswith('DA:'): - # DA:, - parts = line.split(':')[1].split(',') - line_num = int(parts[0]) - hits = int(parts[1]) - if hits == 0: - uncovered.append(line_num) - return uncovered - -if __name__ == "__main__": - target = sys.argv[1] - lcov = 'coverage/lcov.info' - lines = find_uncovered_lines(lcov, target) - print(f"Uncovered lines in {target}: {lines}") diff --git a/scripts/parse_coverage.py b/scripts/parse_coverage.py deleted file mode 100644 index cb16e74..0000000 --- a/scripts/parse_coverage.py +++ /dev/null @@ -1,60 +0,0 @@ -import sys -import os - -def parse_lcov(file_path): - total_lines_found = 0 - total_lines_hit = 0 - files = {} - - if not os.path.exists(file_path): - print(f"Error: {file_path} not found.") - return - - current_file = None - lines_found = 0 - lines_hit = 0 - - with open(file_path, 'r') as f: - for line in f: - line = line.strip() - if line.startswith('SF:'): - current_file = line.split('SF:')[1] - elif line.startswith('LF:'): - lines_found = int(line.split(':')[1]) - elif line.startswith('LH:'): - lines_hit = int(line.split(':')[1]) - elif line == 'end_of_record': - if current_file: - files[current_file] = { - 'found': lines_found, - 'hit': lines_hit, - 'percent': (lines_hit / lines_found * 100) if lines_found > 0 else 0 - } - total_lines_found += lines_found - total_lines_hit += lines_hit - current_file = None - lines_found = 0 - lines_hit = 0 - - print("-" * 80) - print(f"{'File':<50} | {'Coverage':<10} | {'Lines'}") - print("-" * 80) - - # Sort files by coverage (lowest first) to highlight what needs work - sorted_files = sorted(files.items(), key=lambda x: x[1]['percent']) - - for file, stats in sorted_files: - # Show only relative path from lib/ - rel_path = file.split('lib/')[-1] if 'lib/' in file else file - print(f"{rel_path[:48]:<50} | {stats['percent']:>8.2f}% | {stats['hit']}/{stats['found']}") - - print("-" * 80) - total_percent = (total_lines_hit / total_lines_found * 100) if total_lines_found > 0 else 0 - print(f"{'TOTAL':<50} | {total_percent:>8.2f}% | {total_lines_hit}/{total_lines_found}") - print("-" * 80) - -if __name__ == "__main__": - path = 'coverage/lcov.info' - if len(sys.argv) > 1: - path = sys.argv[1] - parse_lcov(path) From aeefde0b9d4a5bc4ce7928662eec803e12e9f1f1 Mon Sep 17 00:00:00 2001 From: omprakash Date: Thu, 4 Jun 2026 13:32:48 +0530 Subject: [PATCH 5/9] =?UTF-8?q?feat(sync):=20offline-sync=20hardening=20?= =?UTF-8?q?=E2=80=94=20push/pull=20race,=20sync=20modes,=20resumable=20pul?= =?UTF-8?q?l,=20pre-flight=20manifest?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - #43: PullApply matches a pulled parent by mobile_uuid when server_name is not yet stamped locally, preventing duplicate rows during an in-flight push INSERT; reconcile (no false conflict) when local server_name is NULL. Extend the hasActivePushFor defer to SyncService pull paths (parity with PullEngine) via SyncStatus.deferredActivePush. - #47: SyncMode {full, pushOnly, pullOnly} on SyncController.syncNow (default full = existing behavior). - #64: checkpoint the PullEngine cursor (complete:false) after every applied page so an app-kill mid initial-sync resumes from the last page, not page 0. - #49: /sync_details pre-flight client (DoctypeService.getSyncDetails) + pure doctypesToSkip; wired into _runUpgradeClosurePull to skip unchanged INCREMENTAL doctypes, with graceful full-pull fallback on any failure. Note: sdk_log.dart and the sdkLog lines in sync_controller/sync_service are pre-existing refactor bits that ride along because these files are shared; the rest of that print->sdkLog refactor is intentionally left uncommitted. --- lib/src/api/doctype_service.dart | 28 +++ lib/src/sdk/frappe_sdk.dart | 58 +++++- lib/src/services/sync_controller.dart | 72 +++++-- lib/src/services/sync_service.dart | 57 ++++-- lib/src/sync/pull_apply.dart | 38 +++- lib/src/sync/pull_engine.dart | 23 ++- lib/src/sync/sync_details.dart | 63 ++++++ lib/src/utils/sdk_log.dart | 16 ++ .../doctype_service_sync_details_test.dart | 69 +++++++ test/sdk/frappe_sdk_sync_details_test.dart | 143 ++++++++++++++ .../sync_controller_sync_mode_test.dart | 72 +++++++ .../sync_service_active_push_defer_test.dart | 71 +++++++ test/sync/pull_apply_test.dart | 76 +++++++ test/sync/pull_engine_test.dart | 187 +++++++++++++----- test/sync/sync_details_test.dart | 74 +++++++ 15 files changed, 956 insertions(+), 91 deletions(-) create mode 100644 lib/src/sync/sync_details.dart create mode 100644 lib/src/utils/sdk_log.dart create mode 100644 test/api/doctype_service_sync_details_test.dart create mode 100644 test/sdk/frappe_sdk_sync_details_test.dart create mode 100644 test/services/sync_controller_sync_mode_test.dart create mode 100644 test/services/sync_service_active_push_defer_test.dart create mode 100644 test/sync/sync_details_test.dart diff --git a/lib/src/api/doctype_service.dart b/lib/src/api/doctype_service.dart index 8e5ec3a..b76dbfe 100644 --- a/lib/src/api/doctype_service.dart +++ b/lib/src/api/doctype_service.dart @@ -6,6 +6,7 @@ import 'dart:math' as math; import 'package:flutter/foundation.dart'; +import '../sync/sync_details.dart'; import 'exceptions.dart'; import 'rest_helper.dart'; import 'utils.dart'; @@ -187,6 +188,33 @@ class DoctypeService { ]; } + /// Pre-flight manifest (#49). Posts the INCREMENTAL doctypes about to be + /// pulled, each with its own `since` watermark, and returns per-doctype + /// change info. Returns null on ANY failure (network, 404, missing endpoint, + /// malformed body) so the caller falls back to a full pull. + Future getSyncDetails( + List> doctypeSince, + ) async { + if (doctypeSince.isEmpty) return null; + try { + final response = await _restHelper.post( + '/api/method/mobile_sync.sync_details', + body: {'doctypes': doctypeSince}, + ); + final dynamic message = response is Map + ? response['message'] + : response; + if (message is! Map) return null; + return SyncDetailsResponse.fromJson(Map.from(message)); + } catch (e, st) { + debugPrint( + 'DoctypeService.getSyncDetails failed — falling back to full pull — ' + '$e\n$st', + ); + return null; + } + } + /// Pages through `frappe.client.get_list` for names, then bulk-fetches /// full documents (parents + child rows) via the server-side /// `mobile_sync.get_docs_with_children` endpoint. Used by the pull diff --git a/lib/src/sdk/frappe_sdk.dart b/lib/src/sdk/frappe_sdk.dart index 149e1f9..551fa2f 100644 --- a/lib/src/sdk/frappe_sdk.dart +++ b/lib/src/sdk/frappe_sdk.dart @@ -2,8 +2,10 @@ // For license information, please see license.txt import 'dart:async'; +import 'dart:convert'; import 'package:flutter/foundation.dart'; +import 'package:http/http.dart' as http; import '../api/client.dart'; import '../api/exceptions.dart'; @@ -11,6 +13,7 @@ import '../concurrency/concurrency_pool.dart'; import '../concurrency/connectivity_watcher.dart'; import '../database/app_database.dart'; import '../database/daos/doctype_meta_dao.dart'; +import '../database/daos/outbox_dao.dart'; import '../database/daos/sdk_meta_dao.dart'; import '../models/doc_type_meta.dart'; import '../models/offline_mode.dart'; @@ -29,8 +32,10 @@ import '../services/offline_repository.dart'; import '../services/offline_transition_service.dart'; import '../services/link_option_service.dart'; import '../services/translation_service.dart'; +import '../sync/cursor.dart'; import '../sync/pull_engine.dart'; import '../sync/push_engine.dart'; +import '../sync/sync_details.dart'; import '../sync/sync_state.dart'; import '../sync/sync_state_notifier.dart'; @@ -164,13 +169,16 @@ class FrappeSDK { enabled: true, isPersisted: true, ), + http.Client? httpClient, }) : databaseAppName = null { _database = database; _modeNotifier = OfflineModeNotifier(offlineMode); _syncCompleteController = StreamController.broadcast(); // Create FrappeClient directly — avoids AuthService.initialize() which // writes to FlutterSecureStorage and is unavailable in widget tests. - _client = FrappeClient(baseUrl); + // [httpClient] lets tests stub the transport (e.g. the /sync_details + // pre-flight) without a real network. + _client = FrappeClient(baseUrl, httpClient: httpClient); // Use AuthService.forTesting so the client and database are wired up // without touching FlutterSecureStorage. This means sdk.auth methods // (e.g. getOrCreateMobileUuid, restoreSession) won't throw "not @@ -348,6 +356,11 @@ class FrappeSDK { getMobileUuid: _resolveMobileUuid, offlineModeNotifier: _modeNotifier!, pushRunner: () => _pushEngine!.runOnce(), + // Parity with PullEngine's guard: SyncService's pull paths + // (pullSyncWaiting, pullSyncMany, forcePullAll) defer a doctype while a + // push is active for it, so a pull can't insert a duplicate over an + // in-flight push INSERT (#43). OutboxDao is a thin wrapper over rawDb. + hasActivePush: (doctype) => OutboxDao(rawDb).hasActivePushFor(doctype), ); // Build UnifiedResolver — single read path for all offline queries. // Probe connectivity once here AND subscribe to the platform @@ -1230,11 +1243,52 @@ class FrappeSDK { pullable.add(doctype); } if (pullable.isEmpty) return const {}; + + // #49 pre-flight: ask the server which INCREMENTAL doctypes actually + // changed, and skip pulling the rest. Only doctypes with a complete + // (delta-ready) cursor are eligible — INITIAL/RESUME doctypes have no + // trustworthy watermark and are always pulled. Any failure (missing + // endpoint, network) leaves `allowed == pullable` → full pull. + var allowed = pullable; + final eligible = {}; // doctype -> since (cursor.modified) + for (final dt in pullable) { + try { + final raw = await _database!.doctypeMetaDao.getLastOkCursor(dt); + if (raw == null || raw.isEmpty) continue; + final cursor = Cursor.fromJson( + jsonDecode(raw) as Map, + ); + if (cursor.complete && cursor.modified != null) { + eligible[dt] = cursor.modified!; + } + } catch (e, st) { + debugPrint( + 'FrappeSDK: sync_details cursor read($dt) failed — $e\n$st', + ); + } + } + if (eligible.isNotEmpty) { + final manifest = await _client!.doctype.getSyncDetails([ + for (final entry in eligible.entries) + {'doctype': entry.key, 'since': entry.value}, + ]); + if (manifest != null) { + final skip = doctypesToSkip(eligible.keys.toSet(), manifest); + if (skip.isNotEmpty) { + allowed = pullable.difference(skip); + debugPrint( + 'FrappeSDK: sync_details skipped ${skip.length}/' + '${eligible.length} unchanged doctypes', + ); + } + } + } + // Hold the SyncMutex for the entire closure pull so concurrent // single-doctype callers (pullSync, pullSyncWaiting) serialise // behind it — same contract as the former pullSyncMany call path. return await _syncService!.protect( - () => _pullEngine!.run(closure, allowedDoctypes: pullable), + () => _pullEngine!.run(closure, allowedDoctypes: allowed), ); } catch (e, st) { debugPrint('FrappeSDK: closure pull failed — $e\n$st'); diff --git a/lib/src/services/sync_controller.dart b/lib/src/services/sync_controller.dart index 5dacfd2..09ec274 100644 --- a/lib/src/services/sync_controller.dart +++ b/lib/src/services/sync_controller.dart @@ -6,6 +6,7 @@ import '../models/outbox_row.dart'; import '../sync/sync_state.dart'; import '../sync/sync_state_notifier.dart'; import 'retry_priority.dart'; +import '../utils/sdk_log.dart'; /// Choice the user makes on a conflict row in the SyncErrorsScreen. /// `pullAndOverwriteLocal` accepts the server snapshot (local edits @@ -13,6 +14,16 @@ import 'retry_priority.dart'; /// re-fetches the server version and runs ThreeWayMerge again. enum ConflictAction { pullAndOverwriteLocal, keepLocalAndRetry } +/// Selects how much of the sync cycle [SyncController.syncNow] runs. +/// +/// Cost profile (document for app authors choosing a background cadence): +/// - [pushOnly] — cheap; one outbox drain. Recommended default for periodic +/// background wakeups (WorkManager / BGTaskScheduler) on metered networks. +/// - [pullOnly] — moderate; reads changed doctypes, no writes. +/// - [full] — heaviest; recommended on charge + Wi-Fi, or on explicit +/// user action. +enum SyncMode { full, pushOnly, pullOnly } + typedef RunFn = Future Function(); /// Pull runner — returns the set of doctypes the engine deferred because @@ -93,12 +104,32 @@ class SyncController { SyncState get state => notifier.value; Stream get state$ => notifier.stream; - /// Pull then push. Doctypes deferred during the pull (because a push - /// was active for them) are re-pulled after the push completes — SIG-2. - /// Used by `Sync now` button + connectivity-restore hooks. No-ops while - /// paused. - Future syncNow() async { + /// Runs the sync cycle selected by [mode]. No-ops while paused. + /// + /// - [SyncMode.full] (default): pull → push → re-pull the doctypes the pull + /// deferred because a push was active for them (SIG-2). Used by the + /// `Sync now` button + connectivity-restore hooks. + /// - [SyncMode.pushOnly]: drain the outbox only. Safe to run concurrently + /// with a foreground pull (#43 closes the race). No deferred handling. + /// - [SyncMode.pullOnly]: pull only; no push. The deferred set is NOT + /// re-pulled here — without a push to clear those rows an immediate + /// re-pull would just defer again; they are picked up by the next full + /// sync. + /// + /// Best-effort: a failed pull still lets push drain in [SyncMode.full]; + /// failures are surfaced via [SyncState], not thrown. + Future syncNow({SyncMode mode = SyncMode.full}) async { if (notifier.value.isPaused) return; + + if (mode == SyncMode.pushOnly) { + try { + await runPush(); + } catch (e, st) { + sdkLog('SyncController.syncNow(pushOnly): runPush failed — $e\n$st'); + } + return; + } + Set deferred = const {}; try { deferred = await runPull(); @@ -107,16 +138,25 @@ class SyncController { // when the server-read side has issues. PullEngine.run's own // finally already resets `isPulling`. syncNow is best-effort: // surface via SyncState.lastError, not by throwing. - // ignore: avoid_print - print('SyncController.syncNow: runPull failed — $e\n$st'); + sdkLog('SyncController.syncNow: runPull failed — $e\n$st'); + } + + if (mode == SyncMode.pullOnly) { + if (deferred.isNotEmpty) { + sdkLog( + 'SyncController.syncNow(pullOnly): ${deferred.length} doctype(s) ' + 'deferred for active push; left for next full sync — $deferred', + ); + } + return; } + try { await runPush(); } catch (e, st) { // PushEngine.runOnce wraps its body in try/finally, so isPushing // is already reset. Same best-effort contract as the pull above. - // ignore: avoid_print - print('SyncController.syncNow: runPush failed — $e\n$st'); + sdkLog('SyncController.syncNow: runPush failed — $e\n$st'); } if (deferred.isNotEmpty) { try { @@ -124,8 +164,7 @@ class SyncController { } catch (e, st) { // SIG-2 deferred re-pull is best-effort; the doctypes will be // picked up on the next syncNow when push activity has settled. - // ignore: avoid_print - print( + sdkLog( 'SyncController.syncNow: runPullForDoctypes($deferred) failed — ' '$e\n$st', ); @@ -226,10 +265,10 @@ class SyncController { try { await clearLocalConflict!(row.doctype, row.mobileUuid); } catch (e, st) { - // Best-effort. The outbox row still gets closed; logging - // surfaces the cleanup failure without blocking resolution. - // ignore: avoid_print - print( + // Best-effort. The outbox row still gets closed; the debug log + // surfaces the cleanup failure without blocking resolution + // (sdkLog is a no-op in release). + sdkLog( 'SyncController.resolveConflict: clearLocalConflict failed ' 'for ${row.doctype}/${row.mobileUuid} — $e\n$st', ); @@ -283,8 +322,7 @@ class SyncController { ); } } catch (e, st) { - // ignore: avoid_print - print( + sdkLog( 'SyncController.previewDeleteCascade: blockedBy parse failed — $e\n$st', ); blocked = const {}; diff --git a/lib/src/services/sync_service.dart b/lib/src/services/sync_service.dart index e29de0e..fedbf9f 100644 --- a/lib/src/services/sync_service.dart +++ b/lib/src/services/sync_service.dart @@ -11,6 +11,7 @@ import '../models/offline_mode.dart'; import '../models/offline_mode_notifier.dart'; import '../sync/cursor.dart'; import 'offline_repository.dart'; +import '../utils/sdk_log.dart'; /// Per-doctype sync phase observable from outside the SDK. Drives UX: /// `initial` → blocking "preparing offline data" screen, @@ -53,6 +54,12 @@ class SyncService { /// pipeline. Wired by `FrappeSDK` to `PushEngine.runOnce()`. final Future Function()? _pushRunner; + /// Optional. Returns true when a push is active (pending/in-flight outbox + /// rows) for the given doctype. When non-null, [_pullOneInternal] defers a + /// pull for that doctype — parity with `PullEngine`'s existing guard + /// (`pull_engine.dart:133`). Null in tests / when unwired → no defer. + final Future Function(String doctype)? _hasActivePush; + SyncService( this._client, this._repository, @@ -64,9 +71,11 @@ class SyncService { ), OfflineModeNotifier? offlineModeNotifier, Future Function()? pushRunner, + Future Function(String doctype)? hasActivePush, }) : _getMobileUuid = getMobileUuid, _modeNotifier = offlineModeNotifier ?? OfflineModeNotifier(offlineMode), - _pushRunner = pushRunner; + _pushRunner = pushRunner, + _hasActivePush = hasActivePush; /// Check if device is online. /// Returns false when offline mode is disabled, regardless of connectivity, @@ -100,10 +109,9 @@ class SyncService { online = await isOnline(); } catch (e, st) { // Platform-channel failure (e.g. headless test environment without - // connectivity_plus mocks). Treat as offline; surface the failure - // mode in logs rather than silently swallowing it. - // ignore: avoid_print - print('SyncService.pushSync: isOnline() threw — $e\n$st'); + // connectivity_plus mocks). Treat as offline; log the failure mode + // in debug builds (sdkLog is a no-op in release). + sdkLog('SyncService.pushSync: isOnline() threw — $e\n$st'); } if (!online) { return SyncResult( @@ -117,8 +125,7 @@ class SyncService { } final runner = _pushRunner; if (runner == null) { - // ignore: avoid_print - print( + sdkLog( 'SyncService.pushSync: pushRunner not wired; returning empty result', ); return SyncResult.empty(); @@ -159,8 +166,7 @@ class SyncService { : DoctypePullPhase.resume; } catch (e, st) { // Corrupted cursor reads as INITIAL — the next pull will refresh it. - // ignore: avoid_print - print('SyncService.getPullPhase: corrupted cursor — $e\n$st'); + sdkLog('SyncService.getPullPhase: corrupted cursor — $e\n$st'); return DoctypePullPhase.initial; } } @@ -289,8 +295,7 @@ class SyncService { try { out[dt] = await _pullOneInternal(doctype: dt); } catch (e, st) { - // ignore: avoid_print - print('SyncService.pullSyncMany($dt) failed — $e\n$st'); + sdkLog('SyncService.pullSyncMany($dt) failed — $e\n$st'); out[dt] = SyncResult( 0, 0, @@ -384,6 +389,10 @@ class SyncService { @visibleForTesting Future isChildTableForTest(String doctype) => _isChildTable(doctype); + @visibleForTesting + Future pullOneInternalForTest({required String doctype}) => + _pullOneInternal(doctype: doctype); + Future _isChildTable(String doctype) async { final raw = await _database.doctypeMetaDao.getMetaJson(doctype); if (raw == null || raw.isEmpty || raw == '{}') return false; @@ -392,8 +401,7 @@ class SyncService { if (parsed.isEmpty) return false; return DocTypeMeta.fromJson(parsed).isTable; } catch (e, st) { - // ignore: avoid_print - print('SyncService._isChildTable($doctype) parse failed — $e\n$st'); + sdkLog('SyncService._isChildTable($doctype) parse failed — $e\n$st'); return false; } } @@ -407,6 +415,15 @@ class SyncService { return SyncResult(0, 0, 0, null, errors: const []); } + // Active-push defer (parity with PullEngine, pull_engine.dart:133): + // skip the pull while this doctype has pending/in-flight outbox rows so + // we don't pull stale server state over local edits in flight. The + // doctype is retried on the next sync cycle. + final hasActivePush = _hasActivePush; + if (hasActivePush != null && await hasActivePush(doctype)) { + return SyncResult.empty(status: SyncStatus.deferredActivePush); + } + int success = 0; int failed = 0; int total = 0; @@ -431,8 +448,7 @@ class SyncService { cursorComplete = cv == true; } catch (e, st) { // Corrupted cursor — treat as fresh INITIAL pull. - // ignore: avoid_print - print('SyncService.pullSync: corrupted cursor — $e\n$st'); + sdkLog('SyncService.pullSync: corrupted cursor — $e\n$st'); } } @@ -545,8 +561,7 @@ class SyncService { pageLastModified = modifiedAt; pageLastName = serverId; } catch (e, st) { - // ignore: avoid_print - print( + sdkLog( 'SyncService.pullSync applyServerDocument($doctype/$serverId) failed — $e\n$st', ); failed++; @@ -634,8 +649,7 @@ class SyncService { } return false; } catch (e, st) { - // ignore: avoid_print - print( + sdkLog( 'SyncService._doctypeHasChildTables($doctype) parse failed — $e\n$st', ); return false; @@ -682,6 +696,11 @@ enum SyncStatus { /// `error` is non-null with "Sync already in progress". busy, + /// A push was active (pending/in-flight outbox rows) for this doctype, so + /// the pull was skipped this cycle to avoid racing the push. The doctype + /// is picked up on the next sync. `success/failed/total` are zero. + deferredActivePush, + /// The sync executed. `success/failed/total` carry actual row counts; /// when `failed > 0`, `errors` lists per-row details. Note that a /// successful call with no dirty rows ALSO reports `ran` — `success=0` diff --git a/lib/src/sync/pull_apply.dart b/lib/src/sync/pull_apply.dart index 3b46bdc..ae70837 100644 --- a/lib/src/sync/pull_apply.dart +++ b/lib/src/sync/pull_apply.dart @@ -124,21 +124,55 @@ class PullApply { final serverName = r['name'] as String?; if (serverName == null) continue; - final existing = await txn.query( + var existing = await txn.query( parentTable, - columns: ['mobile_uuid', 'sync_status', 'modified'], + columns: ['mobile_uuid', 'sync_status', 'modified', 'server_name'], where: 'server_name = ?', whereArgs: [serverName], limit: 1, ); + // Fallback match by mobile_uuid. When a push INSERT has committed + // server-side but its local writeback hasn't stamped `server_name` + // yet, the server_name lookup misses. The pulled mobile-origin row + // carries the same `mobile_uuid` the device assigned (server stores + // it as a unique field that round-trips), so match on that to update + // the existing row in place instead of inserting a duplicate. + // Desk-origin rows have an empty mobile_uuid and never hit this path. + var matchedByUuid = false; + if (existing.isEmpty) { + final incomingUuid = r['mobile_uuid']?.toString(); + if (incomingUuid != null && incomingUuid.isNotEmpty) { + existing = await txn.query( + parentTable, + columns: ['mobile_uuid', 'sync_status', 'modified', 'server_name'], + where: 'mobile_uuid = ?', + whereArgs: [incomingUuid], + limit: 1, + ); + matchedByUuid = existing.isNotEmpty; + } + } + // Tombstoned rows never resurrect — local DELETE is queued in // outbox waiting to push. Skip silently; once the DELETE outbox // row drains, the row is hard-deleted server-side too. if (existing.isNotEmpty && existing.first['sync_status'] == 'deleted') { continue; } + // False-conflict guard: a row matched via the mobile_uuid fallback + // whose local `server_name` is still NULL is our own INSERT + // round-tripping (push committed server-side, writeback pending) — + // reconcile it (stamp server_name, mark synced) instead of flagging a + // spurious conflict. server_name matches never set matchedByUuid, so + // genuine server-side edits to synced docs still flag conflict. + final localServerName = existing.isEmpty + ? null + : existing.first['server_name'] as String?; + final isOwnInsertRoundtrip = + matchedByUuid && (localServerName == null || localServerName.isEmpty); if (existing.isNotEmpty && + !isOwnInsertRoundtrip && _locallyDirtyStatuses.contains(existing.first['sync_status'])) { // Spec §5.1 requires "server has advanced" before flagging a // conflict. Cursor filtering normally guarantees this, but diff --git a/lib/src/sync/pull_engine.dart b/lib/src/sync/pull_engine.dart index 39ed877..82a097d 100644 --- a/lib/src/sync/pull_engine.dart +++ b/lib/src/sync/pull_engine.dart @@ -41,12 +41,11 @@ typedef SchemaReconcilerFn = /// edits in flight. /// 2. Resolve parent + child metas. /// 3. Loop pages via [PullPageFetcher]. Each page → [PullApply.applyPage] -/// in a transaction. Cursor is held in memory until the doctype is -/// fully drained (last page returned < pageSize rows or threw); then -/// persisted via [DoctypeMetaDao.setLastOkCursor]. -/// 4. Cursor advance is per-doctype atomic — partial pulls leave the -/// persisted cursor untouched, so a relaunch resumes from the last -/// fully-applied page. +/// in a transaction, then the advanced cursor (complete:false) is +/// checkpointed via [DoctypeMetaDao.setLastOkCursor] (#64). When the +/// doctype drains, the cursor is flipped to complete:true. +/// 4. A relaunch mid initial-sync resumes from the last checkpointed page +/// (limit_start = cursor.start) instead of page 0. /// /// Doctypes drain in parallel through [pool] (typically PullPool, sized by /// [DeviceTier]). @@ -245,6 +244,18 @@ class PullEngine { ), ); + // #64: checkpoint the cursor after every successfully-applied page so + // an app-kill mid initial-sync resumes from here (limit_start = + // scratch.start) instead of page 0. `scratch` is complete:false; the + // post-loop markComplete() flip remains the only writer of + // complete:true. Mirrors SyncService._pullOneInternal's per-page + // journal. A crash between the page apply and this write re-applies + // the page on resume — idempotent via PullApply's UPSERT. + final pageCursorJson = scratch.toJson(); + if (pageCursorJson != null) { + await metaDao.setLastOkCursor(doctype, jsonEncode(pageCursorJson)); + } + // Spec §5.1: only break on empty page. A short non-empty page is // still followed by one confirmatory empty fetch — Frappe doesn't // tell us "no more rows" inline; we have to ask. The "fail before diff --git a/lib/src/sync/sync_details.dart b/lib/src/sync/sync_details.dart new file mode 100644 index 0000000..555d365 --- /dev/null +++ b/lib/src/sync/sync_details.dart @@ -0,0 +1,63 @@ +// Pre-flight manifest types for `/sync_details` (#49). The SDK posts the +// INCREMENTAL doctypes it is about to pull (each with its own `since` +// watermark) and uses the response to skip no-op pulls. + +/// One doctype's entry in the `/sync_details` manifest. +class SyncDetailsEntry { + final String doctype; + final bool changed; + final int count; + final bool metaBumped; + + const SyncDetailsEntry({ + required this.doctype, + required this.changed, + this.count = 0, + this.metaBumped = false, + }); + + factory SyncDetailsEntry.fromJson(Map j) => SyncDetailsEntry( + doctype: (j['doctype'] ?? '').toString(), + changed: j['changed'] == true, + count: j['count'] is int ? j['count'] as int : 0, + metaBumped: j['meta_bumped'] == true, + ); +} + +class SyncDetailsResponse { + /// Keyed by doctype. + final Map entries; + final int deleteSignals; + + const SyncDetailsResponse({required this.entries, this.deleteSignals = 0}); + + factory SyncDetailsResponse.fromJson(Map j) { + final entries = {}; + final list = j['doctypes']; + if (list is List) { + for (final raw in list) { + if (raw is Map) { + final e = SyncDetailsEntry.fromJson(Map.from(raw)); + if (e.doctype.isNotEmpty) entries[e.doctype] = e; + } + } + } + final ds = j['delete_signals']; + return SyncDetailsResponse( + entries: entries, + deleteSignals: ds is int ? ds : 0, + ); + } +} + +/// Doctypes safe to skip this pull: present in [eligible], reported by the +/// manifest as `changed == false` AND `metaBumped == false`. A doctype absent +/// from the manifest, or reported changed/meta-bumped, is always pulled. +Set doctypesToSkip(Set eligible, SyncDetailsResponse manifest) { + final skip = {}; + for (final dt in eligible) { + final e = manifest.entries[dt]; + if (e != null && !e.changed && !e.metaBumped) skip.add(dt); + } + return skip; +} diff --git a/lib/src/utils/sdk_log.dart b/lib/src/utils/sdk_log.dart new file mode 100644 index 0000000..cd86a09 --- /dev/null +++ b/lib/src/utils/sdk_log.dart @@ -0,0 +1,16 @@ +// Copyright (c) 2026, Bhushan Barbuddhe and contributors +// Best-effort diagnostic logging for the SDK. Stripped in release builds. + +import 'package:flutter/foundation.dart'; + +/// Logs a best-effort diagnostic message — typically from a catch block whose +/// failure is non-fatal and must not be silent during development. +/// +/// Replaces the scattered raw-stdout calls and their lint suppressors that +/// accumulated across the SDK. Routes through [debugPrint] (throttled and +/// lint-clean, so no per-call suppressor is needed) and compiles out entirely +/// in release via the [kDebugMode] guard. +void sdkLog(String message) { + if (!kDebugMode) return; + debugPrint(message); +} diff --git a/test/api/doctype_service_sync_details_test.dart b/test/api/doctype_service_sync_details_test.dart new file mode 100644 index 0000000..e22cd4a --- /dev/null +++ b/test/api/doctype_service_sync_details_test.dart @@ -0,0 +1,69 @@ +import 'dart:convert'; + +import 'package:flutter_test/flutter_test.dart'; +import 'package:http/http.dart' as http; +import 'package:http/testing.dart'; +import 'package:frappe_mobile_sdk/src/api/doctype_service.dart'; +import 'package:frappe_mobile_sdk/src/api/rest_helper.dart'; +import 'package:frappe_mobile_sdk/src/sync/sync_details.dart'; + +http.Response _json(Object body, [int status = 200]) => + http.Response(jsonEncode(body), status); + +DoctypeService _svc(http.Client client) => + DoctypeService(RestHelper('http://x', client: client)); + +void main() { + test('returns null for empty input without a network call', () async { + var hit = false; + final svc = _svc( + MockClient((_) async { + hit = true; + return _json({'message': {}}); + }), + ); + final r = await svc.getSyncDetails(const []); + expect(r, isNull); + expect(hit, isFalse, reason: 'empty input must not call the network'); + }); + + test('posts the right body and parses a message-wrapped manifest', () async { + String? capturedPath; + Map? capturedBody; + final svc = _svc( + MockClient((req) async { + capturedPath = req.url.path; + capturedBody = jsonDecode(req.body) as Map; + return _json({ + 'message': { + 'doctypes': [ + { + 'doctype': 'Patient', + 'changed': true, + 'count': 2, + 'meta_bumped': false, + }, + ], + 'delete_signals': 0, + }, + }); + }), + ); + final r = await svc.getSyncDetails([ + {'doctype': 'Patient', 'since': '2026-01-01 00:00:00.000000'}, + ]); + expect(capturedPath, '/api/method/mobile_sync.sync_details'); + expect((capturedBody!['doctypes'] as List).first['doctype'], 'Patient'); + expect(r, isA()); + expect(r!.entries['Patient']!.changed, isTrue); + expect(r.entries['Patient']!.count, 2); + }); + + test('returns null on transport error (graceful fallback)', () async { + final svc = _svc(MockClient((_) async => _json({'error': 'boom'}, 500))); + final r = await svc.getSyncDetails([ + {'doctype': 'Patient', 'since': '2026-01-01 00:00:00.000000'}, + ]); + expect(r, isNull); + }); +} diff --git a/test/sdk/frappe_sdk_sync_details_test.dart b/test/sdk/frappe_sdk_sync_details_test.dart new file mode 100644 index 0000000..2e11a16 --- /dev/null +++ b/test/sdk/frappe_sdk_sync_details_test.dart @@ -0,0 +1,143 @@ +import 'dart:convert'; + +import 'package:flutter_test/flutter_test.dart'; +import 'package:http/http.dart' as http; +import 'package:http/testing.dart'; +import 'package:frappe_mobile_sdk/src/concurrency/concurrency_pool.dart'; +import 'package:frappe_mobile_sdk/src/database/app_database.dart'; +import 'package:frappe_mobile_sdk/src/database/daos/doctype_meta_dao.dart'; +import 'package:frappe_mobile_sdk/src/database/daos/outbox_dao.dart'; +import 'package:frappe_mobile_sdk/src/models/doc_field.dart'; +import 'package:frappe_mobile_sdk/src/models/doc_type_meta.dart'; +import 'package:frappe_mobile_sdk/src/models/mobile_form_name.dart'; +import 'package:frappe_mobile_sdk/src/models/offline_mode.dart'; +import 'package:frappe_mobile_sdk/src/sdk/frappe_sdk.dart'; +import 'package:frappe_mobile_sdk/src/sync/pull_engine.dart'; +import 'package:frappe_mobile_sdk/src/sync/pull_page_fetcher.dart'; +import 'package:frappe_mobile_sdk/src/sync/sync_state_notifier.dart'; +import 'package:sqflite_common_ffi/sqflite_ffi.dart'; + +void main() { + setUpAll(() { + sqfliteFfiInit(); + databaseFactory = databaseFactoryFfi; + }); + + const doctype = 'Customer'; + + // Builds an SDK with: Customer registered as a mobile form, a meta on file, + // a COMPLETE pull cursor (so it is eligible for the pre-flight), and a + // PullEngine whose fetcher records which doctypes it actually fetched. + Future<({FrappeSDK sdk, List fetched})> buildSdk( + http.Client httpClient, + ) async { + final appDb = await AppDatabase.inMemoryDatabase(); + addTearDown(() => appDb.close()); + + final meta = DocTypeMeta( + name: doctype, + isTable: false, + fields: [ + DocField(fieldname: 'customer_name', fieldtype: 'Data', label: 'Name'), + ], + ); + await appDb.doctypeMetaDao.upsertMetaJson( + doctype, + jsonEncode(meta.toJson()), + ); + // Complete (delta-ready) cursor → eligible for the /sync_details pre-flight. + await appDb.doctypeMetaDao.setLastOkCursor( + doctype, + jsonEncode({ + 'modified': '2026-01-01 00:00:00.000000', + 'name': 'C-1', + 'complete': true, + }), + ); + + final sdk = FrappeSDK.forTesting( + 'http://localhost', + appDb, + offlineMode: const OfflineMode(enabled: true, isPersisted: true), + httpClient: httpClient, + ); + await sdk.meta.updateMobileFormDoctypesForTest([ + const MobileFormName(mobileDoctype: doctype), + ]); + + final fetched = []; + final engine = PullEngine( + db: appDb.rawDatabase, + metaDao: DoctypeMetaDao(appDb.rawDatabase), + outboxDao: OutboxDao(appDb.rawDatabase), + pool: ConcurrencyPool(maxConcurrent: 1), + fetcher: PullPageFetcher( + listHttp: (dt, _) async { + fetched.add(dt); + return const >[]; + }, + ), + pageSize: 100, + notifier: SyncStateNotifier(), + metaResolver: (_) async => meta, + ); + sdk.injectPullEngineForTesting(engine); + sdk.overrideIsOnlineForTesting(() async => true); + return (sdk: sdk, fetched: fetched); + } + + http.Response jsonResp(Object body) => http.Response(jsonEncode(body), 200); + + test('skips pulling a doctype the manifest reports unchanged', () async { + final built = await buildSdk( + MockClient((req) async { + if (req.url.path.contains('sync_details')) { + return jsonResp({ + 'message': { + 'doctypes': [ + { + 'doctype': doctype, + 'changed': false, + 'count': 0, + 'meta_bumped': false, + }, + ], + 'delete_signals': 0, + }, + }); + } + return jsonResp({'message': []}); + }), + ); + + await built.sdk.runUpgradeClosurePullForTesting(); + + expect( + built.fetched, + isEmpty, + reason: 'unchanged doctype must be skipped — its fetcher is never hit', + ); + }); + + test('falls back to pulling when the manifest call fails', () async { + final built = await buildSdk( + MockClient((req) async { + if (req.url.path.contains('sync_details')) { + return http.Response( + 'boom', + 500, + ); // manifest fails → null → full pull + } + return jsonResp({'message': []}); + }), + ); + + await built.sdk.runUpgradeClosurePullForTesting(); + + expect( + built.fetched, + contains(doctype), + reason: 'on manifest failure the eligible doctype must still be pulled', + ); + }); +} diff --git a/test/services/sync_controller_sync_mode_test.dart b/test/services/sync_controller_sync_mode_test.dart new file mode 100644 index 0000000..bb45025 --- /dev/null +++ b/test/services/sync_controller_sync_mode_test.dart @@ -0,0 +1,72 @@ +import 'package:flutter_test/flutter_test.dart'; +import 'package:frappe_mobile_sdk/src/database/daos/outbox_dao.dart'; +import 'package:frappe_mobile_sdk/src/database/schema/system_tables.dart'; +import 'package:frappe_mobile_sdk/src/services/sync_controller.dart'; +import 'package:frappe_mobile_sdk/src/sync/sync_state_notifier.dart'; +import 'package:sqflite_common_ffi/sqflite_ffi.dart'; + +void main() { + setUpAll(() { + sqfliteFfiInit(); + databaseFactory = databaseFactoryFfi; + }); + + late Database db; + late OutboxDao outbox; + setUp(() async { + db = await databaseFactory.openDatabase(inMemoryDatabasePath); + for (final s in systemTablesDDL()) { + await db.execute(s); + } + outbox = OutboxDao(db); + }); + tearDown(() async => db.close()); + + Future> noopFetch(String dt, String name) async => + {}; + Future noopApply(String dt, Map doc) async {} + + ({SyncController ctrl, List calls}) build() { + final calls = []; + final ctrl = SyncController( + outboxDao: outbox, + notifier: SyncStateNotifier(), + runPull: () async { + calls.add('pull'); + return {'Deferred Doctype'}; + }, + runPullForDoctypes: (d) async => calls.add('repull:${d.join(",")}'), + runPush: () async => calls.add('push'), + fetchSingleDoc: noopFetch, + applySingleDoc: noopApply, + ); + return (ctrl: ctrl, calls: calls); + } + + test('full (default) → pull, push, then re-pull deferred set', () async { + final b = build(); + await b.ctrl.syncNow(); + expect(b.calls, ['pull', 'push', 'repull:Deferred Doctype']); + }); + + test('pushOnly → only push, never pull', () async { + final b = build(); + await b.ctrl.syncNow(mode: SyncMode.pushOnly); + expect(b.calls, ['push']); + }); + + test('pullOnly → only pull, never push, no deferred re-pull', () async { + final b = build(); + await b.ctrl.syncNow(mode: SyncMode.pullOnly); + expect(b.calls, ['pull']); + }); + + test('all modes no-op while paused', () async { + final b = build(); + await b.ctrl.pause(); + await b.ctrl.syncNow(mode: SyncMode.full); + await b.ctrl.syncNow(mode: SyncMode.pushOnly); + await b.ctrl.syncNow(mode: SyncMode.pullOnly); + expect(b.calls, isEmpty); + }); +} diff --git a/test/services/sync_service_active_push_defer_test.dart b/test/services/sync_service_active_push_defer_test.dart new file mode 100644 index 0000000..a12f402 --- /dev/null +++ b/test/services/sync_service_active_push_defer_test.dart @@ -0,0 +1,71 @@ +import 'dart:convert'; + +import 'package:flutter_test/flutter_test.dart'; +import 'package:frappe_mobile_sdk/src/api/client.dart'; +import 'package:frappe_mobile_sdk/src/database/app_database.dart'; +import 'package:frappe_mobile_sdk/src/models/doc_field.dart'; +import 'package:frappe_mobile_sdk/src/models/doc_type_meta.dart'; +import 'package:frappe_mobile_sdk/src/services/offline_repository.dart'; +import 'package:frappe_mobile_sdk/src/services/sync_service.dart'; +import 'package:sqflite_common_ffi/sqflite_ffi.dart'; + +void main() { + setUpAll(() { + sqfliteFfiInit(); + databaseFactory = databaseFactoryFfi; + }); + + late AppDatabase db; + + setUp(() async { + db = await AppDatabase.inMemoryDatabase(); + final meta = DocTypeMeta( + name: 'Patient', + isTable: false, + fields: [DocField(fieldname: 'foo', fieldtype: 'Data', label: 'Foo')], + ); + await db.doctypeMetaDao.upsertMetaJson( + 'Patient', + jsonEncode(meta.toJson()), + ); + }); + + tearDown(() async => await db.close()); + + SyncService build({required Future Function(String)? hasActivePush}) { + final client = FrappeClient('http://localhost'); + final repo = OfflineRepository(db, client: client); + return SyncService(client, repo, db, hasActivePush: hasActivePush); + } + + test('defers the pull when a push is active for the doctype', () async { + final sync = build(hasActivePush: (dt) async => true); + final result = await sync.pullOneInternalForTest(doctype: 'Patient'); + expect(result.status, SyncStatus.deferredActivePush); + expect(result.total, 0); + }); + + test('does NOT defer when no push is active (proceeds to pull)', () async { + var consulted = false; + final sync = build( + hasActivePush: (dt) async { + consulted = true; + return false; + }, + ); + // With no active push the guard does not short-circuit, so the pull + // proceeds to the network. `_pullOneInternal` does not swallow network + // errors (its callers do), so the unreachable localhost surfaces as a + // throw — proving the call got PAST the defer rather than returning + // `deferredActivePush`. + await expectLater( + sync.pullOneInternalForTest(doctype: 'Patient'), + throwsA(anything), + ); + expect( + consulted, + isTrue, + reason: 'the active-push guard must be consulted before pulling', + ); + }); +} diff --git a/test/sync/pull_apply_test.dart b/test/sync/pull_apply_test.dart index 5fd53bc..66c3955 100644 --- a/test/sync/pull_apply_test.dart +++ b/test/sync/pull_apply_test.dart @@ -685,4 +685,80 @@ void main() { ); }, ); + + test('pull matches existing local row by mobile_uuid when server_name not ' + 'yet stamped — no duplicate parent row', () async { + // Local row exists from an in-flight INSERT: mobile_uuid set, server_name + // still NULL (writeback hasn't run yet), sync_status synced. + await db.insert('docs__sales_order', { + 'mobile_uuid': 'uuid-A', + 'server_name': null, + 'sync_status': 'synced', + 'local_modified': 0, + 'customer': 'Old Cust', + }); + + // Server returns the same doc: it now HAS a server_name, and carries the + // same mobile_uuid the device assigned. + await PullApply.applyPage( + db: db, + parentMeta: parentMeta, + parentTable: 'docs__sales_order', + childMetasByFieldname: const {}, + rows: [ + { + 'name': 'SO-001', + 'mobile_uuid': 'uuid-A', + 'modified': '2026-02-01 10:00:00.000000', + 'customer': 'New Cust', + }, + ], + ); + + final all = await db.query('docs__sales_order'); + expect(all.length, 1, reason: 'must update in place, not insert a dup'); + expect(all.first['mobile_uuid'], 'uuid-A'); + expect(all.first['server_name'], 'SO-001'); + expect(all.first['customer'], 'New Cust'); + }); + + test('pull reconciles (no conflict) when matched by mobile_uuid and local ' + 'server_name is NULL — our own INSERT round-tripping', () async { + // Local row is locally-dirty (push pending) with NO server_name yet. + await db.insert('docs__sales_order', { + 'mobile_uuid': 'uuid-B', + 'server_name': null, + 'sync_status': 'dirty', + 'local_modified': 0, + 'modified': '2026-01-01 09:00:00.000000', + 'customer': 'Draft', + }); + + await PullApply.applyPage( + db: db, + parentMeta: parentMeta, + parentTable: 'docs__sales_order', + childMetasByFieldname: const {}, + rows: [ + { + 'name': 'SO-002', + 'mobile_uuid': 'uuid-B', + 'modified': '2026-02-01 10:00:00.000000', + 'customer': 'Draft', + }, + ], + ); + + final row = (await db.query( + 'docs__sales_order', + where: 'mobile_uuid = ?', + whereArgs: ['uuid-B'], + )).single; + expect( + row['sync_status'], + 'synced', + reason: 'reconciled — our own INSERT came back, not a server edit', + ); + expect(row['server_name'], 'SO-002'); + }); } diff --git a/test/sync/pull_engine_test.dart b/test/sync/pull_engine_test.dart index 8f6ff7f..0669834 100644 --- a/test/sync/pull_engine_test.dart +++ b/test/sync/pull_engine_test.dart @@ -235,51 +235,65 @@ void main() { expect(called, isFalse); }); - test('does not advance cursor on mid-page failure', () async { - var page = 0; - final fetcher = PullPageFetcher( - listHttp: (doctype, params) async { - page++; - if (page == 1) { - return [ - {'name': 'C-1', 'modified': '2026-01-01', 'customer_name': 'A'}, - ]; - } - throw Exception('network'); - }, - ); - final closure = const ClosureResult( - doctypes: ['Customer'], - graph: { - 'Customer': DepGraph( - doctype: 'Customer', - tier: 0, - outgoing: [], - incoming: [], - ), - }, - childDoctypes: {}, - warnings: [], - ); - final engine = PullEngine( - db: db, - metaDao: metaDao, - outboxDao: OutboxDao(db), - pool: ConcurrencyPool(maxConcurrent: 2), - fetcher: fetcher, - pageSize: 500, - notifier: SyncStateNotifier(), - metaResolver: (dt) async => - DocTypeMeta(name: dt, fields: [f('customer_name', 'Data')]), - ); - await engine.run(closure); - final cursor = await metaDao.getLastOkCursor('Customer'); - expect( - cursor, - isNull, - reason: 'cursor must NOT advance when page 2 throws', - ); - }); + test( + 'mid-page failure leaves a resumable checkpoint (complete:false)', + () async { + var page = 0; + final fetcher = PullPageFetcher( + listHttp: (doctype, params) async { + page++; + if (page == 1) { + return [ + {'name': 'C-1', 'modified': '2026-01-01', 'customer_name': 'A'}, + ]; + } + throw Exception('network'); + }, + ); + final closure = const ClosureResult( + doctypes: ['Customer'], + graph: { + 'Customer': DepGraph( + doctype: 'Customer', + tier: 0, + outgoing: [], + incoming: [], + ), + }, + childDoctypes: {}, + warnings: [], + ); + final engine = PullEngine( + db: db, + metaDao: metaDao, + outboxDao: OutboxDao(db), + pool: ConcurrencyPool(maxConcurrent: 2), + fetcher: fetcher, + pageSize: 500, + notifier: SyncStateNotifier(), + metaResolver: (dt) async => + DocTypeMeta(name: dt, fields: [f('customer_name', 'Data')]), + ); + await engine.run(closure); + final cursor = await metaDao.getLastOkCursor('Customer'); + expect( + cursor, + isNotNull, + reason: 'page 1 succeeded — its progress must be a durable checkpoint', + ); + final parsed = jsonDecode(cursor!) as Map; + expect( + parsed['complete'], + isFalse, + reason: 'drain was interrupted — not yet incremental', + ); + expect( + parsed['name'], + 'C-1', + reason: 'checkpoint is positioned at the last applied row', + ); + }, + ); test('multiple doctypes drain in parallel via the pool', () async { // Add a second doctype. @@ -762,4 +776,87 @@ void main() { expect(rows.first['customer_name'], 'Gamma'); }, ); + + test( + 'resumes from the persisted offset after a crash (no re-download)', + () async { + const rows = [ + {'name': 'C-1', 'modified': '2026-01-01', 'customer_name': 'A'}, + {'name': 'C-2', 'modified': '2026-01-02', 'customer_name': 'B'}, + ]; + List> pageAt(int start) => + start >= rows.length ? const [] : [rows[start]]; + + const closure = ClosureResult( + doctypes: ['Customer'], + graph: { + 'Customer': DepGraph( + doctype: 'Customer', + tier: 0, + outgoing: [], + incoming: [], + ), + }, + childDoctypes: {}, + warnings: [], + ); + PullEngine engineWith(PullPageFetcher fetcher) => PullEngine( + db: db, + metaDao: metaDao, + outboxDao: OutboxDao(db), + pool: ConcurrencyPool(maxConcurrent: 2), + fetcher: fetcher, + pageSize: 1, + notifier: SyncStateNotifier(), + metaResolver: (dt) async => + DocTypeMeta(name: dt, fields: [f('customer_name', 'Data')]), + ); + + // Run 1: applies page at offset 0, then "crashes" on the next fetch. + var run1Calls = 0; + await engineWith( + PullPageFetcher( + listHttp: (doctype, params) async { + run1Calls++; + if (run1Calls >= 2) throw Exception('crash'); + return pageAt(params['limit_start'] as int); + }, + ), + ).run(closure); + + expect( + (await db.query('docs__customer')).length, + 1, + reason: 'only the first page was applied before the crash', + ); + + // Run 2: fresh engine, same DB. Must start from limit_start == 1. + final observedStarts = []; + await engineWith( + PullPageFetcher( + listHttp: (doctype, params) async { + final start = params['limit_start'] as int; + observedStarts.add(start); + return pageAt(start); + }, + ), + ).run(closure); + + expect( + observedStarts.first, + 1, + reason: + 'resume must continue from the persisted offset, not re-pull ' + 'page 0', + ); + expect((await db.query('docs__customer')).length, 2); + final parsed = + jsonDecode((await metaDao.getLastOkCursor('Customer'))!) as Map; + expect( + parsed['complete'], + isTrue, + reason: 'doctype fully drained on the second run', + ); + }, + ); } diff --git a/test/sync/sync_details_test.dart b/test/sync/sync_details_test.dart new file mode 100644 index 0000000..0f0f4a8 --- /dev/null +++ b/test/sync/sync_details_test.dart @@ -0,0 +1,74 @@ +import 'package:flutter_test/flutter_test.dart'; +import 'package:frappe_mobile_sdk/src/sync/sync_details.dart'; + +void main() { + group('SyncDetailsResponse.fromJson', () { + test('parses the frozen schema', () { + final r = SyncDetailsResponse.fromJson({ + 'doctypes': [ + { + 'doctype': 'Patient', + 'changed': true, + 'count': 14, + 'meta_bumped': false, + }, + { + 'doctype': 'Village', + 'changed': false, + 'count': 0, + 'meta_bumped': true, + }, + ], + 'delete_signals': 0, + }); + expect(r.entries.length, 2); + expect(r.entries['Patient']!.changed, isTrue); + expect(r.entries['Patient']!.count, 14); + expect(r.entries['Village']!.changed, isFalse); + expect(r.entries['Village']!.metaBumped, isTrue); + expect(r.deleteSignals, 0); + }); + + test('tolerates missing optional fields', () { + final r = SyncDetailsResponse.fromJson({ + 'doctypes': [ + {'doctype': 'X', 'changed': true}, + ], + }); + expect(r.entries['X']!.count, 0); + expect(r.entries['X']!.metaBumped, isFalse); + expect(r.deleteSignals, 0); + }); + }); + + group('doctypesToSkip', () { + final resp = SyncDetailsResponse.fromJson({ + 'doctypes': [ + { + 'doctype': 'Quiet', + 'changed': false, + 'count': 0, + 'meta_bumped': false, + }, + {'doctype': 'Busy', 'changed': true, 'count': 3, 'meta_bumped': false}, + { + 'doctype': 'MetaMoved', + 'changed': false, + 'count': 0, + 'meta_bumped': true, + }, + ], + }); + + test('skips only unchanged AND not-meta-bumped doctypes', () { + final skip = doctypesToSkip({'Quiet', 'Busy', 'MetaMoved'}, resp); + expect(skip, {'Quiet'}); + }); + + test('never skips a doctype absent from the manifest', () { + final skip = doctypesToSkip({'Quiet', 'NotInManifest'}, resp); + expect(skip.contains('NotInManifest'), isFalse); + expect(skip, {'Quiet'}); + }); + }); +} From f59af58091703c7ed8a67a9c5de1fe6f4fea4b15 Mon Sep 17 00:00:00 2001 From: omprakash Date: Thu, 4 Jun 2026 15:03:12 +0530 Subject: [PATCH 6/9] chore(logging): replace print() with sdkLog across services and UI Route stray print() diagnostics through sdkLog (a no-op in release builds) and drop the `// ignore: avoid_print` suppressions, importing sdk_log where needed. No behavioral change. --- lib/src/api/auth.dart | 4 +-- lib/src/screens/mobile_home_screen.dart | 30 ++++++++----------- lib/src/services/local_writer.dart | 7 ++--- lib/src/services/offline_repository.dart | 28 +++++++---------- .../services/offline_transition_service.dart | 10 +++---- lib/src/services/sync_engine_builder.dart | 7 ++--- lib/src/ui/doctype_list_screen.dart | 7 ++--- lib/src/ui/document_list_screen.dart | 13 ++++---- lib/src/ui/form_screen.dart | 16 ++++------ 9 files changed, 49 insertions(+), 73 deletions(-) diff --git a/lib/src/api/auth.dart b/lib/src/api/auth.dart index d379417..c309efa 100644 --- a/lib/src/api/auth.dart +++ b/lib/src/api/auth.dart @@ -2,6 +2,7 @@ // For license information, please see license.txt import 'rest_helper.dart'; +import '../utils/sdk_log.dart'; abstract class SessionStorage { Future saveSession(String sid); @@ -41,8 +42,7 @@ class AuthService { try { await _restHelper.post('/api/method/mobile_auth.logout'); } catch (e, st) { - // ignore: avoid_print - print( + sdkLog( 'AuthApi: server logout failed (continuing local cleanup) — $e\n$st', ); } finally { diff --git a/lib/src/screens/mobile_home_screen.dart b/lib/src/screens/mobile_home_screen.dart index 600f8a1..26b087c 100644 --- a/lib/src/screens/mobile_home_screen.dart +++ b/lib/src/screens/mobile_home_screen.dart @@ -6,6 +6,7 @@ import '../models/link_filter_result.dart'; import '../sdk/frappe_sdk.dart'; import '../ui/document_list_screen.dart'; import '../ui/widgets/form_builder.dart' show FieldChangeHandler, FormValidator; +import '../utils/sdk_log.dart'; /// Generic home screen that renders doctype groups from the SDK's Mobile Configuration. /// @@ -160,8 +161,7 @@ class _MobileHomeScreenState extends State counts[doctype] = results[0]; dirtyCounts[doctype] = results[1]; } catch (e, st) { - // ignore: avoid_print - print('MobileHomeScreen: count load for $doctype failed — $e\n$st'); + sdkLog('MobileHomeScreen: count load for $doctype failed — $e\n$st'); counts[doctype] = 0; dirtyCounts[doctype] = 0; } @@ -178,8 +178,7 @@ class _MobileHomeScreenState extends State errorCounts[r.doctype] = (errorCounts[r.doctype] ?? 0) + 1; } } catch (e, st) { - // ignore: avoid_print - print('MobileHomeScreen: pendingErrors load failed — $e\n$st'); + sdkLog('MobileHomeScreen: pendingErrors load failed — $e\n$st'); } } @@ -193,8 +192,7 @@ class _MobileHomeScreenState extends State }); } } catch (e, st) { - // ignore: avoid_print - print('MobileHomeScreen: _load failed — $e\n$st'); + sdkLog('MobileHomeScreen: _load failed — $e\n$st'); if (mounted) setState(() => _loading = false); } } @@ -210,16 +208,14 @@ class _MobileHomeScreenState extends State try { await widget.sdk.sync.pushSync(); } catch (e, st) { - // ignore: avoid_print - print('MobileHomeScreen: manual pushSync failed — $e\n$st'); + sdkLog('MobileHomeScreen: manual pushSync failed — $e\n$st'); } final doctypes = await widget.sdk.meta.getMobileFormDoctypeNames(); for (final dt in doctypes) { try { await widget.sdk.sync.pullSync(doctype: dt); } catch (e, st) { - // ignore: avoid_print - print('MobileHomeScreen: manual pullSync($dt) failed — $e\n$st'); + sdkLog('MobileHomeScreen: manual pullSync($dt) failed — $e\n$st'); } } } @@ -545,8 +541,7 @@ class _MobileHomeScreenState extends State try { await widget.sdk.logout(); } catch (e, st) { - // ignore: avoid_print - print('MobileHomeScreen: logout failed — $e\n$st'); + sdkLog('MobileHomeScreen: logout failed — $e\n$st'); if (mounted) { ScaffoldMessenger.of(context).showSnackBar( SnackBar( @@ -601,8 +596,7 @@ class _MobileHomeScreenState extends State try { await widget.sdk.forcePullAll(); } catch (e, st) { - // ignore: avoid_print - print('MobileHomeScreen: forcePullAll failed — $e\n$st'); + sdkLog('MobileHomeScreen: forcePullAll failed — $e\n$st'); if (mounted) { ScaffoldMessenger.of(context).showSnackBar( SnackBar( @@ -633,8 +627,9 @@ class _MobileHomeScreenState extends State try { await widget.sdk.sync.pullSync(doctype: doctype); } catch (e, st) { - // ignore: avoid_print - print('MobileHomeScreen: navigate pullSync($doctype) failed — $e\n$st'); + sdkLog( + 'MobileHomeScreen: navigate pullSync($doctype) failed — $e\n$st', + ); } if (mounted) { @@ -665,8 +660,7 @@ class _MobileHomeScreenState extends State _load(); } } catch (e, st) { - // ignore: avoid_print - print('MobileHomeScreen: _navigateToDoctype failed — $e\n$st'); + sdkLog('MobileHomeScreen: _navigateToDoctype failed — $e\n$st'); if (mounted) { ScaffoldMessenger.of(context).hideCurrentSnackBar(); ScaffoldMessenger.of(context).showSnackBar( diff --git a/lib/src/services/local_writer.dart b/lib/src/services/local_writer.dart index 19a78ef..a5bcdbf 100644 --- a/lib/src/services/local_writer.dart +++ b/lib/src/services/local_writer.dart @@ -9,6 +9,7 @@ import '../database/table_name.dart'; import '../models/doc_type_meta.dart'; import '../models/meta_resolver.dart'; import '../sync/child_table_info.dart'; +import '../utils/sdk_log.dart'; /// Writes a form-save payload to the per-doctype `docs__` parent /// table and `docs__` child tables in a single transaction. @@ -60,8 +61,7 @@ class LocalWriter { try { childMetasByDoctype[opt] = await _metaResolver(opt); } catch (e, st) { - // ignore: avoid_print - print( + sdkLog( 'LocalWriter.writeParent: child meta pre-resolve failed for ' '$opt — $e\n$st', ); @@ -144,8 +144,7 @@ class LocalWriter { try { cm = await _metaResolver(opt); } catch (e, st) { - // ignore: avoid_print - print( + sdkLog( 'LocalWriter.writeParentInTxn: child meta resolve failed for ' '$opt — $e\n$st', ); diff --git a/lib/src/services/offline_repository.dart b/lib/src/services/offline_repository.dart index 76497e6..9082f3c 100644 --- a/lib/src/services/offline_repository.dart +++ b/lib/src/services/offline_repository.dart @@ -21,6 +21,7 @@ import '../sync/payload_serializer.dart'; import '../sync/pull_apply.dart'; import 'local_writer.dart'; import 'meta_migration.dart'; +import '../utils/sdk_log.dart'; /// Repository for offline document operations class OfflineRepository { @@ -257,8 +258,7 @@ class OfflineRepository { serverName: serverName, ); } catch (e, st) { - // ignore: avoid_print - print( + sdkLog( 'OfflineRepository.reconcileServerSave: markSynced failed for ' '$doctype/$mobileUuid → $serverName — $e\n$st', ); @@ -269,8 +269,7 @@ class OfflineRepository { _database.rawDatabase, ).cancelPendingFor(doctype: doctype, mobileUuid: mobileUuid); } catch (e, st) { - // ignore: avoid_print - print( + sdkLog( 'OfflineRepository.reconcileServerSave: outbox cancelPendingFor ' 'failed for $doctype/$mobileUuid — $e\n$st', ); @@ -345,8 +344,7 @@ class OfflineRepository { ); doctypes = rows.map((r) => r['doctype'] as String).toList(); } on DatabaseException catch (e, st) { - // ignore: avoid_print - print( + sdkLog( 'OfflineRepository.getDirtyDocuments: doctype_meta scan failed ' '— $e\n$st', ); @@ -368,8 +366,7 @@ class OfflineRepository { out.add(Document.fromResolverRow(dt, r)); } } on DatabaseException catch (e, st) { - // ignore: avoid_print - print( + sdkLog( 'OfflineRepository.getDirtyDocuments: query failed for $dt ' '— $e\n$st', ); @@ -448,8 +445,7 @@ class OfflineRepository { existing = rows.isEmpty ? null : rows.first; } on DatabaseException catch (e, st) { // Per-doctype table not provisioned yet — proceed with INSERT. - // ignore: avoid_print - print( + sdkLog( 'OfflineRepository.saveDocument: existing-row probe failed for ' '$doctype/$mobileUuid (table likely missing) — $e\n$st', ); @@ -555,8 +551,7 @@ class OfflineRepository { try { await db.delete(table, where: where, whereArgs: [mobileUuid]); } on DatabaseException catch (e, st) { - // ignore: avoid_print - print( + sdkLog( 'OfflineRepository.hardDeleteLocalMirror: $table cleanup failed ' 'for $doctype/$mobileUuid (best-effort) — $e\n$st', ); @@ -651,8 +646,7 @@ class OfflineRepository { // success — PR#36 round-4 H8). Only a benignly absent child // table is skipped. if (!_isBenignSchemaAbsence(e)) rethrow; - // ignore: avoid_print - print( + sdkLog( 'OfflineRepository.deleteDocument: child table absent for ' '$childDoctype (stale schema) — skipping cascade. $e\n$st', ); @@ -664,8 +658,7 @@ class OfflineRepository { // delete back; only a benignly absent parent table is skipped. // (A child cascade rethrow also lands here — propagate it.) if (!_isBenignSchemaAbsence(e)) rethrow; - // ignore: avoid_print - print( + sdkLog( 'OfflineRepository.deleteDocument: parent table absent for ' '$doctype/$mobileUuid (stale schema) — $e\n$st', ); @@ -691,8 +684,7 @@ class OfflineRepository { whereArgs: [mobileUuid], ); } on DatabaseException catch (e, st) { - // ignore: avoid_print - print( + sdkLog( 'OfflineRepository.deleteDocument: tombstone update failed for ' '$doctype/$mobileUuid — $e\n$st', ); diff --git a/lib/src/services/offline_transition_service.dart b/lib/src/services/offline_transition_service.dart index 133157b..1639dc9 100644 --- a/lib/src/services/offline_transition_service.dart +++ b/lib/src/services/offline_transition_service.dart @@ -2,6 +2,7 @@ import 'dart:async'; import '../database/app_database.dart'; import 'sync_service.dart'; +import '../utils/sdk_log.dart'; /// Sealed hierarchy of states emitted on the transition stream. sealed class OfflineTransitionState { @@ -123,9 +124,9 @@ class OfflineTransitionService { } } catch (e, st) { // Probe is best-effort — the drain itself is authoritative. - // Log so a misconfigured residueCounter doesn't fail silently. - // ignore: avoid_print - print('OfflineTransitionService: progress probe failed — $e\n$st'); + // Logged in debug builds so a misconfigured residueCounter is + // visible during development (sdkLog is a no-op in release). + sdkLog('OfflineTransitionService: progress probe failed — $e\n$st'); } finally { probeInFlight = false; } @@ -136,8 +137,7 @@ class OfflineTransitionService { throw TimeoutException('Drain timed out after $drainTimeout'), ); } catch (e, st) { - // ignore: avoid_print - print('OfflineTransitionService: pushSync drain failed — $e\n$st'); + sdkLog('OfflineTransitionService: pushSync drain failed — $e\n$st'); lastError = e.toString(); } finally { progressTimer?.cancel(); diff --git a/lib/src/services/sync_engine_builder.dart b/lib/src/services/sync_engine_builder.dart index 86d9d47..7c931cd 100644 --- a/lib/src/services/sync_engine_builder.dart +++ b/lib/src/services/sync_engine_builder.dart @@ -21,6 +21,7 @@ import '../sync/push_engine.dart'; import '../sync/push_error.dart'; import '../sync/sync_state_notifier.dart'; import 'sync_controller.dart'; +import '../utils/sdk_log.dart'; /// True when [e] is Frappe's `QueryDeadlockError` — MySQL/MariaDB error /// 1213 ("Deadlock found when trying to get lock; try restarting @@ -159,8 +160,7 @@ class SyncEngineBuilder { } return null; } catch (e, st) { - // ignore: avoid_print - print( + sdkLog( 'SyncEngineBuilder.serverLookupByUuid: lookup failed for ' '$doctype/$mobileUuid — $e\n$st', ); @@ -201,8 +201,7 @@ class SyncEngineBuilder { final idempotencyStrategy = IdempotencyStrategy( serverHasDedupHook: serverHasDedupHook, onInitWarning: (msg) { - // ignore: avoid_print - print('IdempotencyStrategy: $msg'); + sdkLog('IdempotencyStrategy: $msg'); }, ); diff --git a/lib/src/ui/doctype_list_screen.dart b/lib/src/ui/doctype_list_screen.dart index 750a4cc..a322272 100644 --- a/lib/src/ui/doctype_list_screen.dart +++ b/lib/src/ui/doctype_list_screen.dart @@ -2,6 +2,7 @@ import 'package:flutter/material.dart'; import '../models/app_config.dart'; import '../query/unified_resolver.dart'; import '../services/offline_repository.dart'; +import '../utils/sdk_log.dart'; enum HomeScreenLayout { list, folder } @@ -74,14 +75,12 @@ class _DoctypeListScreenState extends State { try { _documentCounts[doctype] = await widget.resolver.count(doctype); } catch (e, st) { - // ignore: avoid_print - print('DoctypeListScreen: count($doctype) failed — $e\n$st'); + sdkLog('DoctypeListScreen: count($doctype) failed — $e\n$st'); _documentCounts[doctype] = 0; } } } catch (e, st) { - // ignore: avoid_print - print('DoctypeListScreen: _loadDocumentCounts failed — $e\n$st'); + sdkLog('DoctypeListScreen: _loadDocumentCounts failed — $e\n$st'); } finally { setState(() { _isLoading = false; diff --git a/lib/src/ui/document_list_screen.dart b/lib/src/ui/document_list_screen.dart index 704e99e..f19db51 100644 --- a/lib/src/ui/document_list_screen.dart +++ b/lib/src/ui/document_list_screen.dart @@ -24,6 +24,7 @@ import 'widgets/form_builder.dart' FieldChangeHandler, FormValidator; import 'widgets/sync_error_banner.dart' show humanizeOutboxError; +import '../utils/sdk_log.dart'; /// Layout variants for [DocumentListScreen]. enum DocumentListLayout { list, card } @@ -276,8 +277,7 @@ class _DocumentListScreenState extends State { try { await widget.syncService.pullSync(doctype: widget.doctype); } catch (e, st) { - // ignore: avoid_print - print( + sdkLog( 'DocumentListScreen: pullSync(${widget.doctype}) failed — $e\n$st', ); if (mounted) { @@ -294,8 +294,7 @@ class _DocumentListScreenState extends State { await _refreshErrorIndex(); if (mounted) setState(() => _documents = docs); } catch (e, st) { - // ignore: avoid_print - print('DocumentListScreen: _pullDocuments failed — $e\n$st'); + sdkLog('DocumentListScreen: _pullDocuments failed — $e\n$st'); if (mounted) { ScaffoldMessenger.of(context).showSnackBar( SnackBar( @@ -326,8 +325,7 @@ class _DocumentListScreenState extends State { _errorsByUuid.putIfAbsent(r.mobileUuid, () => []).add(r); } } catch (e, st) { - // ignore: avoid_print - print('DocumentListScreen: pendingErrors load failed — $e\n$st'); + sdkLog('DocumentListScreen: pendingErrors load failed — $e\n$st'); } } @@ -660,8 +658,7 @@ class _DocumentListScreenState extends State { ); } catch (e, st) { // Mirror is best-effort; the fresh data still drives the form. - // ignore: avoid_print - print( + sdkLog( 'document_list_screen: applyServerDocument mirror failed for ' '${widget.doctype}/${doc.serverId} — $e\n$st', ); diff --git a/lib/src/ui/form_screen.dart b/lib/src/ui/form_screen.dart index beb99ce..629d120 100644 --- a/lib/src/ui/form_screen.dart +++ b/lib/src/ui/form_screen.dart @@ -24,6 +24,7 @@ import 'widgets/form_builder.dart' OnButtonPressedCallback, FieldChangeHandler, FormValidator; +import '../utils/sdk_log.dart'; /// Visual customization for [FormScreen] action area. class FormScreenStyle { @@ -283,8 +284,7 @@ class _FormScreenState extends State with WidgetsBindingObserver { } catch (e, st) { // Banner is best-effort; a query failure should never block the // form from rendering. - // ignore: avoid_print - print('FormScreen: _loadSyncErrors failed — $e\n$st'); + sdkLog('FormScreen: _loadSyncErrors failed — $e\n$st'); } } @@ -294,8 +294,7 @@ class _FormScreenState extends State with WidgetsBindingObserver { try { await controller.retry(outboxId); } catch (e, st) { - // ignore: avoid_print - print('FormScreen: retry($outboxId) failed — $e\n$st'); + sdkLog('FormScreen: retry($outboxId) failed — $e\n$st'); if (mounted) { ScaffoldMessenger.of(context).showSnackBar( SnackBar( @@ -317,8 +316,7 @@ class _FormScreenState extends State with WidgetsBindingObserver { try { await svc.pushSync(doctype: widget.meta.name); } catch (e, st) { - // ignore: avoid_print - print('FormScreen: pushSync failed — $e\n$st'); + sdkLog('FormScreen: pushSync failed — $e\n$st'); if (mounted) { ScaffoldMessenger.of(context).showSnackBar( SnackBar( @@ -543,8 +541,7 @@ class _FormScreenState extends State with WidgetsBindingObserver { ); if (row != null) return row; } catch (e, st) { - // ignore: avoid_print - print( + sdkLog( 'FormScreen: getRowFromPerDoctypeTable($linkedDoctype, $docName) ' 'failed — $e\n$st', ); @@ -561,8 +558,7 @@ class _FormScreenState extends State with WidgetsBindingObserver { try { return await widget.api!.doctype.getByName(linkedDoctype, docName); } catch (e, st) { - // ignore: avoid_print - print( + sdkLog( 'FormScreen: api.doctype.getByName($linkedDoctype, $docName) ' 'failed — $e\n$st', ); From c7fbcf0cae5469a995176f9949a4f6bb53fb1c80 Mon Sep 17 00:00:00 2001 From: omprakash Date: Mon, 8 Jun 2026 17:29:39 +0530 Subject: [PATCH 7/9] fix(sync): drop SyncStateNotifier writes after close FrappeSDK.dispose() closes the shared notifier, but an in-flight PushEngine/PullEngine/SyncController (all wired to the same instance by SyncEngineBuilder) can resume from an await and write notifier.value afterwards, hitting StreamController.add on a closed controller and throwing StateError during logout/shutdown. dispose() only drained _pendingDrain, not controller-driven syncs. Guard the setter with an isClosed check so a late write from any writer is a safe no-op. Update the notifier test to the new no-throw contract and add a regression test for the teardown race. --- lib/src/sync/sync_state_notifier.dart | 7 ++++ test/sync/sync_state_notifier_test.dart | 55 ++++++++++++++++--------- 2 files changed, 43 insertions(+), 19 deletions(-) diff --git a/lib/src/sync/sync_state_notifier.dart b/lib/src/sync/sync_state_notifier.dart index 5acaa42..3729af0 100644 --- a/lib/src/sync/sync_state_notifier.dart +++ b/lib/src/sync/sync_state_notifier.dart @@ -17,6 +17,13 @@ class SyncStateNotifier { /// trees subscribed via `StreamBuilder` — do not rebuild when a tick of /// engine bookkeeping produces an identical snapshot. set value(SyncState next) { + // Drop writes that race teardown. FrappeSDK.dispose() closes this notifier, + // but an in-flight engine (PushEngine/PullEngine/SyncController all share + // this instance via SyncEngineBuilder) may be suspended at an await and + // resume to write a final state tick afterwards. Without this guard that + // write hits `_controller.add` on a closed broadcast controller and throws + // StateError, crashing the unawaited sync future during logout/shutdown. + if (_controller.isClosed) return; if (_value == next) return; _value = next; _controller.add(next); diff --git a/test/sync/sync_state_notifier_test.dart b/test/sync/sync_state_notifier_test.dart index df97b48..2ca76ae 100644 --- a/test/sync/sync_state_notifier_test.dart +++ b/test/sync/sync_state_notifier_test.dart @@ -181,26 +181,43 @@ void main() { }); }); - test( - 'close prevents further emissions but value getter still works', - () async { - final n = SyncStateNotifier(); - final emitted = []; - final sub = n.stream.listen(emitted.add); + group('write-after-close (lifecycle teardown safety)', () { + test( + 'close stops emissions; the value getter still returns the last value', + () async { + final n = SyncStateNotifier(); + final emitted = []; + final sub = n.stream.listen(emitted.add); - n.value = n.value.copyWith(isOnline: true); - await n.close(); + n.value = n.value.copyWith(isOnline: true); + await n.close(); - // After close, the broadcast stream is done — no further events. - // Subsequent assignments throw because StreamController.add on a closed - // controller throws StateError. - expect( - () => n.value = n.value.copyWith(isOnline: false), - throwsStateError, - ); + await sub.cancel(); + expect(emitted, hasLength(1)); + expect(n.value.isOnline, isTrue); + }, + ); - await sub.cancel(); - expect(emitted, hasLength(1)); - }, - ); + test( + 'a late writer racing teardown is a silent no-op, not a StateError', + () async { + final n = SyncStateNotifier(); + n.value = n.value.copyWith(isOnline: true); + await n.close(); + + // FrappeSDK.dispose() closes this notifier, but an in-flight + // PushEngine/PullEngine holds the same instance (SyncEngineBuilder + // wires one notifier into every engine) and writes value per page + // (pull_engine.dart:234). If that write is suspended at an await when + // dispose() closes the notifier, the resumed write must not crash the + // process with a StateError on the closed broadcast controller. + expect( + () => n.value = n.value.copyWith(isOnline: false), + returnsNormally, + ); + // The dropped write must not mutate observable state mid-teardown. + expect(n.value.isOnline, isTrue); + }, + ); + }); } From 01376ab22ff8f23bf55a7226ba7f860841320376 Mon Sep 17 00:00:00 2001 From: omprakash Date: Mon, 8 Jun 2026 17:31:08 +0530 Subject: [PATCH 8/9] refactor(ui): extract FieldNormalizer from FrappeFormBuilder Move the per-fieldtype normalization switch and the multiselect helper out of the FrappeFormBuilder widget into a pure, headless FieldNormalizer so the rules can be unit-tested directly. Behavior unchanged; the widget delegates at both call sites. Adds 22 characterization tests covering every branch. --- lib/src/ui/widgets/form_builder.dart | 106 +------------------- lib/src/utils/field_normalizer.dart | 117 ++++++++++++++++++++++ test/utils/field_normalizer_test.dart | 139 ++++++++++++++++++++++++++ 3 files changed, 259 insertions(+), 103 deletions(-) create mode 100644 lib/src/utils/field_normalizer.dart create mode 100644 test/utils/field_normalizer_test.dart diff --git a/lib/src/ui/widgets/form_builder.dart b/lib/src/ui/widgets/form_builder.dart index e036ab0..dbaa910 100644 --- a/lib/src/ui/widgets/form_builder.dart +++ b/lib/src/ui/widgets/form_builder.dart @@ -8,8 +8,8 @@ import '../../models/link_filter_result.dart'; import '../../constants/field_types.dart'; import '../../services/link_option_service.dart'; import '../../services/link_field_coordinator.dart'; -import '../../utils/date_helpers.dart'; import '../../utils/depends_on_evaluator.dart'; +import '../../utils/field_normalizer.dart'; import 'fields/field_factory.dart'; import 'fields/base_field.dart'; import 'default_form_style.dart'; @@ -369,106 +369,6 @@ class _FrappeFormBuilderState extends State }); } - List _normalizeMultiSelectPatchedValue(dynamic value) { - if (value == null) return []; - if (value is List) { - return value.map((item) => item.toString()).toList(); - } - final raw = value.toString(); - if (raw.isEmpty) return []; - return raw - .split(',') - .map((item) => item.trim()) - .where((item) => item.isNotEmpty) - .toList(); - } - - dynamic _normalizePatchedValue(DocField field, dynamic value) { - switch (field.fieldtype) { - case FieldTypes.date: - case FieldTypes.datetime: - if (value == null || value == '') return null; - if (value is DateTime) return value; - if (value is String) return DateTime.tryParse(value); - return null; - - case FieldTypes.time: - if (value == null || value == '') return null; - if (value is DateTime) return value; - if (value is TimeOfDay) { - return DateTime(2000, 1, 1, value.hour, value.minute); - } - if (value is String) { - final parts = value.split(':'); - if (parts.length >= 2) { - final hour = int.tryParse(parts[0]); - final minute = int.tryParse(parts[1]); - if (hour != null && minute != null) { - return DateTime(2000, 1, 1, hour, minute); - } - } - } - return null; - - case FieldTypes.check: - if (value is bool) return value; - if (value is int) return value == 1; - if (value is String) { - final normalized = value.trim().toLowerCase(); - return normalized == '1' || normalized == 'true'; - } - return false; - - case FieldTypes.rating: - if (value == null || value == '') return null; - if (value is int) return value; - return int.tryParse(value.toString()); - - case FieldTypes.select: - if (field.options == null || field.options!.trim().isEmpty) { - return value?.toString() ?? ''; - } - if (field.allowMultiple) { - return _normalizeMultiSelectPatchedValue(value); - } - final stringValue = value?.toString(); - return (stringValue == null || stringValue.isEmpty) - ? null - : stringValue; - - case FieldTypes.link: - case FieldTypes.data: - case FieldTypes.text: - case FieldTypes.longText: - case FieldTypes.smallText: - case FieldTypes.password: - case FieldTypes.phone: - case FieldTypes.attach: - case FieldTypes.attachImage: - case FieldTypes.image: - case FieldTypes.readOnly: - return value?.toString() ?? ''; - - case FieldTypes.int: - case FieldTypes.float: - case FieldTypes.currency: - case FieldTypes.percent: - return value?.toString() ?? ''; - - case FieldTypes.duration: - if (value == null || value == '') return ''; - if (value is int) return formatDurationSeconds(value); - return value.toString(); - - case 'Table': - if (value is List) return value; - return []; - - default: - return value; - } - } - Map _normalizePatchValues(Map updates) { final normalized = {}; for (final entry in updates.entries) { @@ -480,7 +380,7 @@ class _FrappeFormBuilderState extends State normalized[entry.key] = entry.value ?? ''; continue; } - normalized[entry.key] = _normalizePatchedValue(fieldMeta, entry.value); + normalized[entry.key] = FieldNormalizer.normalize(fieldMeta, entry.value); } return normalized; } @@ -826,7 +726,7 @@ class _FrappeFormBuilderState extends State // Sync FormBuilder internal state (needed for programmatic updates e.g. auto-select) if (field.fieldname != null && oldValue != value) { _formKey.currentState?.patchValue({ - field.fieldname!: _normalizePatchedValue(field, value), + field.fieldname!: FieldNormalizer.normalize(field, value), }); } diff --git a/lib/src/utils/field_normalizer.dart b/lib/src/utils/field_normalizer.dart new file mode 100644 index 0000000..01afb7f --- /dev/null +++ b/lib/src/utils/field_normalizer.dart @@ -0,0 +1,117 @@ +import 'package:flutter/material.dart'; + +import '../constants/field_types.dart'; +import '../models/doc_field.dart'; +import 'date_helpers.dart'; + +/// Coerces a raw value into the canonical in-form representation for a given +/// [DocField], dispatched on its `fieldtype`. +/// +/// Pure logic with no widget state — extracted from `FrappeFormBuilder` so the +/// per-fieldtype normalization rules can be unit-tested directly instead of +/// only through the widget. +class FieldNormalizer { + const FieldNormalizer._(); + + /// Normalize [value] to the shape the form expects for [field]. + static dynamic normalize(DocField field, dynamic value) { + switch (field.fieldtype) { + case FieldTypes.date: + case FieldTypes.datetime: + if (value == null || value == '') return null; + if (value is DateTime) return value; + if (value is String) return DateTime.tryParse(value); + return null; + + case FieldTypes.time: + if (value == null || value == '') return null; + if (value is DateTime) return value; + if (value is TimeOfDay) { + return DateTime(2000, 1, 1, value.hour, value.minute); + } + if (value is String) { + final parts = value.split(':'); + if (parts.length >= 2) { + final hour = int.tryParse(parts[0]); + final minute = int.tryParse(parts[1]); + if (hour != null && minute != null) { + return DateTime(2000, 1, 1, hour, minute); + } + } + } + return null; + + case FieldTypes.check: + if (value is bool) return value; + if (value is int) return value == 1; + if (value is String) { + final normalized = value.trim().toLowerCase(); + return normalized == '1' || normalized == 'true'; + } + return false; + + case FieldTypes.rating: + if (value == null || value == '') return null; + if (value is int) return value; + return int.tryParse(value.toString()); + + case FieldTypes.select: + if (field.options == null || field.options!.trim().isEmpty) { + return value?.toString() ?? ''; + } + if (field.allowMultiple) { + return _normalizeMultiSelect(value); + } + final stringValue = value?.toString(); + return (stringValue == null || stringValue.isEmpty) + ? null + : stringValue; + + case FieldTypes.link: + case FieldTypes.data: + case FieldTypes.text: + case FieldTypes.longText: + case FieldTypes.smallText: + case FieldTypes.password: + case FieldTypes.phone: + case FieldTypes.attach: + case FieldTypes.attachImage: + case FieldTypes.image: + case FieldTypes.readOnly: + return value?.toString() ?? ''; + + case FieldTypes.int: + case FieldTypes.float: + case FieldTypes.currency: + case FieldTypes.percent: + return value?.toString() ?? ''; + + case FieldTypes.duration: + if (value == null || value == '') return ''; + if (value is int) return formatDurationSeconds(value); + return value.toString(); + + case 'Table': + if (value is List) return value; + return []; + + default: + return value; + } + } + + /// Split/normalize a Select-MultiSelect-style value into a list of strings. + static List _normalizeMultiSelect(dynamic value) { + if (value == null) return []; + if (value is List) { + return value.map((item) => item.toString()).toList(); + } + final raw = value.toString(); + if (raw.isEmpty) return []; + return raw + .split(',') + .map((item) => item.trim()) + .where((item) => item.isNotEmpty) + .toList(); + } +} diff --git a/test/utils/field_normalizer_test.dart b/test/utils/field_normalizer_test.dart new file mode 100644 index 0000000..9083ca0 --- /dev/null +++ b/test/utils/field_normalizer_test.dart @@ -0,0 +1,139 @@ +import 'package:flutter/material.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:frappe_mobile_sdk/src/constants/field_types.dart'; +import 'package:frappe_mobile_sdk/src/models/doc_field.dart'; +import 'package:frappe_mobile_sdk/src/utils/date_helpers.dart'; +import 'package:frappe_mobile_sdk/src/utils/field_normalizer.dart'; + +DocField _f(String type, {String? options, bool allowMultiple = false}) => + DocField(fieldtype: type, options: options, allowMultiple: allowMultiple); + +void main() { + group('normalize — date / datetime', () { + test('null and empty string become null', () { + expect(FieldNormalizer.normalize(_f(FieldTypes.date), ''), isNull); + expect(FieldNormalizer.normalize(_f(FieldTypes.datetime), null), isNull); + }); + test('DateTime passes through unchanged', () { + final dt = DateTime(2026, 6, 8); + expect(FieldNormalizer.normalize(_f(FieldTypes.date), dt), same(dt)); + }); + test('ISO string parses to DateTime', () { + expect( + FieldNormalizer.normalize(_f(FieldTypes.datetime), '2026-06-08 10:30'), + DateTime.tryParse('2026-06-08 10:30'), + ); + }); + test('unparseable string becomes null', () { + expect( + FieldNormalizer.normalize(_f(FieldTypes.date), 'not-a-date'), + isNull, + ); + }); + }); + + group('normalize — time', () { + test('HH:mm string maps to DateTime(2000,1,1,h,m)', () { + expect( + FieldNormalizer.normalize(_f(FieldTypes.time), '14:30'), + DateTime(2000, 1, 1, 14, 30), + ); + }); + test('TimeOfDay maps to DateTime(2000,1,1,h,m)', () { + expect( + FieldNormalizer.normalize( + _f(FieldTypes.time), + const TimeOfDay(hour: 9, minute: 5), + ), + DateTime(2000, 1, 1, 9, 5), + ); + }); + test('malformed time becomes null', () { + expect(FieldNormalizer.normalize(_f(FieldTypes.time), 'noon'), isNull); + }); + }); + + group('normalize — check (bool coercion)', () { + test('bool passes through', () { + expect(FieldNormalizer.normalize(_f(FieldTypes.check), true), isTrue); + }); + test('int 1 is true, anything else false', () { + expect(FieldNormalizer.normalize(_f(FieldTypes.check), 1), isTrue); + expect(FieldNormalizer.normalize(_f(FieldTypes.check), 0), isFalse); + }); + test('"1" and "true" (case-insensitive) are true, others false', () { + expect(FieldNormalizer.normalize(_f(FieldTypes.check), 'TRUE'), isTrue); + expect(FieldNormalizer.normalize(_f(FieldTypes.check), '1'), isTrue); + expect(FieldNormalizer.normalize(_f(FieldTypes.check), 'no'), isFalse); + }); + test('unrecognized type coerces to false', () { + expect(FieldNormalizer.normalize(_f(FieldTypes.check), 3.5), isFalse); + }); + }); + + group('normalize — rating', () { + test('null/empty becomes null', () { + expect(FieldNormalizer.normalize(_f(FieldTypes.rating), ''), isNull); + }); + test('int passes through; numeric string parses', () { + expect(FieldNormalizer.normalize(_f(FieldTypes.rating), 4), 4); + expect(FieldNormalizer.normalize(_f(FieldTypes.rating), '3'), 3); + }); + }); + + group('normalize — select', () { + test('no options returns toString (empty for null)', () { + expect(FieldNormalizer.normalize(_f(FieldTypes.select), 'X'), 'X'); + expect(FieldNormalizer.normalize(_f(FieldTypes.select), null), ''); + }); + test('single select with options: string, or null when empty', () { + final f = _f(FieldTypes.select, options: 'A\nB'); + expect(FieldNormalizer.normalize(f, 'A'), 'A'); + expect(FieldNormalizer.normalize(f, ''), isNull); + }); + test('multi-select splits comma string and passes lists through', () { + final f = _f(FieldTypes.select, options: 'A\nB', allowMultiple: true); + expect(FieldNormalizer.normalize(f, 'A, B'), ['A', 'B']); + expect(FieldNormalizer.normalize(f, ['A', 'B']), ['A', 'B']); + expect(FieldNormalizer.normalize(f, null), []); + }); + }); + + group('normalize — text / link / numeric passthrough as string', () { + test('text-like returns toString (null -> empty)', () { + expect(FieldNormalizer.normalize(_f(FieldTypes.data), 'hi'), 'hi'); + expect(FieldNormalizer.normalize(_f(FieldTypes.link), null), ''); + }); + test('numeric fieldtypes return toString', () { + expect(FieldNormalizer.normalize(_f(FieldTypes.int), 42), '42'); + expect(FieldNormalizer.normalize(_f(FieldTypes.currency), null), ''); + }); + }); + + group('normalize — duration', () { + test('null/empty returns empty string', () { + expect(FieldNormalizer.normalize(_f(FieldTypes.duration), ''), ''); + }); + test('int seconds formatted; non-int returns toString', () { + expect( + FieldNormalizer.normalize(_f(FieldTypes.duration), 90), + formatDurationSeconds(90), + ); + expect(FieldNormalizer.normalize(_f(FieldTypes.duration), '1m'), '1m'); + }); + }); + + group('normalize — Table / default', () { + test('Table: list passes through, non-list becomes empty list', () { + expect(FieldNormalizer.normalize(_f('Table'), [1, 2]), [1, 2]); + expect(FieldNormalizer.normalize(_f('Table'), 'x'), []); + }); + test('unknown fieldtype passes the value through unchanged', () { + final obj = Object(); + expect( + FieldNormalizer.normalize(_f(FieldTypes.geolocation), obj), + same(obj), + ); + }); + }); +} From 18fa4639cad77d5d5f840b13dcef29cfaa93932e Mon Sep 17 00:00:00 2001 From: omprakash Date: Mon, 8 Jun 2026 17:31:34 +0530 Subject: [PATCH 9/9] refactor(example): use SDK getters instead of rebuilding services _handleLoginSuccess and build() re-instantiated MetaService, OfflineRepository, SyncService and a hand-rolled UnifiedResolver that diverged from the SDK's own wiring. Those blocks only ran when initialize() failed, but build() returns the error screen first, so they were unreachable. Remove them; rely on sdk.meta/repository/sync/ linkOptions with a "not initialized" guard. --- example/lib/main.dart | 74 ++++++++----------------------------------- 1 file changed, 13 insertions(+), 61 deletions(-) diff --git a/example/lib/main.dart b/example/lib/main.dart index 7519618..69303c9 100644 --- a/example/lib/main.dart +++ b/example/lib/main.dart @@ -247,34 +247,11 @@ class _HomeScreenState extends State { return; } - // Initialize services if not already done - _metaService ??= MetaService(_authService!.client!, _database!); - _repository ??= OfflineRepository(_database!); - _syncService ??= SyncService( - _authService!.client!, - _repository!, - _database!, - ); - if (_linkOptionService == null) { - final metaSvc = _metaService!; - final syncSvc = _syncService!; - final rawDb = _database!.rawDatabase; - Future metaFn(String dt) => metaSvc.getMeta(dt); - final resolver = UnifiedResolver( - db: rawDb, - metaDao: DoctypeMetaDao(rawDb), - isOnline: () => true, - backgroundFetch: (doctype, _) async { - try { - await syncSvc.pullSync(doctype: doctype); - } catch (_) {} - }, - metaResolver: metaFn, - ); - _linkOptionService = LinkOptionService(resolver, metaFn); - } - - // Initial metadata + data sync for mobile forms + // Services are wired once by sdk.initialize() in _initialize() and exposed + // via sdk.meta / sdk.repository / sdk.sync / sdk.linkOptions. Do NOT rebuild + // them here — a hand-rolled graph diverges from the SDK's wiring (offline + // mode, connectivity-aware resolver, push runner) and would run alongside + // the SDK's own instances. Just run the post-login data sync. await _initialMetaAndDataSync(); setState(() { @@ -388,43 +365,18 @@ class _HomeScreenState extends State { ); } - // Initialize services if not already done (should happen in _handleLoginSuccess, but double-check) + // Services are wired once by sdk.initialize() in _initialize(). If any is + // missing (initialize() failed), surface that rather than rebuilding a + // divergent service graph. if (_metaService == null || _repository == null || _syncService == null || _linkOptionService == null) { - if (_database != null && _authService!.client != null) { - _metaService = MetaService(_authService!.client!, _database!); - _repository = OfflineRepository(_database!); - _syncService = SyncService( - _authService!.client!, - _repository!, - _database!, - getMobileUuid: () => _authService!.getOrCreateMobileUuid(), - ); - final metaSvc = _metaService!; - final syncSvc = _syncService!; - final rawDb = _database!.rawDatabase; - Future metaFn(String dt) => metaSvc.getMeta(dt); - final resolver = UnifiedResolver( - db: rawDb, - metaDao: DoctypeMetaDao(rawDb), - isOnline: () => true, - backgroundFetch: (doctype, _) async { - try { - await syncSvc.pullSync(doctype: doctype); - } catch (_) {} - }, - metaResolver: metaFn, - ); - _linkOptionService = LinkOptionService(resolver, metaFn); - } else { - return const Scaffold( - body: Center( - child: Text('Services not initialized. Please restart the app.'), - ), - ); - } + return const Scaffold( + body: Center( + child: Text('Services not initialized. Please restart the app.'), + ), + ); } if (_appConfig == null) {