Skip to content

fix(redis): prevent concurrent list edit races and lossy binary edits (#1009) #1017

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
ZhuchkaTriplesix merged 1 commit into from
Sep 27, 2026
Merged

fix(redis): prevent concurrent list edit races and lossy binary edits (#1009) #1017

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 20 additions & 4 deletions lib/core/database/redis_connection.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -458,6 +458,16 @@ class RedisConnection {
await sendCommand(['RPUSH', redisCommandArg(key), value]);
}

/// LINDEX key index.
Future<RedisBulkValue?> lindex(Object key, int index) async {
final result =
await sendCommand(['LINDEX', redisCommandArg(key), index]);
if (result == null || _isRedisNil(result)) {
return null;
}
return RedisBulkValue.fromReply(result);
}

/// LSET key index value.
Future<void> lset(Object key, int index, String value) async {
_assertWritable();
Expand Down Expand Up @@ -618,7 +628,7 @@ class RedisConnectionTestFake extends RedisConnection {
this.secondScanKeys = const <String>[],
this.dbSizeResult = 2,
this.getResult,
List<String>? listItems,
List<dynamic>? listItems,
this.llenResult,
this.hashFirstPage = const <String, String>{},
this.hashSecondPage = const <String, String>{},
Expand All @@ -633,7 +643,7 @@ class RedisConnectionTestFake extends RedisConnection {
this.getBytesResult,
this.binaryScanKeys = const <List<int>>[],
}) : listItems =
listItems != null ? List<String>.from(listItems) : <String>[],
listItems != null ? List<dynamic>.from(listItems) : <dynamic>[],
super(
id: -1,
name: 'test-fake',
Expand All @@ -645,7 +655,7 @@ class RedisConnectionTestFake extends RedisConnection {
final List<String> secondScanKeys;
final int dbSizeResult;
final String? getResult;
final List<String> listItems;
final List<dynamic> listItems;
final int? llenResult;
final Map<String, String> hashFirstPage;
final Map<String, String> hashSecondPage;
Expand Down Expand Up @@ -795,6 +805,12 @@ class RedisConnectionTestFake extends RedisConnection {
case 'RPUSH':
listItems.add(args[2].toString());
return 'OK';
case 'LINDEX':
final lindexIdx = int.tryParse(args[2].toString()) ?? 0;
if (lindexIdx >= 0 && lindexIdx < listItems.length) {
return listItems[lindexIdx];
}
return null;
case 'LSET':
final idx = int.tryParse(args[2].toString()) ?? 0;
final val = args[3].toString();
Expand Down Expand Up @@ -832,7 +848,7 @@ class RedisConnectionTestFake extends RedisConnection {
}
}

List<String> _sliceList(List<String> items, List<dynamic> args) {
List<dynamic> _sliceList(List<dynamic> items, List<dynamic> args) {
if (items.isEmpty) return const [];
final start = int.tryParse(args[2].toString()) ?? 0;
var stop = int.tryParse(args[3].toString()) ?? -1;
Expand Down
87 changes: 78 additions & 9 deletions lib/features/redis/redis_key_editor.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -291,7 +291,16 @@ class _RedisKeyEditorState extends material.State<RedisKeyEditor> {
start,
start + redisCollectionPageSize - 1,
);
_listValue.addAll(chunk);
if (!widget.isReadOnly) {
for (final item in chunk) {
if (_isOrphanedDeleteSentinel(item)) {
widget.connection.lrem(_cmdKey, 0, item.commandArg).ignore();
}
}
}
final cleanChunk =
chunk.where((item) => !_isOrphanedDeleteSentinel(item)).toList();
_listValue.addAll(cleanChunk);
_hasMore = _listValue.length < _collectionTotal;
case 'set':
if (reset) {
Expand Down Expand Up @@ -477,10 +486,24 @@ class _RedisKeyEditorState extends material.State<RedisKeyEditor> {
}
}

Future<void> _listSet(int index, String newValue) async {
Future<void> _listSet(
int index,
String newValue, {
RedisBulkValue? expectedCurrent,
}) async {
if (widget.isReadOnly) return;
try {
await widget.connection.selectDatabase(widget.database);
if (expectedCurrent != null) {
final current = await widget.connection.lindex(_cmdKey, index);
if (current == null || current != expectedCurrent) {
await _load();
if (!mounted) return;
setState(() => _error =
'List item at index $index changed concurrently; reloaded latest.');
return;
}
}
await widget.connection.lset(_cmdKey, index, newValue);
await _load();
} catch (e) {
Expand All @@ -501,17 +524,39 @@ class _RedisKeyEditorState extends material.State<RedisKeyEditor> {
if (!mounted || !confirmed) return;
try {
await widget.connection.selectDatabase(widget.database);
final current = await widget.connection.lindex(_cmdKey, index);
if (current == null || current != item) {
await _load();
if (!mounted) return;
setState(() => _error =
'List item at index $index changed concurrently; reloaded latest.');
return;
}
final sentinel =
'__QUERYA_DEL_${DateTime.now().microsecondsSinceEpoch}__';
await widget.connection.lset(_cmdKey, index, sentinel);
await widget.connection.lrem(_cmdKey, 1, sentinel);
try {
await widget.connection.lset(_cmdKey, index, sentinel);
await widget.connection.lrem(_cmdKey, 1, sentinel);
} catch (e) {
try {
await widget.connection.lrem(_cmdKey, 1, sentinel);
} catch (_) {}
rethrow;
}
await _load();
} catch (e) {
if (!mounted) return;
setState(() => _error = 'LREM failed: $e');
}
}

bool _isOrphanedDeleteSentinel(RedisBulkValue value) {
final text = value.text;
return text != null &&
text.startsWith('__QUERYA_DEL_') &&
text.endsWith('__');
}

// Set operations
Future<void> _setAdd(String member) async {
if (widget.isReadOnly) return;
Expand Down Expand Up @@ -1044,19 +1089,22 @@ class _RedisKeyEditorState extends material.State<RedisKeyEditor> {
itemBuilder: (context, i) => _IndexedValueRow(
index: i,
value: _listValue[i].label,
onEdit: widget.isReadOnly
isBinary: !_listValue[i].isUtf8,
onEdit: widget.isReadOnly || !_listValue[i].isUtf8
? null
: () async {
final initialText =
_listValue[i].text ?? _listValue[i].label;
final edited = await showAppDialog<String>(
context: context,
builder: (ctx) => _RedisEditListDialogContent(
index: i,
initialValue: _listValue[i].label,
initialValue: initialText,
),
);
if (edited != null &&
edited != _listValue[i].label) {
await _listSet(i, edited);
if (edited != null && edited != initialText) {
await _listSet(i, edited,
expectedCurrent: _listValue[i]);
}
},
onDelete: widget.isReadOnly
Expand Down Expand Up @@ -1510,6 +1558,7 @@ class _IndexedValueRow extends StatelessWidget {
const _IndexedValueRow({
required this.index,
required this.value,
this.isBinary = false,
this.onEdit,
this.onDelete,
required this.colorScheme,
Expand All @@ -1518,6 +1567,7 @@ class _IndexedValueRow extends StatelessWidget {

final int index;
final String value;
final bool isBinary;
final VoidCallback? onEdit;
final VoidCallback? onDelete;
final ColorScheme colorScheme;
Expand Down Expand Up @@ -1557,6 +1607,25 @@ class _IndexedValueRow extends StatelessWidget {
),
),
),
if (isBinary) ...[
const Gap(8),
material.Container(
padding: const material.EdgeInsets.symmetric(
horizontal: 5, vertical: 1.5),
decoration: material.BoxDecoration(
color: shadcnCs.muted,
borderRadius: material.BorderRadius.circular(3),
),
child: Text(
'binary',
style: material.TextStyle(
fontSize: 10,
fontFamily: 'monospace',
color: shadcnCs.mutedForeground,
),
),
),
],
if (onEdit != null) ...[
const Gap(8),
material.Tooltip(
Expand Down
134 changes: 134 additions & 0 deletions test/features/redis/redis_key_editor_test.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import 'dart:typed_data';

import 'package:flutter/material.dart' as material;
import 'package:flutter_test/flutter_test.dart';
import 'package:querya_desktop/core/database/redis_bulk.dart';
Expand Down Expand Up @@ -473,6 +475,138 @@ void main() {
expect(find.text('No items'), findsOneWidget);
await fake.disconnect();
});

testWidgets(
'RedisKeyEditor list item edit aborts when element changed concurrently',
(tester) async {
final fake = RedisConnectionTestFake(
listItems: ['task1'],
);
await fake.connect();

await pumpEditor(
tester,
fake: fake,
isReadOnly: false,
keyType: 'list',
keyName: 'jobs',
);

expect(find.text('task1'), findsOneWidget);
await tester.tap(find.byTooltip('Edit item'));
await tester.pumpAndSettle();

// Concurrent writer modifies item at index 0 before user saves
fake.listItems[0] = 'task1_concurrent_edit';

final field = find.descendant(
of: find.byType(shadcn.AlertDialog),
matching: find.byType(shadcn.TextField),
);
await tester.enterText(field, 'task1_my_edit');
await tester.pumpAndSettle();

await tester.tap(find.text('Save'));
await tester.pumpAndSettle();

// LSET should not have overwritten the concurrent edit
expect(fake.listItems[0], 'task1_concurrent_edit');
expect(
find.textContaining('changed concurrently; reloaded latest.'),
findsOneWidget,
);
expect(find.text('task1_concurrent_edit'), findsOneWidget);
await fake.disconnect();
});

testWidgets(
'RedisKeyEditor list item delete aborts when element changed concurrently',
(tester) async {
final fake = RedisConnectionTestFake(
listItems: ['task1'],
);
await fake.connect();

await pumpEditor(
tester,
fake: fake,
isReadOnly: false,
keyType: 'list',
keyName: 'jobs',
);

await tester.tap(find.byTooltip('Delete item'));
await tester.pumpAndSettle();

// Concurrent writer modifies item at index 0 before user confirms deletion
fake.listItems[0] = 'task1_concurrent_shift';

await tester.tap(find.byType(material.Checkbox));
await tester.pumpAndSettle();
await tester.tap(find.text('Execute Destructive Statement'));
await tester.pumpAndSettle();

// Deletion aborted; concurrent item preserved
expect(fake.listItems, ['task1_concurrent_shift']);
expect(
find.textContaining('changed concurrently; reloaded latest.'),
findsOneWidget,
);
expect(find.text('task1_concurrent_shift'), findsOneWidget);
await fake.disconnect();
});

testWidgets(
'RedisKeyEditor binary list item hides edit button and shows binary badge',
(tester) async {
final fake = RedisConnectionTestFake(
listItems: [
Uint8List.fromList([0x00, 0xff, 0xfe]),
],
);
await fake.connect();

await pumpEditor(
tester,
fake: fake,
isReadOnly: false,
keyType: 'list',
keyName: 'bin_jobs',
);

expect(find.text('binary'), findsOneWidget);
expect(find.byTooltip('Edit item'), findsNothing);
expect(find.byTooltip('Delete item'), findsOneWidget);
await fake.disconnect();
});

testWidgets(
'RedisKeyEditor cleans up orphaned delete sentinel on list load',
(tester) async {
final fake = RedisConnectionTestFake(
listItems: [
'normal_item',
'__QUERYA_DEL_123456789__',
'another_item',
],
);
await fake.connect();

await pumpEditor(
tester,
fake: fake,
isReadOnly: false,
keyType: 'list',
keyName: 'clean_jobs',
);

expect(find.text('normal_item'), findsOneWidget);
expect(find.text('another_item'), findsOneWidget);
expect(find.text('__QUERYA_DEL_123456789__'), findsNothing);
expect(fake.sentCommands.contains('LREM'), isTrue);
await fake.disconnect();
});

testWidgets(
'RedisKeyEditor Refresh proceeds without dialog when string is unedited',
(tester) async {
Expand Down
Loading