fix: Allow only defaultPersist and defaultModel keys on id field (serverpod#3416)

Author: marcelomendoncasoares

packages/serverpod/lib/src/database/sql_query_builder.dart

@@ -323,16 +323,23 @@ class InsertQueryBuilder {
   }
 
   /// Builds the insert SQL query.
-  String build() {
-    var selectedColumns = _table.columns.where(
-      (column) => column.columnName != 'id',
-    );
+  String? _build(bool onlyWithIdNull) {
+    var selectedColumns = onlyWithIdNull
+        ? _table.columns.where((column) => column.columnName != 'id')
+        : _table.columns;
 
     var columnNames =
         selectedColumns.map((e) => '"${e.columnName}"').join(', ');
 
-    var values =
-        _rows.map((row) => row.toJson() as Map<String, dynamic>).map((row) {
+    var filteredValues = onlyWithIdNull
+        ? _rows.where((row) => row.id == null)
+        : _rows.where((row) => row.id != null);
+
+    if (filteredValues.isEmpty) return null;
+
+    var values = filteredValues
+        .map((row) => row.toJson() as Map<String, dynamic>)
+        .map((row) {
       var values = selectedColumns.map((column) {
         var unformattedValue = row[column.columnName];
         return DatabasePoolManager.encoder.convert(
@@ -347,6 +354,23 @@ class InsertQueryBuilder {
         ? 'INSERT INTO "${_table.tableName}" DEFAULT VALUES RETURNING *'
         : 'INSERT INTO "${_table.tableName}" ($columnNames) VALUES $values RETURNING *';
   }
+
+  /// Builds the insert SQL query.
+  String build() {
+    // Can not be empty because the constructor checks for empty rows.
+    var insertQueries = [true, false].map(_build).nonNulls;
+    if (insertQueries.length == 1) return insertQueries.single;
+
+    return '''
+WITH
+  insertWithIdNull AS (${insertQueries.first}),
+  insertWithIdNotNull AS (${insertQueries.last})
+
+SELECT * FROM insertWithIdNull
+UNION ALL
+SELECT * FROM insertWithIdNotNull
+''';
+  }
 }
 
 /// Builds a SQL query for a count statement.

packages/serverpod/test/database/database_query/insert_sql_query_test.dart

@@ -94,4 +94,52 @@ void main() {
       expect(query, 'INSERT INTO "only_id" DEFAULT VALUES RETURNING *');
     });
   });
+
+  test(
+      'Given model with multiple columns and id column having value when building insert query then its id is used in the query.',
+      () {
+    var query = InsertQueryBuilder(
+      table: PersonTable(),
+      rows: [PersonClass(id: 33, name: 'Alex', age: 33)],
+    ).build();
+
+    expect(query,
+        'INSERT INTO "person" ("id", "name", "age") VALUES (33, \'Alex\', 33) RETURNING *');
+  });
+
+  test(
+      'Given model with only id column and id column having value when building insert query then its id is used in the query.',
+      () {
+    var query = InsertQueryBuilder(
+      table: Table<int>(tableName: 'only_id'),
+      rows: [OnlyIdClass(id: 33)],
+    ).build();
+
+    expect(query, 'INSERT INTO "only_id" ("id") VALUES (33) RETURNING *');
+  });
+
+  test(
+      'Given models a list of models with and without id column having value when building insert query then two separate insert queries are generated.',
+      () {
+    var query = InsertQueryBuilder(
+      table: PersonTable(),
+      rows: [
+        PersonClass(id: 33, name: 'Alex', age: 33),
+        PersonClass(name: 'Isak', age: 33),
+      ],
+    ).build();
+
+    expect(
+      query,
+      '''
+WITH
+  insertWithIdNull AS (INSERT INTO "person" ("name", "age") VALUES ('Isak', 33) RETURNING *),
+  insertWithIdNotNull AS (INSERT INTO "person" ("id", "name", "age") VALUES (33, 'Alex', 33) RETURNING *)
+
+SELECT * FROM insertWithIdNull
+UNION ALL
+SELECT * FROM insertWithIdNotNull
+''',
+    );
+  });
 }

tests/serverpod_test_client/lib/src/protocol/changed_id_type/many_to_many/student.dart

@@ -10,20 +10,19 @@
 
 // ignore_for_file: no_leading_underscores_for_library_prefixes
 import 'package:serverpod_client/serverpod_client.dart' as _i1;
-import 'package:uuid/uuid.dart' as _i2;
-import '../../changed_id_type/many_to_many/enrollment.dart' as _i3;
+import '../../changed_id_type/many_to_many/enrollment.dart' as _i2;
 
 abstract class StudentUuid implements _i1.SerializableModel {
   StudentUuid._({
-    _i1.UuidValue? id,
+    this.id,
     required this.name,
     this.enrollments,
-  }) : id = id ?? _i2.Uuid().v4obj();
+  });
 
   factory StudentUuid({
     _i1.UuidValue? id,
     required String name,
-    List<_i3.EnrollmentInt>? enrollments,
+    List<_i2.EnrollmentInt>? enrollments,
   }) = _StudentUuidImpl;
 
   factory StudentUuid.fromJson(Map<String, dynamic> jsonSerialization) {
@@ -33,7 +32,7 @@ abstract class StudentUuid implements _i1.SerializableModel {
           : _i1.UuidValueJsonExtension.fromJson(jsonSerialization['id']),
       name: jsonSerialization['name'] as String,
       enrollments: (jsonSerialization['enrollments'] as List?)
-          ?.map((e) => _i3.EnrollmentInt.fromJson((e as Map<String, dynamic>)))
+          ?.map((e) => _i2.EnrollmentInt.fromJson((e as Map<String, dynamic>)))
           .toList(),
     );
   }
@@ -45,15 +44,15 @@ abstract class StudentUuid implements _i1.SerializableModel {
 
   String name;
 
-  List<_i3.EnrollmentInt>? enrollments;
+  List<_i2.EnrollmentInt>? enrollments;
 
   /// Returns a shallow copy of this [StudentUuid]
   /// with some or all fields replaced by the given arguments.
   @_i1.useResult
   StudentUuid copyWith({
     _i1.UuidValue? id,
     String? name,
-    List<_i3.EnrollmentInt>? enrollments,
+    List<_i2.EnrollmentInt>? enrollments,
   });
   @override
   Map<String, dynamic> toJson() {
@@ -77,7 +76,7 @@ class _StudentUuidImpl extends StudentUuid {
   _StudentUuidImpl({
     _i1.UuidValue? id,
     required String name,
-    List<_i3.EnrollmentInt>? enrollments,
+    List<_i2.EnrollmentInt>? enrollments,
   }) : super._(
           id: id,
           name: name,
@@ -96,7 +95,7 @@ class _StudentUuidImpl extends StudentUuid {
     return StudentUuid(
       id: id is _i1.UuidValue? ? id : this.id,
       name: name ?? this.name,
-      enrollments: enrollments is List<_i3.EnrollmentInt>?
+      enrollments: enrollments is List<_i2.EnrollmentInt>?
           ? enrollments
           : this.enrollments?.map((e0) => e0.copyWith()).toList(),
     );

tests/serverpod_test_client/lib/src/protocol/changed_id_type/nested_one_to_many/player.dart

@@ -10,22 +10,21 @@
 
 // ignore_for_file: no_leading_underscores_for_library_prefixes
 import 'package:serverpod_client/serverpod_client.dart' as _i1;
-import 'package:uuid/uuid.dart' as _i2;
-import '../../changed_id_type/nested_one_to_many/team.dart' as _i3;
+import '../../changed_id_type/nested_one_to_many/team.dart' as _i2;
 
 abstract class PlayerUuid implements _i1.SerializableModel {
   PlayerUuid._({
-    _i1.UuidValue? id,
+    this.id,
     required this.name,
     this.teamId,
     this.team,
-  }) : id = id ?? _i2.Uuid().v4obj();
+  });
 
   factory PlayerUuid({
     _i1.UuidValue? id,
     required String name,
     int? teamId,
-    _i3.TeamInt? team,
+    _i2.TeamInt? team,
   }) = _PlayerUuidImpl;
 
   factory PlayerUuid.fromJson(Map<String, dynamic> jsonSerialization) {
@@ -37,7 +36,7 @@ abstract class PlayerUuid implements _i1.SerializableModel {
       teamId: jsonSerialization['teamId'] as int?,
       team: jsonSerialization['team'] == null
           ? null
-          : _i3.TeamInt.fromJson(
+          : _i2.TeamInt.fromJson(
               (jsonSerialization['team'] as Map<String, dynamic>)),
     );
   }
@@ -51,7 +50,7 @@ abstract class PlayerUuid implements _i1.SerializableModel {
 
   int? teamId;
 
-  _i3.TeamInt? team;
+  _i2.TeamInt? team;
 
   /// Returns a shallow copy of this [PlayerUuid]
   /// with some or all fields replaced by the given arguments.
@@ -60,7 +59,7 @@ abstract class PlayerUuid implements _i1.SerializableModel {
     _i1.UuidValue? id,
     String? name,
     int? teamId,
-    _i3.TeamInt? team,
+    _i2.TeamInt? team,
   });
   @override
   Map<String, dynamic> toJson() {
@@ -85,7 +84,7 @@ class _PlayerUuidImpl extends PlayerUuid {
     _i1.UuidValue? id,
     required String name,
     int? teamId,
-    _i3.TeamInt? team,
+    _i2.TeamInt? team,
   }) : super._(
           id: id,
           name: name,
@@ -107,7 +106,7 @@ class _PlayerUuidImpl extends PlayerUuid {
       id: id is _i1.UuidValue? ? id : this.id,
       name: name ?? this.name,
       teamId: teamId is int? ? teamId : this.teamId,
-      team: team is _i3.TeamInt? ? team : this.team?.copyWith(),
+      team: team is _i2.TeamInt? ? team : this.team?.copyWith(),
     );
   }
 }

tests/serverpod_test_client/lib/src/protocol/changed_id_type/one_to_one/company.dart

@@ -10,22 +10,21 @@
 
 // ignore_for_file: no_leading_underscores_for_library_prefixes
 import 'package:serverpod_client/serverpod_client.dart' as _i1;
-import 'package:uuid/uuid.dart' as _i2;
-import '../../changed_id_type/one_to_one/town.dart' as _i3;
+import '../../changed_id_type/one_to_one/town.dart' as _i2;
 
 abstract class CompanyUuid implements _i1.SerializableModel {
   CompanyUuid._({
-    _i1.UuidValue? id,
+    this.id,
     required this.name,
     required this.townId,
     this.town,
-  }) : id = id ?? _i2.Uuid().v4obj();
+  });
 
   factory CompanyUuid({
     _i1.UuidValue? id,
     required String name,
     required int townId,
-    _i3.TownInt? town,
+    _i2.TownInt? town,
   }) = _CompanyUuidImpl;
 
   factory CompanyUuid.fromJson(Map<String, dynamic> jsonSerialization) {
@@ -37,7 +36,7 @@ abstract class CompanyUuid implements _i1.SerializableModel {
       townId: jsonSerialization['townId'] as int,
       town: jsonSerialization['town'] == null
           ? null
-          : _i3.TownInt.fromJson(
+          : _i2.TownInt.fromJson(
               (jsonSerialization['town'] as Map<String, dynamic>)),
     );
   }
@@ -51,7 +50,7 @@ abstract class CompanyUuid implements _i1.SerializableModel {
 
   int townId;
 
-  _i3.TownInt? town;
+  _i2.TownInt? town;
 
   /// Returns a shallow copy of this [CompanyUuid]
   /// with some or all fields replaced by the given arguments.
@@ -60,7 +59,7 @@ abstract class CompanyUuid implements _i1.SerializableModel {
     _i1.UuidValue? id,
     String? name,
     int? townId,
-    _i3.TownInt? town,
+    _i2.TownInt? town,
   });
   @override
   Map<String, dynamic> toJson() {
@@ -85,7 +84,7 @@ class _CompanyUuidImpl extends CompanyUuid {
     _i1.UuidValue? id,
     required String name,
     required int townId,
-    _i3.TownInt? town,
+    _i2.TownInt? town,
   }) : super._(
           id: id,
           name: name,
@@ -107,7 +106,7 @@ class _CompanyUuidImpl extends CompanyUuid {
       id: id is _i1.UuidValue? ? id : this.id,
       name: name ?? this.name,
       townId: townId ?? this.townId,
-      town: town is _i3.TownInt? ? town : this.town?.copyWith(),
+      town: town is _i2.TownInt? ? town : this.town?.copyWith(),
     );
   }
 }