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

Make use of ryw_delay to minimize retries on IAM fetch #2207

Merged
merged 7 commits into from
Oct 31, 2024
Merged

Conversation

rgomezp
Copy link
Contributor

@rgomezp rgomezp commented Oct 24, 2024

Description

One Line Summary

Encapsulate rywToken and rywDelay into a RywData class and update usage throughout the codebase including delaying IAM fetch by the specified delay.

Details

Motivation

This update introduces the RywData class to group rywToken and rywDelay into a single data object. This ensures both values are handled together, allowing for accurate delay management based on the most recent rywToken.

Along with ryw_token, ryw_delay is returned by user & subscription create & update endpoints. It essentially says "this is about when we expect the change will be ready". The goal is to minimize retries.

Scope

  • Encapsulated rywToken and rywDelay into the new RywData class to centralize their usage.
  • Updated method signatures: All previous usages of String (rywToken) are replaced with the new RywData object.
    • Renamed setRywToken method to setRywData to reflect this change.
  • IAM fetch logic adjusted: The delay for IAM fetch now leverages the rywDelay value from RywData. A fallback mechanism ensures minimal retries to the backend.
  • Renamed methods for clarity, ensuring method names convey when a value is expected to be returned.

Testing

Unit testing

Tests were updated & passed.

Manual testing

Tested accurate segment membership calculation & display of IAMs. e.g:

  1. Create two IAMs targeting segments with version 1, & another targeting version 2 of the app
  2. Set the IAMs to display every time conditions are met
  3. Switch back & forth between versions in the example app
  4. See that you get the correct IAM

Affected code checklist

  • Notifications
    • Display
    • Open
    • Push Processing
    • Confirm Deliveries
  • Outcomes
  • Sessions
  • In-App Messaging
  • REST API requests
  • Public API changes

Checklist

Overview

  • I have filled out all REQUIRED sections above
  • PR does one thing
    • If it is hard to explain how any codes changes are related to each other then it most likely needs to be more than one PR
  • Any Public API changes are explained in the PR details and conform to existing APIs

Testing

  • I have included test coverage for these changes, or explained why they are not needed
  • All automated tests pass, or I explained why that is not possible
  • I have personally tested this on my device, or explained why that is not possible

Final pass

  • Code is as readable as possible.
    • Simplify with less code, followed by splitting up code into well named functions and variables, followed by adding comments to the code.
  • I have reviewed this PR myself, ensuring it meets each checklist item
    • WIP (Work In Progress) is ok, but explain what is still in progress and what you would like feedback on. Start the PR title with "WIP" to indicate this.

This change is Reviewable


@rgomezp rgomezp force-pushed the ryw-follow-up branch 3 times, most recently from c71182e to bb4a0b7 Compare October 25, 2024 17:13
Motivation: we need to keep the two values together in order to know how much to delay by (based on what rywToken is newest)
Motivation: it's unclear from the previous method name that we are expecting something to be returned.
Motivation: make use of the new object to pull out the `rywDelay` value & delay by that amount.

By doing so we help minimize the number of retries that will hit the backend.
Motivation: update all usages of `rywToken` to now make use of the new `RywData` object.

In `ConsistencyManager` we also renamed the method `setRywToken` to `setRywData`
Motivation: we switched to return a RywData object so method name didn't make sense any more
Motivation: add clarity on why the method maxOrNull (which uses lexicographic evaulation for strings) works for this use case.
Motivation: rywData shouldn't be considered a valid rywData object if rywToken is undefined.

We update usage across the board to account for this type change.
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