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.

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

Fixes part of #1096: Crashlytics in Domain layer. #1319

Merged
merged 40 commits into from
Jun 15, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
40 commits
Select commit Hold shift + click to select a range
8d50619
crashlytics wrapper
Sarthak2601 May 31, 2020
c3b97eb
wrapper check
Sarthak2601 May 31, 2020
fc73449
Test implementation of crashlytics mocks
vinitamurthi Jun 1, 2020
7eee6a5
Crashlytics Wrapper in utility
Sarthak2601 Jun 1, 2020
b71d8eb
nits
Sarthak2601 Jun 1, 2020
ce39c92
nits
Sarthak2601 Jun 2, 2020
682c5a7
nit
Sarthak2601 Jun 2, 2020
96b3fe5
Firebase config file and dependency changes
Sarthak2601 Jun 2, 2020
af362b8
nits
Sarthak2601 Jun 2, 2020
1291c04
nits | renaming of wrapper
Sarthak2601 Jun 3, 2020
1e50e5e
nits + interface
Sarthak2601 Jun 3, 2020
ef941b4
ktlint nits
Sarthak2601 Jun 3, 2020
26b5afd
json deletion for seperate PR
Sarthak2601 Jun 3, 2020
00866d4
new testing approach
Sarthak2601 Jun 4, 2020
7097fa1
Merge branch 'develop' into crashlytics_wrapper
Sarthak2601 Jun 4, 2020
e02be67
new testing approach
Sarthak2601 Jun 4, 2020
9ab53f2
nits
Sarthak2601 Jun 5, 2020
bfbc746
changes
Sarthak2601 Jun 5, 2020
fcd8bf8
changes
Sarthak2601 Jun 10, 2020
62a1e7f
testing related changes.
Sarthak2601 Jun 10, 2020
84517a8
Merge branch 'develop' into crashlytics_wrapper
Sarthak2601 Jun 10, 2020
bd8737e
Tests.
Sarthak2601 Jun 11, 2020
0033e7a
Merge remote-tracking branch 'origin/crashlytics_wrapper' into crashl…
Sarthak2601 Jun 11, 2020
db9a32e
module addition in domain files.
Sarthak2601 Jun 11, 2020
5ef8dd4
Merge branch 'develop' into crashlytics_wrapper
Sarthak2601 Jun 11, 2020
be040f1
Merge branch 'develop' into crashlytics_wrapper
Sarthak2601 Jun 12, 2020
baf0d00
lint fix
Sarthak2601 Jun 12, 2020
1155611
changes.
Sarthak2601 Jun 12, 2020
b747379
testLogReportingModule in app module
Sarthak2601 Jun 12, 2020
4fecd09
Robolectric tests passing.
Sarthak2601 Jun 12, 2020
b773b65
Merge branch 'develop' into crashlytics_wrapper
Sarthak2601 Jun 12, 2020
fc94de5
espresso tests running as well.
Sarthak2601 Jun 12, 2020
d609f07
Merge remote-tracking branch 'origin/crashlytics_wrapper' into crashl…
Sarthak2601 Jun 12, 2020
67c436a
early commit.
Sarthak2601 Jun 13, 2020
d9ac080
Merge branch 'develop' into exception-log-implementation
Sarthak2601 Jun 13, 2020
e896e1c
start.
Sarthak2601 Jun 13, 2020
4735e71
tests + implementation.
Sarthak2601 Jun 14, 2020
5001bab
annotation correction.
Sarthak2601 Jun 14, 2020
40c952b
additional tests.
Sarthak2601 Jun 14, 2020
c72849f
nits.
Sarthak2601 Jun 15, 2020
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
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import androidx.test.espresso.intent.matcher.IntentMatchers.hasComponent
import androidx.test.espresso.intent.matcher.IntentMatchers.hasExtra
import androidx.test.espresso.matcher.ViewMatchers.withId
import androidx.test.ext.junit.runners.AndroidJUnit4
import com.google.firebase.FirebaseApp
import org.hamcrest.CoreMatchers.allOf
import org.junit.After
import org.junit.Before
Expand All @@ -36,6 +37,7 @@ class StoryActivityTest {
@Before
fun setUp() {
Intents.init()
FirebaseApp.initializeApp(ApplicationProvider.getApplicationContext())
}

@After
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import kotlinx.coroutines.launch
import org.oppia.util.caching.AssetRepository
import org.oppia.util.caching.CacheAssetsLocally
import org.oppia.util.data.AsyncResult
import org.oppia.util.logging.ExceptionLogger
import org.oppia.util.logging.Logger
import org.oppia.util.threading.BackgroundDispatcher
import java.io.IOException
Expand All @@ -33,6 +34,7 @@ import kotlin.concurrent.withLock
class AudioPlayerController @Inject constructor(
private val logger: Logger,
private val assetRepository: AssetRepository,
private val exceptionLogger: ExceptionLogger,
@BackgroundDispatcher private val backgroundDispatcher: CoroutineDispatcher,
@CacheAssetsLocally private val cacheAssetsLocally: Boolean
) {
Expand Down Expand Up @@ -180,6 +182,7 @@ class AudioPlayerController @Inject constructor(
}
mediaPlayer.prepareAsync()
} catch (e: IOException) {
exceptionLogger.logException(e)
logger.e("AudioPlayerController", "Failed to set data source for media player", e)
}
playProgress?.value = AsyncResult.pending()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import androidx.lifecycle.MutableLiveData
import org.oppia.app.model.Exploration
import org.oppia.util.data.AsyncResult
import org.oppia.util.data.DataProviders
import org.oppia.util.logging.ExceptionLogger
import javax.inject.Inject

private const val EXPLORATION_DATA_PROVIDER_ID = "ExplorationDataProvider"
Expand All @@ -19,7 +20,8 @@ private const val EXPLORATION_DATA_PROVIDER_ID = "ExplorationDataProvider"
class ExplorationDataController @Inject constructor(
private val explorationProgressController: ExplorationProgressController,
private val explorationRetriever: ExplorationRetriever,
private val dataProviders: DataProviders
private val dataProviders: DataProviders,
private val exceptionLogger: ExceptionLogger
) {
/** Returns an [Exploration] given an ID. */
fun getExplorationById(id: String): LiveData<AsyncResult<Exploration>> {
Expand All @@ -45,6 +47,7 @@ class ExplorationDataController @Inject constructor(
explorationProgressController.beginExplorationAsync(explorationId)
MutableLiveData(AsyncResult.success<Any?>(null))
} catch (e: Exception) {
exceptionLogger.logException(e)
MutableLiveData(AsyncResult.failed(e))
}
}
Expand All @@ -58,6 +61,7 @@ class ExplorationDataController @Inject constructor(
explorationProgressController.finishExplorationAsync()
MutableLiveData(AsyncResult.success<Any?>(null))
} catch (e: Exception) {
exceptionLogger.logException(e)
MutableLiveData(AsyncResult.failed(e))
}
}
Expand All @@ -67,6 +71,7 @@ class ExplorationDataController @Inject constructor(
return try {
AsyncResult.success(explorationRetriever.loadExploration(explorationId))
} catch (e: Exception) {
exceptionLogger.logException(e)
AsyncResult.failed(e)
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import org.oppia.domain.classify.AnswerClassificationController
import org.oppia.util.data.AsyncDataSubscriptionManager
import org.oppia.util.data.AsyncResult
import org.oppia.util.data.DataProviders
import org.oppia.util.logging.ExceptionLogger
import java.util.concurrent.locks.ReentrantLock
import javax.inject.Inject
import javax.inject.Singleton
Expand All @@ -34,7 +35,8 @@ class ExplorationProgressController @Inject constructor(
private val dataProviders: DataProviders,
private val asyncDataSubscriptionManager: AsyncDataSubscriptionManager,
private val explorationRetriever: ExplorationRetriever,
private val answerClassificationController: AnswerClassificationController
private val answerClassificationController: AnswerClassificationController,
private val exceptionLogger: ExceptionLogger
) {
// TODO(#180): Add support for hints.
// TODO(#179): Add support for parameters.
Expand Down Expand Up @@ -142,6 +144,7 @@ class ExplorationProgressController @Inject constructor(
return MutableLiveData(AsyncResult.success(answerOutcome))
}
} catch (e: Exception) {
exceptionLogger.logException(e)
return MutableLiveData(AsyncResult.failed(e))
}
}
Expand Down Expand Up @@ -177,6 +180,7 @@ class ExplorationProgressController @Inject constructor(
return MutableLiveData(AsyncResult.success(hint))
}
} catch (e: Exception) {
exceptionLogger.logException(e)
return MutableLiveData(AsyncResult.failed(e))
}
}
Expand Down Expand Up @@ -213,6 +217,7 @@ class ExplorationProgressController @Inject constructor(
return MutableLiveData(AsyncResult.success(solution))
}
} catch (e: Exception) {
exceptionLogger.logException(e)
return MutableLiveData(AsyncResult.failed(e))
}
}
Expand Down Expand Up @@ -244,6 +249,7 @@ class ExplorationProgressController @Inject constructor(
}
return MutableLiveData(AsyncResult.success<Any?>(null))
} catch (e: Exception) {
exceptionLogger.logException(e)
return MutableLiveData(AsyncResult.failed(e))
}
}
Expand Down Expand Up @@ -279,6 +285,7 @@ class ExplorationProgressController @Inject constructor(
}
return MutableLiveData(AsyncResult.success<Any?>(null))
} catch (e: Exception) {
exceptionLogger.logException(e)
return MutableLiveData(AsyncResult.failed(e))
}
}
Expand Down Expand Up @@ -310,6 +317,7 @@ class ExplorationProgressController @Inject constructor(
return try {
retrieveCurrentStateWithinCacheAsync()
} catch (e: Exception) {
exceptionLogger.logException(e)
AsyncResult.failed(e)
}
}
Expand Down Expand Up @@ -340,6 +348,7 @@ class ExplorationProgressController @Inject constructor(
finishLoadExploration(exploration!!, explorationProgress)
AsyncResult.success(explorationProgress.stateDeck.getCurrentEphemeralState())
} catch (e: Exception) {
exceptionLogger.logException(e)
AsyncResult.failed<EphemeralState>(e)
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import org.oppia.domain.topic.RATIOS_EXPLORATION_ID_2
import org.oppia.domain.topic.RATIOS_EXPLORATION_ID_3
import org.oppia.domain.util.JsonAssetRetriever
import org.oppia.domain.util.StateRetriever
import org.oppia.util.logging.ExceptionLogger
import java.io.IOException
import javax.inject.Inject

Expand All @@ -25,7 +26,8 @@ const val TEST_EXPLORATION_ID_7 = "3"
/** Internal class for actually retrieving an exploration object for uses in domain controllers. */
class ExplorationRetriever @Inject constructor(
private val jsonAssetRetriever: JsonAssetRetriever,
private val stateRetriever: StateRetriever
private val stateRetriever: StateRetriever,
private val exceptionLogger: ExceptionLogger
) {
// TODO(#169): Force callers of this method on a background thread.
/** Loads and returns an exploration for the specified exploration ID, or fails. */
Expand Down Expand Up @@ -58,6 +60,7 @@ class ExplorationRetriever @Inject constructor(
.putAllStates(createStatesFromJsonObject(explorationObject.getJSONObject("states")))
.build()
} catch (e: IOException) {
exceptionLogger.logException(e)
throw(Throwable("Failed to load and parse the json asset file. %s", e))
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import org.oppia.data.persistence.PersistentCacheStore
import org.oppia.util.data.AsyncResult
import org.oppia.util.data.DataProvider
import org.oppia.util.data.DataProviders
import org.oppia.util.logging.ExceptionLogger
import org.oppia.util.logging.Logger
import org.oppia.util.profile.DirectoryManagementUtil
import java.io.File
Expand Down Expand Up @@ -57,7 +58,8 @@ class ProfileManagementController @Inject constructor(
cacheStoreFactory: PersistentCacheStore.Factory,
private val dataProviders: DataProviders,
private val context: Context,
private val directoryManagementUtil: DirectoryManagementUtil
private val directoryManagementUtil: DirectoryManagementUtil,
private val exceptionLogger: ExceptionLogger
) {
private var currentProfileId: Int = -1
private val profileDataStore = cacheStoreFactory.create("profile_database", ProfileDatabase.getDefaultInstance())
Expand Down Expand Up @@ -584,6 +586,7 @@ class ProfileManagementController @Inject constructor(
.compress(Bitmap.CompressFormat.PNG, /* quality= */ 100, fos)
}
} catch (e: Exception) {
exceptionLogger.logException(e)
logger.e("ProfileManagementController", "Failed to store user submitted avatar image", e)
return null
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import org.oppia.util.data.AsyncResult
import org.oppia.util.data.DataProvider
import org.oppia.util.data.DataProviders
import org.oppia.util.data.DataProviders.NestedTransformedDataProvider
import org.oppia.util.logging.ExceptionLogger
import java.util.concurrent.locks.ReentrantLock
import javax.inject.Inject
import javax.inject.Singleton
Expand All @@ -35,7 +36,8 @@ private const val EMPTY_QUESTIONS_LIST_DATA_PROVIDER_ID = "EmptyQuestionsListDat
class QuestionAssessmentProgressController @Inject constructor(
private val dataProviders: DataProviders,
private val asyncDataSubscriptionManager: AsyncDataSubscriptionManager,
private val answerClassificationController: AnswerClassificationController
private val answerClassificationController: AnswerClassificationController,
private val exceptionLogger: ExceptionLogger
) {
// TODO(#247): Add support for populating the list of skill IDs to review at the end of the training session.
// TODO(#248): Add support for the assessment ending prematurely due to learner demonstrating sufficient proficiency.
Expand Down Expand Up @@ -140,6 +142,7 @@ class QuestionAssessmentProgressController @Inject constructor(
return MutableLiveData(AsyncResult.success(answeredQuestionOutcome))
}
} catch (e: Exception) {
exceptionLogger.logException(e)
return MutableLiveData(AsyncResult.failed(e))
}
}
Expand Down Expand Up @@ -175,6 +178,7 @@ class QuestionAssessmentProgressController @Inject constructor(
}
return MutableLiveData(AsyncResult.success<Any?>(null))
} catch (e: Exception) {
exceptionLogger.logException(e)
return MutableLiveData(AsyncResult.failed(e))
}
}
Expand Down Expand Up @@ -227,6 +231,7 @@ class QuestionAssessmentProgressController @Inject constructor(
TrainStage.SUBMITTING_ANSWER -> AsyncResult.pending()
}
} catch (e: Exception) {
exceptionLogger.logException(e)
AsyncResult.failed(e)
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import org.oppia.domain.topic.TopicController
import org.oppia.util.data.AsyncResult
import org.oppia.util.data.DataProvider
import org.oppia.util.data.DataProviders
import org.oppia.util.logging.ExceptionLogger
import javax.inject.Inject
import javax.inject.Singleton
import kotlin.random.Random
Expand All @@ -20,6 +21,7 @@ class QuestionTrainingController @Inject constructor(
private val questionAssessmentProgressController: QuestionAssessmentProgressController,
private val topicController: TopicController,
private val dataProviders: DataProviders,
private val exceptionLogger: ExceptionLogger,
@QuestionCountPerTrainingSession private val questionCountPerSession: Int,
@QuestionTrainingSeed private val questionTrainingSeed: Long
) {
Expand Down Expand Up @@ -49,6 +51,7 @@ class QuestionTrainingController @Inject constructor(
) { it }
dataProviders.convertToLiveData(erasedDataProvider)
} catch (e: Exception) {
exceptionLogger.logException(e)
MutableLiveData(AsyncResult.failed(e))
}
}
Expand Down Expand Up @@ -98,6 +101,7 @@ class QuestionTrainingController @Inject constructor(
questionAssessmentProgressController.finishQuestionTrainingSession()
MutableLiveData(AsyncResult.success<Any?>(null))
} catch (e: Exception) {
exceptionLogger.logException(e)
MutableLiveData(AsyncResult.failed(e))
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ import org.oppia.domain.util.StateRetriever
import org.oppia.util.data.AsyncResult
import org.oppia.util.data.DataProvider
import org.oppia.util.data.DataProviders
import org.oppia.util.logging.ExceptionLogger
import javax.inject.Inject
import javax.inject.Singleton

Expand Down Expand Up @@ -104,7 +105,8 @@ class TopicController @Inject constructor(
private val dataProviders: DataProviders,
private val jsonAssetRetriever: JsonAssetRetriever,
private val stateRetriever: StateRetriever,
private val storyProgressController: StoryProgressController
private val storyProgressController: StoryProgressController,
private val exceptionLogger: ExceptionLogger
) {

/**
Expand Down Expand Up @@ -200,6 +202,7 @@ class TopicController @Inject constructor(
try {
AsyncResult.success(retrieveReviewCard(topicId, subtopicId))
} catch (e: Exception) {
exceptionLogger.logException(e)
AsyncResult.failed<RevisionCard>(e)
}
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@ import org.mockito.Mockito.verify
import org.mockito.junit.MockitoJUnit
import org.mockito.junit.MockitoRule
import org.oppia.domain.audio.AudioPlayerController.PlayStatus
import org.oppia.testing.FakeExceptionLogger
import org.oppia.testing.TestLogReportingModule
import org.oppia.util.caching.CacheAssetsLocally
import org.oppia.util.data.AsyncResult
import org.oppia.util.logging.EnableConsoleLog
Expand All @@ -39,6 +41,7 @@ import org.robolectric.Shadows
import org.robolectric.annotation.Config
import org.robolectric.shadows.ShadowMediaPlayer
import org.robolectric.shadows.util.DataSource
import java.io.IOException
import javax.inject.Inject
import javax.inject.Qualifier
import javax.inject.Singleton
Expand Down Expand Up @@ -73,11 +76,13 @@ class AudioPlayerControllerTest {
@Inject lateinit var context: Context

@Inject lateinit var audioPlayerController: AudioPlayerController

@Inject lateinit var fakeExceptionLogger: FakeExceptionLogger
private lateinit var shadowMediaPlayer: ShadowMediaPlayer

private val TEST_URL = "https://www.soundhelix.com/examples/mp3/SoundHelix-Song-1.mp3"
private val TEST_URL2 = "https://www.soundhelix.com/examples/mp3/SoundHelix-Song-2.mp3"

private val TEST_FAIL_URL = "https://www.soundhelix.com/examples/mp3/SoundHelix-Song-2"
@Before
fun setUp() {
setUpTestApplicationComponent()
Expand Down Expand Up @@ -398,6 +403,18 @@ class AudioPlayerControllerTest {
assertThat(exception).hasMessageThat().contains("Media Player not in a prepared state")
}

@Test
fun testController_initializePlayer_invokePrepared_reportsfailure_logsException() {
audioPlayerController.initializeMediaPlayer()
audioPlayerController.changeDataSource(TEST_FAIL_URL)

shadowMediaPlayer.invokePreparedListener()
val exception = fakeExceptionLogger.getMostRecentException()

assertThat(exception).isInstanceOf(IOException::class.java)
assertThat(exception).hasMessageThat().contains("Invalid URL")
}

private fun arrangeMediaPlayer() {
audioPlayerController.initializeMediaPlayer().observeForever(mockAudioPlayerObserver)
audioPlayerController.changeDataSource(TEST_URL)
Expand All @@ -407,9 +424,11 @@ class AudioPlayerControllerTest {
private fun addMediaInfo() {
val dataSource = DataSource.toDataSource(context , Uri.parse(TEST_URL))
val dataSource2 = DataSource.toDataSource(context , Uri.parse(TEST_URL2))
val dataSource3 = DataSource.toDataSource(context, Uri.parse(TEST_FAIL_URL))
val mediaInfo = ShadowMediaPlayer.MediaInfo(/* duration= */ 1000,/* preparationDelay= */ 0)
ShadowMediaPlayer.addMediaInfo(dataSource, mediaInfo)
ShadowMediaPlayer.addMediaInfo(dataSource2, mediaInfo)
ShadowMediaPlayer.addException(dataSource3, IOException("Invalid URL"))
}

// TODO(#89): Move to a common test library.
Expand Down Expand Up @@ -488,7 +507,7 @@ class AudioPlayerControllerTest {

// TODO(#89): Move this to a common test application component.
@Singleton
@Component(modules = [TestModule::class])
@Component(modules = [TestModule::class, TestLogReportingModule::class])
interface TestApplicationComponent {
@Component.Builder
interface Builder {
Expand Down
Loading