diff --git a/app/lib/data/database/app_database.dart b/app/lib/data/database/app_database.dart index b1f4fd6..a90238f 100644 --- a/app/lib/data/database/app_database.dart +++ b/app/lib/data/database/app_database.dart @@ -28,6 +28,7 @@ class AppDatabase { dbPath, version: Schema.version, onCreate: Schema.onCreate, + onUpgrade: Schema.onUpgrade, ); if (path == null) _instance = db; diff --git a/app/lib/data/database/schema.dart b/app/lib/data/database/schema.dart index 302684d..1aaf2a0 100644 --- a/app/lib/data/database/schema.dart +++ b/app/lib/data/database/schema.dart @@ -8,7 +8,10 @@ import 'package:sqflite/sqflite.dart'; /// Mirrors the Python schema in nexanote/storage/database.py. /// Keep this file as the single source of truth for table definitions. class Schema { - static const int version = 1; + /// Bumped to 2 when remote_id/remote_path columns were added so the + /// SyncService can map a local note to its canonical .md file on the + /// WebDAV/NAS without inventing a fresh row on every pull. + static const int version = 2; static const String _createNotebooks = ''' CREATE TABLE IF NOT EXISTS notebooks ( @@ -38,6 +41,8 @@ class Schema { is_archived INTEGER NOT NULL DEFAULT 0, is_deleted INTEGER NOT NULL DEFAULT 0, sync_status TEXT NOT NULL DEFAULT 'local_only', + remote_id TEXT, + remote_path TEXT, created_at TEXT NOT NULL, updated_at TEXT NOT NULL, FOREIGN KEY (notebook_id) REFERENCES notebooks(id) @@ -77,6 +82,12 @@ class Schema { static const String _indexNotesUpdated = 'CREATE INDEX IF NOT EXISTS idx_notes_updated ON notes(updated_at)'; + static const String _indexNotesRemoteId = + 'CREATE INDEX IF NOT EXISTS idx_notes_remote_id ON notes(remote_id)'; + + static const String _indexNotesRemotePath = + 'CREATE INDEX IF NOT EXISTS idx_notes_remote_path ON notes(remote_path)'; + static const String _indexStrokesNote = 'CREATE INDEX IF NOT EXISTS idx_strokes_note ON strokes(note_id)'; @@ -91,7 +102,25 @@ class Schema { await db.execute(_createStrokePoints); await db.execute(_indexNotesNotebook); await db.execute(_indexNotesUpdated); + await db.execute(_indexNotesRemoteId); + await db.execute(_indexNotesRemotePath); await db.execute(_indexStrokesNote); await db.execute(_indexStrokePointsStroke); } + + /// Forward-only migrations. Each `if` block applies the steps needed to + /// reach the next version; SQLite's ALTER TABLE ADD COLUMN is enough for + /// the nullable remote_id/remote_path additions in v2. + static Future onUpgrade( + Database db, + int oldVersion, + int newVersion, + ) async { + if (oldVersion < 2) { + await db.execute('ALTER TABLE notes ADD COLUMN remote_id TEXT'); + await db.execute('ALTER TABLE notes ADD COLUMN remote_path TEXT'); + await db.execute(_indexNotesRemoteId); + await db.execute(_indexNotesRemotePath); + } + } } diff --git a/app/lib/data/models/note.dart b/app/lib/data/models/note.dart index 806baf9..a92a7d9 100644 --- a/app/lib/data/models/note.dart +++ b/app/lib/data/models/note.dart @@ -9,6 +9,16 @@ import 'dart:convert'; /// [syncStatus] values: 'local_only' | 'synced' | 'modified' | 'conflict' /// [noteType] values: 'typed' | 'handwritten' | 'mixed' /// +/// [remoteId] is the stable identifier the WebDAV/NAS source of truth uses +/// for this note (the frontmatter `id` for notes with NexaNote frontmatter, +/// or the synthetic `md.` id for plain Markdown files dropped in by +/// the user). When set it lets the sync engine adopt the remote .md file +/// instead of creating a duplicate. +/// +/// [remotePath] is the relative path of the canonical .md file on the +/// remote (e.g. `notes/Hello World.md`). Stored so renames on disk can be +/// followed without losing the link. +/// /// Mirrors Note in nexanote/models/note.py. class Note { final String id; @@ -21,6 +31,8 @@ class Note { final bool isArchived; final bool isDeleted; final String syncStatus; + final String? remoteId; + final String? remotePath; final DateTime createdAt; final DateTime updatedAt; @@ -35,6 +47,8 @@ class Note { this.isArchived = false, this.isDeleted = false, this.syncStatus = 'local_only', + this.remoteId, + this.remotePath, required this.createdAt, required this.updatedAt, }); @@ -53,6 +67,8 @@ class Note { isArchived: ((map['is_archived'] as int?) ?? 0) == 1, isDeleted: ((map['is_deleted'] as int?) ?? 0) == 1, syncStatus: (map['sync_status'] as String?) ?? 'local_only', + remoteId: map['remote_id'] as String?, + remotePath: map['remote_path'] as String?, createdAt: DateTime.parse(map['created_at'] as String), updatedAt: DateTime.parse(map['updated_at'] as String), ); @@ -70,12 +86,16 @@ class Note { 'is_archived': isArchived ? 1 : 0, 'is_deleted': isDeleted ? 1 : 0, 'sync_status': syncStatus, + 'remote_id': remoteId, + 'remote_path': remotePath, 'created_at': createdAt.toIso8601String(), 'updated_at': updatedAt.toIso8601String(), }; } Note copyWith({ + String? notebookId, + bool clearNotebookId = false, String? title, String? noteType, List? tags, @@ -84,11 +104,14 @@ class Note { bool? isArchived, bool? isDeleted, String? syncStatus, + String? remoteId, + String? remotePath, DateTime? updatedAt, }) { return Note( id: id, - notebookId: notebookId, + notebookId: + clearNotebookId ? null : (notebookId ?? this.notebookId), title: title ?? this.title, noteType: noteType ?? this.noteType, tags: tags ?? this.tags, @@ -97,6 +120,8 @@ class Note { isArchived: isArchived ?? this.isArchived, isDeleted: isDeleted ?? this.isDeleted, syncStatus: syncStatus ?? this.syncStatus, + remoteId: remoteId ?? this.remoteId, + remotePath: remotePath ?? this.remotePath, createdAt: createdAt, updatedAt: updatedAt ?? this.updatedAt, ); diff --git a/app/lib/data/repositories/note_repository.dart b/app/lib/data/repositories/note_repository.dart index 417edf8..905baf8 100644 --- a/app/lib/data/repositories/note_repository.dart +++ b/app/lib/data/repositories/note_repository.dart @@ -109,6 +109,20 @@ class NoteRepository { return Note.fromMap(rows.first); } + /// Returns the local note linked to [remoteId], or null if no local row + /// has been adopted for that remote yet. Used by SyncService to decide + /// between adopting an existing local row and inserting a new one. + Future getNoteByRemoteId(String remoteId) async { + final rows = await _db.query( + 'notes', + where: 'remote_id = ?', + whereArgs: [remoteId], + limit: 1, + ); + if (rows.isEmpty) return null; + return Note.fromMap(rows.first); + } + /// Soft-deletes a note by setting [is_deleted = 1] and marking it modified. Future deleteNote(String id) async { final now = DateTime.now().toUtc().toIso8601String(); @@ -135,25 +149,38 @@ class NoteRepository { return rows.map(Note.fromMap).toList(); } - /// Replaces notebooks/notes with the supplied records in a single - /// transaction. Strokes and stroke_points are intentionally **not** - /// touched: handwritten ink is user content and Phase 4A sync is - /// metadata-only. Strokes whose parent note disappears are left as - /// orphans for a future stroke-aware sync phase to reconcile. - Future replaceAll({ - required List notebooks, - required List notes, - }) async { - await _db.transaction((txn) async { - await txn.delete('notes'); - await txn.delete('notebooks'); - for (final nb in notebooks) { - await txn.insert('notebooks', nb.toMap()); - } - for (final note in notes) { - await txn.insert('notes', note.toMap()); - } - }); + /// Inserts [note] or replaces the existing row with the same primary key. + /// + /// Used by the sync engine to adopt a remote .md file into the local DB + /// without going through the duplicating `INSERT` path of [createNote]. + Future upsertNote(Note note) async { + await _db.insert( + 'notes', + note.toMap(), + conflictAlgorithm: ConflictAlgorithm.replace, + ); + } + + /// Inserts [notebook] or replaces an existing row with the same id. + Future upsertNotebook(Notebook notebook) async { + await _db.insert( + 'notebooks', + notebook.toMap(), + conflictAlgorithm: ConflictAlgorithm.replace, + ); + } + + /// Removes the row with [id] from `notes`. Hard delete — used by sync to + /// drop notes that were 'synced' locally but no longer exist on the + /// remote (the user deleted them server-side). Local-only and modified + /// notes are skipped by callers, so they survive a pull. + Future hardDeleteNote(String id) async { + return _db.delete('notes', where: 'id = ?', whereArgs: [id]); + } + + /// Removes the row with [id] from `notebooks` (hard delete). + Future hardDeleteNotebook(String id) async { + return _db.delete('notebooks', where: 'id = ?', whereArgs: [id]); } // ----------------------------------------------------------------------- diff --git a/app/lib/services/api_client.dart b/app/lib/services/api_client.dart index 3f4e4e0..b2907aa 100644 --- a/app/lib/services/api_client.dart +++ b/app/lib/services/api_client.dart @@ -4,6 +4,8 @@ import 'dart:convert'; import 'package:http/http.dart' as http; +import 'title_cleaner.dart'; + class Notebook { final String id; final String name; @@ -57,7 +59,10 @@ class Note { factory Note.fromJson(Map j) => Note( id: j['id'], - title: j['title'], + // Strip slug/extension artifacts so list views and the editor + // never display things like `Foo__Md.Q2Hhd`. The server side keeps + // the raw title for storage; cleanup is a presentation concern. + title: cleanRemoteTitle((j['title'] as String?) ?? ''), noteType: j['note_type'] ?? 'typed', notebookId: j['notebook_id'], tags: List.from(j['tags'] ?? []), diff --git a/app/lib/services/local_note_service.dart b/app/lib/services/local_note_service.dart index 0a6155f..513cb50 100644 --- a/app/lib/services/local_note_service.dart +++ b/app/lib/services/local_note_service.dart @@ -54,6 +54,14 @@ class LocalNoteService { }) => _repo.createNote(title, notebookId: notebookId, noteType: noteType); Future getNoteById(String id) => _repo.getNoteById(id); + Future getNoteByRemoteId(String remoteId) => + _repo.getNoteByRemoteId(remoteId); + Future upsertNote(Note note) => _repo.upsertNote(note); + Future upsertNotebook(Notebook notebook) => + _repo.upsertNotebook(notebook); + Future hardDeleteNote(String id) => _repo.hardDeleteNote(id); + Future hardDeleteNotebook(String id) => + _repo.hardDeleteNotebook(id); // Strokes Future saveStroke(Stroke stroke) => _repo.saveStroke(stroke); @@ -62,26 +70,15 @@ class LocalNoteService { /// Snapshot of all locally-stored notebooks and notes. /// - /// Strokes are intentionally excluded — Phase 4A sync covers metadata only. - /// Used by SyncService to push local state to the backend. + /// Strokes are intentionally excluded — sync covers metadata only. + /// Used by SyncService to enumerate the local state both for the push + /// half of a cycle and for the merge step of a pull. Future exportAllData() async { final notebooks = await _repo.getNotebooks(includeArchived: true); final notes = await _repo.getAllNotes(includeDeleted: true); return LocalSnapshot(notebooks: notebooks, notes: notes); } - /// Replaces local notebooks and notes with [notebooks] and [notes]. - /// - /// Local strokes are **preserved** — Phase 4A sync moves metadata only, - /// and ink is the user's irreplaceable content. Strokes whose parent - /// note has disappeared remain in the database as orphans until a - /// stroke-aware sync phase reconciles them. - Future importAllData({ - required List notebooks, - required List notes, - }) => - _repo.replaceAll(notebooks: notebooks, notes: notes); - /// Closes the database opened by this service. No-op when an injected /// [database] was supplied — the caller owns that connection. Future close() async { diff --git a/app/lib/services/sync_service.dart b/app/lib/services/sync_service.dart index 605cad8..14149d3 100644 --- a/app/lib/services/sync_service.dart +++ b/app/lib/services/sync_service.dart @@ -1,7 +1,10 @@ +import 'dart:convert'; + import '../data/models/note.dart'; import '../data/models/notebook.dart'; import 'api_client.dart' as api; import 'local_note_service.dart'; +import 'title_cleaner.dart' as title_cleaner; /// Outcome of a single [SyncService.sync] call. class SyncResult { @@ -9,25 +12,43 @@ class SyncResult { final int notesPushed; final int notebooksPulled; final int notesPulled; + final int notesAdopted; const SyncResult({ required this.notebooksPushed, required this.notesPushed, required this.notebooksPulled, required this.notesPulled, + this.notesAdopted = 0, }); String get summary => 'pushed $notebooksPushed nb / $notesPushed notes, ' - 'pulled $notebooksPulled nb / $notesPulled notes'; + 'pulled $notebooksPulled nb / $notesPulled notes ' + '(adopted $notesAdopted)'; } -/// Manual sync between the local SQLite store and the Python backend. +/// Sync between the local SQLite store and the WebDAV/NAS-backed Python +/// backend that exposes `.md` files as the canonical note representation. +/// +/// **Pull is adopt-not-replace.** Existing remote `.md` files become local +/// notes: each is keyed by its server id (real UUID for NexaNote-frontmatter +/// notes, synthetic `md.` id for plain Markdown files), persisted +/// in the local row's [Note.remoteId]. Subsequent pulls find the existing +/// row and update in place — running sync three times after the first +/// import does not multiply the note count. /// -/// Phase 4A keeps this deliberately blunt: push everything local to the -/// backend, then replace the local DB with whatever the backend reports. -/// No incremental sync, no timestamp diffing, no conflict resolution — -/// that work belongs to a later phase. +/// **Push only sends genuinely new local rows.** A note that already +/// carries a `remoteId` (i.e. has been pulled/adopted at least once) is +/// skipped to avoid creating a duplicate via the non-idempotent `POST +/// /notes` endpoint. +/// +/// **Conflict policy.** When a remote note's id matches a local one we +/// keep the side with the newer `updatedAt` (remote wins ties). Local +/// `local_only` and `modified` notes are preserved across pulls so the +/// user does not lose work in flight. When a remote note arrives with +/// the same cleaned title but a different id than an existing local +/// row, both are kept and the local row is marked `conflict`. class SyncService { final api.ApiClient _api; final LocalNoteService _local; @@ -36,7 +57,7 @@ class SyncService { : _api = apiClient, _local = local; - /// Push local state, then pull-and-replace from the backend. + /// Push local-only changes, then adopt the remote view. Future sync() async { final pushed = await pushLocal(); final pulled = await pullRemote(); @@ -45,24 +66,16 @@ class SyncService { notesPushed: pushed.notes, notebooksPulled: pulled.notebooks, notesPulled: pulled.notes, + notesAdopted: pulled.adopted, ); } - /// Uploads only locally-originated or locally-modified records to the - /// backend. Records already marked `synced` are skipped. - /// - /// **Duplicate-upload guard.** The backend's create endpoints assign new - /// server-side IDs on every call and there is no upsert endpoint yet. - /// Pushing a record that already exists on the server would create a - /// duplicate. To avoid that we only push records whose [syncStatus] is - /// not `synced`. After [pullRemote] runs, every local record carries - /// `synced`, so subsequent [pushLocal] calls become no-ops until the - /// user creates or edits something locally. - /// - /// This guard is correct as long as nothing else mutates [syncStatus] - /// behind our back. Once the backend grows an idempotent upsert - /// endpoint, this filter can be relaxed. - Future<_Counts> pushLocal() async { + /// Uploads only locally-originated records that have never been seen on + /// the server. A row is considered "needs push" iff it has no + /// [Note.remoteId] AND its `syncStatus` is not `synced`. After a pull, + /// every adopted note carries `synced` plus a `remoteId`, so subsequent + /// pushes are no-ops until the user creates something new. + Future<_PushCounts> pushLocal() async { final snapshot = await _local.exportAllData(); var notebooks = 0; var notes = 0; @@ -73,28 +86,152 @@ class SyncService { } for (final note in snapshot.notes) { if (_isAlreadySynced(note.syncStatus)) continue; + // Already adopted from a remote .md file — never re-create on the + // server. The non-idempotent createNote endpoint would mint a fresh + // UUID and a duplicate file alongside the existing one. + if (note.remoteId != null && note.remoteId!.isNotEmpty) continue; await _api.createNote( - title: note.title, + title: cleanRemoteTitle(note.title), noteType: note.noteType, notebookId: note.notebookId, ); notes++; } - return _Counts(notebooks: notebooks, notes: notes); + return _PushCounts(notebooks: notebooks, notes: notes); } static bool _isAlreadySynced(String status) => status == 'synced'; - /// Fetches the backend's notebooks and notes and overwrites the local DB. - Future<_Counts> pullRemote() async { + /// Adopts the remote view of notebooks and notes into the local DB. The + /// merge is idempotent: running it back to back yields the same row set. + /// + /// Behaviour summary: + /// - Each remote note is keyed by `id` (or `remote_id`) and upserted in + /// place; a brand-new row is inserted only when no local match exists. + /// - Plain `.md` files (synthetic `md.` id) are adopted with the + /// server id stored as [Note.remoteId] so the next pull finds the + /// existing row instead of creating a duplicate. + /// - Local-only and locally-modified notes are preserved. + /// - Synced local notes that disappear from the remote are removed + /// (server-side delete propagates). + /// - When a remote note carries the same cleaned title as an unrelated + /// local row (different id), both are kept and the local row is + /// flagged with `sync_status='conflict'`. + Future<_PullCounts> pullRemote() async { final remoteNotebooks = await _api.getNotebooks(); final remoteNotes = await _api.getNotes(includeDeleted: true); - final notebooks = remoteNotebooks.map(_toLocalNotebook).toList(); - final notes = remoteNotes.map(_toLocalNote).toList(); + final localSnapshot = await _local.exportAllData(); + var adopted = 0; + + // ---------------- Notebooks ---------------- + final remoteNbIds = {}; + for (final nb in remoteNotebooks) { + remoteNbIds.add(nb.id); + await _local.upsertNotebook(_toLocalNotebook(nb)); + } + for (final localNb in localSnapshot.notebooks) { + if (remoteNbIds.contains(localNb.id)) continue; + // A notebook that disappeared on the server. Drop only if it was + // already 'synced' — local-only notebooks survive. + if (localNb.syncStatus == 'synced') { + await _local.hardDeleteNotebook(localNb.id); + } + } + + // ---------------- Notes ---------------- + final localById = {for (final n in localSnapshot.notes) n.id: n}; + final localByRemoteId = { + for (final n in localSnapshot.notes) + if (n.remoteId != null && n.remoteId!.isNotEmpty) n.remoteId!: n + }; + final localByRemotePath = { + for (final n in localSnapshot.notes) + if (n.remotePath != null && n.remotePath!.isNotEmpty) + n.remotePath!: n + }; + final localTitleIndex = {}; + for (final n in localSnapshot.notes) { + final cleaned = cleanRemoteTitle(n.title).toLowerCase(); + localTitleIndex.putIfAbsent(cleaned, () => n); + } + + final remoteIds = {}; + final remotePaths = {}; + for (final r in remoteNotes) { + remoteIds.add(r.id); + final cleanedRemoteTitle = cleanRemoteTitle(r.title); + final derivedPath = _derivedRemotePath(r.id); + if (derivedPath != null) remotePaths.add(derivedPath); + + final existing = localById[r.id] ?? + localByRemoteId[r.id] ?? + (derivedPath != null ? localByRemotePath[derivedPath] : null); + if (existing != null) { + // Same identity on both sides — newer timestamp wins, but a local + // 'modified' row beats an older or equal-time remote so user edits + // aren't clobbered before the next push round. + final remoteUpdated = _parseUtc(r.updatedAt); + final keepLocal = existing.syncStatus == 'modified' && + existing.updatedAt.isAfter(remoteUpdated); + if (keepLocal) continue; + await _local.upsertNote(existing.copyWith( + notebookId: r.notebookId, + clearNotebookId: r.notebookId == null, + title: cleanedRemoteTitle, + noteType: r.noteType, + tags: r.tags, + isPinned: r.isPinned, + isDeleted: r.isDeleted, + syncStatus: 'synced', + remoteId: r.id, + remotePath: derivedPath ?? existing.remotePath, + updatedAt: remoteUpdated, + )); + continue; + } + + // No id match. Check whether the remote shares a cleaned title with + // an unrelated local row — that's the "same title, different id" + // case the conflict policy speaks to. Keep both, flag the local. + final titleClash = + localTitleIndex[cleanedRemoteTitle.toLowerCase()]; + if (titleClash != null && titleClash.remoteId == null) { + await _local.upsertNote(titleClash.copyWith( + syncStatus: 'conflict', + updatedAt: titleClash.updatedAt, + )); + } - await _local.importAllData(notebooks: notebooks, notes: notes); - return _Counts(notebooks: notebooks.length, notes: notes.length); + await _local.upsertNote(_toLocalNote( + r, + cleanedTitle: cleanedRemoteTitle, + derivedPath: derivedPath, + )); + adopted++; + } + for (final localNote in localSnapshot.notes) { + if (remoteIds.contains(localNote.id)) continue; + if (localNote.remoteId != null && + remoteIds.contains(localNote.remoteId)) { + continue; + } + if (localNote.remotePath != null && + remotePaths.contains(localNote.remotePath)) { + continue; + } + // Drop only fully-synced rows. Local-only / modified / conflict + // rows are preserved; the next push promotes them upstream. + if (localNote.syncStatus == 'synced') { + await _local.hardDeleteNote(localNote.id); + } + } + + return _PullCounts( + notebooks: remoteNotebooks.length, + notes: remoteNotes.length, + adopted: adopted, + ); } Notebook _toLocalNotebook(api.Notebook nb) { @@ -110,31 +247,74 @@ class SyncService { ); } - Note _toLocalNote(api.Note n) { + Note _toLocalNote( + api.Note n, { + String? cleanedTitle, + String? derivedPath, + }) { final updated = _parseUtc(n.updatedAt); final created = _parseUtc(n.createdAt, fallback: updated); return Note( id: n.id, notebookId: n.notebookId, - title: n.title, + title: cleanedTitle ?? cleanRemoteTitle(n.title), noteType: n.noteType, tags: n.tags, isPinned: n.isPinned, isDeleted: n.isDeleted, syncStatus: 'synced', + remoteId: n.id, + remotePath: derivedPath ?? _derivedRemotePath(n.id), createdAt: created, updatedAt: updated, ); } + /// For plain Markdown remotes (synthetic id `md.` of the + /// file stem), recover the canonical relative path so the note can be + /// matched even if the server-issued id later changes. Returns null for + /// real-UUID notes — the API doesn't surface their paths today. + static String? _derivedRemotePath(String remoteId) { + if (!remoteId.startsWith('md.')) return null; + final encoded = remoteId.substring(3); + if (encoded.isEmpty) return null; + if (!_base64UrlSafeRe.hasMatch(encoded)) return null; + try { + final padded = encoded + '=' * ((4 - encoded.length % 4) % 4); + final bytes = base64Url.decode(padded); + final stem = utf8.decode(bytes); + return 'notes/$stem.md'; + } catch (_) { + return null; + } + } + + static final RegExp _base64UrlSafeRe = RegExp(r'^[A-Za-z0-9_-]+$'); + static DateTime _parseUtc(String iso, {DateTime? fallback}) { if (iso.isEmpty) return fallback ?? DateTime.now().toUtc(); return DateTime.tryParse(iso)?.toUtc() ?? fallback ?? DateTime.now().toUtc(); } + + /// Re-export of the shared title cleaner for tests and callers that + /// already import [SyncService]. + static String cleanRemoteTitle(String raw) => + title_cleaner.cleanRemoteTitle(raw); } -class _Counts { +class _PushCounts { final int notebooks; final int notes; - const _Counts({required this.notebooks, required this.notes}); + const _PushCounts({required this.notebooks, required this.notes}); +} + +class _PullCounts { + final int notebooks; + final int notes; + final int adopted; + const _PullCounts({ + required this.notebooks, + required this.notes, + required this.adopted, + }); } diff --git a/app/lib/services/title_cleaner.dart b/app/lib/services/title_cleaner.dart new file mode 100644 index 0000000..30d75ce --- /dev/null +++ b/app/lib/services/title_cleaner.dart @@ -0,0 +1,40 @@ +/// Strips display artifacts that leak into note titles from the WebDAV +/// slug or filesystem. Lives in its own file so both the sync engine and +/// the live API client can call it without dragging the rest of the sync +/// machinery into api_client.dart (which would create an import cycle +/// through SyncService). +/// +/// Specifically removes: +/// - a trailing `.md` extension, +/// - the `__` slug suffix appended by the WebDAV provider — +/// `__` round-trips through `.title()` into +/// things like `Foo Bar__Md.Q2Hhd` for plain Markdown files whose +/// synthetic id starts with `md.`, +/// - any combination of the two (e.g. `My Note__Md.Q2Hhd.md`). +/// +/// The cleanup is conservative: when the suffix doesn't look like an +/// id-prefix the title is returned unchanged. +String cleanRemoteTitle(String raw) { + var title = raw.trim(); + while (true) { + final stripped = _stripOnce(title); + if (stripped == title) break; + title = stripped; + } + return title.isEmpty ? raw.trim() : title; +} + +String _stripOnce(String input) { + var out = input; + if (out.toLowerCase().endsWith('.md')) { + out = out.substring(0, out.length - 3).trimRight(); + } + final match = _slugSuffixRe.firstMatch(out); + if (match != null) { + out = out.substring(0, match.start).trimRight(); + } + return out; +} + +final RegExp _slugSuffixRe = + RegExp(r'__(?:[0-9a-fA-F]{1,16}|[Mm][Dd]\.[A-Za-z0-9_-]+)\s*$'); diff --git a/app/test/data/database/app_database_test.dart b/app/test/data/database/app_database_test.dart index e22e85f..a2508be 100644 --- a/app/test/data/database/app_database_test.dart +++ b/app/test/data/database/app_database_test.dart @@ -91,5 +91,76 @@ void main() { await db.close(); }); + + test('notes table has remote_id and remote_path columns', () async { + final db = await openDatabase( + inMemoryDatabasePath, + version: Schema.version, + onCreate: Schema.onCreate, + ); + + final cols = await db.rawQuery('PRAGMA table_info(notes)'); + final names = cols.map((c) => c['name'] as String).toSet(); + expect(names, containsAll(['remote_id', 'remote_path'])); + await db.close(); + }); + }); + + group('Schema.onUpgrade', () { + test('adds remote_id/remote_path when migrating v1 → v2', () async { + final dbPath = inMemoryDatabasePath; + + // Open at v1 with the v1 schema (no remote_id/remote_path). + final v1 = await openDatabase( + dbPath, + version: 1, + onCreate: (db, _) async { + await db.execute(''' + CREATE TABLE notes ( + id TEXT PRIMARY KEY, + notebook_id TEXT, + title TEXT NOT NULL DEFAULT 'Untitled', + note_type TEXT NOT NULL DEFAULT 'typed', + tags TEXT NOT NULL DEFAULT '[]', + typed_content TEXT NOT NULL DEFAULT '', + is_pinned INTEGER NOT NULL DEFAULT 0, + is_archived INTEGER NOT NULL DEFAULT 0, + is_deleted INTEGER NOT NULL DEFAULT 0, + sync_status TEXT NOT NULL DEFAULT 'local_only', + created_at TEXT NOT NULL, + updated_at TEXT NOT NULL + ) + '''); + }, + ); + + final now = DateTime.utc(2024, 1, 1).toIso8601String(); + await v1.insert('notes', { + 'id': 'legacy', 'title': 'Legacy', 'created_at': now, 'updated_at': now, + }); + + final cols = await v1.rawQuery('PRAGMA table_info(notes)'); + expect( + cols.map((c) => c['name'] as String).toSet(), + isNot(contains('remote_id')), + ); + + // Manually upgrade — exercise the same migration code openDatabase + // would run on a real install. + await Schema.onUpgrade(v1, 1, Schema.version); + + final upgraded = await v1.rawQuery('PRAGMA table_info(notes)'); + final names = upgraded.map((c) => c['name'] as String).toSet(); + expect(names, containsAll(['remote_id', 'remote_path'])); + + // Existing row survives the migration; new columns default to NULL. + final row = + (await v1.query('notes', where: 'id = ?', whereArgs: ['legacy'])).first; + expect(row['title'], 'Legacy'); + expect(row['remote_id'], isNull); + expect(row['remote_path'], isNull); + + await v1.close(); + }); }); } diff --git a/app/test/data/models/note_test.dart b/app/test/data/models/note_test.dart index f968cf9..2d327d6 100644 --- a/app/test/data/models/note_test.dart +++ b/app/test/data/models/note_test.dart @@ -20,6 +20,8 @@ void main() { isArchived: false, isDeleted: false, syncStatus: 'modified', + remoteId: 'md.SGVsbG8', + remotePath: 'notes/Hello.md', createdAt: _ts, updatedAt: _ts, ); @@ -36,10 +38,41 @@ void main() { expect(restored.isArchived, note.isArchived); expect(restored.isDeleted, note.isDeleted); expect(restored.syncStatus, note.syncStatus); + expect(restored.remoteId, note.remoteId); + expect(restored.remotePath, note.remotePath); expect(restored.createdAt, note.createdAt); expect(restored.updatedAt, note.updatedAt); }); + test('remoteId and remotePath default to null', () { + final note = Note( + id: 'note-default', + title: 'Default', + createdAt: _ts, + updatedAt: _ts, + ); + expect(note.remoteId, isNull); + expect(note.remotePath, isNull); + final restored = Note.fromMap(note.toMap()); + expect(restored.remoteId, isNull); + expect(restored.remotePath, isNull); + }); + + test('copyWith updates remoteId and remotePath', () { + final note = Note( + id: 'n', + title: 'T', + createdAt: _ts, + updatedAt: _ts, + ); + final updated = note.copyWith( + remoteId: 'md.AAAA', + remotePath: 'notes/Foo.md', + ); + expect(updated.remoteId, 'md.AAAA'); + expect(updated.remotePath, 'notes/Foo.md'); + }); + test('tags are serialized as JSON array string', () { final note = Note( id: 'note-1', diff --git a/app/test/services/sync_service_test.dart b/app/test/services/sync_service_test.dart index fc419ed..2e83593 100644 --- a/app/test/services/sync_service_test.dart +++ b/app/test/services/sync_service_test.dart @@ -73,6 +73,44 @@ class FakeApiClient extends api.ApiClient { } } +api.Note _remoteNote({ + required String id, + required String title, + String noteType = 'typed', + String? notebookId, + bool isPinned = false, + bool isDeleted = false, + String? updatedAt, +}) { + final now = updatedAt ?? DateTime.now().toUtc().toIso8601String(); + return api.Note( + id: id, + title: title, + noteType: noteType, + notebookId: notebookId, + tags: const [], + isPinned: isPinned, + isDeleted: isDeleted, + pageCount: 0, + updatedAt: now, + createdAt: now, + ); +} + +api.Notebook _remoteNotebook({ + required String id, + String name = 'Notebook', + String color = '#000000', + String? updatedAt, +}) => + api.Notebook( + id: id, + name: name, + color: color, + icon: 'notebook', + updatedAt: updatedAt ?? DateTime.now().toUtc().toIso8601String(), + ); + void main() { setUpAll(() { sqfliteFfiInit(); @@ -89,6 +127,7 @@ void main() { inMemoryDatabasePath, version: Schema.version, onCreate: Schema.onCreate, + onUpgrade: Schema.onUpgrade, ); local = LocalNoteService(database: db); await local.initialize(); @@ -100,6 +139,10 @@ void main() { await db.close(); }); + // -------------------------------------------------------------------- + // pushLocal + // -------------------------------------------------------------------- + test('pushLocal sends every local notebook and note to the API', () async { final nb = await local.createNotebook('Work', color: '#abcdef'); await local.createNote('Meeting notes', notebookId: nb.id); @@ -119,33 +162,60 @@ void main() { ); }); - test('pullRemote replaces the local store with the API response', () async { - // Seed local data that should be wiped. - final stale = await local.createNotebook('Stale'); - await local.createNote('Stale note', notebookId: stale.id); + test('pushLocal skips records already marked synced', () async { + fakeApi.remoteNotebooks = [_remoteNotebook(id: 'nb-pulled', name: 'Pulled')]; + fakeApi.remoteNotes = [ + _remoteNote(id: 'note-pulled', title: 'Pulled note', notebookId: 'nb-pulled'), + ]; + await sync.pullRemote(); + fakeApi.createdNotebooks.clear(); + fakeApi.createdNotes.clear(); + + final counts = await sync.pushLocal(); + + expect(counts.notebooks, 0); + expect(counts.notes, 0); + expect(fakeApi.createdNotebooks, isEmpty); + expect(fakeApi.createdNotes, isEmpty); + }); + + test('pushLocal does not re-create notes that already carry a remote_id', + () async { + // Adopt a remote note, then locally bump its sync_status back to + // 'modified' (as the editor would on a content edit) — pushLocal + // must not try to POST it as a brand-new note since the server + // already owns the row. + fakeApi.remoteNotes = [_remoteNote(id: 'remote-42', title: 'Hello')]; + await sync.pullRemote(); + + await db.update( + 'notes', + {'sync_status': 'modified'}, + where: 'id = ?', + whereArgs: ['remote-42'], + ); + + final counts = await sync.pushLocal(); + expect(counts.notes, 0); + expect(fakeApi.createdNotes, isEmpty); + }); + + // -------------------------------------------------------------------- + // pullRemote — adoption + // -------------------------------------------------------------------- + test('pullRemote adopts remote notebooks and notes by id', () async { final now = DateTime.now().toUtc().toIso8601String(); fakeApi.remoteNotebooks = [ - api.Notebook( - id: 'nb-1', - name: 'Remote NB', - color: '#112233', - icon: 'notebook', - updatedAt: now, - ), + _remoteNotebook(id: 'nb-1', name: 'Remote NB', updatedAt: now), ]; fakeApi.remoteNotes = [ - api.Note( + _remoteNote( id: 'note-1', title: 'Remote note', - noteType: 'typed', notebookId: 'nb-1', - tags: const ['x'], isPinned: true, - isDeleted: false, - pageCount: 1, updatedAt: now, - createdAt: now, ), ]; @@ -154,7 +224,6 @@ void main() { final notebooks = await local.getNotebooks(); expect(notebooks, hasLength(1)); expect(notebooks.first.id, 'nb-1'); - expect(notebooks.first.name, 'Remote NB'); expect(notebooks.first.syncStatus, 'synced'); final notes = await local.getNotesForNotebook('nb-1'); @@ -162,25 +231,152 @@ void main() { expect(notes.first.id, 'note-1'); expect(notes.first.title, 'Remote note'); expect(notes.first.isPinned, isTrue); + expect(notes.first.remoteId, 'note-1'); + expect(notes.first.syncStatus, 'synced'); + }); + + test('pullRemote preserves local-only notes/notebooks the server does not know about', + () async { + final localNb = await local.createNotebook('Pristine'); + await local.createNote('Draft', notebookId: localNb.id); - // Stale notebook is gone. - expect(notebooks.where((n) => n.id == stale.id), isEmpty); + fakeApi.remoteNotebooks = [_remoteNotebook(id: 'nb-r', name: 'Remote')]; + fakeApi.remoteNotes = [ + _remoteNote(id: 'note-r', title: 'Remote note', notebookId: 'nb-r'), + ]; + + await sync.pullRemote(); + + final all = await local.exportAllData(); + expect(all.notebooks.map((n) => n.id), containsAll([localNb.id, 'nb-r'])); + expect(all.notes.map((n) => n.title), containsAll(['Draft', 'Remote note'])); }); - test('sync runs push then pull and reports counts', () async { - final nb = await local.createNotebook('Local'); - await local.createNote('To push', notebookId: nb.id); + test('running sync three times after first import does not duplicate notes', + () async { + fakeApi.remoteNotebooks = [_remoteNotebook(id: 'nb-1', name: 'NB')]; + fakeApi.remoteNotes = [ + _remoteNote(id: 'note-1', title: 'Hello', notebookId: 'nb-1'), + _remoteNote(id: 'note-2', title: 'World', notebookId: 'nb-1'), + ]; - final now = DateTime.now().toUtc().toIso8601String(); - fakeApi.remoteNotebooks = [ - api.Notebook( - id: 'nb-r', - name: 'After', - color: '#000000', - icon: 'notebook', - updatedAt: now, + await sync.pullRemote(); + final firstCount = (await local.exportAllData()).notes.length; + expect(firstCount, 2); + + await sync.pullRemote(); + await sync.pullRemote(); + + final after = await local.exportAllData(); + expect(after.notes.length, firstCount); + expect(after.notebooks.length, 1); + }); + + test('plain Markdown remote ids (md.) are adopted with stable mapping', + () async { + // Mimic what the backend returns for a plain `.md` file dropped on + // the WebDAV/NAS: synthetic id, raw filename-derived title with + // slug suffix artifacts. + fakeApi.remoteNotes = [ + _remoteNote( + id: 'md.Q2hhdWRyZQ', + title: 'Chaudré De Saucisses__Md.Q2Hhd', ), ]; + + await sync.pullRemote(); + final after = await sync.pullRemote(); + expect(after.notes, 1); + + final notes = (await local.exportAllData()).notes; + expect(notes, hasLength(1)); + expect(notes.first.remoteId, 'md.Q2hhdWRyZQ'); + expect(notes.first.title, 'Chaudré De Saucisses'); + expect(notes.first.title, isNot(contains('.md'))); + expect(notes.first.title, isNot(contains('__'))); + }); + + test('remote update with same id replaces local fields when newer', + () async { + final older = DateTime.utc(2024, 1, 1).toIso8601String(); + final newer = DateTime.utc(2024, 6, 1).toIso8601String(); + + fakeApi.remoteNotes = [ + _remoteNote(id: 'note-x', title: 'Old', updatedAt: older), + ]; + await sync.pullRemote(); + + fakeApi.remoteNotes = [ + _remoteNote(id: 'note-x', title: 'New', updatedAt: newer, isPinned: true), + ]; + await sync.pullRemote(); + + final notes = (await local.exportAllData()).notes; + expect(notes, hasLength(1)); + expect(notes.first.title, 'New'); + expect(notes.first.isPinned, isTrue); + }); + + test('local "modified" notes win over older remote during a pull', () async { + final older = DateTime.utc(2024, 1, 1).toIso8601String(); + + fakeApi.remoteNotes = [_remoteNote(id: 'note-y', title: 'Server', updatedAt: older)]; + await sync.pullRemote(); + + final newer = DateTime.utc(2025, 1, 1).toIso8601String(); + await db.update( + 'notes', + { + 'title': 'Local edit', + 'sync_status': 'modified', + 'updated_at': newer, + }, + where: 'id = ?', + whereArgs: ['note-y'], + ); + + fakeApi.remoteNotes = [_remoteNote(id: 'note-y', title: 'Server', updatedAt: older)]; + await sync.pullRemote(); + + final note = await local.getNoteById('note-y'); + expect(note!.title, 'Local edit'); + expect(note.syncStatus, 'modified'); + }); + + test('same cleaned title with different ids keeps both and flags conflict', + () async { + final localNb = await local.createNotebook('NB'); + await local.createNote('Recipes', notebookId: localNb.id); + + fakeApi.remoteNotes = [_remoteNote(id: 'remote-recipes', title: 'Recipes')]; + + await sync.pullRemote(); + + final notes = (await local.exportAllData()).notes; + expect(notes, hasLength(2)); + + final localCopy = notes.firstWhere((n) => n.id != 'remote-recipes'); + final remoteCopy = notes.firstWhere((n) => n.id == 'remote-recipes'); + expect(localCopy.syncStatus, 'conflict'); + expect(remoteCopy.title, 'Recipes'); + }); + + test('synced local note disappearing from remote is removed', () async { + fakeApi.remoteNotes = [_remoteNote(id: 'note-z', title: 'Bye')]; + await sync.pullRemote(); + expect(await local.getNoteById('note-z'), isNotNull); + + fakeApi.remoteNotes = []; + await sync.pullRemote(); + expect(await local.getNoteById('note-z'), isNull); + }); + + test('sync runs push then pull and reports counts (push survives, remote adopted)', + () async { + final nb = await local.createNotebook('Local'); + await local.createNote('To push', notebookId: nb.id); + + fakeApi.remoteNotebooks = [_remoteNotebook(id: 'nb-r', name: 'After')]; fakeApi.remoteNotes = []; final result = await sync.sync(); @@ -191,11 +387,10 @@ void main() { expect(result.notesPulled, 0); final notebooks = await local.getNotebooks(); - expect(notebooks.single.id, 'nb-r'); + expect(notebooks.map((n) => n.id), containsAll([nb.id, 'nb-r'])); }); test('pullRemote preserves local strokes (metadata-only sync)', () async { - // Local note with a hand-written stroke that must survive sync. final nb = await local.createNotebook('Sketches'); final note = await local.createNote('Doodle', notebookId: nb.id); final stroke = Stroke( @@ -212,29 +407,13 @@ void main() { ); await local.saveStroke(stroke); - // Backend reports the same note back (metadata only — no strokes). - final now = DateTime.now().toUtc().toIso8601String(); - fakeApi.remoteNotebooks = [ - api.Notebook( - id: nb.id, - name: 'Sketches', - color: nb.color, - icon: 'notebook', - updatedAt: now, - ), - ]; + fakeApi.remoteNotebooks = [_remoteNotebook(id: nb.id, name: 'Sketches')]; fakeApi.remoteNotes = [ - api.Note( + _remoteNote( id: note.id, title: 'Doodle', noteType: 'handwritten', notebookId: nb.id, - tags: const [], - isPinned: false, - isDeleted: false, - pageCount: 0, - updatedAt: now, - createdAt: now, ), ]; @@ -247,47 +426,6 @@ void main() { expect(strokes.first.points[1].x, 3.0); }); - test('pushLocal skips records already marked synced', () async { - // Simulate a post-pull state: notebooks/notes pulled from the - // backend land in the DB with sync_status='synced'. A subsequent - // push must not re-upload them, otherwise the backend (no upsert) - // grows duplicates on every sync cycle. - final now = DateTime.now().toUtc().toIso8601String(); - fakeApi.remoteNotebooks = [ - api.Notebook( - id: 'nb-pulled', - name: 'Pulled', - color: '#000000', - icon: 'notebook', - updatedAt: now, - ), - ]; - fakeApi.remoteNotes = [ - api.Note( - id: 'note-pulled', - title: 'Pulled note', - noteType: 'typed', - notebookId: 'nb-pulled', - tags: const [], - isPinned: false, - isDeleted: false, - pageCount: 0, - updatedAt: now, - createdAt: now, - ), - ]; - await sync.pullRemote(); - fakeApi.createdNotebooks.clear(); - fakeApi.createdNotes.clear(); - - final counts = await sync.pushLocal(); - - expect(counts.notebooks, 0); - expect(counts.notes, 0); - expect(fakeApi.createdNotebooks, isEmpty); - expect(fakeApi.createdNotes, isEmpty); - }); - test('exportAllData returns the snapshot used by SyncService', () async { final nb = await local.createNotebook('A'); await local.createNote('one', notebookId: nb.id); @@ -297,4 +435,43 @@ void main() { expect(snap.notebooks, hasLength(1)); expect(snap.notes, hasLength(2)); }); + + // -------------------------------------------------------------------- + // Title cleanup + // -------------------------------------------------------------------- + + group('SyncService.cleanRemoteTitle', () { + test('strips a trailing .md', () { + expect(SyncService.cleanRemoteTitle('Hello.md'), 'Hello'); + expect(SyncService.cleanRemoteTitle('Hello.MD'), 'Hello'); + }); + + test('strips a hex __ suffix', () { + expect(SyncService.cleanRemoteTitle('Foo__abcd1234'), 'Foo'); + }); + + test('strips a plain-MD __Md. suffix', () { + expect( + SyncService.cleanRemoteTitle('Chaudré De Saucisses__Md.Q2Hhd'), + 'Chaudré De Saucisses', + ); + }); + + test('strips combined __.md', () { + expect(SyncService.cleanRemoteTitle('My Note__abcd1234.md'), 'My Note'); + expect( + SyncService.cleanRemoteTitle('My Note__Md.Q2Hhd.md'), + 'My Note', + ); + }); + + test('leaves plain titles alone', () { + expect(SyncService.cleanRemoteTitle('Hello World'), 'Hello World'); + expect(SyncService.cleanRemoteTitle('Foo__bar baz'), 'Foo__bar baz'); + }); + + test('keeps the original when stripping would empty the title', () { + expect(SyncService.cleanRemoteTitle('.md'), '.md'); + }); + }); } diff --git a/app/test/services/title_cleaner_test.dart b/app/test/services/title_cleaner_test.dart new file mode 100644 index 0000000..0556e5b --- /dev/null +++ b/app/test/services/title_cleaner_test.dart @@ -0,0 +1,47 @@ +import 'package:flutter_test/flutter_test.dart'; + +import 'package:nexanote/services/title_cleaner.dart'; + +void main() { + group('cleanRemoteTitle', () { + test('strips a trailing .md extension', () { + expect(cleanRemoteTitle('Hello.md'), 'Hello'); + expect(cleanRemoteTitle('Hello.MD'), 'Hello'); + }); + + test('strips a hex __ WebDAV slug suffix', () { + expect(cleanRemoteTitle('Foo__abcd1234'), 'Foo'); + expect(cleanRemoteTitle('Multi Word Title__deadbeef'), 'Multi Word Title'); + }); + + test('strips the title-cased Md. form left by the slug parser', + () { + expect( + cleanRemoteTitle('Chaudré De Saucisses__Md.Q2Hhd'), + 'Chaudré De Saucisses', + ); + expect(cleanRemoteTitle('Foo__md.bar_baz-qux'), 'Foo'); + }); + + test('strips combined __.md', () { + expect(cleanRemoteTitle('My Note__abcd1234.md'), 'My Note'); + expect(cleanRemoteTitle('My Note__Md.Q2Hhd.md'), 'My Note'); + }); + + test('leaves plain titles alone', () { + expect(cleanRemoteTitle('Hello World'), 'Hello World'); + // Two underscores are only suspicious when followed by an id-prefix. + expect(cleanRemoteTitle('Foo__bar baz'), 'Foo__bar baz'); + }); + + test('keeps the original when stripping would empty the title', () { + expect(cleanRemoteTitle('.md'), '.md'); + expect(cleanRemoteTitle(' '), ''); + }); + + test('trims whitespace and never returns an artifact-only title', () { + expect(cleanRemoteTitle(' Hello '), 'Hello'); + expect(cleanRemoteTitle('Hello.md '), 'Hello'); + }); + }); +}