Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix #5: GAE Abstraction part 1 - Topic Page handler [Blocked: #85] #78

Merged
merged 29 commits into from
Sep 10, 2019
Merged

Fix #5: GAE Abstraction part 1 - Topic Page handler [Blocked: #85] #78

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
7269d15
Initial introduction of the data source module.
BenHenning Aug 27, 2019
ac16d10
Data module dependencies, retrofit and models
Aug 27, 2019
bf96010
Removed kapt from build.gradle in data
Aug 27, 2019
ef62740
Javadoc comments
Aug 27, 2019
93d703b
Introduce one possible integration with Dagger 2.
BenHenning Aug 28, 2019
3679c4c
Javadoc comments added
Aug 28, 2019
a4e0ad5
Reolved Javadoc mistakes
Aug 28, 2019
d187b56
Update build.gradle proto directive to use 'compile' instead of (#81)
Aug 28, 2019
d27d161
Merge remote-tracking branch 'upstream/introduce-data-module' into ga…
Aug 28, 2019
5d5da22
Renaming test cases
Aug 28, 2019
07b51d5
Test cases for TopicService
Aug 30, 2019
a52bb57
Made changes to OppiaGaeClient
Aug 30, 2019
1d6c39b
Made changes to OppiaGaeClient
Aug 30, 2019
b476e8e
TopicService test code using Json response
Sep 3, 2019
4aadf7a
Revert changes in codeStyles/Project.xml
Sep 3, 2019
d03a527
Revert changes in codeStyles/Project.xml
Sep 3, 2019
a380eaa
Optimise imports
Sep 3, 2019
1da9475
Check NetworkInterceptor part
Sep 3, 2019
842dc21
Fix part of #9: Introduce Retrofit classroom data handler service (#83)
veena14cs Sep 4, 2019
ffde992
Resolve typo
Sep 5, 2019
f5ae749
Merge branch 'gae-abstraction-base' of https://github.com/oppia/oppia…
Sep 5, 2019
0049548
Resolve merge conflicts with develop branch
Sep 5, 2019
cd94945
NetworkModule introduced
Sep 5, 2019
e392406
Daggerify NetworkInterceptor
Sep 5, 2019
5eccceb
Make retrofit dependency as api
Sep 5, 2019
1751f36
Javadocs and linking of oppia-web to models
Sep 5, 2019
798cf41
Changes in Javadoc, naming and test cases code
Sep 6, 2019
80249a8
Merge remote-tracking branch 'upstream/develop' into gae-abstraction-…
Sep 10, 2019
bb62037
Updated TODO comments and NetworkInterceptorTest
Sep 10, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Javadoc comments added
  • Loading branch information
Rajat Talesra committed Aug 28, 2019
commit 3679c4ca3aba9c3280033032a52712800fefa76f
4 changes: 4 additions & 0 deletions data/build.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
apply plugin: 'com.android.library'
apply plugin: 'kotlin-android'
apply plugin: 'kotlin-android-extensions'
apply plugin: 'kotlin-kapt'

android {
compileSdkVersion 28
Expand Down Expand Up @@ -50,5 +51,8 @@ dependencies {
)
androidTestImplementation('androidx.test:runner:1.2.0',
'androidx.test.espresso:espresso-core:3.2.0')

implementation project(":utility")

kapt("com.squareup.moshi:moshi-kotlin-codegen:1.8.0")
}
5 changes: 3 additions & 2 deletions data/src/main/java/org/oppia/data/Constants.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
package org.oppia.data

/** Constant values for data module will be defined here */
/** An object that contains constants for data module */
object Constants {

const val RESPONSE_SUCCESS = 200
/** Constant which defines successful API call */
const val HTTP_OK = 200

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@BenHenning Not sure how to dagger-ify this.

}
6 changes: 3 additions & 3 deletions data/src/main/java/org/oppia/data/backends/gae/NetworkInterceptor.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@ import java.io.IOException

/**
* Interceptor on top of Retrofit to modify requests and response
* The Interceptor intercepts requests and response and in every response
* it checks for XSSI_PREFIX and removes it to make a valid Json
*
* The Interceptor removes XSSI_PREFIX from every response to produce valid Json
*/
class NetworkInterceptor : Interceptor {

Expand All @@ -18,7 +18,7 @@ class NetworkInterceptor : Interceptor {
val request = chain.request()
val response = chain.proceed(request)

if (response.code() == Constants.RESPONSE_SUCCESS) {
if (response.code() == Constants.HTTP_OK) {
if (response.body() != null) {
var rawJson = response.body()!!.string()
rawJson = removeXSSIPrefixFromResponse(rawJson)
Expand Down
23 changes: 21 additions & 2 deletions data/src/main/java/org/oppia/data/backends/gae/NetworkSettings.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,29 @@
package org.oppia.data.backends.gae

/** NetworkSettings contains functions and constants specifically related to network only. */
/** An object that contains functions and constants specifically related to network only. */
object NetworkSettings {

private var isDeveloperMode: Boolean = true

/** DEVELOPER URL which connects to development server */
// TODO(#74): Move this to DI graph
const val BASE_URL = "https://oppia.org"
private const val DEVELOPER_URL = "https://oppia.org"
/** PRODUCTION URL which connects to production server */
private const val PROD_URL = "https://oppia.org"
/**
* Prefix in Json response for extra layer of security in API calls
*
* @see <a href="https://github.com/oppia/oppia/blob/8f9eed9652d7c2d318798792f3c2c38a909abc67/feconf.py#L319">XSSI_PREFIX</a>
*
* Remove this prefix from every Json response which is achieved in [NetworkInterceptor]
*/
const val XSSI_PREFIX = ")]}\'"
rt4914 marked this conversation as resolved.
Show resolved Hide resolved

fun getBaseUrl(): String {
return if (isDeveloperMode) {
DEVELOPER_URL
} else {
PROD_URL
}
}
}
4 changes: 2 additions & 2 deletions data/src/main/java/org/oppia/data/backends/gae/OppiaGaeClient.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import okhttp3.OkHttpClient
import retrofit2.Retrofit
import retrofit2.converter.moshi.MoshiConverterFactory

/** This file manages Retrofit configuration and connection with Oppia backend */
/** An object that contains the Retrofit configuration with the Oppia backend. */
object OppiaGaeClient {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's go ahead and move this to the Dagger graph so that we can inject TopicService directly. We probably don't need OppiaGaeClient at all, and we can also hide the retrofit instance. A pseudocode implementation of this is:

@Module
class NetworkModule {
  @Qualifier private annotation class OppiaRetrofit

  @OppiaRetrofit
  @Provides
  @Singleton
  fun provideRetrofitInstance() {
    return retrofit2.Retrofit.Builder().
      baseUrl(...)
      ...
      .build()
  }

  @Provides
  @Singleton
  fun provideTopicService(@OppiaRetrofit retrofit: Retrofit) {
    return retrofit.create(TopicService::class.java)
  }
}

NB: This module needs to be included in the application component's module list.

And with this module, we can now inject TopicService wherever we need it.


private var retrofit: Retrofit? = null
Expand All @@ -16,7 +16,7 @@ object OppiaGaeClient {
client.addInterceptor(NetworkInterceptor())

retrofit = retrofit2.Retrofit.Builder()
.baseUrl(NetworkSettings.BASE_URL)
.baseUrl(NetworkSettings.getBaseUrl())
.addConverterFactory(MoshiConverterFactory.create())
.client(client.build())
.build()
Expand Down