-
Notifications
You must be signed in to change notification settings - Fork 14
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
feat: Prevent excessive fetch calls #54
Changes from all commits
fe528b8
d3a63ae
593923c
9b4aac8
f269fd2
a6dbf74
fa8887b
9bc0717
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -257,7 +257,27 @@ class UnleashClient extends EventEmitter { | |
} | ||
} | ||
|
||
/// Checks if any of the provided context fields are different from the current ones. | ||
bool _anyFieldHasChanged(Map<String, String> fields) { | ||
for (var entry in fields.entries) { | ||
String key = entry.key; | ||
String newValue = entry.value; | ||
|
||
if (key == 'userId') { | ||
if (context.userId != newValue) return true; | ||
} else if (key == 'sessionId') { | ||
if (context.sessionId != newValue) return true; | ||
} else if (key == 'remoteAddress') { | ||
if (context.remoteAddress != newValue) return true; | ||
} else { | ||
if (context.properties[key] != newValue) return true; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm guessing this one also handles cases where the key didn't exist before? That is, if I don't see a unit test for that, but it might be worth adding one (or modifying the current one)? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Very good point. I will add a test and I think we're missing field removal in update context |
||
} | ||
} | ||
return false; | ||
} | ||
|
||
Future<void> updateContext(UnleashContext unleashContext) async { | ||
if (unleashContext == context) return; | ||
if (started == false) { | ||
await _waitForEvent('initialized'); | ||
_updateContextFields(unleashContext); | ||
|
@@ -286,6 +306,7 @@ class UnleashClient extends EventEmitter { | |
} | ||
|
||
Future<void> setContextFields(Map<String, String> fields) async { | ||
if (!_anyFieldHasChanged(fields)) return; | ||
if (clientState == ClientState.ready) { | ||
fields.forEach((field, value) { | ||
_updateContextField(field, value); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
added missing start method in README