Skip to content
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

Fix [BUG] https://github.com/Azure/azure-sdk-for-ios/issues/1672 #1673

Merged
merged 1 commit into from
Jan 17, 2024

Conversation

mattials
Copy link
Contributor

@mattials mattials commented Jan 8, 2024

#1672

Use of @available(iOSApplicationExtension, unavailable) for Xcode 14 and 15

@github-actions github-actions bot added Azure.Core Community Contribution Community members are working on the issue customer-reported Issues that are reported by GitHub users external to the Azure organization. labels Jan 8, 2024
Copy link

github-actions bot commented Jan 8, 2024

Thank you for your contribution @mattials! We will review the pull request and get back to you soon.

@angellan-msft
Copy link
Contributor

angellan-msft commented Jan 10, 2024

@mattials Thanks for suggesting the code change to "azure core" file.

First, let's evaluate more on whether this code change is needed.
I'm able to see push notification coming in when testing the sample app in XCode 15.1.
Could you please provide a more detailed description on what steps you were going through, what error did you see on a particular step and which XCode version are you using? How did you come to the conclusion that the bug was in azure core?

Second, if the bug was in azure core,
I noticed the IOS core CI pipeline failed with your suggested change. I believe this code change can't be put in use until the CI pipeline runs successfully. We either need to make more changes in azure core or use another way to fix.
Also, I'm not sure if external contributor has the permission to merge any code changes into azure core. We will need some guidance from our IOS azure reviewer @tjprescott on this question.

@mattials
Copy link
Contributor Author

@angellan-msft,

Thank you for your prompt response. I understand that there might be some confusion regarding the necessity of the code change in the "azure core" file. Let me provide you with a detailed explanation and address the concerns raised

I always get the same error in your example https://github.com/Azure-Samples/communication-services-ios-quickstarts/tree/main/add-chat-push-notifications and following your documentation on advanced push notifications

Testing Environment:
I conducted tests on different machines with varying configurations to ensure the universality of the error and the effectiveness of the proposed fix. The tested environments include:

Apple M2, macOS 13.5.2, XCode 15.0
Intel i5 2019, macOS 13.5.2, XCode 15.2

##First
It will probably work in an already compiled app or using the simple notifications method, but the repository https://github.com/Azure-Samples/communication-services-ios-quickstarts/tree/main/add-chat-push-notifications doesn't compile, the first bug the repository gives with XCode 15 is:

DT_TOOLCHAIN_DIR cannot be used to evaluate LIBRARY_SEARCH_PATHS, use TOOLCHAIN_DIR instead
This has an easy solution, change the Podfile and it's not a concern.

I always get the same error on any machines trying to compile the test project https://github.com/Azure-Samples/communication-services-ios-quickstarts/tree/main/add-chat-push-notifications, if locally we apply the fix I have in this PR it works correctly.

MicrosoftTeams-image (2)

##Second
I have checked your pipe and the tests fail:

✗ test_ShouldBeCalledAgainAfterFirstRefreshCall, XCTAssertEqual failed: ("2") is not equal to ("1")
✗ test_ShouldBeCalledAgainAfterFirstRefreshCall, XCTAssertEqual fallido: ("3") no es igual a ("2")

The test relies on asynchronous execution using DispatchQueue.asyncAfter, and the timing might not be precise. If the token refresh takes longer than expected or if there are delays in the execution of the asynchronous blocks, the assertions may not be accurate.

The test involves asynchronous operations and multiple DispatchQueue calls. If there are shared resources or state changes that are not handled properly in a concurrent environment, it could lead to unexpected behavior.

@tjprescott
Copy link
Member

I'm reluctant to accept any change to Azure.Core if the issue could simply be the example documentation. Are you following the example verbatim? @angellan-msft do you know what the expected environment was when that sample was written? Are you able to repro the issue?

@tjprescott
Copy link
Member

/azp run ios - core - ci

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@tjprescott
Copy link
Member

The AzureCommunicationCommon units tests are known to be flaky. I've re-run the CI which should clear them up.

@tjprescott
Copy link
Member

Okay, the checks pass. @angellan-msft please let me know the results of your investigation.

@angellan-msft
Copy link
Contributor

@mattials Thanks for your detailed description. So the issue you met with is the advanced version of push notification, which I have not tested yet. What I tested is the basic version. I'll try to reproduce the issue today and see if this fix works. Will provide an update ASAP.

@angellan-msft
Copy link
Contributor

hi @mattials , could you try the below workaround to see if it can solve your issue:

Set "Require Only App-Extension-Safe API" as "No" for Pod Target - AzureCore.
Why we need this: https://stackoverflow.com/questions/48122769/facebook-cocoapods-sharedapplication-is-unavailable-not-available-on-ios-app (Opens in new window or tab)

I just found that this workaround was documented in our internal wiki but not in public wiki. If it works for you, I would suggest this workaround rather than making extra code changes to azure core. Thanks!

@mattials
Copy link
Contributor Author

@angellan-msft @tjprescott
Thank you for providing the suggested workaround. I have implemented the fix by setting "Require Only App-Extension-Safe API" to "No" for the Pod Target - AzureCore, and it appears to have resolved the compilation issue. I appreciate your prompt assistance.

While the workaround addresses the immediate concern, our team is committed to ensuring the long-term stability of our product. Therefore, we are interested in contributing to a more permanent solution by updating AzureCore. Could you please provide insights into when a stable version of AzureCore, beyond the current 1.0.0-beta.15, is expected to be released? This information will be valuable for our planning and integration efforts.

Thank you once again for your support, and we look forward to continued collaboration.

@tjprescott
Copy link
Member

Hi @mattials there are currently no plans to move Azure.Core out of beta. We'll evaluate this change for incorporation into Azure.Core 1.0.0-beta.16.

@tjprescott tjprescott merged commit cab9c27 into Azure:main Jan 17, 2024
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Azure.Core Community Contribution Community members are working on the issue customer-reported Issues that are reported by GitHub users external to the Azure organization.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants