Merge pull request #9893 from nextcloud/remoteIdNull

remoteId not needed when deleting a file

scripts/analysis/findbugs-results.txt

@@ -1 +1 @@
-641
+640

src/androidTest/java/com/owncloud/android/FileIT.java

@@ -12,6 +12,7 @@ import org.junit.runner.RunWith;
 import androidx.test.ext.junit.runners.AndroidJUnit4;
 
 import static junit.framework.TestCase.assertTrue;
+import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNull;
 
 /**
@@ -63,4 +64,17 @@ public class FileIT extends AbstractOnServerIT {
                                 getStorageManager())
             .execute(client);
     }
+
+    @Test
+    public void testRemoteIdNull() {
+        getStorageManager().deleteAllFiles();
+        assertEquals(0, getStorageManager().getAllFiles().size());
+
+        OCFile test = new OCFile("/123.txt");
+        getStorageManager().saveFile(test);
+        assertEquals(1, getStorageManager().getAllFiles().size());
+
+        getStorageManager().deleteAllFiles();
+        assertEquals(0, getStorageManager().getAllFiles().size());
+    }
 }

src/androidTest/java/com/owncloud/android/datamodel/FileDataStorageManagerContentResolverIT.kt

@@ -23,7 +23,6 @@ package com.owncloud.android.datamodel
 
 import org.junit.Assert
 import org.junit.Test
-import java.util.ArrayList
 
 class FileDataStorageManagerContentResolverIT : FileDataStorageManagerIT() {
     companion object {
@@ -41,7 +40,7 @@ class FileDataStorageManagerContentResolverIT : FileDataStorageManagerIT() {
     @Test
     fun testFolderWithManyFiles() {
         // create folder
-        val folderA = OCFile("/folderA/", "00001") // remote Id must never be null
+        val folderA = OCFile("/folderA/")
         folderA.setFolder().parentId = sut.getFileByDecryptedRemotePath("/")!!.fileId
         sut.saveFile(folderA)
         Assert.assertTrue(sut.fileExists("/folderA/"))
@@ -50,7 +49,7 @@ class FileDataStorageManagerContentResolverIT : FileDataStorageManagerIT() {
 
         // create files
         val newFiles = (1..MANY_FILES_AMOUNT).map {
-            val file = OCFile("/folderA/file$it", it.toString())
+            val file = OCFile("/folderA/file$it")
             file.parentId = folderAId
             sut.saveFile(file)
 

src/androidTest/java/com/owncloud/android/ui/activity/ConflictsResolveActivityIT.java

@@ -64,7 +64,7 @@ public class ConflictsResolveActivityIT extends AbstractIT {
         newFile.setModificationTimestamp(1522019340);
         newFile.setStoragePath(FileStorageUtils.getSavePath(user.getAccountName()) + "/nonEmpty.txt");
 
-        OCFile existingFile = new OCFile("/newFile.txt", "4546546");
+        OCFile existingFile = new OCFile("/newFile.txt");
         existingFile.setFileLength(1024000);
         existingFile.setModificationTimestamp(1582019340);
 
@@ -157,7 +157,7 @@ public class ConflictsResolveActivityIT extends AbstractIT {
                                           "/newFile.txt",
                                           user.getAccountName());
 
-        OCFile existingFile = new OCFile("/newFile.txt", "123221");
+        OCFile existingFile = new OCFile("/newFile.txt");
         existingFile.setFileLength(1024000);
         existingFile.setModificationTimestamp(1582019340);
 
@@ -198,7 +198,7 @@ public class ConflictsResolveActivityIT extends AbstractIT {
                                           "/newFile.txt",
                                           user.getAccountName());
 
-        OCFile existingFile = new OCFile("/newFile.txt", "121312");
+        OCFile existingFile = new OCFile("/newFile.txt");
         existingFile.setFileLength(1024000);
         existingFile.setModificationTimestamp(1582019340);
 
@@ -243,7 +243,7 @@ public class ConflictsResolveActivityIT extends AbstractIT {
                                           "/newFile.txt",
                                           user.getAccountName());
 
-        OCFile existingFile = new OCFile("/newFile.txt", "12354455");
+        OCFile existingFile = new OCFile("/newFile.txt");
         existingFile.setFileLength(1024000);
         existingFile.setModificationTimestamp(1582019340);
         existingFile.setRemoteId("123abc");
@@ -289,7 +289,7 @@ public class ConflictsResolveActivityIT extends AbstractIT {
                                           "/newFile.txt",
                                           user.getAccountName());
 
-        OCFile existingFile = new OCFile("/newFile.txt", "564564");
+        OCFile existingFile = new OCFile("/newFile.txt");
         existingFile.setFileLength(1024000);
         existingFile.setModificationTimestamp(1582019340);
 

src/androidTest/java/com/owncloud/android/ui/adapter/OCFileListAdapterIT.kt

@@ -38,34 +38,34 @@ class OCFileListAdapterIT : AbstractIT() {
         assertEquals(0, storageManager.getGalleryItems(startDate, endDate).size)
 
         // create dummy files
-        OCFile("/test.txt", "01").apply {
+        OCFile("/test.txt").apply {
             mimeType = "text/plain"
         }.let {
             storageManager.saveFile(it)
         }
 
-        OCFile("/image.png", "02").apply {
+        OCFile("/image.png").apply {
             mimeType = "image/png"
             modificationTimestamp = 1000000
         }.let {
             storageManager.saveFile(it)
         }
 
-        OCFile("/image2.png", "03").apply {
+        OCFile("/image2.png").apply {
             mimeType = "image/png"
             modificationTimestamp = 1000050
         }.let {
             storageManager.saveFile(it)
         }
 
-        OCFile("/video.mpg", "04").apply {
+        OCFile("/video.mpg").apply {
             mimeType = "video/mpg"
             modificationTimestamp = 1000045
         }.let {
             storageManager.saveFile(it)
         }
 
-        OCFile("/video2.avi", "05").apply {
+        OCFile("/video2.avi").apply {
             mimeType = "video/avi"
             modificationTimestamp = endDate + 10
         }.let {

src/androidTest/java/com/owncloud/android/ui/fragment/BackupListFragmentIT.kt

@@ -42,7 +42,7 @@ class BackupListFragmentIT : AbstractIT() {
     @ScreenshotTest
     fun showLoading() {
         val sut = testActivityRule.launchActivity(null)
-        val file = OCFile("/", "00000001")
+        val file = OCFile("/")
         val transaction = sut.supportFragmentManager.beginTransaction()
 
         transaction.replace(R.id.frame_container, BackupListFragment.newInstance(file, user))
@@ -58,7 +58,7 @@ class BackupListFragmentIT : AbstractIT() {
         val sut = testActivityRule.launchActivity(null)
         val transaction = sut.supportFragmentManager.beginTransaction()
         val file = getFile("vcard.vcf")
-        val ocFile = OCFile("/vcard.vcf", "00000002")
+        val ocFile = OCFile("/vcard.vcf")
         ocFile.storagePath = file.absolutePath
         ocFile.mimeType = "text/vcard"
 
@@ -76,7 +76,7 @@ class BackupListFragmentIT : AbstractIT() {
         val sut = testActivityRule.launchActivity(null)
         val transaction = sut.supportFragmentManager.beginTransaction()
         val file = getFile("calendar.ics")
-        val ocFile = OCFile("/Private calender_2020-09-01_10-45-20.ics.ics", "00000003")
+        val ocFile = OCFile("/Private calender_2020-09-01_10-45-20.ics.ics")
         ocFile.storagePath = file.absolutePath
         ocFile.mimeType = "text/calendar"
 
@@ -94,12 +94,12 @@ class BackupListFragmentIT : AbstractIT() {
         val transaction = sut.supportFragmentManager.beginTransaction()
 
         val calendarFile = getFile("calendar.ics")
-        val calendarOcFile = OCFile("/Private calender_2020-09-01_10-45-20.ics", "00000003")
+        val calendarOcFile = OCFile("/Private calender_2020-09-01_10-45-20.ics")
         calendarOcFile.storagePath = calendarFile.absolutePath
         calendarOcFile.mimeType = "text/calendar"
 
         val contactFile = getFile("vcard.vcf")
-        val contactOcFile = OCFile("/vcard.vcf", "00000002")
+        val contactOcFile = OCFile("/vcard.vcf")
         contactOcFile.storagePath = contactFile.absolutePath
         contactOcFile.mimeType = "text/vcard"
 

src/androidTest/java/com/owncloud/android/ui/fragment/FileDetailFragmentStaticServerIT.kt

@@ -40,7 +40,7 @@ class FileDetailFragmentStaticServerIT : AbstractIT() {
     @get:Rule
     val testActivityRule = IntentsTestRule(TestActivity::class.java, true, false)
 
-    val file = OCFile("/", "00000001")
+    val file = OCFile("/")
 
     @Test
     @ScreenshotTest

src/androidTest/java/com/owncloud/android/ui/fragment/OCFileListFragmentStaticServerIT.kt

@@ -48,14 +48,14 @@ class OCFileListFragmentStaticServerIT : AbstractIT() {
     fun showFiles() {
         val sut = testActivityRule.launchActivity(null)
 
-        val textFile = OCFile("/1.png", "00000001")
+        val textFile = OCFile("/1.png")
         textFile.mimeType = "image/png"
         textFile.fileLength = 1024000
         textFile.modificationTimestamp = 1188206955000
         textFile.parentId = sut.storageManager.getFileByEncryptedRemotePath("/").fileId
         sut.storageManager.saveFile(textFile)
 
-        val imageFile = OCFile("/image.png", "00000002")
+        val imageFile = OCFile("/image.png")
         imageFile.mimeType = "image/png"
         imageFile.isPreviewAvailable = false
         imageFile.fileLength = 3072000
@@ -214,7 +214,7 @@ class OCFileListFragmentStaticServerIT : AbstractIT() {
         folder.setFolder()
         sut.storageManager.saveFile(folder)
 
-        val imageFile = OCFile("/test/image.png", "00000001")
+        val imageFile = OCFile("/test/image.png")
         imageFile.mimeType = "image/png"
         imageFile.fileLength = 1024000
         imageFile.modificationTimestamp = 1188206955000
@@ -238,7 +238,7 @@ class OCFileListFragmentStaticServerIT : AbstractIT() {
         val activity = testActivityRule.launchActivity(null)
         val sut = OCFileListFragment()
 
-        val folder = OCFile("/test/", "00001")
+        val folder = OCFile("/test/")
         folder.setFolder()
         activity.storageManager.saveFile(folder)
 

src/main/java/com/owncloud/android/datamodel/OCFile.java

@@ -124,12 +124,6 @@ public class OCFile implements Parcelable, Comparable<OCFile>, ServerFileInterfa
         remotePath = path;
     }
 
-    @VisibleForTesting
-    public OCFile(String path, String remoteId) {
-        this(path);
-        this.remoteId = remoteId;
-    }
-
     /**
      * Reconstruct from parcel
      *

src/main/java/com/owncloud/android/providers/FileContentProvider.java

@@ -232,7 +232,7 @@ public class FileContentProvider extends ContentProvider {
         }
 
         if (uri.getPathSegments().size() > MINIMUM_PATH_SEGMENTS_SIZE) {
-            count += deleteWithuri(db, uri, where, whereArgs);
+            count += deleteWithUri(db, uri, where, whereArgs);
         }
 
         return count;
@@ -241,31 +241,21 @@ public class FileContentProvider extends ContentProvider {
     private int deleteSingleFile(SQLiteDatabase db, Uri uri, String where, String... whereArgs) {
         int count = 0;
 
-        Cursor c = query(db, uri, PROJECTION_REMOTE_ID, where, whereArgs, null);
-        String remoteId = "";
-        try {
-            if (c != null && c.moveToFirst()) {
-                remoteId = c.getString(c.getColumnIndexOrThrow(ProviderTableMeta.FILE_REMOTE_ID));
+        try (Cursor c = query(db, uri, PROJECTION_REMOTE_ID, where, whereArgs, null)) {
+            if (c.moveToFirst()) {
+                String id = c.getString(c.getColumnIndexOrThrow(ProviderTableMeta._ID));
+                Log_OC.d(TAG, "Removing FILE " + id);
             }
-            Log_OC.d(TAG, "Removing FILE " + remoteId);
 
-            if (remoteId == null) {
-                return 0;
-            } else {
-                count = deleteWithuri(db, uri, where, whereArgs);
-            }
+            count = deleteWithUri(db, uri, where, whereArgs);
         } catch (Exception e) {
             Log_OC.d(TAG, "DB-Error removing file!", e);
-        } finally {
-            if (c != null) {
-                c.close();
-            }
         }
 
         return count;
     }
 
-    private int deleteWithuri(SQLiteDatabase db, Uri uri, String where, String[] whereArgs) {
+    private int deleteWithUri(SQLiteDatabase db, Uri uri, String where, String[] whereArgs) {
         final String[] argsWithUri = VerificationUtils.prependUriFirstSegmentToSelectionArgs(whereArgs, uri);
         return db.delete(ProviderTableMeta.FILE_TABLE_NAME,
                          ProviderTableMeta._ID + "=?"