feat: Allow users to provide Android context instance to AndroidContextUtil instance#361
feat: Allow users to provide Android context instance to AndroidContextUtil instance#361lgoldstein wants to merge 3 commits into
Conversation
lgoldstein
changed the title
lgoldstein
force-pushed
the
external-android-context
branch
from
f5640e6 to
5d93955
Compare
|
Reverted min. API level to 9, but left Java JDK at 7 |
tony19
left a comment
tony19
left a comment
There was a problem hiding this comment.
Thanks for this, @lgoldstein — providing the Context explicitly instead of relying on the AppGlobals.getInitialApplication() reflection hack is a solid improvement, especially given Google's non-SDK interface restrictions. The design (reduce to application context, hold in an AtomicReference, fall back to reflection) is clean.
A few edits I'd suggest before merging:
-
README references a non-existent class. The docs (and both code snippets) call
AndroidContextUtils.setApplicationContext(...)— plural — but the class isAndroidContextUtil(singular), so the examples won't compile as written. Should beAndroidContextUtil.setApplicationContext(...). -
No test coverage for the new API. Worth adding tests that (a) a provided context is used by
getContext(), and (b) clearing it (setApplicationContext(null)) falls back to the reflection workaround. BecauseCONTEXT_HOLDERisstatic, the holder should also be reset in the test setup/teardown so it can't leak between tests.
Two things that are fine as-is, for the record: the fully-qualified android.content.Context references are necessary (importing it would collide with ch.qos.logback.core.Context), and the sourceCompatibility 1.6 → 1.7 bump is needed for the new AtomicReference<>() diamond operator.
I've applied edits 1 and 2 on top of your change in #391 (draft) so the maintainer can review a ready version — full credit to you for the original work. Happy to fold these back here instead if you'd prefer.
Generated by Claude Code
|
Thanks for finally getting back on this - this is something I personally look forward to have in the upcoming version |
Adds AndroidContextUtil.setApplicationContext(Context) so an application can supply the Android Context instead of relying on the reflection-based AppGlobals.getInitialApplication() workaround, which may break under Google's non-SDK interface restrictions. The provided context is reduced to its application context and held in a static AtomicReference; getContext() prefers it and falls back to reflection when none was provided. Also modernizes hardcoded API-level literals to Build.VERSION_CODES constants and uses getLongVersionCode() on API 28+. This builds on the work from #361 by lgoldstein, adding test coverage for the new API (including static-holder reset between tests) and fixing the README example that referenced a non-existent "AndroidContextUtils" class. Claude-Session: https://claude.ai/code/session_01BBrtdK6G1WtVPvCxenWAXz Co-authored-by: Claude <noreply@anthropic.com>
|
Closed by #391 |
|
Thanks again for this, @lgoldstein. Your change has landed on Generated by Claude Code |
2 participants
Description
In order to support various special properties the code requires access to an Android context instance. By default, the framework uses a workaround based on reflection that seems to work for the time being. However, in view of Google's restrictions on non-SDK interfaces this code might not work anymore. Therefore, the framework should provide a special API that enables the application to provide an Android context instance that will be used instead of the workaround. See the README file for description as to how this feature would be used if necessary.
Linked Issues
None
Additional context
Note:
Java 6 level no longer supported. This should not be an issue as it is highly unlikely that the code will run on (extremely) old versions. However, if backward compatibility to Java 6 is still required, this setting can be reverted (though still need to fix the compilation error).