Commit 06c4128
committed
Here's the commit message I've drafted:
feat: Add stub List/ListAll methods with corrected PIMPL pattern
This commit introduces stub implementations for StorageReference's
`List()` and `ListAll()` methods in the Firebase C++ Storage SDK.
It follows a corrected PIMPL (pointer to implementation) pattern for
the `ListResult` class, strictly adhering to the internal architecture
observed in classes like `firebase::storage::Metadata` and its
`MetadataInternalCommon` helper.
This version addresses feedback from previous attempts and ensures:
1. **`ListResultInternalCommon` as Static Helpers**:
- A class `firebase::storage::internal::ListResultInternalCommon` is
defined within `storage/src/common/list_result.cc`.
- It contains only static methods responsible for managing the
lifecycle (cleanup registration, PIMPL deletion) of
`ListResult`'s internal implementation. This mirrors
`MetadataInternalCommon`.
2. **Platform-Specific `ListResultInternal`**:
- Each platform (Desktop, Android, iOS) defines its own concrete
`firebase::storage::internal::ListResultInternal` class.
- These are located in files like `storage/src/desktop/list_result_desktop.h`
(and `.cc`), `storage/src/android/list_result_android.h` (and `.cc`),
and `storage/src/ios/list_result_ios.h` (and `.mm`).
- These classes hold the stub data (empty item/prefix lists, empty
page token) and a pointer to their platform's
`StorageReferenceInternal` for context (primarily for cleanup).
- They provide a copy constructor for `ListResult`'s copy operations.
- They do not inherit from a common PIMPL base class.
3. **Public `ListResult` Refactoring**:
- The public `firebase::storage::ListResult` class now holds a raw
pointer (`internal::ListResultInternal* internal_`) to its
platform-specific PIMPL object.
- Its constructors, destructor, copy, and move assignment operators
utilize the static helper methods in `ListResultInternalCommon`
to manage the `internal_` pointer's lifecycle and cleanup
registration, similar to how `Metadata` manages its `internal_`.
4. **`StorageReference` Methods Update**:
- `StorageReference::ListAll()` and `StorageReference::List()` in
`storage/src/common/storage_reference.cc` now correctly create
`ListResult` instances. They instantiate the appropriate
platform-specific `internal::ListResultInternal` via a direct
`new internal::ListResultInternal(...)` call (relying on the
build system to link the correct platform's implementation) and
pass this PIMPL object to a private constructor of `ListResult`.
5. **Build System**:
- `storage/CMakeLists.txt` was updated to include the correctly named
platform-specific source files (e.g., `list_result_desktop.cc`).
6. **Integration Tests**:
- Existing integration tests were reviewed and confirmed to be
compatible with these internal changes.
This implementation provides the `List` and `ListAll` API as stubs,
returning empty results, with a robust internal structure that aligns
with the SDK's established PIMPL and lifecycle management patterns.1 parent fe7932c commit 06c4128
File tree
17 files changed
+457
-253
lines changed- storage
- src
- android
- common
- desktop
- include/firebase/storage
- ios
17 files changed
+457
-253
lines changed| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
38 | 38 | | |
39 | 39 | | |
40 | 40 | | |
41 | | - | |
| 41 | + | |
| 42 | + | |
42 | 43 | | |
43 | 44 | | |
44 | 45 | | |
| |||
47 | 48 | | |
48 | 49 | | |
49 | 50 | | |
50 | | - | |
| 51 | + | |
| 52 | + | |
51 | 53 | | |
52 | 54 | | |
53 | 55 | | |
| |||
58 | 60 | | |
59 | 61 | | |
60 | 62 | | |
61 | | - | |
| 63 | + | |
| 64 | + | |
62 | 65 | | |
63 | 66 | | |
64 | 67 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
23 | 23 | | |
24 | 24 | | |
25 | 25 | | |
26 | | - | |
| 26 | + | |
27 | 27 | | |
28 | 28 | | |
29 | 29 | | |
| |||
735 | 735 | | |
736 | 736 | | |
737 | 737 | | |
738 | | - | |
739 | | - | |
740 | | - | |
741 | | - | |
742 | | - | |
743 | | - | |
744 | 738 | | |
745 | 739 | | |
746 | 740 | | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
23 | 23 | | |
24 | 24 | | |
25 | 25 | | |
26 | | - | |
| 26 | + | |
27 | 27 | | |
28 | 28 | | |
29 | 29 | | |
| |||
154 | 154 | | |
155 | 155 | | |
156 | 156 | | |
157 | | - | |
158 | | - | |
159 | | - | |
160 | | - | |
161 | 157 | | |
162 | 158 | | |
163 | 159 | | |
| |||
0 commit comments