Merge pull request #9694 from nextcloud/fix/sqlite-row-too-big

FileContentProvider: use projections for helper queries

.github/workflows/analysis.yml

@@ -18,4 +18,4 @@ jobs:
                 run: |
                     mkdir -p $HOME/.gradle
                     echo "org.gradle.jvmargs=-Xmx2g -XX:MaxMetaspaceSize=512m -XX:+HeapDumpOnOutOfMemoryError" > $HOME/.gradle/gradle.properties
-                    scripts/analysis/analysis-wrapper.sh ${{ secrets.GIT_USERNAME }} ${{ secrets.GIT_TOKEN }} $GITHUB_REF ${{ secrets.LOG_USERNAME }} ${{ secrets.LOG_PASSWORD }} $GITHUB_RUN_NUMBER ${{ github.event.pull_request.number }}
+                    scripts/analysis/analysis-wrapper.sh ${{ secrets.GIT_USERNAME }} ${{ secrets.GIT_TOKEN }} ${{ github.head_ref }} ${{ secrets.LOG_USERNAME }} ${{ secrets.LOG_PASSWORD }} $GITHUB_RUN_NUMBER ${{ github.event.pull_request.number }}

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

@@ -1,32 +0,0 @@
-/*
- *
- * Nextcloud Android client application
- *
- * @author Tobias Kaminsky
- * Copyright (C) 2020 Tobias Kaminsky
- * Copyright (C) 2020 Nextcloud GmbH
- *
- * This program is free software: you can redistribute it and/or modify
- * it under the terms of the GNU Affero General Public License as published by
- * the Free Software Foundation, either version 3 of the License, or
- * (at your option) any later version.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
- * GNU Affero General Public License for more details.
- *
- * You should have received a copy of the GNU Affero General Public License
- * along with this program. If not, see <https://www.gnu.org/licenses/>.
- */
-
-package com.owncloud.android.datamodel;
-
-public class FileDataStorageManagerContentResolverIT extends FileDataStorageManagerIT {
-    @Override
-    public void before() {
-        sut = new FileDataStorageManager(user, targetContext.getContentResolver());
-
-        super.before();
-    }
-}

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

@@ -0,0 +1,71 @@
+/*
+ *
+ * Nextcloud Android client application
+ *
+ * @author Tobias Kaminsky
+ * Copyright (C) 2020 Tobias Kaminsky
+ * Copyright (C) 2020 Nextcloud GmbH
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Affero General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU Affero General Public License for more details.
+ *
+ * You should have received a copy of the GNU Affero General Public License
+ * along with this program. If not, see <https://www.gnu.org/licenses/>.
+ */
+package com.owncloud.android.datamodel
+
+import org.junit.Assert
+import org.junit.Test
+import java.util.ArrayList
+
+class FileDataStorageManagerContentResolverIT : FileDataStorageManagerIT() {
+    companion object {
+        private const val MANY_FILES_AMOUNT = 5000
+    }
+
+    override fun before() {
+        sut = FileDataStorageManager(user, targetContext.contentResolver)
+        super.before()
+    }
+
+    /**
+     * only on FileDataStorageManager
+     */
+    @Test
+    fun testFolderWithManyFiles() {
+        // create folder
+        val folderA = OCFile("/folderA/", "00001") // remote Id must never be null
+        folderA.setFolder().parentId = sut.getFileByDecryptedRemotePath("/")!!.fileId
+        sut.saveFile(folderA)
+        Assert.assertTrue(sut.fileExists("/folderA/"))
+        Assert.assertEquals(0, sut.getFolderContent(folderA, false).size)
+        val folderAId = sut.getFileByDecryptedRemotePath("/folderA/")!!.fileId
+
+        // create files
+        val newFiles = (1..MANY_FILES_AMOUNT).map {
+            val file = OCFile("/folderA/file$it", it.toString())
+            file.parentId = folderAId
+            sut.saveFile(file)
+
+            val storedFile = sut.getFileByDecryptedRemotePath("/folderA/file$it")
+            Assert.assertNotNull(storedFile)
+            storedFile
+        }
+
+        // save files in folder
+        sut.saveFolder(
+            folderA,
+            newFiles,
+            ArrayList()
+        )
+        // check file count is correct
+        Assert.assertEquals(MANY_FILES_AMOUNT, sut.getFolderContent(folderA, false).size)
+    }
+}

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

@@ -58,8 +58,10 @@ import com.owncloud.android.utils.MimeType;
 
 import java.io.File;
 import java.util.ArrayList;
+import java.util.Collections;
 import java.util.HashMap;
 import java.util.Locale;
+import java.util.Map;
 
 import javax.inject.Inject;
 
@@ -100,6 +102,30 @@ public class FileContentProvider extends ContentProvider {
     public static final int ARBITRARY_DATA_TABLE_INTRODUCTION_VERSION = 20;
     public static final int MINIMUM_PATH_SEGMENTS_SIZE = 1;
 
+    private static final String[] PROJECTION_CONTENT_TYPE = new String[]{
+        ProviderTableMeta._ID, ProviderTableMeta.FILE_CONTENT_TYPE
+    };
+    private static final String[] PROJECTION_REMOTE_ID = new String[]{
+        ProviderTableMeta._ID, ProviderTableMeta.FILE_REMOTE_ID
+    };
+    private static final String[] PROJECTION_FILE_AND_STORAGE_PATH = new String[]{
+        ProviderTableMeta._ID, ProviderTableMeta.FILE_STORAGE_PATH, ProviderTableMeta.FILE_PATH
+    };
+    private static final String[] PROJECTION_FILE_PATH_AND_OWNER = new String[]{
+        ProviderTableMeta._ID, ProviderTableMeta.FILE_PATH, ProviderTableMeta.FILE_ACCOUNT_OWNER
+    };
+
+    private static final Map<String, String> FILE_PROJECTION_MAP;
+
+    static {
+        HashMap<String,String> tempMap = new HashMap<>();
+        for (String projection : ProviderTableMeta.FILE_ALL_COLUMNS) {
+            tempMap.put(projection, projection);
+        }
+        FILE_PROJECTION_MAP = Collections.unmodifiableMap(tempMap);
+    }
+
+
     @Inject protected Clock clock;
     private DataBaseHelper mDbHelper;
     private Context mContext;
@@ -181,7 +207,8 @@ public class FileContentProvider extends ContentProvider {
 
     private int deleteDirectory(SQLiteDatabase db, Uri uri, String where, String... whereArgs) {
         int count = 0;
-        Cursor children = query(uri, null, null, null, null);
+
+        Cursor children = query(uri, PROJECTION_CONTENT_TYPE, null, null, null);
         if (children != null) {
             if (children.moveToFirst()) {
                 long childId;
@@ -213,7 +240,8 @@ public class FileContentProvider extends ContentProvider {
 
     private int deleteSingleFile(SQLiteDatabase db, Uri uri, String where, String... whereArgs) {
         int count = 0;
-        Cursor c = query(db, uri, null, where, whereArgs, null);
+
+        Cursor c = query(db, uri, PROJECTION_REMOTE_ID, where, whereArgs, null);
         String remoteId = "";
         try {
             if (c != null && c.moveToFirst()) {
@@ -289,17 +317,13 @@ public class FileContentProvider extends ContentProvider {
         switch (mUriMatcher.match(uri)) {
             case ROOT_DIRECTORY:
             case SINGLE_FILE:
-                String[] projection = new String[]{
-                    ProviderTableMeta._ID, ProviderTableMeta.FILE_PATH,
-                    ProviderTableMeta.FILE_ACCOUNT_OWNER
-                };
                 String where = ProviderTableMeta.FILE_PATH + "=? AND " + ProviderTableMeta.FILE_ACCOUNT_OWNER + "=?";
 
                 String remotePath = values.getAsString(ProviderTableMeta.FILE_PATH);
                 String accountName = values.getAsString(ProviderTableMeta.FILE_ACCOUNT_OWNER);
                 String[] whereArgs = {remotePath, accountName};
 
-                Cursor doubleCheck = query(db, uri, projection, where, whereArgs, null);
+                Cursor doubleCheck = query(db, uri, PROJECTION_FILE_PATH_AND_OWNER, where, whereArgs, null);
                 // ugly patch; serious refactoring is needed to reduce work in
                 // FileDataStorageManager and bring it to FileContentProvider
                 if (!doubleCheck.moveToFirst()) {
@@ -608,13 +632,7 @@ public class FileContentProvider extends ContentProvider {
         // only file list is accessible via content provider, so only this has to be protected with projectionMap
         if ((uriMatch == ROOT_DIRECTORY || uriMatch == SINGLE_FILE ||
             uriMatch == DIRECTORY) && projectionArray != null) {
-            HashMap<String, String> projectionMap = new HashMap<>();
-
-            for (String projection : ProviderTableMeta.FILE_ALL_COLUMNS) {
-                projectionMap.put(projection, projection);
-            }
-
-            sqlQuery.setProjectionMap(projectionMap);
+            sqlQuery.setProjectionMap(FILE_PROJECTION_MAP);
         }
 
         // if both are null, let them pass to query
@@ -989,7 +1007,7 @@ public class FileContentProvider extends ContentProvider {
             ProviderTableMeta.FILE_STORAGE_PATH + " IS NOT NULL";
 
         Cursor c = db.query(ProviderTableMeta.FILE_TABLE_NAME,
-                            null,
+                            PROJECTION_FILE_AND_STORAGE_PATH,
                             whereClause,
                             new String[]{newAccountName},
                             null, null, null);