Unity native variables fetch #168
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThis update adds support for fetching variables within the Unity native environment. A new private field is introduced to track a fetch callback ID, and the corresponding fetch logic is added and integrated into the event management system. New event constants and additional cases in the event builders and event queue enable the handling of a new "FetchEvent" type. Methods for constructing fetch events and processing them in the event manager have been added, with proper error handling in place when expected components are not available. Changes
Sequence Diagram(s)sequenceDiagram
participant UVP as UnityNativePlatformVariable
participant UEM as UnityNativeEventManager
participant URB as UnityNativeRaisedEventBuilder
participant UEQM as UnityNativeEventQueueManager
participant Logger as Logger/ErrorHandler
UVP->>UEM: FetchVariables(callbackId)
UEM->>URB: BuildFetchEvent(type)
URB-->>UEM: Return eventDetails
UEM->>UEQM: Queue FetchEvent(eventDetails)
%% Event is processed by the event queue
UEQM-->>UVP: TriggerVariablesFetched (includes callbackId)
UVP->>Logger: Log success or errors if components missing
Possibly related PRs
Suggested reviewers
Poem
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
nzagorchev
changed the base branch from
unity_native_variables
to
unity_native_variables_responses
|
@CodeRabbit review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
CleverTap/Runtime/Native/UnityNativeWrapper/EventBuilders/UnityNativeRaisedEventBuilder.cs (1)
55-67: Event builder method follows existing patterns, but lacks validation.The
BuildFetchEventmethod correctly constructs an event for fetching variables, but unlike other builder methods in this class (such asBuildandBuildChargedEvent), it doesn't perform any validation on the inputtypeparameter.Consider adding validation for the
typeparameter to ensure only valid types are processed:internal UnityNativeEventBuilderResult<Dictionary<string, object>> BuildFetchEvent(int type) { + // Validate that type is a supported fetch type + if (type != UnityNativeConstants.Event.WZRK_FETCH_TYPE_VARIABLES) { + var validationError = new UnityNativeValidationResult( + UnityNativeValidationErrorCode.UnsupportedFetchType, + $"Unsupported fetch type: {type}"); + return new UnityNativeEventBuilderResult<Dictionary<string, object>>( + new List<UnityNativeValidationResult> { validationError }, + null); + } + var eventDetails = new Dictionary<string, object> { { UnityNativeConstants.Event.EVENT_NAME, UnityNativeConstants.Event.EVENT_WZRK_FETCH } }; var properties = new Dictionary<string, object> { { "t", type } }; eventDetails.Add(UnityNativeConstants.Event.EVENT_DATA, properties); return new UnityNativeEventBuilderResult<Dictionary<string, object>>(new List<UnityNativeValidationResult>(), eventDetails); }CleverTap/Runtime/Native/UnityNativeWrapper/UnityNativeEventManager.cs (2)
388-405: Ensure error handling is robust for fetch operations.The current implementation checks if
eventBuilderResult.EventResult == null, but theBuildFetchEventmethod doesn't include code paths that would return null results.Either remove the null check if it's unnecessary or ensure the
BuildFetchEventmethod properly handles error cases that could return null:internal void FetchVariables() { if (ShouldDeferEvent(() => { FetchVariables(); })) { return; } var eventBuilderResult = new UnityNativeRaisedEventBuilder(_eventValidator) .BuildFetchEvent(UnityNativeConstants.Event.WZRK_FETCH_TYPE_VARIABLES); - if (eventBuilderResult.EventResult == null) + if (eventBuilderResult.EventResult == null || eventBuilderResult.ValidationResults.Any(vr => !vr.IsSuccess)) + { + CleverTapLogger.LogError($"Failed to build fetch event: {string.Join(", ", eventBuilderResult.ValidationResults.Select(vr => vr.ErrorMsg))}"); return; + } var eventDetails = eventBuilderResult.EventResult; UnityNativeEvent @event = BuildEvent(UnityNativeEventType.FetchEvent, eventDetails, true); _eventQueueManager.QueueEvent(@event); }
400-400: Consider adding error logging for failed fetch attempts.When a fetch event can't be created, the method silently returns without providing any feedback.
Add error logging to help with debugging:
if (eventBuilderResult.EventResult == null) + { + CleverTapLogger.LogError("Failed to build fetch event: result was null"); return; + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
CleverTap/Runtime/Native/UnityNativePlatformVariable.cs(3 hunks)CleverTap/Runtime/Native/UnityNativeWrapper/Constants/UnityNativeConstants.cs(1 hunks)CleverTap/Runtime/Native/UnityNativeWrapper/EventBuilders/UnityNativeBaseEventBuilder.cs(1 hunks)CleverTap/Runtime/Native/UnityNativeWrapper/EventBuilders/UnityNativeRaisedEventBuilder.cs(1 hunks)CleverTap/Runtime/Native/UnityNativeWrapper/UnityNativeEventManager.cs(1 hunks)CleverTap/Runtime/Native/UnityNativeWrapper/UnityNativeEventQueueManager.cs(1 hunks)
🧰 Additional context used
🧬 Code Definitions (3)
CleverTap/Runtime/Native/UnityNativeWrapper/EventBuilders/UnityNativeBaseEventBuilder.cs (1)
CleverTap/Runtime/Native/UnityNativeWrapper/Constants/UnityNativeConstants.cs (2)
UnityNativeConstants(7-236)Event(104-152)
CleverTap/Runtime/Native/UnityNativePlatformVariable.cs (6)
CleverTap/Runtime/Common/CleverTapPlatformVariable.cs (2)
FetchVariables(63-71)FetchVariables(178-178)CleverTap/Runtime/Native/UnityNativeWrapper/UnityNativeEventManager.cs (1)
FetchVariables(388-405)CleverTap/Runtime/CleverTap.cs (1)
FetchVariables(576-577)CleverTap/Runtime/IOS/IOSPlatformVariable.cs (1)
FetchVariables(14-15)CleverTap/Runtime/Android/AndroidPlatformVariable.cs (1)
FetchVariables(15-16)CTExample/Assets/Scripts/Variables/Variables.cs (1)
FetchVariables(172-178)
CleverTap/Runtime/Native/UnityNativeWrapper/UnityNativeEventManager.cs (4)
CleverTap/Runtime/Native/UnityNativeWrapper/EventBuilders/UnityNativeBaseEventBuilder.cs (3)
Dictionary(19-56)Dictionary(58-63)Dictionary(65-110)CleverTap/Runtime/Native/UnityNativeWrapper/UnityNativeVarCache.cs (1)
Dictionary(206-209)CleverTap/Runtime/Native/UnityNativeWrapper/UnityNativeEventQueueManager.cs (1)
QueueEvent(47-65)CleverTap/Runtime/Native/UnityNativeWrapper/EventBuilders/UnityNativeRaisedEventBuilder.cs (2)
UnityNativeRaisedEventBuilder(7-128)UnityNativeRaisedEventBuilder(10-12)
🔇 Additional comments (7)
CleverTap/Runtime/Native/UnityNativeWrapper/UnityNativeEventQueueManager.cs (1)
55-55: Appropriate event handling addedThe addition of
UnityNativeEventType.FetchEventto be processed in the same way asRaisedEventis correct. This ensures fetch events are properly queued in the_raisedEventsQueuefor processing.CleverTap/Runtime/Native/UnityNativePlatformVariable.cs (3)
21-21: Well-initialized fetch callback trackingGood initialization of the callback ID to -1, which is an appropriate sentinel value indicating no active callback.
142-150: Successfully enhanced callback mechanismThe update to include the
callbackIdin the fetch response message is well-implemented. This ensures that the correct callback can be identified and triggered when the fetch completes.
193-204: Proper implementation with error handlingThe implementation of
FetchVariablescorrectly checks for null dependencies before proceeding and provides a clear error message if the platform variable isn't loaded. The method also properly stores the callback ID for later use.CleverTap/Runtime/Native/UnityNativeWrapper/Constants/UnityNativeConstants.cs (1)
111-112: Consistent constants for fetch functionalityThe added constants for fetch events are appropriate. Note that
wzrk_fetchis already listed in theRESTRICTED_NAMESlist (line 192), which is correct as these are system-reserved event names.CleverTap/Runtime/Native/UnityNativeWrapper/EventBuilders/UnityNativeBaseEventBuilder.cs (1)
32-33: Consistent event type handlingGood approach to handle
FetchEventthe same way asRaisedEventby adding it to the case statement without a break in between. This ensures both event types are processed with the same logic, avoiding code duplication.CleverTap/Runtime/Native/UnityNativeWrapper/UnityNativeEventManager.cs (1)
372-407: Good implementation of variables sync and fetch functionality.The new Variables region organizes related functionality together and follows the established patterns for event handling. The
FetchVariablesmethod correctly uses the newly addedBuildFetchEventmethod and properly handles event queueing.
nzagorchev
force-pushed
the
unity_native_variables_fetch
branch
from
ece4b03 to
6ec62ce
Compare
|
@CodeRabbit review |
✅ Actions performedReview triggered.
|
* Merge pull request #153 from CleverTap/setup_tests * Merge pull request #152 from CleverTap/unity_native_variables_define * Merge pull request #156 from CleverTap/unity_native_variables_merge * EventQueue changes (#159) * Unity native variables sync (#160) * Unity Native Variables - Handle Variables responses and Trigger Callbacks (#167) * Unity native variables fetch (#168) * Unity Native Variables - define and sync File variables (#169) * Launch on Unity Native only (#170) * Fix defineVars CORS issue (#174)
Overview
Implement Unity Native Variables Fetch. Trigger Variables Fetch callbacks.
Summary by CodeRabbit