There are some issues on the Network layer

[hoangnguyen92dn]

• OkHttp provides the Interceptors mechanism to process the same step on every request/response, therefore I would suggest making use of Interceptors to handle Authorization as the most common way instead of using @Header on every request 😄

• With your implementation by validating the expiry time returned from the request, the application could receive unexpected authorization issue returns from the server-side, therefore, we should handle the refreshing mechanism on every 4xx error response rather than always validating expiry time on local 😄

survey-exam/data/src/main/java/com/example/nimblesurveys/data/repository/AuthRepositoryImpl.kt

Lines 45 to 54 in 3ae3ac7

	val tokenEntity =
	if (cachedToken.expiry <= timeRepository.getCurrentTime()) {
	val newToken = fetchNewToken(cachedToken.refreshToken)
	val tokenEntity = adapter.toEntity(newToken)
	authDao.deleteToken()
	authDao.insertToken(tokenEntity)
	tokenEntity
	} else {
	cachedToken
	}

[ryan-conway]

OkHttp provides the Interceptors mechanism to process the same step on every request/response, therefore I would suggest making use of Interceptors to handle Authorization as the most common way instead of using @Header on every request 😄

This is a good point, I'll make use of this advice 😄

With your implementation by validating the expiry time returned from the request, the application could receive unexpected authorization issue returns from the server-side, therefore, we should handle the refreshing mechanism on every 4xx error response rather than always validating expiry time on local 😄

My reasoning for this was if we could validate the expiry of a token before making a request, we could prevent an additional network call for the failed request. Although this makes sense if a token may be invalidated for any other reason 😄 I'll apply this as well

[ryan-conway]

Created an AuthInterceptor class and applied it to okHttpClient in PR #21

[hoangnguyen92dn]

@ryan-conway

1. I see you are using the No-Authentication to determine which API needs to attach the authorization header, with this implementation, we have to declare the specific endpoints with the header or not. So I'd like to share with you another approach by creating different network stuff (Api, Okhttp, Retrofit) instances with qualifier annotation, then we would separate the list of endpoints in a different kind of resource which requires the Authorization header or not.

survey-exam/data/src/main/java/com/example/nimblesurveys/data/api/auth/AuthInterceptor.kt

Lines 12 to 14 in 9b8b6c1

	override fun intercept(chain: Interceptor.Chain): Response {
	val hasAuth = chain.request().header("No-Authentication") == null
	val requestBuilder = chain.request().newBuilder()

2. Furthermore, IMO we should avoid passing the access token from public method to interceptor like this:

survey-exam/data/src/main/java/com/example/nimblesurveys/data/api/auth/AuthInterceptor.kt

Lines 28 to 30 in 9b8b6c1

	fun setAccessToken(token: Token) {
	this.token = token
	}

survey-exam/data/src/main/java/com/example/nimblesurveys/data/repository/AuthRepositoryImpl.kt

Lines 85 to 87 in 9b8b6c1

	private fun setInterceptorAuthToken(token: Token) {
	authInterceptor.setAccessToken(token)
	}

The Interceptor instance is injected into the Repository layer to just pass the access token, and the Interceptor now relies on the access token passed via setAccessToken , what happens if we are passing an invalid access token to the Interceptor? Why don't we get the access token from the Repository or UseCase as the source truth? 🤔

3. Regarding the refreshing access token, if you are implementing it like this, we always need to call authRepository.refreshAccessToken() once the request failed, right? The hint is you can use Authenticator to handle refreshing access token whenever the API endpoint returns 4xx error.

survey-exam/domain/src/main/java/com/example/nimblesurveys/domain/usecase/GetSurveysUseCase.kt

Lines 21 to 28 in 9b8b6c1

	if (result is Result.Failure && result.cause is UnauthorizedException) {
	result = try {
	authRepository.refreshAccessToken()
	getSurveysResult()
	} catch (e: Throwable) {
	Result.Failure(e)
	}
	}