From 8df277cbb6ac244442b96b621369839a8276004e Mon Sep 17 00:00:00 2001 From: Wojtek Zieba Date: Wed, 1 Nov 2023 18:58:06 +0100 Subject: [PATCH 01/17] refactor: extract local storage related code To LocalStorageRepository.java --- .../LocalStorageRepository.java | 104 ++++++++++++++++++ .../parselyandroid/ParselyTracker.java | 101 ++--------------- 2 files changed, 115 insertions(+), 90 deletions(-) create mode 100644 parsely/src/main/java/com/parsely/parselyandroid/LocalStorageRepository.java diff --git a/parsely/src/main/java/com/parsely/parselyandroid/LocalStorageRepository.java b/parsely/src/main/java/com/parsely/parselyandroid/LocalStorageRepository.java new file mode 100644 index 00000000..b15028e4 --- /dev/null +++ b/parsely/src/main/java/com/parsely/parselyandroid/LocalStorageRepository.java @@ -0,0 +1,104 @@ +package com.parsely.parselyandroid; + +import static com.parsely.parselyandroid.ParselyTracker.PLog; + +import android.content.Context; + +import androidx.annotation.NonNull; + +import java.io.EOFException; +import java.io.FileInputStream; +import java.io.FileNotFoundException; +import java.io.FileOutputStream; +import java.io.ObjectInputStream; +import java.io.ObjectOutputStream; +import java.util.ArrayList; +import java.util.HashSet; +import java.util.List; +import java.util.Map; + +class LocalStorageRepository { + private static final String STORAGE_KEY = "parsely-events.ser"; + + private final Context context; + + public LocalStorageRepository(Context context) { + this.context = context; + } + + /** + * Persist an object to storage. + * + * @param o Object to store. + */ + void persistObject(Object o) { + try { + FileOutputStream fos = context.getApplicationContext().openFileOutput( + STORAGE_KEY, + android.content.Context.MODE_PRIVATE + ); + ObjectOutputStream oos = new ObjectOutputStream(fos); + oos.writeObject(o); + oos.close(); + } catch (Exception ex) { + PLog("Exception thrown during queue serialization: %s", ex.toString()); + } + } + + /** + * Delete the stored queue from persistent storage. + */ + void purgeStoredQueue() { + persistObject(new ArrayList>()); + } + + /** + * Get the stored event queue from persistent storage. + * + * @return The stored queue of events. + */ + @NonNull + ArrayList> getStoredQueue() { + ArrayList> storedQueue = null; + try { + FileInputStream fis = context.getApplicationContext().openFileInput(STORAGE_KEY); + ObjectInputStream ois = new ObjectInputStream(fis); + //noinspection unchecked + storedQueue = (ArrayList>) ois.readObject(); + ois.close(); + } catch (EOFException ex) { + // Nothing to do here. + } catch (FileNotFoundException ex) { + // Nothing to do here. Means there was no saved queue. + } catch (Exception ex) { + PLog("Exception thrown during queue deserialization: %s", ex.toString()); + } + + if (storedQueue == null) { + storedQueue = new ArrayList<>(); + } + return storedQueue; + } + + /** + * Delete an event from the stored queue. + */ + void expelStoredEvent() { + ArrayList> storedQueue = getStoredQueue(); + storedQueue.remove(0); + } + + /** + * Save the event queue to persistent storage. + */ + synchronized void persistQueue(@NonNull final List> inMemoryQueue) { + PLog("Persisting event queue"); + ArrayList> storedQueue = getStoredQueue(); + HashSet> hs = new HashSet<>(); + hs.addAll(storedQueue); + hs.addAll(inMemoryQueue); + storedQueue.clear(); + storedQueue.addAll(hs); + persistObject(storedQueue); + } +} diff --git a/parsely/src/main/java/com/parsely/parselyandroid/ParselyTracker.java b/parsely/src/main/java/com/parsely/parselyandroid/ParselyTracker.java index 7784970e..ec75cbd5 100644 --- a/parsely/src/main/java/com/parsely/parselyandroid/ParselyTracker.java +++ b/parsely/src/main/java/com/parsely/parselyandroid/ParselyTracker.java @@ -29,13 +29,7 @@ import com.fasterxml.jackson.databind.ObjectMapper; -import java.io.EOFException; -import java.io.FileInputStream; -import java.io.FileNotFoundException; -import java.io.FileOutputStream; import java.io.IOException; -import java.io.ObjectInputStream; -import java.io.ObjectOutputStream; import java.io.StringWriter; import java.util.ArrayList; import java.util.Formatter; @@ -58,7 +52,6 @@ public class ParselyTracker { private static final int DEFAULT_ENGAGEMENT_INTERVAL_MILLIS = 10500; private static final int QUEUE_SIZE_LIMIT = 50; private static final int STORAGE_SIZE_LIMIT = 100; - private static final String STORAGE_KEY = "parsely-events.ser"; @SuppressWarnings("StringOperationCanBeSimplified") // private static final String ROOT_URL = "http://10.0.2.2:5001/".intern(); // emulator localhost private static final String ROOT_URL = "https://p1.parsely.com/".intern(); @@ -72,7 +65,10 @@ public class ParselyTracker { private String lastPageviewUuid = null; @NonNull private final EventsBuilder eventsBuilder; - @NonNull final HeartbeatIntervalCalculator intervalCalculator = new HeartbeatIntervalCalculator(new Clock()); + @NonNull + private final HeartbeatIntervalCalculator intervalCalculator = new HeartbeatIntervalCalculator(new Clock()); + @NonNull + private final LocalStorageRepository localStorageRepository; /** * Create a new ParselyTracker instance. @@ -80,6 +76,7 @@ public class ParselyTracker { protected ParselyTracker(String siteId, int flushInterval, Context c) { context = c.getApplicationContext(); eventsBuilder = new EventsBuilder(context, siteId); + localStorageRepository = new LocalStorageRepository(context); // get the adkey straight away on instantiation timer = new Timer(); @@ -89,7 +86,7 @@ protected ParselyTracker(String siteId, int flushInterval, Context c) { flushManager = new FlushManager(timer, flushInterval * 1000L); - if (getStoredQueue().size() > 0) { + if (localStorageRepository.getStoredQueue().size() > 0) { startFlushTimer(); } @@ -474,85 +471,9 @@ private boolean isReachable() { return netInfo != null && netInfo.isConnectedOrConnecting(); } - /** - * Save the event queue to persistent storage. - */ - private synchronized void persistQueue() { - PLog("Persisting event queue"); - ArrayList> storedQueue = getStoredQueue(); - HashSet> hs = new HashSet<>(); - hs.addAll(storedQueue); - hs.addAll(eventQueue); - storedQueue.clear(); - storedQueue.addAll(hs); - persistObject(storedQueue); - } - - /** - * Get the stored event queue from persistent storage. - * - * @return The stored queue of events. - */ - @NonNull - private ArrayList> getStoredQueue() { - ArrayList> storedQueue = null; - try { - FileInputStream fis = context.getApplicationContext().openFileInput(STORAGE_KEY); - ObjectInputStream ois = new ObjectInputStream(fis); - //noinspection unchecked - storedQueue = (ArrayList>) ois.readObject(); - ois.close(); - } catch (EOFException ex) { - // Nothing to do here. - } catch (FileNotFoundException ex) { - // Nothing to do here. Means there was no saved queue. - } catch (Exception ex) { - PLog("Exception thrown during queue deserialization: %s", ex.toString()); - } - - if (storedQueue == null) { - storedQueue = new ArrayList<>(); - } - return storedQueue; - } - void purgeEventsQueue() { eventQueue.clear(); - purgeStoredQueue(); - } - - /** - * Delete the stored queue from persistent storage. - */ - private void purgeStoredQueue() { - persistObject(new ArrayList>()); - } - - /** - * Delete an event from the stored queue. - */ - private void expelStoredEvent() { - ArrayList> storedQueue = getStoredQueue(); - storedQueue.remove(0); - } - - /** - * Persist an object to storage. - * - * @param o Object to store. - */ - private void persistObject(Object o) { - try { - FileOutputStream fos = context.getApplicationContext().openFileOutput( - STORAGE_KEY, - android.content.Context.MODE_PRIVATE - ); - ObjectOutputStream oos = new ObjectOutputStream(fos); - oos.writeObject(o); - oos.close(); - } catch (Exception ex) { - PLog("Exception thrown during queue serialization: %s", ex.toString()); - } + localStorageRepository.purgeStoredQueue(); } /** @@ -621,7 +542,7 @@ public int queueSize() { * @return The number of events stored in persistent storage. */ public int storedEventsCount() { - ArrayList> ar = getStoredQueue(); + ArrayList> ar = localStorageRepository.getStoredQueue(); return ar.size(); } @@ -631,11 +552,11 @@ protected Void doInBackground(Void... params) { // if event queue is too big, push to persisted storage if (eventQueue.size() > QUEUE_SIZE_LIMIT) { PLog("Queue size exceeded, expelling oldest event to persistent memory"); - persistQueue(); + localStorageRepository.persistQueue(eventQueue); eventQueue.remove(0); // if persisted storage is too big, expel one if (storedEventsCount() > STORAGE_SIZE_LIMIT) { - expelStoredEvent(); + localStorageRepository.expelStoredEvent(); } } return null; @@ -645,7 +566,7 @@ protected Void doInBackground(Void... params) { private class FlushQueue extends AsyncTask { @Override protected synchronized Void doInBackground(Void... params) { - ArrayList> storedQueue = getStoredQueue(); + ArrayList> storedQueue = localStorageRepository.getStoredQueue(); PLog("%d events in queue, %d stored events", eventQueue.size(), storedEventsCount()); // in case both queues have been flushed and app quits, don't crash if ((eventQueue == null || eventQueue.size() == 0) && storedQueue.size() == 0) { From a0ad4734d342f5146201a37125b63d4677ab40bd Mon Sep 17 00:00:00 2001 From: Wojtek Zieba Date: Thu, 2 Nov 2023 10:06:30 +0100 Subject: [PATCH 02/17] tests: test that `expelStoredEvent` doesn't have an effect The added test proves, that method `expelStoredEvent` doesn't have any affect on the stored queue. Removing it is safe, as it asserts the previous behavior. --- .../LocalStorageRepositoryTest.kt | 31 +++++++++++++++++++ 1 file changed, 31 insertions(+) create mode 100644 parsely/src/test/java/com/parsely/parselyandroid/LocalStorageRepositoryTest.kt diff --git a/parsely/src/test/java/com/parsely/parselyandroid/LocalStorageRepositoryTest.kt b/parsely/src/test/java/com/parsely/parselyandroid/LocalStorageRepositoryTest.kt new file mode 100644 index 00000000..5145ece2 --- /dev/null +++ b/parsely/src/test/java/com/parsely/parselyandroid/LocalStorageRepositoryTest.kt @@ -0,0 +1,31 @@ +package com.parsely.parselyandroid + +import androidx.test.core.app.ApplicationProvider +import org.assertj.core.api.Assertions.assertThat +import org.junit.Before +import org.junit.Test +import org.junit.runner.RunWith +import org.robolectric.RobolectricTestRunner + +@RunWith(RobolectricTestRunner::class) +class LocalStorageRepositoryTest { + + private lateinit var sut: LocalStorageRepository + + @Before + fun setUp() { + sut = LocalStorageRepository(ApplicationProvider.getApplicationContext()) + } + + @Test + fun `when expelling stored event, then assert that it has no effect`() { + // given + sut.persistQueue((1..100).map { mapOf("index" to it) }) + + // when + sut.expelStoredEvent() + + // then + assertThat(sut.storedQueue).hasSize(100) + } +} From b137fbdfcda4ad6bed75df848014b1689542f8f9 Mon Sep 17 00:00:00 2001 From: Wojtek Zieba Date: Thu, 2 Nov 2023 10:28:05 +0100 Subject: [PATCH 03/17] tests: add unit tests coverage for LocalStorageRepository --- .../LocalStorageRepositoryTest.kt | 45 +++++++++++++++++++ 1 file changed, 45 insertions(+) diff --git a/parsely/src/test/java/com/parsely/parselyandroid/LocalStorageRepositoryTest.kt b/parsely/src/test/java/com/parsely/parselyandroid/LocalStorageRepositoryTest.kt index 5145ece2..18b95f2e 100644 --- a/parsely/src/test/java/com/parsely/parselyandroid/LocalStorageRepositoryTest.kt +++ b/parsely/src/test/java/com/parsely/parselyandroid/LocalStorageRepositoryTest.kt @@ -28,4 +28,49 @@ class LocalStorageRepositoryTest { // then assertThat(sut.storedQueue).hasSize(100) } + + @Test + fun `given the list of events, when persisting the list, then querying the list returns the same result`() { + // given + val eventsList = (1..10).map { mapOf("index" to it) } + + // when + sut.persistQueue(eventsList) + + // then + assertThat(sut.storedQueue).hasSize(10).containsExactlyInAnyOrderElementsOf(eventsList) + } + + @Test + fun `given no locally stored list, when requesting stored queue, then return an empty list`() { + assertThat(sut.storedQueue).isEmpty() + } + + @Test + fun `given stored queue with some elements, when persisting in-memory queue, then assert there'll be no duplicates and queues will be combined`() { + // given + val storedQueue = (1..5).map { mapOf("index" to it) } + val inMemoryQueue = (3..10).map { mapOf("index" to it) } + sut.persistQueue(storedQueue) + + // when + sut.persistQueue(inMemoryQueue) + + // then + val expectedQueue = (1..10).map { mapOf("index" to it) } + assertThat(sut.storedQueue).hasSize(10).containsExactlyInAnyOrderElementsOf(expectedQueue) + } + + @Test + fun `given stored queue, when purging stored queue, then assert queue is purged`() { + // given + val eventsList = (1..10).map { mapOf("index" to it) } + sut.persistQueue(eventsList) + + // when + sut.purgeStoredQueue() + + // then + assertThat(sut.storedQueue).isEmpty() + } } From a11691e0067e9aae229c122844e6af8ede9f82ea Mon Sep 17 00:00:00 2001 From: Wojtek Zieba Date: Thu, 2 Nov 2023 10:33:37 +0100 Subject: [PATCH 04/17] fix: add the correct visibility modifier to `persistObject` method --- .../com/parsely/parselyandroid/LocalStorageRepository.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/parsely/src/main/java/com/parsely/parselyandroid/LocalStorageRepository.java b/parsely/src/main/java/com/parsely/parselyandroid/LocalStorageRepository.java index b15028e4..6e9897a5 100644 --- a/parsely/src/main/java/com/parsely/parselyandroid/LocalStorageRepository.java +++ b/parsely/src/main/java/com/parsely/parselyandroid/LocalStorageRepository.java @@ -22,7 +22,7 @@ class LocalStorageRepository { private final Context context; - public LocalStorageRepository(Context context) { + LocalStorageRepository(Context context) { this.context = context; } @@ -31,7 +31,7 @@ public LocalStorageRepository(Context context) { * * @param o Object to store. */ - void persistObject(Object o) { + private void persistObject(Object o) { try { FileOutputStream fos = context.getApplicationContext().openFileOutput( STORAGE_KEY, From 56a62b2b8f230261872299db0b0e7124e4d18376 Mon Sep 17 00:00:00 2001 From: Wojtek Zieba Date: Thu, 2 Nov 2023 11:28:40 +0100 Subject: [PATCH 05/17] tests: assert correct serialization of the Java file This test is added to assert correctness of serialization after migrating LocalStorageRepository from Java to Kotlin. Two languages might have some differences in how they serialize objects, so this test will assert that older storage files (created by Java) will have compatible content with new version of LocalStorageRepository, written in Kotlin. --- .../LocalStorageRepositoryTest.kt | 49 +++++++++++++++++- .../resources/valid-java-parsely-events.ser | Bin 0 -> 881 bytes 2 files changed, 48 insertions(+), 1 deletion(-) create mode 100644 parsely/src/test/resources/valid-java-parsely-events.ser diff --git a/parsely/src/test/java/com/parsely/parselyandroid/LocalStorageRepositoryTest.kt b/parsely/src/test/java/com/parsely/parselyandroid/LocalStorageRepositoryTest.kt index 18b95f2e..09d255d8 100644 --- a/parsely/src/test/java/com/parsely/parselyandroid/LocalStorageRepositoryTest.kt +++ b/parsely/src/test/java/com/parsely/parselyandroid/LocalStorageRepositoryTest.kt @@ -1,6 +1,8 @@ package com.parsely.parselyandroid +import android.content.Context import androidx.test.core.app.ApplicationProvider +import java.io.File import org.assertj.core.api.Assertions.assertThat import org.junit.Before import org.junit.Test @@ -11,10 +13,11 @@ import org.robolectric.RobolectricTestRunner class LocalStorageRepositoryTest { private lateinit var sut: LocalStorageRepository + private val context = ApplicationProvider.getApplicationContext() @Before fun setUp() { - sut = LocalStorageRepository(ApplicationProvider.getApplicationContext()) + sut = LocalStorageRepository(context) } @Test @@ -73,4 +76,48 @@ class LocalStorageRepositoryTest { // then assertThat(sut.storedQueue).isEmpty() } + + @Test + fun `given stored file with serialized events, when querying the queue, then list has expected events`() { + // given + val file = File(context.filesDir.path + "/parsely-events.ser") + File(ClassLoader.getSystemResource("valid-java-parsely-events.ser")?.path!!).copyTo(file) + + // when + val queue = sut.storedQueue + + // then + assertThat(queue).isEqualTo( + listOf( + mapOf( + "idsite" to "example.com", + "urlref" to "http://example.com/", + "data" to mapOf( + "manufacturer" to "Google", + "os" to "android", + "os_version" to "33", + "parsely_site_uuid" to "b325e2c9-498c-4331-a967-2d6049317c77", + "ts" to 1698918720863L + ), + "pvid" to "272cc2b8-5acc-4a70-80c7-20bb6eb843e4", + "action" to "pageview", + "url" to "http://example.com/article1.html" + ), + mapOf( + "idsite" to "example.com", + "urlref" to "http://example.com/", + "data" to mapOf( + "manufacturer" to "Google", + "os" to "android", + "os_version" to "33", + "parsely_site_uuid" to "b325e2c9-498c-4331-a967-2d6049317c77", + "ts" to 1698918742375L + ), + "pvid" to "e94567ec-3459-498c-bf2e-6a1b85ed5a82", + "action" to "pageview", + "url" to "http://example.com/article1.html" + ) + ) + ) + } } diff --git a/parsely/src/test/resources/valid-java-parsely-events.ser b/parsely/src/test/resources/valid-java-parsely-events.ser new file mode 100644 index 0000000000000000000000000000000000000000..2d514a8c616fb3d20524d50a9fde13961c85fd12 GIT binary patch literal 881 zcmds#y>1gh5P-*LI|)gI5?P{)gaqyR=l%^vpa}#8Uct=mjqO8g@3A{)Unr2$(6|T% z6coHcUVsu&(9l3h0|hPbz}g@s5>U}m>=d&*v)_EXXWyY|G_)S!4BI8=vVB`?JU+?| zS0|tLUcJZHb*LRe!{kq>DgeNlHm=)?~&?W>v9&%QgVmo^RN=W7itPTArDliD8Q>oD`DULQQPC)Kb%sp*3bMZge^q zb~_A>3>nR1cnZ#MO`8mkG7RfD&9urhhTQ_SA$dGl-ZUu7W$t?Fdm(wE?*{!uxPkBY zTt#_l==;4`#4*FBGJ_duldGw1>-!Ab6P%VqJ6)+sThKd7jilvw*jP1}S860q zN9`ju9o<}fd;a0{+7}0E_n&4fF1F%$GTbybdRCdf_ zwqo^SPY5qf+z^Fj#klJxU13?fX&O~8#X_w(HwqllET=g`-FjrW`Ui=qITw<8 z?J-Ye^%s}_+XF3sOzWrMH=_%JsUL(haR~R_R=JzEX2h7H^J2ppCSdv literal 0 HcmV?d00001 From a83cf6f89388c3a94e9048dcbd5dab9a6db9d981 Mon Sep 17 00:00:00 2001 From: Wojtek Zieba Date: Thu, 2 Nov 2023 11:30:39 +0100 Subject: [PATCH 06/17] Rename .java to .kt --- .../{LocalStorageRepository.java => LocalStorageRepository.kt} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename parsely/src/main/java/com/parsely/parselyandroid/{LocalStorageRepository.java => LocalStorageRepository.kt} (100%) diff --git a/parsely/src/main/java/com/parsely/parselyandroid/LocalStorageRepository.java b/parsely/src/main/java/com/parsely/parselyandroid/LocalStorageRepository.kt similarity index 100% rename from parsely/src/main/java/com/parsely/parselyandroid/LocalStorageRepository.java rename to parsely/src/main/java/com/parsely/parselyandroid/LocalStorageRepository.kt From 797b98a144c96997e92f9849502760505ccce199 Mon Sep 17 00:00:00 2001 From: Wojtek Zieba Date: Thu, 2 Nov 2023 11:30:39 +0100 Subject: [PATCH 07/17] refactor: move `LocalStorageRepository` to Kotlin --- .../parselyandroid/LocalStorageRepository.kt | 141 ++++++++---------- 1 file changed, 64 insertions(+), 77 deletions(-) diff --git a/parsely/src/main/java/com/parsely/parselyandroid/LocalStorageRepository.kt b/parsely/src/main/java/com/parsely/parselyandroid/LocalStorageRepository.kt index 6e9897a5..1b0973ee 100644 --- a/parsely/src/main/java/com/parsely/parselyandroid/LocalStorageRepository.kt +++ b/parsely/src/main/java/com/parsely/parselyandroid/LocalStorageRepository.kt @@ -1,104 +1,91 @@ -package com.parsely.parselyandroid; +package com.parsely.parselyandroid -import static com.parsely.parselyandroid.ParselyTracker.PLog; - -import android.content.Context; - -import androidx.annotation.NonNull; - -import java.io.EOFException; -import java.io.FileInputStream; -import java.io.FileNotFoundException; -import java.io.FileOutputStream; -import java.io.ObjectInputStream; -import java.io.ObjectOutputStream; -import java.util.ArrayList; -import java.util.HashSet; -import java.util.List; -import java.util.Map; - -class LocalStorageRepository { - private static final String STORAGE_KEY = "parsely-events.ser"; - - private final Context context; - - LocalStorageRepository(Context context) { - this.context = context; - } +import android.content.Context +import java.io.EOFException +import java.io.FileNotFoundException +import java.io.ObjectInputStream +import java.io.ObjectOutputStream +internal class LocalStorageRepository(private val context: Context) { /** * Persist an object to storage. * * @param o Object to store. */ - private void persistObject(Object o) { + private fun persistObject(o: Any) { try { - FileOutputStream fos = context.getApplicationContext().openFileOutput( - STORAGE_KEY, - android.content.Context.MODE_PRIVATE - ); - ObjectOutputStream oos = new ObjectOutputStream(fos); - oos.writeObject(o); - oos.close(); - } catch (Exception ex) { - PLog("Exception thrown during queue serialization: %s", ex.toString()); + val fos = context.applicationContext.openFileOutput( + STORAGE_KEY, + Context.MODE_PRIVATE + ) + val oos = ObjectOutputStream(fos) + oos.writeObject(o) + oos.close() + } catch (ex: Exception) { + ParselyTracker.PLog("Exception thrown during queue serialization: %s", ex.toString()) } } /** * Delete the stored queue from persistent storage. */ - void purgeStoredQueue() { - persistObject(new ArrayList>()); + fun purgeStoredQueue() { + persistObject(ArrayList>()) } - /** - * Get the stored event queue from persistent storage. - * - * @return The stored queue of events. - */ - @NonNull - ArrayList> getStoredQueue() { - ArrayList> storedQueue = null; - try { - FileInputStream fis = context.getApplicationContext().openFileInput(STORAGE_KEY); - ObjectInputStream ois = new ObjectInputStream(fis); - //noinspection unchecked - storedQueue = (ArrayList>) ois.readObject(); - ois.close(); - } catch (EOFException ex) { - // Nothing to do here. - } catch (FileNotFoundException ex) { - // Nothing to do here. Means there was no saved queue. - } catch (Exception ex) { - PLog("Exception thrown during queue deserialization: %s", ex.toString()); - } - - if (storedQueue == null) { - storedQueue = new ArrayList<>(); + val storedQueue: ArrayList?> + /** + * Get the stored event queue from persistent storage. + * + * @return The stored queue of events. + */ + get() { + var storedQueue: ArrayList?>? = null + try { + val fis = context.applicationContext.openFileInput(STORAGE_KEY) + val ois = ObjectInputStream(fis) + storedQueue = ois.readObject() as ArrayList?> + ois.close() + } catch (ex: EOFException) { + // Nothing to do here. + } catch (ex: FileNotFoundException) { + // Nothing to do here. Means there was no saved queue. + } catch (ex: Exception) { + ParselyTracker.PLog( + "Exception thrown during queue deserialization: %s", + ex.toString() + ) + } + if (storedQueue == null) { + storedQueue = ArrayList() + } + return storedQueue } - return storedQueue; - } /** * Delete an event from the stored queue. */ - void expelStoredEvent() { - ArrayList> storedQueue = getStoredQueue(); - storedQueue.remove(0); + fun expelStoredEvent() { + val storedQueue = storedQueue + storedQueue.removeAt(0) } /** * Save the event queue to persistent storage. */ - synchronized void persistQueue(@NonNull final List> inMemoryQueue) { - PLog("Persisting event queue"); - ArrayList> storedQueue = getStoredQueue(); - HashSet> hs = new HashSet<>(); - hs.addAll(storedQueue); - hs.addAll(inMemoryQueue); - storedQueue.clear(); - storedQueue.addAll(hs); - persistObject(storedQueue); + @Synchronized + fun persistQueue(inMemoryQueue: List?>) { + ParselyTracker.PLog("Persisting event queue") + val storedQueue = storedQueue + val hs = HashSet?>() + hs.addAll(storedQueue) + hs.addAll(inMemoryQueue) + storedQueue.clear() + storedQueue.addAll(hs) + persistObject(storedQueue) + } + + companion object { + private const val STORAGE_KEY = "parsely-events.ser" } -} +} \ No newline at end of file From 37f6b3483662db0e944a119793996b152ba7517f Mon Sep 17 00:00:00 2001 From: Wojtek Zieba Date: Thu, 2 Nov 2023 11:32:55 +0100 Subject: [PATCH 08/17] refactor: make queues keys non-nullable --- .../parsely/parselyandroid/LocalStorageRepository.kt | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/parsely/src/main/java/com/parsely/parselyandroid/LocalStorageRepository.kt b/parsely/src/main/java/com/parsely/parselyandroid/LocalStorageRepository.kt index 1b0973ee..ce785439 100644 --- a/parsely/src/main/java/com/parsely/parselyandroid/LocalStorageRepository.kt +++ b/parsely/src/main/java/com/parsely/parselyandroid/LocalStorageRepository.kt @@ -33,18 +33,18 @@ internal class LocalStorageRepository(private val context: Context) { persistObject(ArrayList>()) } - val storedQueue: ArrayList?> + val storedQueue: ArrayList?> /** * Get the stored event queue from persistent storage. * * @return The stored queue of events. */ get() { - var storedQueue: ArrayList?>? = null + var storedQueue: ArrayList?>? = null try { val fis = context.applicationContext.openFileInput(STORAGE_KEY) val ois = ObjectInputStream(fis) - storedQueue = ois.readObject() as ArrayList?> + storedQueue = ois.readObject() as ArrayList?> ois.close() } catch (ex: EOFException) { // Nothing to do here. @@ -74,10 +74,10 @@ internal class LocalStorageRepository(private val context: Context) { * Save the event queue to persistent storage. */ @Synchronized - fun persistQueue(inMemoryQueue: List?>) { + fun persistQueue(inMemoryQueue: List?>) { ParselyTracker.PLog("Persisting event queue") val storedQueue = storedQueue - val hs = HashSet?>() + val hs = HashSet?>() hs.addAll(storedQueue) hs.addAll(inMemoryQueue) storedQueue.clear() From 560fec128d20bcdec48055f5c1984343eaa6b951 Mon Sep 17 00:00:00 2001 From: Wojtek Zieba Date: Thu, 2 Nov 2023 11:46:28 +0100 Subject: [PATCH 09/17] refactor: extract QueueManager to a separate class --- .../parselyandroid/ParselyTracker.java | 21 +--------- .../parsely/parselyandroid/QueueManager.java | 38 +++++++++++++++++++ 2 files changed, 39 insertions(+), 20 deletions(-) create mode 100644 parsely/src/main/java/com/parsely/parselyandroid/QueueManager.java diff --git a/parsely/src/main/java/com/parsely/parselyandroid/ParselyTracker.java b/parsely/src/main/java/com/parsely/parselyandroid/ParselyTracker.java index ec75cbd5..11c13b15 100644 --- a/parsely/src/main/java/com/parsely/parselyandroid/ParselyTracker.java +++ b/parsely/src/main/java/com/parsely/parselyandroid/ParselyTracker.java @@ -50,8 +50,6 @@ public class ParselyTracker { private static ParselyTracker instance = null; private static final int DEFAULT_FLUSH_INTERVAL_SECS = 60; private static final int DEFAULT_ENGAGEMENT_INTERVAL_MILLIS = 10500; - private static final int QUEUE_SIZE_LIMIT = 50; - private static final int STORAGE_SIZE_LIMIT = 100; @SuppressWarnings("StringOperationCanBeSimplified") // private static final String ROOT_URL = "http://10.0.2.2:5001/".intern(); // emulator localhost private static final String ROOT_URL = "https://p1.parsely.com/".intern(); @@ -415,7 +413,7 @@ public void resetVideo() { void enqueueEvent(Map event) { // Push it onto the queue eventQueue.add(event); - new QueueManager().execute(); + new QueueManager(this, localStorageRepository).execute(); if (!flushTimerIsActive()) { startFlushTimer(); PLog("Flush flushTimer set to %ds", (flushManager.getIntervalMillis() / 1000)); @@ -546,23 +544,6 @@ public int storedEventsCount() { return ar.size(); } - private class QueueManager extends AsyncTask { - @Override - protected Void doInBackground(Void... params) { - // if event queue is too big, push to persisted storage - if (eventQueue.size() > QUEUE_SIZE_LIMIT) { - PLog("Queue size exceeded, expelling oldest event to persistent memory"); - localStorageRepository.persistQueue(eventQueue); - eventQueue.remove(0); - // if persisted storage is too big, expel one - if (storedEventsCount() > STORAGE_SIZE_LIMIT) { - localStorageRepository.expelStoredEvent(); - } - } - return null; - } - } - private class FlushQueue extends AsyncTask { @Override protected synchronized Void doInBackground(Void... params) { diff --git a/parsely/src/main/java/com/parsely/parselyandroid/QueueManager.java b/parsely/src/main/java/com/parsely/parselyandroid/QueueManager.java new file mode 100644 index 00000000..7a9a38ee --- /dev/null +++ b/parsely/src/main/java/com/parsely/parselyandroid/QueueManager.java @@ -0,0 +1,38 @@ +package com.parsely.parselyandroid; + +import android.os.AsyncTask; + +import androidx.annotation.NonNull; + +class QueueManager extends AsyncTask { + private static final int QUEUE_SIZE_LIMIT = 50; + private static final int STORAGE_SIZE_LIMIT = 100; + + @NonNull + private final ParselyTracker parselyTracker; + @NonNull + private final LocalStorageRepository localStorageRepository; + + public QueueManager( + @NonNull ParselyTracker parselyTracker, + @NonNull LocalStorageRepository localStorageRepository + ) { + this.parselyTracker = parselyTracker; + this.localStorageRepository = localStorageRepository; + } + + @Override + protected Void doInBackground(Void... params) { + // if event queue is too big, push to persisted storage + if (parselyTracker.eventQueue.size() > QUEUE_SIZE_LIMIT) { + ParselyTracker.PLog("Queue size exceeded, expelling oldest event to persistent memory"); + localStorageRepository.persistQueue(parselyTracker.eventQueue); + parselyTracker.eventQueue.remove(0); + // if persisted storage is too big, expel one + if (parselyTracker.storedEventsCount() > STORAGE_SIZE_LIMIT) { + localStorageRepository.expelStoredEvent(); + } + } + return null; + } +} From 56c8b9605702639284ba778d8ff8f5e32ae95dbd Mon Sep 17 00:00:00 2001 From: Wojtek Zieba Date: Fri, 3 Nov 2023 15:39:24 +0100 Subject: [PATCH 10/17] tests: add unit test coverage for QueueManager --- .../parselyandroid/LocalStorageRepository.kt | 8 +- .../parselyandroid/ParselyTracker.java | 7 +- .../parsely/parselyandroid/QueueManager.java | 10 +- .../parselyandroid/QueueManagerTest.kt | 109 ++++++++++++++++++ 4 files changed, 124 insertions(+), 10 deletions(-) create mode 100644 parsely/src/test/java/com/parsely/parselyandroid/QueueManagerTest.kt diff --git a/parsely/src/main/java/com/parsely/parselyandroid/LocalStorageRepository.kt b/parsely/src/main/java/com/parsely/parselyandroid/LocalStorageRepository.kt index ce785439..63c695ac 100644 --- a/parsely/src/main/java/com/parsely/parselyandroid/LocalStorageRepository.kt +++ b/parsely/src/main/java/com/parsely/parselyandroid/LocalStorageRepository.kt @@ -6,7 +6,7 @@ import java.io.FileNotFoundException import java.io.ObjectInputStream import java.io.ObjectOutputStream -internal class LocalStorageRepository(private val context: Context) { +internal open class LocalStorageRepository(private val context: Context) { /** * Persist an object to storage. * @@ -33,7 +33,7 @@ internal class LocalStorageRepository(private val context: Context) { persistObject(ArrayList>()) } - val storedQueue: ArrayList?> + open val storedQueue: ArrayList?> /** * Get the stored event queue from persistent storage. * @@ -65,7 +65,7 @@ internal class LocalStorageRepository(private val context: Context) { /** * Delete an event from the stored queue. */ - fun expelStoredEvent() { + open fun expelStoredEvent() { val storedQueue = storedQueue storedQueue.removeAt(0) } @@ -74,7 +74,7 @@ internal class LocalStorageRepository(private val context: Context) { * Save the event queue to persistent storage. */ @Synchronized - fun persistQueue(inMemoryQueue: List?>) { + open fun persistQueue(inMemoryQueue: List?>) { ParselyTracker.PLog("Persisting event queue") val storedQueue = storedQueue val hs = HashSet?>() diff --git a/parsely/src/main/java/com/parsely/parselyandroid/ParselyTracker.java b/parsely/src/main/java/com/parsely/parselyandroid/ParselyTracker.java index 11c13b15..e4d61c2f 100644 --- a/parsely/src/main/java/com/parsely/parselyandroid/ParselyTracker.java +++ b/parsely/src/main/java/com/parsely/parselyandroid/ParselyTracker.java @@ -35,6 +35,7 @@ import java.util.Formatter; import java.util.HashMap; import java.util.HashSet; +import java.util.List; import java.util.Map; import java.util.Timer; import java.util.TimerTask; @@ -53,7 +54,7 @@ public class ParselyTracker { @SuppressWarnings("StringOperationCanBeSimplified") // private static final String ROOT_URL = "http://10.0.2.2:5001/".intern(); // emulator localhost private static final String ROOT_URL = "https://p1.parsely.com/".intern(); - protected ArrayList> eventQueue; + private final ArrayList> eventQueue; private boolean isDebug; private final Context context; private final Timer timer; @@ -97,6 +98,10 @@ protected ParselyTracker(String siteId, int flushInterval, Context c) { ); } + List> getInMemoryQueue() { + return eventQueue; + } + /** * Singleton instance accessor. Note: This must be called after {@link #sharedInstance(String, Context)} * diff --git a/parsely/src/main/java/com/parsely/parselyandroid/QueueManager.java b/parsely/src/main/java/com/parsely/parselyandroid/QueueManager.java index 7a9a38ee..c05d1fe2 100644 --- a/parsely/src/main/java/com/parsely/parselyandroid/QueueManager.java +++ b/parsely/src/main/java/com/parsely/parselyandroid/QueueManager.java @@ -5,8 +5,8 @@ import androidx.annotation.NonNull; class QueueManager extends AsyncTask { - private static final int QUEUE_SIZE_LIMIT = 50; - private static final int STORAGE_SIZE_LIMIT = 100; + static final int QUEUE_SIZE_LIMIT = 50; + static final int STORAGE_SIZE_LIMIT = 100; @NonNull private final ParselyTracker parselyTracker; @@ -24,10 +24,10 @@ public QueueManager( @Override protected Void doInBackground(Void... params) { // if event queue is too big, push to persisted storage - if (parselyTracker.eventQueue.size() > QUEUE_SIZE_LIMIT) { + if (parselyTracker.getInMemoryQueue().size() > QUEUE_SIZE_LIMIT) { ParselyTracker.PLog("Queue size exceeded, expelling oldest event to persistent memory"); - localStorageRepository.persistQueue(parselyTracker.eventQueue); - parselyTracker.eventQueue.remove(0); + localStorageRepository.persistQueue(parselyTracker.getInMemoryQueue()); + parselyTracker.getInMemoryQueue().remove(0); // if persisted storage is too big, expel one if (parselyTracker.storedEventsCount() > STORAGE_SIZE_LIMIT) { localStorageRepository.expelStoredEvent(); diff --git a/parsely/src/test/java/com/parsely/parselyandroid/QueueManagerTest.kt b/parsely/src/test/java/com/parsely/parselyandroid/QueueManagerTest.kt new file mode 100644 index 00000000..d73bbd3f --- /dev/null +++ b/parsely/src/test/java/com/parsely/parselyandroid/QueueManagerTest.kt @@ -0,0 +1,109 @@ +package com.parsely.parselyandroid + +import androidx.test.core.app.ApplicationProvider +import com.parsely.parselyandroid.QueueManager.QUEUE_SIZE_LIMIT +import com.parsely.parselyandroid.QueueManager.STORAGE_SIZE_LIMIT +import org.assertj.core.api.Assertions.assertThat +import org.junit.Before +import org.junit.Test +import org.junit.runner.RunWith +import org.robolectric.RobolectricTestRunner +import org.robolectric.annotation.LooperMode +import org.robolectric.shadows.ShadowLooper.shadowMainLooper + +@RunWith(RobolectricTestRunner::class) +@LooperMode(LooperMode.Mode.PAUSED) +internal class QueueManagerTest { + + private lateinit var sut: QueueManager + + private val tracker = FakeTracker() + private val repository = FakeLocalRepository() + + @Before + fun setUp() { + sut = QueueManager(tracker, repository) + } + + @Test + fun `given the queue is smaller than any threshold, when querying flush manager, do nothing`() { + // given + val initialInMemoryQueue = listOf(mapOf("test" to "test")) + tracker.applyFakeQueue(initialInMemoryQueue) + + // when + sut.execute().get() + shadowMainLooper().idle(); + + // then + assertThat(tracker.inMemoryQueue).isEqualTo(initialInMemoryQueue) + assertThat(repository.storedQueue).isEmpty() + } + + @Test + fun `given the in-memory queue is above the in-memory limit, when querying flush manager, then save queue to local storage and remove first event`() { + // given + val initialInMemoryQueue = (1..QUEUE_SIZE_LIMIT + 1).map { mapOf("test" to it) } + tracker.applyFakeQueue(initialInMemoryQueue) + + // when + sut.execute().get() + shadowMainLooper().idle(); + + // then + assertThat(repository.storedQueue).isEqualTo(initialInMemoryQueue) + assertThat(tracker.inMemoryQueue).hasSize(QUEUE_SIZE_LIMIT) + } + + @Test + fun `given the in-memory queue is above the in-memory limit and stored events queue is above stored-queue limit, when querying flush manager, then expel the last event from local storage`() { + // given + val initialInMemoryQueue = (1..QUEUE_SIZE_LIMIT + 1).map { mapOf("in memory" to it) } + tracker.applyFakeQueue(initialInMemoryQueue) + val initialStoredQueue = (1..STORAGE_SIZE_LIMIT + 1).map { mapOf("storage" to it) } + repository.persistQueue(initialStoredQueue) + + // when + sut.execute().get() + shadowMainLooper().idle(); + + // then + assertThat(repository.wasEventExpelled).isTrue + } + + inner class FakeTracker : ParselyTracker( + "siteId", 10, ApplicationProvider.getApplicationContext() + ) { + + private var fakeQueue: List> = emptyList() + + internal override fun getInMemoryQueue(): List> = fakeQueue + + fun applyFakeQueue(fakeQueue: List>) { + this.fakeQueue = fakeQueue.toList() + } + + override fun storedEventsCount(): Int { + return repository.storedQueue.size + } + } + + class FakeLocalRepository : + LocalStorageRepository(ApplicationProvider.getApplicationContext()) { + + private var localFileQueue = emptyList?>() + var wasEventExpelled = false + + override fun persistQueue(inMemoryQueue: List?>) { + this.localFileQueue += inMemoryQueue + } + + override val storedQueue: ArrayList?> + get() = ArrayList(localFileQueue) + + + override fun expelStoredEvent() { + wasEventExpelled = true + } + } +} From 38e0c2e3ba32f3b9a6bfa9de9494c4315da1c595 Mon Sep 17 00:00:00 2001 From: Wojtek Zieba Date: Fri, 3 Nov 2023 15:48:44 +0100 Subject: [PATCH 11/17] style: make getStoredQueue a method --- .../parselyandroid/LocalStorageRepository.kt | 19 +++++++++---------- .../LocalStorageRepositoryTest.kt | 12 ++++++------ .../parselyandroid/QueueManagerTest.kt | 10 ++++------ 3 files changed, 19 insertions(+), 22 deletions(-) diff --git a/parsely/src/main/java/com/parsely/parselyandroid/LocalStorageRepository.kt b/parsely/src/main/java/com/parsely/parselyandroid/LocalStorageRepository.kt index 63c695ac..ac5ef795 100644 --- a/parsely/src/main/java/com/parsely/parselyandroid/LocalStorageRepository.kt +++ b/parsely/src/main/java/com/parsely/parselyandroid/LocalStorageRepository.kt @@ -33,13 +33,12 @@ internal open class LocalStorageRepository(private val context: Context) { persistObject(ArrayList>()) } - open val storedQueue: ArrayList?> - /** - * Get the stored event queue from persistent storage. - * - * @return The stored queue of events. - */ - get() { + /** + * Get the stored event queue from persistent storage. + * + * @return The stored queue of events. + */ + open fun getStoredQueue(): ArrayList?> { var storedQueue: ArrayList?>? = null try { val fis = context.applicationContext.openFileInput(STORAGE_KEY) @@ -66,7 +65,7 @@ internal open class LocalStorageRepository(private val context: Context) { * Delete an event from the stored queue. */ open fun expelStoredEvent() { - val storedQueue = storedQueue + val storedQueue = getStoredQueue() storedQueue.removeAt(0) } @@ -76,7 +75,7 @@ internal open class LocalStorageRepository(private val context: Context) { @Synchronized open fun persistQueue(inMemoryQueue: List?>) { ParselyTracker.PLog("Persisting event queue") - val storedQueue = storedQueue + val storedQueue = getStoredQueue() val hs = HashSet?>() hs.addAll(storedQueue) hs.addAll(inMemoryQueue) @@ -88,4 +87,4 @@ internal open class LocalStorageRepository(private val context: Context) { companion object { private const val STORAGE_KEY = "parsely-events.ser" } -} \ No newline at end of file +} diff --git a/parsely/src/test/java/com/parsely/parselyandroid/LocalStorageRepositoryTest.kt b/parsely/src/test/java/com/parsely/parselyandroid/LocalStorageRepositoryTest.kt index 09d255d8..2879fdab 100644 --- a/parsely/src/test/java/com/parsely/parselyandroid/LocalStorageRepositoryTest.kt +++ b/parsely/src/test/java/com/parsely/parselyandroid/LocalStorageRepositoryTest.kt @@ -29,7 +29,7 @@ class LocalStorageRepositoryTest { sut.expelStoredEvent() // then - assertThat(sut.storedQueue).hasSize(100) + assertThat(sut.getStoredQueue()).hasSize(100) } @Test @@ -41,12 +41,12 @@ class LocalStorageRepositoryTest { sut.persistQueue(eventsList) // then - assertThat(sut.storedQueue).hasSize(10).containsExactlyInAnyOrderElementsOf(eventsList) + assertThat(sut.getStoredQueue()).hasSize(10).containsExactlyInAnyOrderElementsOf(eventsList) } @Test fun `given no locally stored list, when requesting stored queue, then return an empty list`() { - assertThat(sut.storedQueue).isEmpty() + assertThat(sut.getStoredQueue()).isEmpty() } @Test @@ -61,7 +61,7 @@ class LocalStorageRepositoryTest { // then val expectedQueue = (1..10).map { mapOf("index" to it) } - assertThat(sut.storedQueue).hasSize(10).containsExactlyInAnyOrderElementsOf(expectedQueue) + assertThat(sut.getStoredQueue()).hasSize(10).containsExactlyInAnyOrderElementsOf(expectedQueue) } @Test @@ -74,7 +74,7 @@ class LocalStorageRepositoryTest { sut.purgeStoredQueue() // then - assertThat(sut.storedQueue).isEmpty() + assertThat(sut.getStoredQueue()).isEmpty() } @Test @@ -84,7 +84,7 @@ class LocalStorageRepositoryTest { File(ClassLoader.getSystemResource("valid-java-parsely-events.ser")?.path!!).copyTo(file) // when - val queue = sut.storedQueue + val queue = sut.getStoredQueue() // then assertThat(queue).isEqualTo( diff --git a/parsely/src/test/java/com/parsely/parselyandroid/QueueManagerTest.kt b/parsely/src/test/java/com/parsely/parselyandroid/QueueManagerTest.kt index d73bbd3f..309b52e6 100644 --- a/parsely/src/test/java/com/parsely/parselyandroid/QueueManagerTest.kt +++ b/parsely/src/test/java/com/parsely/parselyandroid/QueueManagerTest.kt @@ -37,7 +37,7 @@ internal class QueueManagerTest { // then assertThat(tracker.inMemoryQueue).isEqualTo(initialInMemoryQueue) - assertThat(repository.storedQueue).isEmpty() + assertThat(repository.getStoredQueue()).isEmpty() } @Test @@ -51,7 +51,7 @@ internal class QueueManagerTest { shadowMainLooper().idle(); // then - assertThat(repository.storedQueue).isEqualTo(initialInMemoryQueue) + assertThat(repository.getStoredQueue()).isEqualTo(initialInMemoryQueue) assertThat(tracker.inMemoryQueue).hasSize(QUEUE_SIZE_LIMIT) } @@ -84,7 +84,7 @@ internal class QueueManagerTest { } override fun storedEventsCount(): Int { - return repository.storedQueue.size + return repository.getStoredQueue().size } } @@ -98,9 +98,7 @@ internal class QueueManagerTest { this.localFileQueue += inMemoryQueue } - override val storedQueue: ArrayList?> - get() = ArrayList(localFileQueue) - + override fun getStoredQueue() = ArrayList(localFileQueue) override fun expelStoredEvent() { wasEventExpelled = true From 407429c787ded8639189072585e87ffccfce407f Mon Sep 17 00:00:00 2001 From: Wojtek Zieba Date: Fri, 3 Nov 2023 16:15:14 +0100 Subject: [PATCH 12/17] refactor: make getStoredQueue more concise --- .../parselyandroid/LocalStorageRepository.kt | 38 +++++++++---------- 1 file changed, 18 insertions(+), 20 deletions(-) diff --git a/parsely/src/main/java/com/parsely/parselyandroid/LocalStorageRepository.kt b/parsely/src/main/java/com/parsely/parselyandroid/LocalStorageRepository.kt index ac5ef795..dcabe195 100644 --- a/parsely/src/main/java/com/parsely/parselyandroid/LocalStorageRepository.kt +++ b/parsely/src/main/java/com/parsely/parselyandroid/LocalStorageRepository.kt @@ -39,27 +39,25 @@ internal open class LocalStorageRepository(private val context: Context) { * @return The stored queue of events. */ open fun getStoredQueue(): ArrayList?> { - var storedQueue: ArrayList?>? = null - try { - val fis = context.applicationContext.openFileInput(STORAGE_KEY) - val ois = ObjectInputStream(fis) - storedQueue = ois.readObject() as ArrayList?> - ois.close() - } catch (ex: EOFException) { - // Nothing to do here. - } catch (ex: FileNotFoundException) { - // Nothing to do here. Means there was no saved queue. - } catch (ex: Exception) { - ParselyTracker.PLog( - "Exception thrown during queue deserialization: %s", - ex.toString() - ) - } - if (storedQueue == null) { - storedQueue = ArrayList() - } - return storedQueue + var storedQueue: ArrayList?> = ArrayList() + try { + val fis = context.applicationContext.openFileInput(STORAGE_KEY) + val ois = ObjectInputStream(fis) + @Suppress("UNCHECKED_CAST") + storedQueue = ois.readObject() as ArrayList?> + ois.close() + } catch (ex: EOFException) { + // Nothing to do here. + } catch (ex: FileNotFoundException) { + // Nothing to do here. Means there was no saved queue. + } catch (ex: Exception) { + ParselyTracker.PLog( + "Exception thrown during queue deserialization: %s", + ex.toString() + ) } + return storedQueue + } /** * Delete an event from the stored queue. From 9ccb5325dcb52f6c266826f7ba3f0173ab463633 Mon Sep 17 00:00:00 2001 From: Wojtek Zieba Date: Fri, 3 Nov 2023 16:29:53 +0100 Subject: [PATCH 13/17] Rename .java to .kt --- .../parsely/parselyandroid/{QueueManager.java => QueueManager.kt} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename parsely/src/main/java/com/parsely/parselyandroid/{QueueManager.java => QueueManager.kt} (100%) diff --git a/parsely/src/main/java/com/parsely/parselyandroid/QueueManager.java b/parsely/src/main/java/com/parsely/parselyandroid/QueueManager.kt similarity index 100% rename from parsely/src/main/java/com/parsely/parselyandroid/QueueManager.java rename to parsely/src/main/java/com/parsely/parselyandroid/QueueManager.kt From 8e573f05049bc55249f8a328ac7012b76055e5df Mon Sep 17 00:00:00 2001 From: Wojtek Zieba Date: Fri, 3 Nov 2023 16:29:53 +0100 Subject: [PATCH 14/17] tests: add unit test coverage for QueueManager --- .../parsely/parselyandroid/QueueManager.kt | 46 ++++++++----------- .../parselyandroid/QueueManagerTest.kt | 4 +- 2 files changed, 20 insertions(+), 30 deletions(-) diff --git a/parsely/src/main/java/com/parsely/parselyandroid/QueueManager.kt b/parsely/src/main/java/com/parsely/parselyandroid/QueueManager.kt index c05d1fe2..6a786cc1 100644 --- a/parsely/src/main/java/com/parsely/parselyandroid/QueueManager.kt +++ b/parsely/src/main/java/com/parsely/parselyandroid/QueueManager.kt @@ -1,38 +1,28 @@ -package com.parsely.parselyandroid; +package com.parsely.parselyandroid -import android.os.AsyncTask; +import android.os.AsyncTask -import androidx.annotation.NonNull; +internal class QueueManager( + private val parselyTracker: ParselyTracker, + private val localStorageRepository: LocalStorageRepository +) : AsyncTask() { -class QueueManager extends AsyncTask { - static final int QUEUE_SIZE_LIMIT = 50; - static final int STORAGE_SIZE_LIMIT = 100; - - @NonNull - private final ParselyTracker parselyTracker; - @NonNull - private final LocalStorageRepository localStorageRepository; - - public QueueManager( - @NonNull ParselyTracker parselyTracker, - @NonNull LocalStorageRepository localStorageRepository - ) { - this.parselyTracker = parselyTracker; - this.localStorageRepository = localStorageRepository; - } - - @Override - protected Void doInBackground(Void... params) { + override fun doInBackground(vararg params: Void?): Void? { // if event queue is too big, push to persisted storage - if (parselyTracker.getInMemoryQueue().size() > QUEUE_SIZE_LIMIT) { - ParselyTracker.PLog("Queue size exceeded, expelling oldest event to persistent memory"); - localStorageRepository.persistQueue(parselyTracker.getInMemoryQueue()); - parselyTracker.getInMemoryQueue().remove(0); + if (parselyTracker.inMemoryQueue.size > QUEUE_SIZE_LIMIT) { + ParselyTracker.PLog("Queue size exceeded, expelling oldest event to persistent memory") + localStorageRepository.persistQueue(parselyTracker.inMemoryQueue) + parselyTracker.inMemoryQueue.removeAt(0) // if persisted storage is too big, expel one if (parselyTracker.storedEventsCount() > STORAGE_SIZE_LIMIT) { - localStorageRepository.expelStoredEvent(); + localStorageRepository.expelStoredEvent() } } - return null; + return null + } + + companion object { + const val QUEUE_SIZE_LIMIT = 50 + const val STORAGE_SIZE_LIMIT = 100 } } diff --git a/parsely/src/test/java/com/parsely/parselyandroid/QueueManagerTest.kt b/parsely/src/test/java/com/parsely/parselyandroid/QueueManagerTest.kt index 309b52e6..2bb00887 100644 --- a/parsely/src/test/java/com/parsely/parselyandroid/QueueManagerTest.kt +++ b/parsely/src/test/java/com/parsely/parselyandroid/QueueManagerTest.kt @@ -1,8 +1,8 @@ package com.parsely.parselyandroid import androidx.test.core.app.ApplicationProvider -import com.parsely.parselyandroid.QueueManager.QUEUE_SIZE_LIMIT -import com.parsely.parselyandroid.QueueManager.STORAGE_SIZE_LIMIT +import com.parsely.parselyandroid.QueueManager.Companion.QUEUE_SIZE_LIMIT +import com.parsely.parselyandroid.QueueManager.Companion.STORAGE_SIZE_LIMIT import org.assertj.core.api.Assertions.assertThat import org.junit.Before import org.junit.Test From ec36cd4be8b07ba32055e607f9ff7e52b18bcca0 Mon Sep 17 00:00:00 2001 From: Wojtek Zieba Date: Mon, 6 Nov 2023 15:11:28 +0100 Subject: [PATCH 15/17] refactor: simplify method --- .../com/parsely/parselyandroid/LocalStorageRepository.kt | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/parsely/src/main/java/com/parsely/parselyandroid/LocalStorageRepository.kt b/parsely/src/main/java/com/parsely/parselyandroid/LocalStorageRepository.kt index dcabe195..1f4f903b 100644 --- a/parsely/src/main/java/com/parsely/parselyandroid/LocalStorageRepository.kt +++ b/parsely/src/main/java/com/parsely/parselyandroid/LocalStorageRepository.kt @@ -73,13 +73,7 @@ internal open class LocalStorageRepository(private val context: Context) { @Synchronized open fun persistQueue(inMemoryQueue: List?>) { ParselyTracker.PLog("Persisting event queue") - val storedQueue = getStoredQueue() - val hs = HashSet?>() - hs.addAll(storedQueue) - hs.addAll(inMemoryQueue) - storedQueue.clear() - storedQueue.addAll(hs) - persistObject(storedQueue) + persistObject((inMemoryQueue + getStoredQueue()).distinct()) } companion object { From 3034d23447beb0da6f83db9467f79690198e7022 Mon Sep 17 00:00:00 2001 From: Wojtek Zieba Date: Mon, 6 Nov 2023 15:22:03 +0100 Subject: [PATCH 16/17] style: add deprecation annotations --- .../src/main/java/com/parsely/parselyandroid/QueueManager.kt | 2 ++ .../test/java/com/parsely/parselyandroid/QueueManagerTest.kt | 1 + 2 files changed, 3 insertions(+) diff --git a/parsely/src/main/java/com/parsely/parselyandroid/QueueManager.kt b/parsely/src/main/java/com/parsely/parselyandroid/QueueManager.kt index 6a786cc1..59d5401b 100644 --- a/parsely/src/main/java/com/parsely/parselyandroid/QueueManager.kt +++ b/parsely/src/main/java/com/parsely/parselyandroid/QueueManager.kt @@ -2,11 +2,13 @@ package com.parsely.parselyandroid import android.os.AsyncTask +@Suppress("DEPRECATION") internal class QueueManager( private val parselyTracker: ParselyTracker, private val localStorageRepository: LocalStorageRepository ) : AsyncTask() { + @Deprecated("Deprecated in Java") override fun doInBackground(vararg params: Void?): Void? { // if event queue is too big, push to persisted storage if (parselyTracker.inMemoryQueue.size > QUEUE_SIZE_LIMIT) { diff --git a/parsely/src/test/java/com/parsely/parselyandroid/QueueManagerTest.kt b/parsely/src/test/java/com/parsely/parselyandroid/QueueManagerTest.kt index 2bb00887..86613295 100644 --- a/parsely/src/test/java/com/parsely/parselyandroid/QueueManagerTest.kt +++ b/parsely/src/test/java/com/parsely/parselyandroid/QueueManagerTest.kt @@ -11,6 +11,7 @@ import org.robolectric.RobolectricTestRunner import org.robolectric.annotation.LooperMode import org.robolectric.shadows.ShadowLooper.shadowMainLooper +@Suppress("DEPRECATION") @RunWith(RobolectricTestRunner::class) @LooperMode(LooperMode.Mode.PAUSED) internal class QueueManagerTest { From 1b8ffca4719f7eded1ba1222a9f0fade8b133b5a Mon Sep 17 00:00:00 2001 From: Wojtek Zieba Date: Mon, 6 Nov 2023 15:46:41 +0100 Subject: [PATCH 17/17] docs: update coordinates to STORAGE_SIZE_LIMIT const --- .../main/java/com/parsely/parselyandroid/ParselyTracker.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/parsely/src/main/java/com/parsely/parselyandroid/ParselyTracker.java b/parsely/src/main/java/com/parsely/parselyandroid/ParselyTracker.java index e4d61c2f..609ed087 100644 --- a/parsely/src/main/java/com/parsely/parselyandroid/ParselyTracker.java +++ b/parsely/src/main/java/com/parsely/parselyandroid/ParselyTracker.java @@ -411,7 +411,7 @@ public void resetVideo() { * Place a data structure representing the event into the in-memory queue for later use. *

* **Note**: Events placed into this queue will be discarded if the size of the persistent queue - * store exceeds {@link #STORAGE_SIZE_LIMIT}. + * store exceeds {@link QueueManager#STORAGE_SIZE_LIMIT}. * * @param event The event Map to enqueue. */