Skip to content

Commit b0050f0

Browse files
Fix: Correct include path for list_result.h in StorageReferenceInternal
This commit corrects the #include path for the public `firebase/storage/list_result.h` header within the desktop platform's `StorageReferenceInternal` implementation (`storage/src/desktop/storage_reference_desktop.cc`). I changed the path from a relative "firebase/storage/list_result.h" to a more explicit "storage/src/include/firebase/storage/list_result.h" to resolve a potential build issue. Include paths for Android and iOS platforms were reviewed and found to be correctly handling the inclusion of `firebase/storage/list_result.h` via their respective `StorageReferenceInternal` header files using standard include paths. Additionally, I removed a minor extraneous agent-added comment from `storage/src/ios/storage_reference_ios.h`. I ran the code formatting script after these changes.
1 parent 12e873c commit b0050f0

File tree

11 files changed

+92
-63
lines changed

11 files changed

+92
-63
lines changed

storage/integration_test/src/integration_test.cc

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1624,11 +1624,12 @@ TEST_F(FirebaseStorageTest, TestInvalidatingReferencesWhenDeletingApp) {
16241624
}
16251625

16261626
// Test the StorageReference::ListAll() method.
1627-
// This test currently only verifies that the stubbed method returns an empty result.
1627+
// This test currently only verifies that the stubbed method returns an empty
1628+
// result.
16281629
TEST_F(FirebaseStorageTest, TestListAll) {
16291630
if (skip_tests_) return;
16301631

1631-
firebase::storage::Storage* storage = storage_; // Use the member variable
1632+
firebase::storage::Storage* storage = storage_; // Use the member variable
16321633
firebase::storage::StorageReference reference = storage->GetReference();
16331634

16341635
firebase::Future<firebase::storage::ListResult> future = reference.ListAll();
@@ -1647,11 +1648,12 @@ TEST_F(FirebaseStorageTest, TestListAll) {
16471648
}
16481649

16491650
// Test the StorageReference::List() method with no page token.
1650-
// This test currently only verifies that the stubbed method returns an empty result.
1651+
// This test currently only verifies that the stubbed method returns an empty
1652+
// result.
16511653
TEST_F(FirebaseStorageTest, TestListNoPageToken) {
16521654
if (skip_tests_) return;
16531655

1654-
firebase::storage::Storage* storage = storage_; // Use the member variable
1656+
firebase::storage::Storage* storage = storage_; // Use the member variable
16551657
firebase::storage::StorageReference reference = storage->GetReference();
16561658

16571659
firebase::Future<firebase::storage::ListResult> future = reference.List();
@@ -1670,16 +1672,17 @@ TEST_F(FirebaseStorageTest, TestListNoPageToken) {
16701672
}
16711673

16721674
// Test the StorageReference::List() method with a page token.
1673-
// This test currently only verifies that the stubbed method returns an empty result
1674-
// and that the page token is passed (though not used by the stub).
1675+
// This test currently only verifies that the stubbed method returns an empty
1676+
// result and that the page token is passed (though not used by the stub).
16751677
TEST_F(FirebaseStorageTest, TestListWithPageToken) {
16761678
if (skip_tests_) return;
16771679

1678-
firebase::storage::Storage* storage = storage_; // Use the member variable
1680+
firebase::storage::Storage* storage = storage_; // Use the member variable
16791681
firebase::storage::StorageReference reference = storage->GetReference();
16801682
const char* page_token = "test_page_token";
16811683

1682-
firebase::Future<firebase::storage::ListResult> future = reference.List(page_token);
1684+
firebase::Future<firebase::storage::ListResult> future =
1685+
reference.List(page_token);
16831686
WaitForCompletion(future, "List (with page token)");
16841687

16851688
ASSERT_EQ(future.status(), firebase::kFutureStatusComplete);

storage/src/android/list_result_android.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,8 @@
66
#include <vector>
77

88
#include "firebase/storage/storage_reference.h"
9-
#include "storage/src/android/storage_reference_android.h"
109
#include "storage/src/android/storage_internal_android.h"
10+
#include "storage/src/android/storage_reference_android.h"
1111

1212
namespace firebase {
1313
namespace storage {

storage/src/android/storage_reference_android.cc

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@
2323
#include "app/src/include/firebase/app.h"
2424
#include "app/src/util_android.h"
2525
#include "storage/src/android/controller_android.h"
26-
#include "storage/src/android/list_result_android.h" // Defines the android internal::ListResultInternal
26+
#include "storage/src/android/list_result_android.h" // Defines the android internal::ListResultInternal
2727
#include "storage/src/android/metadata_android.h"
2828
#include "storage/src/android/storage_android.h"
2929
#include "storage/src/include/firebase/storage.h"
@@ -736,13 +736,13 @@ jint StorageReferenceInternal::CppByteUploaderReadBytes(
736736
}
737737

738738
Future<ListResult> StorageReferenceInternal::ListAll() {
739-
StorageReference self(this); // Public self for future context
739+
StorageReference self(this); // Public self for future context
740740
ReferenceCountedFutureImpl* ref_future =
741741
future()->Alloc<ListResult>(kStorageReferenceFnCount);
742742
Future<ListResult> future = MakeFuture(ref_future, self);
743743

744-
internal::ListResultInternal* list_pimpl =
745-
new internal::ListResultInternal(this, nullptr); // 'this' is StorageReferenceInternal* (Android)
744+
internal::ListResultInternal* list_pimpl = new internal::ListResultInternal(
745+
this, nullptr); // 'this' is StorageReferenceInternal* (Android)
746746

747747
ListResult result_to_complete(list_pimpl);
748748

@@ -751,7 +751,7 @@ Future<ListResult> StorageReferenceInternal::ListAll() {
751751
}
752752

753753
Future<ListResult> StorageReferenceInternal::List(const char* page_token) {
754-
StorageReference self(this); // Public self for future context
754+
StorageReference self(this); // Public self for future context
755755
ReferenceCountedFutureImpl* ref_future =
756756
future()->Alloc<ListResult>(kStorageReferenceFnCount);
757757
Future<ListResult> future = MakeFuture(ref_future, self);

storage/src/android/storage_reference_android.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,9 +22,9 @@
2222
#include "app/src/include/firebase/internal/common.h"
2323
#include "app/src/reference_counted_future_impl.h"
2424
#include "app/src/util_android.h"
25+
#include "firebase/storage/list_result.h"
2526
#include "storage/src/android/storage_android.h"
2627
#include "storage/src/include/firebase/storage/storage_reference.h"
27-
#include "firebase/storage/list_result.h"
2828

2929
namespace firebase {
3030
namespace storage {

storage/src/common/list_result.cc

Lines changed: 39 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -1,25 +1,25 @@
11
// File: storage/src/common/list_result.cc
22

33
#include "firebase/storage/list_result.h"
4-
#include "app/src/include/firebase/internal/platform.h"
4+
55
#include "app/src/cleanup_notifier.h"
6+
#include "app/src/include/firebase/internal/platform.h"
67
#include "app/src/log.h"
78
#include "firebase/storage/storage_reference.h"
89

9-
// Platform-specific headers that define internal::ListResultInternal (the PIMPL class)
10-
// and internal::StorageInternal (for CleanupNotifier context).
10+
// Platform-specific headers that define internal::ListResultInternal (the PIMPL
11+
// class) and internal::StorageInternal (for CleanupNotifier context).
1112
#if FIREBASE_PLATFORM_ANDROID
1213
#include "storage/src/android/list_result_android.h"
1314
#include "storage/src/android/storage_internal_android.h"
1415
#elif FIREBASE_PLATFORM_IOS || FIREBASE_PLATFORM_TVOS
1516
#include "storage/src/ios/list_result_ios.h"
1617
#include "storage/src/ios/storage_internal_ios.h"
17-
#else // Desktop
18+
#else // Desktop
1819
#include "storage/src/desktop/list_result_desktop.h"
1920
#include "storage/src/desktop/storage_internal_desktop.h"
2021
#endif
2122

22-
2323
namespace firebase {
2424
namespace storage {
2525

@@ -43,7 +43,10 @@ class ListResultInternalCommon {
4343
// Relies on ListResultInternal having associated_storage_internal().
4444
StorageInternal* storage_ctx = pimpl_obj->associated_storage_internal();
4545
if (storage_ctx == nullptr) {
46-
LogWarning("ListResultInternal %p has no associated StorageInternal for cleanup.", pimpl_obj);
46+
LogWarning(
47+
"ListResultInternal %p has no associated StorageInternal for "
48+
"cleanup.",
49+
pimpl_obj);
4750
}
4851
return storage_ctx;
4952
}
@@ -52,10 +55,12 @@ class ListResultInternalCommon {
5255
static void CleanupPublicListResultObject(void* public_obj_void) {
5356
ListResult* public_obj = reinterpret_cast<ListResult*>(public_obj_void);
5457
if (public_obj) {
55-
LogDebug("CleanupNotifier: Cleaning up ListResult %p.", public_obj);
56-
DeleteInternal(public_obj);
58+
LogDebug("CleanupNotifier: Cleaning up ListResult %p.", public_obj);
59+
DeleteInternal(public_obj);
5760
} else {
58-
LogWarning("CleanupNotifier: CleanupPublicListResultObject called with null object.");
61+
LogWarning(
62+
"CleanupNotifier: CleanupPublicListResultObject called with null "
63+
"object.");
5964
}
6065
}
6166

@@ -65,12 +70,15 @@ class ListResultInternalCommon {
6570
if (!pimpl_obj) return;
6671
StorageInternal* storage_ctx = GetStorageInternalContext(pimpl_obj);
6772
if (storage_ctx) {
68-
storage_ctx->cleanup().RegisterObject(public_obj, CleanupPublicListResultObject);
69-
LogDebug("ListResult %p (PIMPL %p) registered for cleanup.",
70-
public_obj, pimpl_obj);
73+
storage_ctx->cleanup().RegisterObject(public_obj,
74+
CleanupPublicListResultObject);
75+
LogDebug("ListResult %p (PIMPL %p) registered for cleanup.", public_obj,
76+
pimpl_obj);
7177
} else {
72-
LogWarning("Could not register ListResult %p for cleanup: no StorageInternal context.",
73-
public_obj);
78+
LogWarning(
79+
"Could not register ListResult %p for cleanup: no StorageInternal "
80+
"context.",
81+
public_obj);
7482
}
7583
}
7684

@@ -81,14 +89,18 @@ class ListResultInternalCommon {
8189
StorageInternal* storage_ctx = GetStorageInternalContext(pimpl_obj);
8290
if (storage_ctx) {
8391
storage_ctx->cleanup().UnregisterObject(public_obj);
84-
LogDebug("ListResult %p (associated with PIMPL %p) unregistered from cleanup.",
85-
public_obj, pimpl_obj);
92+
LogDebug(
93+
"ListResult %p (associated with PIMPL %p) unregistered from cleanup.",
94+
public_obj, pimpl_obj);
8695
} else {
87-
LogWarning("Could not unregister ListResult %p: no StorageInternal context.", public_obj);
96+
LogWarning(
97+
"Could not unregister ListResult %p: no StorageInternal context.",
98+
public_obj);
8899
}
89100
}
90101

91-
// Deletes the PIMPL object, unregisters from cleanup, and nulls the pointer in public_obj.
102+
// Deletes the PIMPL object, unregisters from cleanup, and nulls the pointer
103+
// in public_obj.
92104
static void DeleteInternal(ListResult* public_obj) {
93105
FIREBASE_ASSERT(public_obj != nullptr);
94106
if (!public_obj->internal_) return;
@@ -108,8 +120,7 @@ const std::vector<StorageReference> ListResult::s_empty_items_;
108120
const std::vector<StorageReference> ListResult::s_empty_prefixes_;
109121
const std::string ListResult::s_empty_page_token_;
110122

111-
ListResult::ListResult() : internal_(nullptr) {
112-
}
123+
ListResult::ListResult() : internal_(nullptr) {}
113124

114125
ListResult::ListResult(internal::ListResultInternal* internal_pimpl)
115126
: internal_(internal_pimpl) {
@@ -135,7 +146,7 @@ ListResult& ListResult::operator=(const ListResult& other) {
135146
if (this == &other) {
136147
return *this;
137148
}
138-
internal::ListResultInternalCommon::DeleteInternal(this); // Clean up current
149+
internal::ListResultInternalCommon::DeleteInternal(this); // Clean up current
139150

140151
if (other.internal_) {
141152
internal::StorageReferenceInternal* sri_context =
@@ -150,8 +161,10 @@ ListResult& ListResult::operator=(const ListResult& other) {
150161
ListResult::ListResult(ListResult&& other) : internal_(other.internal_) {
151162
other.internal_ = nullptr;
152163
if (internal_) {
153-
// New public object 'this' takes ownership. Unregister 'other', register 'this'.
154-
internal::ListResultInternalCommon::UnregisterFromCleanup(&other, internal_);
164+
// New public object 'this' takes ownership. Unregister 'other', register
165+
// 'this'.
166+
internal::ListResultInternalCommon::UnregisterFromCleanup(&other,
167+
internal_);
155168
internal::ListResultInternalCommon::RegisterForCleanup(this, internal_);
156169
}
157170
}
@@ -160,14 +173,15 @@ ListResult& ListResult::operator=(ListResult&& other) {
160173
if (this == &other) {
161174
return *this;
162175
}
163-
internal::ListResultInternalCommon::DeleteInternal(this); // Clean up current
176+
internal::ListResultInternalCommon::DeleteInternal(this); // Clean up current
164177

165178
internal_ = other.internal_;
166179
other.internal_ = nullptr;
167180

168181
if (internal_) {
169182
// Similar to move constructor: unregister 'other', register 'this'.
170-
internal::ListResultInternalCommon::UnregisterFromCleanup(&other, internal_);
183+
internal::ListResultInternalCommon::UnregisterFromCleanup(&other,
184+
internal_);
171185
internal::ListResultInternalCommon::RegisterForCleanup(this, internal_);
172186
}
173187
return *this;

storage/src/common/storage_reference.cc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,10 +14,10 @@
1414

1515
#include "storage/src/include/firebase/storage/storage_reference.h"
1616

17-
#include "firebase/storage/list_result.h"
1817
#include "app/src/assert.h"
1918
#include "app/src/future_manager.h"
2019
#include "app/src/include/firebase/internal/platform.h"
20+
#include "firebase/storage/list_result.h"
2121

2222
// Platform-specific ListResultInternal definition is no longer needed here.
2323
// #if FIREBASE_PLATFORM_ANDROID

storage/src/desktop/list_result_desktop.cc

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,14 +15,16 @@ ListResultInternal::ListResultInternal(
1515
prefixes_ = other_to_copy_from->prefixes_;
1616
page_token_ = other_to_copy_from->page_token_;
1717
}
18-
// If other_to_copy_from is null, members are default-initialized (empty for stub).
18+
// If other_to_copy_from is null, members are default-initialized (empty for
19+
// stub).
1920
}
2021

2122
// Destructor is default. Members are cleaned up automatically.
2223
// Lifecycle of this PIMPL object is managed by the public ListResult class
2324
// via ListResultInternalCommon static helpers.
2425

25-
// Accessor methods (items(), prefixes(), page_token()) are inline in the header.
26+
// Accessor methods (items(), prefixes(), page_token()) are inline in the
27+
// header.
2628

2729
} // namespace internal
2830
} // namespace storage

storage/src/desktop/list_result_desktop.h

Lines changed: 14 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -6,39 +6,44 @@
66
#include <vector>
77

88
#include "firebase/storage/storage_reference.h"
9-
#include "storage/src/desktop/storage_reference_desktop.h"
109
#include "storage/src/desktop/storage_internal_desktop.h"
10+
#include "storage/src/desktop/storage_reference_desktop.h"
1111

1212
namespace firebase {
1313
namespace storage {
1414
namespace internal {
1515

1616
/// Desktop platform's internal implementation for ListResult.
17-
/// This class holds the data for a list operation specific to the desktop platform.
18-
/// Its lifecycle is managed by the public ListResult class via static helpers.
17+
/// This class holds the data for a list operation specific to the desktop
18+
/// platform. Its lifecycle is managed by the public ListResult class via static
19+
/// helpers.
1920
class ListResultInternal {
2021
public:
2122
/// Constructor.
22-
/// @param[in] platform_sri The desktop StorageReferenceInternal this list result
23+
/// @param[in] platform_sri The desktop StorageReferenceInternal this list
24+
/// result
2325
/// is associated with; used for context.
2426
/// @param[in] other_to_copy_from If not null, initializes this instance by
2527
/// copying data from other_to_copy_from.
2628
explicit ListResultInternal(
2729
StorageReferenceInternal* platform_sri,
2830
const ListResultInternal* other_to_copy_from = nullptr);
2931

30-
// Destructor is default as members clean themselves up and lifecycle is external.
32+
// Destructor is default as members clean themselves up and lifecycle is
33+
// external.
3134

3235
const std::vector<StorageReference>& items() const { return items_; }
3336
const std::vector<StorageReference>& prefixes() const { return prefixes_; }
3437
const std::string& page_token() const { return page_token_; }
3538

36-
/// Provides access to the StorageReferenceInternal this object is associated with.
39+
/// Provides access to the StorageReferenceInternal this object is associated
40+
/// with.
3741
StorageReferenceInternal* storage_reference_internal() const {
3842
return platform_sri_;
3943
}
4044

41-
/// Provides access to the StorageInternal context, typically for cleanup registration.
45+
/// Provides access to the StorageInternal context, typically for cleanup
46+
/// registration.
4247
StorageInternal* associated_storage_internal() const {
4348
return platform_sri_ ? platform_sri_->storage_internal() : nullptr;
4449
}
@@ -47,7 +52,8 @@ class ListResultInternal {
4752
// Disallow copy assignment; copy construction is handled via the constructor.
4853
ListResultInternal& operator=(const ListResultInternal&);
4954

50-
StorageReferenceInternal* platform_sri_; // Associated StorageReference, not owned.
55+
StorageReferenceInternal*
56+
platform_sri_; // Associated StorageReference, not owned.
5157

5258
// Data for list results (stubs are empty).
5359
std::vector<StorageReference> items_;

storage/src/desktop/storage_reference_desktop.cc

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -30,15 +30,17 @@
3030
#include "app/src/thread.h"
3131
#include "storage/src/common/common_internal.h"
3232
#include "storage/src/desktop/controller_desktop.h"
33+
#include "storage/src/desktop/list_result_desktop.h" // Defines the desktop internal::ListResultInternal
3334
#include "storage/src/desktop/metadata_desktop.h"
3435
#include "storage/src/desktop/storage_desktop.h"
3536
#include "storage/src/include/firebase/storage.h"
3637
#include "storage/src/include/firebase/storage/common.h"
37-
#include "firebase/storage/list_result.h"
38-
#include "storage/src/desktop/list_result_desktop.h" // Defines the desktop internal::ListResultInternal
38+
#include "storage/src/include/firebase/storage/list_result.h"
3939

40-
// Note: app/src/future_manager.h is indirectly included via storage_reference_desktop.h -> reference_counted_future_impl.h
41-
// Note: app/src/include/firebase/app.h is indirectly included via storage_reference_desktop.h
40+
// Note: app/src/future_manager.h is indirectly included via
41+
// storage_reference_desktop.h -> reference_counted_future_impl.h Note:
42+
// app/src/include/firebase/app.h is indirectly included via
43+
// storage_reference_desktop.h
4244

4345
namespace firebase {
4446
namespace storage {
@@ -704,7 +706,7 @@ ReferenceCountedFutureImpl* StorageReferenceInternal::future() {
704706
}
705707

706708
Future<ListResult> StorageReferenceInternal::ListAll() {
707-
StorageReference self(this); // Create public object from internal 'this'
709+
StorageReference self(this); // Create public object from internal 'this'
708710
ReferenceCountedFutureImpl* ref_future =
709711
future()->Alloc<ListResult>(kStorageReferenceFnCount);
710712
Future<ListResult> future = MakeFuture(ref_future, self);
@@ -719,7 +721,7 @@ Future<ListResult> StorageReferenceInternal::ListAll() {
719721
}
720722

721723
Future<ListResult> StorageReferenceInternal::List(const char* page_token) {
722-
StorageReference self(this); // Create public object from internal 'this'
724+
StorageReference self(this); // Create public object from internal 'this'
723725
ReferenceCountedFutureImpl* ref_future =
724726
future()->Alloc<ListResult>(kStorageReferenceFnCount);
725727
Future<ListResult> future = MakeFuture(ref_future, self);

0 commit comments

Comments
 (0)