Fix ConstructorThrow SpotBugs warnings in core module

[DomGarguilo]

This PR fixes the warnings from the core module. Here is a description from the ticket:

This warning gets emitted when an exception is thrown within an constructor.

There are several ways to avoid this warning:

1. make the class final
2. remove the thrown exception from the constructor
3. add a finalize() method so it cant be overridden
4. make the constructor private

There are pros and cons to each fix.

• making a class final prohibits inheriting it
• removing the thrown exception usually means forgoing validation
• adding a finalize() method means we also need to ignore the deprecation warning since finalizer() is deprecated
• making the constructor private usually takes a lot of refactoring to move to a different pattern to create the object

I think the best approach here is to work through a couple modules at a time and fix these.

I removed omit block from the root pom and instead moved it into each module so that we can just ignore this spotbugs warning until we fix them in each module.

[ddanielr]

Moving this check out from the top-level pom to the modules makes sense.

Do we need to make release notes changes for finalizing any of the classes that are defined in our public API?

[DomGarguilo]

Do we need to make release notes changes for finalizing any of the classes that are defined in our public API?

That is a good question. I am not sure.

[DomGarguilo]

I think feedback regarding the method of fixing this warning would be helpful (making the class final vs. adding the finalize() method). I tried to only make things final if it was an inner class and/or something that seemed very unlikely to be subclassed, however I just used my best judgement so would like others input on this part.

[ddanielr]

Do we need to make release notes changes for finalizing any of the classes that are defined in our public API?

That is a good question. I am not sure.

I think feedback regarding the method of fixing this warning would be helpful (making the class final vs. adding the finalize() method). I tried to only make things final if it was an inner class and/or something that seemed very unlikely to be subclassed, however I just used my best judgement so would like others input on this part.

I don't see an issue with finalizing the default implementation classes since users should be implementing new classes from the interface rather than extending the clientImpl classes.

Since finalize() is deprecated, does this SpotBugs warning go away if we were to switch to a newer java version?

[dlmarion]

I think feedback regarding the method of fixing this warning would be helpful (making the class final vs. adding the finalize() method).

I have no issue with making classes final where we can and where it makes sense. I'm not sure adding a finalizer to fix the others is a good long-term approach though it's likely the easiest fix. Finalizers themselves are deprecated and will be removed in a future Java release. I'm wondering if fixing the objects so that they don't throw exceptions from the constructor is the better choice, for example, using an init method on the objects that needs to be called post-construction but before use. There is now a jvm option, --finalization=disabled, in JDK 18 that disables all Finalizers as a step toward removal.

[ctubbsii]

These are pretty broad suppressions. I think it's nice that you're trying to narrow them down and triage them one at a time, but I'm wondering if this should be considered a WIP, or is this the intended final PR?

[DomGarguilo]

This PR is no longer a WIP.

To fix these spotbugs warnings I either made the class final (best solution, not possible everywhere) or I added a @SuppressFBWarnings tag to the class.

[keith-turner]

All the java changes look good, could not really review the maven changes very well though.