Fixing more Storage code style issues (#594)

Author: schmidt-sebastian

firebase-firestore/src/main/java/com/google/firebase/firestore/FirebaseFirestore.java

@@ -261,15 +261,15 @@ public Query collectionGroup(@NonNull String collectionId) {
    * @param executor The executor to run the transaction callback on.
    * @return The task returned from the updateFunction.
    */
-  private <TResult> Task<TResult> runTransaction(
-      Transaction.Function<TResult> updateFunction, Executor executor) {
+  private <ResultT> Task<ResultT> runTransaction(
+      Transaction.Function<ResultT> updateFunction, Executor executor) {
     ensureClientConfigured();
 
     // We wrap the function they provide in order to
     // 1. Use internal implementation classes for Transaction,
     // 2. Convert exceptions they throw into Tasks, and
     // 3. Run the user callback on the user queue.
-    Function<com.google.firebase.firestore.core.Transaction, Task<TResult>> wrappedUpdateFunction =
+    Function<com.google.firebase.firestore.core.Transaction, Task<ResultT>> wrappedUpdateFunction =
         internalTransaction ->
             Tasks.call(
                 executor,

firebase-storage/src/main/java/com/google/firebase/storage/StreamDownloadTask.java

@@ -38,24 +38,24 @@
 public class StreamDownloadTask extends StorageTask<StreamDownloadTask.TaskSnapshot> {
   static final long PREFERRED_CHUNK_SIZE = 256 * 1024;
   private static final String TAG = "StreamDownloadTask";
-  private StorageReference mStorageRef;
-  private ExponentialBackoffSender mSender;
-  private volatile Exception mException = null;
-  private volatile int mResultCode = 0;
-  private StreamProcessor mProcessor;
-
-  private long mTotalBytes = -1;
-  private long mBytesDownloaded;
-  private long mBytesDownloadedSnapped;
-  private InputStream mInputStream;
-  private NetworkRequest mRequest;
-  private String mETagVerification;
+  private StorageReference storageRef;
+  private ExponentialBackoffSender sender;
+  private volatile Exception exception = null;
+  private volatile int resultCode = 0;
+  private StreamProcessor processor;
+
+  private long totalBytes = -1;
+  private long bytesDownloaded;
+  private long bytesDownloadedSnapped;
+  private InputStream inputStream;
+  private NetworkRequest request;
+  private String eTagVerification;
 
   /*package*/ StreamDownloadTask(@NonNull StorageReference storageRef) {
-    mStorageRef = storageRef;
+    this.storageRef = storageRef;
 
-    FirebaseStorage storage = mStorageRef.getStorage();
-    mSender =
+    FirebaseStorage storage = this.storageRef.getStorage();
+    sender =
         new ExponentialBackoffSender(
             storage.getApp().getApplicationContext(),
             storage.getAuthProvider(),
@@ -71,16 +71,16 @@ public class StreamDownloadTask extends StorageTask<StreamDownloadTask.TaskSnaps
    */
   /*package*/ StreamDownloadTask setStreamProcessor(@NonNull StreamProcessor processor) {
     Preconditions.checkNotNull(processor);
-    Preconditions.checkState(mProcessor == null);
-    this.mProcessor = processor;
+    Preconditions.checkState(this.processor == null);
+    this.processor = processor;
     return this;
   }
 
   /** @return the target of the download. */
   @Override
   @NonNull
   /*package*/ StorageReference getStorage() {
-    return mStorageRef;
+    return storageRef;
   }
 
   /**
@@ -89,16 +89,16 @@ public class StreamDownloadTask extends StorageTask<StreamDownloadTask.TaskSnaps
    */
   @SuppressWarnings("unused")
   /*package*/ long getTotalBytes() {
-    return mTotalBytes;
+    return totalBytes;
   }
 
   void recordDownloadedBytes(long bytesDownloaded) {
-    mBytesDownloaded += bytesDownloaded;
-    if (mBytesDownloadedSnapped + PREFERRED_CHUNK_SIZE <= mBytesDownloaded) {
+    this.bytesDownloaded += bytesDownloaded;
+    if (bytesDownloadedSnapped + PREFERRED_CHUNK_SIZE <= this.bytesDownloaded) {
       if (getInternalState() == INTERNAL_STATE_IN_PROGRESS) {
         tryChangeState(INTERNAL_STATE_IN_PROGRESS, false);
       } else {
-        mBytesDownloadedSnapped = mBytesDownloaded;
+        bytesDownloadedSnapped = this.bytesDownloaded;
       }
     }
   }
@@ -112,37 +112,37 @@ protected void schedule() {
 
   @SuppressWarnings({"JavaDoc", "ThrowableResultOfMethodCallIgnored"})
   private InputStream createDownloadStream() throws Exception {
-    mSender.reset();
+    sender.reset();
 
-    if (mRequest != null) {
-      mRequest.performRequestEnd();
+    if (request != null) {
+      request.performRequestEnd();
     }
 
-    mRequest =
-        new GetNetworkRequest(mStorageRef.getStorageUri(), mStorageRef.getApp(), mBytesDownloaded);
+    request =
+        new GetNetworkRequest(storageRef.getStorageUri(), storageRef.getApp(), bytesDownloaded);
 
-    mSender.sendWithExponentialBackoff(mRequest, false);
-    mResultCode = mRequest.getResultCode();
-    mException = mRequest.getException() != null ? mRequest.getException() : mException;
+    sender.sendWithExponentialBackoff(request, false);
+    resultCode = request.getResultCode();
+    exception = request.getException() != null ? request.getException() : exception;
     boolean success =
-        isValidHttpResponseCode(mResultCode)
-            && mException == null
+        isValidHttpResponseCode(resultCode)
+            && exception == null
             && getInternalState() == INTERNAL_STATE_IN_PROGRESS;
 
     if (success) {
-      String newEtag = mRequest.getResultString("ETag");
+      String newEtag = request.getResultString("ETag");
       if (!TextUtils.isEmpty(newEtag)
-          && mETagVerification != null
-          && !mETagVerification.equals(newEtag)) {
-        mResultCode = HttpURLConnection.HTTP_CONFLICT;
+          && eTagVerification != null
+          && !eTagVerification.equals(newEtag)) {
+        resultCode = HttpURLConnection.HTTP_CONFLICT;
         throw new IOException("The ETag on the server changed.");
       }
 
-      mETagVerification = newEtag;
-      if (mTotalBytes == -1) {
-        mTotalBytes = mRequest.getResultingContentLength();
+      eTagVerification = newEtag;
+      if (totalBytes == -1) {
+        totalBytes = request.getResultingContentLength();
       }
-      return mRequest.getStream();
+      return request.getStream();
     } else {
       throw new IOException("Could not open resulting stream.");
     }
@@ -152,7 +152,7 @@ private InputStream createDownloadStream() throws Exception {
   @SuppressWarnings({"JavaDoc", "ThrowableResultOfMethodCallIgnored"})
   @Override
   /*package*/ void run() {
-    if (mException != null) {
+    if (exception != null) {
       tryChangeState(INTERNAL_STATE_FAILURE, false);
       return;
     }
@@ -170,31 +170,31 @@ public InputStream call() throws Exception {
               }
             },
             StreamDownloadTask.this);
-    mInputStream = new BufferedInputStream(streamWrapper);
+    inputStream = new BufferedInputStream(streamWrapper);
 
     try {
       // Open stream to fetch initial state.
       streamWrapper.ensureStream();
 
-      if (mProcessor != null) {
+      if (processor != null) {
         try {
-          mProcessor.doInBackground(snapState(), mInputStream);
+          processor.doInBackground(snapState(), inputStream);
         } catch (Exception e) {
           Log.w(TAG, "Exception occurred calling doInBackground.", e);
-          mException = e;
+          exception = e;
         }
       }
     } catch (IOException e) {
       Log.d(TAG, "Initial opening of Stream failed", e);
-      mException = e;
+      exception = e;
     }
 
-    if (mInputStream == null) {
-      mRequest.performRequestEnd();
-      mRequest = null;
+    if (inputStream == null) {
+      request.performRequestEnd();
+      request = null;
     }
 
-    boolean success = mException == null && getInternalState() == INTERNAL_STATE_IN_PROGRESS;
+    boolean success = exception == null && getInternalState() == INTERNAL_STATE_IN_PROGRESS;
 
     if (success) {
       tryChangeState(INTERNAL_STATE_IN_PROGRESS, false);
@@ -228,19 +228,18 @@ public boolean pause() {
   @Override
   TaskSnapshot snapStateImpl() {
     return new TaskSnapshot(
-        StorageException.fromExceptionAndHttpCode(mException, mResultCode),
-        mBytesDownloadedSnapped);
+        StorageException.fromExceptionAndHttpCode(exception, resultCode), bytesDownloadedSnapped);
   }
 
   @Override
   protected void onCanceled() {
-    mSender.cancel();
-    mException = StorageException.fromErrorStatus(Status.RESULT_CANCELED);
+    sender.cancel();
+    exception = StorageException.fromErrorStatus(Status.RESULT_CANCELED);
   }
 
   @Override
   protected void onProgress() {
-    mBytesDownloadedSnapped = mBytesDownloaded;
+    bytesDownloadedSnapped = bytesDownloaded;
   }
 
   private boolean isValidHttpResponseCode(int code) {
@@ -377,9 +376,9 @@ public void close() throws IOException {
         mWrappedStream.close();
       }
       mStreamClosed = true;
-      if (mParentTask != null && mParentTask.mRequest != null) {
-        mParentTask.mRequest.performRequestEnd();
-        mParentTask.mRequest = null;
+      if (mParentTask != null && mParentTask.request != null) {
+        mParentTask.request.performRequestEnd();
+        mParentTask.request = null;
       }
 
       checkCancel();
@@ -506,7 +505,7 @@ public long getTotalByteCount() {
      */
     @PublicApi
     public InputStream getStream() {
-      return StreamDownloadTask.this.mInputStream;
+      return StreamDownloadTask.this.inputStream;
     }
   }
 }

firebase-storage/src/main/java/com/google/firebase/storage/network/NetworkRequest.java

@@ -137,6 +137,16 @@ public static String getPathWithoutBucket(@NonNull Uri gsUri) {
     return path;
   }
 
+  /**
+   * Returns the path of the object but excludes the bucket name
+   *
+   * @return the path in string form.
+   */
+  @Nullable
+  public String getPathWithoutBucket() {
+    return getPathWithoutBucket(mGsUri);
+  }
+
   @NonNull
   protected abstract String getAction();
 
@@ -150,16 +160,6 @@ protected String getURL() {
     return getDefaultURL(mGsUri);
   }
 
-  /**
-   * Returns the path of the object but excludes the bucket name
-   *
-   * @return the path in string form.
-   */
-  @Nullable
-  public String getPathWithoutBucket() {
-    return getPathWithoutBucket(mGsUri);
-  }
-
   /**
    * Can be overridden to return a JSONObject to populate the request body.
    *
@@ -418,22 +418,6 @@ private void parseResponse(@NonNull HttpURLConnection conn) throws IOException {
     }
   }
 
-  private void processResponseStream() throws IOException {
-    if (isResultSuccess()) {
-      parseSuccessulResponse(resultInputStream);
-    } else {
-      parseErrorResponse(resultInputStream);
-    }
-  }
-
-  protected void parseSuccessulResponse(@Nullable InputStream resultStream) throws IOException {
-    parseResponse(resultStream);
-  }
-
-  protected void parseErrorResponse(@Nullable InputStream resultStream) throws IOException {
-    parseResponse(resultStream);
-  }
-
   @SuppressWarnings("TryFinallyCanBeTryWithResources")
   private void parseResponse(@Nullable InputStream resultStream) throws IOException {
     StringBuilder sb = new StringBuilder();
@@ -455,6 +439,22 @@ private void parseResponse(@Nullable InputStream resultStream) throws IOExceptio
     }
   }
 
+  private void processResponseStream() throws IOException {
+    if (isResultSuccess()) {
+      parseSuccessulResponse(resultInputStream);
+    } else {
+      parseErrorResponse(resultInputStream);
+    }
+  }
+
+  protected void parseSuccessulResponse(@Nullable InputStream resultStream) throws IOException {
+    parseResponse(resultStream);
+  }
+
+  protected void parseErrorResponse(@Nullable InputStream resultStream) throws IOException {
+    parseResponse(resultStream);
+  }
+
   @Nullable
   public String getRawResult() {
     return rawStringResponse;

firebase-storage/src/test/java/com/google/firebase/storage/DependencyTest.java

@@ -73,11 +73,9 @@ public class DependencyTest {
    * If this test fails, its because you added a new method/overload to Task and you need to let
    * someone in Firebase Storage know. Otherwise users will see NotImplementedException on these new
    * methods for Storage Tasks. Please contact benwu@ for more info.
-   *
-   * @throws Exception
    */
   @Test
-  public void catchNewTaskMethods() throws Exception {
+  public void catchNewTaskMethods() {
     StringBuilder builder = new StringBuilder();
 
     try {

firebase-storage/src/test/java/com/google/firebase/storage/RetryRule.java

@@ -20,12 +20,13 @@
 
 /** Retries tests that fail due to the test harnesses' unpredictable threading behavior. */
 public class RetryRule implements TestRule {
-  private int retryCount;
+  private final int retryCount;
 
   public RetryRule(int retryCount) {
     this.retryCount = retryCount;
   }
 
+  @Override
   public Statement apply(final Statement base, Description description) {
     return new Statement() {
       @Override

firebase-storage/src/test/java/com/google/firebase/storage/TestUtil.java

@@ -111,7 +111,7 @@ private static void verifyTaskStateChanges(@Nullable InputStream inputStream, St
         } else {
           if (!originalLine.equals(newLine)) {
             System.err.println("Original:");
-            System.err.println(baselineContents.toString());
+            System.err.println(baselineContents);
             System.err.println("New:");
             System.err.println(contents);
           }