Merge 5c07001 into 2bdbdeb

CHANGELOG.md

@@ -1,6 +1,10 @@
 # App Center SDK for Android Change Log
 
-## Version 1.11.0 (Under active development)
+## Version 1.10.1 (Under active development)
+
+### AppCenter
+
+* **[Fix]** Do not delete old logs when trying to add a log larger than the maximum storage capacity.
 
 ### AppCenterCrashes
 

apps/sasquatch/src/androidTest/java/com/microsoft/appcenter/sasquatch/activities/AnalyticsTest.java

@@ -7,7 +7,6 @@
 import com.microsoft.appcenter.Constants;
 import com.microsoft.appcenter.sasquatch.R;
 import com.microsoft.appcenter.sasquatch.listeners.SasquatchAnalyticsListener;
-import com.microsoft.appcenter.sasquatch.listeners.SasquatchCrashesListener;
 
 import org.junit.After;
 import org.junit.Before;

sdk/appcenter-crashes/src/androidTest/java/com/microsoft/appcenter/crashes/WrapperSdkExceptionManagerAndroidTest.java

@@ -11,7 +11,6 @@
 import com.microsoft.appcenter.crashes.ingestion.models.Exception;
 import com.microsoft.appcenter.crashes.utils.ErrorLogHelper;
 import com.microsoft.appcenter.utils.storage.SharedPreferencesManager;
-import com.microsoft.appcenter.utils.storage.FileManager;
 
 import org.junit.Assert;
 import org.junit.Before;

sdk/appcenter-crashes/src/main/java/com/microsoft/appcenter/crashes/utils/ErrorLogHelper.java

@@ -77,7 +77,7 @@ public class ErrorLogHelper {
      * For huge exception cause chains, we keep only beginning and end of causes according to this limit.
      */
     @VisibleForTesting
-    public static final int CAUSE_LIMIT = 16;
+    static final int CAUSE_LIMIT = 16;
 
     /**
      * We keep the first half of the limit of causes from the beginning and the second half from end.

sdk/appcenter/src/androidTest/java/com/microsoft/appcenter/persistence/DatabasePersistenceAndroidTest.java

@@ -51,6 +51,7 @@
 import static com.microsoft.appcenter.Flags.PERSISTENCE_NORMAL;
 import static com.microsoft.appcenter.ingestion.models.json.MockLog.MOCK_LOG_TYPE;
 import static com.microsoft.appcenter.persistence.DatabasePersistence.SCHEMA;
+import static com.microsoft.appcenter.test.TestUtils.generateString;
 import static org.hamcrest.CoreMatchers.hasItems;
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.junit.Assert.assertEquals;
@@ -447,9 +448,10 @@ public void putTooManyLogsMixedPriorities() throws PersistenceException {
     }
 
     @Test
-    public void putNormalLogLargerThanMaxSizeClearsEverythingWithNormalPriority() throws PersistenceException {
+    public void putLogTooLargeIsNotEvenTried() throws PersistenceException {
 
         /* Initialize database persistence. */
+        int someLogCount = 3;
         DatabasePersistence persistence = new DatabasePersistence(sContext);
         assertTrue(persistence.setMaxStorageSize(MAX_STORAGE_SIZE_IN_BYTES));
 
@@ -460,6 +462,46 @@ public void putNormalLogLargerThanMaxSizeClearsEverythingWithNormalPriority() th
         try {
 
             /* Generate some logs that will be evicted. */
+            for (int i = 0; i < someLogCount; i++) {
+                persistence.putLog(AndroidTestUtils.generateMockLog(), "test-p1", PERSISTENCE_NORMAL);
+            }
+            assertEquals(someLogCount, persistence.countLogs("test-p1"));
+
+            /* Generate a log that is so large that it eventually fails. */
+            LogWithProperties log = AndroidTestUtils.generateMockLog();
+            int size = 32 * 1024;
+            Map<String, String> properties = new HashMap<>();
+            properties.put("key", generateString(size, 'x'));
+            log.setProperties(properties);
+            try {
+                persistence.putLog(log, "test-p1", PERSISTENCE_NORMAL);
+                fail("putLog was expected to fail");
+            } catch (PersistenceException e) {
+
+                /* Verify the behavior: not inserted and database isn't empty. */
+                assertEquals(someLogCount, persistence.countLogs("test-p1"));
+            }
+        } finally {
+
+            //noinspection ThrowFromFinallyBlock
+            persistence.close();
+        }
+    }
+
+    @Test
+    public void putNormalLogCloseToMaxSizeClearsEverything() throws PersistenceException {
+
+        /* Initialize database persistence. */
+        DatabasePersistence persistence = new DatabasePersistence(sContext);
+        assertTrue(persistence.setMaxStorageSize(MAX_STORAGE_SIZE_IN_BYTES));
+
+        /* Set a mock log serializer. */
+        LogSerializer logSerializer = new DefaultLogSerializer();
+        logSerializer.addLogFactory(MOCK_LOG_TYPE, new MockLogFactory());
+        persistence.setLogSerializer(logSerializer);
+        try {
+
+            /* Generate some logs of both priority that will be evicted. */
             int someLogCount = 3;
             for (int i = 0; i < someLogCount; i++) {
                 persistence.putLog(AndroidTestUtils.generateMockLog(), "test-p1", PERSISTENCE_NORMAL);
@@ -468,16 +510,13 @@ public void putNormalLogLargerThanMaxSizeClearsEverythingWithNormalPriority() th
 
             /*
              * Generate a log that is so large that will empty all the database and
-             * eventually fails.
+             * eventually fails because close to the limit we check and the overhead of columns/index
+             * is larger than max size.
              */
             LogWithProperties log = AndroidTestUtils.generateMockLog();
             int size = 30 * 1024;
-            StringBuilder largeValue = new StringBuilder(size);
-            for (int i = 0; i < size; i++) {
-                largeValue.append("x");
-            }
             Map<String, String> properties = new HashMap<>();
-            properties.put("key", largeValue.toString());
+            properties.put("key", generateString(size, 'x'));
             log.setProperties(properties);
             try {
                 persistence.putLog(log, "test-p1", PERSISTENCE_NORMAL);
@@ -495,7 +534,7 @@ public void putNormalLogLargerThanMaxSizeClearsEverythingWithNormalPriority() th
     }
 
     @Test
-    public void putCriticalLogLargerThanMaxSizeClearsEverything() throws PersistenceException {
+    public void putCriticalLogCloseToMaxSizeClearsEverything() throws PersistenceException {
 
         /* Initialize database persistence. */
         DatabasePersistence persistence = new DatabasePersistence(sContext);
@@ -519,16 +558,13 @@ public void putCriticalLogLargerThanMaxSizeClearsEverything() throws Persistence
 
             /*
              * Generate a log that is so large that will empty all the database and
-             * eventually fails.
+             * eventually fails because close to the limit we check and the overhead of columns/index
+             * is larger than max size.
              */
             LogWithProperties log = AndroidTestUtils.generateMockLog();
             int size = 30 * 1024;
-            StringBuilder largeValue = new StringBuilder(size);
-            for (int i = 0; i < size; i++) {
-                largeValue.append("x");
-            }
             Map<String, String> properties = new HashMap<>();
-            properties.put("key", largeValue.toString());
+            properties.put("key", generateString(size, 'x'));
             log.setProperties(properties);
             try {
                 persistence.putLog(log, "test-p1", PERSISTENCE_CRITICAL);
@@ -546,7 +582,7 @@ public void putCriticalLogLargerThanMaxSizeClearsEverything() throws Persistence
     }
 
     @Test
-    public void putNormalLogLargerThanMaxSizeKeepsCritical() throws PersistenceException {
+    public void putNormalLogCloseToMaxSizeKeepsCritical() throws PersistenceException {
 
         /* Initialize database persistence. */
         DatabasePersistence persistence = new DatabasePersistence(sContext);
@@ -579,12 +615,8 @@ public void putNormalLogLargerThanMaxSizeKeepsCritical() throws PersistenceExcep
              */
             LogWithProperties log = AndroidTestUtils.generateMockLog();
             int size = 30 * 1024;
-            StringBuilder largeValue = new StringBuilder(size);
-            for (int i = 0; i < size; i++) {
-                largeValue.append("x");
-            }
             Map<String, String> properties = new HashMap<>();
-            properties.put("key", largeValue.toString());
+            properties.put("key", generateString(size, 'x'));
             log.setProperties(properties);
             try {
                 persistence.putLog(log, "test-p1", PERSISTENCE_NORMAL);

sdk/appcenter/src/main/java/com/microsoft/appcenter/Flags.java

@@ -9,11 +9,6 @@
  */
 public final class Flags {
 
-    /**
-     * Mask for persistence within flags.
-     */
-    private static final int PERSISTENCE_MASK = 0xFF;
-
     /**
      * An event can be lost due to low bandwidth or disk space constraints.
      */
@@ -29,6 +24,11 @@ public final class Flags {
      */
     public static final int DEFAULTS = PERSISTENCE_NORMAL;
 
+    /**
+     * Mask for persistence within flags.
+     */
+    private static final int PERSISTENCE_MASK = 0xFF;
+
     /**
      * Get persistence priority flag.
      *

sdk/appcenter/src/main/java/com/microsoft/appcenter/persistence/DatabasePersistence.java

@@ -257,7 +257,8 @@ public long putLog(@NonNull Log log, @NonNull String group, @IntRange(from = Fla
             AppCenterLog.debug(LOG_TAG, "Storing a log to the Persistence database for log type " + log.getType() + " with flags=" + flags);
             String payload = getLogSerializer().serializeLog(log);
             ContentValues contentValues;
-            boolean isLargePayload = payload.getBytes("UTF-8").length >= PAYLOAD_MAX_SIZE;
+            int payloadSize = payload.getBytes("UTF-8").length;
+            boolean isLargePayload = payloadSize >= PAYLOAD_MAX_SIZE;
             String targetKey;
             String targetToken;
             if (log instanceof CommonSchemaLog) {
@@ -271,6 +272,11 @@ public long putLog(@NonNull Log log, @NonNull String group, @IntRange(from = Fla
                 targetKey = null;
                 targetToken = null;
             }
+            long maxSize = mDatabaseManager.getMaxSize();
+            if (!isLargePayload && maxSize <= payloadSize) {
+                throw new PersistenceException("Log is too large (" + payloadSize + " bytes) to store in database. " +
+                        "Current maximum database size is " + maxSize + " bytes.");
+            }
             contentValues = getContentValues(group, isLargePayload ? null : payload, targetToken, log.getType(), targetKey, Flags.getPersistenceFlag(flags, false));
             long databaseId = mDatabaseManager.put(contentValues, COLUMN_PRIORITY);
             if (databaseId == -1) {

sdk/appcenter/src/main/java/com/microsoft/appcenter/utils/storage/DatabaseManager.java

@@ -14,7 +14,6 @@
 import android.support.annotation.VisibleForTesting;
 import android.text.TextUtils;
 
-import com.microsoft.appcenter.AppCenter;
 import com.microsoft.appcenter.utils.AppCenterLog;
 
 import java.io.Closeable;
@@ -221,22 +220,25 @@ public long put(@NonNull ContentValues values, @NonNull String priorityColumn) {
                 } catch (SQLiteFullException e) {
 
                     /* Delete the oldest log. */
+                    AppCenterLog.debug(LOG_TAG, "Storage is full, trying to delete the oldest log that has the lowest priority which is lower or equal priority than the new log");
                     if (cursor == null) {
                         String priority = values.getAsString(priorityColumn);
                         SQLiteQueryBuilder queryBuilder = SQLiteUtils.newSQLiteQueryBuilder();
                         queryBuilder.appendWhere(priorityColumn + " <= ?");
                         cursor = getCursor(queryBuilder, SELECT_PRIMARY_KEY, new String[]{priority}, priorityColumn + " , " + PRIMARY_KEY);
                     }
                     if (cursor.moveToNext()) {
-                        delete(cursor.getLong(0));
+                        long deletedId = cursor.getLong(0);
+                        delete(deletedId);
+                        AppCenterLog.debug(LOG_TAG, "Deleted log id=" + deletedId);
                     } else {
                         throw e;
                     }
                 }
             }
         } catch (RuntimeException e) {
             id = -1L;
-            AppCenterLog.error(AppCenter.LOG_TAG, String.format("Failed to insert values (%s) to database.", values.toString()), e);
+            AppCenterLog.error(LOG_TAG, String.format("Failed to insert values (%s) to database.", values.toString()), e);
         }
         if (cursor != null) {
             try {
@@ -268,7 +270,7 @@ public void delete(@NonNull List<Long> idList) {
         try {
             getDatabase().execSQL(String.format("DELETE FROM " + mTable + " WHERE " + PRIMARY_KEY + " IN (%s);", TextUtils.join(", ", idList)));
         } catch (RuntimeException e) {
-            AppCenterLog.error(AppCenter.LOG_TAG, String.format("Failed to delete IDs (%s) from database.", Arrays.toString(idList.toArray())), e);
+            AppCenterLog.error(LOG_TAG, String.format("Failed to delete IDs (%s) from database.", Arrays.toString(idList.toArray())), e);
         }
     }
 
@@ -282,7 +284,7 @@ public void delete(@Nullable String key, @Nullable Object value) {
         try {
             getDatabase().delete(mTable, key + " = ?", new String[]{String.valueOf(value)});
         } catch (RuntimeException e) {
-            AppCenterLog.error(AppCenter.LOG_TAG, String.format("Failed to delete values that match key=\"%s\" and value=\"%s\" from database.", key, value), e);
+            AppCenterLog.error(LOG_TAG, String.format("Failed to delete values that match key=\"%s\" and value=\"%s\" from database.", key, value), e);
         }
     }
 
@@ -293,7 +295,7 @@ public void clear() {
         try {
             getDatabase().delete(mTable, null, null);
         } catch (RuntimeException e) {
-            AppCenterLog.error(AppCenter.LOG_TAG, "Failed to clear the table.", e);
+            AppCenterLog.error(LOG_TAG, "Failed to clear the table.", e);
         }
     }
 
@@ -305,7 +307,7 @@ public void close() {
         try {
             getDatabase().close();
         } catch (RuntimeException e) {
-            AppCenterLog.error(AppCenter.LOG_TAG, "Failed to close the database.", e);
+            AppCenterLog.error(LOG_TAG, "Failed to close the database.", e);
         }
     }
 
@@ -318,7 +320,7 @@ public final long getRowCount() {
         try {
             return DatabaseUtils.queryNumEntries(getDatabase(), mTable);
         } catch (RuntimeException e) {
-            AppCenterLog.error(AppCenter.LOG_TAG, "Failed to get row count of database.", e);
+            AppCenterLog.error(LOG_TAG, "Failed to get row count of database.", e);
             return -1;
         }
     }
@@ -405,8 +407,7 @@ public boolean setMaxSize(long maxStorageSizeInBytes) {
      *
      * @return The maximum size of database in bytes.
      */
-    @VisibleForTesting
-    long getMaxSize() {
+    public long getMaxSize() {
         return getDatabase().getMaximumSize();
     }
 

sdk/appcenter/src/test/java/com/microsoft/appcenter/persistence/DatabasePersistenceTest.java

@@ -8,14 +8,11 @@
 import android.database.sqlite.SQLiteQueryBuilder;
 
 import com.microsoft.appcenter.AppCenter;
-import com.microsoft.appcenter.Flags;
 import com.microsoft.appcenter.ingestion.models.Log;
-import com.microsoft.appcenter.ingestion.models.json.DefaultLogSerializer;
 import com.microsoft.appcenter.ingestion.models.json.LogSerializer;
 import com.microsoft.appcenter.utils.AppCenterLog;
 import com.microsoft.appcenter.utils.storage.DatabaseManager;
 
-import org.json.JSONException;
 import org.junit.Rule;
 import org.junit.Test;
 import org.mockito.invocation.InvocationOnMock;
@@ -30,17 +27,13 @@
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertTrue;
-import static org.junit.Assert.fail;
 import static org.mockito.Matchers.any;
 import static org.mockito.Matchers.anyLong;
 import static org.mockito.Matchers.anyString;
 import static org.mockito.Matchers.eq;
 import static org.mockito.Matchers.isNotNull;
 import static org.mockito.Matchers.isNull;
-import static org.mockito.Mockito.doReturn;
 import static org.mockito.Mockito.mock;
-import static org.mockito.Mockito.spy;
-import static org.mockito.Mockito.times;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
 import static org.powermock.api.mockito.PowerMockito.mockStatic;
@@ -54,34 +47,6 @@ public class DatabasePersistenceTest {
     @Rule
     public PowerMockRule mPowerMockRule = new PowerMockRule();
 
-    @Test
-    public void databaseOperationException() throws JSONException {
-
-        /* Mock instances. */
-        mockStatic(AppCenterLog.class);
-        LogSerializer mockSerializer = mock(DefaultLogSerializer.class);
-        when(mockSerializer.serializeLog(any(Log.class))).thenReturn("{}");
-        DatabasePersistence mockPersistence = spy(new DatabasePersistence(mock(Context.class), 1, DatabasePersistence.SCHEMA));
-        doReturn(mockSerializer).when(mockPersistence).getLogSerializer();
-        try {
-
-            /* Generate a log and persist. */
-            Log log = mock(Log.class);
-            mockPersistence.putLog(log, "test-p1", Flags.PERSISTENCE_NORMAL);
-            fail("Expected persistence exception");
-        } catch (Persistence.PersistenceException ignore) {
-        } finally {
-
-            /* Close. */
-            //noinspection ThrowFromFinallyBlock
-            mockPersistence.close();
-        }
-
-        /* There are two error logs on putLog and close. */
-        verifyStatic(times(2));
-        AppCenterLog.error(eq(AppCenter.LOG_TAG), anyString(), any(RuntimeException.class));
-    }
-
     @Test
     public void countLogsWithGetCountException() throws Exception {