Fix push notifications for UnifiedPush distributors without provided matrix gateway

[SpiritCroc]

Fixes #3571

Content

We need to account for UnifiedPush distributors not providing a Matrix gateway, to properly use our fallback gateway.
To quote p1gp1g, we need to:

• return the customGateway if the check returns 200 and discoveryResponse.unifiedpush.gateway == "matrix"
• return the default gateway if the check returns 200 and there isn't discoveryResponse.unifiedpush.gateway == "matrix"
• return the default gateway if the check returns 404
• return the previous gateway if it fails
• if there isn't a previous gateway, we can use one or the other, it doesn't really matter

Motivation and context

Fixes #3571

Tests

• Set up a distributor which does feature a matrix gateway, such as ntfy
• Select ntfy as push distributor
• Check "Troubleshoot notifications" that the custom gateway is used and push works
• Set up a distributor which does not feature a matrix gateway, such as gCompat
• Select "gCompat UP-Distributor" as push distributor
• Check "Troubleshoot notifications" that the fallback gateway is used and push works

Tested devices

NOTE: I only tested on SchildiChat Next

• Physical device
• Emulator
• OS version(s): 14

Checklist

• Changes have been tested on an Android device or Android emulator with API 23
• Pull request is based on the develop branch
• Pull request title will be used in the release note, it clearly define what will change for the user
• You've made a self review of your PR

[CLAassistant]

All committers have signed the CLA.

[github-actions]

Thank you for your contribution! Here are a few things to check in the PR to ensure it's reviewed as quickly as possible:

• Your branch should be based on origin/develop, at least when it was created.
• The title of the PR will be used for release notes, so it needs to describe the change visible to the user.
• The test pass locally running ./gradlew test.
• The code quality check suite pass locally running ./gradlew runQualityChecks.
• If you modified anything related to the UI, including previews, you'll have to run the Record screenshots GH action in your forked repo: that will generate compatible new screenshots. However, given Github Actions limitations, it will prevent the CI from running temporarily, until you upload a new commit after that one. To do so, just pull the latest changes and push an empty commit.

[bmarty]

Thanks for the change. I do not really like to provide the previous value as a parameter for UnifiedPushGatewayResolver.getGateway() but I do not see any better solution. Returning String? could be an alternative, but it may not fit all the needs.

[bmarty]

Could use HttpURLConnection.HTTP_NOT_FOUND.

[spaetz]

I love that functionality, thanks for that. That having said, I agree with bmarty that passing in previousGateway feels really ugly. Returning None/Null/Empty string in the case where we want to use the previous gateway feels much better as in "don't change current value".

That having said, I won't be able to provide patches so whatever is picked, I love it ❤️

[SpiritCroc]

Yeah agreed, passing previousGateway isn't very elegant. Seems to be the most straightforward approach to fulfil the spec I got without over-engineering it though. I assume just always returning null from getGateway() and falling back outside to the previous value would work except for the "if there isn't a previous gateway, we can use one or the other, it doesn't really matter" case. If "it doesn't matter" is to be interpreted that we can also fallback to no gateway at all, then that's probably the most elegant way, I don't know right now how relevant this is in practice.

[bmarty]

OK, let's be pragmatic and merge those changes.

There are 2 failing tests, can you handle that please? Else let me know, I can take the point

VectorUnifiedPushMessagingReceiverTest > onNewEndpoint, if registration fails, the endpoint should not be stored FAILED
    java.lang.AssertionError at VectorUnifiedPushMessagingReceiverTest.kt:131

VectorUnifiedPushMessagingReceiverTest > onNewEndpoint run the expected tasks FAILED
    java.lang.AssertionError at VectorUnifiedPushMessagingReceiverTest.kt:91

[SpiritCroc]

OK, let's be pragmatic and merge those changes.

There are 2 failing tests, can you handle that please? Else let me know, I can take the point

VectorUnifiedPushMessagingReceiverTest > onNewEndpoint, if registration fails, the endpoint should not be stored FAILED
    java.lang.AssertionError at VectorUnifiedPushMessagingReceiverTest.kt:131

VectorUnifiedPushMessagingReceiverTest > onNewEndpoint run the expected tasks FAILED
    java.lang.AssertionError at VectorUnifiedPushMessagingReceiverTest.kt:91

I probably can look at those again earliest next weekend. I had a quick look before but didn't see immediately what the appropriate fix for these two would be. So if you can fix them earlier that would be great, thanks!