diff --git a/angel_orm/lib/src/join_builder.dart b/angel_orm/lib/src/join_builder.dart index 5636dabb..bf5836dd 100644 --- a/angel_orm/lib/src/join_builder.dart +++ b/angel_orm/lib/src/join_builder.dart @@ -31,7 +31,12 @@ class JoinBuilder { } String compile(Set trampoline) { - if (to == null) return null; + var compiledTo = to(); + if (compiledTo == null) { + print( + 'NULLLLL $to; from $from; key: $key, value: $value, addl: $additionalFields'); + } + if (compiledTo == null) return null; var b = StringBuffer(); var left = '${from.tableName}.$key'; var right = fieldName; @@ -54,7 +59,7 @@ class JoinBuilder { break; } - b.write(' ${to()}'); + b.write(' $compiledTo'); if (alias != null) b.write(' $alias'); b.write(' ON $left$op$right'); return b.toString(); diff --git a/angel_orm/lib/src/query.dart b/angel_orm/lib/src/query.dart index 7addbbe7..ce066897 100644 --- a/angel_orm/lib/src/query.dart +++ b/angel_orm/lib/src/query.dart @@ -219,6 +219,8 @@ abstract class Query extends QueryBase { b.write(' '); List f; + var compiledJoins = {}; + if (fields == null) { f = ['*']; } else { @@ -229,10 +231,16 @@ abstract class Query extends QueryBase { return ss; })); _joins.forEach((j) { - var additional = j.additionalFields.map(j.nameFor).toList(); - // if (!additional.contains(j.fieldName)) - // additional.insert(0, j.fieldName); - f.addAll(additional); + var c = compiledJoins[j] = j.compile(trampoline); + if (c != null) { + var additional = j.additionalFields.map(j.nameFor).toList(); + f.addAll(additional); + } else { + // If compilation failed, fill in NULL placeholders. + for (var i = 0; i < j.additionalFields.length; i++) { + f.add('NULL'); + } + } }); } if (withFields) b.write(f.join(', ')); @@ -243,7 +251,7 @@ abstract class Query extends QueryBase { if (preamble == null) { if (_crossJoin != null) b.write(' CROSS JOIN $_crossJoin'); for (var join in _joins) { - var c = join.compile(trampoline); + var c = compiledJoins[join]; if (c != null) b.write(' $c'); } } diff --git a/angel_orm_generator/lib/src/orm_build_context.dart b/angel_orm_generator/lib/src/orm_build_context.dart index 134b65cc..6013c63f 100644 --- a/angel_orm_generator/lib/src/orm_build_context.dart +++ b/angel_orm_generator/lib/src/orm_build_context.dart @@ -3,7 +3,6 @@ import 'dart:async'; import 'package:analyzer/dart/constant/value.dart'; import 'package:analyzer/dart/element/element.dart'; import 'package:analyzer/dart/element/type.dart'; -import 'package:analyzer/src/dart/constant/value.dart'; import 'package:angel_model/angel_model.dart'; import 'package:angel_orm/angel_orm.dart'; import 'package:angel_serialize/angel_serialize.dart'; diff --git a/angel_orm_generator/lib/src/orm_generator.dart b/angel_orm_generator/lib/src/orm_generator.dart index 6077456e..b7f7dc93 100644 --- a/angel_orm_generator/lib/src/orm_generator.dart +++ b/angel_orm_generator/lib/src/orm_generator.dart @@ -281,6 +281,7 @@ class OrmGenerator extends GeneratorForAnnotation { .assign(queryWhereType.newInstance([refer('this')])), ); + // Note: this is where subquery fields for relations are added. ctx.relations.forEach((fieldName, relation) { //var name = ctx.buildContext.resolveFieldName(fieldName); if (relation.type == RelationshipType.belongsTo || @@ -289,43 +290,103 @@ class OrmGenerator extends GeneratorForAnnotation { var foreign = relation.throughContext ?? relation.foreign; // If this is a many-to-many, add the fields from the other object. - var additionalFields = relation.foreign.effectiveFields - // .where((f) => f.name != 'id' || !isSpecialId(ctx, f)) - .map((f) => literalString(relation.foreign.buildContext - .resolveFieldName(f.name))); + + var additionalStrs = relation.foreign.effectiveFields.map((f) => + relation.foreign.buildContext.resolveFieldName(f.name)); + var additionalFields = additionalStrs.map(literalString); var joinArgs = [relation.localKey, relation.foreignKey] .map(literalString) .toList(); - // In the past, we would either do a join on the table name - // itself, or create an instance of a query. - // - // From this point on, however, we will create a field for each - // join, so that users can customize the generated query. - // - // There'll be a private `_field`, and then a getter, named `field`, - // that returns the subqueryb object. - var foreignQueryType = refer( - foreign.buildContext.modelClassNameRecase.pascalCase + - 'Query'); - clazz - ..fields.add(Field((b) => b - ..name = '_$fieldName' - ..type = foreignQueryType)) - ..methods.add(Method((b) => b - ..name = fieldName - ..type = MethodType.getter - ..returns = foreignQueryType - ..body = refer('_$fieldName').returned.statement)); + // In the case of a many-to-many, we don't generate a subquery field, + // as it easily leads to stack overflows. + if (relation.isManyToMany) { + // We can't simply join against the "through" table; this itself must + // be a join. + // (SELECT role_users.role_id, + // FROM users + // LEFT JOIN role_users ON role_users.user_id=users.id) + var foreignFields = additionalStrs + .map((f) => '${relation.foreign.tableName}.$f'); + var b = StringBuffer('(SELECT '); + // role_users.role_id + b.write('${relation.throughContext.tableName}'); + b.write('.${relation.foreignKey}'); + // , + b.write(foreignFields.isEmpty + ? '' + : ', ' + foreignFields.join(', ')); + // FROM users + b.write(' FROM '); + b.write(relation.foreign.tableName); + // LEFT JOIN role_users + b.write(' LEFT JOIN ${relation.throughContext.tableName}'); + // Figure out which field on the "through" table points to users (foreign). + var throughRelation = + relation.throughContext.relations.values.firstWhere((e) { + return e.foreignTable == relation.foreign.tableName; + }, orElse: () { + // _Role has a many-to-many to _User through _RoleUser, but + // _RoleUser has no relation pointing to _User. + var b = StringBuffer(); + b.write(ctx.buildContext.modelClassName); + b.write('has a many-to-many relationship to '); + b.write(relation.foreign.buildContext.modelClassName); + b.write(' through '); + b.write( + relation.throughContext.buildContext.modelClassName); + b.write(', but '); + b.write( + relation.throughContext.buildContext.modelClassName); + b.write('has no relation pointing to '); + b.write(relation.foreign.buildContext.modelClassName); + b.write('.'); + throw b.toString(); + }); - // Assign a value to `_field`. - var queryInstantiation = foreignQueryType.newInstance([], { - 'trampoline': refer('trampoline'), - 'parent': refer('this') - }); - joinArgs.insert( - 0, refer('_$fieldName').assign(queryInstantiation)); + // ON role_users.user_id=users.id) + b.write(' ON '); + b.write('${relation.throughContext.tableName}'); + b.write('.'); + b.write(throughRelation.localKey); + b.write('='); + b.write(relation.foreign.tableName); + b.write('.'); + b.write(throughRelation.foreignKey); + b.write(')'); + + joinArgs.insert(0, literalString(b.toString())); + } else { + // In the past, we would either do a join on the table name + // itself, or create an instance of a query. + // + // From this point on, however, we will create a field for each + // join, so that users can customize the generated query. + // + // There'll be a private `_field`, and then a getter, named `field`, + // that returns the subquery object. + var foreignQueryType = refer( + foreign.buildContext.modelClassNameRecase.pascalCase + + 'Query'); + clazz + ..fields.add(Field((b) => b + ..name = '_$fieldName' + ..type = foreignQueryType)) + ..methods.add(Method((b) => b + ..name = fieldName + ..type = MethodType.getter + ..returns = foreignQueryType + ..body = refer('_$fieldName').returned.statement)); + + // Assign a value to `_field`. + var queryInstantiation = foreignQueryType.newInstance([], { + 'trampoline': refer('trampoline'), + 'parent': refer('this') + }); + joinArgs.insert( + 0, refer('_$fieldName').assign(queryInstantiation)); + } var joinType = relation.joinTypeString; b.addExpression(refer(joinType).call(joinArgs, { diff --git a/angel_orm_test/lib/src/many_to_many_test.dart b/angel_orm_test/lib/src/many_to_many_test.dart index c5588b6f..531c88e3 100644 --- a/angel_orm_test/lib/src/many_to_many_test.dart +++ b/angel_orm_test/lib/src/many_to_many_test.dart @@ -3,6 +3,7 @@ import 'dart:io'; import 'package:angel_orm/angel_orm.dart'; import 'package:test/test.dart'; import 'models/user.dart'; +import 'util.dart'; manyToManyTests(FutureOr Function() createExecutor, {FutureOr Function(QueryExecutor) close}) { @@ -61,6 +62,7 @@ manyToManyTests(FutureOr Function() createExecutor, print('=== THOSAKWE: ${thosakwe?.toJson()}'); // Allow thosakwe to publish... + printSeparator('Allow thosakwe to publish'); var thosakwePubQuery = RoleUserQuery(); thosakwePubQuery.values ..userId = int.parse(thosakwe.id) @@ -68,6 +70,7 @@ manyToManyTests(FutureOr Function() createExecutor, await thosakwePubQuery.insert(executor); // Allow thosakwe to subscribe... + printSeparator('Allow thosakwe to subscribe'); var thosakweSubQuery = RoleUserQuery(); thosakweSubQuery.values ..userId = int.parse(thosakwe.id) @@ -78,8 +81,8 @@ manyToManyTests(FutureOr Function() createExecutor, // await dumpQuery('select * from users;'); // await dumpQuery('select * from roles;'); // await dumpQuery('select * from role_users;'); - var query = RoleQuery()..where.id.equals(canPub.idAsInt); - await dumpQuery(query.compile(Set())); + // var query = RoleQuery()..where.id.equals(canPub.idAsInt); + // await dumpQuery(query.compile(Set())); print('\n'); print('=================================================='); @@ -95,6 +98,7 @@ manyToManyTests(FutureOr Function() createExecutor, } test('fetch roles for user', () async { + printSeparator('Fetch roles for user test'); var user = await fetchThosakwe(); expect(user.roles, hasLength(2)); expect(user.roles, contains(canPub)); @@ -108,4 +112,21 @@ manyToManyTests(FutureOr Function() createExecutor, expect(r.users.toList(), [thosakwe]); } }); + + test('only fetches linked', () async { + // Create a new user. The roles list should be empty, + // be there are no related rules. + var userQuery = UserQuery(); + userQuery.values + ..username = 'Prince' + ..password = 'Rogers' + ..email = 'Nelson'; + var user = await userQuery.insert(executor); + expect(user.roles, isEmpty); + + // Fetch again, just to be doubly sure. + var query = UserQuery()..where.id.equals(user.idAsInt); + var fetched = await query.getOne(executor); + expect(fetched.roles, isEmpty); + }); } diff --git a/angel_orm_test/lib/src/models/email_indexed.g.dart b/angel_orm_test/lib/src/models/email_indexed.g.dart index 85fffb87..42c387fe 100644 --- a/angel_orm_test/lib/src/models/email_indexed.g.dart +++ b/angel_orm_test/lib/src/models/email_indexed.g.dart @@ -64,8 +64,10 @@ class RoleQuery extends Query { trampoline ??= Set(); trampoline.add(tableName); _where = RoleQueryWhere(this); - leftJoin(_users = RoleUserQuery(trampoline: trampoline, parent: this), - 'role', 'role_role', + leftJoin( + '(SELECT role_users.role_role , users.email, users.name, users.password FROM users LEFT JOIN role_users ON role_users.user_email=users.email)', + 'role', + 'role_role', additionalFields: const ['email', 'name', 'password'], trampoline: trampoline); } @@ -75,8 +77,6 @@ class RoleQuery extends Query { RoleQueryWhere _where; - RoleUserQuery _users; - @override get casts { return {}; @@ -119,10 +119,6 @@ class RoleQuery extends Query { return parseRow(row); } - RoleUserQuery get users { - return _users; - } - @override bool canCompile(trampoline) { return (!(trampoline.contains('roles') && @@ -338,9 +334,12 @@ class UserQuery extends Query { trampoline ??= Set(); trampoline.add(tableName); _where = UserQueryWhere(this); - leftJoin(_roles = RoleUserQuery(trampoline: trampoline, parent: this), - 'email', 'user_email', - additionalFields: const ['role'], trampoline: trampoline); + leftJoin( + '(SELECT role_users.user_email , roles.role FROM roles LEFT JOIN role_users ON role_users.role_role=roles.role)', + 'email', + 'user_email', + additionalFields: const ['role'], + trampoline: trampoline); } @override @@ -348,8 +347,6 @@ class UserQuery extends Query { UserQueryWhere _where; - RoleUserQuery _roles; - @override get casts { return {}; @@ -395,10 +392,6 @@ class UserQuery extends Query { return parseRow(row); } - RoleUserQuery get roles { - return _roles; - } - @override bool canCompile(trampoline) { return (!(trampoline.contains('users') && diff --git a/angel_orm_test/lib/src/models/unorthodox.g.dart b/angel_orm_test/lib/src/models/unorthodox.g.dart index 558674e0..976ced94 100644 --- a/angel_orm_test/lib/src/models/unorthodox.g.dart +++ b/angel_orm_test/lib/src/models/unorthodox.g.dart @@ -208,9 +208,12 @@ class WeirdJoinQuery extends Query { leftJoin(_numbas = NumbaQuery(trampoline: trampoline, parent: this), 'id', 'parent', additionalFields: const ['i', 'parent'], trampoline: trampoline); - leftJoin(_foos = FooPivotQuery(trampoline: trampoline, parent: this), 'id', + leftJoin( + '(SELECT foo_pivots.weird_join_id , foos.bar FROM foos LEFT JOIN foo_pivots ON foo_pivots.foo_bar=foos.bar)', + 'id', 'weird_join_id', - additionalFields: const ['bar'], trampoline: trampoline); + additionalFields: const ['bar'], + trampoline: trampoline); } @override @@ -224,8 +227,6 @@ class WeirdJoinQuery extends Query { NumbaQuery _numbas; - FooPivotQuery _foos; - @override get casts { return {}; @@ -294,10 +295,6 @@ class WeirdJoinQuery extends Query { return _numbas; } - FooPivotQuery get foos { - return _foos; - } - @override bool canCompile(trampoline) { return (!(trampoline.contains('weird_joins') && @@ -612,9 +609,12 @@ class FooQuery extends Query { trampoline ??= Set(); trampoline.add(tableName); _where = FooQueryWhere(this); - leftJoin(_weirdJoins = FooPivotQuery(trampoline: trampoline, parent: this), - 'bar', 'foo_bar', - additionalFields: const ['id', 'join_name'], trampoline: trampoline); + leftJoin( + '(SELECT foo_pivots.foo_bar , weird_joins.id, weird_joins.join_name FROM weird_joins LEFT JOIN foo_pivots ON foo_pivots.weird_join_id=weird_joins.id)', + 'bar', + 'foo_bar', + additionalFields: const ['id', 'join_name'], + trampoline: trampoline); } @override @@ -622,8 +622,6 @@ class FooQuery extends Query { FooQueryWhere _where; - FooPivotQuery _weirdJoins; - @override get casts { return {}; @@ -666,10 +664,6 @@ class FooQuery extends Query { return parseRow(row); } - FooPivotQuery get weirdJoins { - return _weirdJoins; - } - @override bool canCompile(trampoline) { return (!(trampoline.contains('foos') && diff --git a/angel_orm_test/lib/src/models/user.g.dart b/angel_orm_test/lib/src/models/user.g.dart index 5bcad71f..2e113ab5 100644 --- a/angel_orm_test/lib/src/models/user.g.dart +++ b/angel_orm_test/lib/src/models/user.g.dart @@ -66,7 +66,9 @@ class UserQuery extends Query { trampoline ??= Set(); trampoline.add(tableName); _where = UserQueryWhere(this); - leftJoin(_roles = RoleUserQuery(trampoline: trampoline, parent: this), 'id', + leftJoin( + '(SELECT role_users.user_id , roles.id, roles.created_at, roles.updated_at, roles.name FROM roles LEFT JOIN role_users ON role_users.role_id=roles.id)', + 'id', 'user_id', additionalFields: const ['id', 'created_at', 'updated_at', 'name'], trampoline: trampoline); @@ -77,8 +79,6 @@ class UserQuery extends Query { UserQueryWhere _where; - RoleUserQuery _roles; - @override get casts { return {}; @@ -134,10 +134,6 @@ class UserQuery extends Query { return parseRow(row); } - RoleUserQuery get roles { - return _roles; - } - @override bool canCompile(trampoline) { return (!(trampoline.contains('users') && @@ -405,7 +401,9 @@ class RoleQuery extends Query { trampoline ??= Set(); trampoline.add(tableName); _where = RoleQueryWhere(this); - leftJoin(_users = RoleUserQuery(trampoline: trampoline, parent: this), 'id', + leftJoin( + '(SELECT role_users.role_id , users.id, users.created_at, users.updated_at, users.username, users.password, users.email FROM users LEFT JOIN role_users ON role_users.user_id=users.id)', + 'id', 'role_id', additionalFields: const [ 'id', @@ -423,8 +421,6 @@ class RoleQuery extends Query { RoleQueryWhere _where; - RoleUserQuery _users; - @override get casts { return {}; @@ -471,10 +467,6 @@ class RoleQuery extends Query { return parseRow(row); } - RoleUserQuery get users { - return _users; - } - @override bool canCompile(trampoline) { return (!(trampoline.contains('roles') &&