Skip to content

Commit 46df64d

Browse files
rohit9625rohit9625
authored
Prevent deletion of other structured data when editing depicts (commons-app#5741)
* restructure : minor changes to comments to improve readability * api: remove clear flag to prevent deletion of structured data * WikiBaseInterface: add new api methods Get Method: to get claims for an entity Post method: to delete claims * WikiBaseClient: add methods to handle response for new APIs * typo: update call to method with updated typo * DepictEditHelper: call update property method with entity id * refactor: dismiss progress dialog on error * DepictsDao: remove usage of runBlocking as it was blocking main thread Refactor methods to perform well with coroutines * refactor: update usage of method to match changes in DepictsDao * refactor: use named parameters to improve readability * claims: add new data classes to represent remove claims * WikidataEditService: modify update depicts property method Performs deletion of old claims and creation of new claims * refactor: make methods more organized
1 parent 31bb1a7 commit 46df64d

File tree

12 files changed

+188
-124
lines changed

12 files changed

+188
-124
lines changed

‎app/src/main/java/fr/free/nrw/commons/upload/WikiBaseInterface.kt

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
package fr.free.nrw.commons.upload
22

3+
import fr.free.nrw.commons.upload.depicts.Claims
34
import fr.free.nrw.commons.wikidata.WikidataConstants
45
import fr.free.nrw.commons.wikidata.mwapi.MwPostResponse
56
import fr.free.nrw.commons.wikidata.mwapi.MwQueryResponse
@@ -34,7 +35,7 @@ interface WikiBaseInterface {
3435
</MwPostResponse> */
3536
@Headers("Cache-Control: no-cache")
3637
@FormUrlEncoded
37-
@POST(WikidataConstants.MW_API_PREFIX + "action=wbeditentity&site=commonswiki&clear=1")
38+
@POST(WikidataConstants.MW_API_PREFIX + "action=wbeditentity&site=commonswiki")
3839
fun postEditEntityByFilename(
3940
@Field("title") filename: String,
4041
@Field("token") editToken: String,
@@ -59,4 +60,19 @@ interface WikiBaseInterface {
5960
@Field("language") language: String?,
6061
@Field("value") captionValue: String?
6162
): Observable<MwPostResponse>
63+
64+
@GET(WikidataConstants.MW_API_PREFIX + "action=wbgetclaims")
65+
fun getClaimsByProperty(
66+
@Query("entity") entityId: String,
67+
@Query("property") property: String
68+
) : Observable<Claims>
69+
70+
@Headers("Cache-Control: no-cache")
71+
@FormUrlEncoded
72+
@POST(WikidataConstants.MW_API_PREFIX + "action=wbeditentity")
73+
fun postDeleteClaims(
74+
@Field("token") editToken: String,
75+
@Field("id") entityId: String,
76+
@Field("data") data: String
77+
): Observable<MwPostResponse>
6278
}

‎app/src/main/java/fr/free/nrw/commons/upload/depicts/Claims.kt

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
package fr.free.nrw.commons.upload.depicts
2+
3+
import com.google.gson.annotations.SerializedName
4+
import fr.free.nrw.commons.wikidata.model.Statement_partial
5+
6+
data class Claims(
7+
@SerializedName(value = "claims")
8+
val claims: Map<String, List<Statement_partial>> = emptyMap()
9+
)

‎app/src/main/java/fr/free/nrw/commons/upload/depicts/DepictEditHelper.kt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ class DepictEditHelper @Inject constructor (notificationHelper: NotificationHelp
6969
*/
7070
private fun addDepiction(media: Media, depictions: List<String>): Observable<Boolean> {
7171
Timber.d("thread is adding depiction %s", Thread.currentThread().name)
72-
return wikidataEditService.updateDepictsProperty(media.filename, depictions)
72+
return wikidataEditService.updateDepictsProperty(media.pageId, depictions)
7373
}
7474

7575
/**

‎app/src/main/java/fr/free/nrw/commons/upload/depicts/DepictsDao.kt

Lines changed: 53 additions & 66 deletions
Original file line numberDiff line numberDiff line change
@@ -1,112 +1,99 @@
11
package fr.free.nrw.commons.upload.depicts
22

3-
import androidx.room.*
3+
import androidx.room.Dao
4+
import androidx.room.Delete
5+
import androidx.room.Insert
6+
import androidx.room.OnConflictStrategy
7+
import androidx.room.Query
48
import fr.free.nrw.commons.upload.structure.depictions.DepictedItem
9+
import kotlinx.coroutines.CoroutineScope
10+
import kotlinx.coroutines.Deferred
511
import kotlinx.coroutines.Dispatchers
12+
import kotlinx.coroutines.async
613
import kotlinx.coroutines.launch
7-
import kotlinx.coroutines.runBlocking
8-
import java.util.*
14+
import java.util.Date
915

1016
/**
1117
* Dao class for DepictsRoomDataBase
1218
*/
1319
@Dao
1420
abstract class DepictsDao {
15-
16-
/**
17-
* insert Depicts in DepictsRoomDataBase
18-
*/
21+
/** The maximum number of depicts allowed in the database. */
22+
private val maxItemsAllowed = 10
23+
1924
@Insert(onConflict = OnConflictStrategy.REPLACE)
2025
abstract suspend fun insert(depictedItem: Depicts)
21-
22-
/**
23-
* get all Depicts from roomdatabase
24-
*/
26+
2527
@Query("Select * From depicts_table order by lastUsed DESC")
26-
abstract suspend fun getAllDepict(): List<Depicts>
28+
abstract suspend fun getAllDepicts(): List<Depicts>
2729

28-
/**
29-
* get all Depicts which need to delete from roomdatabase
30-
*/
3130
@Query("Select * From depicts_table order by lastUsed DESC LIMIT :n OFFSET 10")
32-
abstract suspend fun getItemToDelete(n: Int): List<Depicts>
31+
abstract suspend fun getDepictsForDeletion(n: Int): List<Depicts>
3332

34-
/**
35-
* Delete Depicts from roomdatabase
36-
*/
3733
@Delete
3834
abstract suspend fun delete(depicts: Depicts)
3935

40-
lateinit var allDepict: List<Depicts>
41-
lateinit var listOfDelete: List<Depicts>
42-
4336
/**
44-
* get all depicts from DepictsRoomDatabase
37+
* Gets all Depicts objects from the database, ordered by lastUsed in descending order.
38+
*
39+
* @return A list of Depicts objects.
4540
*/
46-
fun depictsList(): List<Depicts> {
47-
runBlocking {
48-
launch(Dispatchers.IO) {
49-
allDepict = getAllDepict()
50-
}
51-
}
52-
return allDepict
41+
fun depictsList(): Deferred<List<Depicts>> = CoroutineScope(Dispatchers.IO).async {
42+
getAllDepicts()
5343
}
5444

5545
/**
56-
* insert Depicts in DepictsRoomDataBase
46+
* Inserts a Depicts object into the database.
47+
*
48+
* @param depictedItem The Depicts object to insert.
5749
*/
58-
fun insertDepict(depictes: Depicts) {
59-
runBlocking {
60-
launch(Dispatchers.IO) {
61-
insert(depictes)
62-
}
63-
}
50+
private fun insertDepict(depictedItem: Depicts) = CoroutineScope(Dispatchers.IO).launch {
51+
insert(depictedItem)
6452
}
6553

6654
/**
67-
* get all Depicts item which need to delete
55+
* Gets a list of Depicts objects that need to be deleted from the database.
56+
*
57+
* @param n The number of depicts to delete.
58+
* @return A list of Depicts objects to delete.
6859
*/
69-
fun getItemTodelete(number: Int): List<Depicts> {
70-
runBlocking {
71-
launch(Dispatchers.IO) {
72-
listOfDelete = getItemToDelete(number)
73-
}
74-
}
75-
return listOfDelete
60+
private suspend fun depictsForDeletion(n: Int): Deferred<List<Depicts>> = CoroutineScope(Dispatchers.IO).async {
61+
getDepictsForDeletion(n)
7662
}
7763

7864
/**
79-
* delete Depicts in DepictsRoomDataBase
65+
* Deletes a Depicts object from the database.
66+
*
67+
* @param depicts The Depicts object to delete.
8068
*/
81-
fun deleteDepicts(depictes: Depicts) {
82-
runBlocking {
83-
launch(Dispatchers.IO) {
84-
delete(depictes)
85-
}
86-
}
69+
private suspend fun deleteDepicts(depicts: Depicts) = CoroutineScope(Dispatchers.IO).launch {
70+
delete(depicts)
8771
}
8872

8973
/**
90-
* save Depicts in DepictsRoomDataBase
74+
* Saves a list of DepictedItems in the DepictsRoomDataBase.
9175
*/
9276
fun savingDepictsInRoomDataBase(listDepictedItem: List<DepictedItem>) {
93-
var numberofItemInRoomDataBase: Int
94-
val maxNumberOfItemSaveInRoom = 10
77+
CoroutineScope(Dispatchers.IO).launch {
78+
for (depictsItem in listDepictedItem) {
79+
depictsItem.isSelected = false
80+
insertDepict(Depicts(depictsItem, Date()))
81+
}
9582

96-
for (depictsItem in listDepictedItem) {
97-
depictsItem.isSelected = false
98-
insertDepict(Depicts(depictsItem, Date()))
83+
// Deletes old Depicts objects from the database if
84+
// the number of depicts exceeds the maximum allowed.
85+
deleteOldDepictions(depictsList().await().size)
9986
}
87+
}
10088

101-
numberofItemInRoomDataBase = depictsList().size
102-
// delete the depictItem from depictsroomdataBase when number of element in depictsroomdataBase is greater than 10
103-
if (numberofItemInRoomDataBase > maxNumberOfItemSaveInRoom) {
89+
private suspend fun deleteOldDepictions(depictsListSize: Int) {
90+
if(depictsListSize > maxItemsAllowed) {
91+
val depictsForDeletion = depictsForDeletion(depictsListSize).await()
10492

105-
val listOfDepictsToDelete: List<Depicts> =
106-
getItemTodelete(numberofItemInRoomDataBase)
107-
for (i in listOfDepictsToDelete) {
108-
deleteDepicts(i)
93+
for(depicts in depictsForDeletion) {
94+
deleteDepicts(depicts)
10995
}
11096
}
11197
}
112-
}
98+
}
99+

‎app/src/main/java/fr/free/nrw/commons/upload/depicts/DepictsPresenter.kt

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,9 @@ import io.reactivex.android.schedulers.AndroidSchedulers
1616
import io.reactivex.disposables.CompositeDisposable
1717
import io.reactivex.processors.PublishProcessor
1818
import io.reactivex.schedulers.Schedulers
19+
import kotlinx.coroutines.CoroutineScope
20+
import kotlinx.coroutines.Dispatchers
21+
import kotlinx.coroutines.launch
1922
import timber.log.Timber
2023
import java.lang.reflect.Proxy
2124
import java.util.*
@@ -223,9 +226,8 @@ class DepictsPresenter @Inject constructor(
223226
if (error is InvalidLoginTokenException) {
224227
view.navigateToLoginScreen();
225228
} else {
226-
Timber.e(
227-
"Failed to update depictions"
228-
)
229+
view.dismissProgressDialog()
230+
Timber.e("Failed to update depictions")
229231
}
230232
})
231233
)
@@ -268,10 +270,13 @@ class DepictsPresenter @Inject constructor(
268270
*/
269271
fun getRecentDepictedItems(): MutableList<DepictedItem> {
270272
val depictedItemList: MutableList<DepictedItem> = ArrayList()
271-
val depictsList = depictsDao.depictsList()
272-
for (i in depictsList.indices) {
273-
val depictedItem = depictsList[i].item
274-
depictedItemList.add(depictedItem)
273+
CoroutineScope(Dispatchers.IO).launch {
274+
val depictsList = depictsDao.depictsList().await()
275+
276+
for (i in depictsList.indices) {
277+
val depictedItem = depictsList[i].item
278+
depictedItemList.add(depictedItem)
279+
}
275280
}
276281
return depictedItemList
277282
}

‎app/src/main/java/fr/free/nrw/commons/wikidata/WikiBaseClient.kt

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@ import fr.free.nrw.commons.upload.WikiBaseInterface
88
import fr.free.nrw.commons.wikidata.mwapi.MwPostResponse
99
import fr.free.nrw.commons.wikidata.mwapi.MwQueryResponse
1010
import io.reactivex.Observable
11-
import timber.log.Timber
1211
import javax.inject.Inject
1312
import javax.inject.Named
1413
import javax.inject.Singleton
@@ -42,12 +41,29 @@ class WikiBaseClient @Inject constructor(
4241
}
4342
}
4443

44+
fun getClaimIdsByProperty(fileEntityId: String, property: String ): Observable<List<String>> {
45+
return wikiBaseInterface.getClaimsByProperty(fileEntityId, property).map { claimsResponse ->
46+
claimsResponse.claims[property]?.mapNotNull { claim -> claim.id } ?: emptyList()
47+
}
48+
}
49+
50+
fun postDeleteClaims(entityId: String, data: String): Observable<Boolean> {
51+
return csrfToken().switchMap { editToken ->
52+
wikiBaseInterface.postDeleteClaims(editToken, entityId, data)
53+
.map { response: MwPostResponse -> response.successVal == 1 }
54+
}
55+
}
56+
4557
fun getFileEntityId(uploadResult: UploadResult): Observable<Long> {
4658
return wikiBaseInterface.getFileEntityId(uploadResult.createCanonicalFileName())
4759
.map { response: MwQueryResponse -> response.query()!!.pages()!![0].pageId().toLong() }
4860
}
4961

50-
fun addLabelstoWikidata(fileEntityId: Long, languageCode: String?, captionValue: String?): Observable<MwPostResponse> {
62+
fun addLabelsToWikidata(
63+
fileEntityId: Long,
64+
languageCode: String?,
65+
captionValue: String?
66+
): Observable<MwPostResponse> {
5167
return csrfToken().switchMap { editToken ->
5268
wikiBaseInterface.addLabelstoWikidata(
5369
PAGE_ID_PREFIX + fileEntityId,

0 commit comments

Comments
 (0)