Add NetworkModule to ApplicationModule and tests once migrated off of Moshi #1675
Assignees
Labels
Z-ibt
Temporary label for Ben to keep track of issues he's triaged.
Comments
|
Taking this. I was able to get Moshi to work in a separate Kotlin project that's going to download backend JSON structures & encode them in proto for the app. We either will outright remove Moshi, or update it to work based on that project (haven't decided yet). |
|
Since this is blocking some of @jcqli's work, going to go ahead and patch the repository to properly hook into Moshi. Going to think about how to actually verify this is working, too (e.g. by introducing additional testing). |
8 tasks
BenHenning
added a commit
that referenced
this issue
Mar 31, 2021
…le data module in Bazel (#3025) * Enable Moshi support in Bazel. This includes some reorganizing of the data module, but it introduces support for generating GAE models through Moshi. Corresponding mock services & tests are still failing, but //data & //:oppia build. * Make mock services build for testing module. * Add working data tests to Bazel build. This also fixes a regression from an earlier commit that led to the Moshi objects not actually being generated correctly, so the data service tests are good proof that Moshi is being used correctly. * Remove TODOs for #1675. * Lint fixes. * Bazel lint fix. Avoid relying on native java_library. * Fix broken Gradle build. This includes explicitly specifying okhttp version since Gradle was previously randomly picking one. * Add debug details for affected tests script. * Lint fixes. * Attempt to force Bazel cache to clear. This is an attempt to work around a theorized OOM happening when trying to compute the sibling build graph for changes build files. * Post-merge fix. * Fix another post-merge failure. This regressed in a recent PR likely due to other Bazel failures hiding it in CI. * Upgrade to Robolectric 4.4. This also moves off of Oppia's fork of robolectric-bazel. * Switch TopicRevisionFragmentTest to test image loaders. The test should have been using them before, but this might have been missed. The test image loaders are necessary in the Robolectric test environment, and something about Robolectric 4.4 + Bazel specifically exposed this.
BenHenning
added a commit
that referenced
this issue
Apr 17, 2021
…2907) * Fix typos * Create data class objets for feedback reporting * Remove topic database from feedback reporting proto * Finish adding all data class objects * Fix typo * Clarify comment * Apply review comments * Clarify proto enums * Update data class models after, add API key to NetworkModule * Create service mock and fake JSON data for testing * Add newline at eof * Add newline at eof * Write service tests * Update timestamp data type to be double * Linter * Undo changes to cache store test * Fix linter errors * Create protos for feedback report * Add bool to app startup state for crash * Define data objects and service for sending feebdack reports * Fix typo * Include unspecified for enum types * Add protos to model bazel * Fix typos * Create data class objets for feedback reporting * Remove topic database from feedback reporting proto * Finish adding all data class objects * Clarify comment * Fix typo * Apply review comments * Clarify proto enums * Update data class models after, add API key to NetworkModule * Create service mock and fake JSON data for testing * Add newline at eof * Add newline at eof * Write service tests * Update timestamp data type to be double * Linter * Undo changes to cache store test * Fix linter errors * Remove comma at end of data class Json items * Adjust service defintion * Create oneof for each issue category * Address review comments * Adjust test to execute and check for success * Update data classes to match protobufs * Remove unused imports * Clarify field names / data * Address review comments * Clarify locale * Apply review comments * Rename to AppContext * Add typo to issue * Update GAE models to match protos, fix names, add logs to app context * Fix linter error * Clarify country locale * Update service test to use JSON to make GAE object * Stub controller * Remove controller from this PR * Start management controller for feedback reporting * Apply review comments * Add api key and authorization class * Remove api key from network module * Add KDoc comments to explain GAE model fields * Fix typo in comment * Fix typo * Update URL path for service * Add functions to report management controller that fetches / saves reports in local store * Provide implementation for feedback reporting service * Apply review comments * Fix typo * Add service call to controller * Remove api key from moshi object to use in interceptor for request header * Adjust comment * Change JSON field name to match backend * Implement cases for different issues for user supplied info * Add context info to controller * Get event logs * Fix error message * Add headers using network interceptor * Create event logs * Resolve conflicts * Change topic_progress to completed_exploration_ids * Fix comment * Add entry point as a data class * Add entry point in controller * Start tests * Rename original interceptor into json prefix and change references * Separate header function for testing * Separate header function for testing * Inject context in interceptor * Add test cases for adding and replacing headers * Fix linter errors * Add comments to clarify controller * Configure bazel?' * Add annotation class for network api key * Bazel build? * Clarify country code for locales * Specify language code' * Fix names to match JSON * Mock retrofit and service in test * Move mock services to testing module * Update data fields to match backend * Read logcat logs from file * Test builds * Remove completed explorations list * Make fake file for logcat * Init mocks in test * Update codeowners and fix linter errors * Undo bazel changes * Move mock services and api utils to testing module * Remove extra newline * Change field names to match backend * Use class const for language codes * Explain logcat file helper in a comment * Fix tests * Fix bazel build * Add moshi for annotations in bazel? * Fix injects in test * Remove moshi from bazel * Change audio language code to audio language * Update data classes to match backend declarations * Fix linter errors * Use Callback for request * Update field to match JSON * Fix linter errors * Fix linter errors * Create language code extensions * Linter fixes * Use Call in service, change timestamp to int for seconds * Fix JSON test errors * Cleanup comments, code * Fix codeowners for JSON in data module * Move JSON to data module * Enable Moshi support in Bazel. This includes some reorganizing of the data module, but it introduces support for generating GAE models through Moshi. Corresponding mock services & tests are still failing, but //data & //:oppia build. * Make mock services build for testing module. * Add working data tests to Bazel build. This also fixes a regression from an earlier commit that led to the Moshi objects not actually being generated correctly, so the data service tests are good proof that Moshi is being used correctly. * Remove TODOs for #1675. * Lint fixes. * Bazel lint fix. Avoid relying on native java_library. * Fix broken Gradle build. This includes explicitly specifying okhttp version since Gradle was previously randomly picking one. * Add debug details for affected tests script. * Lint fixes. * Attempt to force Bazel cache to clear. This is an attempt to work around a theorized OOM happening when trying to compute the sibling build graph for changes build files. * Create module for report schema version * Add tests for schema validator * Add FeedbackReportingModule to the application component * Adjust injection for schema version, tests build and run * Add test to check fields of high-level feedback report * Add field checks for all data classes * Use helper to verify data classes * Add entry point to model bazel, remove items from domain bazel * Add bazel dependencies * Make when statement exhaustive with no-op blocks * Remove imports and udpate JSON field names * Nit fixes * Fix nits * Use JVM annotation to inject api key * Nit fixes * Update POST url * Add newly-named interceptors to bazel build * Use ContextExtensions to get package info * Set up test application in test * Use application component provider * Fix typo * Add extensions to view model bazel * Set test application * Use context values for testing header * Fix linter issues * Clean up test * Remove duplicate dependency naming * Add MockWebServer to controller test * Fix linter errors * Specify language code to string, add profile proto to utility bazel * Apply review comments * Fix import * Make function internal for testing use * Fix build linter errors * Remove network interceptors from services tests * Revert "Remove network interceptors from services tests" This reverts commit 18c958f. * Remove network interceptor from feedback reporting service test * Hook up mock web server to test * Remove domain controller changes from this branch * Update data objects to remove controller changes * Add comma * Update data objects * Name interceptor in json prefix test * Start server * Use guava executors? * Call okhttp directly for service * Use okhttp instead of retrofit for tests * Add retrofit to second test case * Use retrofit again * Fix missing space * Don't use mock retrofit * Use mock web server instead of mock retrofit for both interceptor tests * Make function private * Fix data class names in test * Remove test provider for mock service * Shut down mock server * Add not null assertion, set up server with retrofit * Call execute on okhttp client to specify request * Apply review comments * Apply review comments * Add null-type check for headers * Fix dependency * Update module paths * Add mockwebserver to data bazel build * Fix build for data tests Co-authored-by: Ben Henning <henning.benmax@gmail.com>
This was referenced Jul 29, 2021
BenHenning
added
the
Z-ibt
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Bazel doesn't support Moshi and, at the moment, Oppia Android's data module uses Moshi but is not functional. Files that were depending on NetworkModule were failing to build with Bazel because of this. So, NetworkModule has been removed from any files previously depending on it. Once data module is migrated off of Moshi and becomes fully functional with Bazel - these references will need to be re-populated.
The text was updated successfully, but these errors were encountered: