Skip to content

Propagate error for ListAll #713

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 3 commits into from
Aug 16, 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
14 changes: 10 additions & 4 deletions firebase-storage/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,13 +1,19 @@
# Unreleased (17.1.0)
# 19.0.1
- [fixed] `listAll()` now propagates the error messages if the List operation
was denied by a Security Rule.

# 19.0.0
- [changed] Added missing nullability annotations for better Kotlin interop.
- [internal] Removed ``@PublicApi` annotations as they are no longer enforced
and have no semantic meaning.

# 18.1.0
- [feature] Added `StorageReference.list()` and `StorageReference.listAll()`,
which allows developers to list the files and folders under the given
StorageReference.
- [changed] Added validation to `StorageReference.getDownloadUrl()` and
`StorageReference.getMetadata()` to return an error if the reference is the
root of the bucket.
- [changed] Added missing nullability annotations for better Kotlin interop.
- [internal] Removed ``@PublicApi` annotations as they are no longer enforced
and have no semantic meaning.

# 17.0.0
- [internal] Updated the SDK initialization process and removed usages of
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -626,15 +626,20 @@ public Task<ListResult> listAll() {
new Continuation<ListResult, Task<Void>>() {
@Override
public Task<Void> then(@NonNull Task<ListResult> currentPage) {
ListResult result = currentPage.getResult();
prefixes.addAll(result.getPrefixes());
items.addAll(result.getItems());

if (result.getPageToken() != null) {
Task<ListResult> nextPage = listHelper(/* maxResults= */ null, result.getPageToken());
nextPage.continueWithTask(executor, this);
if (currentPage.isSuccessful()) {
ListResult result = currentPage.getResult();
prefixes.addAll(result.getPrefixes());
items.addAll(result.getItems());

if (result.getPageToken() != null) {
Task<ListResult> nextPage =
listHelper(/* maxResults= */ null, result.getPageToken());
nextPage.continueWithTask(executor, this);
} else {
pendingResult.setResult(new ListResult(prefixes, items, /* pageToken= */ null));
}
} else {
pendingResult.setResult(new ListResult(prefixes, items, /* pageToken= */ null));
pendingResult.setException(currentPage.getException());
}

return Tasks.forResult(null);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,18 @@ public void listResultsWithMultiplePages() throws InterruptedException {
TestUtil.verifyTaskStateChanges("listMultiplePages", task.getResult().toString());
}

@Test
public void listFailure() throws InterruptedException {
MockConnectionFactory factory =
NetworkLayerMock.ensureNetworkMock("listSinglePageFailed", false);
Task<StringBuilder> task = TestCommandHelper.listFiles(/* pageSize= */ 10, /* pageCount= */ 1);

TestUtil.await(task);

factory.verifyOldMock();
TestUtil.verifyTaskStateChanges("listSinglePageFailed", task.getResult().toString());
}

@Test
public void listAll() throws InterruptedException {
MockConnectionFactory factory = NetworkLayerMock.ensureNetworkMock("listAll", true);
Expand All @@ -113,4 +125,15 @@ public void listAll() throws InterruptedException {
factory.verifyOldMock();
TestUtil.verifyTaskStateChanges("listAll", task.getResult().toString());
}

@Test
public void listAllWithFailure() throws InterruptedException {
MockConnectionFactory factory = NetworkLayerMock.ensureNetworkMock("listAllFailed", false);
Task<StringBuilder> task = TestCommandHelper.listAllFiles();

TestUtil.await(task);

factory.verifyOldMock();
TestUtil.verifyTaskStateChanges("listAllFailed", task.getResult().toString());
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@

<new>
Url:https://firebasestorage.googleapis.com/v0/b/fooey.appspot.com/o?prefix=largeDirectory%2F&delimiter=%2F
setRequestMethod:GET
setRequestProperty:X-Firebase-Storage-Version,Android/[No Gmscore]
setRequestProperty:x-firebase-gmpid,fooey
setRequestProperty:Content-Length,0
setUseCaches:false
setDoInput:true
getResponseCode:403
getHeaderFields:
null:[HTTP/1.1 403 Forbidden]
Access-Control-Allow-Origin:[*]
Access-Control-Expose-Headers:[Content-Range, X-Firebase-Storage-XSRF]
Alt-Svc:[quic=":443"; ma=2592000; v="46,43,39"]
Cache-Control:[private, max-age=0]
Content-Length:[106]
Content-Type:[application/json; charset=UTF-8]
Date:[Fri, 16 Aug 2019 17:51:19 GMT]
Expires:[Fri, 16 Aug 2019 17:51:19 GMT]
Server:[UploadServer]
X-Android-Received-Millis:[1565977885114]
X-Android-Response-Source:[NETWORK 403]
X-Android-Selected-Protocol:[http/1.1]
X-Android-Sent-Millis:[1565977884943]
X-Content-Type-Options:[nosniff]
X-GUploader-UploadID:[AEnB2Up1Fu4ZwtUsQe7mXrzuQ_S2KZvRwUttfOYcrQsZm4kuJcUJFvMzTRxkmydO8ErBV1bI_Ku_B7xqFBp5r4bt-hEu4a2JBQ]
getErrorStream:{ "error": { "code": 403, "message": "Permission denied. Could not perform this operation" }}
disconnect:
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@

listAll:
onComplete:Success=false
done.
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@

<new>
Url:https://firebasestorage.googleapis.com/v0/b/fooey.appspot.com/o?prefix=smallDirectory%2F&delimiter=%2F&maxResults=10
setRequestMethod:GET
setRequestProperty:X-Firebase-Storage-Version,Android/[No Gmscore]
setRequestProperty:x-firebase-gmpid,fooey
setRequestProperty:Content-Length,0
setUseCaches:false
setDoInput:true
getResponseCode:403
getHeaderFields:
null:[HTTP/1.1 403 Forbidden]
Access-Control-Allow-Origin:[*]
Access-Control-Expose-Headers:[Content-Range, X-Firebase-Storage-XSRF]
Alt-Svc:[quic=":443"; ma=2592000; v="46,43,39"]
Cache-Control:[private, max-age=0]
Content-Length:[106]
Content-Type:[application/json; charset=UTF-8]
Date:[Fri, 16 Aug 2019 17:51:19 GMT]
Expires:[Fri, 16 Aug 2019 17:51:19 GMT]
Server:[UploadServer]
X-Android-Received-Millis:[1565977885114]
X-Android-Response-Source:[NETWORK 403]
X-Android-Selected-Protocol:[http/1.1]
X-Android-Sent-Millis:[1565977884943]
X-Content-Type-Options:[nosniff]
X-GUploader-UploadID:[AEnB2Up1Fu4ZwtUsQe7mXrzuQ_S2KZvRwUttfOYcrQsZm4kuJcUJFvMzTRxkmydO8ErBV1bI_Ku_B7xqFBp5r4bt-hEu4a2JBQ]
getErrorStream:{ "error": { "code": 403, "message": "Permission denied. Could not perform this operation" }}
disconnect:
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@

list:
onComplete:Success=false
done.
Original file line number Diff line number Diff line change
Expand Up @@ -242,12 +242,12 @@ public static Task<StringBuilder> listFiles(int pageSize, int pageCount) {
public void onComplete(@NonNull Task<ListResult> task) {
++pagesReceived[0];

ListResult listResult = task.getResult();

builder.append("\nlist:");
builder.append("\n onComplete:Success=").append(task.isSuccessful());

if (task.isSuccessful()) {
ListResult listResult = task.getResult();

builder.append("\n Received Prefixes:");
for (StorageReference prefix : listResult.getPrefixes()) {
builder.append("\n ").append(prefix.getPath());
Expand All @@ -257,12 +257,14 @@ public void onComplete(@NonNull Task<ListResult> task) {
builder.append("\n ").append(item.getPath());
}
builder.append("\n Page Token:").append(listResult.getPageToken());
}

if (pagesReceived[0] == pageCount) {
result.setResult(builder);
if (pagesReceived[0] == pageCount) {
result.setResult(builder);
} else {
reference.list(pageSize, listResult.getPageToken()).addOnCompleteListener(this);
}
} else {
reference.list(pageSize, listResult.getPageToken()).addOnCompleteListener(this);
result.setResult(builder);
}
}
});
Expand All @@ -280,12 +282,11 @@ public static Task<StringBuilder> listAllFiles() {
listFiles.addOnCompleteListener(
executor,
task -> {
ListResult listResult = task.getResult();

builder.append("\nlistAll:");
builder.append("\n onComplete:Success=").append(task.isSuccessful());

if (task.isSuccessful()) {
ListResult listResult = task.getResult();
builder.append("\n Received Prefixes:");
for (StorageReference prefix : listResult.getPrefixes()) {
builder.append("\n ").append(prefix.getPath());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@
import com.google.android.gms.tasks.Continuation;
import com.google.android.gms.tasks.Task;
import com.google.android.gms.tasks.TaskCompletionSource;
import com.google.firebase.FirebaseApp;
import com.google.firebase.auth.FirebaseAuth;
import com.google.firebase.storage.FirebaseStorage;
import com.google.firebase.storage.TestCommandHelper;
Expand Down Expand Up @@ -104,6 +105,7 @@ protected void onCreate(Bundle savedInstanceState) {
super.onCreate(savedInstanceState);
setContentView(R.layout.activity_main);

FirebaseApp.initializeApp(getApplicationContext());
Copy link
Contributor

@fredzqm fredzqm Aug 16, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OOC, why this line weren't there before?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This lets us use the test app without the Google Services plugin, which we had to remove as it breaks the CI build.

FirebaseAuth.getInstance().signInAnonymously();

Button clickButton = findViewById(R.id.streamDownload);
Expand Down
2 changes: 1 addition & 1 deletion firebase-storage/test-app/test-app.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -67,4 +67,4 @@ dependencies {
// androidTest integration tests.
// ==========================================================================
ext.packageName = "com.example.storage"
apply from: '../../gradle/googleServices.gradle'
apply from: '../../gradle/googleServices.gradle'