Skip to content
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.

Sign up for GitHub

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

fix: call fetch on start by default #43

Merged
merged 3 commits into from
Nov 9, 2023
Merged

fix: call fetch on start by default #43

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
71be90d
fix: call fetch on start by default
bgiori Nov 8, 2023
ba9f809
fix: make true config default
bgiori Nov 8, 2023
13e5020
fix: lint
bgiori Nov 8, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 2 additions & 15 deletions sdk/src/main/java/com/amplitude/experiment/DefaultExperimentClient.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ import com.amplitude.experiment.util.UserSessionExposureTracker
import com.amplitude.experiment.util.backoff
import com.amplitude.experiment.util.convertToVariant
import com.amplitude.experiment.util.isLocalEvaluationMode
import com.amplitude.experiment.util.isRemoteEvaluationMode
import com.amplitude.experiment.util.merge
import com.amplitude.experiment.util.toEvaluationContext
import com.amplitude.experiment.util.toJson
Expand Down Expand Up @@ -132,28 +131,16 @@ internal class DefaultExperimentClient internal constructor(
return this.executorService.submit(
Callable {
val flagsFuture = doFlags()
var remoteFlags = config.fetchOnStart
?: allFlags().values.any { it.isRemoteEvaluationMode() }
if (remoteFlags) {
flagsFuture.get()
if (config.fetchOnStart) {
fetchInternal(
getUserMergedWithProviderOrWait(10000),
config.fetchTimeoutMillis,
config.retryFetchOnFailure,
null
)
flagsFuture.get()
} else {
flagsFuture.get()
remoteFlags = config.fetchOnStart
?: allFlags().values.any { it.isRemoteEvaluationMode() }
if (remoteFlags) {
fetchInternal(
getUserMergedWithProviderOrWait(10000),
config.fetchTimeoutMillis,
config.retryFetchOnFailure,
null
)
}
}
this
}
Expand Down
8 changes: 2 additions & 6 deletions sdk/src/main/java/com/amplitude/experiment/ExperimentClient.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,8 @@ interface ExperimentClient {
* local flag configurations have been updated, and the fetch()
* result has been received (if the request was made).
*
* This function determines whether to fetch() based on the result of
* the flag configurations cached locally or received in the initial flag
* configuration response.
*
* To explicitly force this request to fetch or not, set the
* fetchOnStart configuration option when initializing the SDK.
* To force this function not to fetch variants, set the {@link fetchOnStart}
* configuration option to `false` when initializing the SDK.
*
* Finally, this function will start polling for flag configurations at a
* fixed interval. To disable polling, set the pollOnStart
Expand Down
6 changes: 3 additions & 3 deletions sdk/src/main/java/com/amplitude/experiment/ExperimentConfig.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ class ExperimentConfig internal constructor(
@JvmField
val pollOnStart: Boolean = Defaults.POLL_ON_START,
@JvmField
val fetchOnStart: Boolean? = Defaults.FETCH_ON_START,
val fetchOnStart: Boolean = Defaults.FETCH_ON_START,
@JvmField
val automaticFetchOnAmplitudeIdentityChange: Boolean = Defaults.AUTOMATIC_FETCH_ON_AMPLITUDE_IDENTITY_CHANGE,
@JvmField
Expand Down Expand Up @@ -129,7 +129,7 @@ class ExperimentConfig internal constructor(
/**
* null
*/
val FETCH_ON_START: Boolean? = null
const val FETCH_ON_START: Boolean = true

/**
* false
Expand Down Expand Up @@ -229,7 +229,7 @@ class ExperimentConfig internal constructor(
}

fun fetchOnStart(fetchOnStart: Boolean?) = apply {
this.fetchOnStart = fetchOnStart
this.fetchOnStart = fetchOnStart ?: true
}

fun automaticFetchOnAmplitudeIdentityChange(automaticFetchOnAmplitudeIdentityChange: Boolean) = apply {
Expand Down
4 changes: 2 additions & 2 deletions sdk/src/test/java/com/amplitude/experiment/ExperimentClientTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1078,7 +1078,7 @@ class ExperimentClientTest {
}

@Test
fun `start - with local evaluation only, does not call fetchInternal`() {
fun `start - with local evaluation only, calls fetchInternal`() {
val client = DefaultExperimentClient(
API_KEY,
ExperimentConfig(),
Expand All @@ -1089,7 +1089,7 @@ class ExperimentClientTest {
val spyClient = spyk(client)
every { spyClient.allFlags() } returns emptyMap()
spyClient.start(null).get()
verify(exactly = 0) { spyClient.fetchInternal(any(), any(), any(), any()) }
verify(exactly = 1) { spyClient.fetchInternal(any(), any(), any(), any()) }
}

@Test
Expand Down
Loading