feat(android): Remove all gms usages and use guava instead

[meypod]

hi @mikehardy , sorry about mentioning, please take a look at this PR if you can merge this
this is related to #493

I tried running tests and all passed
the code may look inconsistent in indentations, this is intentional, since there were many changes, I want to keep the noise down
you can format the code later using the formatter you use to fix the formatting issues.

thanks !

[CLAassistant]

All committers have signed the CLA.

[mikehardy]

wow! pretty quick from idea to a PR, very cool. This isn't the sort of change I can (or should or would) ingest unilaterally so I need to find some time to sync up with @helenaford on it, but the general idea of "replacing async programming library with something that works the same but is 'free software' compatible is something I support. And the churn in the diff isn't too crazy, though it needs review of course

left just a couple specific comments which you've likely already seen...

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 77.08%. Comparing base (15c1166) to head (a2a4f81).
Report is 2 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #510   +/-   ##
=======================================
  Coverage   77.08%   77.08%           
=======================================
  Files          32       32           
  Lines        1727     1727           
  Branches      579      578    -1     
=======================================
  Hits         1331     1331           
+ Misses        395      343   -52     
- Partials        1       53   +52     

[helenaford]

ah nice, i like this idea of having everything fully open source :) happy to release this as an alpha from your p/r if that helps? but won't be able to fully test this before merging to master for a couple of weeks as I'm going away.

[meypod]

It would be very helpful,
I haven't test if it passes f-droid checks yet, but I just went with the idea to see how it goes

edit: I probably will update this branch tomorrow around the same time, are you available till then ?

[helenaford]

@meypod nope unfortunately not... but there's a way to use patch-package i believe. I'm going on vacation for two weeks from tomorrow

[mikehardy]

I'm not sure this one works well with patch-package, however I know from previous collaboration on the idea that you a build script that pulls notifee core from git and references it from source when building the react-native module - no reason you can't switch both of those to point at your local branch.

More than anything, I think it was important just to see what Helena's initial reaction was - that was important for me anyway. Seeing it start with "ah nice" makes me think this will merge over time so it shouldn't be too dangerous to go off your branches for a bit while we work through it

[meypod]

Have a good vacation !
and I think you are right mikehardy, I can switch to my own branch on the script for now
thanks a lot for the info, help and idea on this !

[helenaford]

Thank you. I had a great vacation. I am always open to moving to open source libraries, I'm not going to lie though that this change does make me feel a bit nervous as it's a big change 😅 Just might need some more testing than smaller p/rs

[meypod]

Nice
meanwhile since I needed this change, I used it inside my app and got it released on f-droid, and It's been working fine so far

[mikehardy]

This looks good to me, and honestly, there is nothing better for making sure something works than an actual deployed app with users (and you have 8 stars on github - no joke, just having the first few people watch/follow/use an app is where almost all the issues are discovered...)

Nice one

[meypod]

I'm getting few reports from my app users that it's not getting triggered reliably despite disabling battery optimizations

Since it's started happening after this update, I'd say it's possible that I have broken sth that I'm not aware

The app been triggering as expected for myself so I have my doubts if it's the update causing this problem alone

[meypod]

Is it possible that it has caused a memory leak and os terminating it due to leak after sometime on low end devices?

[mikehardy]

@meypod anything is possible? Using the Android Studio profiler can say

[meypod]

well, profiler did not show any sign of leak, memory usage was consistent
I guess the problem is somewhere else then

edit: it seems my app was crashing sometimes due to service trying to unbind under some condition that would throw an exception, so that maybe was why app was not working properly sometimes

[meypod]

After hours of debugging I figured there was indeed some issue with the PR
it seems that while the tests passed, there was still some issues that wasn't really visible until I filtered the logs for notifee package

I hope it was the only fixes needed

[meypod]

@mikehardy any idea when and if this will be merged?

[meypod]

I merged main into it, so it should be ready to go I guess.
I have updated my app to use this commit, I'll be testing it for the next days to ensure it's working as expected on my end.
at the end, its not up to me to decide if it's going to get merged or not.
I don't blame maintainers for taking long either. maintaining open source projects, especially when there's a large (and in this case a little bit scary) change, takes a lot of time and effort.

[meypod]

it's been working without any problem for me past days.

[Rexogamer]

I also adopted the fork and can confirm that it works as normal :3

[8BallBomBom]

Been testing this fork on and off for a while and so far everything seems to be working as expected.
Hopefully a merge is on the horizon 😅

[mikehardy]

Hey hey 👋

Fixed the conflict locally, got this running locally so e2e tests passed, ran it through our java auto-formatter and things appear to work well

I did have to re-add GMS to the e2e test because we use react-native-firebase there and it does app initialization for the firebase components, but it compiles just fine without it so I don't believe there are any incorrect dependencies (GMS vs Guava) internally

I did notice one tiny thing with a static listener in the ChannelManager and used in NotifeeCore when it seems it should be the opposite? I may make that change and push it but otherwise it looks like a pretty mechanical change as expected

Hoping to merge shortly and then as I'm validating all the issues piled up in the repository it will get a lot of testing

[mikehardy]

Really struggling with CI on this one, and I was pretty certain I had that all ironed out so I'm not ignoring it, I'm going to fight through it. Should go green once I fix what appears to be like 4 different ways the iOS thing is failing now

[mikehardy]

really close now. I've seen this flake locally as well, will resolve then I think this will be okay

Wed, 02 Oct 2024 20:03:17 GMT
12) Cancel Notifications: cancels all notifications and resets badge count to 0  ❌
Wed, 02 Oct 2024 20:03:17 GMT
   expected 1 to equal +0

[mikehardy]

We are 🤏 this close! I have everything the way I want it in the code now

(this last change here just refactoring / moving the main executor service to Notifee was the only change really a854867?diff=unified&w=1)

Final steps are solely mechanical and in the area of release engineering:

1- I've peeled all the commits unrelated to this into #1111 and I want that to go green to give me confidence things are working
2- that PR contains what appears to me to be a real fix for the iOS platform (badge clearing broke...) so I want to then release that one so it's a clean fix-only release before this lands
3- I'll rebase this PR here post-release to clear out unrelated commits, then squash-merge the result into main
4- then I'll release this as a feature release

I may be interrupted in this process but it will hopefully all happen in next few hours

Great work with this change, and thanks for the multi-year (!) patience landing it.

[meypod]

we are here because you pointed me at the right direction 2 years ago
I'm happy I could help
thank you for working on this

[mikehardy]

After quite the unexpected battle with CI, this is out now!

https://github.com/invertase/notifee/releases/tag/%40notifee%2Freact-native%409.1.0

🍾

[8BallBomBom]

Honestly i was really wondering if this would ever see the light of day 🥲
@mikehardy Thanks for all your efforts pushing this forward and good work 😃
@meypod Thanks for the initial commit and hard work on this over the years, took a while to get moving ahaha 😅

[mikehardy]

I'm a big fan of open source so this was important to me but did seem like something always came up higher priority 😅. All credit to meypod though sincerely. This PR was good enough to go almost from day one

Now to solve all the new architecture / listener issues! There is a huge pile of those...