Address all JDBC ratification comments with small fixes #1

[vfraga]

In order to avoid too many PRs, I've addressed all comments that had small fixes (1 to 10 lines of code) in a single PR.

Changes:

• Suppress Spotbugs errors (They weren't showing up before moving plugin to flight-jdbc-driver module only).
• Moved changes in java/pom.xml to java/flight/flight-jdbc-driver/pom.xml. [link]
• Added some missing exception messages. [link 1] [link 2]
• Documented Holder classes (used in the Accessors). [link]
• Changed timeZone == null behavior to use UTC instead of system default. [link]
• Changed BitVector getObjectClass() to Boolean.class. [link]
• Changed URL acceptance error messages. [link 1] [link 2]
• Changed ArrowFlightJdbcDriver#createVersion() to not use break. [link]
• Add suppressed exceptions for allocator.close() in ArrowFlightConnection. [link]
• Removed ExceptionTemplateThrower class to a private method. [link]
• Refactored FlightStreamQueueTest to use ErrorCollector properly. [link]

[rafael-telles]

Why this annotation? What was the error that FindBugs reported?

[vfraga]

EI: May expose internal representation by returning reference to mutable object (EI_EXPOSE_REP)
Returning a reference to a mutable object value stored in one of the object's fields exposes the internal representation of the object. If instances are accessed by untrusted code, and unchecked changes to the mutable object would compromise security or other important properties, you will need to do something different. Returning a new copy of the object is better approach in many situations.

[vfraga]

EI2: May expose internal representation by incorporating reference to mutable object (EI_EXPOSE_REP2)
This code stores a reference to an externally mutable object into the internal representation of the object. If instances are accessed by untrusted code, and unchecked changes to the mutable object would compromise security or other important properties, you will need to do something different. Storing a copy of the object is better approach in many situations.

[jduo]

Do we need to return a mutable reference to the properties though?
Should we instead of exposing the Properties object itself, add getProperty() function? Should we return an immutable view over the Properties?

[vfraga]

I was able to make it work with copies of Properties, and with ChildAllocators for some BufferAllocators. But the rest still apply (for vectors).

[jduo]

Are we sure this dependency doesn't show up elsewhere? If it does, let's put it in dependency management.

[vfraga]

It isn't used. All dependencies were added here are only for flight-jdbc-driver.

[jduo]

This must be used elsewhere in the project. Let's make sure we use dependencyManagement appropriately.

[vfraga]

I'm overriding the version.

[jduo]

Why is this necessary? Should we update the version generally instead?

[vfraga]

It added ErrorCollector.checkThrows and assertThrows: https://github.com/junit-team/junit4/blob/HEAD/doc/ReleaseNotes4.13.md

[jduo]

Do you think we can express the test using the older JUnit API? I'm not sure if it's a good idea to have different components use different versions of JUnit. We should either universally update the JUnit version or universally use the older versions.

Do we have inconsistent versions of JUnit elsewhere?

[vfraga]

You're right. JUnit 4 is only defined in java/pom.xml to keep running older tests; which also makes me think about maybe starting using JUnit 5 in the future? Not sure about what the implications where when the project started.
I made a simple assertThrows based on JUnit's, which made building with 4.12 possible again.

[jduo]

Is there a more appropriate package for this annotation?

[vfraga]

All SpotBugs/FindBugs annotations are from this package: https://spotbugs.readthedocs.io/en/latest/annotations.html

[vfraga]

We solved by using an XML filter instead.

[jduo]

getResourceAsStream returns null if the path isn't found. Suggest we throw something more helpful if we get that (this will probably be an NPE)

[vfraga]

Done.

[jduo]

In these two error messages, can we also mention the expected format for the URI?

[vfraga]

Done.

[rafael-telles]

Lets remove all @SupressFBWarnings annotations and declare suppresions on a separate .xml file

[vfraga]

Done.

[jduo]

Doesn't this happen automatically because it's a ClassRule?

[vfraga]

You're right. Removed it.