Remote Feature Flags: Add cache expiry logic and make authenticated request if possible

[hassaanelgarem]

Closes #19279 Closes #19291

Description

• Added cache expiry logic to RemoteFeatureFlagStore, and added a parameter to RemoteFeatureFlagStore.updateIfNeeded to be able to bypass this logic.
• Refactored RemoteFeatureFlagStore.updateIfNeeded to accept an instance of FeatureFlagRemote so we can pass an authenticated remote if possible.
• Now we attempt to update the feature flags on every launch and relaunch.
• We also attempt to update the feature flags whenever the user logs in or out.

Testing Instructions

Pre-testing notes

1. The current endpoint does not support integer build numbers so by default the request always fails. Before testing, please manually update the app's build number to 1 for example.
2. In order to easily test the cache expiry logic, you can manually update the value of RemoteFeatureFlagStore.Constants.CacheTTL to a small value. (60 seconds for example)
3. Some testing instructions will require a network debugging app like Charles or Proxyman.

Cache Expiry

1. Delete the app
2. Install and run the app
3. Once the app is launched check that a request to the feature-flags endpoint is made.
4. Before the TTL passes, send the app to the background then re-open it.
5. Check that a request to the feature-flags endpoint is NOT made.
6. Wait for the TTL to pass
7. Send the app to the background then re-open it.
8. Check that a request to the feature-flags endpoint is made.
9. Before the TTL passes, terminate the app and then launch it.
10. Check that a request to the feature-flags endpoint is NOT made.

Authenticated requests

1. Make sure you're logged in
2. Trigger a refresh by relaunching the app after the TTL has passed
3. Check the request and make sure that the headers contain the Bearer token
4. Log out
5. Trigger a refresh by relaunching the app after the TTL has passed
6. Check the request and make sure that the headers do not contain the Bearer token

Different Triggers

1. Make sure to set a long TTL
2. Delete the app
3. Install and run the app
4. Once the app is launched check that a request to the feature-flags endpoint is made.
5. Log in to the app
6. Make sure that a request to the feature-flags endpoint is made.
7. Logout of the app
8. Make sure that a request to the feature-flags endpoint is made.
9. Signup for a new account
10. Make sure that a request to the feature-flags endpoint is made.

Regression Notes

1. Potential unintended areas of impact N/A

2. What I did to test those areas of impact (or what existing automated tests I relied on) N/A

3. What automated tests I added (or what prevented me from doing so) Will add unit tests in another PR.

PR submission checklist:

• [x] I have completed the Regression Notes.
• [x] I have considered adding unit tests for my changes.
• [x] I have considered adding accessibility improvements for my changes.
• [x] I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.

[wpmobilebot]

You can test the changes in Jetpack from this Pull Request by:

• Clicking here or scanning the QR code below to access App Center
• Then installing the build number pr19293-4043c0a on your iPhone

If you need access to App Center, please ask a maintainer to add you.

[wpmobilebot]

You can test the changes in WordPress from this Pull Request by:

• Clicking here or scanning the QR code below to access App Center
• Then installing the build number pr19293-4043c0a on your iPhone

If you need access to App Center, please ask a maintainer to add you.

[hassaanelgarem]

Forgot to mention earlier that we should add release notes.

@momo-ozawa I feel that none of the changes here are user-facing. They're also too low-level and would be challenging to test during beta testing. Wdyt?

[momo-ozawa]

Hey @hassaanelgarem 👋

I feel that none of the changes here are user-facing

I was thinking of using the [internal] tag.

They're also too low-level and would be challenging to test during beta testing. Wdyt?

Hmm, might be worth checking in with the quality team about this.

[hassaanelgarem]

might be worth checking in with the quality team about this.

Agreed!

@pachlava This PR amends existing logic surrounding the remote feature flags. Now we make a request to the /feature-flags endpoint on each launch and relaunch, given that cache hasn't expired yet. We also refresh the flags on each login and logout. We currently don't use any remote feature flags, so there's no noticeable changes to the app. Given these changes, do you think it warrants an internal release note?

[pachlava]

@pachlava This PR amends existing logic surrounding the remote feature flags. Now we make a request to the /feature-flags endpoint on each launch and relaunch, given that cache hasn't expired yet. We also refresh the flags on each login and logout. We currently don't use any remote feature flags, so there's no noticeable changes to the app. Given these changes, do you think it warrants an internal release note?

@hassaanelgarem 👋

I'm sorry for such a long delay before my reply. With this exact PR, and the described testing scenarios - I would not expect this to be a part of release notes, usually. However, if you want the PR to get an additional attention during internal beta testing, I think it can be prefixed with [Internal] then. In such a case, it won't be mentioned it in the public CfT.

Usually, the CfT runner and creator (cc @tiagomar, who runs the WPiOS ones) takes a look into the whole milestone, and decides if PRs not listed in the release notes should still be tested during beta testing. But as I wrote above, this can be facilitated by using the [Internal] prefix.

[hassaanelgarem]

We will no longer implement a cache expiry logic. More details here: https://github.com/wordpress-mobile/WordPress-iOS/issues/19279#issuecomment-1271087569

[hassaanelgarem]

@momo-ozawa @jkmassel I removed the TTL logic completely. The PR is now ready for a re-review 🙏

[mokagio]

Looks like all of Jeremy's comments have been addressed, plus @momo-ozawa has approved this. I'm going to merge this so it can be part of the 21.0 beta.

It should be straightforward to disable this in the release branch if some issues will be identified by removing the few calls to the method that tiggers the fetch.