Fix lint warnings in Syncbase Java API code.

Made a pass through the Syncbase API code fixing Lint warnings,

* Added sanity check in Syncbase.login to make sure we don't get a
  null pointer exception.

* Replace throwing of RuntimeException with throwing of more specific
  standard unchecked exception, and add @throws tag to javadoc
  documenting them.

* Reduce visibility of fields and methods from protected to
  package-local or private.

* Make fields final where possible.

* Remove unused variables.

* Remove unnecessary casts.

* Add missing text after Javadoc tags.

* Remove unused SyncgroupSpec return type from Syncgroup.join

* Replace Arrays.asList(x) with Collections.singletonList(x)

Change-Id: I91b56fab9019894ba8bcbde8529ab8fb4563482a
diff --git a/syncbase/src/main/java/io/v/syncbase/AccessList.java b/syncbase/src/main/java/io/v/syncbase/AccessList.java
index a1afb5b..a6f4d55 100644
--- a/syncbase/src/main/java/io/v/syncbase/AccessList.java
+++ b/syncbase/src/main/java/io/v/syncbase/AccessList.java
@@ -24,11 +24,14 @@
 
     public Map<String, AccessLevel> users;
 
+    /**
+     * @throws IllegalArgumentException if accessList is not valid
+     */
     private static Set<String> parsedAccessListToUserIds(Map<String, Set<String>> accessList) {
         Set<String> res = new HashSet<>();
         if (accessList.containsKey(Permissions.NOT_IN) &&
                 !accessList.get(Permissions.NOT_IN).isEmpty()) {
-            throw new RuntimeException("Non-empty not-in section: " + accessList);
+            throw new IllegalArgumentException("Non-empty not-in section: " + accessList);
         }
         for (String blessingPattern : accessList.get(Permissions.IN)) {
             // TODO(sadovsky): Ignore cloud peer's blessing pattern?
@@ -44,7 +47,10 @@
         this.users = new HashMap<>();
     }
 
-    protected AccessList(Permissions corePermissions) {
+    /**
+     * @throws IllegalArgumentException if corePermissions are not valid
+     */
+    AccessList(Permissions corePermissions) {
         Map<String, Map<String, Set<String>>> parsedPermissions = corePermissions.parse();
         Set<String> resolvers = parsedAccessListToUserIds(parsedPermissions.get(Permissions.Tags.RESOLVE));
         Set<String> readers = parsedAccessListToUserIds(parsedPermissions.get(Permissions.Tags.READ));
@@ -52,13 +58,13 @@
         Set<String> admins = parsedAccessListToUserIds(parsedPermissions.get(Permissions.Tags.ADMIN));
 
         if (!readers.containsAll(writers)) {
-            throw new RuntimeException("Some readers are not resolvers: " + readers + ", " + resolvers);
+            throw new IllegalArgumentException("Some readers are not resolvers: " + readers + ", " + resolvers);
         }
         if (!readers.containsAll(writers)) {
-            throw new RuntimeException("Some writers are not readers: " + writers + ", " + readers);
+            throw new IllegalArgumentException("Some writers are not readers: " + writers + ", " + readers);
         }
         if (!writers.containsAll(admins)) {
-            throw new RuntimeException("Some admins are not writers: " + admins + ", " + writers);
+            throw new IllegalArgumentException("Some admins are not writers: " + admins + ", " + writers);
         }
         for (String userId : readers) {
             users.put(userId, AccessLevel.READ);
@@ -84,7 +90,7 @@
     /**
      * Computes a new Permissions object based on delta.
      */
-    protected static Permissions applyDelta(Permissions corePermissions, AccessList delta) {
+    static Permissions applyDelta(Permissions corePermissions, AccessList delta) {
         Map<String, Map<String, Set<String>>> parsedPermissions = corePermissions.parse();
         for (String userId : delta.users.keySet()) {
             AccessLevel level = delta.users.get(userId);
diff --git a/syncbase/src/main/java/io/v/syncbase/BatchDatabase.java b/syncbase/src/main/java/io/v/syncbase/BatchDatabase.java
index e662b44..91e8892 100644
--- a/syncbase/src/main/java/io/v/syncbase/BatchDatabase.java
+++ b/syncbase/src/main/java/io/v/syncbase/BatchDatabase.java
@@ -11,17 +11,20 @@
  * {@code Database.beginBatch} for concurrency semantics.
  */
 public class BatchDatabase extends DatabaseHandle {
-    protected io.v.syncbase.core.BatchDatabase mCoreBatchDatabase;
+    private final io.v.syncbase.core.BatchDatabase mCoreBatchDatabase;
 
-    protected BatchDatabase(io.v.syncbase.core.BatchDatabase coreBatchDatabase) {
+    BatchDatabase(io.v.syncbase.core.BatchDatabase coreBatchDatabase) {
         super(coreBatchDatabase);
         mCoreBatchDatabase = coreBatchDatabase;
     }
 
+    /**
+     * @throws IllegalArgumentException if opts.withoutSyncgroup not false
+     */
     @Override
     public Collection collection(String name, CollectionOptions opts) throws VError {
         if (!opts.withoutSyncgroup) {
-            throw new RuntimeException("Cannot create syncgroup in a batch");
+            throw new IllegalArgumentException("Cannot create syncgroup in a batch");
         }
         Collection res = getCollection(new Id(Syncbase.getPersonalBlessingString(), name));
         res.createIfMissing();
diff --git a/syncbase/src/main/java/io/v/syncbase/Collection.java b/syncbase/src/main/java/io/v/syncbase/Collection.java
index 3757e95..9af0777 100644
--- a/syncbase/src/main/java/io/v/syncbase/Collection.java
+++ b/syncbase/src/main/java/io/v/syncbase/Collection.java
@@ -18,13 +18,13 @@
     private final DatabaseHandle mDatabaseHandle;
     private final Id mId;
 
-    protected Collection(io.v.syncbase.core.Collection coreCollection, DatabaseHandle databaseHandle) {
+    Collection(io.v.syncbase.core.Collection coreCollection, DatabaseHandle databaseHandle) {
         mCoreCollection = coreCollection;
         mDatabaseHandle = databaseHandle;
         mId = new Id(coreCollection.id());
     }
 
-    protected void createIfMissing() {
+    void createIfMissing() {
         try {
             mCoreCollection.create(Syncbase.defaultCollectionPerms());
         } catch (VError vError) {
@@ -45,10 +45,11 @@
     /**
      * Shortcut for {@code Database.getSyncgroup(collection.getId())}, helpful for the common case
      * of one syncgroup per collection.
+     * @throws IllegalArgumentException if this is collection on a batch database.
      */
     public Syncgroup getSyncgroup() {
         if (mDatabaseHandle instanceof BatchDatabase) {
-            throw new RuntimeException("Must not call getSyncgroup within batch");
+            throw new IllegalArgumentException("Must not call getSyncgroup within batch");
         }
         return ((Database) mDatabaseHandle).getSyncgroup(getId());
     }
diff --git a/syncbase/src/main/java/io/v/syncbase/Database.java b/syncbase/src/main/java/io/v/syncbase/Database.java
index 30822d5..0ef2804 100644
--- a/syncbase/src/main/java/io/v/syncbase/Database.java
+++ b/syncbase/src/main/java/io/v/syncbase/Database.java
@@ -26,14 +26,14 @@
     private final Object mSyncgroupInviteHandlersMu = new Object();
     private final Object mWatchChangeHandlersMu = new Object();
     private final Map<SyncgroupInviteHandler, Long> mSyncgroupInviteHandlers = new HashMap<>();
-    private Map<WatchChangeHandler, Runnable> mWatchChangeHandlers = new HashMap<>();
+    private final Map<WatchChangeHandler, Runnable> mWatchChangeHandlers = new HashMap<>();
 
-    protected Database(io.v.syncbase.core.Database coreDatabase) {
+    Database(io.v.syncbase.core.Database coreDatabase) {
         super(coreDatabase);
         mCoreDatabase = coreDatabase;
     }
 
-    protected void createIfMissing() throws VError {
+    void createIfMissing() throws VError {
         try {
             mCoreDatabase.create(Syncbase.defaultDatabasePerms());
         } catch (VError vError) {
@@ -71,17 +71,18 @@
      * @param name        name of the syncgroup
      * @param collections collections in the syncgroup
      * @param opts        options for syncgroup creation
+     * @throws IllegalArgumentException if no collections or collections don't all have same creator
      * @return the syncgroup
      */
     public Syncgroup syncgroup(String name, List<Collection> collections, SyncgroupOptions opts)
             throws VError {
         if (collections.isEmpty()) {
-            throw new RuntimeException("No collections specified");
+            throw new IllegalArgumentException("No collections specified");
         }
         Id id = new Id(collections.get(0).getId().getBlessing(), name);
         for (Collection collection : collections) {
             if (!collection.getId().getBlessing().equals(id.getBlessing())) {
-                throw new RuntimeException("Collections must all have the same creator");
+                throw new IllegalArgumentException("Collections must all have the same creator");
             }
         }
         Syncgroup syncgroup = new Syncgroup(mCoreDatabase.syncgroup(id.toCoreId()), this);
@@ -252,7 +253,7 @@
         // Note: This will be one of the last things we implement.
         // TODO(sadovsky): Maybe document how to read/write rejection metadata in the userdata
         // collection, for advanced users.
-        throw new RuntimeException("Not implemented");
+        throw new UnsupportedOperationException("Not implemented");
     }
 
     /**
@@ -370,13 +371,13 @@
         // TODO(sadovsky): Support specifying resumeMarker. Note, watch-from-resumeMarker may be
         // problematic in that we don't track the governing ACL for changes in the watch log.
         if (opts.resumeMarker != null && opts.resumeMarker.length != 0) {
-            throw new RuntimeException("Specifying resumeMarker is not yet supported");
+            throw new UnsupportedOperationException("Specifying resumeMarker is not yet supported");
         }
 
         mCoreDatabase.watch(null, ImmutableList.of(new CollectionRowPattern("%", "%", "%")),
                 new io.v.syncbase.core.Database.WatchPatternsCallbacks() {
                     private boolean mGotFirstBatch = false;
-                    private List<WatchChange> mBatch = new ArrayList<>();
+                    private final List<WatchChange> mBatch = new ArrayList<>();
 
                     @Override
                     public void onChange(io.v.syncbase.core.WatchChange coreWatchChange) {
@@ -404,7 +405,7 @@
             mWatchChangeHandlers.put(h, new Runnable() {
                 @Override
                 public void run() {
-                    throw new RuntimeException("Not implemented");
+                    throw new UnsupportedOperationException("Not implemented");
                 }
             });
         }
diff --git a/syncbase/src/main/java/io/v/syncbase/DatabaseHandle.java b/syncbase/src/main/java/io/v/syncbase/DatabaseHandle.java
index c413ee4..b12cec5 100644
--- a/syncbase/src/main/java/io/v/syncbase/DatabaseHandle.java
+++ b/syncbase/src/main/java/io/v/syncbase/DatabaseHandle.java
@@ -14,9 +14,9 @@
  * Represents a handle to a database, possibly in a batch.
  */
 public abstract class DatabaseHandle {
-    protected io.v.syncbase.core.DatabaseHandle mCoreDatabaseHandle;
+    private final io.v.syncbase.core.DatabaseHandle mCoreDatabaseHandle;
 
-    protected DatabaseHandle(io.v.syncbase.core.DatabaseHandle coreDatabaseHandle) {
+    DatabaseHandle(io.v.syncbase.core.DatabaseHandle coreDatabaseHandle) {
         mCoreDatabaseHandle = coreDatabaseHandle;
     }
 
diff --git a/syncbase/src/main/java/io/v/syncbase/Id.java b/syncbase/src/main/java/io/v/syncbase/Id.java
index 0e8cb2f..149c107 100644
--- a/syncbase/src/main/java/io/v/syncbase/Id.java
+++ b/syncbase/src/main/java/io/v/syncbase/Id.java
@@ -8,23 +8,26 @@
  * Uniquely identifies a database, collection, or syncgroup.
  */
 public class Id {
-    private io.v.syncbase.core.Id mId;
+    private final io.v.syncbase.core.Id mId;
 
-    protected Id(io.v.syncbase.core.Id id) {
+    Id(io.v.syncbase.core.Id id) {
         mId = id;
     }
 
-    protected Id(String blessing, String name) {
+    Id(String blessing, String name) {
         mId = new io.v.syncbase.core.Id(blessing, name);
     }
 
     // TODO(sadovsky): Replace encode and decode method implementations with calls to Cgo.
     private static final String SEPARATOR = ",";
 
+    /**
+     * @throws IllegalArgumentException if invalid encodedId
+     */
     public static Id decode(String encodedId) {
         String[] parts = encodedId.split(SEPARATOR);
         if (parts.length != 2) {
-            throw new RuntimeException("Invalid encoded id: " + encodedId);
+            throw new IllegalArgumentException("Invalid encoded id: " + encodedId);
         }
         return new Id(parts[0], parts[1]);
     }
@@ -33,11 +36,11 @@
         return mId.encode();
     }
 
-    protected io.v.syncbase.core.Id toCoreId() {
+    io.v.syncbase.core.Id toCoreId() {
         return mId;
     }
 
-    protected String getBlessing() {
+    String getBlessing() {
         return mId.blessing;
     }
 
diff --git a/syncbase/src/main/java/io/v/syncbase/Syncbase.java b/syncbase/src/main/java/io/v/syncbase/Syncbase.java
index d08e435..9df5fae 100644
--- a/syncbase/src/main/java/io/v/syncbase/Syncbase.java
+++ b/syncbase/src/main/java/io/v/syncbase/Syncbase.java
@@ -57,19 +57,19 @@
         // the arguments to login() are ignored.
         public boolean testLogin;
 
-        protected String getPublishSyncbaseName() {
+        String getPublishSyncbaseName() {
             if (disableSyncgroupPublishing) {
                 return null;
             }
             return mountPoints.get(0) + "cloud";
         }
 
-        protected String getCloudBlessingString() {
+        String getCloudBlessingString() {
             return "dev.v.io:u:" + adminUserId;
         }
     }
 
-    protected static Options sOpts;
+    static Options sOpts;
     private static Database sDatabase;
     private static final Object sScanMappingMu = new Object();
     private static final Map<ScanNeighborhoodForUsersCallback, Long> sScanMapping = new HashMap<>();
@@ -77,13 +77,13 @@
     // TODO(sadovsky): Maybe set DB_NAME to "db__" so that it is less likely to collide with
     // developer-specified names.
 
-    protected static final String
+    static final String
             TAG = "syncbase",
             DIR_NAME = "syncbase",
             DB_NAME = "db",
             USERDATA_SYNCGROUP_NAME = "userdata__";
 
-    protected static void enqueue(final Runnable r) {
+    private static void enqueue(final Runnable r) {
         // Note, we use Timer rather than Handler because the latter must be mocked out for tests,
         // which is rather annoying.
         //new Handler().post(r);
@@ -103,7 +103,7 @@
     /**
      * Sets the initial options. If the user is already logged in, Syncbase will be started.
      *
-     * @param opts
+     * @param opts initial options
      */
     public static void init(Options opts) throws VError {
         System.loadLibrary("syncbase");
@@ -148,7 +148,7 @@
      * Returns the currently logged in user.
      */
     public static User getLoggedInUser() {
-        throw new RuntimeException("Not implemented");
+        throw new UnsupportedOperationException("Not implemented");
     }
 
     public static abstract class LoginCallback {
@@ -174,10 +174,11 @@
      * @param authToken The OAuth token for the user to be logged in.
      * @param provider  The provider of the OAuth token.
      * @param cb        The callback to call when the login was done.
+     * @throws IllegalArgumentException if provider is not one of those listed above
      */
     public static void login(final String authToken, final String provider, final LoginCallback cb) {
         if (!(provider.equals(User.PROVIDER_GOOGLE) || provider.equals(User.PROVIDER_NONE))) {
-            throw new RuntimeException("Unsupported provider: " + provider);
+            throw new IllegalArgumentException("Unsupported provider: " + provider);
         }
 
         Syncbase.enqueue(new Runnable() {
@@ -186,6 +187,10 @@
                 try {
                     io.v.syncbase.internal.Service.Login(provider, authToken);
                     sDatabase = database();
+                    if (sDatabase==null) {
+                        cb.onError(new IllegalStateException("not logged in"));
+                        return;
+                    }
                     sDatabase.createIfMissing();
                     if (sOpts.disableUserdataSyncgroup) {
                         Database.CollectionOptions cxOpts = new DatabaseHandle.CollectionOptions();
@@ -201,7 +206,7 @@
                         // FIXME(sadovsky): Implement create-or-join (and watch) of userdata
                         // syncgroup. For the new JNI API, we'll need to add Go code for this,
                         // since Java can't make RPCs.
-                        cb.onError(new RuntimeException(
+                        cb.onError(new UnsupportedOperationException(
                                 "Synced userdata collection is not yet supported"));
                     }
                 } catch (VError vError) {
@@ -315,11 +320,11 @@
         return parts[parts.length - 1];
     }
 
-    protected static String getPersonalBlessingString() throws VError {
-        return Blessings.UserBlessingFromContext().toString();
+    static String getPersonalBlessingString() throws VError {
+        return Blessings.UserBlessingFromContext();
     }
 
-    protected static Permissions defaultDatabasePerms() throws VError {
+    static Permissions defaultDatabasePerms() throws VError {
         // TODO(sadovsky): Revisit these default perms, which were copied from the Todos app.
         Map anyone = ImmutableMap.of(Permissions.IN, ImmutableList.of("..."));
         Map selfAndCloud = selfAndCloud();
@@ -330,7 +335,7 @@
                 Permissions.Tags.ADMIN, selfAndCloud));
     }
 
-    protected static Permissions defaultCollectionPerms() throws VError {
+    static Permissions defaultCollectionPerms() throws VError {
         // TODO(sadovsky): Revisit these default perms, which were copied from the Todos app.
         Map selfAndCloud = selfAndCloud();
         return new Permissions(ImmutableMap.of(
@@ -339,7 +344,7 @@
                 Permissions.Tags.ADMIN, selfAndCloud));
     }
 
-    protected static Permissions defaultSyncgroupPerms() throws VError {
+    static Permissions defaultSyncgroupPerms() throws VError {
         // TODO(sadovsky): Revisit these default perms, which were copied from the Todos app.
         Map selfAndCloud = selfAndCloud();
         return new Permissions(ImmutableMap.of(
diff --git a/syncbase/src/main/java/io/v/syncbase/Syncgroup.java b/syncbase/src/main/java/io/v/syncbase/Syncgroup.java
index 031b3a6..2970cfc 100644
--- a/syncbase/src/main/java/io/v/syncbase/Syncgroup.java
+++ b/syncbase/src/main/java/io/v/syncbase/Syncgroup.java
@@ -8,7 +8,6 @@
 import java.util.Collections;
 import java.util.List;
 
-import io.v.syncbase.core.Permissions;
 import io.v.syncbase.core.SyncgroupMemberInfo;
 import io.v.syncbase.core.SyncgroupSpec;
 import io.v.syncbase.core.VError;
@@ -22,12 +21,12 @@
     private final Database mDatabase;
     private final io.v.syncbase.core.Syncgroup mCoreSyncgroup;
 
-    protected Syncgroup(io.v.syncbase.core.Syncgroup coreSyncgroup, Database database) {
+    Syncgroup(io.v.syncbase.core.Syncgroup coreSyncgroup, Database database) {
         mCoreSyncgroup = coreSyncgroup;
         mDatabase = database;
     }
 
-    protected void createIfMissing(List<Collection> collections) throws VError {
+    void createIfMissing(List<Collection> collections) throws VError {
         ArrayList<io.v.syncbase.core.Id> ids = new ArrayList<>();
         for (Collection cx : collections) {
             ids.add(cx.getId().toCoreId());
@@ -145,9 +144,8 @@
         } catch (VError vError) {
             throw new RuntimeException("getSpec failed", vError);
         }
-        Permissions newPermissions = AccessList.applyDelta(
+        versionedSyncgroupSpec.syncgroupSpec.permissions = AccessList.applyDelta(
                 versionedSyncgroupSpec.syncgroupSpec.permissions, delta);
-        versionedSyncgroupSpec.syncgroupSpec.permissions = newPermissions;
         mCoreSyncgroup.setSpec(versionedSyncgroupSpec);
         // TODO(sadovsky): There's a race here - it's possible for a collection to get destroyed
         // after getSpec() but before db.getCollection().
diff --git a/syncbase/src/main/java/io/v/syncbase/SyncgroupInvite.java b/syncbase/src/main/java/io/v/syncbase/SyncgroupInvite.java
index b9c61c3..cf1dda2 100644
--- a/syncbase/src/main/java/io/v/syncbase/SyncgroupInvite.java
+++ b/syncbase/src/main/java/io/v/syncbase/SyncgroupInvite.java
@@ -11,8 +11,8 @@
  * Represents an invitation to join a syncgroup.
  */
 public class SyncgroupInvite {
-    private Id mId;
-    private List<String> mInviterBlessingNames;
+    private final Id mId;
+    private final List<String> mInviterBlessingNames;
 
     protected SyncgroupInvite(Id id, List<String> inviterBlessingNames) {
         mId = id;
diff --git a/syncbase/src/main/java/io/v/syncbase/User.java b/syncbase/src/main/java/io/v/syncbase/User.java
index 2fa2f4f..b123caa 100644
--- a/syncbase/src/main/java/io/v/syncbase/User.java
+++ b/syncbase/src/main/java/io/v/syncbase/User.java
@@ -11,8 +11,8 @@
     public static final String PROVIDER_GOOGLE = "google";
     public static final String PROVIDER_NONE = "";
 
-    private String mAlias;
-    private String mProvider;
+    private final String mAlias;
+    private final String mProvider;
 
     public User(String alias) {
         mAlias = alias;
diff --git a/syncbase/src/main/java/io/v/syncbase/WatchChange.java b/syncbase/src/main/java/io/v/syncbase/WatchChange.java
index 27de4cb..435cde8 100644
--- a/syncbase/src/main/java/io/v/syncbase/WatchChange.java
+++ b/syncbase/src/main/java/io/v/syncbase/WatchChange.java
@@ -28,7 +28,7 @@
     private final boolean mFromSync;
     private final boolean mContinued;
 
-    protected WatchChange(io.v.syncbase.core.WatchChange change) {
+    WatchChange(io.v.syncbase.core.WatchChange change) {
         if (change.entityType == io.v.syncbase.core.WatchChange.EntityType.ROOT) {
             mEntityType = EntityType.ROOT;
         } else if (change.entityType == io.v.syncbase.core.WatchChange.EntityType.COLLECTION) {
diff --git a/syncbase/src/main/java/io/v/syncbase/core/BatchDatabase.java b/syncbase/src/main/java/io/v/syncbase/core/BatchDatabase.java
index 8c26394..5f01ba7 100644
--- a/syncbase/src/main/java/io/v/syncbase/core/BatchDatabase.java
+++ b/syncbase/src/main/java/io/v/syncbase/core/BatchDatabase.java
@@ -7,9 +7,9 @@
 import java.util.List;
 
 public class BatchDatabase extends DatabaseHandle {
-    protected String batchHandle;
+    private final String batchHandle;
 
-    protected BatchDatabase(Id id, String batchHandle) {
+    BatchDatabase(Id id, String batchHandle) {
         super(id);
         this.batchHandle = batchHandle;
     }
diff --git a/syncbase/src/main/java/io/v/syncbase/core/Collection.java b/syncbase/src/main/java/io/v/syncbase/core/Collection.java
index 09bf49b..be91fa4 100644
--- a/syncbase/src/main/java/io/v/syncbase/core/Collection.java
+++ b/syncbase/src/main/java/io/v/syncbase/core/Collection.java
@@ -10,11 +10,11 @@
 import io.v.v23.syncbase.RowRange;
 
 public class Collection {
-    protected String batchHandle;
-    private Id id;
-    protected String fullName;
+    private final String batchHandle;
+    private final Id id;
+    private final String fullName;
 
-    protected Collection(String parentFullName, Id id, String batchHandle) {
+    Collection(String parentFullName, Id id, String batchHandle) {
         this.batchHandle = batchHandle;
         this.id = id;
         this.fullName = Util.NamingJoin(Arrays.asList(parentFullName, id.encode()));
diff --git a/syncbase/src/main/java/io/v/syncbase/core/Database.java b/syncbase/src/main/java/io/v/syncbase/core/Database.java
index de536de..f9fa510 100644
--- a/syncbase/src/main/java/io/v/syncbase/core/Database.java
+++ b/syncbase/src/main/java/io/v/syncbase/core/Database.java
@@ -7,8 +7,7 @@
 import java.util.List;
 
 public class Database extends DatabaseHandle {
-
-    protected Database(Id id) {
+    Database(Id id) {
         super(id);
     }
 
diff --git a/syncbase/src/main/java/io/v/syncbase/core/DatabaseHandle.java b/syncbase/src/main/java/io/v/syncbase/core/DatabaseHandle.java
index 2193a3a..bec2dc7 100644
--- a/syncbase/src/main/java/io/v/syncbase/core/DatabaseHandle.java
+++ b/syncbase/src/main/java/io/v/syncbase/core/DatabaseHandle.java
@@ -7,10 +7,10 @@
 import java.util.List;
 
 public class DatabaseHandle {
-    protected Id id;
-    protected String fullName;
+    final Id id;
+    final String fullName;
 
-    protected DatabaseHandle(Id id) {
+    DatabaseHandle(Id id) {
         this.id = id;
         fullName = id.encode();
     }
diff --git a/syncbase/src/main/java/io/v/syncbase/core/Permissions.java b/syncbase/src/main/java/io/v/syncbase/core/Permissions.java
index f043075..89629cc 100644
--- a/syncbase/src/main/java/io/v/syncbase/core/Permissions.java
+++ b/syncbase/src/main/java/io/v/syncbase/core/Permissions.java
@@ -53,7 +53,7 @@
      * }
      * </pre>
      *
-     * @return
+     * @return map describing the permissions
      */
     public Map<String, Map<String, Set<String>>> parse() {
         Map<String, Map<String, Set<String>>> permissions = new HashMap<>();
@@ -66,7 +66,7 @@
             }
         } catch (JSONException e) {
             // TODO(razvanm): Should we do something else? Logging?
-            throw new RuntimeException("Permissions parsing failure", e);
+            throw new IllegalArgumentException("Permissions parsing failure", e);
         }
 
         return permissions;
diff --git a/syncbase/src/main/java/io/v/syncbase/core/Row.java b/syncbase/src/main/java/io/v/syncbase/core/Row.java
index d1628ad..22542db 100644
--- a/syncbase/src/main/java/io/v/syncbase/core/Row.java
+++ b/syncbase/src/main/java/io/v/syncbase/core/Row.java
@@ -9,11 +9,11 @@
 import io.v.syncbase.internal.Util;
 
 public class Row {
-    private String batchHandle;
-    private String key;
-    private String fullName;
+    private final String batchHandle;
+    private final String key;
+    private final String fullName;
 
-    protected Row(String parentFullName, String key, String batchHandle) {
+    Row(String parentFullName, String key, String batchHandle) {
         this.batchHandle = batchHandle;
         this.key = key;
         this.fullName = Util.NamingJoin(Arrays.asList(parentFullName, key));
diff --git a/syncbase/src/main/java/io/v/syncbase/core/Syncgroup.java b/syncbase/src/main/java/io/v/syncbase/core/Syncgroup.java
index eb63a87..dd54924 100644
--- a/syncbase/src/main/java/io/v/syncbase/core/Syncgroup.java
+++ b/syncbase/src/main/java/io/v/syncbase/core/Syncgroup.java
@@ -8,10 +8,10 @@
 import java.util.Map;
 
 public class Syncgroup {
-    private String dbFullName;
-    private Id id;
+    private final String dbFullName;
+    private final Id id;
 
-    protected Syncgroup(Database database, Id id) {
+    Syncgroup(Database database, Id id) {
         dbFullName = database.fullName;
         this.id = id;
     }
diff --git a/syncbase/src/main/java/io/v/syncbase/core/VError.java b/syncbase/src/main/java/io/v/syncbase/core/VError.java
index 2e80bf7..37d6938 100644
--- a/syncbase/src/main/java/io/v/syncbase/core/VError.java
+++ b/syncbase/src/main/java/io/v/syncbase/core/VError.java
@@ -4,6 +4,8 @@
 
 package io.v.syncbase.core;
 
+import java.util.Arrays;
+
 import io.v.v23.verror.VException;
 
 public class VError extends Exception {
@@ -16,14 +18,15 @@
     public String message;
     public String stack;
 
-    public VError() {
+    /** Called in JNI */
+    private VError() {
     }
 
     public VError(VException e) {
         this.id = e.getID();
         this.actionCode = e.getAction().getValue();
         this.message = e.getMessage();
-        this.stack = e.getStackTrace().toString();
+        this.stack = Arrays.toString(e.getStackTrace());
     }
 
     public String toString() {
diff --git a/syncbase/src/test/java/io/v/syncbase/SyncbaseTest.java b/syncbase/src/test/java/io/v/syncbase/SyncbaseTest.java
index c0dcb42..861cc8f 100644
--- a/syncbase/src/test/java/io/v/syncbase/SyncbaseTest.java
+++ b/syncbase/src/test/java/io/v/syncbase/SyncbaseTest.java
@@ -29,7 +29,7 @@
 
 public class SyncbaseTest {
     @Rule
-    public TemporaryFolder folder = new TemporaryFolder();
+    public final TemporaryFolder folder = new TemporaryFolder();
 
     // To run these tests from Android Studio, add the following VM option to the default JUnit
     // build configuration, via Run > Edit Configurations... > Defaults > JUnit > VM options:
@@ -185,7 +185,7 @@
                 // TODO(razvanm): Check the entire contents of each change.
                 // 1st change: the root entity.
                 assertTrue(values.hasNext());
-                WatchChange watchChange = (WatchChange)values.next();
+                WatchChange watchChange = values.next();
                 assertEquals(WatchChange.EntityType.ROOT, watchChange.getEntityType());
                 assertEquals(WatchChange.ChangeType.PUT, watchChange.getChangeType());
                 // 2nd change: the collection entity for the "c" collection.
@@ -215,7 +215,7 @@
             @Override
             public void onChangeBatch(Iterator<WatchChange> changes) {
                 assertTrue(changes.hasNext());
-                WatchChange watchChange = (WatchChange)changes.next();
+                WatchChange watchChange = changes.next();
                 assertEquals(WatchChange.ChangeType.DELETE, watchChange.getChangeType());
                 // TODO(razvanm): Uncomment after the POJO start working.
                 //assertEquals(1, watchChange.getValue());
diff --git a/syncbase/src/test/java/io/v/syncbase/core/BatchDatabaseTest.java b/syncbase/src/test/java/io/v/syncbase/core/BatchDatabaseTest.java
index dee7df3..d026ad3 100644
--- a/syncbase/src/test/java/io/v/syncbase/core/BatchDatabaseTest.java
+++ b/syncbase/src/test/java/io/v/syncbase/core/BatchDatabaseTest.java
@@ -16,7 +16,7 @@
 
 public class BatchDatabaseTest {
     @ClassRule
-    public static TemporaryFolder folder = new TemporaryFolder();
+    public static final TemporaryFolder folder = new TemporaryFolder();
 
     @BeforeClass
     public static void setUp() throws Exception {
diff --git a/syncbase/src/test/java/io/v/syncbase/core/CollectionTest.java b/syncbase/src/test/java/io/v/syncbase/core/CollectionTest.java
index 5e59934..babecd4 100644
--- a/syncbase/src/test/java/io/v/syncbase/core/CollectionTest.java
+++ b/syncbase/src/test/java/io/v/syncbase/core/CollectionTest.java
@@ -22,7 +22,7 @@
 
 public class CollectionTest {
     @ClassRule
-    public static TemporaryFolder folder = new TemporaryFolder();
+    public static final TemporaryFolder folder = new TemporaryFolder();
 
     @BeforeClass
     public static void setUp() throws Exception {
diff --git a/syncbase/src/test/java/io/v/syncbase/core/DatabaseHandleTest.java b/syncbase/src/test/java/io/v/syncbase/core/DatabaseHandleTest.java
index 23ba5ee..7b94b7b 100644
--- a/syncbase/src/test/java/io/v/syncbase/core/DatabaseHandleTest.java
+++ b/syncbase/src/test/java/io/v/syncbase/core/DatabaseHandleTest.java
@@ -20,7 +20,7 @@
 
 public class DatabaseHandleTest {
     @ClassRule
-    public static TemporaryFolder folder = new TemporaryFolder();
+    public static final TemporaryFolder folder = new TemporaryFolder();
 
     @BeforeClass
     public static void setUp() throws Exception {
diff --git a/syncbase/src/test/java/io/v/syncbase/core/DatabaseTest.java b/syncbase/src/test/java/io/v/syncbase/core/DatabaseTest.java
index 7582646..9bf9139 100644
--- a/syncbase/src/test/java/io/v/syncbase/core/DatabaseTest.java
+++ b/syncbase/src/test/java/io/v/syncbase/core/DatabaseTest.java
@@ -22,7 +22,7 @@
 
 public class DatabaseTest {
     @ClassRule
-    public static TemporaryFolder folder = new TemporaryFolder();
+    public static final TemporaryFolder folder = new TemporaryFolder();
 
     @BeforeClass
     public static void setUp() throws Exception {
diff --git a/syncbase/src/test/java/io/v/syncbase/core/RowTest.java b/syncbase/src/test/java/io/v/syncbase/core/RowTest.java
index 059c1b7..4e6ff05 100644
--- a/syncbase/src/test/java/io/v/syncbase/core/RowTest.java
+++ b/syncbase/src/test/java/io/v/syncbase/core/RowTest.java
@@ -19,7 +19,7 @@
 
 public class RowTest {
     @ClassRule
-    public static TemporaryFolder folder = new TemporaryFolder();
+    public static final TemporaryFolder folder = new TemporaryFolder();
 
     @BeforeClass
     public static void setUp() throws Exception {
diff --git a/syncbase/src/test/java/io/v/syncbase/core/ServiceTest.java b/syncbase/src/test/java/io/v/syncbase/core/ServiceTest.java
index 8da1fe4..1ebabd3 100644
--- a/syncbase/src/test/java/io/v/syncbase/core/ServiceTest.java
+++ b/syncbase/src/test/java/io/v/syncbase/core/ServiceTest.java
@@ -18,7 +18,7 @@
 
 public class ServiceTest {
     @ClassRule
-    public static TemporaryFolder folder = new TemporaryFolder();
+    public static final TemporaryFolder folder = new TemporaryFolder();
 
     @BeforeClass
     public static void setUp() throws Exception {
diff --git a/syncbase/src/test/java/io/v/syncbase/core/SyncgroupTest.java b/syncbase/src/test/java/io/v/syncbase/core/SyncgroupTest.java
index e6aeae5..38aec09 100644
--- a/syncbase/src/test/java/io/v/syncbase/core/SyncgroupTest.java
+++ b/syncbase/src/test/java/io/v/syncbase/core/SyncgroupTest.java
@@ -11,13 +11,13 @@
 import org.junit.rules.TemporaryFolder;
 
 import java.util.ArrayList;
-import java.util.Arrays;
 import java.util.List;
 import java.util.Map;
 
 import static io.v.syncbase.core.TestConstants.anyCollectionPermissions;
 import static io.v.syncbase.core.TestConstants.anyDbPermissions;
 import static io.v.syncbase.core.TestConstants.anySyncgroupPermissions;
+import static java.util.Collections.singletonList;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertTrue;
@@ -25,7 +25,7 @@
 
 public class SyncgroupTest {
     @ClassRule
-    public static TemporaryFolder folder = new TemporaryFolder();
+    public static final TemporaryFolder folder = new TemporaryFolder();
 
     @BeforeClass
     public static void setUp() throws Exception {
@@ -49,7 +49,7 @@
             db.create(anyDbPermissions());
             db.collection(collectionId).create(anyCollectionPermissions());
             SyncgroupSpec spec = new SyncgroupSpec();
-            spec.collections = Arrays.asList(collectionId);
+            spec.collections = singletonList(collectionId);
             spec.permissions = anySyncgroupPermissions();
             SyncgroupMemberInfo info = new SyncgroupMemberInfo();
             // TODO(razvanm): Pick some meaningful values.
@@ -133,7 +133,6 @@
     @Test
     public void leave() {
         Id dbId = new Id("idp:a:angrybirds", "core_leave_syncgroups");
-        String dbName = dbId.encode();
         Id sgId = new Id("idp:u:alice", "syncgroup");
         boolean exceptionThrown = false;
         try {
@@ -150,7 +149,6 @@
     @Test
     public void eject() {
         Id dbId = new Id("idp:a:angrybirds", "core_eject_from_syncgroup");
-        String dbName = dbId.encode();
         Id sgId = new Id("idp:u:alice", "syncgroup");
         boolean exceptionThrown = false;
         try {
diff --git a/syncbase/src/test/java/io/v/syncbase/internal/BlessingsTest.java b/syncbase/src/test/java/io/v/syncbase/internal/BlessingsTest.java
index 7b1d768..d9eab60 100644
--- a/syncbase/src/test/java/io/v/syncbase/internal/BlessingsTest.java
+++ b/syncbase/src/test/java/io/v/syncbase/internal/BlessingsTest.java
@@ -19,7 +19,7 @@
 
 public class BlessingsTest {
     @ClassRule
-    public static TemporaryFolder folder = new TemporaryFolder();
+    public static final TemporaryFolder folder = new TemporaryFolder();
 
     @BeforeClass
     public static void setUp() throws Exception {
diff --git a/syncbase/src/test/java/io/v/syncbase/internal/CollectionTest.java b/syncbase/src/test/java/io/v/syncbase/internal/CollectionTest.java
index 6adaf19..9fd1d7e 100644
--- a/syncbase/src/test/java/io/v/syncbase/internal/CollectionTest.java
+++ b/syncbase/src/test/java/io/v/syncbase/internal/CollectionTest.java
@@ -32,7 +32,7 @@
 
 public class CollectionTest {
     @ClassRule
-    public static TemporaryFolder folder = new TemporaryFolder();
+    public static final TemporaryFolder folder = new TemporaryFolder();
 
     @BeforeClass
     public static void setUp() throws Exception {
diff --git a/syncbase/src/test/java/io/v/syncbase/internal/DatabaseTest.java b/syncbase/src/test/java/io/v/syncbase/internal/DatabaseTest.java
index 0a47727..0139508 100644
--- a/syncbase/src/test/java/io/v/syncbase/internal/DatabaseTest.java
+++ b/syncbase/src/test/java/io/v/syncbase/internal/DatabaseTest.java
@@ -32,6 +32,7 @@
 import static io.v.syncbase.core.TestConstants.anyCollectionPermissions;
 import static io.v.syncbase.core.TestConstants.anyDbPermissions;
 import static io.v.syncbase.core.TestConstants.anySyncgroupPermissions;
+import static java.util.Collections.singletonList;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNotEquals;
@@ -41,7 +42,7 @@
 
 public class DatabaseTest {
     @ClassRule
-    public static TemporaryFolder folder = new TemporaryFolder();
+    public static final TemporaryFolder folder = new TemporaryFolder();
 
     @BeforeClass
     public static void setUp() throws Exception {
@@ -202,7 +203,7 @@
             Collection.Create(collectionName, batchHandle, anyCollectionPermissions());
             Database.Commit(dbName, batchHandle);
             SyncgroupSpec spec = new SyncgroupSpec();
-            spec.collections = Arrays.asList(collectionId);
+            spec.collections = singletonList(collectionId);
             spec.permissions = anySyncgroupPermissions();
             SyncgroupMemberInfo info = new SyncgroupMemberInfo();
             // TODO(razvanm): Pick some meaningful values.
@@ -283,7 +284,7 @@
         Id sgId = new Id("idp:u:alice", "syncgroup");
         boolean exceptionThrown = false;
         try {
-            SyncgroupSpec spec = Database.JoinSyncgroup(
+            Database.JoinSyncgroup(
                     dbName, "", new ArrayList<String>(), sgId, new SyncgroupMemberInfo());
         } catch (VError vError) {
             assertEquals("v.io/v23/verror.NoExist", vError.id);
@@ -336,7 +337,7 @@
             Database.Create(dbName, anyDbPermissions());
             String batchHandle = Database.BeginBatch(dbName, null);
             byte[] marker = Database.GetResumeMarker(dbName, batchHandle);
-            List<CollectionRowPattern> patterns = Arrays.asList(new CollectionRowPattern());
+            List<CollectionRowPattern> patterns = singletonList(new CollectionRowPattern());
             Database.WatchPatterns(dbName, marker, patterns, new Database.WatchPatternsCallbacks() {
                 @Override
                 public void onChange(WatchChange watchChange) {
@@ -377,7 +378,7 @@
             CollectionRowPattern pattern = new CollectionRowPattern();
             pattern.collectionBlessing = collectionId.blessing;
             pattern.collectionName = collectionId.name;
-            List<CollectionRowPattern> patterns = Arrays.asList(pattern);
+            List<CollectionRowPattern> patterns = singletonList(pattern);
             Database.WatchPatterns(dbName, new byte[]{}, patterns,
                     new Database.WatchPatternsCallbacks() {
                 @Override
@@ -403,7 +404,7 @@
         try {
             done.get(1, TimeUnit.SECONDS);
         } catch (InterruptedException|ExecutionException|TimeoutException e) {
-            fail("Timeout waiting for onChange");
+            fail("Timeout waiting for onChange: " + e);
         }
     }
 }
diff --git a/syncbase/src/test/java/io/v/syncbase/internal/RowTest.java b/syncbase/src/test/java/io/v/syncbase/internal/RowTest.java
index 680fc88..5bc7488 100644
--- a/syncbase/src/test/java/io/v/syncbase/internal/RowTest.java
+++ b/syncbase/src/test/java/io/v/syncbase/internal/RowTest.java
@@ -24,7 +24,7 @@
 
 public class RowTest {
     @ClassRule
-    public static TemporaryFolder folder = new TemporaryFolder();
+    public static final TemporaryFolder folder = new TemporaryFolder();
 
     @BeforeClass
     public static void setUp() throws Exception {
diff --git a/syncbase/src/test/java/io/v/syncbase/internal/ServiceTest.java b/syncbase/src/test/java/io/v/syncbase/internal/ServiceTest.java
index 10c5a51..2799fde 100644
--- a/syncbase/src/test/java/io/v/syncbase/internal/ServiceTest.java
+++ b/syncbase/src/test/java/io/v/syncbase/internal/ServiceTest.java
@@ -20,7 +20,7 @@
 
 public class ServiceTest {
     @ClassRule
-    public static TemporaryFolder folder = new TemporaryFolder();
+    public static final TemporaryFolder folder = new TemporaryFolder();
 
     @BeforeClass
     public static void setUp() throws Exception {