Skip to content

Commit fa9363e

Browse files
refactor: Align Windows Analytics stubs with SDK patterns
This commit incorporates detailed review feedback to align the Windows Analytics C++ implementation (src/analytics_desktop.cc) with common Firebase C++ SDK patterns for managing Futures and including headers. Key changes: 1. **Future Management for Stubbed APIs:** - Replaced the previous Future creation mechanism (MakeFuture) for stubbed functions (GetAnalyticsInstanceId, GetSessionId, and their LastResult counterparts) with the standard FutureData pattern. - Added a static FutureData instance (g_future_data), initialized in firebase::analytics::Initialize() and cleaned up in Terminate(). - Defined an internal AnalyticsFn enum for Future function tracking. - Stubbed Future-returning methods now use g_future_data to create and complete Futures. - As per feedback, these Futures are completed with error code 0 and a null error message. A LogWarning is now used to inform developers that the called API is not supported on Desktop. - Added checks for g_future_data initialization in these methods. 2. **Include Path Updates:** - Corrected #include paths for common Firebase headers (app.h, variant.h, future.h, log.h) to use full module-prefixed paths (e.g., "app/src/include/firebase/app.h") instead of direct "firebase/" paths. - Removed a stale include comment. These changes improve the consistency of the Windows Analytics stubs with the rest of the Firebase C++ SDK.
1 parent b829441 commit fa9363e

File tree

1 file changed

+65
-30
lines changed

1 file changed

+65
-30
lines changed

src/analytics_desktop.cc

Lines changed: 65 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -13,34 +13,53 @@
1313
// limitations under the License.
1414

1515
#include "analytics/src/windows/analytics_windows.h"
16-
#include "firebase/app.h"
17-
#include "firebase/analytics.h"
18-
#include "firebase/variant.h"
19-
#include "firebase/future.h"
20-
#include "firebase/log.h" // <-- New include
16+
#include "app/src/include/firebase/app.h"
17+
#include "analytics/src/include/firebase/analytics.h" // Path confirmed to remain as is
18+
#include "common/src/include/firebase/variant.h"
19+
#include "app/src/include/firebase/future.h"
20+
#include "app/src/include/firebase/log.h"
21+
#include "app/src/future_manager.h" // For FutureData
2122

2223
#include <vector>
2324
#include <string>
2425
#include <map>
2526

26-
// Error code for Analytics features not supported on this platform.
27-
const int kAnalyticsErrorNotSupportedOnPlatform = 1; // Or a more specific range
27+
// Error code for Analytics features not supported on this platform. (Will be removed)
28+
// const int kAnalyticsErrorNotSupportedOnPlatform = 1; // Or a more specific range
2829

