Feat/sdk 4030 updated: IncorrectContextViolation #763
Conversation
- uses version check and fixes windowmanager fetch - removes redundant code for pixes which was unused
- reduces service fetches from context - reduces version checks - refactors code to add placeholder class - returns window result as whole
- catches exception - removes decimal adjustment which was done twice.
WalkthroughThe changes refactor how device metrics are computed in the SDK. In the core module, separate pixel fields are replaced by a new method that calculates window dimensions and DPI via a unified Changes
Sequence Diagram(s)sequenceDiagram
participant DC as DeviceCachedInfo
participant API as Android API
participant WM as WindowManager
participant WS as WindowSize
DC->>DC: Invoke getWindowSizeData()
alt API Level >= 30
DC->>API: Retrieve window metrics (WindowMetrics)
else API Level < 30
DC->>API: Retrieve display metrics (DisplayMetrics)
end
API-->>DC: Return pixel dimensions and DPI
DC->>WS: Create WindowSize (width, height, dpi)
WS-->>DC: Return WindowSize object
DC->>WM: Retrieve WindowManager instance via getWindowManager()
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
⏰ Context from checks skipped due to timeout of 90000ms (2)
🔇 Additional comments (5)
✨ Finishing Touches
🪧 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 (
|
Anush-Shand
changed the title
Anush-Shand
changed the title
![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 (2)
clevertap-core/src/main/java/com/clevertap/android/sdk/DeviceInfo.java (2)
107-122: Consider logging whenWindowSizedefaults to zero-size.
IfgetWindowManager()returnsnull, the resulting zero width/height might cause silent behavior. Logging that situation can aid troubleshooting.if (wm == null) { + Logger.v("DeviceInfo", "WindowManager is null, returning zero dimension for width/height"); return new WindowSize(0, 0, 0); }
326-340: Log the caughtUnsupportedOperationException.
Catching the exception silently makes debugging harder ifcreateWindowContextfails. Consider logging it for clarity.} catch (UnsupportedOperationException e) { - wm = null; + Logger.d("DeviceInfo", "Creating WindowContext not supported: " + e.getMessage()); + wm = null; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
clevertap-core/src/main/java/com/clevertap/android/sdk/DeviceInfo.java(4 hunks)clevertap-core/src/test/java/com/clevertap/android/sdk/MockDeviceInfo.kt(0 hunks)
💤 Files with no reviewable changes (1)
- clevertap-core/src/test/java/com/clevertap/android/sdk/MockDeviceInfo.kt
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: lint-staticChecks-test-build
🔇 Additional comments (4)
clevertap-core/src/main/java/com/clevertap/android/sdk/DeviceInfo.java (4)
27-27: Import of NonNull looks good.
The addition of the@NonNullannotation is coherent with its subsequent usage.
88-88: Use of final forappBucketis appropriate.
DeclaringappBucketas final enhances immutability and clarity by ensuring it's only assigned once.
123-172: Check for a non-zero DPI to avoid potential numeric anomalies.
Currently, there's no guard for zero or negative DPI values. While unlikely, adding a safety check prevents edge-case issues (e.g., xdpi=0 leading to division by zero).double doubleWidth = 0; double doubleHeight = 0; if (localXDpi > 0 && localYDpi > 0) { doubleWidth = localWidth / localXDpi; doubleHeight = localHeight / localYDpi; } else { + Logger.v("DeviceInfo", "Detected zero DPI; skipping dimension calculation or using fallback"); }
311-325: Lightweight data holder approach is well-designed.
The newWindowSizeclass with final fields is straightforward, ensuring immutability.
Summary by CodeRabbit
Refactor
Tests
Fixes #674