-
Notifications
You must be signed in to change notification settings - Fork 243
NSOperationQueue Migration & Test Reorganization #1546
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
base: 4.0.0-alpha.0
Are you sure you want to change the base?
Conversation
… with v1/install request.
This reverts commit 0ad4564.
This reverts commit 53d322c.
This reverts commit ef025f1.
-> Removed DeepLinkDemo, TestDeepLinking and TestHost Apps for the repo. -> Removed Unit tests from Branch Test Bed App -> Added Test Host App and Unit tests to BranchSDK Workspace.
This reverts commit 83cd178.
This reverts commit af22cad.
* Release 3.13.3 Updates.
- Fixed Package.swift syntax error on line 1 (removed extra 'c' character) - Moved BNCConfig.h from Private to Public group in Xcode project to match actual file location at Sources/BranchSDK/Public/BNCConfig.h These changes resolve compilation errors where BNCConfig.h could not be found during builds.
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
|
Important PR Review SkippedPR review skipped as per the configuration setting. Run a manually review by commenting /matter review 💡Tips to use MatterAICommand List
|
…rocessing' into feature/remove-bridging-code
| #import <LinkPresentation/LPLinkMetadata.h> | ||
| #import "BranchQRCode.h" | ||
| #import "Branch.h" | ||
| #import "BNCServerRequest.h" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@wpinho-branch Is this extra inclusion ? I dont see any other change in this file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Resolved merge conflicts: - Sources/BranchSDK/Branch.m: Keep deprecated processRequest method commented out (request processing now handled by BNCServerRequestOperation), incorporated master's comment about response code handling - Branch-TestBed/project.pbxproj: Keep feature branch's test reorganization
The BNCAppleReceipt class was removed in commit e29ddd9 as part of removing apple_receipt and apple_testflight params from server requests. This test file was inadvertently re-added during the test reorganization.
The loadODMInfoWithTimeOut:andCompletionHandler: method was changed to loadODMInfoWithCompletionHandler: - update test to use the current API.
- Use BranchConfigurationController (ObjC) instead of ConfigurationController (Swift) when building with Xcode. The Swift version is only available with SPM builds. - Update import path to Private/BranchConfigurationController.h - Change framework detection assertions to use XCTAssertNotNil since actual values depend on runtime environment (simulator vs device)
Add HEADER_SEARCH_PATHS to Debug and Release configurations for BranchSDKTests to allow importing private SDK headers: - $(SRCROOT)/Sources/BranchSDK - $(SRCROOT)/Sources/BranchSDK/Private - $(SRCROOT)/Sources/BranchSDK/Public Also includes the deployment target fix (18.5 -> 12.0) from previous session to allow running on iOS 18.4 simulators.
The merge with master brought version 3.13.3 which should not be part of this PR. Reverting all version references back to 3.12.1 to maintain consistency with the 4.0.0-alpha.0 branch. Files reverted: - BranchSDK.podspec - Sources/BranchSDK/BNCConfig.m - scripts/version.sh - ChangeLog.md - BranchSDK.xcodeproj/project.pbxproj
|
Important PR Review SkippedPR review skipped as per the configuration setting. Run a manually review by commenting /matter review 💡Tips to use MatterAICommand List
|
Response to Review CommentsRegarding
|
- Add BNCServerRequestOperation.m to BranchSDK-static target sources - Add BNCServerRequestOperation.m to BranchSDK-tvOS target sources - Remove duplicate BranchConstants.h import from BranchQRCode.m This fixes the CI pipeline failures caused by undefined symbol errors in Carthage, CocoaPods, xcframework, and static framework builds. The class was previously only included in the main BranchSDK target.
|
Important PR Review SkippedPR review skipped as per the configuration setting. Run a manually review by commenting /matter review 💡Tips to use MatterAICommand List
|
Remove Public/ and Private/ path prefixes from import statements in Branch-Bridging-Header.h. CocoaPods flattens all headers into a single directory, so relative paths don't work in the built framework.
|
Important PR Review SkippedPR review skipped as per the configuration setting. Run a manually review by commenting /matter review 💡Tips to use MatterAICommand List
|
Summary
This PR implements 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.