Skip to content

Fixing more Storage code style issues #594

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jul 10, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand All @@ -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;
}

/**
Expand All @@ -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;
}
}
}
Expand All @@ -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.");
}
Expand All @@ -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;
}
Expand All @@ -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);
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -506,7 +505,7 @@ public long getTotalByteCount() {
*/
@PublicApi
public InputStream getStream() {
return StreamDownloadTask.this.mInputStream;
return StreamDownloadTask.this.inputStream;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand All @@ -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.
*
Expand Down Expand Up @@ -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();
Expand All @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
Loading