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

Jump to bottom

Fix crash on API <24 and make sure this error will not occur again. #4962

Closed
wants to merge 3 commits into from
Closed

Fix crash on API <24 and make sure this error will not occur again. #4962

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
c15f90a
Remove temporary change on the "NewApi" lint error.
bmarty Jan 17, 2022
414d871
Create and use `removeIfCompat` (#4961)
bmarty Jan 17, 2022
8f30e84
Changelog
bmarty Jan 17, 2022
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
1 change: 1 addition & 0 deletions changelog.d/4962.bugfix
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fix crash on API <24 and make sure this error will not occur again.
27 changes: 27 additions & 0 deletions library/core-utils/src/main/java/im/vector/lib/core/utils/compat/MutableCollectionCompat.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
/*
* Copyright (c) 2022 New Vector Ltd
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package im.vector.lib.core.utils.compat

import android.os.Build

fun <E> MutableCollection<E>.removeIfCompat(predicate: (E) -> Boolean) {
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.N) {
removeIf(predicate)
} else {
removeAll(filter(predicate).toSet())
Copy link
Member Author

Choose a reason for hiding this comment

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

I have been asked to convert to Set for performance reason.

Copy link
Contributor

Choose a reason for hiding this comment

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

as this is only for the keys to remove there shouldn't be any changes for lists with duplicated items?

listOf(1, 1, 2).removeIfCompat { it == 2 }  // would still expect [1, 1]

Copy link
Member Author

Choose a reason for hiding this comment

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

Now that you say it, I am not sure what will happen with
listOf(1, 1, 2).removeIfCompat { it == 1 } // would still expect [2] but I think we will get [1, 2]
Let me play a bit with it.

Copy link
Member Author

Choose a reason for hiding this comment

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

Seems to work as expected:
image
(https://pl.kotl.in/vIVLDMXe-)

Copy link
Contributor

Choose a reason for hiding this comment

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

thanks for checking!

}
}
5 changes: 1 addition & 4 deletions vector/lint.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@
<issue id="RtlSymmetry" severity="error" />

<!-- Code -->
<issue id="NewApi" severity="error" />
Copy link
Contributor

Choose a reason for hiding this comment

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

💯

<issue id="SetTextI18n" severity="error" />
<issue id="ViewConstructor" severity="error" />
<issue id="UseValueOf" severity="error" />
Expand Down Expand Up @@ -82,10 +83,6 @@
<ignore path="**/generated/resolved/**/resolved.xml" />
</issue>

<!-- Bug in lint agp 4.1 incorrectly thinks kotlin forEach is using java 8 API's. -->
<!-- FIXME this workaround should be removed in a near future -->
<issue id="NewApi" severity="warning" />

<!-- DI -->
<issue id="JvmStaticProvidesInObjectDetector" severity="error" />
</lint>
5 changes: 3 additions & 2 deletions vector/src/main/java/im/vector/app/features/analytics/DecryptionFailureTracker.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package im.vector.app.features.analytics

import im.vector.app.core.time.Clock
import im.vector.app.features.analytics.plan.Error
import im.vector.lib.core.utils.compat.removeIfCompat
import im.vector.lib.core.utils.flow.tickerFlow
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.Dispatchers
Expand Down Expand Up @@ -89,7 +90,7 @@ class DecryptionFailureTracker @Inject constructor(
fun onTimeLineDisposed(roomId: String) {
scope.launch(Dispatchers.Default) {
synchronized(failures) {
failures.removeIf { it.roomId == roomId }
failures.removeIfCompat { it.roomId == roomId }
}
}
}
Expand All @@ -105,7 +106,7 @@ class DecryptionFailureTracker @Inject constructor(

private fun removeFailureForEventId(eventId: String) {
synchronized(failures) {
failures.removeIf { it.failedEventId == eventId }
failures.removeIfCompat { it.failedEventId == eventId }
}
}

Expand Down
10 changes: 2 additions & 8 deletions vector/src/main/java/im/vector/app/features/reactions/EmojiRecyclerAdapter.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ import androidx.transition.AutoTransition
import androidx.transition.TransitionManager
import im.vector.app.R
import im.vector.app.features.reactions.data.EmojiData
import im.vector.lib.core.utils.compat.removeIfCompat
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.launch
Expand Down Expand Up @@ -215,14 +216,7 @@ class EmojiRecyclerAdapter @Inject constructor() :
override fun onViewRecycled(holder: ViewHolder) {
if (holder is EmojiViewHolder) {
holder.data = null
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.N) {
toUpdateWhenNotBusy.removeIf { it.second == holder }
} else {
val index = toUpdateWhenNotBusy.indexOfFirst { it.second == holder }
if (index != -1) {
toUpdateWhenNotBusy.removeAt(index)
}
}
toUpdateWhenNotBusy.removeIfCompat { it.second == holder }
}
super.onViewRecycled(holder)
}
Expand Down