Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rework serialization #327

Merged
merged 103 commits into from Nov 15, 2022
Merged

Conversation

fischerscode
Copy link
Contributor

@fischerscode fischerscode commented Oct 9, 2022

This PR reworks the serialization system.

Main changes:

  • Serialized objects are not warped with a class name any more. For deserialization the expected Type is provided to the system. For each type encountered when running serverpod generate a deserializer is added to the SerializationManager. (streamMessages are still warped. Since there is no type context available.)
  • (De)-Serialization is done recursively. Nested Lists/Maps... are supported.
  • Imports in generated files are scoped.
  • Protocol definitions (yaml) allow referencing data types from other packages. (prefix type with serverpod: or protocol: or module:SERVERPOD_MODULE:, VALID_DART_IMPORT:)
  • Store enums as integers in the database

Advantages:

  • More efficient network traffic
  • Nested Lists...
  • Easier to maintain (adding new supported types should be straight forward)
  • Even classes that are not generated by serverpod can be send. (Allows developers to use freezed)

#309

Pre-launch Checklist

  • I read the Contribute page and followed the process outlined there for submitting PRs.
  • I read and followed the Dart Style Guide and formatted the code with dart format.
  • I listed at least one issue that this PR fixes in the description above.
  • I updated/added relevant documentation (doc comments with ///), and made sure that the documentation follows the same style as other Serverpod documentation. I checked spelling and grammar.
  • I added new tests to check the change I am making.
  • All existing and new tests are passing.
  • Any breaking changes are documented below.
  • Delete old stuff, that isn't used any more

If you need help, consider asking for advice on the discussion board.

Breaking changes

@vlidholt
Copy link
Collaborator

@fischerscode Hi! I saw the PR. Unfortunately, I won't have time to look through it in detail until Friday this week as I'm preparing a large Flutter/Serverpod event in Stockholm on Thursday. Overall, your work looks very cool. Love the idea of making the serialization compatible with Freezed and many of your other improvements.

Would you be available for a quick chat on video sometime next days? Would love to synch up. Feel free to add me on Linkedin and we can take it from there: https://www.linkedin.com/in/viktorlidholt/

@vlidholt
Copy link
Collaborator

vlidholt commented Nov 10, 2022

I created a new branch for this with my updates here:
https://github.com/serverpod/serverpod/tree/serialization

I fixed most of the issues now. I think that the only thing left to do is the custom constructors.

@fischerscode fischerscode changed the base branch from dev to serialization November 11, 2022 14:42
Comment on lines 47 to 59
for (var sessionCallback in channel) {
try {
sessionCallback(message);
} on TypeError catch (_) {
// Ignore since the developer has added callbacks taking different types to this Channel.
} catch (e, stackTrace) {
sessionCallback.session.log(
'Failed to execute callback in channel $channelName',
exception: e,
level: LogLevel.error,
stackTrace: stackTrace,
);
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd have added this part. Should I readd it?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please make sure that you are pulling my changes before you make any new edits.

@fischerscode
Copy link
Contributor Author

I created a new branch for this with my updates here: https://github.com/serverpod/serverpod/tree/serialization

This PR points to your branch now.

I fixed most of the issues now. I think that the only thing left to do is the custom constructors.

#327 (comment)

A constructor named .fromJson(dynamic)

I'd prefer .fromJson(dynamic data, SerializationManager serializationManager). (That's also how it is at the moment.) (Constructor, factory or static method are fine)

@vlidholt
Copy link
Collaborator

Will that work with Freezed?

@fischerscode
Copy link
Contributor Author

Will that work with Freezed?

import 'package:freezed_annotation/freezed_annotation.dart';
import 'package:serverpod/serverpod.dart';

part 'freezedtype.freezed.dart';
part 'freezedtype.g.dart';

@freezed
class FreezedType with _$FreezedType {
  const factory FreezedType({
    required String firstName,
    required String lastName,
    required int age,
  }) = _FreezedType;

  factory FreezedType.fromJson(Map<String, Object?> json,
          SerializationManager serializationManager) =>
      _$FreezedTypeFromJson(json);
}

It does. At least there are no analyzer errors.

@fischerscode
Copy link
Contributor Author

fischerscode commented Nov 11, 2022

!!THIS IS JUST A DRAFT!!

I'm surprised it's that short, but I don't think I missed something.

Migrate to serverpod 1.0

Serverpod 1.0 is incompatible with previous versions!
Make sure to rerun serverpod generate.

Protocol specification

Types in your protocol from other modules must be prefixed with module:MODULE_NICKNAME:.
Types in your protocol from serverpod (package:serverpod/serverpod.dart and package:serverpod_client/serverpod_client.dart) must be prefixed with serverpod:.
Types in your protocol from your protocol specification can be prefixed with protocol:. (Should be detected automatically.)
Types in your protocol from your serverpod project must be prefixed with project:.
Types in your protocol from an other package must be prefixed with eg. package:PACKAGENAME/PACKAGENAME.dart.

Cache

Cache.get('key') as DataClass becomes Cache.get<DataClass>('key')
The type must be specified!

FutureCalls

The type of the parameter invoke requires must now be set as an generic of FutureCall.
New syntax:

class ExampleFutureCall extends FutureCall<Example> {
  @override
  Future<void> invoke(Session session, Example? object) async {
    // Do something interesting in the future here.
  }
}

DateTime

From now on, every DateTime handled by serverpod is converted to UTC. This includes returned DateTimes.

Enums

Enums are now serialized and stored in the database as integers.

You can migrate your database by running ALTER TABLE table_name ALTER COLUMN column_name TYPE integer USING (column_name->'data'->>'index')::integer;.

In case the enum detection does not work for your edge case you can add enum to your field specification.

@fischerscode fischerscode marked this pull request as ready for review November 11, 2022 20:40
@fischerscode fischerscode changed the base branch from serialization to dev November 11, 2022 20:42
@fischerscode
Copy link
Contributor Author

I think it's done.

I added a small migration guide. But I have no project to test if something else is necessary.

@fischerscode
Copy link
Contributor Author

@vlidholt Just a friendly ping, since I'm not sure if you've seen it.

@vlidholt
Copy link
Collaborator

  1. The syntax for including custom classes is still not correctly updated to my proposal.
  2. There should be no need to define an extra constructor for nullable types. Constructors generally do not return nullable objects so we shouldn't require users to define an extra factory method for that. This needs to be reflected in the tests too.

@fischerscode These changes are needed for me to be able to merge this pull request. Will you have the bandwidth to fix it in the next few days or should I go ahead and try to implement the fixes?

@vlidholt
Copy link
Collaborator

Ok, I fixed those things. I changed the syntax for adding extra classes to:

extraClasses:
  - project:src/custom_classes.dart:CustomClass

The deserialization of nullable types still feel a bit like a hack, but I'm going to do some final testing then merge this now.

@vlidholt vlidholt merged commit 069876b into serverpod:dev Nov 15, 2022
@github-actions github-actions bot locked and limited conversation to collaborators Nov 15, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants