diff --git a/lib/src/bones_api_entity_db_object_directory.dart b/lib/src/bones_api_entity_db_object_directory.dart index 86283ea..daab98a 100644 --- a/lib/src/bones_api_entity_db_object_directory.dart +++ b/lib/src/bones_api_entity_db_object_directory.dart @@ -749,14 +749,18 @@ class DBObjectDirectoryAdapter return _finishOperation(op, id, preFinish); } - Future _saveObject( - String table, - Object? id, - Map obj, - ) async { + /// Writes synchronously, and must stay synchronous. + /// + /// Every reader in this adapter checks the filesystem synchronously + /// ([Directory.listSync], [File.existsSync]), so an asynchronous write would + /// let a store return before its object is visible: a `store` immediately + /// followed by a `selectAll`/`selectByID` could miss it, and + /// [_doSelectAllImpl] would silently drop it (a not-yet-written file reads + /// back as `null`, which `resolveAllNotNull` discards). + void _saveObject(String table, Object? id, Map obj) { var file = _resolveObjectFile(table, id); var enc = dart_convert.json.encode(obj); - await file.writeAsString(enc); + file.writeAsStringSync(enc); } Future?> _readObject(String table, Object? id) async { diff --git a/test/bones_api_entity_db_directory_test.dart b/test/bones_api_entity_db_directory_test.dart index 304a6e2..972f88b 100644 --- a/test/bones_api_entity_db_directory_test.dart +++ b/test/bones_api_entity_db_directory_test.dart @@ -1,13 +1,15 @@ @TestOn('vm') @Tags(['entities']) -@Timeout(Duration(seconds: 30)) +@Timeout(Duration(seconds: 60)) import 'dart:io'; +import 'dart:typed_data'; import 'package:bones_api/bones_api_db_directory.dart'; import 'package:bones_api/bones_api_test.dart'; import 'package:test/test.dart'; import 'bones_api_entity_db_tests_base.dart'; +import 'bones_api_test_entities.dart'; class MemoryTestConfig extends APITestConfigDBSQLMemory { MemoryTestConfig() @@ -23,6 +25,78 @@ Future main() async { await _runTest(false, false); await _runTest(true, true); await _runTest(false, true); + + _runStoreVisibilityTest(); +} + +/// REGRESSION: `DBObjectDirectoryAdapter._saveObject` used to be `async`, and +/// `doInsert`/`doUpdate` dropped its `Future`. Since every reader in that +/// adapter inspects the filesystem synchronously, a `store` could return +/// before its object was on disk — and `selectAll` would then *silently omit* +/// it, because a not-yet-written file reads back as `null` and is discarded by +/// `resolveAllNotNull`. +/// +/// That is what made `Pagination [objectAdapter]` flaky on CI: entries went +/// missing from the result, and which ones varied per run. Confirmed by adding +/// a 30ms delay before the (un-awaited) write, which reproduces that failure +/// exactly. +/// +/// Note this test only *fails* where the write is slow enough to lose the +/// race — it does not on a fast local disk. It is kept as a cheap statement of +/// the invariant; `Pagination [objectAdapter]` remains the sensitive guard. +void _runStoreVisibilityTest() { + group('DBObjectDirectoryAdapter', () { + test('a stored object is immediately visible', () async { + var tempDir = Directory.systemTemp.createTempSync( + 'bones_api_tests_object_dir_visibility', + ); + + var provider = createEntityRepositoryProvider2( + true, + (p, dbPort, dbConfig) => + DBObjectDirectoryAdapter(tempDir, parentRepositoryProvider: p), + 0, + null, + ); + + addTearDown(() { + provider.close(); + try { + tempDir.deleteSync(recursive: true); + } catch (_) {} + }); + + await provider.ensureInitialized(); + + var photoRepo = provider.photoAPIRepository; + + // Big enough to give a slow filesystem a chance to lose the race: + var data = Uint8List(512 * 1024); + + var ids = []; + + for (var i = 1; i <= 4; ++i) { + var id = 'PG-SYNC-0$i'; + ids.add(id); + + expect(await photoRepo.store(Photo.fromData(data, id: id)), equals(id)); + + expect( + await photoRepo.selectByID(id), + isNotNull, + reason: '`$id` not readable right after `store`', + ); + } + + var all = await photoRepo.selectAll(); + + expect( + all.map((e) => e.id).where(ids.contains).toList()..sort(), + equals(ids), + reason: '`selectAll` dropped a stored object', + ); + }); + }); } Future _runTest(bool useReflection, bool populateSource) {