Skip to content

Conversation

@kaushalkapasi
Copy link
Collaborator

@kaushalkapasi kaushalkapasi commented Jul 3, 2025

  • fix: updates cache keys to include the SDK Version
  • fix: cleanup cached configs from other SDK Versions
  • chore: update tests for Cached Configs and Config Migrations

@kaushalkapasi kaushalkapasi requested review from Copilot and jsalaber July 3, 2025 15:57

This comment was marked as outdated.

@kaushalkapasi kaushalkapasi force-pushed the fix-cache-configs-by-sdk-version branch from 3fec712 to a0263ef Compare July 3, 2025 18:04
@kaushalkapasi kaushalkapasi requested review from Copilot and jsalaber July 3, 2025 18:05
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR ensures that all configuration cache keys include the SDK version and adds cleanup for any old, non-versioned entries.

  • Prefixes config keys with VERSION_<sdkVersion> in both the service and tests
  • Implements cleanupDeprecatedCachedConfigs() to remove stale entries from previous SDK versions
  • Updates unit tests to expect and verify versioned cache keys

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
DevCycleTests/Models/DevCycleUserTest.swift Tests updated to seed and assert on versioned cache keys
DevCycle/Models/Cache.swift Added versionPrefix, adjusted CacheKeys, and added cleanup for deprecated keys
Comments suppressed due to low confidence (3)

DevCycle/Models/Cache.swift:139

  • The migration logic currently uses the versioned key CacheKeys.identifiedConfigKey as the oldKey, so legacy data stored under the unversioned key won’t be migrated. Consider using the legacy constant (CacheKeys.identifiedConfig) for oldKey to correctly migrate pre-versioned data.
                return nil

DevCycle/Models/Cache.swift:135

  • There is no unit test for cleanupDeprecatedCachedConfigs(). Adding a test to verify that non-versioned keys are removed when migrateLegacyCache() runs would improve coverage and prevent regressions.
    private func cleanupDeprecatedCachedConfigs() {

DevCycleTests/Models/DevCycleUserTest.swift:243

  • [nitpick] The test’s versionPrefix omits the trailing dot present in CacheKeys.versionPrefix. Consider including the dot in this constant (e.g., "VERSION_\(PlatformDetails().sdkVersion).") to avoid manual concatenation and ensure consistency.
        let versionPrefix = "VERSION_\(PlatformDetails().sdkVersion)"

@kaushalkapasi
Copy link
Collaborator Author

Going to use the approach as defined in #244

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants