Fix pmd and findbugs error to be able to activate checks in CI

[AndyScherzinger]

#321 will introduce code quality metrics for checkstyle, pmd, findbugs and lint. The checks would actually be turned of for the nightly builds since they report several issues so the builds would fail.

For the build to successfully run with these checks activated (gradle check) the pmd and findbugs issues need to be resolved.
To do so simply checkout the branch of the PR #321, compile, run gradle check and have a look at the reports in build\reports for pmd and checkstyle.

[JubrilEdu]

Hello @AndyScherzinger , does this mean i should run gradle check and solve issues brought up in the code?

[AndyScherzinger]

Hi @djubreel yes, that it what I mean. So when the checks are successful after the necessary changes we can then activate the checks in our CI builds from that time onward 😃

[JubrilEdu]

Okay,I'd work on it.

[AndyScherzinger]

You are awesome @djubreel 🚀 🚀 🚀

[JubrilEdu]

Hello @AndyScherzinger when I run gradle check i get errors like this in the report log

I actually found the line of code that was causing the issue.

checking findbugs website i get to see this

should I implement recommendation from their website and do that for subsequent issues?

[AndyScherzinger]

Hi @djubreel I think pmd would be better, since #330 already provides a PR for the findbugs fixes, which leaves pmd that needs fixing.

[iskradelta]

@djubreel In my opinion it was enough to @SuppressFBWarnings on that one, since the usage of Random in that place (createAccount if this is an oauth type of account), was going to happen once or few times in anycase (how often will user try to createAccount with oauth in same session/activity?) The DMI warning/FindBugs recommendation, is in the case when a Random object is reused many times. I doubted on doing new SecureRandom().nextLong() because that can block if there isnt enough entropy - and maybe was the reason previous developers chose to do new random with current time in millis as seed?

The other place were it was invoked (NotificationUtils.cancelWithDealy), was to generate a unique name for a notification delayer thread of type HandlerThread - and thats not security sensitive - doesnt matter if the delay is not very randomized - its good enough, collisions not likely, but can be improved. A better solution though for that case would be to use UUID.randomUUID() or something like that instead to guarantee uniqueness of thread-names. Perhaps thats something you can do here as a next pull-request? Then we can remove the SupressFBWarnings DMI on that one. What do you think @AndyScherzinger

Another issue I suppressedfbwarnings on, was the "Bx unboxing boxing primitive to .compareTo, use Long.compare(long,long) instead" because that would require stepping up API level, and I dont know if we can do that.

BTW; Im down from about 297 PMD bugs to 140~, mostly squashed many "use braces for if and for statements".

[JubrilEdu]

Thanks for the explanation @iskradelta , nice job too. @AndyScherzinger I'm currently tied with tests in school is why I couldn't pick up the issues once I'm done I'd pick up available issues

[AndyScherzinger]

@iskradelta yes moving on to UUID for unique notification identifiers seems the right thing to do 👍
@djubreel thanks for your help and no worries, tests in school are the priority (imho)