feat: Global default enum value to prevent breaking the client (serverpod#3341)

Author: vanlooverenkoen

tests/serverpod_test_client/lib/src/protocol/defaults/enum/enums/default_value_enum.dart

@@ -0,0 +1,33 @@
+/* AUTOMATICALLY GENERATED CODE DO NOT MODIFY */
+/*   To generate run: "serverpod generate"    */
+
+// ignore_for_file: implementation_imports
+// ignore_for_file: library_private_types_in_public_api
+// ignore_for_file: non_constant_identifier_names
+// ignore_for_file: public_member_api_docs
+// ignore_for_file: type_literal_in_constant_pattern
+// ignore_for_file: use_super_parameters
+
+// ignore_for_file: no_leading_underscores_for_library_prefixes
+import 'package:serverpod_client/serverpod_client.dart' as _i1;
+
+enum DefaultValueEnum implements _i1.SerializableModel {
+  value1,
+  value2;
+
+  static DefaultValueEnum fromJson(int index) {
+    switch (index) {
+      case 0:
+        return DefaultValueEnum.value1;
+      case 1:
+        return DefaultValueEnum.value2;
+      default:
+        return DefaultValueEnum.value2;
+    }
+  }
+
+  @override
+  int toJson() => index;
+  @override
+  String toString() => name;
+}

tests/serverpod_test_client/lib/src/protocol/protocol.dart

@@ -0,0 +1,33 @@
+/* AUTOMATICALLY GENERATED CODE DO NOT MODIFY */
+/*   To generate run: "serverpod generate"    */
+
+// ignore_for_file: implementation_imports
+// ignore_for_file: library_private_types_in_public_api
+// ignore_for_file: non_constant_identifier_names
+// ignore_for_file: public_member_api_docs
+// ignore_for_file: type_literal_in_constant_pattern
+// ignore_for_file: use_super_parameters
+
+// ignore_for_file: no_leading_underscores_for_library_prefixes
+import 'package:serverpod/serverpod.dart' as _i1;
+
+enum DefaultValueEnum implements _i1.SerializableModel {
+  value1,
+  value2;
+
+  static DefaultValueEnum fromJson(int index) {
+    switch (index) {
+      case 0:
+        return DefaultValueEnum.value1;
+      case 1:
+        return DefaultValueEnum.value2;
+      default:
+        return DefaultValueEnum.value2;
+    }
+  }
+
+  @override
+  int toJson() => index;
+  @override
+  String toString() => name;
+}

tests/serverpod_test_server/lib/src/generated/defaults/enum/enums/default_value_enum.dart

@@ -0,0 +1,5 @@
+enum: DefaultValueEnum
+default: value2
+values:
+  - value1
+  - value2

tests/serverpod_test_server/lib/src/generated/protocol.dart

@@ -139,5 +139,42 @@ void main() async {
         );
       },
     );
+
+    group('Given a class with "default" enum fields,', () {
+      test(
+        'when deserializing an invalid index for DefaultValueEnum, it should default to value2',
+        () async {
+          var object = DefaultValueEnum.fromJson(-1);
+          expect(DefaultValueEnum.values.length, 2);
+          expect(object, DefaultValueEnum.value2);
+        },
+      );
+
+      test(
+        'when deserializing an invalid index for ByIndexEnum, it should throw an ArgumentError',
+        () async {
+          expect(
+            () => ByIndexEnum.fromJson(-1),
+            throwsA(predicate((e) =>
+                e is ArgumentError &&
+                e.message ==
+                    'Value "-1" cannot be converted to "ByIndexEnum"')),
+          );
+        },
+      );
+
+      test(
+        'when deserializing an invalid name for ByNameEnum, it should throw an ArgumentError',
+        () async {
+          expect(
+            () => ByNameEnum.fromJson('Invalid'),
+            throwsA(predicate((e) =>
+                e is ArgumentError &&
+                e.message ==
+                    'Value "Invalid" cannot be converted to "ByNameEnum"')),
+          );
+        },
+      );
+    });
   });
 }

tests/serverpod_test_server/lib/src/models/defaults/enum/enums/default_value_enum.spy.yaml

@@ -342,6 +342,9 @@ class EnumDefinition extends SerializableModelDefinition {
   /// The type of serialization this enum should use.
   final EnumSerialization serialized;
 
+  /// The default value of the enum when parsing of the enum fails.
+  final ProtocolEnumValueDefinition? defaultValue;
+
   /// All the values of the enum.
   /// This also contains possible documentation for them.
   List<ProtocolEnumValueDefinition> values;
@@ -355,6 +358,7 @@ class EnumDefinition extends SerializableModelDefinition {
     required super.sourceFileName,
     required super.className,
     required this.serialized,
+    required this.defaultValue,
     required this.values,
     required super.serverOnly,
     required super.type,

tests/serverpod_test_server/test_integration/database_operations/default/enum/enum_default_test.dart

@@ -181,6 +181,8 @@ class ModelParser {
       '${protocolSource.moduleAlias}:$className',
       extraClasses: [],
     );
+    var defaultEnumDefinitionValue =
+        _parseEnumDefaultValue(documentContents, values);
 
     var enumDef = EnumDefinition(
       fileName: outFileName,
@@ -189,6 +191,7 @@ class ModelParser {
       values: values,
       serialized: serializeAs,
       documentation: enumDocumentation,
+      defaultValue: defaultEnumDefinitionValue,
       subDirParts: protocolSource.subDirPathParts,
       serverOnly: serverOnly,
       type: enumType,
@@ -605,6 +608,15 @@ class ModelParser {
     return nodeValue is bool ? nodeValue : false;
   }
 
+  static ProtocolEnumValueDefinition? _parseEnumDefaultValue(
+    YamlMap documentContents,
+    List<ProtocolEnumValueDefinition> values,
+  ) {
+    final defaultValue =
+        _parseDefaultValue(documentContents, Keyword.defaultKey);
+    return values.where((value) => value.name == defaultValue).firstOrNull;
+  }
+
   static List<ProtocolEnumValueDefinition> _parseEnumValues(
     YamlMap documentContents,
     YamlDocumentationExtractor docsExtractor,

tools/serverpod_cli/lib/src/analyzer/models/definitions.dart

@@ -1233,6 +1233,32 @@ class Restrictions {
     return [];
   }
 
+  List<SourceSpanSeverityException> validateEnumDefaultValue(
+    String parentNodeName,
+    dynamic content,
+    SourceSpan? span,
+  ) {
+    if (content is! String) {
+      return [
+        SourceSpanSeverityException(
+          'The "default" property must be a String.',
+          span,
+        )
+      ];
+    }
+
+    var definition = documentDefinition;
+    if (definition is! EnumDefinition) return [];
+    final values = definition.values;
+    if (values.any((value) => value.name == content)) return [];
+    return [
+      SourceSpanSeverityException(
+        '"$content" is not a valid default value. Allowed values are: ${values.map((e) => e.name).join(', ')}.',
+        span,
+      )
+    ];
+  }
+
   List<SourceSpanSeverityException> validateEnumValues(
     String parentNodeName,
     dynamic content,

tools/serverpod_cli/lib/src/analyzer/models/model_parser/model_parser.dart

@@ -19,6 +19,10 @@ class EnumYamlDefinition {
         valueRestriction:
             EnumValueRestriction(enums: EnumSerialization.values).validate,
       ),
+      ValidateNode(
+        Keyword.defaultKey,
+        valueRestriction: restrictions.validateEnumDefaultValue,
+      ),
       ValidateNode(
         Keyword.serverOnly,
         valueRestriction: BooleanValueRestriction().validate,

tools/serverpod_cli/lib/src/analyzer/models/validation/restrictions.dart

@@ -2393,9 +2393,7 @@ class SerializableModelLibraryGenerator {
                   Code(
                     'case $i: return ${enumDefinition.className}.${enumDefinition.values[i].name};',
                   ),
-                Code(
-                  'default: throw ArgumentError(\'Value "\$index" cannot be converted to "${enumDefinition.className}"\');',
-                ),
+                _buildDefaultSwitchCase(enumDefinition, 'index'),
                 const Code('}'),
               ]))
             .build()),
@@ -2429,9 +2427,7 @@ class SerializableModelLibraryGenerator {
                   Code(
                     "case '${value.name}': return ${enumDefinition.className}.${value.name};",
                   ),
-                Code(
-                  'default: throw ArgumentError(\'Value "\$name" cannot be converted to "${enumDefinition.className}"\');',
-                ),
+                _buildDefaultSwitchCase(enumDefinition, 'name'),
                 const Code('}'),
               ]))
             .build()),
@@ -2476,6 +2472,18 @@ class SerializableModelLibraryGenerator {
 
     return refer(field.name);
   }
+
+  Code _buildDefaultSwitchCase(
+      EnumDefinition enumDefinition, String valueFieldName) {
+    if (enumDefinition.defaultValue == null) {
+      return Code(
+        'default: throw ArgumentError(\'Value "\$$valueFieldName" cannot be converted to "${enumDefinition.className}"\');',
+      );
+    }
+    return Code(
+      'default: return ${enumDefinition.className}.${enumDefinition.defaultValue!.name};',
+    );
+  }
 }
 
 class SimpleData {}

tools/serverpod_cli/lib/src/analyzer/models/yaml_definitions/enum_yaml_definition.dart

@@ -1,6 +1,7 @@
 import 'package:serverpod_cli/analyzer.dart';
 import 'package:serverpod_cli/src/analyzer/models/stateful_analyzer.dart';
 import 'package:serverpod_cli/src/generator/code_generation_collector.dart';
+import 'package:serverpod_service_client/serverpod_service_client.dart';
 import 'package:test/test.dart';
 
 import '../../../../test_util/builders/generator_config_builder.dart';
@@ -54,6 +55,8 @@ void main() {
 
     expect(model.type.className, model.className);
     expect(model.type.enumDefinition, model);
+    expect(model.defaultValue, isNull);
+    expect(model.serialized, EnumSerialization.byIndex);
   });
 
   test(

tools/serverpod_cli/lib/src/generator/dart/library_generators/model_library_generator.dart

@@ -0,0 +1,117 @@
+import 'package:serverpod_cli/src/analyzer/models/definitions.dart';
+import 'package:serverpod_cli/src/analyzer/models/stateful_analyzer.dart';
+import 'package:serverpod_cli/src/generator/code_generation_collector.dart';
+import 'package:test/test.dart';
+
+import '../../../../test_util/builders/generator_config_builder.dart';
+import '../../../../test_util/builders/model_source_builder.dart';
+
+void main() {
+  var config = GeneratorConfigBuilder().build();
+
+  group(
+      'Given a valid enum definition with default value set to valid value when validating',
+      () {
+    late CodeGenerationCollector collector;
+    late EnumDefinition definition;
+    setUp(() {
+      var modelSources = [
+        ModelSourceBuilder().withYaml(
+          '''
+        enum: ExampleEnum
+        default: first
+        values:
+          - first
+          - second
+        ''',
+        ).build()
+      ];
+
+      collector = CodeGenerationCollector();
+      var analyzer =
+          StatefulAnalyzer(config, modelSources, onErrorsCollector(collector));
+
+      var definitions = analyzer.validateAll();
+      definition = definitions.first as EnumDefinition;
+    });
+
+    test('then no errors are collected', () {
+      expect(collector.errors, isEmpty);
+    });
+
+    test('then defaultValue is set to valid value.', () {
+      expect(definition.defaultValue, isNotNull);
+      expect(definition.defaultValue!.name, definition.values.first.name);
+    });
+  });
+
+  group(
+      'Given a valid enum definition with default value set to an invalid String value when validating',
+      () {
+    late CodeGenerationCollector collector;
+
+    setUp(() {
+      var modelSources = [
+        ModelSourceBuilder().withYaml(
+          '''
+        enum: ExampleEnum
+        default: Invalid
+        values:
+          - first
+          - second
+        ''',
+        ).build()
+      ];
+
+      collector = CodeGenerationCollector();
+      var analyzer =
+          StatefulAnalyzer(config, modelSources, onErrorsCollector(collector));
+
+      analyzer.validateAll();
+    });
+
+    test('then 1 error is collected', () {
+      expect(collector.errors, isNotEmpty);
+      expect(collector.errors.length, 1);
+      expect(
+        collector.errors.first.message,
+        '"Invalid" is not a valid default value. Allowed values are: first, second.',
+      );
+    });
+  });
+
+  group(
+      'Given a valid enum definition with default value set to and invalid int value when validating',
+      () {
+    late CodeGenerationCollector collector;
+
+    setUp(() {
+      var modelSources = [
+        ModelSourceBuilder().withYaml(
+          '''
+        enum: ExampleEnum
+        default: 0
+        values:
+          - first
+          - second
+        ''',
+        ).build()
+      ];
+
+      collector = CodeGenerationCollector();
+      var analyzer =
+          StatefulAnalyzer(config, modelSources, onErrorsCollector(collector));
+
+      analyzer.validateAll();
+    });
+
+    test('then 1 error is collected', () {
+      expect(collector.errors, isNotEmpty);
+      expect(collector.errors.length, 1);
+      expect(
+        collector.errors.first.message,
+        'The "default" property must be a String.',
+      );
+    });
+  });
+}

tools/serverpod_cli/test/analyzer/models/stateful_analyzer/model_validation/class_type_test.dart

@@ -442,7 +442,7 @@ void main() {
         var error = collector.errors.first;
         expect(
           error.message,
-          'The "table" property is not allowed for enum type. Valid keys are {enum, serialized, serverOnly, values}.',
+          'The "table" property is not allowed for enum type. Valid keys are {enum, serialized, default, serverOnly, values}.',
         );
       },
     );