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 3 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
6 changes: 2 additions & 4 deletions pkgs/unified_analytics/lib/src/enums.dart
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,6 @@
// for details. All rights reserved. Use of this source code is governed by a
// BSD-style license that can be found in the LICENSE file.

import 'package:collection/collection.dart';

/// The valid dash tool labels stored in the [DashTool] enum.
List<String> get validDashTools =>
DashTool.values.map((e) => e.label).toList()..sort();
Expand Down Expand Up @@ -159,7 +157,7 @@ enum DashEvent {
/// 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.firstWhereOrNull((e) => e.label == label);
DashEvent.values.where((e) => e.label == label).firstOrNull;
}

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

/// This takes in the string label for a given [DashTool] and returns the
/// enum for that string label.
static DashTool? fromLabel(String label) {
static DashTool fromLabel(String label) {
for (final tool in DashTool.values) {
if (tool.label == label) return tool;
}
Copy link
Contributor

@kenzieschmoll kenzieschmoll Mar 29, 2024

Choose a reason for hiding this comment

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

nit: use the simplified syntax from above for consistency
DashTool.values.where((t) => t.label == label).firstOrNull;

Expand Down
58 changes: 26 additions & 32 deletions pkgs/unified_analytics/lib/src/event.dart
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,8 @@

import 'dart:convert';

import 'package:collection/collection.dart';

import 'enums.dart';

final DeepCollectionEquality _deepCollectionEquality =
const DeepCollectionEquality();

final class Event {
final DashEvent eventName;
final Map<String, Object?> eventData;
Expand Down Expand Up @@ -459,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 @@ -511,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 @@ -724,22 +719,9 @@ final class Event {
other is Event &&
other.runtimeType == runtimeType &&
other.eventName == eventName &&
_deepCollectionEquality.equals(other.eventData, eventData);
_compareEventData(other.eventData, eventData);

/// Converts an instance of [Event] to JSON.
///
/// Example for [Event.timing] converted to JSON below.
/// ```json
/// {
/// "eventName": "timing",
/// "eventData": {
/// "workflow": "my-work-flow",
/// "variableName": "my-variable",
/// "elapsedMilliseconds": 123,
/// "label": "my-label"
/// }
/// }
/// ```
String toJson() => jsonEncode({
'eventName': eventName.label,
'eventData': eventData,
Expand All @@ -748,20 +730,32 @@ final class Event {
@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.
///
/// Schema is below with the following notes:
/// - The `eventName` key must have a string value that exists in [DashEvent].
/// - The `eventData` key must have a nested object as its value.
/// ```json
/// {
/// "eventName": "string-label",
/// "eventData": {
/// "myVar1": "value1",
/// "myVar2": 123
/// }
/// }
/// ```
/// 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?>;
Expand Down
1 change: 0 additions & 1 deletion pkgs/unified_analytics/lib/src/survey_handler.dart
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,6 @@ class Survey {
: json['samplingRate'] as double,
excludeDashToolList = (json['excludeDashTools'] as List<dynamic>)
.map((e) => DashTool.fromLabel(e as String))
.whereType<DashTool>()
.toList(),
conditionList = (json['conditions'] as List<dynamic>).map((e) {
return Condition.fromJson(e as Map<String, dynamic>);
Expand Down
1 change: 0 additions & 1 deletion pkgs/unified_analytics/pubspec.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ environment:

dependencies:
clock: ^1.1.1
collection: '>=1.18.0 <2.0.0'
file: '>=6.1.4 <8.0.0'
http: '>=0.13.5 <2.0.0'
intl: '>=0.18.0 <0.20.0'
Expand Down