Use the built-in Database Cache for Connect flow data instead of transients

[diegocurbelo]

Fixes STRIPE-745
Fixes #<github_issue_id>

Changes proposed in this Pull Request:

This PR replaces the transient used during the OAuth Connect with an entry in the built-in Database cache.

Testing instructions

1. Disconnect your Stripe account
2. Go to the Onboard page (WooCommerce > Payments > Stripe)
3. Click Create or connect a test account instead
4. Verify that the connection succeeds.

---

• Covered with tests (or have a good reason not to test in description ☝️)
• Tested on mobile (or does not apply)

Changelog entry

• This Pull Request does not require a changelog entry. (Comment required below)

Changelog Entry Comment

Comment

Post merge

• Added testing instructions to the Release Testing Instructions wiki page (or does not apply)
• Added the needs docs label (or does not apply)
• Included this PR in the Release Thread scope (or does not apply)

[wjrosa]

Looks good to me! Left a typo fix suggestion

[daledupreez]

Gah! I forgot to submit my comments yesterday! Apologies for the delay, @diegocurbelo!

[daledupreez]

I have a concern here around what would happen if the current store mode doesn't match the value of $mode, as the cache code appends the current store mode as part of the cache key. But there is no guarantee that $mode is the same (or will be the same) as the current store mode (if one is even set yet). I think we need to explicitly set the mode before we write to the cache, rather than including $mode in the key.

I ended up testing and it's possible to get into weird states if you do the following:

• Connect a site in test mode - make sure you've completed the connection flow
• Trigger a (re)connection flow, and keep this window/tab open -- it can be for live or test mode
• Note that we create a database cache entries for wcstripe_cache_test_wcs_stripe_connect_state_live and wcstripe_cache_test_wcs_stripe_connect_state_test
• In a new window, disconnect your site from Stripe, which shifts us back into "live" mode because the testmode sub-option is not specified
• Complete the Stripe connection flow in the first window - notice that you end up on the connection screen, as we can't read the database cache due to the mode switching from test to live as a result of the disconnection

While this is a somewhat artificial test, it does feel like we should guard against this in some way. I considered having this code temporarily switch the mode via the testmode sub-option, but that could have other indirect effects for other incoming requests or logic. It feels like it may be better to expose something like WC_Stripe_Database_Cache::set_for_mode(), WC_Stripe_Database_Cache::get_for_mode(), and WC_Stripe_Database_Cache::delete_for_mode() APIs that then allow the key prefix to be calculated based on the mode. Maybe something like the following:

	/**
	 * Adds the CACHE_KEY_PREFIX + plugin mode prefix to the key.
	 * Ex: "wcstripe_cache_[mode]_[key].
	 *
	 * @param string       $key  The key to add the prefix to.
	 * @param bool|null $test_mode If we should force test mode (true) or live mode (false). Null implies we should use the current mode. 
	 *
	 * @return string The key with the prefix.
	 */
	private static function add_key_prefix( $key, ?bool $test_mode = null ): string {
		if ( null === $test_mode ) {
			$is_test_mode = WC_Stripe_Mode::is_test();
		} else {
			$is_test=mode = $test_mode;
		}
		$mode = $is_test_mode ? 'test_' : 'live_';
		return self::CACHE_KEY_PREFIX . $mode . $key;
	}

	/**
	 * Gets a value from the cache.
	 *
	 * The key is automatically prefixed with "wcstripe_cache_[mode]_".
	 *
	 * @param string $key The key to look for.
	 *
	 * @return mixed|null The cache contents. NULL if the cache value is expired or missing.
	 */
	public static function get( $key ) {
		return self::get_with_mode( $key, WC_Stripe_Mode::is_test() );
	}

	/**
	 * Gets a value from the cache for a specified mode.
	 *
	 * The key is automatically prefixed with "wcstripe_cache_[mode]_".
	 *
	 * @param string $key     The key to look for.
	 * @param bool  $is_test_mode Whether we are in test mode or not.
	 *
	 * @return mixed|null The cache contents. NULL if the cache value is expired or missing.
	 */
	public static function get_with_mode( $key, bool $is_test_mode ) {
		$prefixed_key = self::add_key_prefix( $key, $is_test_mode );
		$cache_contents = self::get_from_cache( $prefixed_key );
		if ( is_array( $cache_contents ) && array_key_exists( 'data', $cache_contents ) ) {
			if ( self::is_expired( $prefixed_key, $cache_contents ) ) {
				return null;
			}

			self::maybe_trigger_prefetch( $key, $cache_contents );

			return $cache_contents['data'];
		}

		return null;
	}

[daledupreez]

As above, I am concerned that the current mode (in the testmode setting) may not match the mode at the time the write occurred.