Skip to content

Entitlement verification: Return updated request time from header in CustomerInfo in 304 responses#848

Merged
tonidero merged 11 commits into
entitlements-verificationfrom
toniricodiez/sdk-2897-requestdate-not-returning-updated-value
Mar 8, 2023
Merged

Entitlement verification: Return updated request time from header in CustomerInfo in 304 responses#848
tonidero merged 11 commits into
entitlements-verificationfrom
toniricodiez/sdk-2897-requestdate-not-returning-updated-value

Conversation

@tonidero

@tonidero tonidero commented Mar 7, 2023

Copy link
Copy Markdown
Contributor

Description

Completes SDK-2897

With this PR, we will update CustomerInfo's requestDate with the latest value even when a 304 response is received.

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.

This was a bit awkward... We could make it so if the response is missing the request time header, we fail but I thought I would copy what iOS is doing and just fallback to the request date in the body. Happy to discuss this though.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think this makes sense.

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.

I gave it this name since there is already a request_date which is part of the body of the response. When caching it, I didn't want to override that value, so I serialize it as this key.

@tonidero tonidero marked this pull request as ready for review March 7, 2023 11:46
@tonidero tonidero requested a review from a team March 7, 2023 11:46
@codecov

codecov Bot commented Mar 7, 2023

Copy link
Copy Markdown

Codecov Report

❗ No coverage uploaded for pull request base (toniricodiez/sdk-2896-invalidate-devicecache-cache-if@16f47b9). Click here to learn what that means.
The diff coverage is n/a.

❗ Current head 44c9010 differs from pull request most recent head 60859d1. Consider uploading reports for the commit 60859d1 to get more accurate results

@@                                   Coverage Diff                                    @@
##             toniricodiez/sdk-2896-invalidate-devicecache-cache-if     #848   +/-   ##
========================================================================================
  Coverage                                                         ?   82.70%           
========================================================================================
  Files                                                            ?      137           
  Lines                                                            ?     4532           
  Branches                                                         ?      596           
========================================================================================
  Hits                                                             ?     3748           
  Misses                                                           ?      562           
  Partials                                                         ?      222           

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@tonidero tonidero changed the title Entitlement verification: Return updated request time from header in CustomerInfo Entitlement verification: Return updated request time from header in CustomerInfo on 304 responses Mar 7, 2023
@tonidero tonidero changed the title Entitlement verification: Return updated request time from header in CustomerInfo on 304 responses Entitlement verification: Return updated request time from header in CustomerInfo in 304 responses Mar 7, 2023
Comment thread common/src/test/java/com/revenuecat/purchases/common/DeviceCacheTest.kt Outdated
Comment thread common/src/test/java/com/revenuecat/purchases/common/DeviceCacheTest.kt Outdated

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It would be nice to finish renaming all these to "customer info". Purchaser info was the old name

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.

Agreed... Will only rename the ones I'm touching in this PR, but we definitely should finish the rename

@tonidero tonidero force-pushed the toniricodiez/sdk-2896-invalidate-devicecache-cache-if branch from 4b42115 to fdb918a Compare March 8, 2023 11:55
@tonidero tonidero force-pushed the toniricodiez/sdk-2897-requestdate-not-returning-updated-value branch from 44c9010 to 60859d1 Compare March 8, 2023 12:01

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think this makes sense.

.setResponseCode(expectedResult.responseCode)
.apply {
if (requestDateHeader != null) {
setHeader(HTTPResult.REQUEST_TIME_HEADER_NAME, requestDateHeader.time)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is .time ms?

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.

Yup, that's the official way to get milliseconds from a Date... I guess I could add a Date.epochMilliseconds extension or something like that to make it clearer, but it seems weird considering that's what this official API does.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

No it's fine, just making sure!

Base automatically changed from toniricodiez/sdk-2896-invalidate-devicecache-cache-if to entitlements-verification March 8, 2023 15:25
@tonidero tonidero merged commit f518e50 into entitlements-verification Mar 8, 2023
@tonidero tonidero deleted the toniricodiez/sdk-2897-requestdate-not-returning-updated-value branch March 8, 2023 15:25
tonidero added a commit that referenced this pull request Mar 15, 2023
…CustomerInfo in 304 responses (#848)

### Description
Completes
[SDK-2897](https://linear.app/revenuecat/issue/SDK-2897/requestdate-not-returning-updated-value)

With this PR, we will update `CustomerInfo`'s `requestDate` with the
latest value even when a 304 response is received.
@tonidero tonidero mentioned this pull request Mar 31, 2023
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