Fix #3728, #3817, #3810, part of #1607: Integrate automatic app string language selection support

[BenHenning]

Explanation

Fix #3728
Fix #3817
Fix #3810
Fix part of #1607

This PR introduces support for automatic app string translations based on the system language. Note that while Android takes care of most of the heavy lifting here, this PR is set up specifically to support both in-app language selection (which will be implemented in #52) and custom languages that the Android system doesn't natively support (such as Hinglish). Both of these have been tested manually to work with this system, but aren't yet hooked up to real situations. See the design document linked in & overview from #3794 for more details.

Two other major aspects that contribute to the complexity of this PR:

1. This PR migrates all uses of formatting constructs in both XML & Kotlin/Java to the new locale classes (see Fix part of #3728, #3729, #3730: Introduce infrastructure to support app & content string translations #3794) which has far-reaching effects
2. This PR modularizes the Bazel builds for view, fragment, and activity components to allow introducing some subpackages & specific tests within the app layer. This resulted in separating the components between interfaces & implementations with a runtime cast for injection (which sets up the codebase nicely for splitting out all injectors in future iterations). The implementation still needs to be built in with the rest of the codebase until the injectors are split out, but this is key prerequisite for that future work. If any reviewers are concerned with the new runtime checks that could introduce crashes, note that this entire situation will continue to get better as we move toward more modularization (& eventually Dagger Hilt which will allow us the same flexibilty without the runtime cast due to added compile-time code generation that we're currently missing).

How language selection works

At a high-level, language selection works by building an Android Locale from an OppiaLocaleContext that can be used for string transformations, bidirectional wrapping, date/time computations, and string selection. The locale context is computed based either on pre-configured language definitions (via textproto files introduced in #3794), or the system locale. The app currently defaults to the system locale, but #52 will introduce an in-app language picker that will let users overwrite this behavior to select a specific language, instead.

Note essentially all locale operations are forcibly hidden via API changes & regex checks behind domain & utility classes that can ensure the correct locale is used in the correct circumstance. For users, this generally comes down deciding between two things:

• Display vs. machine locale (display locales are for creating strings that will be shown to the user, and the machine locale is for all other cases)
• For display strings, whether to bidirectionally wrap arguments (generally, always use wrapping for the last transformation before the string is shown to the user)

There's a lot of complexity behind the formatting changes above, and both this & #3794's PR descriptions cover those in more detail.

The main complexity in this PR is around the app string piece. We can't actually tell Android (easily) to use a particular Locale when selecting a language. However, we can force it by overriding the default Locale in the application context & recreating the activity (if the locale changes), so long as we override the locale prior to activity initialization happening. This is why InjectableAppCompatActivity has changed a lot, and why bootstrapping is now needed in both SplashActivity & throughout tests. I'm not yet sure if this is the final approach we want to take for language selection, but it seems like it might be the only option if we want to either support an in-app language selector or non-native Android languages (since the resource qualifying will work even for non-standard or non-natively supported locales; I've tested app strings successfully with Hinglish as a proof-of-concept).

To aid with this complexity, a mixin was introduced to detect when to automatically recreate the activity. Some care is taken to ensure an infinite loop doesn't get introduced, either, since the app language can depend on the system language & the system language can trigger a configuration change/activity recreation from the system. Note that it's expected language selections from the system language menu can cause 2 activity recreations before the app stabilizes, but this is generally hidden from the user. This is also an area where we can probably make improvements in the future.

One key design philosophy here was ensuring that the display locale be fixed for the lifetime of the activity (hence the need for recreation). Besides the fact that the force-locale mechanism seems to work much better when done during activity initialization, this also simplifies the design since it allows the existing of an activity-scoped resource handler which can managed access to the display locale directly & automatically tie it to the activity's resources for a nicer resource retrieval mechanism. This has the added side effect of more complexity in tests, though, since we now need an activity in many more cases than we did before.

Finally, one design alternative that was considered (& is currently documented in the design doc for this project) was to use an intent to carry the Oppia locale (via its context) through the activity tree. This would have prevented #3800 at the cost of more boilerplate (especially around activity starting). However, I discovered during implementation that the activity's intent is not available in onAttachBase since it hasn't yet been initialized, so this approach isn't possible unfortunately (which is why the singleton needed to be introduced, instead).

New tests & test changes

New tests:

1. Many new regex checks were added to prohibit:
   • Using formatting functions both via databinding & Java/Kotlin utilities in favor of the new locale-based ones
   • Manually handling configuration changes (to guard against handling locale changes that could break the automatic switching mechanism--see previous section)
   • Subclassing Activity, DialogFragment, or AppCompatActivity directly (we should always subclass the injectable activities except when bootstrapping, i.e. SplashActivity); existing fragments have been updated to match this
   • Using BidiFormatter directly (bidirectional wrapping should happen through DisplayLocale)
   • Using non-string type specifiers or positional arguments in resource strings (only string types, i.e. '%s' should be used for better interoperability with bidirectional wrapping). This isn't enforced for non-translated strings, and string conversion can (and now does) happen in Kotlin instead of the formatter.
   • Using resource string retrieval methods from Resources directly (DisplayLocale should be used for this, too, to encourage explicitly using/not using bidi wrapping methods)
2. The changes to HomeActivityTest don't work in Espresso due to complexity around the recreation mechanism (& partly don't on Robolectric, either). Update automatic language switching mechanism to work correctly on Espresso/Robolectric #3840 is tracking iterating on this mechanism, though it's been verified to work manually & in QA testing.
3. New tests ultimately need to be added for end-to-end testing scenarios to ensure the recreation mechanism works correctly. Due to limited time, these weren't added in this PR. Add end-to-end tests for localization #3839 has been filed to track this work.

Test considerations:

• Some tests needed to be updated to utilize an activity so that they can gain access to the per-activity resource handler. TestActivity was introduced for this purpose, and may serve as our long-term test staging activity once Dagger Hilt provides a means to get rid of all specialized test activities.
• A new test rule has been introduced to provide the means to initialize the locale context for tests (to avoid needing to change it at runtime & wait for the DataProvider update and then needing to recreate the activity). Note that the error cases in this rule can't be tested with automated tests, but they've been manually verified.
• An activity recreator needed to be introduced since trying to recreate the activity in production code introduces a lot of issues with Robolectric. This isn't an ideal solution, but it seemed like the best option.
• Modules aren't tested in this PR (unlike Fix part of #3728, #3729, #3730: Introduce infrastructure to support app & content string translations #3794) to keep the PR smaller. These are trivial tests which can be added by future open source contributors as part of the 100% test coverage project. There's little risk in not testing these right now since peripheral tests that rely on the injection will fail if something breaks in the Dagger bindings.
• A new AndroidX dependency was added to make asserting intent subjects easier (& potentially other Android objects in the future) using Truth. Due to an interoperability issue, this led to a required update in our Truth proto lite dependency, and Truth itself (requiring 1 test fix & version changes in both Gradle & Bazel build configuration files, plus a Maven dependency re-pin in Bazel).
• TestActivity is in app/ instead of testing/ since it depends on app/ things. This works fine in Bazel, but introduces a cycle in the Gradle build graph, so this is a temporary measure until Gradle is removed.
• The new JUnit rule uses reflection rather than directly referencing app/ layer dependencies since the latter would require the testing module to depend on the app module (fine in Bazel, but in Gradle Android libraries cannot depend on Android applications)
• RunOn was updated to add support for both Bazel & Gradle test isolation since some new tests can't run on Gradle due to not pulling in a language configuration (see Fix part of #3728, #3729, #3730: Introduce infrastructure to support app & content string translations #3794 for an explanation on the new language configuration textprotos)
• Some test files needed to be excluded from the app build due to strong incompatibilities with the Gradle environment (namely, the missing textproto language configuration file). These are still running successfully in Bazel.
• OppiaDateTimeFormatterTest was removed without its tests being retained. This is partly because TextViewBindingAdapters doesn't have a test suite, and partly because these behaviors are already tested through MachineLocaleImpl's tests & HomeActivityTest. Add tests for TextViewBindingAdapters #3846 has been filed to track adding these & other tests to a new test suite for TextViewBindingAdapters.
• This PR updates the Gradle test workflows to ensure all separate Gradle jobs run regardless of the others failing (so that widespread errors can be fixed with fewer CI re-runs)

Specific known issues & fix explanations

• The app will crash repeatedly when Android kills it due to low-memory cases. Due to limited time & added complexity, that wasn't fixed in this PR. Handle low memory death crashes due to localization #3800 is tracking it.
• Toolbar titles sometimes don't automatically update with the system language. Toolbar titles sometimes don't automatically update when the system language changes #3838 is tracking.
• This PR fixes Bazel Binary APK frozen on Splash Screen #3810. The first version of the PR led to a scenario where multiple recreations could occur while also handling a language change. This has been fixed by removing the active observer in splash activity.
• This PR fixes [UninitializedPropertyAccessException] displayLocale in AppLanguageLocaleHandler #3817 for the same reason mentioned above.
• This PR fixes part of Modularize app module Bazel build #1607 per the app modularization explanation above (see the overview section at the top)

Script exemptions

• An a11y accessibility label exemption was added for the new TestActivity since it's a test activity
• See test considerations above for the test file exemptions regarding Dagger modules. The other exemptions should correspond to renames of existing files or newly introduced interfaces.
• DefineAppLanguageLocaleContext is tested as part of the rule's test suite which is why it's exempted (plus, annotations can't be directly tested, anyway, since they don't contain logic)
• A specific exemption was made for some test utilities where it was easier just to use the forbidden pattern (usually for formatting) rather than introducing a potentially inverted dependency or perform the refactor (such as a test in the utility layer depending on a utility from the domain layer)

Other miscellaneous considerations

• A lot of view models have been moved to the 'view models with resources' bucket in app/BUILD.bazel due to now needing access to resources since this PR moves string formatting to Kotlin (which in most cases means view models).
• Extensions needed to be introduced for JSON, Bundles, and WorkManager Data classes since they all have 'getString' methods which unfortunately overlap with the regex checks mentioned above. While it's a bit hacky, the easiest workaround was to proxy these methods so that only a few new exemptions needed to be added.
• OppiaDateTimeFormatter was removed in favor of the new AppLanguageResourceHandler which relies on DisplayLocale to compute a locale-friendly date/time

Essential Checklist

• The PR title and explanation each start with "Fix #bugnum: " (If this PR fixes part of an issue, prefix the title with "Fix part of #bugnum: ...".)
• Any changes to scripts/assets files have their rationale included in the PR explanation.
• The PR follows the style guide.
• The PR does not contain any unnecessary code changes from Android Studio (reference).
• The PR is made from a branch that's not called "develop" and is up-to-date with "develop".
• The PR is assigned to the appropriate reviewers (reference).

For UI-specific PRs only

Overview screenshots & video (with real RTL language screenshots)

Note that there aren't meaningful "before" screenshots for this feature since it's introducing infrastructural support for something that the system natively supports (automatic string switching when a language is chosen within the system). Screenshots & a video are provided below to help better understand the nature of the changes behind this PR. Also, note that only the home screen is covered for the portrait/landscape screen as an example. Every activity in the app is affected by this PR.

Portrait (with changes) -- English:

Portrait (with changes) -- Arabic:

Landscape (with changes) -- English:

Landscape (with changes) -- Arabic:

Video demonstration of the feature:
https://user-images.githubusercontent.com/12983742/134875862-4a3eba7a-430c-4120-880a-e93c1b4a4955.mp4

Accessibility overview

Accessibility isn't expected to be changed as part of this PR, but for thoroughness here's the same video above with Talkback enabled for both supported languages.

Video with Talkback for English -> Arabic:
https://user-images.githubusercontent.com/12983742/134876927-1a58e1ff-3d24-490e-b5cc-09aebbcdc763.mp4

Video with Talkback for English -> Brazilian Portuguese:
https://user-images.githubusercontent.com/12983742/134876968-3f6c885d-605b-4dea-befd-6f51b2b44c6a.mp4

Espresso tests

The two Espresso tests directly affected by this PR are HomeActivityTest & SplashActivityTest. As mentioned above, the former is having issues on Espresso so that's not being demonstrated here. See the following screenshot for SplashActivityTest changes passing:

[BenHenning]

Gradle CI failure:

org.oppia.android.testing.data.DataProviderTestMonitorTest > testEnsureNextResultIsFailing_differentValues_notified_wait_returnsLatest FAILED

Given this is a new suite, this could be a flake. I'll restart the CI job, but I'll also run the suite locally in this branch to see how flaky it actually is.

[BenHenning]

Looks like it's flaky and will require some investigationg:

//testing/src/test/java/org/oppia/android/testing/data:DataProviderTestMonitorTest FAILED in 4 out of 160 in 31.2

[rt4914]

LGTM, thanks.

[rt4914]

Should this be renamed to createActivityComponentImpl ?

[BenHenning]

The contract comes from ActivityComponentFactory which must return ActivityComponent (in order for the dependencies to build correctly in Gradle, and conceptually an impl shouldn't be returned). This is just overriding the return type to be more specific.

Updated this to return ActivityComponent instead since it doesn't need to actually be more specific in order to build.

[anandwana001]

Looked over test files. Need to look over the domain file as well in the next pass.

[oppiabot]

Unassigning @anandwana001 since the review is done.

[BenHenning]

I think both of your comments are addressed. PTAL @rt4914 & @anandwana001.

Also, @anandwana001 PTAL at eec5a9f specifically since it came in after your last review round.

[oppiabot]

Unassigning @BenHenning since a re-review was requested. @BenHenning, please make sure you have addressed all review comments. Thanks!

[anandwana001]

I think both of your comments are addressed. PTAL @rt4914 & @anandwana001.

Also, @anandwana001 PTAL at eec5a9f specifically since it came in after your last review round.

This LGTM

[BenHenning]

Ah, I forgot to include this earlier but https://app.buildbuddy.io/invocation/036f70b3-353d-4536-bc84-5f1ad6f4b9da are the results demonstrating that DataProviderMonitorTest is no longer flaky.

[oppiabot]

Unassigning @vinitamurthi since they have already approved the PR.

[anandwana001]

Domain files LGTM

[BenHenning]

Thanks all! Going ahead and merging this to avoid new changes causing conflicts.

@rt4914 if you have any concerns with my resolution of your comment above, please follow up with me. I'm taking your approval to mean that you're fine with me merging this as-is.