Skip to content

Commit 0a56e13

Browse files
PavloNetrebchukPavloNetrebchuk
committed
refactor: Address NestedBlockDepth warnings by reducing block nesting
1 parent 920499c commit 0a56e13

File tree

11 files changed

+280
-245
lines changed

11 files changed

+280
-245
lines changed

‎app/src/main/java/org/openedx/app/data/networking/HandleErrorInterceptor.kt‎

Lines changed: 28 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -14,37 +14,39 @@ class HandleErrorInterceptor(
1414
override fun intercept(chain: Interceptor.Chain): Response {
1515
val response = chain.proceed(chain.request())
1616

17-
val responseCode = response.code
18-
if (responseCode in 400..500 && response.body != null) {
19-
val jsonStr = response.body!!.string()
20-
21-
try {
22-
val errorResponse = gson.fromJson(jsonStr, ErrorResponse::class.java)
23-
if (errorResponse?.error != null) {
24-
when (errorResponse.error) {
25-
ERROR_INVALID_GRANT -> {
26-
throw EdxError.InvalidGrantException()
27-
}
28-
29-
ERROR_USER_NOT_ACTIVE -> {
30-
throw EdxError.UserNotActiveException()
31-
}
32-
33-
else -> {
34-
return response
35-
}
36-
}
37-
} else if (errorResponse?.errorDescription != null) {
38-
throw EdxError.ValidationException(errorResponse.errorDescription ?: "")
39-
}
40-
} catch (e: JsonSyntaxException) {
41-
throw IOException("JsonSyntaxException $jsonStr", e)
42-
}
17+
if (isErrorResponse(response)) {
18+
val jsonStr = response.body?.string() ?: return response
19+
return handleErrorResponse(response, jsonStr)
4320
}
4421

4522
return response
4623
}
4724

25+
private fun isErrorResponse(response: Response): Boolean {
26+
return response.code in 400..500 && response.body != null
27+
}
28+
29+
private fun handleErrorResponse(response: Response, jsonStr: String): Response {
30+
return try {
31+
val errorResponse = gson.fromJson(jsonStr, ErrorResponse::class.java)
32+
handleParsedErrorResponse(errorResponse) ?: response
33+
} catch (e: JsonSyntaxException) {
34+
throw IOException("JsonSyntaxException $jsonStr", e)
35+
}
36+
}
37+
38+
private fun handleParsedErrorResponse(errorResponse: ErrorResponse?): Response? {
39+
val exception = when {
40+
errorResponse?.error == ERROR_INVALID_GRANT -> EdxError.InvalidGrantException()
41+
errorResponse?.error == ERROR_USER_NOT_ACTIVE -> EdxError.UserNotActiveException()
42+
errorResponse?.errorDescription != null ->
43+
EdxError.ValidationException(errorResponse.errorDescription.orEmpty())
44+
45+
else -> return null
46+
}
47+
throw exception
48+
}
49+
4850
companion object {
4951
const val ERROR_INVALID_GRANT = "invalid_grant"
5052
const val ERROR_USER_NOT_ACTIVE = "user_not_active"

‎app/src/main/java/org/openedx/app/deeplink/DeepLinkRouter.kt‎

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -78,9 +78,7 @@ class DeepLinkRouter(
7878
navigateToCourseDashboard(fm, deepLink, courseTitle)
7979
}
8080

81-
DeepLinkType.UNENROLL, DeepLinkType.REMOVE_BETA_TESTER -> { /* Just navigate to dashboard */
82-
}
83-
81+
DeepLinkType.UNENROLL, DeepLinkType.REMOVE_BETA_TESTER -> {} // Just navigate to dashboard
8482
DeepLinkType.COURSE_VIDEOS -> navigateToCourseVideos(fm, deepLink)
8583
DeepLinkType.COURSE_DATES -> navigateToCourseDates(fm, deepLink)
8684
DeepLinkType.COURSE_DISCUSSION -> navigateToCourseDiscussion(fm, deepLink)
@@ -94,8 +92,7 @@ class DeepLinkRouter(
9492
}
9593

9694
DeepLinkType.FORUM_COMMENT -> navigateToDiscussionCommentWithDiscussion(fm, deepLink)
97-
else -> { /* ignore */
98-
}
95+
else -> {} // ignore
9996
}
10097
}
10198

‎config/detekt.yml‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -76,9 +76,6 @@ complexity:
7676
ignoreAnnotatedFunctions: [ 'Composable' ]
7777
ignoreOverridden: true
7878
ignorePrivate: true
79-
NestedBlockDepth:
80-
active: true
81-
threshold: 6
8279
CyclomaticComplexMethod:
8380
active: true
8481
ignoreAnnotated: [ 'Composable' ]

‎core/src/main/java/org/openedx/core/domain/model/Block.kt‎

Lines changed: 9 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -158,33 +158,16 @@ data class EncodedVideos(
158158
}
159159

160160
private fun getDefaultVideoInfoForDownloading(): VideoInfo? {
161-
var result: VideoInfo? = null
162-
163-
if (isPreferredVideoInfo(mobileLow)) {
164-
result = mobileLow
165-
} else if (isPreferredVideoInfo(mobileHigh)) {
166-
result = mobileHigh
167-
} else if (isPreferredVideoInfo(desktopMp4)) {
168-
result = desktopMp4
169-
} else {
170-
fallback?.let {
171-
if (isPreferredVideoInfo(it) &&
172-
!VideoUtil.videoHasFormat(it.url, AppDataConstants.VIDEO_FORMAT_M3U8)
173-
) {
174-
result = fallback
175-
}
176-
}
177-
178-
if (result == null) {
179-
hls?.let {
180-
if (isPreferredVideoInfo(it)) {
181-
result = hls
182-
}
183-
}
184-
}
161+
return when {
162+
isPreferredVideoInfo(mobileLow) -> mobileLow
163+
isPreferredVideoInfo(mobileHigh) -> mobileHigh
164+
isPreferredVideoInfo(desktopMp4) -> desktopMp4
165+
fallback != null && isPreferredVideoInfo(fallback) &&
166+
!VideoUtil.videoHasFormat(fallback!!.url, AppDataConstants.VIDEO_FORMAT_M3U8) -> fallback
167+
168+
hls != null && isPreferredVideoInfo(hls) -> hls
169+
else -> null
185170
}
186-
187-
return result
188171
}
189172

190173
private fun isPreferredVideoInfo(videoInfo: VideoInfo?): Boolean {

‎core/src/main/java/org/openedx/core/module/download/AbstractDownloader.kt‎

Lines changed: 29 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -38,41 +38,43 @@ abstract class AbstractDownloader : KoinComponent {
3838
): DownloadResult {
3939
isCanceled = false
4040
return try {
41-
val response = downloadApi.downloadFile(url).body()
42-
if (response != null) {
43-
val file = File(path)
44-
if (file.exists()) {
45-
file.delete()
41+
val responseBody = downloadApi.downloadFile(url).body() ?: return DownloadResult.ERROR
42+
initializeFile(path)
43+
responseBody.byteStream().use { inputStream ->
44+
FileOutputStream(File(path)).use { outputStream ->
45+
writeToFile(inputStream, outputStream)
4646
}
47-
file.createNewFile()
48-
input = response.byteStream()
49-
currentDownloadingFilePath = path
50-
fos = FileOutputStream(file)
51-
fos.use { output ->
52-
val buffer = ByteArray(BUFFER_SIZE)
53-
var read: Int
54-
while (input!!.read(buffer).also { read = it } != -1) {
55-
output?.write(buffer, 0, read)
56-
}
57-
output?.flush()
58-
}
59-
DownloadResult.SUCCESS
60-
} else {
61-
DownloadResult.ERROR
6247
}
48+
DownloadResult.SUCCESS
6349
} catch (e: Exception) {
6450
e.printStackTrace()
65-
if (isCanceled) {
66-
DownloadResult.CANCELED
67-
} else {
68-
DownloadResult.ERROR
69-
}
51+
if (isCanceled) DownloadResult.CANCELED else DownloadResult.ERROR
7052
} finally {
71-
fos?.close()
72-
input?.close()
53+
closeResources()
7354
}
7455
}
7556

57+
private fun initializeFile(path: String) {
58+
val file = File(path)
59+
if (file.exists()) file.delete()
60+
file.createNewFile()
61+
currentDownloadingFilePath = path
62+
}
63+
64+
private fun writeToFile(inputStream: InputStream, outputStream: FileOutputStream) {
65+
val buffer = ByteArray(BUFFER_SIZE)
66+
var bytesRead: Int
67+
while (inputStream.read(buffer).also { bytesRead = it } != -1) {
68+
outputStream.write(buffer, 0, bytesRead)
69+
}
70+
outputStream.flush()
71+
}
72+
73+
private fun closeResources() {
74+
fos?.close()
75+
input?.close()
76+
}
77+
7678
suspend fun cancelDownloading() {
7779
isCanceled = true
7880
withContext(Dispatchers.IO) {

‎core/src/main/java/org/openedx/core/module/download/BaseDownloadViewModel.kt‎

Lines changed: 56 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -60,33 +60,54 @@ abstract class BaseDownloadViewModel(
6060

6161
private suspend fun updateDownloadModelsStatus(models: List<DownloadModel>) {
6262
val downloadModelMap = models.associateBy { it.id }
63-
for (item in downloadableChildrenMap) {
64-
var downloadingCount = 0
65-
var downloadedCount = 0
66-
item.value.forEach { blockId ->
67-
val downloadModel = downloadModelMap[blockId]
68-
if (downloadModel != null) {
69-
if (downloadModel.downloadedState.isWaitingOrDownloading) {
70-
downloadModelsStatus[blockId] = DownloadedState.DOWNLOADING
71-
downloadingCount++
72-
} else if (downloadModel.downloadedState.isDownloaded) {
73-
downloadModelsStatus[blockId] = DownloadedState.DOWNLOADED
74-
downloadedCount++
75-
}
76-
} else {
77-
downloadModelsStatus[blockId] = DownloadedState.NOT_DOWNLOADED
63+
64+
downloadableChildrenMap.forEach { (parentId, children) ->
65+
val (downloadingCount, downloadedCount) = updateChildrenStatus(children, downloadModelMap)
66+
updateParentStatus(parentId, children.size, downloadingCount, downloadedCount)
67+
}
68+
69+
downloadingModelsList = models.filter { it.downloadedState.isWaitingOrDownloading }
70+
_downloadingModelsFlow.emit(downloadingModelsList)
71+
}
72+
73+
private fun updateChildrenStatus(
74+
children: List<String>,
75+
downloadModelMap: Map<String, DownloadModel>
76+
): Pair<Int, Int> {
77+
var downloadingCount = 0
78+
var downloadedCount = 0
79+
80+
children.forEach { blockId ->
81+
val downloadModel = downloadModelMap[blockId]
82+
downloadModelsStatus[blockId] = when {
83+
downloadModel?.downloadedState?.isWaitingOrDownloading == true -> {
84+
downloadingCount++
85+
DownloadedState.DOWNLOADING
86+
}
87+
88+
downloadModel?.downloadedState?.isDownloaded == true -> {
89+
downloadedCount++
90+
DownloadedState.DOWNLOADED
7891
}
79-
}
8092

81-
downloadModelsStatus[item.key] = when {
82-
downloadingCount > 0 -> DownloadedState.DOWNLOADING
83-
downloadedCount == item.value.size -> DownloadedState.DOWNLOADED
8493
else -> DownloadedState.NOT_DOWNLOADED
8594
}
8695
}
8796

88-
downloadingModelsList = models.filter { it.downloadedState.isWaitingOrDownloading }
89-
_downloadingModelsFlow.emit(downloadingModelsList)
97+
return downloadingCount to downloadedCount
98+
}
99+
100+
private fun updateParentStatus(
101+
parentId: String,
102+
childrenSize: Int,
103+
downloadingCount: Int,
104+
downloadedCount: Int
105+
) {
106+
downloadModelsStatus[parentId] = when {
107+
downloadingCount > 0 -> DownloadedState.DOWNLOADING
108+
downloadedCount == childrenSize -> DownloadedState.DOWNLOADED
109+
else -> DownloadedState.NOT_DOWNLOADED
110+
}
90111
}
91112

92113
protected fun setBlocks(list: List<Block>) {
@@ -200,23 +221,27 @@ abstract class BaseDownloadViewModel(
200221
}
201222
}
202223

224+
@Suppress("NestedBlockDepth")
203225
protected fun addDownloadableChildrenForSequentialBlock(sequentialBlock: Block) {
204-
for (item in sequentialBlock.descendants) {
205-
allBlocks[item]?.let { blockDescendant ->
206-
if (blockDescendant.type == BlockType.VERTICAL) {
207-
for (unitBlockId in blockDescendant.descendants) {
208-
val block = allBlocks[unitBlockId]
209-
if (block?.isDownloadable == true) {
210-
val id = sequentialBlock.id
211-
val children = downloadableChildrenMap[id] ?: listOf()
212-
downloadableChildrenMap[id] = children + block.id
213-
}
226+
sequentialBlock.descendants.forEach { descendantId ->
227+
val blockDescendant = allBlocks[descendantId] ?: return@forEach
228+
229+
if (blockDescendant.type == BlockType.VERTICAL) {
230+
blockDescendant.descendants.forEach { unitBlockId ->
231+
val block = allBlocks[unitBlockId]
232+
if (block?.isDownloadable == true) {
233+
addDownloadableChild(sequentialBlock.id, block.id)
214234
}
215235
}
216236
}
217237
}
218238
}
219239

240+
private fun addDownloadableChild(parentId: String, childId: String) {
241+
val children = downloadableChildrenMap[parentId] ?: listOf()
242+
downloadableChildrenMap[parentId] = children + childId
243+
}
244+
220245
fun logBulkDownloadToggleEvent(toggle: Boolean) {
221246
logEvent(
222247
CoreAnalyticsEvent.VIDEO_BULK_DOWNLOAD_TOGGLE,

‎core/src/main/java/org/openedx/core/module/download/DownloadHelper.kt‎

Lines changed: 7 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -93,22 +93,14 @@ class DownloadHelper(
9393
}
9494

9595
private fun calculateDirectorySize(directory: File): Long {
96-
var size: Long = 0
96+
if (!directory.exists()) return 0
9797

98-
if (directory.exists()) {
99-
val files = directory.listFiles()
100-
101-
if (files != null) {
102-
for (file in files) {
103-
size += if (file.isDirectory) {
104-
calculateDirectorySize(file)
105-
} else {
106-
file.length()
107-
}
108-
}
98+
return directory.listFiles()?.sumOf { file ->
99+
if (file.isDirectory) {
100+
calculateDirectorySize(file)
101+
} else {
102+
file.length()
109103
}
110-
}
111-
112-
return size
104+
} ?: 0
113105
}
114106
}

0 commit comments

Comments
 (0)