Skip to content

Conversation

@inuEbisu
Copy link
Contributor

@inuEbisu inuEbisu commented Aug 30, 2025

Current Progress

Implementing json_serializable + JsonToStringMixin for core/ctap2 and src/server entities and requests:

  • Replace handwritten toString with JsonToStringMixin
  • Generate toJson via json_serializable (createFactory: false, no fromJson needed)
  • No behavioral changes; purely for logging/serialization consistency
  • Configure github workflow CI for dart run build_runner build check.

Special Cases

Two classes remain with manual toJson implementation:

  • CoseKey: extends MapView<int, dynamic>, json_serializable incompatible. Manual toJson handles numeric key conversion.
  • CtapError: extends Error, inherited StackTrace causes generation issues. Simple manual toJson for status + name.

@inuEbisu inuEbisu marked this pull request as ready for review September 1, 2025 09:23
@inuEbisu inuEbisu changed the title refactor: migrate to json_serializable-based toJson and mixin toString (WIP) refactor: migrate to json_serializable-based toJson and mixin toString Sep 1, 2025
@Harry-Chen Harry-Chen requested a review from Copilot September 1, 2025 13:13
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR refactors the codebase to use json_serializable for automatic toJson generation and introduces a JsonToStringMixin for consistent toString behavior across entities and requests. It replaces manual toString implementations with JSON-based serialization while maintaining the same logging/debugging functionality.

  • Migrates 20+ classes to use @JsonSerializable annotations with automatic toJson generation
  • Introduces JsonToStringMixin that provides toString() via jsonEncode(toJson())
  • Adds CI workflow validation to ensure generated code stays in sync

Reviewed Changes

Copilot reviewed 30 out of 30 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
pubspec.yaml Adds json_annotation, json_serializable, and build_runner dependencies
lib/src/utils/serialization.dart Introduces JsonToStringMixin for consistent JSON-based toString
Generated .g.dart files Auto-generated toJson factory functions for annotated classes
CTAP2 entities and requests Converts manual toString to JsonSerializable + JsonToStringMixin
Server entities Migrates server-side data classes to use json_serializable
lib/src/cose.dart Updates CoseKey with custom toJson due to MapView inheritance
lib/src/ctap.dart Converts CtapResponse and CtapError to use new serialization pattern
.github/workflows/test.yml Adds build_runner validation to CI
example/json_serializable_example.dart Demonstrates new serialization capabilities

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@Harry-Chen Harry-Chen merged commit c6a7fb5 into nfcim:main Sep 1, 2025
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants