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

Serializing/deserializing methods for Event instances #251

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
44f9bcc
`.fromJson` and `.toJson` added + move utils func
eliasyishak Mar 13, 2024
ce19f12
Use nullable static method to parse string json
eliasyishak Mar 13, 2024
9a55df9
Update event.dart
eliasyishak Mar 14, 2024
1846d97
Fix test
eliasyishak Mar 14, 2024
65c353d
Merge remote-tracking branch 'upstream/main' into 250-helper-method-t…
eliasyishak Mar 25, 2024
03dcdbe
Tests for encoding/decoding json
eliasyishak Mar 25, 2024
7a7511e
Store intersection in local var
eliasyishak Mar 26, 2024
9d2bd10
Remove exception thrown for nullable return
eliasyishak Mar 26, 2024
886b253
Use conditionals to check for any type errors
eliasyishak Mar 26, 2024
9df20b3
Test case added to check for invalid eventData
eliasyishak Mar 26, 2024
f16b37a
Update CHANGELOG.md
eliasyishak Mar 26, 2024
9602053
Refactor `Event.fromJson` to use pattern matching
eliasyishak Mar 27, 2024
2d58a3d
Use package:collection for comparing eventData
eliasyishak Mar 27, 2024
760d9b7
Use range for collection version
eliasyishak Mar 27, 2024
3977ae8
`fromLabel` static method renaming
eliasyishak Mar 27, 2024
d71b224
Fix test by refactoring unrelated DashTool static method
eliasyishak Mar 27, 2024
bd2fae1
Remove `when` clause and check inside if statement
eliasyishak Mar 28, 2024
7fd9071
`_deepCollectionEquality` to global scope + nit fix
eliasyishak Mar 29, 2024
8cacb0b
Remove collection dep + schema in dartdoc + nit fixes
eliasyishak Mar 29, 2024
1a85335
Add'l context to `Event.fromJson` static method
eliasyishak Mar 29, 2024
b412b59
Store intersection in local variable
eliasyishak Mar 29, 2024
e1d45d5
Refactor `DashTool.fromLabel`
eliasyishak Mar 29, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions pkgs/unified_analytics/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
- Get rid of `late` variables throughout implementation class, `AnalyticsImpl`
- Any error events (`Event.analyticsException`) encountered within package will be sent when invoking `Analytics.close`; replacing `ErrorHandler` functionality
- Exposing new method for `FakeAnalytics.sendPendingErrorEvents` to send error events on command
- Added `Event.fromJson` static method to generate instance of `Event` from JSON

## 5.8.8

Expand Down
12 changes: 8 additions & 4 deletions pkgs/unified_analytics/lib/src/enums.dart
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,11 @@ enum DashEvent {
required this.description,
this.toolOwner,
});

/// This takes in the string label for a given [DashEvent] and returns the
/// enum for that string label.
static DashEvent? fromLabel(String label) =>
DashEvent.values.where((e) => e.label == label).firstOrNull;
}

/// Officially-supported clients of this package as logical
Expand Down Expand Up @@ -199,10 +204,9 @@ enum DashTool {

/// This takes in the string label for a given [DashTool] and returns the
/// enum for that string label.
static DashTool getDashToolByLabel(String label) {
for (final tool in DashTool.values) {
if (tool.label == label) return tool;
}
static DashTool fromLabel(String label) {
final tool = DashTool.values.where((t) => t.label == label).firstOrNull;
if (tool != null) return tool;

throw Exception('The tool $label from the survey metadata file is not '
'a valid DashTool enum value\n'
Expand Down
68 changes: 63 additions & 5 deletions pkgs/unified_analytics/lib/src/event.dart
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
import 'dart:convert';

import 'enums.dart';
import 'utils.dart';

final class Event {
final DashEvent eventName;
Expand Down Expand Up @@ -455,7 +454,6 @@ final class Event {
: eventName = DashEvent.hotReloadTime,
eventData = {'timeMs': timeMs};

// TODO: eliasyishak, add better dartdocs to explain each param
/// Events to be sent for the Flutter Hot Runner.
Event.hotRunnerInfo({
required String label,
Expand Down Expand Up @@ -507,6 +505,7 @@ final class Event {
if (reloadVMTimeInMs != null) 'reloadVMTimeInMs': reloadVMTimeInMs,
};

// TODO: eliasyishak, add better dartdocs to explain each param
/// Event that is emitted periodically to report the number of times each lint
/// has been enabled.
///
Expand Down Expand Up @@ -708,6 +707,10 @@ final class Event {
if (label != null) 'label': label,
};

/// Private constructor to be used when deserializing JSON into an instance
/// of [Event].
Event._({required this.eventName, required this.eventData});

@override
int get hashCode => Object.hash(eventName, jsonEncode(eventData));

Expand All @@ -716,11 +719,66 @@ final class Event {
other is Event &&
other.runtimeType == runtimeType &&
other.eventName == eventName &&
compareEventData(other.eventData, eventData);
_compareEventData(other.eventData, eventData);

@override
String toString() => jsonEncode({
/// Converts an instance of [Event] to JSON.
String toJson() => jsonEncode({
'eventName': eventName.label,
'eventData': eventData,
});

@override
String toString() => toJson();

/// Utility function to take in two maps [a] and [b] and compares them
/// to ensure that they have the same keys and values
bool _compareEventData(Map<String, Object?> a, Map<String, Object?> b) {
Copy link
Contributor

Choose a reason for hiding this comment

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

instead of creating our own, can we use something from package:collection? like DeepCollectionEquality?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good idea, this commit has it updated to use DeepCollectionEquality.equals to compare instead

@christopherfujino, i know we discussed trimming down dependencies for this package, do you think this is a valid reason to add another dep?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Following up on offline discussions with @christopherfujino, going to remove the dep on collection and use the same function for comparing two separate maps to reduce dependencies for this package

final keySetA = a.keys.toSet();
final keySetB = b.keys.toSet();
final intersection = keySetA.intersection(keySetB);

// Ensure that the keys are the same for each object
if (intersection.length != keySetA.length ||
intersection.length != keySetB.length) {
return false;
}

// Ensure that each of the key's values are the same
for (final key in a.keys) {
if (a[key] != b[key]) return false;
}

return true;
}

/// Returns a valid instance of [Event] if [json] follows the correct schema.
///
/// Common use case for this static method involves clients of this package
/// that have a client-server setup where the server sends events that the
/// client creates.
static Event? fromJson(String json) {
try {
final jsonMap = jsonDecode(json) as Map<String, Object?>;

// Ensure that eventName is a string and a valid label and
// eventData is a nested object
if (jsonMap
case {
'eventName': final String eventName,
'eventData': final Map<String, Object?> eventData,
}) {
final dashEvent = DashEvent.fromLabel(eventName);
if (dashEvent == null) return null;

return Event._(
eventName: dashEvent,
eventData: eventData,
);
}

return null;
} on FormatException {
return null;
}
}
}
7 changes: 3 additions & 4 deletions pkgs/unified_analytics/lib/src/survey_handler.dart
Original file line number Diff line number Diff line change
Expand Up @@ -134,10 +134,9 @@ class Survey {
samplingRate = json['samplingRate'] is String
? double.parse(json['samplingRate'] as String)
: json['samplingRate'] as double,
excludeDashToolList =
(json['excludeDashTools'] as List<dynamic>).map((e) {
return DashTool.getDashToolByLabel(e as String);
}).toList(),
excludeDashToolList = (json['excludeDashTools'] as List<dynamic>)
.map((e) => DashTool.fromLabel(e as String))
.toList(),
conditionList = (json['conditions'] as List<dynamic>).map((e) {
return Condition.fromJson(e as Map<String, dynamic>);
}).toList(),
Expand Down
20 changes: 0 additions & 20 deletions pkgs/unified_analytics/lib/src/utils.dart
Original file line number Diff line number Diff line change
Expand Up @@ -36,26 +36,6 @@ bool checkDirectoryForWritePermissions(Directory directory) {
return fileStat.modeString()[1] == 'w';
}

/// Utility function to take in two maps [a] and [b] and compares them
/// to ensure that they have the same keys and values
bool compareEventData(Map<String, Object?> a, Map<String, Object?> b) {
final keySetA = a.keys.toSet();
final keySetB = b.keys.toSet();

// Ensure that the keys are the same for each object
if (keySetA.intersection(keySetB).length != keySetA.length ||
keySetA.intersection(keySetB).length != keySetB.length) {
return false;
}

// Ensure that each of the key's values are the same
for (final key in a.keys) {
if (a[key] != b[key]) return false;
}

return true;
}

eliasyishak marked this conversation as resolved.
Show resolved Hide resolved
/// Format time as 'yyyy-MM-dd HH:mm:ss Z' where Z is the difference between the
/// timezone of t and UTC formatted according to RFC 822.
String formatDateTime(DateTime t) {
Expand Down
55 changes: 54 additions & 1 deletion pkgs/unified_analytics/test/event_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -556,7 +556,9 @@ void main() {
test('Confirm all constructors were checked', () {
var constructorCount = 0;
for (var declaration in reflectClass(Event).declarations.keys) {
if (declaration.toString().contains('Event.')) constructorCount++;
// Count public constructors but omit private constructors
if (declaration.toString().contains('Event.') &&
!declaration.toString().contains('Event._')) constructorCount++;
}

// Change this integer below if your PR either adds or removes
Expand All @@ -568,4 +570,55 @@ void main() {
'`pkgs/unified_analytics/test/event_test.dart` '
'to reflect the changes made');
});

test('Serializing event to json successful', () {
final event = Event.analyticsException(
workflow: 'workflow',
error: 'error',
description: 'description',
);

final expectedResult = '{"eventName":"analytics_exception",'
'"eventData":{"workflow":"workflow",'
'"error":"error",'
'"description":"description"}}';

expect(event.toJson(), expectedResult);
});

test('Deserializing string to event successful', () {
final eventJson = '{"eventName":"analytics_exception",'
'"eventData":{"workflow":"workflow",'
'"error":"error",'
'"description":"description"}}';

final eventConstructed = Event.fromJson(eventJson);
expect(eventConstructed, isNotNull);
eventConstructed!;

expect(eventConstructed.eventName, DashEvent.analyticsException);
expect(eventConstructed.eventData, {
'workflow': 'workflow',
'error': 'error',
'description': 'description',
});
});

test('Deserializing string to event unsuccessful for invalid eventName', () {
final eventJson = '{"eventName":"NOT_VALID_NAME",'
'"eventData":{"workflow":"workflow",'
'"error":"error",'
'"description":"description"}}';

final eventConstructed = Event.fromJson(eventJson);
expect(eventConstructed, isNull);
});

test('Deserializing string to event unsuccessful for invalid eventData', () {
final eventJson = '{"eventName":"analytics_exception",'
'"eventData": "not_valid_event_data"}';

final eventConstructed = Event.fromJson(eventJson);
expect(eventConstructed, isNull);
});
}