Skip to content

Store and return ETag last refresh time header#978

Merged
tonidero merged 4 commits into
mainfrom
toniricodiez/sdk-3025-store-and-update-etag-validation-date
Apr 26, 2023
Merged

Store and return ETag last refresh time header#978
tonidero merged 4 commits into
mainfrom
toniricodiez/sdk-3025-store-and-update-etag-validation-date

Conversation

@tonidero

@tonidero tonidero commented Apr 20, 2023

Copy link
Copy Markdown
Contributor

Description

SDK-3025

Now, when we store a backend result in the cache, we will also store the time it was stored. When we perform the same request with a stored time, we will add a X-RC-Last-Refresh-Time header to the request.

Tested scenarios:

  • Nothing saved for that request: We only send an empty X-RevenueCat-ETag header
  • Saved ETag for a request but no last refresh time saved: We send the X-RevenueCat-ETag header only
  • Saved ETag and last refresh time for a request: We send both the X-RevenueCat-ETag and X-RC-Last-Refresh-Time headers.

@tonidero tonidero added the perf label Apr 20, 2023
@tonidero tonidero changed the title Store and return Etag last refresh time header Store and return ETag last refresh time header Apr 20, 2023
@tonidero tonidero marked this pull request as ready for review April 20, 2023 08:52
@tonidero tonidero requested a review from a team April 20, 2023 08:53
val eTagData = if (refreshETag) null else getETagData(path)
return mapOf(
HTTPRequest.ETAG_HEADER_NAME to eTagData?.eTag.orEmpty(),
HTTPRequest.ETAG_LAST_REFRESH_NAME to eTagData?.lastRefreshTime?.time?.toString(),

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

If lastRefreshTime is null, this will send a null value. Do we want that? Or is it better to not send the header at all

@tonidero tonidero Apr 26, 2023

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

In the caller (In HTTPClient), we filter headers with null values so we wouldn't send this header if it's null. I thought about sending an empty value like we do for the ETAG but I do think it's preferred to not send the header at all.

mockCachedHTTPResult(expectedETag = null, path = path)

val eTagHeaders = underTest.getETagHeaders(path)
val lastRefreshTimeHeader = eTagHeaders[HTTPRequest.ETAG_LAST_REFRESH_NAME]

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This will return null both if the value is null or if the key is not present. I asked in another comment, but do we want to even have the header if the value is null?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

As mentioned in the other comment, we will filter the header out in the HTTPClient so it won't actually be sent.

@tonidero

Copy link
Copy Markdown
Contributor Author

I added some tests to verify the behavior of filtering null headers in HTTPClient. Will merge this now but lmk if you still have any comments!

@tonidero tonidero enabled auto-merge (squash) April 26, 2023 09:27
@tonidero tonidero merged commit fb53fb6 into main Apr 26, 2023
@tonidero tonidero deleted the toniricodiez/sdk-3025-store-and-update-etag-validation-date branch April 26, 2023 09:42
tonidero added a commit that referenced this pull request May 18, 2023
**This is an automatic release.**

### New Features
* CAT-859 Expose whether or not a SubscriptionOption is Prepaid in the
SDK (#1005) via Deema AlShamaa (@dalshamaa)
### Bugfixes
* [CF-1324] Fix personalizedPrice defaulting to false (#952) via beylmk
(@beylmk)
### Performance Improvements
* Store and return ETag last refresh time header (#978) via Toni Rico
(@tonidero)
### Dependency Updates
* Bump fastlane-plugin-revenuecat_internal from `3b03efa` to `fe45299`
(#991) via dependabot[bot] (@dependabot[bot])
* Bump danger from 9.2.0 to 9.3.0 (#981) via dependabot[bot]
(@dependabot[bot])
* Bump fastlane-plugin-revenuecat_internal from `8482a43` to `3b03efa`
(#974) via dependabot[bot] (@dependabot[bot])
* Bump fastlane from 2.212.1 to 2.212.2 (#973) via dependabot[bot]
(@dependabot[bot])
* Bump fastlane-plugin-revenuecat_internal from `9255366` to `8482a43`
(#961) via dependabot[bot] (@dependabot[bot])
### Other Changes
* Add proration modes to post to backend (#977) via swehner (@swehner)
* Added ENTITLEMENTS_COMPUTED_ON_DEVICE (#939) via Cesar de la Vega
(@vegaro)
* Fix flaky test in OfflineCustomerInfoCalculatorTest (#997) via Cesar
de la Vega (@vegaro)
* Fix `OfflineCustomerInfoCalculatorTest` `Unresolved reference:
ProducType` (#995) via Cesar de la Vega (@vegaro)
* Add support for product_plan_identifier for offline customer info
(#959) via Cesar de la Vega (@vegaro)
* Add non-subscriptions support to offline customer info (#958) via
Cesar de la Vega (@vegaro)
* Query only active purchases when generating offline entitlements
customer info (#1003) via Toni Rico (@tonidero)
* Fix `PurchasesIntegrationTest` building issue (#996 into main) (#998)
via Cesar de la Vega (@vegaro)
* Fail offline entitlements computation if product entitlement mapping
not available (#999) via Toni Rico (@tonidero)
* Fix  build_magic_weather lane (#993) via Cesar de la Vega (@vegaro)
* Add backend integration tests and test product entitlement mapping
endpoint (#988) via Toni Rico (@tonidero)
* Fix purchases integration tests (#980) via Toni Rico (@tonidero)
* Disable offline entitlements if active inapp purchases exist (#983)
via Toni Rico (@tonidero)
* Clear cached customer info upon entering offline entitlements mode
(#989) via Toni Rico (@tonidero)
* Update product entitlement mapping request to new format (#976) via
Toni Rico (@tonidero)
* Support enabling/disabling offline entitlements (#964) via Toni Rico
(@tonidero)
* Add back integration tests automation (#972) via Toni Rico (@tonidero)
* Upgrade to AGP 8.0 (#975) via Toni Rico (@tonidero)
* Extract post receipt logic to PostReceiptHelper (#967) via Toni Rico
(@tonidero)
* Add isServerDown to error callback for postReceipt and getCustomerInfo
requests (#963) via Toni Rico (@tonidero)
* Add back integration test flavors (#962) via Toni Rico (@tonidero)
* Fix storing test results (#966) via Cesar de la Vega (@vegaro)
* Extract detekt job from test job (#965) via Cesar de la Vega (@vegaro)


[CF-1324]:
https://revenuecats.atlassian.net/browse/CF-1324?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ

---------

Co-authored-by: revenuecat-ops <ops@revenuecat.com>
Co-authored-by: Toni Rico <antonio.rico.diez@revenuecat.com>
@vegaro vegaro added pr:other and removed pr:perf labels Sep 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants