basho/riak-java-client — pull request #177 by dclements (2012-11-19 18:02:33+00:00)
Title: General cleanup and thread safety improvements.

A large number of very minor changes for the purposes of cleaning up findbugs warnings and documenting thread safety.

General Cleanup:
- Added the FindBugs JSR305 jar as a dependency. This is technically a compile-time dependency only and shouldn't be needed by anyone downstream, but I left it in as a standard "compile" phase dependency.
- Removed unused imports.
- Added some wildcard generics to casting to prevent raw type exceptions.
- Marked packages as @Deprecated where the javadoc indicated that they were.
- Use TypeFactory.defaultInstance().constructMapType instead of the deprecated TypeFactory.mapType.
- Started explicitly documenting things as @Nullable that can be null.
- Implemented hashCode() where equals() was overridden (and added unit tests for this).
- There were some inner classes that could be turned into static nested classes (per Effective Java).
- Switched some instances of iterating over a keySet on a map to iterating over an entrySet.
- Switched from using new Integer() to either Integer.valueOf or just relying on autoboxing.
- Removed unnecessary object instantiation for unused variables.

Thread safety:
- Some classes used synchronization unnecessarily (e.g., on accessing final string). This has been removed.
- Some classes synchronized around locks in the constructor. This is only necessary if you leak `this` from inside the constructor (e.g., by starting a thread, which is a bad idea anyways), given that this wasn't happening the synchronization has been removed.
- I documented where I thought an implementation was supposed to be thread safe (from use of synchronization or from the javadoc) or looks like it was deliberately not thread safe (no synchronization but mutable). This was done using JSR305's @ThreadSafe and @NotThreadSafe.
- Marked several things as final that should have been but weren't (never changed values, public static strings, etc).
- Marked a few things as @Immutable if they were, in fact, immutable.
- Tried to document use of lock objects with @GuardedBy.
- Made sure lock objects were final.
- The MapReduce class was clearly meant to be thread safe, but wasn't (changes to timeout weren't necessarily visible and followed a check-then-act pattern). Timeout was turned into an AtomicLong.
- In AbstractLogicalFilter at one point there's synchronization around an argument rather than around the member variable. Since everywhere else it is around the member variable, this has been changed.
- Made defensive copies where it seemed wise to do so.