Skip to content

Conversation

@requilence
Copy link
Contributor

Updated membership data fetching to use middleware cache for faster app startup.

Problem

Middleware now returns cached membership/tiers data immediately (with noCache: false) instead of blocking on network calls. App startup was slow because we were forcing network calls with noCache: true.

Changes

Fast Startup

  • Changed startSubscription() to use noCache: false (uses middleware in-memory cache)
  • Initial load now instant - uses middleware's in-memory data

Event Handling

  • Added membershipTiersUpdate event handling
  • Both membershipUpdate and membershipTiersUpdate call getTiers() to refresh
  • Service handles all filtering logic (test tiers, iOS compatibility)

Approach

  • Simple and straightforward - reuses existing service filtering
  • Events trigger RPC calls with noCache: false (fast, in-memory on middleware)
  • Accepts duplicate getTiers() calls as inexpensive
  • No architectural changes to storage or UI layers

Trade-offs

Compare with another PR with bigger refactoring #4111

Pros:

  • Minimal code changes
  • Reuses existing filtering logic in service
  • Simple to understand and maintain

Cons:

  • Extra RPC calls on events (though cheap - in-memory cache)
  • Service called from both startup and event handlers

@requilence requilence requested a review from a team as a code owner October 17, 2025 14:04
@requilence requilence changed the title IOS-5365 implement the async membership handling IOS-5365 implement the async membership handling (simpler approach) Oct 17, 2025
@claude
Copy link
Contributor

claude bot commented Oct 17, 2025

Code Review

Bugs/Issues

MembershipStatusStorage.swift:68-75 - Missing error handling for membership status updates
When membershipTiersUpdate event is received and currentMembershipData is nil, the status won't be updated but no error is logged. This could hide issues during initial app states before any membership data is received.

MembershipStatusStorage.swift:38 - Race condition potential
noCache: false is now used during startSubscription(), but if middleware cache is empty on first launch, this could return stale/empty data while setupSubscription() hasn't received the first update yet. Consider logging if cache is empty or handling the empty state explicitly.

Best Practices

MembershipStatusStorage.swift:55 - Property not cleaned up
currentMembershipData is set to nil in stopSubscriptionAndClean() but should also be cleared when receiving membershipUpdate after it's used to prevent holding onto old data unnecessarily (line 70 already updates it, so this is technically handled, but the pattern is unclear).

Libraryfile:1 - Non-standard middleware version
Using branch name go-6337-make-tiersmembership-fetching-async instead of a version tag. This should be updated to a proper version/tag before merging to ensure reproducible builds.

MembershipCoordinatorModel.swift:29 - Missing initial state fetch
Added subscription to tiersPublisher but tiers property won't be populated until the publisher emits. Should initialize with membershipStatusStorage.currentTiers for immediate display.


Summary: ⚠️ Minor Issues - Missing error handling, race condition potential, non-standard middleware version needs fixing before merge

@@ -1 +1 @@
MIDDLE_VERSION=v0.44.0-nightly.20251016.1
Copy link
Member

Choose a reason for hiding this comment

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

we will not merge this pr to develop until there will be proper middleware veriosn

Copy link
Member

@ignatovv ignatovv left a comment

Choose a reason for hiding this comment

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

blocked until mw nightly build will arrive

private var membershipService: any MembershipServiceProtocol
@Injected(\.membershipModelBuilder)
private var builder: any MembershipModelBuilderProtocol
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please disable "trim whitespace-only lines" in Xcode and rollback all this changes



nonisolated init() { }
Copy link
Collaborator

Choose a reason for hiding this comment

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

properties first, methods next. Please move order back

var currentTiers: [MembershipTier] { _tiers }
@Published private var _tiers: [MembershipTier] = []

private var currentMembershipData: Anytype_Model_Membership?
Copy link
Collaborator

Choose a reason for hiding this comment

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

Make alias in Service, MembershipStatus.swift file on the top. We don't use the middle names directly. There are big

@requilence requilence closed this Oct 30, 2025
@github-actions github-actions bot locked and limited conversation to collaborators Oct 30, 2025
@requilence
Copy link
Contributor Author

closed in favor of #4111

@ignatovv ignatovv deleted the ios-5365-membership-data-fetching-refactoring-integration-ios-simple branch November 26, 2025 14:17
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants