From 347b359db37328f5be79ae10e1268b4cb4808cd0 Mon Sep 17 00:00:00 2001 From: AR Abdul Azeez Date: Thu, 9 Apr 2026 13:37:08 -0400 Subject: [PATCH 01/13] fix(ci): diff coverage uses PR base SHA and aggregate gate - Compare base.sha...head.sha on pull_request so the target branch matches the PR base. - Enforce >= threshold on aggregate touched executable lines; keep per-file detail informational. - Fail when main-source files in the diff cannot be mapped to JaCoCo. - Rename workflow step for clarity. Made-with: Cursor --- .github/workflows/ci.yml | 6 +- OneSignalSDK/coverage/checkCoverage.sh | 181 +++++++++++++++---------- 2 files changed, 114 insertions(+), 73 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index a925c0706..6e857b194 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -63,9 +63,13 @@ jobs: else echo "bypass=false" >> $GITHUB_OUTPUT fi - - name: "[Diff Coverage] Check coverage" + - name: "[PR] Coverage on changed lines (min ${{ env.DIFF_COVERAGE_THRESHOLD }}%)" if: github.event_name == 'pull_request' || github.event_name == 'workflow_dispatch' working-directory: OneSignalSDK + env: + # Exact PR diff (vs merge ref / wrong base branch) + DIFF_BASE_SHA: ${{ github.event_name == 'pull_request' && github.event.pull_request.base.sha || '' }} + DIFF_HEAD_SHA: ${{ github.event_name == 'pull_request' && github.event.pull_request.head.sha || '' }} run: | # Use the shared coverage check script for consistency # Generate markdown report for PR comments diff --git a/OneSignalSDK/coverage/checkCoverage.sh b/OneSignalSDK/coverage/checkCoverage.sh index 26708f0af..4b3eb461e 100755 --- a/OneSignalSDK/coverage/checkCoverage.sh +++ b/OneSignalSDK/coverage/checkCoverage.sh @@ -21,6 +21,9 @@ NC='\033[0m' # No Color # Configuration COVERAGE_THRESHOLD=${DIFF_COVERAGE_THRESHOLD:-80} BASE_BRANCH=${BASE_BRANCH:-origin/main} +# When set (e.g. GitHub PR), diff against these SHAs so we measure exactly the PR patch, not the merge ref +DIFF_BASE_SHA=${DIFF_BASE_SHA:-} +DIFF_HEAD_SHA=${DIFF_HEAD_SHA:-} GENERATE_MARKDOWN=${GENERATE_MARKDOWN:-false} # Set to 'true' for CI/CD to generate markdown report SKIP_COVERAGE_CHECK=${SKIP_COVERAGE_CHECK:-false} # Set to 'true' to bypass coverage check (still runs but doesn't fail) @@ -71,15 +74,27 @@ fi echo -e "${GREEN}✓ Coverage report generated${NC}\n" # Step 2: Check diff coverage using manual method (reliable path matching) -echo -e "${YELLOW}[2/3] Checking diff coverage against $BASE_BRANCH...${NC}" -echo -e "${YELLOW}Threshold: ${COVERAGE_THRESHOLD}%${NC}\n" +if [ -n "$DIFF_BASE_SHA" ] && [ -n "$DIFF_HEAD_SHA" ]; then + DIFF_RANGE="${DIFF_BASE_SHA}...${DIFF_HEAD_SHA}" + echo -e "${YELLOW}[2/3] Checking diff coverage for PR range ${DIFF_BASE_SHA:0:7}...${DIFF_HEAD_SHA:0:7}${NC}" +else + DIFF_RANGE="${BASE_BRANCH}...HEAD" + echo -e "${YELLOW}[2/3] Checking diff coverage against ${BASE_BRANCH}...HEAD${NC}" +fi +echo -e "${YELLOW}Threshold: ${COVERAGE_THRESHOLD}% (aggregate on changed executable lines)${NC}\n" # Get changed files (run from project root) -# Include committed changes, staged changes, and unstaged changes cd "$REPO_ROOT" -COMMITTED_FILES=$(git diff --name-only "$BASE_BRANCH"...HEAD 2>/dev/null | grep -E '\.(kt|java)$' || true) -STAGED_FILES=$(git diff --cached --name-only 2>/dev/null | grep -E '\.(kt|java)$' || true) -UNSTAGED_FILES=$(git diff --name-only 2>/dev/null | grep -E '\.(kt|java)$' || true) +if [ -n "$DIFF_BASE_SHA" ] && [ -n "$DIFF_HEAD_SHA" ]; then + COMMITTED_FILES=$(git diff --name-only "$DIFF_RANGE" 2>/dev/null | grep -E '\.(kt|java)$' || true) + STAGED_FILES="" + UNSTAGED_FILES="" +else + # Local / workflow_dispatch: committed vs base, plus working tree + COMMITTED_FILES=$(git diff --name-only "$BASE_BRANCH"...HEAD 2>/dev/null | grep -E '\.(kt|java)$' || true) + STAGED_FILES=$(git diff --cached --name-only 2>/dev/null | grep -E '\.(kt|java)$' || true) + UNSTAGED_FILES=$(git diff --name-only 2>/dev/null | grep -E '\.(kt|java)$' || true) +fi # Combine all, remove duplicates, and filter to OneSignalSDK files CHANGED_FILES=$(echo -e "$COMMITTED_FILES\n$STAGED_FILES\n$UNSTAGED_FILES" | grep -E '^OneSignalSDK/' | sort -u || true) @@ -102,6 +117,7 @@ else export MARKDOWN_REPORT export BASE_BRANCH export REPO_ROOT + export DIFF_RANGE python3 << PYEOF import xml.etree.ElementTree as ET import re @@ -116,47 +132,50 @@ generate_markdown = os.environ.get('GENERATE_MARKDOWN', 'false').lower() == 'tru markdown_report = os.environ.get('MARKDOWN_REPORT', 'diff_coverage.md') base_branch = os.environ.get('BASE_BRANCH', 'origin/main') repo_root_env = os.environ.get('REPO_ROOT') +diff_range = os.environ.get('DIFF_RANGE', '').strip() def get_changed_lines(file_path, project_root): """Get line numbers of added/modified lines from git diff""" try: - # First try to get diff from committed changes - result = subprocess.run( - ['git', 'diff', '--unified=0', base_branch + '...HEAD', '--', file_path], - capture_output=True, - text=True, - cwd=project_root - ) - - # If no committed changes, check staged changes - if result.returncode != 0 or not result.stdout.strip(): + if diff_range: result = subprocess.run( - ['git', 'diff', '--cached', '--unified=0', '--', file_path], + ['git', 'diff', '--unified=0', diff_range, '--', file_path], capture_output=True, text=True, cwd=project_root ) - - # If no staged changes, check unstaged changes - if result.returncode != 0 or not result.stdout.strip(): - result = subprocess.run( - ['git', 'diff', '--unified=0', '--', file_path], - capture_output=True, - text=True, - cwd=project_root - ) - - # If still nothing, try alternative base branch format - if result.returncode != 0 or not result.stdout.strip(): + if result.returncode != 0 or not result.stdout.strip(): + return None + else: result = subprocess.run( - ['git', 'diff', '--unified=0', base_branch, 'HEAD', '--', file_path], + ['git', 'diff', '--unified=0', base_branch + '...HEAD', '--', file_path], capture_output=True, text=True, cwd=project_root ) - - if result.returncode != 0 or not result.stdout.strip(): - return None + if result.returncode != 0 or not result.stdout.strip(): + result = subprocess.run( + ['git', 'diff', '--cached', '--unified=0', '--', file_path], + capture_output=True, + text=True, + cwd=project_root + ) + if result.returncode != 0 or not result.stdout.strip(): + result = subprocess.run( + ['git', 'diff', '--unified=0', '--', file_path], + capture_output=True, + text=True, + cwd=project_root + ) + if result.returncode != 0 or not result.stdout.strip(): + result = subprocess.run( + ['git', 'diff', '--unified=0', base_branch, 'HEAD', '--', file_path], + capture_output=True, + text=True, + cwd=project_root + ) + if result.returncode != 0 or not result.stdout.strip(): + return None changed_lines = set() current_new_line = None @@ -217,11 +236,15 @@ total_uncovered = 0 total_lines = 0 files_below_threshold = [] files_checked = [] +eligible_paths = [] markdown_output = [] if generate_markdown: markdown_output.append("## Diff Coverage Report (Changed Lines Only)\n") - markdown_output.append(f"**Threshold:** {threshold}%\n\n") + markdown_output.append( + f"**Gate:** aggregate coverage on **changed executable lines** must be ≥ **{threshold}%** " + "(JaCoCo line data for lines touched in the diff).\n\n" + ) markdown_output.append("### Changed Files Coverage\n\n") for changed_file in changed_files: @@ -237,7 +260,9 @@ for changed_file in changed_files: match = re.search(r'src/main/(java|kotlin)/(.+)/([^/]+\.(kt|java))$', path_part) if not match: continue - + + eligible_paths.append(changed_file) + package_path = match.group(2) filename = match.group(3) package_name = package_path.replace('/', '/') @@ -273,18 +298,14 @@ for changed_file in changed_files: coverage_pct = (file_covered / file_total * 100) if file_total > 0 else 100 if generate_markdown: - status = "✅" if coverage_pct >= threshold else "❌" - changed_info = f" ({len(changed_lines)} changed lines)" if changed_lines else " (all lines - could not determine changed lines)" - markdown_output.append(f"- {status} **{filename}**: {file_covered}/{file_total} changed lines ({coverage_pct:.1f}%){changed_info}") + changed_info = f" ({len(changed_lines)} touched lines in diff)" if changed_lines else " (all lines — could not resolve diff)" + markdown_output.append(f"- **{filename}**: {file_covered}/{file_total} touched executable lines ({coverage_pct:.1f}%){changed_info}") if coverage_pct < threshold: files_below_threshold.append((filename, coverage_pct, file_uncovered)) - markdown_output.append(f" - ⚠️ Below threshold: {file_uncovered} uncovered changed lines") + markdown_output.append(f" - {file_uncovered} uncovered touched lines in this file") else: - status = "✓" if coverage_pct >= threshold else "✗" - color = "" if coverage_pct >= threshold else "\033[0;31m" - reset = "\033[0m" if color else "" - changed_info = f" ({len(changed_lines)} changed lines)" if changed_lines else " (all lines - could not determine changed lines)" - print(f" {color}{status}{reset} {filename}: {file_covered}/{file_total} changed lines ({coverage_pct:.1f}%){changed_info}") + changed_info = f" ({len(changed_lines)} touched lines)" if changed_lines else " (all lines — could not resolve diff)" + print(f" {filename}: {file_covered}/{file_total} touched executable lines ({coverage_pct:.1f}%){changed_info}") if coverage_pct < threshold: files_below_threshold.append((filename, coverage_pct, file_uncovered)) break @@ -299,39 +320,43 @@ for changed_file in changed_files: if total_lines > 0: overall_coverage = ((total_lines - total_uncovered) / total_lines * 100) - + overall_pass = overall_coverage >= threshold + if generate_markdown: - markdown_output.append(f"\n### Overall Coverage (Changed Lines Only)\n") - markdown_output.append(f"**{total_lines - total_uncovered}/{total_lines}** changed lines covered ({overall_coverage:.1f}%)\n") - + markdown_output.append(f"\n### Overall (aggregate gate)\n") + markdown_output.append( + f"**{total_lines - total_uncovered}/{total_lines}** touched executable lines covered " + f"({overall_coverage:.1f}% — requires ≥ {threshold}%)\n" + ) if files_below_threshold: - markdown_output.append(f"\n### ❌ Coverage Check Failed\n") - markdown_output.append(f"Files below {threshold}% threshold:\n") + markdown_output.append(f"\n**Per-file detail** (informational; gate is aggregate above):\n") for filename, pct, uncovered in files_below_threshold: - markdown_output.append(f"- **{filename}**: {pct:.1f}% ({uncovered} uncovered changed lines)\n") - + markdown_output.append(f"- **{filename}**: {pct:.1f}% ({uncovered} uncovered touched lines)\n") + + if not overall_pass: + markdown_output.append(f"\n### ❌ Coverage Check Failed\n") + markdown_output.append( + f"Aggregate coverage on touched lines is **{overall_coverage:.1f}%** (minimum **{threshold}%**).\n" + ) + # Write markdown file with open(markdown_report, 'w') as f: f.write('\n'.join(markdown_output)) - + # Print to console print('\n'.join(markdown_output)) - - if files_below_threshold: - sys.exit(1) - else: - sys.exit(0) + + sys.exit(0 if overall_pass else 1) else: - print(f"\n Overall: {(total_lines - total_uncovered)}/{total_lines} changed lines covered ({overall_coverage:.1f}%)") - + print(f"\n Overall: {(total_lines - total_uncovered)}/{total_lines} touched executable lines covered ({overall_coverage:.1f}%)") + print(f" Gate: ≥ {threshold}% aggregate on touched lines — {'PASS' if overall_pass else 'FAIL'}") + if files_below_threshold: - print(f"\n Files below {threshold}% threshold:") + print(f"\n Per-file (informational):") for filename, pct, uncovered in files_below_threshold: - print(f" • {filename}: {pct:.1f}% ({uncovered} uncovered changed lines)") - sys.exit(1) - else: - print(f"\n ✓ All files meet {threshold}% threshold for changed lines") - sys.exit(0) + print(f" • {filename}: {pct:.1f}% ({uncovered} uncovered)") + + sys.exit(0 if overall_pass else 1) elif files_checked: # Files were found but had no executable lines if generate_markdown: @@ -343,14 +368,26 @@ elif files_checked: print("\n ✓ All checked files have no executable lines (or fully covered)") sys.exit(0) else: + if eligible_paths: + msg = ( + "Changed Kotlin/Java sources under `src/main` could not be matched to the merged JaCoCo report " + "(or the diff has no executable touched lines). Check module wiring and that unit tests run for those sources." + ) + if generate_markdown: + markdown_output.append(f"\n### ❌ No usable coverage for changed sources\n\n{msg}\n") + with open(markdown_report, 'w') as f: + f.write('\n'.join(markdown_output)) + print('\n'.join(markdown_output)) + else: + print(f"\n ✗ {msg}") + sys.exit(1) if generate_markdown: - markdown_output.append(f"\n### ⚠️ No Coverage Data\n") - markdown_output.append("No coverage data found for changed files\n") + markdown_output.append(f"\n### ✅ No main-source changes to gate\n") + markdown_output.append("No `src/main` Kotlin/Java changes in the diff.\n") with open(markdown_report, 'w') as f: f.write('\n'.join(markdown_output)) else: - print("\n ⚠ No coverage data found for changed files") - print(" This may mean files aren't being compiled or tested") + print("\n ✓ No main-source Kotlin/Java changes in the diff") sys.exit(0) PYEOF @@ -358,10 +395,10 @@ PYEOF if [ $CHECK_RESULT -eq 1 ]; then if [ "$GENERATE_MARKDOWN" != "true" ]; then if [ -n "$BYPASS_REASON" ]; then - echo -e "\n${YELLOW}⚠ Coverage below threshold (files below ${COVERAGE_THRESHOLD}%)${NC}" + echo -e "\n${YELLOW}⚠ Aggregate touched-line coverage below ${COVERAGE_THRESHOLD}%${NC}" echo -e "${YELLOW} Build will not fail due to bypass: $BYPASS_REASON${NC}\n" else - echo -e "\n${RED}✗ Coverage check failed (files below ${COVERAGE_THRESHOLD}% threshold)${NC}\n" + echo -e "\n${RED}✗ Coverage check failed (aggregate on touched lines < ${COVERAGE_THRESHOLD}%)${NC}\n" fi else # In markdown mode, update the report to indicate bypass if applicable From 769382528cb3cd5662951669d3be5781cb0b7574 Mon Sep 17 00:00:00 2001 From: AR Abdul Azeez Date: Tue, 14 Apr 2026 11:55:39 -0400 Subject: [PATCH 02/13] SDK-4363: Turbine remote SDK feature flags and foreground refresh - GET /apps/:app_id/sdk/features/android/:sdk_version with validated version label - FeatureFlagsJsonParser (kotlinx.serialization), metadata as sibling JsonObjects - FeatureFlagsRefreshService with ~10 min foreground polling; ConfigModelChangeTags.REMOTE_FEATURE_FLAGS - FeatureManager merges remote flags; IFeatureManager.remoteFeatureFlagMetadata() - SDK_BACKGROUND_THREADING / sdk_background_threading; CoreModule wiring and tests Made-with: Cursor --- OneSignalSDK/build.gradle | 18 +-- OneSignalSDK/onesignal/core/build.gradle | 2 + .../java/com/onesignal/core/CoreModule.kt | 5 + .../backend/IFeatureFlagsBackendService.kt | 28 +++++ .../impl/FeatureFlagsBackendService.kt | 90 +++++++++++++ .../backend/impl/FeatureFlagsJsonParser.kt | 100 +++++++++++++++ .../core/internal/config/ConfigModel.kt | 21 +++- .../internal/config/ConfigModelChangeTags.kt | 13 ++ .../config/impl/FeatureFlagsRefreshService.kt | 119 ++++++++++++++++++ .../core/internal/features/FeatureFlag.kt | 12 +- .../core/internal/features/FeatureManager.kt | 35 +++++- .../core/internal/startup/StartupService.kt | 2 +- .../crash/OneSignalCrashUploaderWrapper.kt | 2 +- .../com/onesignal/internal/OneSignalImp.kt | 2 +- .../impl/FeatureFlagsBackendServiceTest.kt | 50 ++++++++ .../impl/FeatureFlagsJsonParserTests.kt | 90 +++++++++++++ .../internal/features/FeatureFlagTests.kt | 8 +- .../internal/features/FeatureManagerTests.kt | 57 +++++++-- .../internal/startup/StartupServiceTests.kt | 3 +- .../OneSignalCrashUploaderWrapperTest.kt | 3 +- 20 files changed, 621 insertions(+), 39 deletions(-) create mode 100644 OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/backend/IFeatureFlagsBackendService.kt create mode 100644 OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/backend/impl/FeatureFlagsBackendService.kt create mode 100644 OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/backend/impl/FeatureFlagsJsonParser.kt create mode 100644 OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/config/ConfigModelChangeTags.kt create mode 100644 OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/config/impl/FeatureFlagsRefreshService.kt create mode 100644 OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/backend/impl/FeatureFlagsBackendServiceTest.kt create mode 100644 OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/backend/impl/FeatureFlagsJsonParserTests.kt diff --git a/OneSignalSDK/build.gradle b/OneSignalSDK/build.gradle index c70a36b00..c5a8437f5 100644 --- a/OneSignalSDK/build.gradle +++ b/OneSignalSDK/build.gradle @@ -17,6 +17,7 @@ buildscript { kotlinVersion = '1.9.25' dokkaVersion = '1.9.10' coroutinesVersion = '1.7.3' + kotlinxSerializationJsonVersion = '1.6.3' kotestVersion = '5.8.0' ioMockVersion = '1.13.2' // AndroidX Lifecycle and Activity versions @@ -38,14 +39,15 @@ buildscript { maven { url 'https://developer.huawei.com/repo/' } } sharedDeps = [ - "com.android.tools.build:gradle:$androidGradlePluginVersion", - "com.google.gms:google-services:$googleServicesGradlePluginVersion", - "com.huawei.agconnect:agcp:$huaweiAgconnectVersion", - "org.jetbrains.kotlin:kotlin-gradle-plugin:$kotlinVersion", - "org.jetbrains.dokka:dokka-gradle-plugin:$dokkaVersion", - "io.gitlab.arturbosch.detekt:detekt-gradle-plugin:$detektVersion", - "com.diffplug.spotless:spotless-plugin-gradle:$spotlessVersion", - "com.vanniktech.maven.publish:com.vanniktech.maven.publish.gradle.plugin:0.32.0" + "com.android.tools.build:gradle:$androidGradlePluginVersion", + "com.google.gms:google-services:$googleServicesGradlePluginVersion", + "com.huawei.agconnect:agcp:$huaweiAgconnectVersion", + "org.jetbrains.kotlin:kotlin-gradle-plugin:$kotlinVersion", + "org.jetbrains.kotlin:kotlin-serialization:$kotlinVersion", + "org.jetbrains.dokka:dokka-gradle-plugin:$dokkaVersion", + "io.gitlab.arturbosch.detekt:detekt-gradle-plugin:$detektVersion", + "com.diffplug.spotless:spotless-plugin-gradle:$spotlessVersion", + "com.vanniktech.maven.publish:com.vanniktech.maven.publish.gradle.plugin:0.32.0" ] } diff --git a/OneSignalSDK/onesignal/core/build.gradle b/OneSignalSDK/onesignal/core/build.gradle index bf1142512..41d78a3a1 100644 --- a/OneSignalSDK/onesignal/core/build.gradle +++ b/OneSignalSDK/onesignal/core/build.gradle @@ -1,6 +1,7 @@ plugins { id 'com.android.library' id 'kotlin-android' + id 'org.jetbrains.kotlin.plugin.serialization' id 'com.diffplug.spotless' id 'com.vanniktech.maven.publish' id 'io.gitlab.arturbosch.detekt' @@ -73,6 +74,7 @@ dependencies { implementation "org.jetbrains.kotlin:kotlin-stdlib:$kotlinVersion" implementation "org.jetbrains.kotlinx:kotlinx-coroutines-core:$coroutinesVersion" implementation "org.jetbrains.kotlinx:kotlinx-coroutines-android:$coroutinesVersion" + implementation "org.jetbrains.kotlinx:kotlinx-serialization-json:$kotlinxSerializationJsonVersion" // AndroidX Lifecycle and Activity implementation "androidx.lifecycle:lifecycle-viewmodel-ktx:$lifecycleVersion" diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/CoreModule.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/CoreModule.kt index 9d34231d6..9998fd213 100644 --- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/CoreModule.kt +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/CoreModule.kt @@ -4,12 +4,15 @@ import com.onesignal.common.modules.IModule import com.onesignal.common.services.ServiceBuilder import com.onesignal.core.internal.application.IApplicationService import com.onesignal.core.internal.application.impl.ApplicationService +import com.onesignal.core.internal.backend.IFeatureFlagsBackendService import com.onesignal.core.internal.backend.IParamsBackendService +import com.onesignal.core.internal.backend.impl.FeatureFlagsBackendService import com.onesignal.core.internal.backend.impl.ParamsBackendService import com.onesignal.core.internal.background.IBackgroundManager import com.onesignal.core.internal.background.impl.BackgroundManager import com.onesignal.core.internal.config.ConfigModelStore import com.onesignal.core.internal.config.impl.ConfigModelStoreListener +import com.onesignal.core.internal.config.impl.FeatureFlagsRefreshService import com.onesignal.core.internal.database.IDatabaseProvider import com.onesignal.core.internal.database.impl.DatabaseProvider import com.onesignal.core.internal.device.IDeviceService @@ -61,7 +64,9 @@ internal class CoreModule : IModule { builder.register().provides() builder.register().provides() builder.register().provides() + builder.register().provides() builder.register().provides() + builder.register().provides() // Operations builder.register().provides() diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/backend/IFeatureFlagsBackendService.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/backend/IFeatureFlagsBackendService.kt new file mode 100644 index 000000000..fcad601a4 --- /dev/null +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/backend/IFeatureFlagsBackendService.kt @@ -0,0 +1,28 @@ +package com.onesignal.core.internal.backend + +import kotlinx.serialization.json.JsonObject + +/** + * Result of the dedicated SDK feature-flags endpoint (separate from [IParamsBackendService]). + * + * @param enabledKeys Feature keys that should be treated as enabled for this device/SDK. + * @param metadata Optional per-flag payload (e.g. weights), keyed by flag id. Parsed from sibling + * keys in the response JSON (see [com.onesignal.core.internal.backend.impl.FeatureFlagsJsonParser]). + */ +internal data class RemoteFeatureFlagsResult( + val enabledKeys: List, + val metadata: JsonObject?, +) { + companion object { + val EMPTY = RemoteFeatureFlagsResult(emptyList(), null) + } +} + +/** + * Fetches remote feature flags for the current app via **GET** + * `apps/{app_id}/sdk/features/{platform}/{sdk_version}` (Turbine; see + * [com.onesignal.core.internal.backend.impl.FeatureFlagsBackendService]). + */ +internal interface IFeatureFlagsBackendService { + suspend fun fetchRemoteFeatureFlags(appId: String): RemoteFeatureFlagsResult +} diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/backend/impl/FeatureFlagsBackendService.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/backend/impl/FeatureFlagsBackendService.kt new file mode 100644 index 000000000..0258a305b --- /dev/null +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/backend/impl/FeatureFlagsBackendService.kt @@ -0,0 +1,90 @@ +package com.onesignal.core.internal.backend.impl + +import com.onesignal.common.OneSignalUtils +import com.onesignal.core.internal.backend.IFeatureFlagsBackendService +import com.onesignal.core.internal.backend.RemoteFeatureFlagsResult +import com.onesignal.core.internal.http.IHttpClient +import com.onesignal.debug.LogLevel +import com.onesignal.debug.internal.logging.Logging +import java.net.URLEncoder +import java.nio.charset.StandardCharsets + +/** + * Turbine SDK feature flags endpoint ([OneSignal/turbine#1681](https://github.com/OneSignal/turbine/pull/1681)). + * + * **GET** `apps/{app_id}/sdk/features/{platform}/{sdk_version}` relative to + * [com.onesignal.core.internal.config.ConfigModel.apiUrl] (app-provided base URL). + * + * - **platform** is always **`android`** for this SDK client. + * - **sdk_version** is [OneSignalUtils.sdkVersion] (same label as the `SDK-Version` header segment), e.g. + * `050801` or `050801-beta`; see [isValidFeaturesSdkVersionLabel]. + * + * Response: `{ "features": [ "flag_key", ... ] }`. + */ +internal class FeatureFlagsBackendService( + private val http: IHttpClient, +) : IFeatureFlagsBackendService { + override suspend fun fetchRemoteFeatureFlags(appId: String): RemoteFeatureFlagsResult { + Logging.log(LogLevel.DEBUG, "FeatureFlagsBackendService.fetchRemoteFeatureFlags(appId=$appId)") + + val sdkVersion = OneSignalUtils.sdkVersion + if (!isValidFeaturesSdkVersionLabel(sdkVersion)) { + Logging.warn( + "FeatureFlagsBackendService: sdk version not usable for Turbine path (expected " + + "6-digit label optional -suffix, e.g. 050801 or 050801-beta): '$sdkVersion'", + ) + return RemoteFeatureFlagsResult.EMPTY + } + + val path = + buildFeatureFlagsGetPath( + appId = appId, + platform = TURBINE_FEATURES_PLATFORM_ANDROID, + sdkVersion = sdkVersion, + ) + + val response = http.get(path, null) + if (!response.isSuccess || response.payload.isNullOrBlank()) { + Logging.debug( + "FeatureFlagsBackendService: non-success or empty body, status=${response.statusCode}", + ) + return RemoteFeatureFlagsResult.EMPTY + } + + return FeatureFlagsJsonParser.parse(response.payload!!) + } + + companion object { + /** + * Turbine `:platform` segment for the OneSignal Android SDK (this client). + */ + const val TURBINE_FEATURES_PLATFORM_ANDROID = "android" + + /** + * Labels produced by [OneSignalUtils.formatVersion] / [OneSignalUtils.sdkVersion]: six digits, + * optionally `-` and a non-empty prerelease/build suffix (no slashes or whitespace). + */ + private val FEATURES_SDK_VERSION_LABEL_REGEX = Regex("""^\d{6}(-[^/\s]+)?$""") + + /** + * Returns true when [label] is safe to send as the Turbine `:sdk_version` path segment. + */ + fun isValidFeaturesSdkVersionLabel(label: String): Boolean = FEATURES_SDK_VERSION_LABEL_REGEX.matches(label) + + /** + * Path only (relative to API base), matching `/apps/:app_id/sdk/features/:platform/:sdk_version`. + */ + internal fun buildFeatureFlagsGetPath( + appId: String, + platform: String, + sdkVersion: String, + ): String { + val p = encodePathSegment(platform) + val v = encodePathSegment(sdkVersion) + return "apps/$appId/sdk/features/$p/$v" + } + + private fun encodePathSegment(value: String): String = + URLEncoder.encode(value, StandardCharsets.UTF_8.name()).replace("+", "%20") + } +} diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/backend/impl/FeatureFlagsJsonParser.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/backend/impl/FeatureFlagsJsonParser.kt new file mode 100644 index 000000000..8c7fdb7f9 --- /dev/null +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/backend/impl/FeatureFlagsJsonParser.kt @@ -0,0 +1,100 @@ +package com.onesignal.core.internal.backend.impl + +import com.onesignal.core.internal.backend.RemoteFeatureFlagsResult +import kotlinx.serialization.json.Json +import kotlinx.serialization.json.JsonArray +import kotlinx.serialization.json.JsonElement +import kotlinx.serialization.json.JsonObject +import kotlinx.serialization.json.JsonPrimitive +import kotlinx.serialization.json.buildJsonObject + +/** + * Parses the feature-flags API response with kotlinx.serialization. + * + * Expected shape: + * ```json + * { + * "features": ["feature_a", "feature_b"], + * "feature_a": { "weight": 0.1 }, + * "feature_b": { ... } + * } + * ``` + * + * Turbine returns only `features` ([OneSignal/turbine#1681](https://github.com/OneSignal/turbine/pull/1681)). + * If the same flag id also appears as a root property with a JSON object value, that object is copied + * into [RemoteFeatureFlagsResult.metadata] for forward-compatible extras (flags without a sibling + * object stay enabled via [RemoteFeatureFlagsResult.enabledKeys] only). + * + * Payload must be valid JSON (e.g. a comma between `"features": [...]` and the next property). + */ +internal object FeatureFlagsJsonParser { + /** + * RFC 8259–style JSON only (no lenient tokens like unquoted keys, `NaN`, trailing commas). + * That keeps [encodeMetadata] strings portable: the same text can be parsed later by + * `org.json.JSONObject`, Gson, or kotlinx without relying on kotlinx-only quirks. + */ + val format = + Json { + ignoreUnknownKeys = true + isLenient = false + allowSpecialFloatingPointValues = false + prettyPrint = false + } + + fun parse(payload: String): RemoteFeatureFlagsResult { + return try { + val root = format.parseToJsonElement(payload) as? JsonObject ?: return RemoteFeatureFlagsResult.EMPTY + parseRoot(root) + } catch (_: Throwable) { + RemoteFeatureFlagsResult.EMPTY + } + } + + private fun parseRoot(root: JsonObject): RemoteFeatureFlagsResult { + val featuresEl = root["features"] ?: return RemoteFeatureFlagsResult.EMPTY + val featuresArray = featuresEl as? JsonArray ?: return RemoteFeatureFlagsResult.EMPTY + val keys = + featuresArray.mapNotNull { el -> + (el as? JsonPrimitive) + ?.takeIf { it.isString } + ?.content + ?.takeIf { it.isNotBlank() } + } + if (keys.isEmpty()) { + return RemoteFeatureFlagsResult(emptyList(), null) + } + + val metadata = + buildJsonObject { + for (key in keys) { + when (val v = root[key]) { + is JsonObject -> put(key, v) + else -> Unit + } + } + } + val metaOut = if (metadata.isEmpty()) null else metadata + return RemoteFeatureFlagsResult(keys, metaOut) + } + + fun encodeMetadata(metadata: JsonObject?): String? = + metadata?.let { format.encodeToString(JsonElement.serializer(), it) } + + /** + * Decodes [ConfigModel.sdkRemoteFeatureFlagMetadata] (a JSON object of flag id → object) into a map. + * Non-object values are skipped so each entry stays a [JsonObject] for nested decoding (e.g. with `Json.decodeFromJsonElement`). + */ + fun parseStoredMetadataMap(raw: String?): Map { + if (raw.isNullOrBlank()) { + return emptyMap() + } + return try { + val root = format.parseToJsonElement(raw) as? JsonObject ?: return emptyMap() + root.entries.mapNotNull { (key, value) -> + (value as? JsonObject)?.let { key to it } + }.toMap() + } catch (_: Throwable) { + emptyMap() + } + } +} diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/config/ConfigModel.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/config/ConfigModel.kt index a88739e05..004c6cba8 100644 --- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/config/ConfigModel.kt +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/config/ConfigModel.kt @@ -300,6 +300,25 @@ class ConfigModel : Model() { setListProperty(::features.name, value) } + /** + * Feature keys from the dedicated SDK feature-flags HTTP endpoint (see [com.onesignal.core.internal.backend.IFeatureFlagsBackendService]). + * Unioned with [features] for [com.onesignal.core.internal.features.FeatureManager]. + */ + var sdkRemoteFeatureFlags: List + get() = getListProperty(::sdkRemoteFeatureFlags.name) { emptyList() } + set(value) { + setListProperty(::sdkRemoteFeatureFlags.name, value) + } + + /** + * JSON object string: flag id → metadata (e.g. `note` or structured values) from the feature-flags endpoint. + */ + var sdkRemoteFeatureFlagMetadata: String? + get() = getOptStringProperty(::sdkRemoteFeatureFlagMetadata.name) + set(value) { + setOptStringProperty(::sdkRemoteFeatureFlagMetadata.name, value) + } + /** * The outcomes parameters */ @@ -344,7 +363,7 @@ class ConfigModel : Model() { property: String, jsonArray: JSONArray, ): List<*>? { - if (property == ::features.name) { + if (property == ::features.name || property == ::sdkRemoteFeatureFlags.name) { return buildList { for (i in 0 until jsonArray.length()) { val featureName = jsonArray.optString(i, "") diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/config/ConfigModelChangeTags.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/config/ConfigModelChangeTags.kt new file mode 100644 index 000000000..1c634c28d --- /dev/null +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/config/ConfigModelChangeTags.kt @@ -0,0 +1,13 @@ +package com.onesignal.core.internal.config + +/** + * [ModelChangeTags][com.onesignal.common.modeling.ModelChangeTags]-style values used only for + * [ConfigModel] / [ConfigModelStore] replace notifications. + */ +internal object ConfigModelChangeTags { + /** + * Remote feature-flags HTTP endpoint updated [ConfigModel] SDK flag fields only + * ([sdkRemoteFeatureFlags], [sdkRemoteFeatureFlagMetadata]). + */ + const val REMOTE_FEATURE_FLAGS = "REMOTE_FEATURE_FLAGS" +} diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/config/impl/FeatureFlagsRefreshService.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/config/impl/FeatureFlagsRefreshService.kt new file mode 100644 index 000000000..6e7ce3a8b --- /dev/null +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/config/impl/FeatureFlagsRefreshService.kt @@ -0,0 +1,119 @@ +package com.onesignal.core.internal.config.impl + +import com.onesignal.common.modeling.ISingletonModelStoreChangeHandler +import com.onesignal.common.modeling.ModelChangeTags +import com.onesignal.common.modeling.ModelChangedArgs +import com.onesignal.core.internal.config.ConfigModelChangeTags +import com.onesignal.common.threading.launchOnIO +import com.onesignal.core.internal.application.IApplicationLifecycleHandler +import com.onesignal.core.internal.application.IApplicationService +import com.onesignal.core.internal.backend.IFeatureFlagsBackendService +import com.onesignal.core.internal.backend.impl.FeatureFlagsJsonParser +import com.onesignal.core.internal.config.ConfigModel +import com.onesignal.core.internal.config.ConfigModelStore +import com.onesignal.core.internal.startup.IStartableService +import com.onesignal.debug.internal.logging.Logging +import kotlin.coroutines.coroutineContext +import kotlinx.coroutines.Job +import kotlinx.coroutines.delay +import kotlinx.coroutines.isActive + +/** + * Fetches remote SDK feature flags when the app is in the foreground, immediately on focus and then + * every [REFRESH_INTERVAL_MS] while the session stays in the foreground. Updates + * [ConfigModel.sdkRemoteFeatureFlags] / [ConfigModel.sdkRemoteFeatureFlagMetadata] so + * [com.onesignal.core.internal.features.FeatureManager] stays in sync. + */ +internal class FeatureFlagsRefreshService( + private val applicationService: IApplicationService, + private val configModelStore: ConfigModelStore, + private val featureFlagsBackend: IFeatureFlagsBackendService, +) : IStartableService, + IApplicationLifecycleHandler, + ISingletonModelStoreChangeHandler { + private var pollJob: Job? = null + + override fun start() { + applicationService.addApplicationLifecycleHandler(this) + configModelStore.subscribe(this) + if (applicationService.isInForeground) { + onFocus(firedOnSubscribe = true) + } + } + + override fun onFocus(firedOnSubscribe: Boolean) { + restartForegroundPolling() + } + + override fun onUnfocused() { + pollJob?.cancel() + pollJob = null + } + + override fun onModelUpdated( + args: ModelChangedArgs, + tag: String, + ) { + if (args.property != ConfigModel::appId.name) { + return + } + if (applicationService.isInForeground) { + restartForegroundPolling() + } + } + + override fun onModelReplaced( + model: ConfigModel, + tag: String, + ) { + if (tag == ConfigModelChangeTags.REMOTE_FEATURE_FLAGS) { + return + } + if (tag != ModelChangeTags.HYDRATE && tag != ModelChangeTags.NORMAL) { + return + } + if (model.appId.isNotEmpty() && applicationService.isInForeground) { + restartForegroundPolling() + } + } + + private fun restartForegroundPolling() { + pollJob?.cancel() + pollJob = + launchOnIO { + while (coroutineContext.isActive) { + if (!applicationService.isInForeground) { + break + } + val appId = configModelStore.model.appId + if (appId.isNotEmpty()) { + try { + fetchAndApply(appId) + } catch (t: Throwable) { + Logging.warn("FeatureFlagsRefreshService: fetch failed", t) + } + } + delay(REFRESH_INTERVAL_MS) + } + } + } + + private suspend fun fetchAndApply(appId: String) { + val result = featureFlagsBackend.fetchRemoteFeatureFlags(appId) + val current = configModelStore.model + val newMetaString = FeatureFlagsJsonParser.encodeMetadata(result.metadata) + if (result.enabledKeys == current.sdkRemoteFeatureFlags && newMetaString == current.sdkRemoteFeatureFlagMetadata) { + return + } + + val updated = ConfigModel() + updated.initializeFromModel(null, current) + updated.sdkRemoteFeatureFlags = result.enabledKeys + updated.sdkRemoteFeatureFlagMetadata = newMetaString + configModelStore.replace(updated, ConfigModelChangeTags.REMOTE_FEATURE_FLAGS) + } + + companion object { + private const val REFRESH_INTERVAL_MS = 600_000L + } +} diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/features/FeatureFlag.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/features/FeatureFlag.kt index f9bad1176..784589dc2 100644 --- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/features/FeatureFlag.kt +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/features/FeatureFlag.kt @@ -17,6 +17,8 @@ internal enum class FeatureActivationMode { /** * Backend-driven feature switches used by the SDK. + * + * [key] values are **lowercase** strings as returned from remote config / Turbine `features` arrays. */ internal enum class FeatureFlag( val key: String, @@ -24,14 +26,10 @@ internal enum class FeatureFlag( ) { // Threading mode is selected once per app startup to avoid mixed-mode behavior mid-session. // - // Naming convention: - // SDK__ + // Remote key (lowercase) must match backend / ConfigCat flag id. // - // Example: - // SDK_050800_BACKGROUND_THREADING - // - SDK_050800_BACKGROUND_THREADING( - "SDK_050800_BACKGROUND_THREADING", + SDK_BACKGROUND_THREADING( + "sdk_background_threading", FeatureActivationMode.APP_STARTUP ), ; diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/features/FeatureManager.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/features/FeatureManager.kt index 0a05fb5ae..982ace966 100644 --- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/features/FeatureManager.kt +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/features/FeatureManager.kt @@ -4,12 +4,21 @@ import com.onesignal.common.modeling.ISingletonModelStoreChangeHandler import com.onesignal.common.modeling.ModelChangeTags import com.onesignal.common.modeling.ModelChangedArgs import com.onesignal.common.threading.ThreadingMode +import com.onesignal.core.internal.backend.impl.FeatureFlagsJsonParser import com.onesignal.core.internal.config.ConfigModel +import com.onesignal.core.internal.config.ConfigModelChangeTags import com.onesignal.core.internal.config.ConfigModelStore import com.onesignal.debug.internal.logging.Logging +import kotlinx.serialization.json.JsonObject internal interface IFeatureManager { fun isEnabled(feature: FeatureFlag): Boolean + + /** + * Per-flag payloads from [com.onesignal.core.internal.backend.IFeatureFlagsBackendService]. + * Each value is a [JsonObject] so callers can decode nested fields or map to `@Serializable` types. + */ + fun remoteFeatureFlagMetadata(): Map } @Suppress("TooGenericExceptionCaught") @@ -31,13 +40,19 @@ internal class FeatureManager( override fun isEnabled(feature: FeatureFlag): Boolean = featureStates[feature] ?: false + override fun remoteFeatureFlagMetadata(): Map = + FeatureFlagsJsonParser.parseStoredMetadataMap(configModelStore.model.sdkRemoteFeatureFlagMetadata) + @Suppress("TooGenericExceptionCaught") override fun onModelReplaced( model: ConfigModel, tag: String, ) { Logging.debug("OneSignal: FeatureManager.onModelReplaced(tag=$tag)") - if (tag == ModelChangeTags.HYDRATE || tag == ModelChangeTags.NORMAL) { + if (tag == ModelChangeTags.HYDRATE || + tag == ModelChangeTags.NORMAL || + tag == ConfigModelChangeTags.REMOTE_FEATURE_FLAGS + ) { try { refreshEnabledFeatures(model, applyNextRunOnlyFeatures = false) } catch (t: Throwable) { @@ -51,7 +66,10 @@ internal class FeatureManager( args: ModelChangedArgs, tag: String, ) { - if (args.property == ConfigModel::features.name) { + if (args.property == ConfigModel::features.name || + args.property == ConfigModel::sdkRemoteFeatureFlags.name || + args.property == ConfigModel::sdkRemoteFeatureFlagMetadata.name + ) { Logging.debug("OneSignal: FeatureManager.onModelUpdated(property=${args.property}, tag=$tag)") try { refreshEnabledFeatures(configModelStore.model, applyNextRunOnlyFeatures = false) @@ -66,7 +84,12 @@ internal class FeatureManager( model: ConfigModel, applyNextRunOnlyFeatures: Boolean, ) { - val enabledFeatureKeys = (model.features + localFeatureOverrides).toSet() + val enabledFeatureKeys = + ( + model.features + + model.sdkRemoteFeatureFlags + + localFeatureOverrides + ).toSet() if (localFeatureOverrides.isNotEmpty()) { Logging.warn( "OneSignal: Local feature override enabled for testing only: $localFeatureOverrides", @@ -108,7 +131,7 @@ internal class FeatureManager( enabled: Boolean, ) { when (feature) { - FeatureFlag.SDK_050800_BACKGROUND_THREADING -> + FeatureFlag.SDK_BACKGROUND_THREADING -> ThreadingMode.updateUseBackgroundThreading( enabled = enabled, source = "FeatureManager:${feature.activationMode}" @@ -120,10 +143,10 @@ internal class FeatureManager( /** * Local-only test hook for forcing features ON without backend config. * Add feature keys here while testing locally, e.g.: - * setOf(FeatureFlag.BACKGROUND_THREADING.key) + * setOf(FeatureFlag.SDK_BACKGROUND_THREADING.key) */ private val localFeatureOverrides: Set = emptySet() // private val localFeatureOverrides: Set = -// setOf(FeatureFlag.BACKGROUND_THREADING.key) +// setOf(FeatureFlag.SDK_BACKGROUND_THREADING.key) } } diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/startup/StartupService.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/startup/StartupService.kt index b195716cc..7caa7602d 100644 --- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/startup/StartupService.kt +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/startup/StartupService.kt @@ -19,7 +19,7 @@ internal class StartupService( val useBackgroundThreading = try { val featureManager = services.getService() - featureManager.isEnabled(FeatureFlag.SDK_050800_BACKGROUND_THREADING) + featureManager.isEnabled(FeatureFlag.SDK_BACKGROUND_THREADING) } catch (t: Throwable) { Logging.warn("OneSignal: Failed to resolve BACKGROUND_THREADING in StartupService. Falling back to legacy thread.", t) false diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/debug/internal/crash/OneSignalCrashUploaderWrapper.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/debug/internal/crash/OneSignalCrashUploaderWrapper.kt index 8fc34c5d9..5b8fe7497 100644 --- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/debug/internal/crash/OneSignalCrashUploaderWrapper.kt +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/debug/internal/crash/OneSignalCrashUploaderWrapper.kt @@ -50,7 +50,7 @@ internal class OneSignalCrashUploaderWrapper( @Suppress("TooGenericExceptionCaught") override fun start() { if (!OtelSdkSupport.isSupported) return - if (featureManager.isEnabled(FeatureFlag.SDK_050800_BACKGROUND_THREADING)) { + if (featureManager.isEnabled(FeatureFlag.SDK_BACKGROUND_THREADING)) { OneSignalDispatchers.launchOnIO { try { uploader.start() diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/internal/OneSignalImp.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/internal/OneSignalImp.kt index 5cd9cb917..b58d28aba 100644 --- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/internal/OneSignalImp.kt +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/internal/OneSignalImp.kt @@ -180,7 +180,7 @@ internal class OneSignalImp( } return try { - featureManager.isEnabled(FeatureFlag.SDK_050800_BACKGROUND_THREADING) + featureManager.isEnabled(FeatureFlag.SDK_BACKGROUND_THREADING) } catch (t: Throwable) { Logging.warn("OneSignal: Failed to resolve BACKGROUND_THREADING feature, defaulting to legacy mode.", t) false diff --git a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/backend/impl/FeatureFlagsBackendServiceTest.kt b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/backend/impl/FeatureFlagsBackendServiceTest.kt new file mode 100644 index 000000000..705f6f16c --- /dev/null +++ b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/backend/impl/FeatureFlagsBackendServiceTest.kt @@ -0,0 +1,50 @@ +package com.onesignal.core.internal.backend.impl + +import com.onesignal.common.OneSignalUtils +import io.kotest.core.spec.style.FunSpec +import io.kotest.matchers.shouldBe + +class FeatureFlagsBackendServiceTest : FunSpec({ + test("buildFeatureFlagsGetPath matches Turbine /apps/:app_id/sdk/features/:platform/:sdk_version") { + FeatureFlagsBackendService.buildFeatureFlagsGetPath( + appId = "14719551-23f1-4d20-8dab-81496ffca5ea", + platform = FeatureFlagsBackendService.TURBINE_FEATURES_PLATFORM_ANDROID, + sdkVersion = "050801", + ) shouldBe "apps/14719551-23f1-4d20-8dab-81496ffca5ea/sdk/features/android/050801" + } + + test("buildFeatureFlagsGetPath encodes prerelease sdk version label") { + FeatureFlagsBackendService.buildFeatureFlagsGetPath( + appId = "14719551-23f1-4d20-8dab-81496ffca5ea", + platform = FeatureFlagsBackendService.TURBINE_FEATURES_PLATFORM_ANDROID, + sdkVersion = "050801-beta", + ) shouldBe "apps/14719551-23f1-4d20-8dab-81496ffca5ea/sdk/features/android/050801-beta" + } + + test("isValidFeaturesSdkVersionLabel accepts only 6-digit labels with optional -suffix") { + val valid = + listOf( + "050801", + "050801-beta", + "050801-beta1", + "010203-rc.2", + "000000", + ) + val invalid = + listOf( + "5.8.1", + "05080", + "0508010", + "", + "050801-", + "050801/", + "v050801", + ) + valid.forEach { FeatureFlagsBackendService.isValidFeaturesSdkVersionLabel(it) shouldBe true } + invalid.forEach { FeatureFlagsBackendService.isValidFeaturesSdkVersionLabel(it) shouldBe false } + } + + test("OneSignalUtils.sdkVersion from BuildConfig matches Turbine label rules") { + FeatureFlagsBackendService.isValidFeaturesSdkVersionLabel(OneSignalUtils.sdkVersion) shouldBe true + } +}) diff --git a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/backend/impl/FeatureFlagsJsonParserTests.kt b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/backend/impl/FeatureFlagsJsonParserTests.kt new file mode 100644 index 000000000..1c41e0922 --- /dev/null +++ b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/backend/impl/FeatureFlagsJsonParserTests.kt @@ -0,0 +1,90 @@ +package com.onesignal.core.internal.backend.impl + +import com.onesignal.core.internal.backend.RemoteFeatureFlagsResult +import io.kotest.core.spec.style.FunSpec +import io.kotest.matchers.shouldBe +import kotlinx.serialization.json.jsonObject +import kotlinx.serialization.json.jsonPrimitive + +class FeatureFlagsJsonParserTests : FunSpec({ + test("parses features array with sibling flag objects (sample contract)") { + val payload = + """ + { + "features": ["feature_a", "feature_b"], + "feature_a": { "weight": 0.1 }, + "feature_b": { "enabled": true } + } + """.trimIndent() + + val r = FeatureFlagsJsonParser.parse(payload) + + r.enabledKeys shouldBe listOf("feature_a", "feature_b") + val meta = r.metadata!! + meta["feature_a"]!!.toString().contains("weight") shouldBe true + meta["feature_a"]!!.toString().contains("0.1") shouldBe true + meta["feature_b"]!!.toString().contains("enabled") shouldBe true + } + + test("omits metadata entry when flag has no sibling object") { + val payload = """{"features":["only_key"]}""" + + val r = FeatureFlagsJsonParser.parse(payload) + + r.enabledKeys shouldBe listOf("only_key") + r.metadata shouldBe null + } + + test("features list plus sibling object for only some ids (valid JSON with commas)") { + val payload = + """ + { + "features": ["feature_a", "feature_b"], + "feature_a": { "weight": 0.1 } + } + """.trimIndent() + + val r = FeatureFlagsJsonParser.parse(payload) + + r.enabledKeys shouldBe listOf("feature_a", "feature_b") + val meta = r.metadata!! + meta.containsKey("feature_a") shouldBe true + meta.containsKey("feature_b") shouldBe false + meta["feature_a"]!!.jsonObject["weight"]!!.jsonPrimitive.content shouldBe "0.1" + } + + test("invalid json returns empty") { + FeatureFlagsJsonParser.parse("{") shouldBe RemoteFeatureFlagsResult.EMPTY + } + + test("encodeMetadata round-trips for storage string") { + val payload = + """ + {"features":["x"],"x":{"weight":2.5}} + """.trimIndent() + val r = FeatureFlagsJsonParser.parse(payload) + val encoded = FeatureFlagsJsonParser.encodeMetadata(r.metadata)!! + encoded.contains("weight") shouldBe true + encoded.contains("2.5") shouldBe true + } + + test("missing features key returns empty") { + FeatureFlagsJsonParser.parse("""{"feature_a":{}}""") shouldBe RemoteFeatureFlagsResult.EMPTY + } + + test("features not an array returns empty") { + FeatureFlagsJsonParser.parse("""{"features":"bad"}""") shouldBe RemoteFeatureFlagsResult.EMPTY + } + + test("parseStoredMetadataMap splits root object into flag id to JsonObject") { + val map = FeatureFlagsJsonParser.parseStoredMetadataMap("""{"a":{"weight":1},"b":2}""") + map.keys shouldBe setOf("a") + map.getValue("a")["weight"]!!.toString() shouldBe "1" + } + + test("parseStoredMetadataMap blank or invalid returns empty") { + FeatureFlagsJsonParser.parseStoredMetadataMap(null) shouldBe emptyMap() + FeatureFlagsJsonParser.parseStoredMetadataMap("") shouldBe emptyMap() + FeatureFlagsJsonParser.parseStoredMetadataMap("{") shouldBe emptyMap() + } +}) diff --git a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/features/FeatureFlagTests.kt b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/features/FeatureFlagTests.kt index 2a7c77cfe..54cf41b3a 100644 --- a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/features/FeatureFlagTests.kt +++ b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/features/FeatureFlagTests.kt @@ -4,15 +4,15 @@ import io.kotest.core.spec.style.FunSpec import io.kotest.matchers.shouldBe class FeatureFlagTests : FunSpec({ - test("feature flags use SDK_050800 style key naming") { - val keyPattern = Regex("^SDK_[0-9]{6}_[A-Z0-9_]+$") + test("feature flag remote keys are lowercase with underscores") { + val keyPattern = Regex("^[a-z0-9_]+$") FeatureFlag.entries.forEach { feature -> keyPattern.matches(feature.key) shouldBe true } } - test("BACKGROUND_THREADING uses the expected canonical key") { - FeatureFlag.SDK_050800_BACKGROUND_THREADING.key shouldBe "SDK_050800_BACKGROUND_THREADING" + test("SDK_BACKGROUND_THREADING uses the expected remote key") { + FeatureFlag.SDK_BACKGROUND_THREADING.key shouldBe "sdk_background_threading" } }) diff --git a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/features/FeatureManagerTests.kt b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/features/FeatureManagerTests.kt index e7edae371..1b30bfe23 100644 --- a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/features/FeatureManagerTests.kt +++ b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/features/FeatureManagerTests.kt @@ -7,6 +7,7 @@ import com.onesignal.core.internal.config.ConfigModelStore import io.kotest.core.spec.style.FunSpec import io.kotest.matchers.shouldBe import io.mockk.every +import kotlinx.serialization.json.jsonPrimitive import io.mockk.just import io.mockk.mockk import io.mockk.runs @@ -16,10 +17,16 @@ class FeatureManagerTests : FunSpec({ ThreadingMode.useBackgroundThreading = false } + fun stubConfigModel(model: ConfigModel) { + every { model.sdkRemoteFeatureFlags } returns emptyList() + every { model.sdkRemoteFeatureFlagMetadata } returns null + } + test("initial state should enable BACKGROUND_THREADING when feature is present") { // Given val initialModel = mockk() - every { initialModel.features } returns listOf(FeatureFlag.SDK_050800_BACKGROUND_THREADING.key) + stubConfigModel(initialModel) + every { initialModel.features } returns listOf(FeatureFlag.SDK_BACKGROUND_THREADING.key) val configModelStore = mockk() every { configModelStore.model } returns initialModel every { configModelStore.subscribe(any()) } just runs @@ -28,13 +35,29 @@ class FeatureManagerTests : FunSpec({ val manager = FeatureManager(configModelStore) // Then - manager.isEnabled(FeatureFlag.SDK_050800_BACKGROUND_THREADING) shouldBe true + manager.isEnabled(FeatureFlag.SDK_BACKGROUND_THREADING) shouldBe true + ThreadingMode.useBackgroundThreading shouldBe true + } + + test("initial state enables BACKGROUND_THREADING when key only exists on sdk remote flags") { + val initialModel = mockk() + stubConfigModel(initialModel) + every { initialModel.features } returns emptyList() + every { initialModel.sdkRemoteFeatureFlags } returns listOf(FeatureFlag.SDK_BACKGROUND_THREADING.key) + val configModelStore = mockk() + every { configModelStore.model } returns initialModel + every { configModelStore.subscribe(any()) } just runs + + val manager = FeatureManager(configModelStore) + + manager.isEnabled(FeatureFlag.SDK_BACKGROUND_THREADING) shouldBe true ThreadingMode.useBackgroundThreading shouldBe true } test("onModelReplaced should not switch threading mode after startup") { // Given val initialModel = mockk() + stubConfigModel(initialModel) every { initialModel.features } returns emptyList() val configModelStore = mockk() every { configModelStore.model } returns initialModel @@ -42,26 +65,28 @@ class FeatureManagerTests : FunSpec({ val manager = FeatureManager(configModelStore) val updatedModel = mockk() - every { updatedModel.features } returns listOf(FeatureFlag.SDK_050800_BACKGROUND_THREADING.key) + stubConfigModel(updatedModel) + every { updatedModel.features } returns listOf(FeatureFlag.SDK_BACKGROUND_THREADING.key) // When manager.onModelReplaced(updatedModel, ModelChangeTags.HYDRATE) // Then - manager.isEnabled(FeatureFlag.SDK_050800_BACKGROUND_THREADING) shouldBe false + manager.isEnabled(FeatureFlag.SDK_BACKGROUND_THREADING) shouldBe false ThreadingMode.useBackgroundThreading shouldBe false } test("onModelUpdated should not switch threading mode after startup") { // Given val model = mockk() + stubConfigModel(model) every { model.features } returns emptyList() val configModelStore = mockk() every { configModelStore.model } returns model every { configModelStore.subscribe(any()) } just runs val manager = FeatureManager(configModelStore) - every { model.features } returns listOf(FeatureFlag.SDK_050800_BACKGROUND_THREADING.key) + every { model.features } returns listOf(FeatureFlag.SDK_BACKGROUND_THREADING.key) // When manager.onModelUpdated( @@ -72,14 +97,15 @@ class FeatureManagerTests : FunSpec({ ) // Then - manager.isEnabled(FeatureFlag.SDK_050800_BACKGROUND_THREADING) shouldBe false + manager.isEnabled(FeatureFlag.SDK_BACKGROUND_THREADING) shouldBe false ThreadingMode.useBackgroundThreading shouldBe false } test("onModelUpdated should keep startup mode when initial mode is enabled") { // Given val model = mockk() - every { model.features } returns listOf(FeatureFlag.SDK_050800_BACKGROUND_THREADING.key) + stubConfigModel(model) + every { model.features } returns listOf(FeatureFlag.SDK_BACKGROUND_THREADING.key) val configModelStore = mockk() every { configModelStore.model } returns model every { configModelStore.subscribe(any()) } just runs @@ -96,7 +122,22 @@ class FeatureManagerTests : FunSpec({ ) // Then - manager.isEnabled(FeatureFlag.SDK_050800_BACKGROUND_THREADING) shouldBe true + manager.isEnabled(FeatureFlag.SDK_BACKGROUND_THREADING) shouldBe true ThreadingMode.useBackgroundThreading shouldBe true } + + test("remoteFeatureFlagMetadata returns parsed JSON from config") { + val initialModel = mockk() + stubConfigModel(initialModel) + every { initialModel.features } returns emptyList() + every { initialModel.sdkRemoteFeatureFlagMetadata } returns """{"X":{"note":"y"}}""" + val configModelStore = mockk() + every { configModelStore.model } returns initialModel + every { configModelStore.subscribe(any()) } just runs + + val manager = FeatureManager(configModelStore) + + val meta = manager.remoteFeatureFlagMetadata() + meta.getValue("X")["note"]!!.jsonPrimitive.content shouldBe "y" + } }) diff --git a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/startup/StartupServiceTests.kt b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/startup/StartupServiceTests.kt index dc7fa72bb..391aef73e 100644 --- a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/startup/StartupServiceTests.kt +++ b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/startup/StartupServiceTests.kt @@ -26,7 +26,8 @@ class StartupServiceTests : FunSpec({ backgroundThreadingEnabled: Boolean = true, ): ServiceProvider { val featureManager = mockk() - every { featureManager.isEnabled(FeatureFlag.SDK_050800_BACKGROUND_THREADING) } returns backgroundThreadingEnabled + every { featureManager.isEnabled(FeatureFlag.SDK_BACKGROUND_THREADING) } returns backgroundThreadingEnabled + every { featureManager.remoteFeatureFlagMetadata() } returns emptyMap() val serviceBuilder = ServiceBuilder() serviceBuilder.register(featureManager).provides() diff --git a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/debug/internal/crash/OneSignalCrashUploaderWrapperTest.kt b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/debug/internal/crash/OneSignalCrashUploaderWrapperTest.kt index 63ff63b74..b5004ba6e 100644 --- a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/debug/internal/crash/OneSignalCrashUploaderWrapperTest.kt +++ b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/debug/internal/crash/OneSignalCrashUploaderWrapperTest.kt @@ -41,7 +41,8 @@ class OneSignalCrashUploaderWrapperTest : FunSpec({ fun mockFeatureManager(backgroundThreadingEnabled: Boolean = true): IFeatureManager { val featureManager = mockk() - every { featureManager.isEnabled(FeatureFlag.SDK_050800_BACKGROUND_THREADING) } returns backgroundThreadingEnabled + every { featureManager.isEnabled(FeatureFlag.SDK_BACKGROUND_THREADING) } returns backgroundThreadingEnabled + every { featureManager.remoteFeatureFlagMetadata() } returns emptyMap() return featureManager } From c63f686ef2061423771e33ba64f4a193b3c45093 Mon Sep 17 00:00:00 2001 From: AR Abdul Azeez Date: Tue, 14 Apr 2026 12:27:40 -0400 Subject: [PATCH 03/13] feature flag PR tweaks --- .../impl/FeatureFlagsBackendService.kt | 31 ++++----- .../backend/impl/FeatureFlagsJsonParser.kt | 63 ++++++++++++++++--- .../impl/TurbineSdkFeatureFlagsPath.kt | 61 ++++++++++++++++++ .../core/internal/features/FeatureManager.kt | 13 +++- .../impl/FeatureFlagsJsonParserTests.kt | 33 +++++++--- .../impl/TurbineSdkFeatureFlagsPathTest.kt | 26 ++++++++ .../internal/features/FeatureManagerTests.kt | 16 ++++- .../internal/startup/StartupServiceTests.kt | 2 +- .../OneSignalCrashUploaderWrapperTest.kt | 2 +- 9 files changed, 202 insertions(+), 45 deletions(-) create mode 100644 OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/backend/impl/TurbineSdkFeatureFlagsPath.kt create mode 100644 OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/backend/impl/TurbineSdkFeatureFlagsPathTest.kt diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/backend/impl/FeatureFlagsBackendService.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/backend/impl/FeatureFlagsBackendService.kt index 0258a305b..b6a6885ce 100644 --- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/backend/impl/FeatureFlagsBackendService.kt +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/backend/impl/FeatureFlagsBackendService.kt @@ -6,12 +6,13 @@ import com.onesignal.core.internal.backend.RemoteFeatureFlagsResult import com.onesignal.core.internal.http.IHttpClient import com.onesignal.debug.LogLevel import com.onesignal.debug.internal.logging.Logging -import java.net.URLEncoder -import java.nio.charset.StandardCharsets /** * Turbine SDK feature flags endpoint ([OneSignal/turbine#1681](https://github.com/OneSignal/turbine/pull/1681)). * + * HTTP, logging, and [OneSignalUtils] are platform-specific; path shape and validation live in + * [TurbineSdkFeatureFlagsPath], and JSON parsing in [FeatureFlagsJsonParser] (both KMP-friendly). + * * **GET** `apps/{app_id}/sdk/features/{platform}/{sdk_version}` relative to * [com.onesignal.core.internal.config.ConfigModel.apiUrl] (app-provided base URL). * @@ -37,21 +38,22 @@ internal class FeatureFlagsBackendService( } val path = - buildFeatureFlagsGetPath( + TurbineSdkFeatureFlagsPath.buildGetPath( appId = appId, platform = TURBINE_FEATURES_PLATFORM_ANDROID, sdkVersion = sdkVersion, ) val response = http.get(path, null) - if (!response.isSuccess || response.payload.isNullOrBlank()) { + val body = response.payload + if (!response.isSuccess || body.isNullOrBlank()) { Logging.debug( "FeatureFlagsBackendService: non-success or empty body, status=${response.statusCode}", ) return RemoteFeatureFlagsResult.EMPTY } - return FeatureFlagsJsonParser.parse(response.payload!!) + return FeatureFlagsJsonParser.parse(body) } companion object { @@ -60,31 +62,20 @@ internal class FeatureFlagsBackendService( */ const val TURBINE_FEATURES_PLATFORM_ANDROID = "android" - /** - * Labels produced by [OneSignalUtils.formatVersion] / [OneSignalUtils.sdkVersion]: six digits, - * optionally `-` and a non-empty prerelease/build suffix (no slashes or whitespace). - */ - private val FEATURES_SDK_VERSION_LABEL_REGEX = Regex("""^\d{6}(-[^/\s]+)?$""") - /** * Returns true when [label] is safe to send as the Turbine `:sdk_version` path segment. + * @see TurbineSdkFeatureFlagsPath.isValidFeaturesSdkVersionLabel */ - fun isValidFeaturesSdkVersionLabel(label: String): Boolean = FEATURES_SDK_VERSION_LABEL_REGEX.matches(label) + fun isValidFeaturesSdkVersionLabel(label: String): Boolean = TurbineSdkFeatureFlagsPath.isValidFeaturesSdkVersionLabel(label) /** * Path only (relative to API base), matching `/apps/:app_id/sdk/features/:platform/:sdk_version`. + * @see TurbineSdkFeatureFlagsPath.buildGetPath */ internal fun buildFeatureFlagsGetPath( appId: String, platform: String, sdkVersion: String, - ): String { - val p = encodePathSegment(platform) - val v = encodePathSegment(sdkVersion) - return "apps/$appId/sdk/features/$p/$v" - } - - private fun encodePathSegment(value: String): String = - URLEncoder.encode(value, StandardCharsets.UTF_8.name()).replace("+", "%20") + ): String = TurbineSdkFeatureFlagsPath.buildGetPath(appId, platform, sdkVersion) } } diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/backend/impl/FeatureFlagsJsonParser.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/backend/impl/FeatureFlagsJsonParser.kt index 8c7fdb7f9..92a43125a 100644 --- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/backend/impl/FeatureFlagsJsonParser.kt +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/backend/impl/FeatureFlagsJsonParser.kt @@ -26,6 +26,13 @@ import kotlinx.serialization.json.buildJsonObject * object stay enabled via [RemoteFeatureFlagsResult.enabledKeys] only). * * Payload must be valid JSON (e.g. a comma between `"features": [...]` and the next property). + * + * Flag ids from the `features` array are normalized with [Char.lowercaseChar] (Unicode case mapping, no + * `java.util.Locale`) so they align with [com.onesignal.core.internal.features.FeatureFlag.key]. Sibling + * objects are resolved with exact key, then canonical key, then a case-insensitive match against other root + * properties (excluding `features`). + * + * Uses only Kotlin stdlib + kotlinx.serialization (Kotlin Multiplatform-friendly). */ internal object FeatureFlagsJsonParser { /** @@ -41,6 +48,8 @@ internal object FeatureFlagsJsonParser { prettyPrint = false } + private const val FEATURES_PROPERTY = "features" + fun parse(payload: String): RemoteFeatureFlagsResult { return try { val root = format.parseToJsonElement(payload) as? JsonObject ?: return RemoteFeatureFlagsResult.EMPTY @@ -51,32 +60,66 @@ internal object FeatureFlagsJsonParser { } private fun parseRoot(root: JsonObject): RemoteFeatureFlagsResult { - val featuresEl = root["features"] ?: return RemoteFeatureFlagsResult.EMPTY + val featuresEl = root[FEATURES_PROPERTY] ?: return RemoteFeatureFlagsResult.EMPTY val featuresArray = featuresEl as? JsonArray ?: return RemoteFeatureFlagsResult.EMPTY - val keys = + val flagEntries = featuresArray.mapNotNull { el -> (el as? JsonPrimitive) ?.takeIf { it.isString } ?.content - ?.takeIf { it.isNotBlank() } - } - if (keys.isEmpty()) { + ?.trim() + ?.takeIf { it.isNotEmpty() } + ?.let { raw -> raw to canonicalFeatureFlagId(raw) } + }.distinctBy { it.second } + + if (flagEntries.isEmpty()) { return RemoteFeatureFlagsResult(emptyList(), null) } + val keys = flagEntries.map { it.second } + val metadata = buildJsonObject { - for (key in keys) { - when (val v = root[key]) { - is JsonObject -> put(key, v) - else -> Unit - } + for ((rawKey, canonicalKey) in flagEntries) { + findSiblingJsonObject(root, rawKey, canonicalKey)?.let { put(canonicalKey, it) } } } val metaOut = if (metadata.isEmpty()) null else metadata return RemoteFeatureFlagsResult(keys, metaOut) } + private fun findSiblingJsonObject( + root: JsonObject, + rawKeyFromFeaturesArray: String, + canonicalKey: String, + ): JsonObject? { + for (candidate in listOf(rawKeyFromFeaturesArray, canonicalKey)) { + if (candidate == FEATURES_PROPERTY) { + continue + } + when (val v = root[candidate]) { + is JsonObject -> return v + else -> Unit + } + } + for ((k, v) in root) { + if (k == FEATURES_PROPERTY) { + continue + } + if (k.equals(rawKeyFromFeaturesArray, ignoreCase = true) && v is JsonObject) { + return v + } + } + return null + } + + private fun canonicalFeatureFlagId(raw: String): String = + buildString(raw.length) { + for (c in raw) { + append(c.lowercaseChar()) + } + } + fun encodeMetadata(metadata: JsonObject?): String? = metadata?.let { format.encodeToString(JsonElement.serializer(), it) } diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/backend/impl/TurbineSdkFeatureFlagsPath.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/backend/impl/TurbineSdkFeatureFlagsPath.kt new file mode 100644 index 000000000..89f0e9158 --- /dev/null +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/backend/impl/TurbineSdkFeatureFlagsPath.kt @@ -0,0 +1,61 @@ +package com.onesignal.core.internal.backend.impl + +/** + * Builds the Turbine-relative path for SDK feature flags and validates the SDK version label. + * + * Pure Kotlin only (no `java.*` / Android APIs) so this file can be reused from Kotlin Multiplatform `commonMain`. + */ +internal object TurbineSdkFeatureFlagsPath { + private val FEATURES_SDK_VERSION_LABEL_REGEX = Regex("""^\d{6}(-[^/\s]+)?$""") + + /** + * Labels expected in the `:sdk_version` path segment: six digits, optionally `-` and a non-empty suffix + * (no slashes or whitespace). + */ + fun isValidFeaturesSdkVersionLabel(label: String): Boolean = FEATURES_SDK_VERSION_LABEL_REGEX.matches(label) + + /** + * Path only (relative to API base), matching `apps/{app_id}/sdk/features/{platform}/{sdk_version}`. + * [platform] and [sdkVersion] are UTF-8 percent-encoded per RFC 3986 (unreserved bytes left as-is). + */ + fun buildGetPath( + appId: String, + platform: String, + sdkVersion: String, + ): String { + val p = percentEncodePathSegmentUtf8(platform) + val v = percentEncodePathSegmentUtf8(sdkVersion) + return "apps/$appId/sdk/features/$p/$v" + } + + /** + * Percent-encodes a UTF-8 string as a single path segment: unreserved bytes stay literal; everything else + * becomes `%HH` (uppercase hex). Space becomes `%20` (not `+`), matching typical REST path encoding. + */ + internal fun percentEncodePathSegmentUtf8(segment: String): String { + val bytes = segment.encodeToByteArray() + return buildString(bytes.size * 3) { + for (b in bytes) { + val u = b.toInt() and 0xff + if (isUnreservedByte(u)) { + append(u.toChar()) + } else { + append('%') + append(HEX_DIGITS[u shr 4]) + append(HEX_DIGITS[u and 15]) + } + } + } + } + + private fun isUnreservedByte(u: Int): Boolean = + u in 0x41..0x5A || + u in 0x61..0x7A || + u in 0x30..0x39 || + u == 0x2D || + u == 0x2E || + u == 0x5F || + u == 0x7E + + private const val HEX_DIGITS = "0123456789ABCDEF" +} diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/features/FeatureManager.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/features/FeatureManager.kt index 982ace966..2c9646605 100644 --- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/features/FeatureManager.kt +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/features/FeatureManager.kt @@ -17,8 +17,10 @@ internal interface IFeatureManager { /** * Per-flag payloads from [com.onesignal.core.internal.backend.IFeatureFlagsBackendService]. * Each value is a [JsonObject] so callers can decode nested fields or map to `@Serializable` types. + * + * `null` when no metadata has been stored yet ([ConfigModel.sdkRemoteFeatureFlagMetadata] null/blank). */ - fun remoteFeatureFlagMetadata(): Map + fun remoteFeatureFlagMetadata(): Map? } @Suppress("TooGenericExceptionCaught") @@ -40,8 +42,13 @@ internal class FeatureManager( override fun isEnabled(feature: FeatureFlag): Boolean = featureStates[feature] ?: false - override fun remoteFeatureFlagMetadata(): Map = - FeatureFlagsJsonParser.parseStoredMetadataMap(configModelStore.model.sdkRemoteFeatureFlagMetadata) + override fun remoteFeatureFlagMetadata(): Map? { + val raw = configModelStore.model.sdkRemoteFeatureFlagMetadata + if (raw.isNullOrBlank()) { + return null + } + return FeatureFlagsJsonParser.parseStoredMetadataMap(raw) + } @Suppress("TooGenericExceptionCaught") override fun onModelReplaced( diff --git a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/backend/impl/FeatureFlagsJsonParserTests.kt b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/backend/impl/FeatureFlagsJsonParserTests.kt index 1c41e0922..f882af324 100644 --- a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/backend/impl/FeatureFlagsJsonParserTests.kt +++ b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/backend/impl/FeatureFlagsJsonParserTests.kt @@ -20,10 +20,10 @@ class FeatureFlagsJsonParserTests : FunSpec({ val r = FeatureFlagsJsonParser.parse(payload) r.enabledKeys shouldBe listOf("feature_a", "feature_b") - val meta = r.metadata!! - meta["feature_a"]!!.toString().contains("weight") shouldBe true - meta["feature_a"]!!.toString().contains("0.1") shouldBe true - meta["feature_b"]!!.toString().contains("enabled") shouldBe true + val meta = requireNotNull(r.metadata) + meta.getValue("feature_a").toString().contains("weight") shouldBe true + meta.getValue("feature_a").toString().contains("0.1") shouldBe true + meta.getValue("feature_b").toString().contains("enabled") shouldBe true } test("omits metadata entry when flag has no sibling object") { @@ -47,10 +47,27 @@ class FeatureFlagsJsonParserTests : FunSpec({ val r = FeatureFlagsJsonParser.parse(payload) r.enabledKeys shouldBe listOf("feature_a", "feature_b") - val meta = r.metadata!! + val meta = requireNotNull(r.metadata) meta.containsKey("feature_a") shouldBe true meta.containsKey("feature_b") shouldBe false - meta["feature_a"]!!.jsonObject["weight"]!!.jsonPrimitive.content shouldBe "0.1" + val weight = requireNotNull(meta.getValue("feature_a").jsonObject["weight"]).jsonPrimitive.content + weight shouldBe "0.1" + } + + test("normalizes feature ids to lowercase and resolves sibling with mismatched casing") { + val payload = + """ + { + "features": ["SDK_Background_Threading"], + "sdk_background_threading": { "weight": 0.5 } + } + """.trimIndent() + + val r = FeatureFlagsJsonParser.parse(payload) + + r.enabledKeys shouldBe listOf("sdk_background_threading") + val meta = requireNotNull(r.metadata) + meta.getValue("sdk_background_threading").jsonObject.getValue("weight").jsonPrimitive.content shouldBe "0.5" } test("invalid json returns empty") { @@ -63,7 +80,7 @@ class FeatureFlagsJsonParserTests : FunSpec({ {"features":["x"],"x":{"weight":2.5}} """.trimIndent() val r = FeatureFlagsJsonParser.parse(payload) - val encoded = FeatureFlagsJsonParser.encodeMetadata(r.metadata)!! + val encoded = requireNotNull(FeatureFlagsJsonParser.encodeMetadata(r.metadata)) encoded.contains("weight") shouldBe true encoded.contains("2.5") shouldBe true } @@ -79,7 +96,7 @@ class FeatureFlagsJsonParserTests : FunSpec({ test("parseStoredMetadataMap splits root object into flag id to JsonObject") { val map = FeatureFlagsJsonParser.parseStoredMetadataMap("""{"a":{"weight":1},"b":2}""") map.keys shouldBe setOf("a") - map.getValue("a")["weight"]!!.toString() shouldBe "1" + requireNotNull(map.getValue("a")["weight"]).toString() shouldBe "1" } test("parseStoredMetadataMap blank or invalid returns empty") { diff --git a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/backend/impl/TurbineSdkFeatureFlagsPathTest.kt b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/backend/impl/TurbineSdkFeatureFlagsPathTest.kt new file mode 100644 index 000000000..13f16cb34 --- /dev/null +++ b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/backend/impl/TurbineSdkFeatureFlagsPathTest.kt @@ -0,0 +1,26 @@ +package com.onesignal.core.internal.backend.impl + +import io.kotest.core.spec.style.FunSpec +import io.kotest.matchers.shouldBe + +class TurbineSdkFeatureFlagsPathTest : FunSpec({ + test("percentEncodePathSegmentUtf8 leaves unreserved characters unchanged") { + TurbineSdkFeatureFlagsPath.percentEncodePathSegmentUtf8("android") shouldBe "android" + TurbineSdkFeatureFlagsPath.percentEncodePathSegmentUtf8("050801-beta") shouldBe "050801-beta" + TurbineSdkFeatureFlagsPath.percentEncodePathSegmentUtf8("aZ09-._~") shouldBe "aZ09-._~" + } + + test("percentEncodePathSegmentUtf8 encodes reserved and space as percent-hex") { + TurbineSdkFeatureFlagsPath.percentEncodePathSegmentUtf8("a/b") shouldBe "a%2Fb" + TurbineSdkFeatureFlagsPath.percentEncodePathSegmentUtf8("a b") shouldBe "a%20b" + TurbineSdkFeatureFlagsPath.percentEncodePathSegmentUtf8("café") shouldBe "caf%C3%A9" + } + + test("buildGetPath matches expected Turbine relative path") { + TurbineSdkFeatureFlagsPath.buildGetPath( + appId = "14719551-23f1-4d20-8dab-81496ffca5ea", + platform = "android", + sdkVersion = "050801-beta", + ) shouldBe "apps/14719551-23f1-4d20-8dab-81496ffca5ea/sdk/features/android/050801-beta" + } +}) diff --git a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/features/FeatureManagerTests.kt b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/features/FeatureManagerTests.kt index 1b30bfe23..af9e401f6 100644 --- a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/features/FeatureManagerTests.kt +++ b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/features/FeatureManagerTests.kt @@ -137,7 +137,19 @@ class FeatureManagerTests : FunSpec({ val manager = FeatureManager(configModelStore) - val meta = manager.remoteFeatureFlagMetadata() - meta.getValue("X")["note"]!!.jsonPrimitive.content shouldBe "y" + val meta = requireNotNull(manager.remoteFeatureFlagMetadata()) + requireNotNull(meta.getValue("X")["note"]).jsonPrimitive.content shouldBe "y" + } + + test("remoteFeatureFlagMetadata is null when config has no stored metadata") { + val initialModel = mockk() + stubConfigModel(initialModel) + every { initialModel.features } returns emptyList() + every { initialModel.sdkRemoteFeatureFlagMetadata } returns null + val configModelStore = mockk() + every { configModelStore.model } returns initialModel + every { configModelStore.subscribe(any()) } just runs + + FeatureManager(configModelStore).remoteFeatureFlagMetadata() shouldBe null } }) diff --git a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/startup/StartupServiceTests.kt b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/startup/StartupServiceTests.kt index 391aef73e..d8333ac8e 100644 --- a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/startup/StartupServiceTests.kt +++ b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/startup/StartupServiceTests.kt @@ -27,7 +27,7 @@ class StartupServiceTests : FunSpec({ ): ServiceProvider { val featureManager = mockk() every { featureManager.isEnabled(FeatureFlag.SDK_BACKGROUND_THREADING) } returns backgroundThreadingEnabled - every { featureManager.remoteFeatureFlagMetadata() } returns emptyMap() + every { featureManager.remoteFeatureFlagMetadata() } returns null val serviceBuilder = ServiceBuilder() serviceBuilder.register(featureManager).provides() diff --git a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/debug/internal/crash/OneSignalCrashUploaderWrapperTest.kt b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/debug/internal/crash/OneSignalCrashUploaderWrapperTest.kt index b5004ba6e..1f30da699 100644 --- a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/debug/internal/crash/OneSignalCrashUploaderWrapperTest.kt +++ b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/debug/internal/crash/OneSignalCrashUploaderWrapperTest.kt @@ -42,7 +42,7 @@ class OneSignalCrashUploaderWrapperTest : FunSpec({ fun mockFeatureManager(backgroundThreadingEnabled: Boolean = true): IFeatureManager { val featureManager = mockk() every { featureManager.isEnabled(FeatureFlag.SDK_BACKGROUND_THREADING) } returns backgroundThreadingEnabled - every { featureManager.remoteFeatureFlagMetadata() } returns emptyMap() + every { featureManager.remoteFeatureFlagMetadata() } returns null return featureManager } From 3720ec5840d972de2a73ab492bddb78914a5b4f1 Mon Sep 17 00:00:00 2001 From: AR Abdul Azeez Date: Tue, 14 Apr 2026 13:11:39 -0400 Subject: [PATCH 04/13] fix(core): address PR 2612 review for FeatureFlagsRefreshService - Annotate pollJob with @Volatile for cross-thread visibility - Remove duplicate onFocus from start(); ApplicationService already fires on subscribe - Apply remote flags via in-place model property updates (REMOTE_FEATURE_FLAGS tag) to avoid clobbering concurrent params hydration with a stale full-model replace - Use OneSignalDispatchers.launchOnIO so CancellationException is not logged as an error - Compare enabled key sets (order-insensitive) before writing Made-with: Cursor --- .../config/impl/FeatureFlagsRefreshService.kt | 25 +++++++++++-------- 1 file changed, 14 insertions(+), 11 deletions(-) diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/config/impl/FeatureFlagsRefreshService.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/config/impl/FeatureFlagsRefreshService.kt index 6e7ce3a8b..9520a42fa 100644 --- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/config/impl/FeatureFlagsRefreshService.kt +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/config/impl/FeatureFlagsRefreshService.kt @@ -4,7 +4,7 @@ import com.onesignal.common.modeling.ISingletonModelStoreChangeHandler import com.onesignal.common.modeling.ModelChangeTags import com.onesignal.common.modeling.ModelChangedArgs import com.onesignal.core.internal.config.ConfigModelChangeTags -import com.onesignal.common.threading.launchOnIO +import com.onesignal.common.threading.OneSignalDispatchers import com.onesignal.core.internal.application.IApplicationLifecycleHandler import com.onesignal.core.internal.application.IApplicationService import com.onesignal.core.internal.backend.IFeatureFlagsBackendService @@ -14,6 +14,7 @@ import com.onesignal.core.internal.config.ConfigModelStore import com.onesignal.core.internal.startup.IStartableService import com.onesignal.debug.internal.logging.Logging import kotlin.coroutines.coroutineContext +import kotlin.jvm.Volatile import kotlinx.coroutines.Job import kotlinx.coroutines.delay import kotlinx.coroutines.isActive @@ -23,6 +24,9 @@ import kotlinx.coroutines.isActive * every [REFRESH_INTERVAL_MS] while the session stays in the foreground. Updates * [ConfigModel.sdkRemoteFeatureFlags] / [ConfigModel.sdkRemoteFeatureFlagMetadata] so * [com.onesignal.core.internal.features.FeatureManager] stays in sync. + * + * Remote fields are updated in place on the live [ConfigModel] (with [ConfigModelChangeTags.REMOTE_FEATURE_FLAGS]) + * so concurrent hydration cannot be overwritten by a stale full-model snapshot. */ internal class FeatureFlagsRefreshService( private val applicationService: IApplicationService, @@ -31,14 +35,13 @@ internal class FeatureFlagsRefreshService( ) : IStartableService, IApplicationLifecycleHandler, ISingletonModelStoreChangeHandler { + @Volatile private var pollJob: Job? = null override fun start() { applicationService.addApplicationLifecycleHandler(this) configModelStore.subscribe(this) - if (applicationService.isInForeground) { - onFocus(firedOnSubscribe = true) - } + // Foreground-at-subscribe is handled by [IApplicationService.addApplicationLifecycleHandler] (fires onFocus). } override fun onFocus(firedOnSubscribe: Boolean) { @@ -80,7 +83,7 @@ internal class FeatureFlagsRefreshService( private fun restartForegroundPolling() { pollJob?.cancel() pollJob = - launchOnIO { + OneSignalDispatchers.launchOnIO { while (coroutineContext.isActive) { if (!applicationService.isInForeground) { break @@ -102,15 +105,15 @@ internal class FeatureFlagsRefreshService( val result = featureFlagsBackend.fetchRemoteFeatureFlags(appId) val current = configModelStore.model val newMetaString = FeatureFlagsJsonParser.encodeMetadata(result.metadata) - if (result.enabledKeys == current.sdkRemoteFeatureFlags && newMetaString == current.sdkRemoteFeatureFlagMetadata) { + if (result.enabledKeys.toSet() == current.sdkRemoteFeatureFlags.toSet() && + newMetaString == current.sdkRemoteFeatureFlagMetadata + ) { return } - val updated = ConfigModel() - updated.initializeFromModel(null, current) - updated.sdkRemoteFeatureFlags = result.enabledKeys - updated.sdkRemoteFeatureFlagMetadata = newMetaString - configModelStore.replace(updated, ConfigModelChangeTags.REMOTE_FEATURE_FLAGS) + val tag = ConfigModelChangeTags.REMOTE_FEATURE_FLAGS + current.setListProperty(ConfigModel::sdkRemoteFeatureFlags.name, result.enabledKeys, tag) + current.setOptStringProperty(ConfigModel::sdkRemoteFeatureFlagMetadata.name, newMetaString, tag) } companion object { From 64a106837b142057fc2a0238c5335ae93e5c6fbc Mon Sep 17 00:00:00 2001 From: AR Abdul Azeez Date: Mon, 20 Apr 2026 14:29:23 -0400 Subject: [PATCH 05/13] =?UTF-8?q?fix(core):=20PR=202612=20follow-ups=20?= =?UTF-8?q?=E2=80=94=20fetch=20outcome,=20cancellation,=20docs,=20key=20ca?= =?UTF-8?q?sing?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - RemoteFeatureFlagsFetchOutcome Success/Unavailable; parseSuccessful distinguishes valid Turbine JSON (including empty features) from parse/contract failure; no cache wipe on Unavailable - Polling loop: rethrow CancellationException; catch Exception only for fetch errors - FeatureManager: drop dead REMOTE_FEATURE_FLAGS onModelReplaced branch; canonicalize flag keys - ConfigModelChangeTags + FeatureFlag KDoc/comment fixes - Tests: parseSuccessful, case-insensitive remote key Made-with: Cursor --- .../backend/IFeatureFlagsBackendService.kt | 14 ++++++++++++- .../impl/FeatureFlagsBackendService.kt | 16 +++++++++----- .../backend/impl/FeatureFlagsJsonParser.kt | 21 ++++++++++++------- .../internal/config/ConfigModelChangeTags.kt | 2 +- .../config/impl/FeatureFlagsRefreshService.kt | 17 +++++++++------ .../core/internal/features/FeatureFlag.kt | 2 +- .../core/internal/features/FeatureManager.kt | 19 ++++++++++------- .../impl/FeatureFlagsJsonParserTests.kt | 12 +++++++++++ .../internal/features/FeatureManagerTests.kt | 15 +++++++++++++ 9 files changed, 89 insertions(+), 29 deletions(-) diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/backend/IFeatureFlagsBackendService.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/backend/IFeatureFlagsBackendService.kt index fcad601a4..cc71fae45 100644 --- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/backend/IFeatureFlagsBackendService.kt +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/backend/IFeatureFlagsBackendService.kt @@ -18,11 +18,23 @@ internal data class RemoteFeatureFlagsResult( } } +/** + * Outcome of [IFeatureFlagsBackendService.fetchRemoteFeatureFlags]. + * [Unavailable] means the client did not get a trustworthy response (HTTP error, invalid body, etc.); + * callers should keep previously cached flags. [Success] includes a valid HTTP parse, including an + * empty `features` array from the server. + */ +internal sealed class RemoteFeatureFlagsFetchOutcome { + data class Success(val result: RemoteFeatureFlagsResult) : RemoteFeatureFlagsFetchOutcome() + + data object Unavailable : RemoteFeatureFlagsFetchOutcome() +} + /** * Fetches remote feature flags for the current app via **GET** * `apps/{app_id}/sdk/features/{platform}/{sdk_version}` (Turbine; see * [com.onesignal.core.internal.backend.impl.FeatureFlagsBackendService]). */ internal interface IFeatureFlagsBackendService { - suspend fun fetchRemoteFeatureFlags(appId: String): RemoteFeatureFlagsResult + suspend fun fetchRemoteFeatureFlags(appId: String): RemoteFeatureFlagsFetchOutcome } diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/backend/impl/FeatureFlagsBackendService.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/backend/impl/FeatureFlagsBackendService.kt index b6a6885ce..66a6c3b0e 100644 --- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/backend/impl/FeatureFlagsBackendService.kt +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/backend/impl/FeatureFlagsBackendService.kt @@ -2,7 +2,7 @@ package com.onesignal.core.internal.backend.impl import com.onesignal.common.OneSignalUtils import com.onesignal.core.internal.backend.IFeatureFlagsBackendService -import com.onesignal.core.internal.backend.RemoteFeatureFlagsResult +import com.onesignal.core.internal.backend.RemoteFeatureFlagsFetchOutcome import com.onesignal.core.internal.http.IHttpClient import com.onesignal.debug.LogLevel import com.onesignal.debug.internal.logging.Logging @@ -25,7 +25,7 @@ import com.onesignal.debug.internal.logging.Logging internal class FeatureFlagsBackendService( private val http: IHttpClient, ) : IFeatureFlagsBackendService { - override suspend fun fetchRemoteFeatureFlags(appId: String): RemoteFeatureFlagsResult { + override suspend fun fetchRemoteFeatureFlags(appId: String): RemoteFeatureFlagsFetchOutcome { Logging.log(LogLevel.DEBUG, "FeatureFlagsBackendService.fetchRemoteFeatureFlags(appId=$appId)") val sdkVersion = OneSignalUtils.sdkVersion @@ -34,7 +34,7 @@ internal class FeatureFlagsBackendService( "FeatureFlagsBackendService: sdk version not usable for Turbine path (expected " + "6-digit label optional -suffix, e.g. 050801 or 050801-beta): '$sdkVersion'", ) - return RemoteFeatureFlagsResult.EMPTY + return RemoteFeatureFlagsFetchOutcome.Unavailable } val path = @@ -50,10 +50,16 @@ internal class FeatureFlagsBackendService( Logging.debug( "FeatureFlagsBackendService: non-success or empty body, status=${response.statusCode}", ) - return RemoteFeatureFlagsResult.EMPTY + return RemoteFeatureFlagsFetchOutcome.Unavailable } - return FeatureFlagsJsonParser.parse(body) + val parsed = FeatureFlagsJsonParser.parseSuccessful(body) + return if (parsed != null) { + RemoteFeatureFlagsFetchOutcome.Success(parsed) + } else { + Logging.debug("FeatureFlagsBackendService: response body is not valid Turbine feature-flags JSON") + RemoteFeatureFlagsFetchOutcome.Unavailable + } } companion object { diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/backend/impl/FeatureFlagsJsonParser.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/backend/impl/FeatureFlagsJsonParser.kt index 92a43125a..b1f754a83 100644 --- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/backend/impl/FeatureFlagsJsonParser.kt +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/backend/impl/FeatureFlagsJsonParser.kt @@ -50,18 +50,25 @@ internal object FeatureFlagsJsonParser { private const val FEATURES_PROPERTY = "features" - fun parse(payload: String): RemoteFeatureFlagsResult { + fun parse(payload: String): RemoteFeatureFlagsResult = parseSuccessful(payload) ?: RemoteFeatureFlagsResult.EMPTY + + /** + * Parses a successful Turbine response body. Returns `null` if JSON is invalid or the payload does not + * match the expected contract (missing `features` array, wrong types). A valid `{"features":[]}` returns + * [RemoteFeatureFlagsResult] with an empty [RemoteFeatureFlagsResult.enabledKeys] list. + */ + fun parseSuccessful(payload: String): RemoteFeatureFlagsResult? { return try { - val root = format.parseToJsonElement(payload) as? JsonObject ?: return RemoteFeatureFlagsResult.EMPTY - parseRoot(root) + val root = format.parseToJsonElement(payload) as? JsonObject ?: return null + parseRootStrict(root) } catch (_: Throwable) { - RemoteFeatureFlagsResult.EMPTY + null } } - private fun parseRoot(root: JsonObject): RemoteFeatureFlagsResult { - val featuresEl = root[FEATURES_PROPERTY] ?: return RemoteFeatureFlagsResult.EMPTY - val featuresArray = featuresEl as? JsonArray ?: return RemoteFeatureFlagsResult.EMPTY + private fun parseRootStrict(root: JsonObject): RemoteFeatureFlagsResult? { + val featuresEl = root[FEATURES_PROPERTY] ?: return null + val featuresArray = featuresEl as? JsonArray ?: return null val flagEntries = featuresArray.mapNotNull { el -> (el as? JsonPrimitive) diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/config/ConfigModelChangeTags.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/config/ConfigModelChangeTags.kt index 1c634c28d..6811c0332 100644 --- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/config/ConfigModelChangeTags.kt +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/config/ConfigModelChangeTags.kt @@ -7,7 +7,7 @@ package com.onesignal.core.internal.config internal object ConfigModelChangeTags { /** * Remote feature-flags HTTP endpoint updated [ConfigModel] SDK flag fields only - * ([sdkRemoteFeatureFlags], [sdkRemoteFeatureFlagMetadata]). + * ([ConfigModel.sdkRemoteFeatureFlags], [ConfigModel.sdkRemoteFeatureFlagMetadata]). */ const val REMOTE_FEATURE_FLAGS = "REMOTE_FEATURE_FLAGS" } diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/config/impl/FeatureFlagsRefreshService.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/config/impl/FeatureFlagsRefreshService.kt index 9520a42fa..9a2942f04 100644 --- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/config/impl/FeatureFlagsRefreshService.kt +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/config/impl/FeatureFlagsRefreshService.kt @@ -8,6 +8,7 @@ import com.onesignal.common.threading.OneSignalDispatchers import com.onesignal.core.internal.application.IApplicationLifecycleHandler import com.onesignal.core.internal.application.IApplicationService import com.onesignal.core.internal.backend.IFeatureFlagsBackendService +import com.onesignal.core.internal.backend.RemoteFeatureFlagsFetchOutcome import com.onesignal.core.internal.backend.impl.FeatureFlagsJsonParser import com.onesignal.core.internal.config.ConfigModel import com.onesignal.core.internal.config.ConfigModelStore @@ -15,6 +16,7 @@ import com.onesignal.core.internal.startup.IStartableService import com.onesignal.debug.internal.logging.Logging import kotlin.coroutines.coroutineContext import kotlin.jvm.Volatile +import kotlinx.coroutines.CancellationException import kotlinx.coroutines.Job import kotlinx.coroutines.delay import kotlinx.coroutines.isActive @@ -69,9 +71,6 @@ internal class FeatureFlagsRefreshService( model: ConfigModel, tag: String, ) { - if (tag == ConfigModelChangeTags.REMOTE_FEATURE_FLAGS) { - return - } if (tag != ModelChangeTags.HYDRATE && tag != ModelChangeTags.NORMAL) { return } @@ -92,8 +91,10 @@ internal class FeatureFlagsRefreshService( if (appId.isNotEmpty()) { try { fetchAndApply(appId) - } catch (t: Throwable) { - Logging.warn("FeatureFlagsRefreshService: fetch failed", t) + } catch (e: CancellationException) { + throw e + } catch (e: Exception) { + Logging.warn("FeatureFlagsRefreshService: fetch failed", e) } } delay(REFRESH_INTERVAL_MS) @@ -102,7 +103,11 @@ internal class FeatureFlagsRefreshService( } private suspend fun fetchAndApply(appId: String) { - val result = featureFlagsBackend.fetchRemoteFeatureFlags(appId) + val result = + when (val outcome = featureFlagsBackend.fetchRemoteFeatureFlags(appId)) { + RemoteFeatureFlagsFetchOutcome.Unavailable -> return + is RemoteFeatureFlagsFetchOutcome.Success -> outcome.result + } val current = configModelStore.model val newMetaString = FeatureFlagsJsonParser.encodeMetadata(result.metadata) if (result.enabledKeys.toSet() == current.sdkRemoteFeatureFlags.toSet() && diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/features/FeatureFlag.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/features/FeatureFlag.kt index 784589dc2..324421a71 100644 --- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/features/FeatureFlag.kt +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/features/FeatureFlag.kt @@ -26,7 +26,7 @@ internal enum class FeatureFlag( ) { // Threading mode is selected once per app startup to avoid mixed-mode behavior mid-session. // - // Remote key (lowercase) must match backend / ConfigCat flag id. + // Remote key (lowercase) must match backend / Turbine flag id. // SDK_BACKGROUND_THREADING( "sdk_background_threading", diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/features/FeatureManager.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/features/FeatureManager.kt index 2c9646605..6e2dae7f1 100644 --- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/features/FeatureManager.kt +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/features/FeatureManager.kt @@ -6,7 +6,6 @@ import com.onesignal.common.modeling.ModelChangedArgs import com.onesignal.common.threading.ThreadingMode import com.onesignal.core.internal.backend.impl.FeatureFlagsJsonParser import com.onesignal.core.internal.config.ConfigModel -import com.onesignal.core.internal.config.ConfigModelChangeTags import com.onesignal.core.internal.config.ConfigModelStore import com.onesignal.debug.internal.logging.Logging import kotlinx.serialization.json.JsonObject @@ -56,10 +55,7 @@ internal class FeatureManager( tag: String, ) { Logging.debug("OneSignal: FeatureManager.onModelReplaced(tag=$tag)") - if (tag == ModelChangeTags.HYDRATE || - tag == ModelChangeTags.NORMAL || - tag == ConfigModelChangeTags.REMOTE_FEATURE_FLAGS - ) { + if (tag == ModelChangeTags.HYDRATE || tag == ModelChangeTags.NORMAL) { try { refreshEnabledFeatures(model, applyNextRunOnlyFeatures = false) } catch (t: Throwable) { @@ -93,9 +89,9 @@ internal class FeatureManager( ) { val enabledFeatureKeys = ( - model.features + - model.sdkRemoteFeatureFlags + - localFeatureOverrides + model.features.map { canonicalizeFeatureKey(it) } + + model.sdkRemoteFeatureFlags.map { canonicalizeFeatureKey(it) } + + localFeatureOverrides.map { canonicalizeFeatureKey(it) } ).toSet() if (localFeatureOverrides.isNotEmpty()) { Logging.warn( @@ -133,6 +129,13 @@ internal class FeatureManager( featureStates = nextStates } + private fun canonicalizeFeatureKey(key: String): String = + buildString(key.length) { + for (c in key) { + append(c.lowercaseChar()) + } + } + private fun applySideEffects( feature: FeatureFlag, enabled: Boolean, diff --git a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/backend/impl/FeatureFlagsJsonParserTests.kt b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/backend/impl/FeatureFlagsJsonParserTests.kt index f882af324..286ff2339 100644 --- a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/backend/impl/FeatureFlagsJsonParserTests.kt +++ b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/backend/impl/FeatureFlagsJsonParserTests.kt @@ -74,6 +74,18 @@ class FeatureFlagsJsonParserTests : FunSpec({ FeatureFlagsJsonParser.parse("{") shouldBe RemoteFeatureFlagsResult.EMPTY } + test("parseSuccessful returns empty enabled keys for valid empty features array") { + val r = requireNotNull(FeatureFlagsJsonParser.parseSuccessful("""{"features":[]}""")) + r.enabledKeys shouldBe emptyList() + r.metadata shouldBe null + } + + test("parseSuccessful returns null for invalid json or contract") { + FeatureFlagsJsonParser.parseSuccessful("{") shouldBe null + FeatureFlagsJsonParser.parseSuccessful("""{"feature_a":{}}""") shouldBe null + FeatureFlagsJsonParser.parseSuccessful("""{"features":"bad"}""") shouldBe null + } + test("encodeMetadata round-trips for storage string") { val payload = """ diff --git a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/features/FeatureManagerTests.kt b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/features/FeatureManagerTests.kt index af9e401f6..fc3aa6949 100644 --- a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/features/FeatureManagerTests.kt +++ b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/features/FeatureManagerTests.kt @@ -54,6 +54,21 @@ class FeatureManagerTests : FunSpec({ ThreadingMode.useBackgroundThreading shouldBe true } + test("initial state enables BACKGROUND_THREADING when remote key differs only by letter case") { + val initialModel = mockk() + stubConfigModel(initialModel) + every { initialModel.features } returns emptyList() + every { initialModel.sdkRemoteFeatureFlags } returns listOf("SDK_Background_Threading") + val configModelStore = mockk() + every { configModelStore.model } returns initialModel + every { configModelStore.subscribe(any()) } just runs + + val manager = FeatureManager(configModelStore) + + manager.isEnabled(FeatureFlag.SDK_BACKGROUND_THREADING) shouldBe true + ThreadingMode.useBackgroundThreading shouldBe true + } + test("onModelReplaced should not switch threading mode after startup") { // Given val initialModel = mockk() From c7a27201e3dec5bc2c8cd23f96df763d05718276 Mon Sep 17 00:00:00 2001 From: AR Abdul Azeez Date: Tue, 21 Apr 2026 13:44:31 -0400 Subject: [PATCH 06/13] more logging --- .../impl/FeatureFlagsBackendService.kt | 35 ++++- .../config/impl/FeatureFlagsRefreshService.kt | 17 +- .../core/internal/http/HttpResponse.kt | 7 + .../impl/FeatureFlagsBackendServiceTests.kt | 145 ++++++++++++++++++ .../impl/FeatureFlagsJsonParserTests.kt | 5 + examples/demo/build.gradle.kts | 2 + 6 files changed, 203 insertions(+), 8 deletions(-) create mode 100644 OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/backend/impl/FeatureFlagsBackendServiceTests.kt diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/backend/impl/FeatureFlagsBackendService.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/backend/impl/FeatureFlagsBackendService.kt index 66a6c3b0e..f8231ce71 100644 --- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/backend/impl/FeatureFlagsBackendService.kt +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/backend/impl/FeatureFlagsBackendService.kt @@ -47,9 +47,12 @@ internal class FeatureFlagsBackendService( val response = http.get(path, null) val body = response.payload if (!response.isSuccess || body.isNullOrBlank()) { - Logging.debug( - "FeatureFlagsBackendService: non-success or empty body, status=${response.statusCode}", - ) + val msg = + "FeatureFlagsBackendService: non-success status=${response.statusCode} body=${bodySnippet(body)}" + // 4xx is likely a permanent misconfiguration (e.g. 403 Forbidden when the app is not + // enrolled for Turbine feature flags) and worth surfacing at WARN; other failures are + // typically transient (network blip, 5xx) and stay at DEBUG to avoid log spam. + if (response.isClientError) Logging.warn(msg) else Logging.debug(msg) return RemoteFeatureFlagsFetchOutcome.Unavailable } @@ -57,17 +60,41 @@ internal class FeatureFlagsBackendService( return if (parsed != null) { RemoteFeatureFlagsFetchOutcome.Success(parsed) } else { - Logging.debug("FeatureFlagsBackendService: response body is not valid Turbine feature-flags JSON") + Logging.warn( + "FeatureFlagsBackendService: response body is not valid Turbine feature-flags JSON: " + + bodySnippet(body), + ) RemoteFeatureFlagsFetchOutcome.Unavailable } } + /** + * Trim [body] to a short, single-line snippet safe for logcat. Caps at + * [LOG_BODY_SNIPPET_MAX_CHARS] so we never dump large payloads into logs. + */ + private fun bodySnippet(body: String?): String { + if (body.isNullOrEmpty()) return "" + val flattened = body.replace('\n', ' ').replace('\r', ' ') + return if (flattened.length <= LOG_BODY_SNIPPET_MAX_CHARS) { + flattened + } else { + flattened.take(LOG_BODY_SNIPPET_MAX_CHARS) + "…" + } + } + companion object { /** * Turbine `:platform` segment for the OneSignal Android SDK (this client). */ const val TURBINE_FEATURES_PLATFORM_ANDROID = "android" + /** + * Max chars of an HTTP response body included in diagnostic logs. Turbine error bodies + * (e.g. `{"errors":["Forbidden"]}`) are tiny, so 200 chars is plenty and bounds worst-case + * log size if an unexpected payload is returned. + */ + private const val LOG_BODY_SNIPPET_MAX_CHARS = 200 + /** * Returns true when [label] is safe to send as the Turbine `:sdk_version` path segment. * @see TurbineSdkFeatureFlagsPath.isValidFeaturesSdkVersionLabel diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/config/impl/FeatureFlagsRefreshService.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/config/impl/FeatureFlagsRefreshService.kt index 9a2942f04..41baa3d0e 100644 --- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/config/impl/FeatureFlagsRefreshService.kt +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/config/impl/FeatureFlagsRefreshService.kt @@ -97,6 +97,9 @@ internal class FeatureFlagsRefreshService( Logging.warn("FeatureFlagsRefreshService: fetch failed", e) } } + // TODO(revert-before-merge): makes the compiled poll interval obvious in logcat; remove + // for production. If you see 600_000ms here, the app is not using your local AAR. + Logging.debug("FeatureFlagsRefreshService: next fetch in ${REFRESH_INTERVAL_MS}ms") delay(REFRESH_INTERVAL_MS) } } @@ -110,9 +113,14 @@ internal class FeatureFlagsRefreshService( } val current = configModelStore.model val newMetaString = FeatureFlagsJsonParser.encodeMetadata(result.metadata) - if (result.enabledKeys.toSet() == current.sdkRemoteFeatureFlags.toSet() && - newMetaString == current.sdkRemoteFeatureFlagMetadata - ) { + val beforeKeys = current.sdkRemoteFeatureFlags.toSet() + val afterKeys = result.enabledKeys.toSet() + // TODO(revert-before-merge): verbose diff line for manual cold-start + mid-session flip testing. + Logging.debug( + "FeatureFlagsRefreshService: appId=$appId before=${beforeKeys.sorted()} " + + "after=${afterKeys.sorted()} changed=${beforeKeys != afterKeys}", + ) + if (afterKeys == beforeKeys && newMetaString == current.sdkRemoteFeatureFlagMetadata) { return } @@ -122,6 +130,7 @@ internal class FeatureFlagsRefreshService( } companion object { - private const val REFRESH_INTERVAL_MS = 600_000L + // Foreground polling cadence for remote feature flags (8 minutes). + private const val REFRESH_INTERVAL_MS = 480_000L } } diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/http/HttpResponse.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/http/HttpResponse.kt index fb62ccc24..64902b337 100644 --- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/http/HttpResponse.kt +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/http/HttpResponse.kt @@ -34,4 +34,11 @@ class HttpResponse( */ val isSuccess: Boolean get() = statusCode == HttpURLConnection.HTTP_OK || statusCode == HttpURLConnection.HTTP_ACCEPTED || statusCode == HttpURLConnection.HTTP_NOT_MODIFIED || statusCode == HttpURLConnection.HTTP_CREATED + + /** + * Whether the response is a client error (HTTP 4xx). Useful for distinguishing "probably + * permanent misconfiguration" from transient failures (5xx, network errors with statusCode=0). + */ + val isClientError: Boolean + get() = statusCode in HttpURLConnection.HTTP_BAD_REQUEST until HttpURLConnection.HTTP_INTERNAL_ERROR } diff --git a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/backend/impl/FeatureFlagsBackendServiceTests.kt b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/backend/impl/FeatureFlagsBackendServiceTests.kt new file mode 100644 index 000000000..26df536f6 --- /dev/null +++ b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/backend/impl/FeatureFlagsBackendServiceTests.kt @@ -0,0 +1,145 @@ +package com.onesignal.core.internal.backend.impl + +import com.onesignal.core.internal.backend.RemoteFeatureFlagsFetchOutcome +import com.onesignal.core.internal.http.HttpResponse +import com.onesignal.core.internal.http.IHttpClient +import com.onesignal.debug.ILogListener +import com.onesignal.debug.LogLevel +import com.onesignal.debug.OneSignalLogEvent +import com.onesignal.debug.internal.logging.Logging +import io.kotest.core.spec.style.FunSpec +import io.kotest.matchers.collections.shouldHaveSize +import io.kotest.matchers.shouldBe +import io.mockk.coEvery +import io.mockk.mockk + +/** + * Verifies that [FeatureFlagsBackendService] surfaces enough diagnostic information for a + * developer to understand why a remote feature-flags fetch returned + * [RemoteFeatureFlagsFetchOutcome.Unavailable]. In particular: + * - 4xx failures (e.g. 403 Forbidden for apps not enrolled in Turbine) should log at [LogLevel.WARN] + * and include a body snippet so the Turbine error payload shows up in logcat by default. + * - 5xx / transient failures should stay at [LogLevel.DEBUG] to avoid noisy logs for flaky networks. + * - A 200 with an unexpected body shape should log at [LogLevel.WARN] with a body snippet since it + * indicates a real contract/config issue that would otherwise be silent. + */ +class FeatureFlagsBackendServiceTests : FunSpec({ + lateinit var capturedLogs: MutableList + lateinit var listener: ILogListener + var originalLogLevel: LogLevel = LogLevel.WARN + + beforeEach { + originalLogLevel = Logging.logLevel + Logging.logLevel = LogLevel.NONE + capturedLogs = mutableListOf() + listener = ILogListener { event -> capturedLogs.add(event) } + Logging.addListener(listener) + } + + afterEach { + Logging.removeListener(listener) + Logging.logLevel = originalLogLevel + } + + fun logsForService(): List = + capturedLogs.filter { it.entry.contains("FeatureFlagsBackendService:") } + + test("403 Forbidden logs at WARN with body snippet and returns Unavailable") { + val http = mockk() + coEvery { http.get(any(), any()) } returns HttpResponse(403, """{"errors":["Forbidden"]}""") + + val outcome = FeatureFlagsBackendService(http).fetchRemoteFeatureFlags("appId") + + outcome shouldBe RemoteFeatureFlagsFetchOutcome.Unavailable + val warns = logsForService().filter { it.level == LogLevel.WARN } + warns shouldHaveSize 1 + warns[0].entry.contains("status=403") shouldBe true + warns[0].entry.contains("""{"errors":["Forbidden"]}""") shouldBe true + } + + test("404 Not Found also logs at WARN with body snippet") { + val http = mockk() + coEvery { http.get(any(), any()) } returns HttpResponse(404, """{"errors":["Not Found"]}""") + + FeatureFlagsBackendService(http).fetchRemoteFeatureFlags("appId") shouldBe + RemoteFeatureFlagsFetchOutcome.Unavailable + val warns = logsForService().filter { it.level == LogLevel.WARN } + warns shouldHaveSize 1 + warns[0].entry.contains("status=404") shouldBe true + } + + test("500 server error stays at DEBUG to avoid noisy logs for transient failures") { + val http = mockk() + coEvery { http.get(any(), any()) } returns HttpResponse(500, "boom") + + FeatureFlagsBackendService(http).fetchRemoteFeatureFlags("appId") shouldBe + RemoteFeatureFlagsFetchOutcome.Unavailable + logsForService().any { it.level == LogLevel.WARN } shouldBe false + val debugs = logsForService().filter { it.level == LogLevel.DEBUG } + debugs.any { it.entry.contains("status=500") && it.entry.contains("body=boom") } shouldBe true + } + + test("network error (statusCode=0) stays at DEBUG") { + val http = mockk() + coEvery { http.get(any(), any()) } returns HttpResponse(0, null) + + FeatureFlagsBackendService(http).fetchRemoteFeatureFlags("appId") shouldBe + RemoteFeatureFlagsFetchOutcome.Unavailable + logsForService().any { it.level == LogLevel.WARN } shouldBe false + val debugs = logsForService().filter { it.level == LogLevel.DEBUG } + debugs.any { it.entry.contains("status=0") && it.entry.contains("body=") } shouldBe true + } + + test("200 with non-contract JSON body logs at WARN with body snippet") { + val http = mockk() + coEvery { http.get(any(), any()) } returns HttpResponse(200, """{"errors":["Forbidden"]}""") + + FeatureFlagsBackendService(http).fetchRemoteFeatureFlags("appId") shouldBe + RemoteFeatureFlagsFetchOutcome.Unavailable + val warns = + logsForService().filter { + it.level == LogLevel.WARN && it.entry.contains("not valid Turbine feature-flags JSON") + } + warns shouldHaveSize 1 + warns[0].entry.contains("""{"errors":["Forbidden"]}""") shouldBe true + } + + test("body snippet caps long payloads at 200 chars") { + val longBody = "x".repeat(1_000) + val http = mockk() + coEvery { http.get(any(), any()) } returns HttpResponse(403, longBody) + + FeatureFlagsBackendService(http).fetchRemoteFeatureFlags("appId") + val warn = logsForService().first { it.level == LogLevel.WARN } + // Snippet is truncated to 200 chars plus an ellipsis marker. + warn.entry.contains("x".repeat(200) + "…") shouldBe true + warn.entry.contains("x".repeat(201)) shouldBe false + } + + test("newlines in body are flattened so log entry stays single-line") { + val http = mockk() + coEvery { http.get(any(), any()) } returns HttpResponse(403, "line1\nline2\rline3") + + FeatureFlagsBackendService(http).fetchRemoteFeatureFlags("appId") + val warn = logsForService().first { it.level == LogLevel.WARN } + warn.entry.contains("body=line1 line2 line3") shouldBe true + warn.entry.contains("\n") shouldBe false + warn.entry.contains("\r") shouldBe false + } + + test("200 with valid empty features array is Success and does not log at WARN") { + val http = mockk() + coEvery { http.get(any(), any()) } returns HttpResponse(200, """{"features":[]}""") + + val outcome = FeatureFlagsBackendService(http).fetchRemoteFeatureFlags("appId") + outcome.shouldBeSuccessWithEmptyKeys() + logsForService().any { it.level == LogLevel.WARN } shouldBe false + } +}) + +private fun RemoteFeatureFlagsFetchOutcome.shouldBeSuccessWithEmptyKeys() { + check(this is RemoteFeatureFlagsFetchOutcome.Success) { + "expected Success, got $this" + } + this.result.enabledKeys shouldBe emptyList() +} diff --git a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/backend/impl/FeatureFlagsJsonParserTests.kt b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/backend/impl/FeatureFlagsJsonParserTests.kt index 286ff2339..ba84d9e2d 100644 --- a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/backend/impl/FeatureFlagsJsonParserTests.kt +++ b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/backend/impl/FeatureFlagsJsonParserTests.kt @@ -86,6 +86,11 @@ class FeatureFlagsJsonParserTests : FunSpec({ FeatureFlagsJsonParser.parseSuccessful("""{"features":"bad"}""") shouldBe null } + test("parseSuccessful returns null for Turbine error body (e.g. 403 Forbidden)") { + FeatureFlagsJsonParser.parseSuccessful("""{"errors":["Forbidden"]}""") shouldBe null + FeatureFlagsJsonParser.parseSuccessful("""{"errors":["Not Found"]}""") shouldBe null + } + test("encodeMetadata round-trips for storage string") { val payload = """ diff --git a/examples/demo/build.gradle.kts b/examples/demo/build.gradle.kts index 159ff201c..3e21571e6 100644 --- a/examples/demo/build.gradle.kts +++ b/examples/demo/build.gradle.kts @@ -19,6 +19,8 @@ buildscript { allprojects { repositories { + // TODO(revert-before-merge): mavenLocal first so locally-published SDK builds + mavenLocal() google() mavenCentral() // Huawei maven From 8f47b9c3431354d4a61a1b9661aa9ddcd3501e01 Mon Sep 17 00:00:00 2001 From: AR Abdul Azeez Date: Tue, 21 Apr 2026 13:53:54 -0400 Subject: [PATCH 07/13] chore(core): drop revert-before-merge TODOs on kept diagnostics MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Keep the "next fetch in …ms" and before/after diff debug logs in FeatureFlagsRefreshService, and keep mavenLocal() in the demo's allprojects repositories; just remove the TODO(revert-before-merge) markers now that these are staying. Made-with: Cursor --- .../core/internal/config/impl/FeatureFlagsRefreshService.kt | 3 --- examples/demo/build.gradle.kts | 2 +- 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/config/impl/FeatureFlagsRefreshService.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/config/impl/FeatureFlagsRefreshService.kt index 41baa3d0e..dfdbefcbc 100644 --- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/config/impl/FeatureFlagsRefreshService.kt +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/config/impl/FeatureFlagsRefreshService.kt @@ -97,8 +97,6 @@ internal class FeatureFlagsRefreshService( Logging.warn("FeatureFlagsRefreshService: fetch failed", e) } } - // TODO(revert-before-merge): makes the compiled poll interval obvious in logcat; remove - // for production. If you see 600_000ms here, the app is not using your local AAR. Logging.debug("FeatureFlagsRefreshService: next fetch in ${REFRESH_INTERVAL_MS}ms") delay(REFRESH_INTERVAL_MS) } @@ -115,7 +113,6 @@ internal class FeatureFlagsRefreshService( val newMetaString = FeatureFlagsJsonParser.encodeMetadata(result.metadata) val beforeKeys = current.sdkRemoteFeatureFlags.toSet() val afterKeys = result.enabledKeys.toSet() - // TODO(revert-before-merge): verbose diff line for manual cold-start + mid-session flip testing. Logging.debug( "FeatureFlagsRefreshService: appId=$appId before=${beforeKeys.sorted()} " + "after=${afterKeys.sorted()} changed=${beforeKeys != afterKeys}", diff --git a/examples/demo/build.gradle.kts b/examples/demo/build.gradle.kts index 3e21571e6..b0dc0f2a9 100644 --- a/examples/demo/build.gradle.kts +++ b/examples/demo/build.gradle.kts @@ -19,7 +19,7 @@ buildscript { allprojects { repositories { - // TODO(revert-before-merge): mavenLocal first so locally-published SDK builds + // mavenLocal first so locally-published SDK builds resolve over Central when testing against a snapshot. mavenLocal() google() mavenCentral() From ce19184b55570baa71d3270cd8e13223442675af Mon Sep 17 00:00:00 2001 From: AR Abdul Azeez Date: Thu, 23 Apr 2026 12:15:35 -0400 Subject: [PATCH 08/13] Addressed comments --- .../impl/FeatureFlagsBackendService.kt | 8 ++- .../backend/impl/FeatureFlagsJsonParser.kt | 37 ++++++------- .../core/internal/config/ConfigModel.kt | 16 ++++-- .../internal/config/ConfigModelChangeTags.kt | 5 +- .../config/impl/FeatureFlagsRefreshService.kt | 53 +++++++++--------- .../impl/FeatureFlagsBackendServiceTest.kt | 50 ----------------- .../impl/FeatureFlagsBackendServiceTests.kt | 54 +++++++++++++++++++ examples/demo/build.gradle.kts | 2 - 8 files changed, 115 insertions(+), 110 deletions(-) delete mode 100644 OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/backend/impl/FeatureFlagsBackendServiceTest.kt diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/backend/impl/FeatureFlagsBackendService.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/backend/impl/FeatureFlagsBackendService.kt index f8231ce71..0fbd26d5f 100644 --- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/backend/impl/FeatureFlagsBackendService.kt +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/backend/impl/FeatureFlagsBackendService.kt @@ -46,7 +46,7 @@ internal class FeatureFlagsBackendService( val response = http.get(path, null) val body = response.payload - if (!response.isSuccess || body.isNullOrBlank()) { + if (!response.isSuccess) { val msg = "FeatureFlagsBackendService: non-success status=${response.statusCode} body=${bodySnippet(body)}" // 4xx is likely a permanent misconfiguration (e.g. 403 Forbidden when the app is not @@ -55,6 +55,12 @@ internal class FeatureFlagsBackendService( if (response.isClientError) Logging.warn(msg) else Logging.debug(msg) return RemoteFeatureFlagsFetchOutcome.Unavailable } + if (body.isNullOrBlank()) { + Logging.warn( + "FeatureFlagsBackendService: empty body for success status=${response.statusCode}", + ) + return RemoteFeatureFlagsFetchOutcome.Unavailable + } val parsed = FeatureFlagsJsonParser.parseSuccessful(body) return if (parsed != null) { diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/backend/impl/FeatureFlagsJsonParser.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/backend/impl/FeatureFlagsJsonParser.kt index b1f754a83..560bd04a3 100644 --- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/backend/impl/FeatureFlagsJsonParser.kt +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/backend/impl/FeatureFlagsJsonParser.kt @@ -9,28 +9,21 @@ import kotlinx.serialization.json.JsonPrimitive import kotlinx.serialization.json.buildJsonObject /** - * Parses the feature-flags API response with kotlinx.serialization. + * **What this is:** strict JSON parsing for the Turbine SDK feature-flags response + * ([OneSignal/turbine#1681](https://github.com/OneSignal/turbine/pull/1681)). * - * Expected shape: - * ```json - * { - * "features": ["feature_a", "feature_b"], - * "feature_a": { "weight": 0.1 }, - * "feature_b": { ... } - * } - * ``` + * **Wire shape:** root object with a `features` array of **string** flag ids. Optional per-flag JSON + * objects may appear as **sibling root properties** (same name as the id); those are merged into + * [RemoteFeatureFlagsResult.metadata]. * - * Turbine returns only `features` ([OneSignal/turbine#1681](https://github.com/OneSignal/turbine/pull/1681)). - * If the same flag id also appears as a root property with a JSON object value, that object is copied - * into [RemoteFeatureFlagsResult.metadata] for forward-compatible extras (flags without a sibling - * object stay enabled via [RemoteFeatureFlagsResult.enabledKeys] only). + * **Optional metadata:** for a string entry `"foo"`, if the root also has a property `"foo"` (or + * case-insensitive match) whose value is a JSON object, that object is stored per-flag in + * [RemoteFeatureFlagsResult.metadata]. That lets a future API add per-flag config without a new + * top-level field; the SDK persists it in [com.onesignal.core.internal.config.ConfigModel.sdkRemoteFeatureFlagMetadata] + * so a later process or SDK version can read it without re-fetching. * - * Payload must be valid JSON (e.g. a comma between `"features": [...]` and the next property). - * - * Flag ids from the `features` array are normalized with [Char.lowercaseChar] (Unicode case mapping, no - * `java.util.Locale`) so they align with [com.onesignal.core.internal.features.FeatureFlag.key]. Sibling - * objects are resolved with exact key, then canonical key, then a case-insensitive match against other root - * properties (excluding `features`). + * **API surface:** [parseSuccessful] for HTTP 200 bodies; [parse] for lenient “best effort”; + * [encodeMetadata] / [parseStoredMetadataMap] for the persisted metadata column. * * Uses only Kotlin stdlib + kotlinx.serialization (Kotlin Multiplatform-friendly). */ @@ -53,9 +46,9 @@ internal object FeatureFlagsJsonParser { fun parse(payload: String): RemoteFeatureFlagsResult = parseSuccessful(payload) ?: RemoteFeatureFlagsResult.EMPTY /** - * Parses a successful Turbine response body. Returns `null` if JSON is invalid or the payload does not - * match the expected contract (missing `features` array, wrong types). A valid `{"features":[]}` returns - * [RemoteFeatureFlagsResult] with an empty [RemoteFeatureFlagsResult.enabledKeys] list. + * Parses a 200 response body. Returns `null` if the text is not JSON, not an object, or does not + * contain a `features` **array** of the expected element types. Returns an empty result for + * `{"features":[]}`. */ fun parseSuccessful(payload: String): RemoteFeatureFlagsResult? { return try { diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/config/ConfigModel.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/config/ConfigModel.kt index 004c6cba8..bfaf08eb8 100644 --- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/config/ConfigModel.kt +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/config/ConfigModel.kt @@ -291,8 +291,9 @@ class ConfigModel : Model() { } /** - * Remote feature switches controlled by backend. - * Presence of a feature name indicates enabled. + * Feature ids from the main OneSignal config / params response (legacy channel). + * Presence of a name means the feature is on. This list is distinct from [sdkRemoteFeatureFlags]; + * [com.onesignal.core.internal.features.FeatureManager] unions both (plus any local test overrides). */ var features: List get() = getListProperty(::features.name) { emptyList() } @@ -301,8 +302,10 @@ class ConfigModel : Model() { } /** - * Feature keys from the dedicated SDK feature-flags HTTP endpoint (see [com.onesignal.core.internal.backend.IFeatureFlagsBackendService]). - * Unioned with [features] for [com.onesignal.core.internal.features.FeatureManager]. + * Feature ids from the Turbine SDK feature-flags HTTP endpoint + * ([com.onesignal.core.internal.backend.IFeatureFlagsBackendService]), updated while the app is + * foreground. Unioned with [features] in [com.onesignal.core.internal.features.FeatureManager] + * when evaluating [com.onesignal.core.internal.features.FeatureFlag] entries. */ var sdkRemoteFeatureFlags: List get() = getListProperty(::sdkRemoteFeatureFlags.name) { emptyList() } @@ -311,7 +314,10 @@ class ConfigModel : Model() { } /** - * JSON object string: flag id → metadata (e.g. `note` or structured values) from the feature-flags endpoint. + * Compacted JSON object: canonical flag id → metadata object, from the same Turbine response + * as [sdkRemoteFeatureFlags]. Persisted so metadata survives process death and is available + * to [com.onesignal.core.internal.features.IFeatureManager.remoteFeatureFlagMetadata] without + * a new network fetch on the next launch. */ var sdkRemoteFeatureFlagMetadata: String? get() = getOptStringProperty(::sdkRemoteFeatureFlagMetadata.name) diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/config/ConfigModelChangeTags.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/config/ConfigModelChangeTags.kt index 6811c0332..dbe741753 100644 --- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/config/ConfigModelChangeTags.kt +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/config/ConfigModelChangeTags.kt @@ -6,8 +6,9 @@ package com.onesignal.core.internal.config */ internal object ConfigModelChangeTags { /** - * Remote feature-flags HTTP endpoint updated [ConfigModel] SDK flag fields only - * ([ConfigModel.sdkRemoteFeatureFlags], [ConfigModel.sdkRemoteFeatureFlagMetadata]). + * A partial update from the Turbine feature-flags refresh: only + * [ConfigModel.sdkRemoteFeatureFlags] and [ConfigModel.sdkRemoteFeatureFlagMetadata] changed + * (in-place on the live model, not a full [ConfigModel] replace). */ const val REMOTE_FEATURE_FLAGS = "REMOTE_FEATURE_FLAGS" } diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/config/impl/FeatureFlagsRefreshService.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/config/impl/FeatureFlagsRefreshService.kt index dfdbefcbc..7ff63499a 100644 --- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/config/impl/FeatureFlagsRefreshService.kt +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/config/impl/FeatureFlagsRefreshService.kt @@ -3,7 +3,6 @@ package com.onesignal.core.internal.config.impl import com.onesignal.common.modeling.ISingletonModelStoreChangeHandler import com.onesignal.common.modeling.ModelChangeTags import com.onesignal.common.modeling.ModelChangedArgs -import com.onesignal.core.internal.config.ConfigModelChangeTags import com.onesignal.common.threading.OneSignalDispatchers import com.onesignal.core.internal.application.IApplicationLifecycleHandler import com.onesignal.core.internal.application.IApplicationService @@ -11,15 +10,15 @@ import com.onesignal.core.internal.backend.IFeatureFlagsBackendService import com.onesignal.core.internal.backend.RemoteFeatureFlagsFetchOutcome import com.onesignal.core.internal.backend.impl.FeatureFlagsJsonParser import com.onesignal.core.internal.config.ConfigModel +import com.onesignal.core.internal.config.ConfigModelChangeTags import com.onesignal.core.internal.config.ConfigModelStore import com.onesignal.core.internal.startup.IStartableService import com.onesignal.debug.internal.logging.Logging -import kotlin.coroutines.coroutineContext -import kotlin.jvm.Volatile import kotlinx.coroutines.CancellationException import kotlinx.coroutines.Job import kotlinx.coroutines.delay import kotlinx.coroutines.isActive +import kotlin.coroutines.coroutineContext /** * Fetches remote SDK feature flags when the app is in the foreground, immediately on focus and then @@ -37,7 +36,6 @@ internal class FeatureFlagsRefreshService( ) : IStartableService, IApplicationLifecycleHandler, ISingletonModelStoreChangeHandler { - @Volatile private var pollJob: Job? = null override fun start() { @@ -51,8 +49,10 @@ internal class FeatureFlagsRefreshService( } override fun onUnfocused() { - pollJob?.cancel() - pollJob = null + synchronized(this) { + pollJob?.cancel() + pollJob = null + } } override fun onModelUpdated( @@ -80,27 +80,28 @@ internal class FeatureFlagsRefreshService( } private fun restartForegroundPolling() { - pollJob?.cancel() - pollJob = - OneSignalDispatchers.launchOnIO { - while (coroutineContext.isActive) { - if (!applicationService.isInForeground) { - break - } - val appId = configModelStore.model.appId - if (appId.isNotEmpty()) { - try { - fetchAndApply(appId) - } catch (e: CancellationException) { - throw e - } catch (e: Exception) { - Logging.warn("FeatureFlagsRefreshService: fetch failed", e) + synchronized(this) { + pollJob?.cancel() + pollJob = + OneSignalDispatchers.launchOnIO { + while (coroutineContext.isActive) { + if (!applicationService.isInForeground) { + break + } + val appId = configModelStore.model.appId + if (appId.isNotEmpty()) { + try { + fetchAndApply(appId) + } catch (e: CancellationException) { + throw e + } catch (e: Exception) { + Logging.warn("FeatureFlagsRefreshService: fetch failed", e) + } } + delay(REFRESH_INTERVAL_MS) } - Logging.debug("FeatureFlagsRefreshService: next fetch in ${REFRESH_INTERVAL_MS}ms") - delay(REFRESH_INTERVAL_MS) } - } + } } private suspend fun fetchAndApply(appId: String) { @@ -113,10 +114,6 @@ internal class FeatureFlagsRefreshService( val newMetaString = FeatureFlagsJsonParser.encodeMetadata(result.metadata) val beforeKeys = current.sdkRemoteFeatureFlags.toSet() val afterKeys = result.enabledKeys.toSet() - Logging.debug( - "FeatureFlagsRefreshService: appId=$appId before=${beforeKeys.sorted()} " + - "after=${afterKeys.sorted()} changed=${beforeKeys != afterKeys}", - ) if (afterKeys == beforeKeys && newMetaString == current.sdkRemoteFeatureFlagMetadata) { return } diff --git a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/backend/impl/FeatureFlagsBackendServiceTest.kt b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/backend/impl/FeatureFlagsBackendServiceTest.kt deleted file mode 100644 index 705f6f16c..000000000 --- a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/backend/impl/FeatureFlagsBackendServiceTest.kt +++ /dev/null @@ -1,50 +0,0 @@ -package com.onesignal.core.internal.backend.impl - -import com.onesignal.common.OneSignalUtils -import io.kotest.core.spec.style.FunSpec -import io.kotest.matchers.shouldBe - -class FeatureFlagsBackendServiceTest : FunSpec({ - test("buildFeatureFlagsGetPath matches Turbine /apps/:app_id/sdk/features/:platform/:sdk_version") { - FeatureFlagsBackendService.buildFeatureFlagsGetPath( - appId = "14719551-23f1-4d20-8dab-81496ffca5ea", - platform = FeatureFlagsBackendService.TURBINE_FEATURES_PLATFORM_ANDROID, - sdkVersion = "050801", - ) shouldBe "apps/14719551-23f1-4d20-8dab-81496ffca5ea/sdk/features/android/050801" - } - - test("buildFeatureFlagsGetPath encodes prerelease sdk version label") { - FeatureFlagsBackendService.buildFeatureFlagsGetPath( - appId = "14719551-23f1-4d20-8dab-81496ffca5ea", - platform = FeatureFlagsBackendService.TURBINE_FEATURES_PLATFORM_ANDROID, - sdkVersion = "050801-beta", - ) shouldBe "apps/14719551-23f1-4d20-8dab-81496ffca5ea/sdk/features/android/050801-beta" - } - - test("isValidFeaturesSdkVersionLabel accepts only 6-digit labels with optional -suffix") { - val valid = - listOf( - "050801", - "050801-beta", - "050801-beta1", - "010203-rc.2", - "000000", - ) - val invalid = - listOf( - "5.8.1", - "05080", - "0508010", - "", - "050801-", - "050801/", - "v050801", - ) - valid.forEach { FeatureFlagsBackendService.isValidFeaturesSdkVersionLabel(it) shouldBe true } - invalid.forEach { FeatureFlagsBackendService.isValidFeaturesSdkVersionLabel(it) shouldBe false } - } - - test("OneSignalUtils.sdkVersion from BuildConfig matches Turbine label rules") { - FeatureFlagsBackendService.isValidFeaturesSdkVersionLabel(OneSignalUtils.sdkVersion) shouldBe true - } -}) diff --git a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/backend/impl/FeatureFlagsBackendServiceTests.kt b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/backend/impl/FeatureFlagsBackendServiceTests.kt index 26df536f6..c20b933b0 100644 --- a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/backend/impl/FeatureFlagsBackendServiceTests.kt +++ b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/backend/impl/FeatureFlagsBackendServiceTests.kt @@ -1,5 +1,6 @@ package com.onesignal.core.internal.backend.impl +import com.onesignal.common.OneSignalUtils import com.onesignal.core.internal.backend.RemoteFeatureFlagsFetchOutcome import com.onesignal.core.internal.http.HttpResponse import com.onesignal.core.internal.http.IHttpClient @@ -135,6 +136,59 @@ class FeatureFlagsBackendServiceTests : FunSpec({ outcome.shouldBeSuccessWithEmptyKeys() logsForService().any { it.level == LogLevel.WARN } shouldBe false } + + test("200 with empty body logs at WARN for contract anomaly and returns Unavailable") { + val http = mockk() + coEvery { http.get(any(), any()) } returns HttpResponse(200, null) + + val outcome = FeatureFlagsBackendService(http).fetchRemoteFeatureFlags("appId") + outcome shouldBe RemoteFeatureFlagsFetchOutcome.Unavailable + val warns = logsForService().filter { it.level == LogLevel.WARN } + warns.any { it.entry.contains("empty body for success status=200") } shouldBe true + } + + test("buildFeatureFlagsGetPath matches Turbine /apps/:app_id/sdk/features/:platform/:sdk_version") { + FeatureFlagsBackendService.buildFeatureFlagsGetPath( + appId = "14719551-23f1-4d20-8dab-81496ffca5ea", + platform = FeatureFlagsBackendService.TURBINE_FEATURES_PLATFORM_ANDROID, + sdkVersion = "050801", + ) shouldBe "apps/14719551-23f1-4d20-8dab-81496ffca5ea/sdk/features/android/050801" + } + + test("buildFeatureFlagsGetPath encodes prerelease sdk version label") { + FeatureFlagsBackendService.buildFeatureFlagsGetPath( + appId = "14719551-23f1-4d20-8dab-81496ffca5ea", + platform = FeatureFlagsBackendService.TURBINE_FEATURES_PLATFORM_ANDROID, + sdkVersion = "050801-beta", + ) shouldBe "apps/14719551-23f1-4d20-8dab-81496ffca5ea/sdk/features/android/050801-beta" + } + + test("isValidFeaturesSdkVersionLabel accepts only 6-digit labels with optional -suffix") { + val valid = + listOf( + "050801", + "050801-beta", + "050801-beta1", + "010203-rc.2", + "000000", + ) + val invalid = + listOf( + "5.8.1", + "05080", + "0508010", + "", + "050801-", + "050801/", + "v050801", + ) + valid.forEach { FeatureFlagsBackendService.isValidFeaturesSdkVersionLabel(it) shouldBe true } + invalid.forEach { FeatureFlagsBackendService.isValidFeaturesSdkVersionLabel(it) shouldBe false } + } + + test("OneSignalUtils.sdkVersion from BuildConfig matches Turbine label rules") { + FeatureFlagsBackendService.isValidFeaturesSdkVersionLabel(OneSignalUtils.sdkVersion) shouldBe true + } }) private fun RemoteFeatureFlagsFetchOutcome.shouldBeSuccessWithEmptyKeys() { diff --git a/examples/demo/build.gradle.kts b/examples/demo/build.gradle.kts index b0dc0f2a9..159ff201c 100644 --- a/examples/demo/build.gradle.kts +++ b/examples/demo/build.gradle.kts @@ -19,8 +19,6 @@ buildscript { allprojects { repositories { - // mavenLocal first so locally-published SDK builds resolve over Central when testing against a snapshot. - mavenLocal() google() mavenCentral() // Huawei maven From 0a3d18ebc6da138b5d7c37cbec365abe454436d5 Mon Sep 17 00:00:00 2001 From: AR Abdul Azeez Date: Thu, 23 Apr 2026 16:58:40 -0400 Subject: [PATCH 09/13] removing features node in ConfigModel --- .../internal/backend/IParamsBackendService.kt | 1 - .../backend/impl/ParamsBackendService.kt | 14 ------- .../core/internal/config/ConfigModel.kt | 17 ++------- .../config/impl/ConfigModelStoreListener.kt | 1 - .../core/internal/features/FeatureManager.kt | 6 +-- .../internal/features/FeatureManagerTests.kt | 37 ++++--------------- 6 files changed, 12 insertions(+), 64 deletions(-) diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/backend/IParamsBackendService.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/backend/IParamsBackendService.kt index 54f3cd85e..8773a23af 100644 --- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/backend/IParamsBackendService.kt +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/backend/IParamsBackendService.kt @@ -35,7 +35,6 @@ internal class ParamsObject( var locationShared: Boolean? = null, var requiresUserPrivacyConsent: Boolean? = null, var opRepoExecutionInterval: Long? = null, - val features: List = emptyList(), var influenceParams: InfluenceParamsObject, var fcmParams: FCMParamsObject, val remoteLoggingParams: RemoteLoggingParamsObject, diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/backend/impl/ParamsBackendService.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/backend/impl/ParamsBackendService.kt index ec0af8605..dfaaa027d 100644 --- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/backend/impl/ParamsBackendService.kt +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/backend/impl/ParamsBackendService.kt @@ -68,19 +68,6 @@ internal class ParamsBackendService( ) } - val features = - responseJson.optJSONArray("features") - ?.let { featuresJson -> - buildList { - for (i in 0 until featuresJson.length()) { - val featureName = featuresJson.optString(i, "") - if (featureName.isNotBlank()) { - add(featureName) - } - } - } - } ?: emptyList() - return ParamsObject( googleProjectNumber = responseJson.safeString("android_sender_id"), enterprise = responseJson.safeBool("enterp"), @@ -97,7 +84,6 @@ internal class ParamsBackendService( requiresUserPrivacyConsent = responseJson.safeBool("requires_user_privacy_consent"), // TODO: New opRepoExecutionInterval = responseJson.safeLong("oprepo_execution_interval"), - features = features, influenceParams = influenceParams ?: InfluenceParamsObject(), fcmParams = fcmParams ?: FCMParamsObject(), remoteLoggingParams = remoteLoggingParams ?: RemoteLoggingParamsObject(), diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/config/ConfigModel.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/config/ConfigModel.kt index bfaf08eb8..616fa1916 100644 --- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/config/ConfigModel.kt +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/config/ConfigModel.kt @@ -290,22 +290,11 @@ class ConfigModel : Model() { setBooleanProperty(::clearGroupOnSummaryClick.name, value) } - /** - * Feature ids from the main OneSignal config / params response (legacy channel). - * Presence of a name means the feature is on. This list is distinct from [sdkRemoteFeatureFlags]; - * [com.onesignal.core.internal.features.FeatureManager] unions both (plus any local test overrides). - */ - var features: List - get() = getListProperty(::features.name) { emptyList() } - set(value) { - setListProperty(::features.name, value) - } - /** * Feature ids from the Turbine SDK feature-flags HTTP endpoint * ([com.onesignal.core.internal.backend.IFeatureFlagsBackendService]), updated while the app is - * foreground. Unioned with [features] in [com.onesignal.core.internal.features.FeatureManager] - * when evaluating [com.onesignal.core.internal.features.FeatureFlag] entries. + * foreground. [com.onesignal.core.internal.features.FeatureManager] unions these with any local + * test overrides when evaluating [com.onesignal.core.internal.features.FeatureFlag] entries. */ var sdkRemoteFeatureFlags: List get() = getListProperty(::sdkRemoteFeatureFlags.name) { emptyList() } @@ -369,7 +358,7 @@ class ConfigModel : Model() { property: String, jsonArray: JSONArray, ): List<*>? { - if (property == ::features.name || property == ::sdkRemoteFeatureFlags.name) { + if (property == ::sdkRemoteFeatureFlags.name) { return buildList { for (i in 0 until jsonArray.length()) { val featureName = jsonArray.optString(i, "") diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/config/impl/ConfigModelStoreListener.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/config/impl/ConfigModelStoreListener.kt index 9e5c4fc32..5dcfe83b7 100644 --- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/config/impl/ConfigModelStoreListener.kt +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/config/impl/ConfigModelStoreListener.kt @@ -95,7 +95,6 @@ internal class ConfigModelStoreListener( params.locationShared?.let { config.locationShared = it } params.requiresUserPrivacyConsent?.let { config.consentRequired = it } params.opRepoExecutionInterval?.let { config.opRepoExecutionInterval = it } - config.features = params.features params.influenceParams.notificationLimit?.let { config.influenceParams.notificationLimit = it } params.influenceParams.indirectNotificationAttributionWindow?.let { config.influenceParams.indirectNotificationAttributionWindow = it } params.influenceParams.iamLimit?.let { config.influenceParams.iamLimit = it } diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/features/FeatureManager.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/features/FeatureManager.kt index 6e2dae7f1..84227f06d 100644 --- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/features/FeatureManager.kt +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/features/FeatureManager.kt @@ -69,8 +69,7 @@ internal class FeatureManager( args: ModelChangedArgs, tag: String, ) { - if (args.property == ConfigModel::features.name || - args.property == ConfigModel::sdkRemoteFeatureFlags.name || + if (args.property == ConfigModel::sdkRemoteFeatureFlags.name || args.property == ConfigModel::sdkRemoteFeatureFlagMetadata.name ) { Logging.debug("OneSignal: FeatureManager.onModelUpdated(property=${args.property}, tag=$tag)") @@ -89,8 +88,7 @@ internal class FeatureManager( ) { val enabledFeatureKeys = ( - model.features.map { canonicalizeFeatureKey(it) } + - model.sdkRemoteFeatureFlags.map { canonicalizeFeatureKey(it) } + + model.sdkRemoteFeatureFlags.map { canonicalizeFeatureKey(it) } + localFeatureOverrides.map { canonicalizeFeatureKey(it) } ).toSet() if (localFeatureOverrides.isNotEmpty()) { diff --git a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/features/FeatureManagerTests.kt b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/features/FeatureManagerTests.kt index fc3aa6949..f2e2e6f07 100644 --- a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/features/FeatureManagerTests.kt +++ b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/features/FeatureManagerTests.kt @@ -22,27 +22,9 @@ class FeatureManagerTests : FunSpec({ every { model.sdkRemoteFeatureFlagMetadata } returns null } - test("initial state should enable BACKGROUND_THREADING when feature is present") { - // Given - val initialModel = mockk() - stubConfigModel(initialModel) - every { initialModel.features } returns listOf(FeatureFlag.SDK_BACKGROUND_THREADING.key) - val configModelStore = mockk() - every { configModelStore.model } returns initialModel - every { configModelStore.subscribe(any()) } just runs - - // When - val manager = FeatureManager(configModelStore) - - // Then - manager.isEnabled(FeatureFlag.SDK_BACKGROUND_THREADING) shouldBe true - ThreadingMode.useBackgroundThreading shouldBe true - } - - test("initial state enables BACKGROUND_THREADING when key only exists on sdk remote flags") { + test("initial state enables BACKGROUND_THREADING when key is present in sdk remote flags") { val initialModel = mockk() stubConfigModel(initialModel) - every { initialModel.features } returns emptyList() every { initialModel.sdkRemoteFeatureFlags } returns listOf(FeatureFlag.SDK_BACKGROUND_THREADING.key) val configModelStore = mockk() every { configModelStore.model } returns initialModel @@ -57,7 +39,6 @@ class FeatureManagerTests : FunSpec({ test("initial state enables BACKGROUND_THREADING when remote key differs only by letter case") { val initialModel = mockk() stubConfigModel(initialModel) - every { initialModel.features } returns emptyList() every { initialModel.sdkRemoteFeatureFlags } returns listOf("SDK_Background_Threading") val configModelStore = mockk() every { configModelStore.model } returns initialModel @@ -73,7 +54,6 @@ class FeatureManagerTests : FunSpec({ // Given val initialModel = mockk() stubConfigModel(initialModel) - every { initialModel.features } returns emptyList() val configModelStore = mockk() every { configModelStore.model } returns initialModel every { configModelStore.subscribe(any()) } just runs @@ -81,7 +61,7 @@ class FeatureManagerTests : FunSpec({ val updatedModel = mockk() stubConfigModel(updatedModel) - every { updatedModel.features } returns listOf(FeatureFlag.SDK_BACKGROUND_THREADING.key) + every { updatedModel.sdkRemoteFeatureFlags } returns listOf(FeatureFlag.SDK_BACKGROUND_THREADING.key) // When manager.onModelReplaced(updatedModel, ModelChangeTags.HYDRATE) @@ -95,18 +75,17 @@ class FeatureManagerTests : FunSpec({ // Given val model = mockk() stubConfigModel(model) - every { model.features } returns emptyList() val configModelStore = mockk() every { configModelStore.model } returns model every { configModelStore.subscribe(any()) } just runs val manager = FeatureManager(configModelStore) - every { model.features } returns listOf(FeatureFlag.SDK_BACKGROUND_THREADING.key) + every { model.sdkRemoteFeatureFlags } returns listOf(FeatureFlag.SDK_BACKGROUND_THREADING.key) // When manager.onModelUpdated( args = mockk { - every { property } returns ConfigModel::features.name + every { property } returns ConfigModel::sdkRemoteFeatureFlags.name }, tag = ModelChangeTags.NORMAL ) @@ -120,18 +99,18 @@ class FeatureManagerTests : FunSpec({ // Given val model = mockk() stubConfigModel(model) - every { model.features } returns listOf(FeatureFlag.SDK_BACKGROUND_THREADING.key) + every { model.sdkRemoteFeatureFlags } returns listOf(FeatureFlag.SDK_BACKGROUND_THREADING.key) val configModelStore = mockk() every { configModelStore.model } returns model every { configModelStore.subscribe(any()) } just runs val manager = FeatureManager(configModelStore) - every { model.features } returns emptyList() + every { model.sdkRemoteFeatureFlags } returns emptyList() // When manager.onModelUpdated( args = mockk { - every { property } returns ConfigModel::features.name + every { property } returns ConfigModel::sdkRemoteFeatureFlags.name }, tag = ModelChangeTags.NORMAL ) @@ -144,7 +123,6 @@ class FeatureManagerTests : FunSpec({ test("remoteFeatureFlagMetadata returns parsed JSON from config") { val initialModel = mockk() stubConfigModel(initialModel) - every { initialModel.features } returns emptyList() every { initialModel.sdkRemoteFeatureFlagMetadata } returns """{"X":{"note":"y"}}""" val configModelStore = mockk() every { configModelStore.model } returns initialModel @@ -159,7 +137,6 @@ class FeatureManagerTests : FunSpec({ test("remoteFeatureFlagMetadata is null when config has no stored metadata") { val initialModel = mockk() stubConfigModel(initialModel) - every { initialModel.features } returns emptyList() every { initialModel.sdkRemoteFeatureFlagMetadata } returns null val configModelStore = mockk() every { configModelStore.model } returns initialModel From 68f2df3a07cf5eac12ce5bd7335f0e919a3f250f Mon Sep 17 00:00:00 2001 From: Nan Date: Fri, 24 Apr 2026 11:37:18 -0700 Subject: [PATCH 10/13] nit(style): fix spotless formatting and core detekt issues --- .../core/internal/backend/impl/FeatureFlagsJsonParser.kt | 3 +++ .../com/onesignal/core/internal/features/FeatureManager.kt | 2 +- .../onesignal/core/internal/features/FeatureManagerTests.kt | 2 +- 3 files changed, 5 insertions(+), 2 deletions(-) diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/backend/impl/FeatureFlagsJsonParser.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/backend/impl/FeatureFlagsJsonParser.kt index 560bd04a3..963f327a5 100644 --- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/backend/impl/FeatureFlagsJsonParser.kt +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/backend/impl/FeatureFlagsJsonParser.kt @@ -59,6 +59,7 @@ internal object FeatureFlagsJsonParser { } } + @Suppress("ReturnCount") private fun parseRootStrict(root: JsonObject): RemoteFeatureFlagsResult? { val featuresEl = root[FEATURES_PROPERTY] ?: return null val featuresArray = featuresEl as? JsonArray ?: return null @@ -88,6 +89,7 @@ internal object FeatureFlagsJsonParser { return RemoteFeatureFlagsResult(keys, metaOut) } + @Suppress("ReturnCount") private fun findSiblingJsonObject( root: JsonObject, rawKeyFromFeaturesArray: String, @@ -127,6 +129,7 @@ internal object FeatureFlagsJsonParser { * Decodes [ConfigModel.sdkRemoteFeatureFlagMetadata] (a JSON object of flag id → object) into a map. * Non-object values are skipped so each entry stays a [JsonObject] for nested decoding (e.g. with `Json.decodeFromJsonElement`). */ + @Suppress("ReturnCount") fun parseStoredMetadataMap(raw: String?): Map { if (raw.isNullOrBlank()) { return emptyMap() diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/features/FeatureManager.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/features/FeatureManager.kt index 84227f06d..738186db2 100644 --- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/features/FeatureManager.kt +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/features/FeatureManager.kt @@ -90,7 +90,7 @@ internal class FeatureManager( ( model.sdkRemoteFeatureFlags.map { canonicalizeFeatureKey(it) } + localFeatureOverrides.map { canonicalizeFeatureKey(it) } - ).toSet() + ).toSet() if (localFeatureOverrides.isNotEmpty()) { Logging.warn( "OneSignal: Local feature override enabled for testing only: $localFeatureOverrides", diff --git a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/features/FeatureManagerTests.kt b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/features/FeatureManagerTests.kt index f2e2e6f07..128a50b63 100644 --- a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/features/FeatureManagerTests.kt +++ b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/features/FeatureManagerTests.kt @@ -7,10 +7,10 @@ import com.onesignal.core.internal.config.ConfigModelStore import io.kotest.core.spec.style.FunSpec import io.kotest.matchers.shouldBe import io.mockk.every -import kotlinx.serialization.json.jsonPrimitive import io.mockk.just import io.mockk.mockk import io.mockk.runs +import kotlinx.serialization.json.jsonPrimitive class FeatureManagerTests : FunSpec({ beforeEach { From 28611447af3932316ad18bfe23a45e7a7e9f0808 Mon Sep 17 00:00:00 2001 From: AR Abdul Azeez Date: Mon, 27 Apr 2026 11:38:24 -0400 Subject: [PATCH 11/13] magic numbers --- .../impl/TurbineSdkFeatureFlagsPath.kt | 28 +++++++++++-------- 1 file changed, 17 insertions(+), 11 deletions(-) diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/backend/impl/TurbineSdkFeatureFlagsPath.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/backend/impl/TurbineSdkFeatureFlagsPath.kt index 89f0e9158..c533f8e50 100644 --- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/backend/impl/TurbineSdkFeatureFlagsPath.kt +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/backend/impl/TurbineSdkFeatureFlagsPath.kt @@ -34,28 +34,34 @@ internal object TurbineSdkFeatureFlagsPath { */ internal fun percentEncodePathSegmentUtf8(segment: String): String { val bytes = segment.encodeToByteArray() - return buildString(bytes.size * 3) { + return buildString(bytes.size * PCT_ENCODED_MAX_OUTPUT_CHARS_PER_INPUT_BYTE) { for (b in bytes) { - val u = b.toInt() and 0xff + val u = b.toInt() and BYTE_MASK if (isUnreservedByte(u)) { append(u.toChar()) } else { append('%') - append(HEX_DIGITS[u shr 4]) - append(HEX_DIGITS[u and 15]) + append(HEX_DIGITS[u shr HEX_NYBBLE_SHIFT]) + append(HEX_DIGITS[u and HEX_NYBBLE_MASK]) } } } } + /** Per RFC 3986 section 2.3 unreserved (ALPHA / DIGIT / "-" / "." / "_" / "~"). */ private fun isUnreservedByte(u: Int): Boolean = - u in 0x41..0x5A || - u in 0x61..0x7A || - u in 0x30..0x39 || - u == 0x2D || - u == 0x2E || - u == 0x5F || - u == 0x7E + u in 'A'.code..'Z'.code || + u in 'a'.code..'z'.code || + u in '0'.code..'9'.code || + u == '-'.code || + u == '.'.code || + u == '_'.code || + u == '~'.code + + private const val BYTE_MASK: Int = 0xFF + private const val PCT_ENCODED_MAX_OUTPUT_CHARS_PER_INPUT_BYTE: Int = 3 + private const val HEX_NYBBLE_MASK: Int = 0b1111 + private const val HEX_NYBBLE_SHIFT: Int = 4 private const val HEX_DIGITS = "0123456789ABCDEF" } From 87868520c4ee4c8c4d0d1bd1f3dc5cdbc85adfff Mon Sep 17 00:00:00 2001 From: AR Abdul Azeez Date: Mon, 27 Apr 2026 14:07:09 -0400 Subject: [PATCH 12/13] fix(core): preserve feature flag cache against snapshot race and contract-violating responses ConfigModelStoreListener.fetchParams now re-sources sdkRemoteFeatureFlags{,Metadata} from the live model immediately before replace(), so concurrent in-place writes from FeatureFlagsRefreshService survive Model.initializeFromModel's data.clear()+putAll(). FeatureFlagsJsonParser.parseRootStrict now distinguishes an authoritative empty features array from one that filtered down to empty due to element-type violations; the latter returns null -> Unavailable so callers preserve the cached list rather than overwriting it with []. Adds regression tests for both paths. Addresses PR #2612 review comments. Made-with: Cursor --- .../backend/impl/FeatureFlagsJsonParser.kt | 5 +- .../config/impl/ConfigModelStoreListener.kt | 5 ++ .../impl/FeatureFlagsJsonParserTests.kt | 19 +++++ .../impl/ConfigModelStoreListenerTests.kt | 81 +++++++++++++++++++ 4 files changed, 109 insertions(+), 1 deletion(-) create mode 100644 OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/config/impl/ConfigModelStoreListenerTests.kt diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/backend/impl/FeatureFlagsJsonParser.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/backend/impl/FeatureFlagsJsonParser.kt index 963f327a5..9a6a85231 100644 --- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/backend/impl/FeatureFlagsJsonParser.kt +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/backend/impl/FeatureFlagsJsonParser.kt @@ -74,7 +74,10 @@ internal object FeatureFlagsJsonParser { }.distinctBy { it.second } if (flagEntries.isEmpty()) { - return RemoteFeatureFlagsResult(emptyList(), null) + // `[]` is an authoritative empty config; a non-empty array that filtered down + // to empty is a contract violation. Null surfaces as Unavailable upstream so + // callers preserve the cached list instead of overwriting it with []. + return if (featuresArray.isEmpty()) RemoteFeatureFlagsResult(emptyList(), null) else null } val keys = flagEntries.map { it.second } diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/config/impl/ConfigModelStoreListener.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/config/impl/ConfigModelStoreListener.kt index 5dcfe83b7..e15b6faa0 100644 --- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/config/impl/ConfigModelStoreListener.kt +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/config/impl/ConfigModelStoreListener.kt @@ -106,6 +106,11 @@ internal class ConfigModelStoreListener( params.remoteLoggingParams.logLevel?.let { config.remoteLoggingParams.logLevel = it } config.remoteLoggingParams.isEnabled = params.remoteLoggingParams.isEnabled + // Re-source from live model: replace() is a full data.clear()+putAll(), + // and these fields are written in-place by FeatureFlagsRefreshService. + config.sdkRemoteFeatureFlags = _configModelStore.model.sdkRemoteFeatureFlags + config.sdkRemoteFeatureFlagMetadata = _configModelStore.model.sdkRemoteFeatureFlagMetadata + _configModelStore.replace(config, ModelChangeTags.HYDRATE) success = true } catch (ex: BackendException) { diff --git a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/backend/impl/FeatureFlagsJsonParserTests.kt b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/backend/impl/FeatureFlagsJsonParserTests.kt index ba84d9e2d..43983fce8 100644 --- a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/backend/impl/FeatureFlagsJsonParserTests.kt +++ b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/backend/impl/FeatureFlagsJsonParserTests.kt @@ -86,6 +86,25 @@ class FeatureFlagsJsonParserTests : FunSpec({ FeatureFlagsJsonParser.parseSuccessful("""{"features":"bad"}""") shouldBe null } + test("parseSuccessful returns null when features array is non-empty but every element is contract-violating") { + // Element-type violations on a non-empty array must be Unavailable, not Success(empty); + // otherwise callers would overwrite the cached list with []. + FeatureFlagsJsonParser.parseSuccessful("""{"features":[1,2,3]}""") shouldBe null + FeatureFlagsJsonParser.parseSuccessful("""{"features":[null]}""") shouldBe null + FeatureFlagsJsonParser.parseSuccessful("""{"features":[{}]}""") shouldBe null + FeatureFlagsJsonParser.parseSuccessful("""{"features":[[]]}""") shouldBe null + FeatureFlagsJsonParser.parseSuccessful("""{"features":[true,false]}""") shouldBe null + FeatureFlagsJsonParser.parseSuccessful("""{"features":[""]}""") shouldBe null + FeatureFlagsJsonParser.parseSuccessful("""{"features":[" "]}""") shouldBe null + } + + test("parseSuccessful keeps mixed arrays (drops invalid elements, keeps valid ones)") { + // Tolerance contract: any valid string id keeps the result; invalid siblings are dropped. + val r = requireNotNull(FeatureFlagsJsonParser.parseSuccessful("""{"features":["good", 1, null, {}, ""]}""")) + r.enabledKeys shouldBe listOf("good") + r.metadata shouldBe null + } + test("parseSuccessful returns null for Turbine error body (e.g. 403 Forbidden)") { FeatureFlagsJsonParser.parseSuccessful("""{"errors":["Forbidden"]}""") shouldBe null FeatureFlagsJsonParser.parseSuccessful("""{"errors":["Not Found"]}""") shouldBe null diff --git a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/config/impl/ConfigModelStoreListenerTests.kt b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/config/impl/ConfigModelStoreListenerTests.kt new file mode 100644 index 000000000..c1ac8b6e6 --- /dev/null +++ b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/config/impl/ConfigModelStoreListenerTests.kt @@ -0,0 +1,81 @@ +package com.onesignal.core.internal.config.impl + +import com.onesignal.core.internal.backend.IParamsBackendService +import com.onesignal.core.internal.backend.ParamsObject +import com.onesignal.core.internal.config.ConfigModelStore +import com.onesignal.mocks.IOMockHelper +import com.onesignal.mocks.IOMockHelper.awaitIO +import com.onesignal.mocks.MockPreferencesService +import com.onesignal.user.internal.subscriptions.ISubscriptionManager +import com.onesignal.user.internal.subscriptions.SubscriptionList +import com.onesignal.user.subscriptions.IPushSubscription +import io.kotest.core.spec.style.FunSpec +import io.kotest.matchers.shouldBe +import io.mockk.coEvery +import io.mockk.every +import io.mockk.mockk + +/** + * Regression coverage for the snapshot/replace race in [ConfigModelStoreListener.fetchParams]: + * concurrent in-place writes to sdkRemoteFeatureFlags{,Metadata} from + * [FeatureFlagsRefreshService] must survive replace(). + */ +class ConfigModelStoreListenerTests : FunSpec({ + listener(IOMockHelper) + + test("fetchParams does not clobber sdkRemoteFeatureFlags written between snapshot and replace") { + // Real store so replace() goes through Model.initializeFromModel (data.clear()+putAll()). + val store = ConfigModelStore(MockPreferencesService()) + store.model.appId = "test-app-id" + store.model.sdkRemoteFeatureFlags = emptyList() + store.model.sdkRemoteFeatureFlagMetadata = null + + // Race trigger: locationShared is read between snapshot and replace, so its getter + // side-effect lands inside the window. Returning null short-circuits the let { } + // body; only the side-effect matters. + val params = mockk(relaxed = true) + every { params.locationShared } answers { + store.model.sdkRemoteFeatureFlags = listOf("sdk_background_threading") + store.model.sdkRemoteFeatureFlagMetadata = """{"sdk_background_threading":{"x":1}}""" + null + } + + val paramsBackend = mockk() + coEvery { paramsBackend.fetchParams(any(), any()) } returns params + + val pushSub = mockk(relaxed = true) + val subscriptionManager = mockk() + every { subscriptionManager.subscriptions } returns SubscriptionList(emptyList(), pushSub) + + val listener = ConfigModelStoreListener(store, paramsBackend, subscriptionManager) + listener.start() + awaitIO() + + // Without the fix, the stale snapshot wins and these are [] / null. + store.model.sdkRemoteFeatureFlags shouldBe listOf("sdk_background_threading") + store.model.sdkRemoteFeatureFlagMetadata shouldBe """{"sdk_background_threading":{"x":1}}""" + } + + test("fetchParams preserves sdkRemoteFeatureFlags already on the live model when no in-flight write occurs") { + // Smoke: no race -> existing values round-trip through snapshot + replace. + val store = ConfigModelStore(MockPreferencesService()) + store.model.appId = "test-app-id" + store.model.sdkRemoteFeatureFlags = listOf("sdk_background_threading") + store.model.sdkRemoteFeatureFlagMetadata = """{"sdk_background_threading":{"x":1}}""" + + val params = mockk(relaxed = true) + val paramsBackend = mockk() + coEvery { paramsBackend.fetchParams(any(), any()) } returns params + + val pushSub = mockk(relaxed = true) + val subscriptionManager = mockk() + every { subscriptionManager.subscriptions } returns SubscriptionList(emptyList(), pushSub) + + val listener = ConfigModelStoreListener(store, paramsBackend, subscriptionManager) + listener.start() + awaitIO() + + store.model.sdkRemoteFeatureFlags shouldBe listOf("sdk_background_threading") + store.model.sdkRemoteFeatureFlagMetadata shouldBe """{"sdk_background_threading":{"x":1}}""" + } +}) From 98e436dbb11f8afc83746ff761992c2f353a029f Mon Sep 17 00:00:00 2001 From: AR Abdul Azeez Date: Mon, 27 Apr 2026 15:32:46 -0400 Subject: [PATCH 13/13] fix(core): dedup remote feature-flags fetch on same-appId hydrate `ConfigModelStoreListener.fetchParams` replaces the live `ConfigModel` with the HYDRATE tag during init, which fired `onModelReplaced` on `FeatureFlagsRefreshService` and triggered a second Turbine GET right after the focus-driven first one, even though the appId hadn't changed. Track the appId currently being polled and treat a re-trigger for the same appId as a no-op; genuine appId changes still cancel and restart. `onUnfocused` clears the cached appId so the next focus event re-arms polling. Pull the foreground cadence off the constructor and onto an `internal var refreshIntervalMs` (default 8 min, test-only override before `start`). The IoC reflection in `ServiceRegistrationReflection.resolve` cannot resolve a non-service `Long` parameter and was throwing "Could not instantiate service: ServiceRegistrationReflection@..." out of `StartupService.scheduleStart` once the service was registered. Made-with: Cursor --- .../config/impl/FeatureFlagsRefreshService.kt | 50 ++++- .../impl/FeatureFlagsRefreshServiceTests.kt | 190 ++++++++++++++++++ 2 files changed, 234 insertions(+), 6 deletions(-) create mode 100644 OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/config/impl/FeatureFlagsRefreshServiceTests.kt diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/config/impl/FeatureFlagsRefreshService.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/config/impl/FeatureFlagsRefreshService.kt index 7ff63499a..40407f821 100644 --- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/config/impl/FeatureFlagsRefreshService.kt +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/config/impl/FeatureFlagsRefreshService.kt @@ -22,12 +22,23 @@ import kotlin.coroutines.coroutineContext /** * Fetches remote SDK feature flags when the app is in the foreground, immediately on focus and then - * every [REFRESH_INTERVAL_MS] while the session stays in the foreground. Updates + * every [refreshIntervalMs] while the session stays in the foreground. Updates * [ConfigModel.sdkRemoteFeatureFlags] / [ConfigModel.sdkRemoteFeatureFlagMetadata] so * [com.onesignal.core.internal.features.FeatureManager] stays in sync. * * Remote fields are updated in place on the live [ConfigModel] (with [ConfigModelChangeTags.REMOTE_FEATURE_FLAGS]) * so concurrent hydration cannot be overwritten by a stale full-model snapshot. + * + * Polling is keyed on the active [ConfigModel.appId]: once a poll loop is running for a given + * appId, redundant triggers (e.g. [com.onesignal.common.modeling.ModelChangeTags.HYDRATE] replaces + * from [ConfigModelStoreListener.fetchParams] that write the same appId back) are a no-op so we + * don't double-fire the Turbine GET at startup. Genuine appId changes still cancel and restart. + * + * IMPORTANT: Constructor parameters must remain limited to types the IoC reflection can resolve + * via [com.onesignal.common.services.IServiceProvider] (registered services or `List`). + * Configuration knobs like [refreshIntervalMs] live as `internal var` instead so reflection in + * [com.onesignal.common.services.ServiceRegistrationReflection.resolve] still picks the only + * constructor and tests can still override the value before [start]. */ internal class FeatureFlagsRefreshService( private val applicationService: IApplicationService, @@ -36,8 +47,22 @@ internal class FeatureFlagsRefreshService( ) : IStartableService, IApplicationLifecycleHandler, ISingletonModelStoreChangeHandler { + /** + * Foreground polling cadence; defaults to [DEFAULT_REFRESH_INTERVAL_MS] (8 minutes). Test-only + * override (must be set before [start] / focus) – keep out of the constructor so the IoC's + * reflection-based resolver doesn't trip on a non-service `Long` parameter (see class KDoc). + */ + internal var refreshIntervalMs: Long = DEFAULT_REFRESH_INTERVAL_MS + private var pollJob: Job? = null + /** + * AppId currently being polled. Used to dedup redundant `restartForegroundPolling` triggers + * (e.g. HYDRATE replace from [ConfigModelStoreListener.fetchParams] that doesn't change the + * appId). Cleared in [onUnfocused] so the next focus event always re-arms polling. + */ + private var pollingAppId: String? = null + override fun start() { applicationService.addApplicationLifecycleHandler(this) configModelStore.subscribe(this) @@ -52,6 +77,7 @@ internal class FeatureFlagsRefreshService( synchronized(this) { pollJob?.cancel() pollJob = null + pollingAppId = null } } @@ -81,24 +107,36 @@ internal class FeatureFlagsRefreshService( private fun restartForegroundPolling() { synchronized(this) { + val appId = configModelStore.model.appId + if (appId.isEmpty()) { + pollJob?.cancel() + pollJob = null + pollingAppId = null + return + } + // Dedup: an active loop for the same appId is already covering us. + if (pollingAppId == appId) { + return + } pollJob?.cancel() + pollingAppId = appId pollJob = OneSignalDispatchers.launchOnIO { while (coroutineContext.isActive) { if (!applicationService.isInForeground) { break } - val appId = configModelStore.model.appId - if (appId.isNotEmpty()) { + val current = configModelStore.model.appId + if (current.isNotEmpty()) { try { - fetchAndApply(appId) + fetchAndApply(current) } catch (e: CancellationException) { throw e } catch (e: Exception) { Logging.warn("FeatureFlagsRefreshService: fetch failed", e) } } - delay(REFRESH_INTERVAL_MS) + delay(refreshIntervalMs) } } } @@ -125,6 +163,6 @@ internal class FeatureFlagsRefreshService( companion object { // Foreground polling cadence for remote feature flags (8 minutes). - private const val REFRESH_INTERVAL_MS = 480_000L + private const val DEFAULT_REFRESH_INTERVAL_MS = 480_000L } } diff --git a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/config/impl/FeatureFlagsRefreshServiceTests.kt b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/config/impl/FeatureFlagsRefreshServiceTests.kt new file mode 100644 index 000000000..5ceb40e77 --- /dev/null +++ b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/config/impl/FeatureFlagsRefreshServiceTests.kt @@ -0,0 +1,190 @@ +package com.onesignal.core.internal.config.impl + +import com.onesignal.common.modeling.ModelChangeTags +import com.onesignal.core.internal.application.IApplicationLifecycleHandler +import com.onesignal.core.internal.application.IApplicationService +import com.onesignal.core.internal.backend.IFeatureFlagsBackendService +import com.onesignal.core.internal.backend.RemoteFeatureFlagsFetchOutcome +import com.onesignal.core.internal.config.ConfigModel +import com.onesignal.core.internal.config.ConfigModelStore +import com.onesignal.mocks.IOMockHelper +import com.onesignal.mocks.IOMockHelper.awaitIO +import io.kotest.core.spec.style.FunSpec +import io.kotest.matchers.shouldBe +import io.mockk.Runs +import io.mockk.coEvery +import io.mockk.coVerify +import io.mockk.every +import io.mockk.just +import io.mockk.mockk +import io.mockk.slot + +/** + * Regression coverage for the duplicate Turbine feature-flags fetch at SDK startup. + * + * Two startables fire at init: + * 1. [FeatureFlagsRefreshService.start] -> [IApplicationService.addApplicationLifecycleHandler] + * synchronously delivers `onFocus(firedOnSubscribe = true)` while the app is foregrounded, + * which kicks off polling and issues fetch #1. + * 2. [ConfigModelStoreListener.fetchParams] (Android params) finishes and calls + * `_configModelStore.replace(config, ModelChangeTags.HYDRATE)`, which fires `onModelReplaced` + * on every subscriber. The replaced model carries the **same** `appId`, but pre-fix the + * listener still triggered a second poll, producing fetch #2 a few hundred ms after #1. + * + * The fix tracks the appId currently being polled and treats a re-trigger for the same appId as + * a no-op. Tests below assert both directions: same-appId hydrates are deduped, but a genuine + * appId change still cancels and re-fetches. + */ +class FeatureFlagsRefreshServiceTests : FunSpec({ + listener(IOMockHelper) + + fun mockBackend(): Pair Int> { + val backend = mockk() + var count = 0 + coEvery { backend.fetchRemoteFeatureFlags(any()) } answers { + count++ + RemoteFeatureFlagsFetchOutcome.Unavailable + } + return backend to { count } + } + + /** + * Builds an [IApplicationService] mock that mirrors production behavior at init: + * - `addApplicationLifecycleHandler` synchronously fires `onFocus(true)` (matches + * [com.onesignal.core.internal.application.impl.ApplicationService.addApplicationLifecycleHandler] + * when `current != null`). + * - `isInForeground` returns the supplied [foregroundSequence] in order; tests pick exactly + * the right pattern of `true`/`false` so that each polling loop runs one fetch then exits + * on the next iteration. Coupled with `refreshIntervalMs = 0L` this keeps tests + * deterministic without needing a virtual time scheduler. + * + * Per-trigger budget for [foregroundSequence]: + * - Initial `start()` -> `onFocus(true)` -> polling loop: **2** values (`true`, `false`). + * - `onModelReplaced` / `onModelUpdated` event handler condition: **1** value. + * - Each polling loop kicked off by an event handler when not deduped: **2** values. + */ + fun foregroundedAppService(vararg foregroundSequence: Boolean): IApplicationService { + val app = mockk() + val handlerSlot = slot() + every { app.addApplicationLifecycleHandler(capture(handlerSlot)) } answers { + handlerSlot.captured.onFocus(true) + } + every { app.removeApplicationLifecycleHandler(any()) } just Runs + every { app.isInForeground } returnsMany foregroundSequence.toList() + return app + } + + fun mockConfigStore(model: ConfigModel): ConfigModelStore { + val store = mockk() + every { store.model } returns model + every { store.subscribe(any()) } just Runs + every { store.unsubscribe(any()) } just Runs + return store + } + + test("HYDRATE replace with the same appId does not trigger a duplicate fetch") { + val model = ConfigModel().apply { appId = "appId-1" } + val store = mockConfigStore(model) + val (backend, fetchCount) = mockBackend() + // start: [true, false] (loop iter1=true, iter2=false break) + onModelReplaced check: [true] + val app = foregroundedAppService(true, false, true) + + val service = FeatureFlagsRefreshService(app, store, backend).apply { refreshIntervalMs = 0L } + + // Mirrors `OneSignalImp.internalInit` -> `startupService.scheduleStart()`: synchronously + // attaches the lifecycle handler, which fires `onFocus(true)` -> initial fetch. + service.start() + awaitIO() + fetchCount() shouldBe 1 + + // Mirrors `ConfigModelStoreListener.fetchParams` -> `replace(config, HYDRATE)` with the + // same appId already on the live model. Pre-fix this fired a second Turbine GET. + service.onModelReplaced(model, ModelChangeTags.HYDRATE) + awaitIO() + fetchCount() shouldBe 1 + } + + test("NORMAL replace with the same appId does not trigger a duplicate fetch") { + val model = ConfigModel().apply { appId = "appId-1" } + val store = mockConfigStore(model) + val (backend, fetchCount) = mockBackend() + val app = foregroundedAppService(true, false, true) + + val service = FeatureFlagsRefreshService(app, store, backend).apply { refreshIntervalMs = 0L } + service.start() + awaitIO() + fetchCount() shouldBe 1 + + service.onModelReplaced(model, ModelChangeTags.NORMAL) + awaitIO() + fetchCount() shouldBe 1 + } + + test("HYDRATE replace with a different appId cancels and re-fetches") { + val model = ConfigModel().apply { appId = "appId-1" } + val store = mockConfigStore(model) + val (backend, fetchCount) = mockBackend() + // start: [true, false] + onModelReplaced check: [true] + new poll loop: [true, false]. + val app = foregroundedAppService(true, false, true, true, false) + + val service = FeatureFlagsRefreshService(app, store, backend).apply { refreshIntervalMs = 0L } + service.start() + awaitIO() + fetchCount() shouldBe 1 + + // Genuine appId change (e.g. re-init with a different App ID). + model.appId = "appId-2" + service.onModelReplaced(model, ModelChangeTags.HYDRATE) + awaitIO() + fetchCount() shouldBe 2 + + coVerify(exactly = 1) { backend.fetchRemoteFeatureFlags("appId-1") } + coVerify(exactly = 1) { backend.fetchRemoteFeatureFlags("appId-2") } + } + + test("appId property update via onModelUpdated triggers a single re-fetch") { + val model = ConfigModel().apply { appId = "appId-1" } + val store = mockConfigStore(model) + val (backend, fetchCount) = mockBackend() + // start: [true, false] + onModelUpdated check: [true] + new poll loop: [true, false]. + val app = foregroundedAppService(true, false, true, true, false) + + val service = FeatureFlagsRefreshService(app, store, backend).apply { refreshIntervalMs = 0L } + service.start() + awaitIO() + fetchCount() shouldBe 1 + + model.appId = "appId-2" + service.onModelUpdated( + com.onesignal.common.modeling.ModelChangedArgs( + model = model, + path = ConfigModel::appId.name, + property = ConfigModel::appId.name, + oldValue = "appId-1", + newValue = "appId-2", + ), + ModelChangeTags.NORMAL, + ) + awaitIO() + fetchCount() shouldBe 2 + } + + test("unfocus then refocus restarts polling for the same appId") { + val model = ConfigModel().apply { appId = "appId-1" } + val store = mockConfigStore(model) + val (backend, fetchCount) = mockBackend() + // start: [true, false] + onFocus restart loop: [true, false]. + val app = foregroundedAppService(true, false, true, false) + + val service = FeatureFlagsRefreshService(app, store, backend).apply { refreshIntervalMs = 0L } + service.start() + awaitIO() + fetchCount() shouldBe 1 + + // onUnfocused must clear the cached pollingAppId so a subsequent refocus is not deduped. + service.onUnfocused() + service.onFocus(firedOnSubscribe = false) + awaitIO() + fetchCount() shouldBe 2 + } +})