-
Notifications
You must be signed in to change notification settings - Fork 241
NSOperationQueue Migration & Test Reorganization #1545
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Closed
Closed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Work in progress on removing legacy Objective-C bridging code and adding modern Swift implementations to the 4.0.0-alpha.0 branch. Changes: - Added Swift implementations from PR #1533: - BranchRequestQueue.swift (Actor-based queue with Swift Concurrency) - BranchRequestOperation.swift (Modern operation implementation) - ConfigurationController.swift (Configuration management) - Updated Package.swift: - Upgraded swift-tools-version to 5.9 - Added BranchSwiftSDK target - Updated paths and dependencies - Removed bridging files: - Deleted BNCServerRequestQueue.m and BNCServerRequestQueue.h - Removed 16 references from Xcode project - Updated Branch.m: - Removed import of BNCServerRequestQueue.h - Commented out legacy error handling code that used remove: and peekAt: methods (These methods are incompatible with NSOperationQueue implementation) - Updated Branch.h: - Removed import of BNCServerRequestQueue.h - Added forward declarations for BNCServerRequestQueue and BNCServerRequest STATUS: Build currently failing with compilation errors. Additional work needed to: 1. Fix remaining compilation errors in dependent files 2. Implement proper error handling for NSOperationQueue-based request queue 3. Test framework and app builds 4. Complete migration to Swift implementations This is part of the broader effort to modernize the iOS Branch SDK with Swift Concurrency patterns (async/await, actors) while maintaining backward compatibility.
Changes: - Added modern Swift-based BranchRequestQueue (actor pattern with NSOperationQueue) - Added BranchRequestOperation for async request processing - Added ConfigurationController for operational metrics - Updated Package.swift to Swift 5.9 with dual-target architecture (BranchSDK + BranchSwiftSDK) - Restored BNCServerRequestQueue files temporarily for compatibility - Added BNCServerRequest.h imports to BranchQRCode.m and BranchEvent.h - Restored BNCServerRequestQueue.h import in Branch.m Current state: - Both old (BNCServerRequestQueue) and new (BranchRequestQueue) implementations coexist - Framework builds successfully - Next step: Migrate Branch.m to use BranchRequestQueueBridge, then remove old queue
Major changes from PR #1533: 1. Updated BNCServerRequestQueue implementation: - Replaced NSMutableArray with NSOperationQueue for serial processing - Added configureWithServerInterface:branchKey:preferenceHelper: method - Added enqueue:withPriority: for request prioritization - Automatic request processing (no manual peek/remove needed) 2. Added BNCServerRequestOperation: - New NSOperation subclass for async request execution - Handles session validation and error processing - Integrates with BNCCallbackMap for event callbacks 3. Updated Branch.m: - Removed manual queue processing (processNextQueueItem calls) - Removed insertRequestAtFront: method (priority via NSOperationQueue) - Added queue configuration in init - Simplified request enqueueing (automatic processing) 4. Updated Xcode project: - Added BNCServerRequestOperation.m to compile sources - Added BNCServerRequestOperation.h to Private headers Architecture improvements: - Serial queue execution via maxConcurrentOperationCount = 1 - Automatic operation lifecycle management - Better separation of concerns (queue vs operations) - Foundation for future Swift migration Build status: ✅ SUCCESS
Changes: - Removed BranchSwiftSDK target from Package.swift - Deleted all Swift source files (BranchRequestQueue, BranchRequestOperation, ConfigurationController) - NO bridge code, NO Swift dependencies - Pure NSOperationQueue-based Objective-C solution Architecture: - Single Objective-C target (BranchSDK) - NSOperationQueue for request processing - BNCServerRequestOperation wrapper - Thread-safe by design - iOS 12.0+ compatibility Build Status: ✅ Successful (Xcode + SPM)
Changes: - Add BNCConfig.h import to BranchSDK.h - Add BranchConstants.h import to BranchSDK.h - Update BranchConfigurationController.m to use Private/ path for header import Purpose: - Improve header organization - Make private headers explicit via Private/ path - Add missing public header imports - Better separation of public vs private APIs Build Status: ✅ Successful
Changes: - Add new BranchSDKTests/ directory with 48 test files - Comprehensive unit tests for SDK components - Modern XCTest-based test structure - Includes test plan and Info.plist - Update Branch-TestBed/ (28 files modified) - Modernize imports from #import to @import BranchSDK - Update bridging header - Update Xcode schemes - Update project configuration Test Organization: - BranchSDKTests/: New comprehensive test suite - Branch-TestBed/: Updated integration tests - All tests use modern import style (@import) Build Status: ✅ SDK build successful Test Files: 76 files changed (48 new, 28 modified) Note: TestDeepLinking removal will be handled separately
Contributor
|
Important PR Review SkippedPR review skipped as per the configuration setting. Run a manually review by commenting /matter review 💡Tips to use MatterAICommand List
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
This PR implements of the iOS SDK modernization effort, focusing on:
Motivation
The legacy queue implementation used manual array management and synchronization, which was error-prone and harder to maintain. This PR modernizes the architecture using Foundation's NSOperationQueue while maintaining backward compatibility and removing all Swift dependencies as per project requirements.
Changes
NSOperationQueue Migration
BNCServerRequestQueuewith NSOperationQueueBNCServerRequestOperationwrapper for request executionBranch.mto use automatic queue processingprocessNextQueueItemcallsmaxConcurrentOperationCount = 1Swift Code Removal
Test Reorganization (76 files)
BranchSDKTests/directory with 48 comprehensive test filesBranch-TestBed/(28 files) with modern import style (@import BranchSDK)Headers Organization
BNCConfig.himport to BranchSDK.hBranchConstants.himport to BranchSDK.hPrivate/pathTechnical Details
Architecture
Old (Array-based):
New (NSOperationQueue-based):
Key Benefits
Testing
Build Status
Commits
0d9b95e0- WIP: Phase 2 - Remove bridging code and add Swift implementations3c48289d- Add Swift queue implementation alongside existing queuefdb3c610- Migrate to NSOperationQueue-based request processinga7ed8b10- Remove Swift implementation, finalize pure Objective-C architecture2d5f7a53- Phase 5: Reorganize headers and add missing imports67862dc5- Phase 2: Test reorganization and modernizationFiles Changed
Checklist
Breaking Changes
None - This is a pure internal refactor maintaining full backward compatibility.
Migration Notes
No migration needed for SDK users. All changes are internal implementation details. The public API remains unchanged.
Next Steps
After merge to
4.0.0.alpha.0:Note: This PR intentionally excludes Phases 3 (Demo Apps) and 4 (CI/CD) as per project requirements.