Skip to content

Commit 7d25ca8

Browse files
[Storage] Fix formatting, missing doxygen docs, and add release notes
Co-authored-by: AustinBenoit <22805659+AustinBenoit@users.noreply.github.com>
1 parent 88a564c commit 7d25ca8

11 files changed

Lines changed: 112 additions & 90 deletions

File tree

release_build_files/readme.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -615,6 +615,7 @@ code.
615615
## Release Notes
616616
### 13.5.0
617617
- Changes
618+
- Storage: Added `List` API across all platforms.
618619
- General (Android): Update to Firebase Android BoM version 34.10.0.
619620
- General (iOS): Update to Firebase Cocoapods version 12.10.0.
620621
- App Check: Add in support for Limited Use Tokens.

storage/integration_test/src/integration_test.cc

Lines changed: 18 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1651,18 +1651,16 @@ TEST_F(FirebaseStorageTest, TestList) {
16511651

16521652
// Create some files and folders
16531653
std::vector<std::string> file_paths = {
1654-
"folderA/file1.txt",
1655-
"folderA/file2.txt",
1656-
"folderB/file3.txt",
1657-
"folderC/file4.txt",
1658-
"rootFile.txt"
1659-
};
1654+
"folderA/file1.txt", "folderA/file2.txt", "folderB/file3.txt",
1655+
"folderC/file4.txt", "rootFile.txt"};
16601656

16611657
for (const std::string& path : file_paths) {
16621658
firebase::storage::StorageReference ref = root.Child(path);
16631659
WaitForCompletion(RunWithRetry([&]() {
1664-
return ref.PutBytes(&kSimpleTestFile[0], kSimpleTestFile.size());
1665-
}), "PutBytes List Test");
1660+
return ref.PutBytes(&kSimpleTestFile[0],
1661+
kSimpleTestFile.size());
1662+
}),
1663+
"PutBytes List Test");
16661664
}
16671665

16681666
// Allow some time for index to catch up
@@ -1671,9 +1669,10 @@ TEST_F(FirebaseStorageTest, TestList) {
16711669
// List all at the root of the test folder
16721670
firebase::Future<firebase::storage::StorageListResult> list_future;
16731671
WaitForCompletion(RunWithRetry([&]() {
1674-
list_future = root.List(1000);
1675-
return list_future;
1676-
}), "ListRoot");
1672+
list_future = root.List(1000);
1673+
return list_future;
1674+
}),
1675+
"ListRoot");
16771676

16781677
EXPECT_EQ(list_future.error(), firebase::storage::kErrorNone);
16791678

@@ -1702,9 +1701,10 @@ TEST_F(FirebaseStorageTest, TestList) {
17021701
// Test pagination
17031702
firebase::storage::StorageReference folderA = root.Child("folderA");
17041703
WaitForCompletion(RunWithRetry([&]() {
1705-
list_future = folderA.List(1);
1706-
return list_future;
1707-
}), "ListFolderA_Page1");
1704+
list_future = folderA.List(1);
1705+
return list_future;
1706+
}),
1707+
"ListFolderA_Page1");
17081708

17091709
EXPECT_EQ(list_future.error(), firebase::storage::kErrorNone);
17101710
list_result = list_future.result();
@@ -1714,9 +1714,10 @@ TEST_F(FirebaseStorageTest, TestList) {
17141714
std::string next_token = list_result->next_page_token();
17151715

17161716
WaitForCompletion(RunWithRetry([&]() {
1717-
list_future = folderA.List(1, next_token.c_str());
1718-
return list_future;
1719-
}), "ListFolderA_Page2");
1717+
list_future = folderA.List(1, next_token.c_str());
1718+
return list_future;
1719+
}),
1720+
"ListFolderA_Page2");
17201721

17211722
EXPECT_EQ(list_future.error(), firebase::storage::kErrorNone);
17221723
list_result = list_future.result();

storage/src/android/storage_reference_android.cc

Lines changed: 17 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -284,27 +284,27 @@ void StorageReferenceInternal::FutureCallback(JNIEnv* env, jobject result,
284284

285285
std::vector<StorageReference> prefixes;
286286
if (prefixes_list) {
287-
jint size = env->CallIntMethod(
288-
prefixes_list, list::GetMethodId(list::kSize));
287+
jint size =
288+
env->CallIntMethod(prefixes_list, list::GetMethodId(list::kSize));
289289
for (jint i = 0; i < size; ++i) {
290290
jobject prefix_obj = env->CallObjectMethod(
291291
prefixes_list, list::GetMethodId(list::kGet), i);
292-
prefixes.push_back(
293-
StorageReference(new StorageReferenceInternal(data->storage, prefix_obj)));
292+
prefixes.push_back(StorageReference(
293+
new StorageReferenceInternal(data->storage, prefix_obj)));
294294
env->DeleteLocalRef(prefix_obj);
295295
}
296296
env->DeleteLocalRef(prefixes_list);
297297
}
298298

299299
std::vector<StorageReference> items;
300300
if (items_list) {
301-
jint size = env->CallIntMethod(
302-
items_list, list::GetMethodId(list::kSize));
301+
jint size =
302+
env->CallIntMethod(items_list, list::GetMethodId(list::kSize));
303303
for (jint i = 0; i < size; ++i) {
304-
jobject item_obj = env->CallObjectMethod(
305-
items_list, list::GetMethodId(list::kGet), i);
306-
items.push_back(
307-
StorageReference(new StorageReferenceInternal(data->storage, item_obj)));
304+
jobject item_obj =
305+
env->CallObjectMethod(items_list, list::GetMethodId(list::kGet), i);
306+
items.push_back(StorageReference(
307+
new StorageReferenceInternal(data->storage, item_obj)));
308308
env->DeleteLocalRef(item_obj);
309309
}
310310
env->DeleteLocalRef(items_list);
@@ -318,7 +318,8 @@ void StorageReferenceInternal::FutureCallback(JNIEnv* env, jobject result,
318318

319319
StorageListResultInternal* list_internal =
320320
new StorageListResultInternal(prefixes, items, page_token);
321-
data->impl->CompleteWithResult(handle, kErrorNone, StorageListResult(list_internal));
321+
data->impl->CompleteWithResult(handle, kErrorNone,
322+
StorageListResult(list_internal));
322323
} else if (result && env->IsInstanceOf(result, util::string::GetClass())) {
323324
LogDebug("FutureCallback: Completing a Future from a String.");
324325
// Complete a Future<std::string> from a Java String object.
@@ -754,20 +755,21 @@ Future<Metadata> StorageReferenceInternal::PutFileLastResult() {
754755
future()->LastResult(kStorageReferenceFnPutFile));
755756
}
756757

757-
Future<StorageListResult> StorageReferenceInternal::List(int max_results_per_page,
758-
const char *page_token) {
758+
Future<StorageListResult> StorageReferenceInternal::List(
759+
int max_results_per_page, const char* page_token) {
759760
JNIEnv* env = storage_->app()->GetJNIEnv();
760761
ReferenceCountedFutureImpl* future_impl = future();
761762
FutureHandle handle =
762763
future_impl->Alloc<StorageListResult>(kStorageReferenceFnList);
763764

764-
jstring java_page_token = page_token ? env->NewStringUTF(page_token) : nullptr;
765+
jstring java_page_token =
766+
page_token ? env->NewStringUTF(page_token) : nullptr;
765767
jobject task = env->CallObjectMethod(
766768
obj_, storage_reference::GetMethodId(storage_reference::kList),
767769
max_results_per_page, java_page_token);
768770

769771
if (java_page_token) {
770-
env->DeleteLocalRef(java_page_token);
772+
env->DeleteLocalRef(java_page_token);
771773
}
772774

773775
util::RegisterCallbackOnTask(

storage/src/android/storage_reference_android.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -131,7 +131,7 @@ class StorageReferenceInternal {
131131

132132
// List items (files) and prefixes (folders) under this StorageReference.
133133
Future<StorageListResult> List(int max_results_per_page,
134-
const char *page_token);
134+
const char* page_token);
135135

136136
// Returns the result of the most recent call to List();
137137
Future<StorageListResult> ListLastResult();

storage/src/common/list_result.cc

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
// limitations under the License.
1414

1515
#include "storage/src/include/firebase/storage/list_result.h"
16+
1617
#include "storage/src/common/list_result_internal.h"
1718

1819
namespace firebase {
@@ -26,14 +27,19 @@ StorageListResult::~StorageListResult() {
2627
}
2728

2829
StorageListResult::StorageListResult(const StorageListResult& other)
29-
: internal_(other.internal_ ? new internal::StorageListResultInternal(*other.internal_) : nullptr) {}
30+
: internal_(other.internal_
31+
? new internal::StorageListResultInternal(*other.internal_)
32+
: nullptr) {}
3033

31-
StorageListResult& StorageListResult::operator=(const StorageListResult& other) {
34+
StorageListResult& StorageListResult::operator=(
35+
const StorageListResult& other) {
3236
if (this == &other) {
3337
return *this;
3438
}
3539
delete internal_;
36-
internal_ = other.internal_ ? new internal::StorageListResultInternal(*other.internal_) : nullptr;
40+
internal_ = other.internal_
41+
? new internal::StorageListResultInternal(*other.internal_)
42+
: nullptr;
3743
return *this;
3844
}
3945

@@ -68,7 +74,8 @@ const char* StorageListResult::next_page_token() const {
6874
return internal_ ? internal_->next_page_token() : nullptr;
6975
}
7076

71-
StorageListResult::StorageListResult(internal::StorageListResultInternal* internal)
77+
StorageListResult::StorageListResult(
78+
internal::StorageListResultInternal* internal)
7279
: internal_(internal) {}
7380

7481
} // namespace storage

storage/src/common/storage_reference.cc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -247,7 +247,7 @@ Future<Metadata> StorageReference::PutFileLastResult() {
247247
}
248248

249249
Future<StorageListResult> StorageReference::List(int max_results_per_page,
250-
const char *page_token) {
250+
const char* page_token) {
251251
return internal_ ? internal_->List(max_results_per_page, page_token)
252252
: Future<StorageListResult>();
253253
}

storage/src/desktop/curl_requests.cc

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,11 +19,11 @@
1919
#include <string>
2020

2121
#include "app/rest/util.h"
22+
#include "storage/src/common/list_result_internal.h"
2223
#include "storage/src/desktop/metadata_desktop.h"
2324
#include "storage/src/desktop/rest_operation.h"
2425
#include "storage/src/include/firebase/storage/common.h"
2526
#include "storage/src/include/firebase/storage/metadata.h"
26-
#include "storage/src/common/list_result_internal.h"
2727

2828
namespace firebase {
2929
namespace storage {
@@ -285,7 +285,8 @@ void ReturnedListResponse::MarkCompleted() {
285285
if (!path.empty() && path.back() == '/') {
286286
path.pop_back();
287287
}
288-
prefixes.push_back(storage_reference_.storage()->GetReference(path.c_str()));
288+
prefixes.push_back(
289+
storage_reference_.storage()->GetReference(path.c_str()));
289290
}
290291
}
291292
}

storage/src/desktop/storage_reference_desktop.cc

Lines changed: 46 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -694,60 +694,62 @@ StorageReferenceInternal* StorageReferenceInternal::GetParent() {
694694
return new StorageReferenceInternal(storageUri_.GetParent(), storage_);
695695
}
696696

697-
Future<StorageListResult> StorageReferenceInternal::List(int max_results_per_page,
698-
const char *page_token) {
697+
Future<StorageListResult> StorageReferenceInternal::List(
698+
int max_results_per_page, const char* page_token) {
699699
auto* future_api = future();
700-
auto handle = future_api->SafeAlloc<StorageListResult>(kStorageReferenceFnList);
700+
auto handle =
701+
future_api->SafeAlloc<StorageListResult>(kStorageReferenceFnList);
701702

702703
std::string token_str = page_token ? page_token : "";
703-
auto send_request_funct{[&, max_results_per_page, token_str]() -> BlockingResponse* {
704-
auto* future_api = future();
705-
auto handle = future_api->SafeAlloc<StorageListResult>(kStorageReferenceFnListInternal);
706-
ReturnedListResponse* response =
707-
new ReturnedListResponse(handle, future_api, AsStorageReference());
704+
auto send_request_funct{
705+
[&, max_results_per_page, token_str]() -> BlockingResponse* {
706+
auto* future_api = future();
707+
auto handle = future_api->SafeAlloc<StorageListResult>(
708+
kStorageReferenceFnListInternal);
709+
ReturnedListResponse* response =
710+
new ReturnedListResponse(handle, future_api, AsStorageReference());
708711

709-
storage::internal::Request* request = new storage::internal::Request();
712+
storage::internal::Request* request = new storage::internal::Request();
710713

711-
// NOTE: The backend expects the base url for list to be the bucket, not the object.
712-
// So we reconstruct it.
713-
std::string list_url = storage_->get_scheme();
714-
list_url += "://";
715-
list_url += storage_->get_host();
716-
list_url += ":";
717-
list_url += std::to_string(storage_->get_port());
718-
list_url += "/v0/b/";
719-
list_url += bucket();
720-
list_url += "/o";
721-
722-
std::string path = storageUri_.GetPath().str();
723-
if (!path.empty() && path.back() != '/') {
724-
path += '/';
725-
}
714+
// NOTE: The backend expects the base url for list to be the bucket, not
715+
// the object. So we reconstruct it.
716+
std::string list_url = storage_->get_scheme();
717+
list_url += "://";
718+
list_url += storage_->get_host();
719+
list_url += ":";
720+
list_url += std::to_string(storage_->get_port());
721+
list_url += "/v0/b/";
722+
list_url += bucket();
723+
list_url += "/o";
724+
725+
std::string path = storageUri_.GetPath().str();
726+
if (!path.empty() && path.back() != '/') {
727+
path += '/';
728+
}
726729

727-
list_url += "?prefix=";
728-
if (!path.empty()) {
729-
list_url += rest::util::EncodeUrl(path);
730-
}
731-
list_url += "&delimiter=";
732-
list_url += rest::util::EncodeUrl("/");
733-
if (!token_str.empty()) {
734-
list_url += "&pageToken=";
735-
list_url += rest::util::EncodeUrl(token_str);
736-
}
737-
list_url += "&maxResults=";
738-
list_url += std::to_string(max_results_per_page);
730+
list_url += "?prefix=";
731+
if (!path.empty()) {
732+
list_url += rest::util::EncodeUrl(path);
733+
}
734+
list_url += "&delimiter=";
735+
list_url += rest::util::EncodeUrl("/");
736+
if (!token_str.empty()) {
737+
list_url += "&pageToken=";
738+
list_url += rest::util::EncodeUrl(token_str);
739+
}
740+
list_url += "&maxResults=";
741+
list_url += std::to_string(max_results_per_page);
739742

740-
PrepareRequestBlocking(request, list_url.c_str(), rest::util::kGet);
743+
PrepareRequestBlocking(request, list_url.c_str(), rest::util::kGet);
741744

742-
RestCall(request, request->notifier(), response, handle.get(), nullptr,
743-
nullptr);
745+
RestCall(request, request->notifier(), response, handle.get(), nullptr,
746+
nullptr);
744747

745-
return response;
746-
}};
748+
return response;
749+
}};
747750

748-
SendRequestWithRetry(kStorageReferenceFnListInternal,
749-
send_request_funct, handle,
750-
storage_->max_operation_retry_time());
751+
SendRequestWithRetry(kStorageReferenceFnListInternal, send_request_funct,
752+
handle, storage_->max_operation_retry_time());
751753
return ListLastResult();
752754
}
753755

storage/src/desktop/storage_reference_desktop.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -149,7 +149,7 @@ class StorageReferenceInternal {
149149

150150
// List items (files) and prefixes (folders) under this StorageReference.
151151
Future<StorageListResult> List(int max_results_per_page,
152-
const char *page_token);
152+
const char* page_token);
153153

154154
// Returns the result of the most recent call to List();
155155
Future<StorageListResult> ListLastResult();
@@ -194,7 +194,7 @@ class StorageReferenceInternal {
194194

195195
// List operation without metadata.
196196
Future<StorageListResult> ListInternal(int max_results_per_page,
197-
const char *page_token);
197+
const char* page_token);
198198

199199
void RestCall(rest::Request* request, internal::Notifier* request_notifier,
200200
BlockingResponse* response, FutureHandle handle,

storage/src/include/firebase/storage/list_result.h

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,11 +31,17 @@ class StorageListResult {
3131
public:
3232
StorageListResult();
3333
~StorageListResult();
34+
/// @brief Copy constructor.
3435
StorageListResult(const StorageListResult& other);
36+
37+
/// @brief Copy assignment operator.
3538
StorageListResult& operator=(const StorageListResult& other);
3639

3740
#if defined(FIREBASE_USE_MOVE_OPERATORS) || defined(DOXYGEN)
41+
/// @brief Move constructor.
3842
StorageListResult(StorageListResult&& other);
43+
44+
/// @brief Move assignment operator.
3945
StorageListResult& operator=(StorageListResult&& other);
4046
#endif // defined(FIREBASE_USE_MOVE_OPERATORS) || defined(DOXYGEN)
4147

0 commit comments

Comments
 (0)