Skip to content

Commit 007797a

Browse files
committed
Address multiple PR feedback items
1 parent 594a671 commit 007797a

File tree

2 files changed

+41
-8
lines changed

2 files changed

+41
-8
lines changed

packages/dart/lib/src/utils/parse_live_list.dart

Lines changed: 35 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ class ParseLiveList<T extends ParseObject> {
99
List<String>? preloadedColumns,
1010
}) : _preloadedColumns = preloadedColumns ?? const <String>[] {
1111
_debug = isDebugEnabled();
12+
_debugLoggedInit = isDebugEnabled();
1213
}
1314

1415
/// Creates a new [ParseLiveList] for the given [query].
@@ -59,6 +60,8 @@ class ParseLiveList<T extends ParseObject> {
5960
late StreamController<ParseLiveListEvent<T>> _eventStreamController;
6061
int _nextID = 0;
6162
late bool _debug;
63+
// Separate from _debug to allow one-time initialization logging without affecting error logging
64+
late bool _debugLoggedInit;
6265

6366
int get nextID => _nextID++;
6467

@@ -148,8 +151,11 @@ class ParseLiveList<T extends ParseObject> {
148151

149152
Future<ParseResponse> _runQuery() async {
150153
final QueryBuilder<T> query = QueryBuilder<T>.copy(_query);
151-
if (_debug) {
152-
print('ParseLiveList: lazyLoading is ${_lazyLoading ? 'on' : 'off'}');
154+
155+
// Log lazy loading mode only once during initialization to avoid log spam
156+
if (_debugLoggedInit) {
157+
print('ParseLiveList: Initialized with lazyLoading=${_lazyLoading ? 'on' : 'off'}, preloadedColumns=${_preloadedColumns.isEmpty ? 'none' : _preloadedColumns.join(", ")}');
158+
_debugLoggedInit = false;
153159
}
154160

155161
// Only restrict fields if lazy loading is enabled AND preloaded columns are specified
@@ -553,11 +559,15 @@ class ParseLiveList<T extends ParseObject> {
553559

554560
final element = _list[index];
555561

556-
// Double-check: another call might have started loading already
557-
if (element.loaded) {
562+
// Race condition protection: skip if element is already loaded or
563+
// currently being loaded by another concurrent call
564+
if (element.loaded || element._isLoading) {
558565
return;
559566
}
560567

568+
// Set loading flag to prevent concurrent load operations
569+
element._isLoading = true;
570+
561571
try {
562572
final QueryBuilder<T> queryBuilder = QueryBuilder<T>.copy(_query)
563573
..whereEqualTo(
@@ -579,6 +589,14 @@ class ParseLiveList<T extends ParseObject> {
579589
if (response.success &&
580590
response.results != null &&
581591
response.results!.isNotEmpty) {
592+
// Verify we're still updating the same object (list may have been modified)
593+
final currentElement = _list[index];
594+
if (currentElement.object.objectId != element.object.objectId) {
595+
if (_debug) {
596+
print('ParseLiveList: Element at index $index changed during load');
597+
}
598+
return;
599+
}
582600
// Setting the object will mark it as loaded and emit it to the stream
583601
_list[index].object = response.results!.first;
584602
} else if (response.error != null) {
@@ -589,6 +607,13 @@ class ParseLiveList<T extends ParseObject> {
589607
'ParseLiveList: Error loading element at index $index: ${response.error}',
590608
);
591609
}
610+
} else {
611+
// Object not found (possibly deleted between initial query and load)
612+
if (_debug) {
613+
print(
614+
'ParseLiveList: Element at index $index not found during load',
615+
);
616+
}
592617
}
593618
} catch (e, stackTrace) {
594619
// Emit exception to the element's stream
@@ -598,6 +623,9 @@ class ParseLiveList<T extends ParseObject> {
598623
'ParseLiveList: Exception loading element at index $index: $e\n$stackTrace',
599624
);
600625
}
626+
} finally {
627+
// Clear loading flag to allow future retry attempts
628+
element._isLoading = false;
601629
}
602630
}
603631

@@ -734,7 +762,8 @@ class ParseLiveListElement<T extends ParseObject> {
734762
this._object, {
735763
bool loaded = false,
736764
Map<String, dynamic>? updatedSubItems,
737-
}) : _loaded = loaded {
765+
}) : _loaded = loaded,
766+
_isLoading = false {
738767
_updatedSubItems = _toSubscriptionMap(
739768
updatedSubItems ?? <String, dynamic>{},
740769
);
@@ -747,6 +776,7 @@ class ParseLiveListElement<T extends ParseObject> {
747776
final StreamController<T> _streamController = StreamController<T>.broadcast();
748777
T _object;
749778
bool _loaded = false;
779+
bool _isLoading = false;
750780
late Map<PathKey, dynamic> _updatedSubItems;
751781
LiveQuery? _liveQuery;
752782
final Future<void> _subscriptionQueue = Future<void>.value();

packages/dart/test/src/utils/parse_live_list_test.dart

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -143,7 +143,9 @@ void main() {
143143
);
144144

145145
test('lazyLoading=false should NOT restrict fields automatically', () {
146-
// When lazy loading is disabled, fetch all fields
146+
// Verifies baseline: a fresh QueryBuilder has no 'keys' restriction.
147+
// The actual _runQuery() behavior with lazyLoading=false is tested
148+
// indirectly through the loaded flag tests above.
147149
final query = QueryBuilder<ParseObject>(ParseObject('Room'))
148150
..orderByAscending('order');
149151

@@ -158,8 +160,9 @@ void main() {
158160
});
159161

160162
test('lazyLoading=true with preloadedColumns should restrict fields', () {
161-
// When lazy loading with preloaded columns, the initial query should
162-
// only fetch those columns
163+
// Verifies that keysToReturn() sets the 'keys' limiter as expected.
164+
// Note: This simulates _runQuery() behavior; actual integration testing
165+
// would require mocking the network layer.
163166
final query = QueryBuilder<ParseObject>(ParseObject('Room'))
164167
..orderByAscending('order')
165168
..keysToReturn(['name', 'order']); // Simulating what _runQuery does

0 commit comments

Comments
 (0)