IOS-5365 Integrate membership data fetching refactoring for non-blocking startup

[ignatovv]

Summary

• Changed startup to use noCache: false to prevent blocking app launch with network calls
• Added membershipTiersUpdate event handler for async tier updates from middleware
• Cache membership and tiers separately for efficient status rebuilding
• Fixed analytics to log immediately with event data

[claude]

Bugs/Issues

MembershipStatusStorage.swift:43-47
stopSubscriptionAndClean() doesn't clear the new cached properties _currentMembership and _cachedTiers. This could cause stale data to remain after logout/cleanup. Should add:

_currentMembership = nil
_cachedTiers = []

---

⚠️ Minor Issues - Clear cached properties in cleanup method

[claude]

Bugs/Issues

MembershipStatusStorage.swift:72-82, 84-93
Race condition between concurrent Task blocks. When both handleMembershipUpdate and handleMembershipTiersUpdate are called (lines 72-82 and 84-93), they each create independent Task blocks that can run concurrently. Both modify shared state (_currentMembership, _cachedTiers) and call rebuildStatusIfReady() without synchronization. This can lead to:

• Multiple simultaneous calls to rebuildStatusIfReady() with partial state
• Analytics events logged in wrong order or with inconsistent state
• Race condition on _status writes from concurrent rebuilds

The class is marked @MainActor which means all methods run on MainActor, but the explicit Task { } blocks inside these methods create new tasks that may execute concurrently before the actor isolation guarantees ordered execution.

Suggested fix: Remove the explicit Task { } blocks in lines 73-81 and 85-92. Since the class is already @MainActor, the handlers will execute sequentially on the MainActor:

private func handleMembershipUpdate(_ update: Anytype_Event.Membership.Update) {
    _currentMembership = update.data
    rebuildStatusIfReady()
    
    if let tier = _cachedTiers.first(where: { $0.id.id == update.data.tier }) {
        AnytypeAnalytics.instance().logChangePlan(tier: tier)
    }
}

private func handleMembershipTiersUpdate(_ update: Anytype_Event.Membership.TiersUpdate) {
    let filteredTiers = update.tiers
        .filter { FeatureFlags.membershipTestTiers || !$0.isTest }
    
    Task {
        _cachedTiers = await filteredTiers.asyncMap { await builder.buildMembershipTier(tier: $0) }.compactMap { $0 }
        rebuildStatusIfReady()
    }
}

Note: The async work in handleMembershipTiersUpdate still needs Task for the async mapping, but should be isolated to just that operation.

---

⚠️ Minor Issues - Fix race condition in concurrent event handlers

[claude]

Bugs/Issues

MembershipStatusStorage.swift:79-81
The analytics logChangePlan() call attempts to find the tier in _cachedTiers immediately after _currentMembership is updated, but _cachedTiers may still be empty at this point. If membershipUpdate event arrives before membershipTiersUpdate event, the analytics will fail silently to log the plan change because the tier lookup will return nil.

Consider moving the analytics call into rebuildStatusIfReady() where both membership and tiers are guaranteed to be available, or add a conditional check that only logs if tiers are cached.

MembershipStatusStorage.swift:96
Silent error handling with try? will suppress all errors from buildMembershipStatus(). If there's an issue with the membership data or tier matching, it fails silently without any indication. Consider logging the error or using proper error handling.

Best Practices

MembershipStatusStorage.swift:88
The line _cachedTiers = await filteredTiers.asyncMap { await builder.buildMembershipTier(tier: $0) }.compactMap { $0 } is difficult to read. Consider breaking it into multiple lines for better readability:

let builtTiers = await filteredTiers.asyncMap { await builder.buildMembershipTier(tier: $0) }
_cachedTiers = builtTiers.compactMap { $0 }

---

⚠️ Minor Issues - Fix analytics race condition and add error logging

[claude]

Bugs/Issues

MembershipStatusStorage.swift:93
rebuildStatusIfReady() is called synchronously from handleMembershipUpdate() and handleMembershipTiersUpdate(), but these are async functions. However, rebuildStatusIfReady() accesses _status (a @Published property) and _cachedTiers/_currentMembership without any synchronization. If both membershipUpdate and membershipTiersUpdate events arrive simultaneously in handle(events:), they run sequentially due to the for loop, but there's no guarantee about the order of events or race conditions if multiple event batches are processed concurrently.

MembershipStatusStorage.swift:87
filteredTiers.asyncMap returns an array of optionals which is then compactMap-ed. However, if buildMembershipTier throws an error (since asyncMap is rethrows), the entire function will crash or fail. Consider wrapping individual tier building in try? within the map closure to handle partial failures gracefully.

MembershipStatusStorage.swift:38
Changed from noCache: true to noCache: false. This means startup now uses cached data, but if the cache is stale or empty, _status will be .empty. The event handlers (membershipUpdate/membershipTiersUpdate) will eventually update it, but there's now a potential race where currentStatus returns stale/empty data during initial subscription setup if events haven't fired yet. Consider documenting this behavior or ensuring initial cache is valid.

Best Practices

MembershipStatusStorage.swift:102-106
Analytics event logChangePlan is deferred until tiers are available using _pendingPlanChangeTierId. This is good, but if multiple membershipUpdate events arrive before membershipTiersUpdate, only the last tier change will be logged (since _pendingPlanChangeTierId is overwritten at line 78). Consider using an array or set to track all pending tier changes.

---

⚠️ Minor Issues - Address race condition concerns, error handling in asyncMap, and potential multiple plan change events