Skip to content

Commit e123576

Browse files
javad-zobeidijavad-zobeidi
authored
Merge pull request #212 from javad-zobeidi/dev
fix(ORM): Concurrent modification error when including multiple relat…
2 parents 337bb47 + 375f3c6 commit e123576

3 files changed

Lines changed: 141 additions & 92 deletions

File tree

‎lib/src/database/migration/runners/migration_runner.dart‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,9 @@ class MigrationRunner {
3131
} catch (e) {
3232
stopwatch.stop();
3333
if (e is QueryException) {
34-
stderr.write(e.cause);
34+
stderr.writeln(e.cause);
35+
} else {
36+
stderr.writeln(e);
3537
}
3638
stderr.writeln(
3739
'❌ Migration $migrationName failed ......................................\x1B[31m ${stopwatch.elapsedMilliseconds}ms FAILED\x1B[0m',

‎lib/src/database/orm/model.dart‎

Lines changed: 100 additions & 85 deletions
Original file line numberDiff line numberDiff line change
@@ -712,131 +712,146 @@ abstract class Model extends QueryBuilderImpl {
712712
return this;
713713
}
714714

715-
void _clearWithRelation(_RelationQuery r) {
716-
_withRelation = _withRelation.where((item) => item != r).toList();
717-
}
718-
719715
Future<void> _eagerLoadRelation(
720716
List<Map<String, dynamic>> models,
721717
_RelationQuery rq,
722718
Function(dynamic data) callBack,
723719
) async {
724720
String relation = rq.relation;
721+
List<String> wr = relation.split('.');
725722

726-
if (_withRelation.any((r) => r.relation == relation)) {
727-
List<String> wr = relation.split('.');
723+
String primaryRelation = wr.first;
728724

729-
String primaryRelation = wr.first;
725+
List<String> getColumns = ['*'];
726+
final relationParts = primaryRelation.split(':');
730727

731-
List<String> getColumns = ['*'];
732-
final relationParts = primaryRelation.split(':');
728+
if (relationParts.length > 1) {
729+
primaryRelation = relationParts.first.trim();
733730

734-
if (relationParts.length > 1) {
735-
primaryRelation = relationParts.first.trim();
731+
final columnsString = relationParts.last.trim();
736732

737-
final columnsString = relationParts.last.trim();
738-
739-
if (columnsString.isNotEmpty) {
740-
getColumns = columnsString
741-
.split(',')
742-
.map((col) => col.trim())
743-
.where((col) => col.isNotEmpty)
744-
.toList();
745-
}
733+
if (columnsString.isNotEmpty) {
734+
getColumns = columnsString
735+
.split(',')
736+
.map((col) => col.trim())
737+
.where((col) => col.isNotEmpty)
738+
.toList();
746739
}
740+
}
747741

748-
if (!_relations.containsKey(primaryRelation)) {
749-
throw InvalidArgumentException(
750-
'Relation $relation not found in $runtimeType',
751-
);
752-
}
742+
if (!_relations.containsKey(primaryRelation)) {
743+
throw InvalidArgumentException(
744+
'Relation $relation not found in $runtimeType',
745+
);
746+
}
753747

754-
Relation rela = _relations[primaryRelation] as Relation;
755-
Model qb = rela.related;
756-
rela.parent._clearWithRelation(rq);
748+
Relation rela = _relations[primaryRelation] as Relation;
749+
Model qb = rela.related;
757750

758-
if (rq.callback != null) {
759-
qb = rq.callback!(qb) as Model;
760-
}
751+
if (rq.callback != null) {
752+
qb = rq.callback!(qb) as Model;
753+
}
761754

762-
if (rela is MorphRelation) {
763-
if (rela is MorphTo) {
764-
Set ids = models.map((m) => m[rela.morphKey]).toSet();
765-
qb = qb.whereIn(rela.localKey, ids.toList()) as Model;
766-
} else {
767-
Set ids = models.map((m) => m[rela.localKey]).toSet();
768-
769-
if (rela is MorphToMany || rela is MorphedByMany) {
770-
qb =
771-
qb
772-
.whereIn(rela.morphKey, ids.toList())
773-
.whereEqualTo(rela.morphType, rela.type)
774-
.join(
775-
rela.related.tableName,
776-
'${rela.pivotTable}.${rela.relatedMorphKey}',
777-
'=',
778-
'${rela.related.tableName}.${rela.localKey}',
779-
)
780-
as Model;
781-
qb.tableName = rela.pivotTable!;
782-
} else {
783-
qb =
784-
qb
785-
.whereIn(rela.morphKey, ids.toList())
786-
.whereEqualTo(rela.morphType, rela.type)
787-
as Model;
788-
}
755+
if (rela is MorphRelation) {
756+
if (rela is MorphTo) {
757+
Set ids = models.map((m) => m[rela.morphKey]).toSet();
758+
759+
// Early return if no IDs to query
760+
if (ids.isEmpty) {
761+
callBack(rela.match(models, [], primaryRelation));
762+
return;
789763
}
764+
765+
qb = qb.whereIn(rela.localKey, ids.toList()) as Model;
790766
} else {
791-
late final String getLocalKey;
792-
if (rela is BelongsTo) {
793-
getLocalKey =
794-
rela.foreignKey ??
795-
'${rela.related.runtimeType.toString()}_id'.toLowerCase();
796-
} else {
797-
getLocalKey = rela.localKey;
798-
}
767+
Set ids = models.map((m) => m[rela.localKey]).toSet();
799768

800-
Set ids = models.map((m) => m[getLocalKey]).toSet();
769+
// Early return if no IDs to query
770+
if (ids.isEmpty) {
771+
callBack(rela.match(models, [], primaryRelation));
772+
return;
773+
}
801774

802-
if (rela is BelongsToMany) {
775+
if (rela is MorphToMany || rela is MorphedByMany) {
803776
qb =
804777
qb
805-
.whereIn(
806-
'${rela.pivotTable}.${rela.parentPivotKey}',
807-
ids.toList(),
808-
)
778+
.whereIn(rela.morphKey, ids.toList())
779+
.whereEqualTo(rela.morphType, rela.type)
809780
.join(
810-
rela.pivotTable,
811-
'${rela.pivotTable}.${rela.relatedPivotKey}',
781+
rela.related.tableName,
782+
'${rela.pivotTable}.${rela.relatedMorphKey}',
812783
'=',
813-
'${rela.related.tableName}.${rela.relatedLocalKey}',
784+
'${rela.related.tableName}.${rela.localKey}',
814785
)
815786
as Model;
816-
} else if (rela is BelongsTo) {
817-
qb = qb.whereIn(rela.localKey, ids.toList()) as Model;
787+
qb.tableName = rela.pivotTable!;
818788
} else {
819-
qb = qb.whereIn(rela.foreignKey!, ids.toList()) as Model;
789+
qb =
790+
qb
791+
.whereIn(rela.morphKey, ids.toList())
792+
.whereEqualTo(rela.morphType, rela.type)
793+
as Model;
820794
}
821795
}
822-
late final List<Map<String, dynamic>> results;
796+
} else {
797+
late final String getLocalKey;
798+
if (rela is BelongsTo) {
799+
getLocalKey =
800+
rela.foreignKey ??
801+
'${rela.related.runtimeType.toString()}_id'.toLowerCase();
802+
} else {
803+
getLocalKey = rela.localKey;
804+
}
805+
806+
Set ids = models.map((m) => m[getLocalKey]).toSet();
807+
808+
// Early return if no IDs to query
809+
if (ids.isEmpty) {
810+
callBack(rela.match(models, [], primaryRelation));
811+
return;
812+
}
823813

824-
if (wr.length > 1) {
825-
wr.removeAt(0);
826-
results = await qb.include(wr.join('.')).get(getColumns);
814+
if (rela is BelongsToMany) {
815+
qb =
816+
qb
817+
.whereIn(
818+
'${rela.pivotTable}.${rela.parentPivotKey}',
819+
ids.toList(),
820+
)
821+
.join(
822+
rela.pivotTable,
823+
'${rela.pivotTable}.${rela.relatedPivotKey}',
824+
'=',
825+
'${rela.related.tableName}.${rela.relatedLocalKey}',
826+
)
827+
as Model;
828+
} else if (rela is BelongsTo) {
829+
qb = qb.whereIn(rela.localKey, ids.toList()) as Model;
827830
} else {
828-
results = await qb.get(getColumns);
831+
qb = qb.whereIn(rela.foreignKey!, ids.toList()) as Model;
829832
}
833+
}
834+
835+
late final List<Map<String, dynamic>> results;
830836

831-
callBack(rela.match(models, results, primaryRelation));
837+
if (wr.length > 1) {
838+
wr.removeAt(0);
839+
results = await qb.include(wr.join('.')).get(getColumns);
840+
} else {
841+
results = await qb.get(getColumns);
832842
}
843+
844+
callBack(rela.match(models, results, primaryRelation));
833845
}
834846

835847
Future<List<Map<String, dynamic>>> _loadRelations(
836848
List<Map<String, dynamic>> result,
837849
) async {
838850
if (_withRelation.isNotEmpty) {
839-
final relationsToLoad = List<_RelationQuery>.from(_withRelation);
851+
// Create an immutable copy of relations to load and clear the original list
852+
// This prevents concurrent modification when iterating
853+
final relationsToLoad = List<_RelationQuery>.unmodifiable(_withRelation);
854+
_withRelation = [];
840855

841856
for (_RelationQuery relation in relationsToLoad) {
842857
await _eagerLoadRelation(result, relation, (callBackResult) {

‎lib/src/database/query_builder/_where_clauses_builder_impl.dart‎

Lines changed: 38 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -7,15 +7,51 @@ import '_query_builder_impl.dart';
77
int _paramCounter = 0;
88

99
abstract mixin class WhereClausesBuilderImpl implements QueryBuilder {
10+
/// Valid SQL comparison operators to prevent SQL injection.
11+
/// Only these operators are allowed in where clauses.
12+
static const Set<String> _validOperators = {
13+
'=',
14+
'<>',
15+
'!=',
16+
'<',
17+
'>',
18+
'<=',
19+
'>=',
20+
'LIKE',
21+
'NOT LIKE',
22+
'ILIKE', // PostgreSQL case-insensitive LIKE
23+
'NOT ILIKE',
24+
'REGEXP',
25+
'NOT REGEXP',
26+
'RLIKE', // MySQL alias for REGEXP
27+
'SIMILAR TO', // PostgreSQL
28+
};
29+
1030
set paramCounter(int paramN) {
1131
_paramCounter = paramN;
1232
}
1333

34+
/// Returns the current parameter counter value.
35+
/// Useful for synchronizing nested queries.
36+
int get currentParamCounter => _paramCounter;
37+
1438
String _nextParamName() {
1539
_paramCounter++;
1640
return 'p$_paramCounter';
1741
}
1842

43+
/// Validates that the given operator is a valid SQL comparison operator.
44+
/// Throws [InvalidArgumentException] if the operator is not valid.
45+
/// This prevents SQL injection attacks through malicious operators.
46+
void _validateOperator(String operator) {
47+
if (!_validOperators.contains(operator.toUpperCase())) {
48+
throw InvalidArgumentException(
49+
'Invalid SQL operator: "$operator". '
50+
'Allowed operators: ${_validOperators.join(", ")}',
51+
);
52+
}
53+
}
54+
1955
@override
2056
String buildWhereClause() {
2157
return conditions.isNotEmpty ? " WHERE ${conditions.join(" ")}" : "";
@@ -41,6 +77,7 @@ abstract mixin class WhereClausesBuilderImpl implements QueryBuilder {
4177
String boolean = 'and',
4278
]) {
4379
if (condition is String) {
80+
_validateOperator(operator);
4481
final paramName = _nextParamName();
4582
bindings[paramName] = value;
4683
_appendCondition("$condition $operator :$paramName", isOr: true);
@@ -284,6 +321,7 @@ abstract mixin class WhereClausesBuilderImpl implements QueryBuilder {
284321
String boolean = 'and',
285322
]) {
286323
if (condition is String) {
324+
_validateOperator(operator);
287325
final paramName = _nextParamName();
288326
bindings[paramName] = value;
289327
_appendCondition(
@@ -974,12 +1012,6 @@ abstract mixin class WhereClausesBuilderImpl implements QueryBuilder {
9741012
String clause = not ? "NOT IN" : "IN";
9751013

9761014
if (values is List) {
977-
if (values.isEmpty) {
978-
throw InvalidArgumentException(
979-
"The list of values for IN must not be empty.",
980-
);
981-
}
982-
9831015
List<String> paramNames = [];
9841016
for (var i = 0; i < values.length; i++) {
9851017
final paramName = _nextParamName();

0 commit comments

Comments
 (0)