From 7ac7ede663e3199e8fc8881bacb8a59a146be5d6 Mon Sep 17 00:00:00 2001
From: Sebastian Schmidt <mrschmidt@google.com>
Date: Wed, 10 Jul 2019 08:47:09 -0700
Subject: [PATCH] Fixing more Storage code style issues

---
 .../firebase/firestore/FirebaseFirestore.java |   6 +-
 .../firebase/storage/StreamDownloadTask.java  | 117 +++++++++---------
 .../storage/network/NetworkRequest.java       |  52 ++++----
 .../firebase/storage/DependencyTest.java      |   4 +-
 .../google/firebase/storage/RetryRule.java    |   3 +-
 .../com/google/firebase/storage/TestUtil.java |   2 +-
 .../network/MockConnectionFactory.java        |  20 +--
 7 files changed, 101 insertions(+), 103 deletions(-)

diff --git a/firebase-firestore/src/main/java/com/google/firebase/firestore/FirebaseFirestore.java b/firebase-firestore/src/main/java/com/google/firebase/firestore/FirebaseFirestore.java
index 1d8fb8c625c..d6348336a6e 100644
--- a/firebase-firestore/src/main/java/com/google/firebase/firestore/FirebaseFirestore.java
+++ b/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,
diff --git a/firebase-storage/src/main/java/com/google/firebase/storage/StreamDownloadTask.java b/firebase-storage/src/main/java/com/google/firebase/storage/StreamDownloadTask.java
index 4eb7fc18ee6..a617a9b7315 100644
--- a/firebase-storage/src/main/java/com/google/firebase/storage/StreamDownloadTask.java
+++ b/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,8 +71,8 @@ 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;
   }
 
@@ -80,7 +80,7 @@ public class StreamDownloadTask extends StorageTask<StreamDownloadTask.TaskSnaps
   @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;
     }
   }
 }
diff --git a/firebase-storage/src/main/java/com/google/firebase/storage/network/NetworkRequest.java b/firebase-storage/src/main/java/com/google/firebase/storage/network/NetworkRequest.java
index 3bb8050534b..41c9fcd0200 100644
--- a/firebase-storage/src/main/java/com/google/firebase/storage/network/NetworkRequest.java
+++ b/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;
diff --git a/firebase-storage/src/test/java/com/google/firebase/storage/DependencyTest.java b/firebase-storage/src/test/java/com/google/firebase/storage/DependencyTest.java
index 276c6ca3cec..1c1ef9b56ce 100644
--- a/firebase-storage/src/test/java/com/google/firebase/storage/DependencyTest.java
+++ b/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 {
diff --git a/firebase-storage/src/test/java/com/google/firebase/storage/RetryRule.java b/firebase-storage/src/test/java/com/google/firebase/storage/RetryRule.java
index a09e4482c26..ef57cad7cd4 100644
--- a/firebase-storage/src/test/java/com/google/firebase/storage/RetryRule.java
+++ b/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
diff --git a/firebase-storage/src/test/java/com/google/firebase/storage/TestUtil.java b/firebase-storage/src/test/java/com/google/firebase/storage/TestUtil.java
index 6682a48e3af..5dae0c40dc8 100644
--- a/firebase-storage/src/test/java/com/google/firebase/storage/TestUtil.java
+++ b/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);
           }
diff --git a/firebase-storage/src/test/java/com/google/firebase/storage/network/MockConnectionFactory.java b/firebase-storage/src/test/java/com/google/firebase/storage/network/MockConnectionFactory.java
index 07c8033a763..a085936a1c1 100644
--- a/firebase-storage/src/test/java/com/google/firebase/storage/network/MockConnectionFactory.java
+++ b/firebase-storage/src/test/java/com/google/firebase/storage/network/MockConnectionFactory.java
@@ -44,13 +44,13 @@
 
 public class MockConnectionFactory implements HttpURLConnectionFactory {
   private final boolean binaryBody;
-  HttpURLConnection oldMock;
-  List<String> verifications = new ArrayList<>();
-  int count = 0;
-  private BufferedReader br;
+  private HttpURLConnection oldMock;
+  private List<String> verifications = new ArrayList<>();
+  private final BufferedReader br;
+  private final Semaphore pauseSemaphore = new Semaphore(0);
+  private int lineCount = 0;
   private int pauseRecord = Integer.MAX_VALUE;
-  private int currentRecord = 0;
-  private Semaphore pauseSemaphore = new Semaphore(0);
+  private int currentRecord = 0;;
 
   public MockConnectionFactory(String testName, boolean binaryBody) {
     this.binaryBody = binaryBody;
@@ -91,7 +91,7 @@ public synchronized HttpURLConnection createInstance(@NonNull URL url) throws IO
     String line;
     try {
       while ((line = br.readLine()) != null) {
-        count++;
+        lineCount++;
 
         if ("<new>".equalsIgnoreCase(line)) {
           break;
@@ -156,7 +156,7 @@ public synchronized HttpURLConnection createInstance(@NonNull URL url) throws IO
         }
       }
     } catch (ComparisonFailure | IllegalArgumentException e) {
-      System.out.println("**** Error at line:" + count);
+      System.out.println("**** Error at line:" + lineCount);
       throw e;
     }
     currentRecord++;
@@ -179,7 +179,7 @@ public synchronized HttpURLConnection createInstance(@NonNull URL url) throws IO
   }
 
   public synchronized void verifyOldMock() {
-    if (oldMock == null || verifications.size() == 0) {
+    if (oldMock == null || verifications.isEmpty()) {
       return;
     }
     List<String> requestPropertyKeys = new ArrayList<>();
@@ -233,7 +233,7 @@ public synchronized void verifyOldMock() {
             valueCapture.getAllValues().get(keyIndex));
       }
     } catch (MockitoAssertionError | ComparisonFailure e) {
-      System.out.println("**********Error in network Line: " + count);
+      System.out.println("**********Error in network Line: " + lineCount);
       throw e;
     }