-
Notifications
You must be signed in to change notification settings - Fork 104
Add Workflows network layer #3300
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,41 @@ | ||
| @file:OptIn(InternalRevenueCatAPI::class) | ||
|
|
||
| package com.revenuecat.purchases.common.workflows | ||
|
|
||
| import com.revenuecat.purchases.InternalRevenueCatAPI | ||
| import com.revenuecat.purchases.storage.FileRepository | ||
| import kotlinx.coroutines.runBlocking | ||
| import java.io.File | ||
| import java.io.IOException | ||
| import java.net.URL | ||
|
|
||
| /** | ||
| * Fetches compiled workflow JSON from a CDN URL (inject for tests; use [FileCachedWorkflowCdnFetcher] in production). | ||
| */ | ||
| internal fun interface WorkflowCdnFetcher { | ||
| @Throws(IOException::class) | ||
| fun fetchCompiledWorkflowJson(cdnUrl: String): String | ||
| } | ||
|
|
||
| /** | ||
| * Uses [FileRepository] for disk caching when non-null; otherwise reads the URL directly. | ||
| */ | ||
| internal class FileCachedWorkflowCdnFetcher( | ||
| private val fileRepository: FileRepository?, | ||
| ) : WorkflowCdnFetcher { | ||
|
|
||
| @Throws(IOException::class) | ||
| override fun fetchCompiledWorkflowJson(cdnUrl: String): String { | ||
| val url = URL(cdnUrl) | ||
| return if (fileRepository != null) { | ||
| runBlocking { | ||
| val uri = fileRepository.generateOrGetCachedFileURL(url) | ||
| File(uri).readText() | ||
| } | ||
| } else { | ||
| url.openConnection().getInputStream().use { stream -> | ||
| stream.readBytes().decodeToString() | ||
| } | ||
| } | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,54 @@ | ||
| @file:OptIn(InternalRevenueCatAPI::class) | ||
|
|
||
| package com.revenuecat.purchases.common.workflows | ||
|
|
||
| import com.revenuecat.purchases.InternalRevenueCatAPI | ||
| import com.revenuecat.purchases.common.isSuccessful | ||
| import com.revenuecat.purchases.common.networking.HTTPResult | ||
| import org.json.JSONException | ||
| import org.json.JSONObject | ||
| import java.io.IOException | ||
|
|
||
| /** | ||
| * Normalizes a successful workflow-detail HTTP payload: `inline` (unwrap `data`) or `use_cdn` (fetch JSON). | ||
| * Non-success [HTTPResult] is returned unchanged with null [WorkflowDetailHttpProcessingResult.enrolledVariants]. | ||
| */ | ||
| internal class WorkflowDetailHttpProcessor( | ||
| private val workflowCdnFetcher: WorkflowCdnFetcher, | ||
| ) { | ||
|
|
||
| @Throws(JSONException::class, IOException::class) | ||
| fun process(result: HTTPResult): WorkflowDetailHttpProcessingResult { | ||
| if (!result.isSuccessful()) { | ||
| return WorkflowDetailHttpProcessingResult(httpResult = result, enrolledVariants = null) | ||
| } | ||
| val root = JSONObject(result.payload) | ||
| val enrolledVariants = root.optJSONObject("enrolled_variants")?.toEnrolledVariantsMap() | ||
| val rawAction = root.getString("action") | ||
| val action = WorkflowResponseAction.fromValue(rawAction) | ||
| ?: throw JSONException("Unknown workflow response action: $rawAction") | ||
| val normalizedPayload = when (action) { | ||
| WorkflowResponseAction.INLINE -> root.getJSONObject("data").toString() | ||
| WorkflowResponseAction.USE_CDN -> workflowCdnFetcher.fetchCompiledWorkflowJson(root.getString("url")) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I wonder if we should just stick to plain Json serialization here, and avoid the custom parsing we are doing here. Then, do the fetching of the compiled workflow as a higher level later on top of the network call? Feels the network call and the logic are a bit mixed right now... |
||
| } | ||
| return WorkflowDetailHttpProcessingResult( | ||
| httpResult = result.copy(payload = normalizedPayload), | ||
| enrolledVariants = enrolledVariants, | ||
| ) | ||
| } | ||
|
|
||
| private fun JSONObject.toEnrolledVariantsMap(): Map<String, String>? { | ||
| val map = mutableMapOf<String, String>() | ||
| val keyIterator = keys() | ||
| while (keyIterator.hasNext()) { | ||
| val key = keyIterator.next() | ||
| map[key] = optString(key) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
| } | ||
| return map.takeUnless { it.isEmpty() } | ||
| } | ||
| } | ||
|
|
||
| internal data class WorkflowDetailHttpProcessingResult( | ||
| val httpResult: HTTPResult, | ||
| val enrolledVariants: Map<String, String>?, | ||
| ) | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,21 @@ | ||
| @file:OptIn(InternalRevenueCatAPI::class) | ||
|
|
||
| package com.revenuecat.purchases.common.workflows | ||
|
|
||
| import com.revenuecat.purchases.InternalRevenueCatAPI | ||
| import kotlinx.serialization.json.Json | ||
|
|
||
| internal object WorkflowJsonParser { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This feels like an unnecessary abstraction to me. Or what am I missing? Can't we reuse an existing parser? |
||
| private val json = Json { | ||
| ignoreUnknownKeys = true | ||
| isLenient = true | ||
| } | ||
|
|
||
| fun parseWorkflowsListResponse(payload: String): WorkflowsListResponse { | ||
| return json.decodeFromString(payload) | ||
| } | ||
|
|
||
| fun parsePublishedWorkflow(payload: String): PublishedWorkflow { | ||
| return json.decodeFromString(payload) | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm why default here to a different one since we're creating the
Backendinstance in thePurchasesFactorywith a value? If this is for tests, I wonder if we should just make this explicit there... Also, it would makefileRespositorynot nullable, which feels better?