From 9c87a8ba60bd487453643897781ef4d999e3c67b Mon Sep 17 00:00:00 2001 From: Isak Date: Fri, 4 Aug 2023 12:12:04 +0200 Subject: [PATCH 1/3] fix: Deprecate database and api keyword. (#1258) --- .../lib/src/protocol/chat_message.yaml | 6 +- .../serverpod/lib/src/protocol/auth_key.yaml | 2 +- .../lib/src/protocol/object_field_scopes.yaml | 4 +- .../class_yaml_definition.dart | 4 ++ .../entity_analyzer/field_scope_test.dart | 68 ++++++++++++++++--- .../field_source_location_test.dart | 5 +- .../indexes_properties_test.dart | 2 +- 7 files changed, 73 insertions(+), 18 deletions(-) diff --git a/modules/serverpod_chat/serverpod_chat_server/lib/src/protocol/chat_message.yaml b/modules/serverpod_chat/serverpod_chat_server/lib/src/protocol/chat_message.yaml index 457440297e..7d7447f049 100644 --- a/modules/serverpod_chat/serverpod_chat_server/lib/src/protocol/chat_message.yaml +++ b/modules/serverpod_chat/serverpod_chat_server/lib/src/protocol/chat_message.yaml @@ -15,16 +15,16 @@ fields: sender: int ### Information about the sender. - senderInfo: module:auth:UserInfoPublic?, api + senderInfo: module:auth:UserInfoPublic?, !persist ### True, if this message has been removed. removed: bool ### The client message id, used to track if a message has been delivered. - clientMessageId: int?, api + clientMessageId: int?, !persist ### True if the message has been sent. - sent: bool?, api + sent: bool?, !persist ### List of attachments associated with this message. attachments: List? diff --git a/packages/serverpod/lib/src/protocol/auth_key.yaml b/packages/serverpod/lib/src/protocol/auth_key.yaml index 551883370e..0f394006d3 100644 --- a/packages/serverpod/lib/src/protocol/auth_key.yaml +++ b/packages/serverpod/lib/src/protocol/auth_key.yaml @@ -10,7 +10,7 @@ fields: hash: String ### The key sent to the server to authenticate. - key: String?, api + key: String?, !persist ### The scopes this key provides access to. scopeNames: List diff --git a/tests/serverpod_test_server/lib/src/protocol/object_field_scopes.yaml b/tests/serverpod_test_server/lib/src/protocol/object_field_scopes.yaml index 5b5e7d5279..aac4bb029c 100644 --- a/tests/serverpod_test_server/lib/src/protocol/object_field_scopes.yaml +++ b/tests/serverpod_test_server/lib/src/protocol/object_field_scopes.yaml @@ -2,5 +2,5 @@ class: ObjectFieldScopes table: object_field_scopes fields: normal: String - api: String?, api - database: String?, database + api: String?, !persist + database: String?, scope=serverOnly diff --git a/tools/serverpod_cli/lib/src/analyzer/entities/yaml_definitions/class_yaml_definition.dart b/tools/serverpod_cli/lib/src/analyzer/entities/yaml_definitions/class_yaml_definition.dart index b9e57298b7..927f12bd2f 100644 --- a/tools/serverpod_cli/lib/src/analyzer/entities/yaml_definitions/class_yaml_definition.dart +++ b/tools/serverpod_cli/lib/src/analyzer/entities/yaml_definitions/class_yaml_definition.dart @@ -89,6 +89,8 @@ class ClassYamlDefinition { ), ValidateNode( Keyword.database, + isDeprecated: true, + alternativeUsageMessage: 'Use "scope=serverOnly" instead.', valueRestriction: BooleanValueRestriction().validate, mutuallyExclusiveKeys: { Keyword.api, @@ -98,6 +100,8 @@ class ClassYamlDefinition { ), ValidateNode( Keyword.api, + isDeprecated: true, + alternativeUsageMessage: 'Use "!persist" instead.', valueRestriction: BooleanValueRestriction().validate, mutuallyExclusiveKeys: { Keyword.database, diff --git a/tools/serverpod_cli/test/analyzer/entities/entity_analyzer/field_scope_test.dart b/tools/serverpod_cli/test/analyzer/entities/entity_analyzer/field_scope_test.dart index 106b688268..854538cf44 100644 --- a/tools/serverpod_cli/test/analyzer/entities/entity_analyzer/field_scope_test.dart +++ b/tools/serverpod_cli/test/analyzer/entities/entity_analyzer/field_scope_test.dart @@ -1,3 +1,4 @@ +import 'package:serverpod_cli/src/analyzer/code_analysis_collector.dart'; import 'package:serverpod_cli/src/analyzer/entities/definitions.dart'; import 'package:serverpod_cli/src/analyzer/entities/entity_analyzer.dart'; import 'package:serverpod_cli/src/generator/code_generation_collector.dart'; @@ -162,13 +163,29 @@ void main() { expect(collector.errors.length, greaterThan(1)); - var error1 = collector.errors[0]; - var error2 = collector.errors[1]; + var message1 = 'The "api" property is mutually exclusive with the ' + '"database" property.'; + var message2 = 'The "database" property is mutually exclusive with the ' + '"api" property.'; - expect(error1.message, - 'The "database" property is mutually exclusive with the "api" property.'); - expect(error2.message, - 'The "api" property is mutually exclusive with the "database" property.'); + var hasDatabaseError = collector.errors.any( + (error) => error.message == message1, + ); + + var hasApiError = collector.errors.any( + (error) => error.message == message2, + ); + + expect( + hasDatabaseError, + isTrue, + reason: 'Expected error message: $message1', + ); + expect( + hasApiError, + isTrue, + reason: 'Expected error message: $message2', + ); }, ); @@ -198,6 +215,20 @@ void main() { [definition], ); + test('then an error is reported', () { + expect(collector.errors, isNotEmpty); + }); + + test('then an deprecated error message is reported.', () { + var error = collector.errors.first as SourceSpanSeverityException; + expect( + error.message, + 'The "database" property is deprecated. Use "scope=serverOnly" instead.', + ); + + expect(error.severity, SourceSpanSeverity.info); + }, skip: collector.errors.isEmpty); + test('then the generated entity has the serverOnly scope.', () { expect( definition.fields.last.scope, @@ -233,6 +264,20 @@ void main() { [definition], ); + test('then an error is reported', () { + expect(collector.errors, isNotEmpty); + }); + + test('then an deprecated error message is reported.', () { + var error = collector.errors.first as SourceSpanSeverityException; + expect( + error.message, + 'The "api" property is deprecated. Use "!persist" instead.', + ); + + expect(error.severity, SourceSpanSeverity.info); + }, skip: collector.errors.isEmpty); + test('then the generated entity should not be persisted.', () { expect( definition.fields.last.shouldPersist, @@ -336,10 +381,15 @@ void main() { greaterThan(0), reason: 'Expected an error, none was found.', ); - expect( - collector.errors.last.message, - 'The "api" property is mutually exclusive with the "parent" property.', + + var message = + 'The "api" property is mutually exclusive with the "parent" property.'; + + var hasError = collector.errors.any( + (error) => error.message == message, ); + + expect(hasError, isTrue, reason: 'Expected error message: $message'); }, ); }); diff --git a/tools/serverpod_cli/test/analyzer/entities/entity_analyzer/field_source_location_test.dart b/tools/serverpod_cli/test/analyzer/entities/entity_analyzer/field_source_location_test.dart index 8dda9b3118..fe132a64c8 100644 --- a/tools/serverpod_cli/test/analyzer/entities/entity_analyzer/field_source_location_test.dart +++ b/tools/serverpod_cli/test/analyzer/entities/entity_analyzer/field_source_location_test.dart @@ -198,7 +198,8 @@ fields: class: Example table: example fields: - nameId: int, database, + nameId: int, !persist, + ''', Uri(path: 'lib/src/protocol/example.yaml'), ['lib', 'src', 'protocol'], @@ -286,7 +287,7 @@ fields: class: Example table: example fields: - nameId: Invalid-Type, database + nameId: Invalid-Type, !persist ''', Uri(path: 'lib/src/protocol/example.yaml'), ['lib', 'src', 'protocol'], diff --git a/tools/serverpod_cli/test/analyzer/entities/entity_analyzer/indexes_properties_test.dart b/tools/serverpod_cli/test/analyzer/entities/entity_analyzer/indexes_properties_test.dart index c1df90f1a8..721088888c 100644 --- a/tools/serverpod_cli/test/analyzer/entities/entity_analyzer/indexes_properties_test.dart +++ b/tools/serverpod_cli/test/analyzer/entities/entity_analyzer/indexes_properties_test.dart @@ -315,7 +315,7 @@ indexes: expect(collector.errors.length, greaterThan(0)); - var error = collector.errors.first; + var error = collector.errors.last; expect( error.message, From fc70ba23459aac602677cb4845a75932ca9548dd Mon Sep 17 00:00:00 2001 From: Isakdl Date: Fri, 4 Aug 2023 22:17:11 +0200 Subject: [PATCH 2/3] fix: weird bug with this conversion the bug stops. --- .../entities/converter/converter.dart | 21 ++++--- .../entities/converter/converter_test.dart | 60 +++++++++++++++++++ 2 files changed, 74 insertions(+), 7 deletions(-) create mode 100644 tools/serverpod_cli/test/analyzer/entities/converter/converter_test.dart diff --git a/tools/serverpod_cli/lib/src/analyzer/entities/converter/converter.dart b/tools/serverpod_cli/lib/src/analyzer/entities/converter/converter.dart index b4a82181b6..b01fb7a727 100644 --- a/tools/serverpod_cli/lib/src/analyzer/entities/converter/converter.dart +++ b/tools/serverpod_cli/lib/src/analyzer/entities/converter/converter.dart @@ -108,9 +108,12 @@ Iterable> _extractKeyValuePairs( void Function(String key, SourceSpan? span)? onNegatedKeyWithValue, required DeepNestedNodeHandler handleDeepNestedNodes, }) { + print('fieldOptions'); if (content == null) return []; - var fieldPairs = fieldOptions.map((stringifiedKeyValuePair) { + List> fieldPairs = []; + + for (var stringifiedKeyValuePair in fieldOptions) { var keyValueSpan = _extractSubSpan(content, span, stringifiedKeyValuePair); if (_hasNestedStringifiedValues(stringifiedKeyValuePair)) { @@ -121,16 +124,18 @@ Iterable> _extractKeyValuePairs( var stringifiedContent = nestedComponents.last; if (stringifiedContent == '') { - return _createdYamlScalarNode( + fieldPairs.add(_createdYamlScalarNode( key, null, keyValueSpan, - ); + )); + continue; } else { var nestedSpan = _extractSubSpan(content, span, stringifiedContent); var nodeMap = handleDeepNestedNodes(stringifiedContent, nestedSpan); - return _createYamlMapNode(key, nodeMap, keyValueSpan); + fieldPairs.add(_createYamlMapNode(key, nodeMap, keyValueSpan)); + continue; } } @@ -155,13 +160,15 @@ Iterable> _extractKeyValuePairs( } } - return _createdYamlScalarNode( + fieldPairs.add(_createdYamlScalarNode( key, value, keyValueSpan, - ); - }); + )); + continue; + } + print(fieldPairs); return fieldPairs; } diff --git a/tools/serverpod_cli/test/analyzer/entities/converter/converter_test.dart b/tools/serverpod_cli/test/analyzer/entities/converter/converter_test.dart new file mode 100644 index 0000000000..b60a77e4c1 --- /dev/null +++ b/tools/serverpod_cli/test/analyzer/entities/converter/converter_test.dart @@ -0,0 +1,60 @@ +import 'package:serverpod_cli/src/analyzer/entities/converter/converter.dart'; +import 'package:test/test.dart'; +import 'package:yaml/yaml.dart'; + +main() { + test(' ', () { + var document = loadYamlDocument( + ''' + field: String, !relation=value + ''', + ); + + YamlMap node = document.contents as YamlMap; + + var contentNode = node.nodes['field']!; + var content = contentNode.value; + + var yamlMap = convertStringifiedNestedNodesToYamlMap( + content, + contentNode.span, + firstKey: 'type', + onDuplicateKey: (key, span) { + print('Duplicate key: $key'); + }, + onNegatedKeyWithValue: (key, span) { + print('Negated key with value: $key'); + }, + ); + + print('yamlMap: $yamlMap'); + }); + + + test(' ', () { + var document = loadYamlDocument( + ''' + field: String, relation(!key=value, duplicated, duplicated) + ''', + ); + + YamlMap node = document.contents as YamlMap; + + var contentNode = node.nodes['field']!; + var content = contentNode.value; + + var yamlMap = convertStringifiedNestedNodesToYamlMap( + content, + contentNode.span, + firstKey: 'type', + onDuplicateKey: (key, span) { + print('Duplicate key: $key'); + }, + onNegatedKeyWithValue: (key, span) { + print('Negated key with value: $key'); + }, + ); + + print('yamlMap: $yamlMap'); + }); +} From ecfe2bdd3e056f8bb94230301b75333c89af4021 Mon Sep 17 00:00:00 2001 From: Isakdl Date: Fri, 4 Aug 2023 22:20:17 +0200 Subject: [PATCH 3/3] remove: shit --- .../test/analyzer/entities/converter/converter_test.dart | 4 ---- 1 file changed, 4 deletions(-) diff --git a/tools/serverpod_cli/test/analyzer/entities/converter/converter_test.dart b/tools/serverpod_cli/test/analyzer/entities/converter/converter_test.dart index b60a77e4c1..9192f9a938 100644 --- a/tools/serverpod_cli/test/analyzer/entities/converter/converter_test.dart +++ b/tools/serverpod_cli/test/analyzer/entities/converter/converter_test.dart @@ -26,8 +26,6 @@ main() { print('Negated key with value: $key'); }, ); - - print('yamlMap: $yamlMap'); }); @@ -54,7 +52,5 @@ main() { print('Negated key with value: $key'); }, ); - - print('yamlMap: $yamlMap'); }); }