2930
namespace firebase {
3031
namespace analytics {
3132

33+
namespace internal { // Or directly in firebase::analytics if that's the pattern
34+
// Functions that return a Future.
35+
enum AnalyticsFn {
36+
kAnalyticsFn_GetAnalyticsInstanceId = 0,
37+
kAnalyticsFn_GetSessionId,
38+
kAnalyticsFnCount // Must be the last enum value.
39+
};
40+
} // namespace internal
41+
42+
// Future data for analytics.
43+
// This is initialized in `Initialize()` and cleaned up in `Terminate()`.
44+
static FutureData* g_future_data = nullptr;
45+
3246
// Initializes the Analytics desktop API.
3347
// This function must be called before any other Analytics methods.
3448
void Initialize(const App& app) {
3549
// The 'app' parameter is not directly used by the underlying Google Analytics C API
3650
// for Windows for global initialization. It's included for API consistency
3751
// with other Firebase platforms.
38-
// (void)app; // Mark as unused if applicable by style guides.
52+
(void)app; // Mark as unused if applicable by style guides.
3953

4054
// The underlying Google Analytics C API for Windows does not have an explicit
4155
// global initialization function.
4256
// This function is provided for API consistency with other Firebase platforms
4357
// and for any future global initialization needs for the desktop wrapper.
58+
if (g_future_data) {
59+
LogWarning("Analytics: Initialize() called when already initialized.");
60+
} else {
61+
g_future_data = new FutureData(internal::kAnalyticsFnCount);
62+
}
4463
}
4564

4665
// Terminates the Analytics desktop API.
@@ -51,6 +70,12 @@ void Terminate() {
5170
// are managed at the point of their use (e.g., destroyed after logging).
5271
// This function is provided for API consistency with other Firebase platforms
5372
// and for any future global cleanup needs for the desktop wrapper.
73+
if (g_future_data) {
74+
delete g_future_data;
75+
g_future_data = nullptr;
76+
} else {
77+
LogWarning("Analytics: Terminate() called when not initialized or already terminated.");
78+
}
5479
}
5580

5681
static void ConvertParametersToGAParams(
@@ -342,37 +367,47 @@ void SetSessionTimeoutDuration(int64_t milliseconds) {
342367
}
343368

344369
Future<std::string> GetAnalyticsInstanceId() {
345-
// Not supported by the Windows C API.
346-
// Return a Future that is already completed with an error.
347-
firebase::FutureHandle handle; // Dummy handle for error
348-
// TODO(jules): Ensure g_future_api_table is appropriate or replace with direct Future creation.
349-
auto future = MakeFuture<std::string>(&firebase::g_future_api_table, handle);
350-
future.Complete(handle, kAnalyticsErrorNotSupportedOnPlatform, "GetAnalyticsInstanceId is not supported on Desktop.");
351-
return future;
370+
LogWarning("Analytics: GetAnalyticsInstanceId() is not supported on Desktop.");
371+
if (!g_future_data) {
372+
LogError("Analytics: API not initialized; call Initialize() first.");
373+
static firebase::Future<std::string> invalid_future; // Default invalid
374+
if (!g_future_data) return invalid_future; // Or some other error future
375+
}
376+
const auto handle =
377+
g_future_data->CreateFuture(internal::kAnalyticsFn_GetAnalyticsInstanceId, nullptr);
378+
g_future_data->CompleteFuture(handle, 0 /* error_code */, nullptr /* error_message_string */);
379+
return g_future_data->GetFuture<std::string>(handle);
352380
}
353381

354382
Future<std::string> GetAnalyticsInstanceIdLastResult() {
355-
// This typically returns the last result of the async call.
356-
// Since GetAnalyticsInstanceId is not supported, this also returns a failed future.
357-
firebase::FutureHandle handle;
358-
auto future = MakeFuture<std::string>(&firebase::g_future_api_table, handle);
359-
future.Complete(handle, kAnalyticsErrorNotSupportedOnPlatform, "GetAnalyticsInstanceId (and thus LastResult) is not supported on Desktop.");
360-
return future;
383+
if (!g_future_data) {
384+
LogError("Analytics: API not initialized; call Initialize() first.");
385+
static firebase::Future<std::string> invalid_future;
386+
return invalid_future;
387+
}
388+
return g_future_data->LastResult<std::string>(internal::kAnalyticsFn_GetAnalyticsInstanceId);
361389
}
362390

363391
Future<int64_t> GetSessionId() {
364-
// Not supported by the Windows C API.
365-
firebase::FutureHandle handle;
366-
auto future = MakeFuture<int64_t>(&firebase::g_future_api_table, handle);
367-
future.Complete(handle, kAnalyticsErrorNotSupportedOnPlatform, "GetSessionId is not supported on Desktop.");
368-
return future;
392+
LogWarning("Analytics: GetSessionId() is not supported on Desktop.");
393+
if (!g_future_data) {
394+
LogError("Analytics: API not initialized; call Initialize() first.");
395+
static firebase::Future<int64_t> invalid_future;
396+
return invalid_future;
397+
}
398+
const auto handle =
399+
g_future_data->CreateFuture(internal::kAnalyticsFn_GetSessionId, nullptr);
400+
g_future_data->CompleteFuture(handle, 0 /* error_code */, nullptr /* error_message_string */);
401+
return g_future_data->GetFuture<int64_t>(handle);
369402
}
370403

371404
Future<int64_t> GetSessionIdLastResult() {
372-
firebase::FutureHandle handle;
373-
auto future = MakeFuture<int64_t>(&firebase::g_future_api_table, handle);
374-
future.Complete(handle, kAnalyticsErrorNotSupportedOnPlatform, "GetSessionId (and thus LastResult) is not supported on Desktop.");
375-
return future;
405+
if (!g_future_data) {
406+
LogError("Analytics: API not initialized; call Initialize() first.");
407+
static firebase::Future<int64_t> invalid_future;
408+
return invalid_future;
409+
}
410+
return g_future_data->LastResult<int64_t>(internal::kAnalyticsFn_GetSessionId);
376411
}
377412

378413
} // namespace analytics

0 commit comments

Comments
 (0)